Use System.Security.Cryptography for DSA #1458
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This is the analogue of the RSA change sshnet#1373 for DSA. This has a couple of caveats: - The BCL supports only FIPS 186-compliant keys, that is, public (P, Q) lengths of (512 <= P <= 1024, 160) for FIPS 186-1/186-2; and (2048, 256), (3072, 256) for FIPS 186-3/186-4. The latter also specifies (2048, 224) but due to a quirk in the Windows API, the BCL does not support Q values of length 224[^1]. - OpenSSH, based on the SSH spec, only supports (supported) Q values of length 160, but appears to also work in non-FIPS-compliant cases such as in our integration tests with a (2048, 160) host key. That test now fails and I changed that host key to (1024, 160). This basically means that (1024, 160) is the largest DSA key size supported by both SSH.NET and OpenSSH. However, given that OpenSSH deprecated DSA in 2015[^2], and the alternative that I have been considering is just to delete support for DSA in the library, this change seems reasonable to me. I don't think we can justify keeping the current handwritten code around. I think we may still consider dropping DSA from the library, I just had this branch laying around and figured I'd finish it off. [^1]: https://github.com/dotnet/runtime/blob/fadd8313653f71abd0068c8bf914be88edb2c8d3/src/libraries/Common/src/System/Security/Cryptography/DSACng.ImportExport.cs#L259-L265 [^2]: https://www.openssh.com/txt/release-7.0
|
Naturally, running .NET Framework tests on Linux struggles with Windows CNG. I think we need a different approach to the tests |
|
So I think we should give up on the "mono on .NET Framework" scenario. An interesting test but ultimately a waste of precious time trying to support dead technology. Until we find a proper way to run the integration tests for .NET Framework, we can stick to running them locally. |
This reverts commit 881eefe.
|
Ok it's back to green |
WojciechNagorski
approved these changes
Aug 11, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is the analogue of the RSA change #1373 for DSA. This has a couple of caveats:
This basically means that (1024, 160) is the largest DSA key size supported by both SSH.NET and OpenSSH. However, given that OpenSSH deprecated DSA in 20152, and the alternative that I have been considering is just to delete support for DSA in the library, this change seems reasonable to me. I don't think we can justify keeping the current handwritten code around.
I think we may still consider dropping DSA from the library, I just had this branch laying around and figured I'd finish it off.
Footnotes
https://github.com/dotnet/runtime/blob/fadd8313653f71abd0068c8bf914be88edb2c8d3/src/libraries/Common/src/System/Security/Cryptography/DSACng.ImportExport.cs#L259-L265 ↩
https://www.openssh.com/txt/release-7.0 ↩