-
Notifications
You must be signed in to change notification settings - Fork 68
Turn off full signing on non-Windows platforms #566
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
crummel
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One small fix.
| <KeyFileDir>$(RepoRoot)src/referencePackages/snk/</KeyFileDir> | ||
| <SignAssembly>true</SignAssembly> | ||
| <PublicSign>true</PublicSign> | ||
| <FullAssemblySigningSupported Condition="('$(FullAssemblySigningSupported) == '') and ('$(OS)' != 'Windows_NT')">false</FullAssemblySigningSupported> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| <FullAssemblySigningSupported Condition="('$(FullAssemblySigningSupported) == '') and ('$(OS)' != 'Windows_NT')">false</FullAssemblySigningSupported> | |
| <FullAssemblySigningSupported Condition="('$(FullAssemblySigningSupported)' == '') and ('$(OS)' != 'Windows_NT')">false</FullAssemblySigningSupported> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks! You were too fast with the review :)
Full Signing doesn't work on some platforms (eg RHEL 9, which disables RSA+SHA1 which Full Signing needs). Full Signing is also really on required for .NET Framework support. It should not be needed on modern .NET (5 and later). More details at dotnet/runtime#65874
8ccc80c to
f603646
Compare
Full Signing requires RSA+SHA1 which is disabled in some environments (eg, RHEL 9, CentOS Stream 9). Expose a top-level flag to allow that to be disabled easily by users. The actual implementation of that flag is not here; it's in arcade (see dotnet/arcade#12749). Some repos have some nice-to-have fixes (eg, dotnet/source-build-reference-packages#566), but it's mostly optional. The bootstrap arcade SDK being used to build this needs needs to contain the changes for us to be able to build SBRP and then arcade and then all the other repos that will use arcade. For more context around RSA+SHA1 and the alternative (using public signing), see: - dotnet/runtime#65874 - dotnet/source-build#3202 - dotnet/arcade#12515
Full Signing requires RSA+SHA1 which is disabled in some environments (eg, RHEL 9, CentOS Stream 9). Default to turning it off. Expose a top-level property to allow Full Signing to be re-enabled by users. The actual implementation of that flag is not here; it's in arcade (see dotnet/arcade#12749). Some repos have some nice-to-have fixes (eg, dotnet/source-build-reference-packages#566), but it's mostly optional. The bootstrap arcade SDK being used to build this needs needs to contain the changes for us to be able to build SBRP and arcade; then all the other repos that will use the just-built arcade. For more context around RSA+SHA1 and the alternative (using public signing), see: - dotnet/runtime#65874 - dotnet/source-build#3202 - dotnet/arcade#12515
Full Signing doesn't work on some platforms (eg RHEL 9, which disables RSA+SHA1 which Full Signing needs). Full Signing is also really on required for .NET Framework support. It should not be needed on modern .NET (5 and later).
More details at dotnet/runtime#65874