Use System.Security.Cryptography for RSA#1373
Merged
WojciechNagorski merged 7 commits intosshnet:developfrom May 10, 2024
Merged
Use System.Security.Cryptography for RSA#1373WojciechNagorski merged 7 commits intosshnet:developfrom
WojciechNagorski merged 7 commits intosshnet:developfrom
Conversation
Collaborator
Author
|
After this, I will do something similar for DSA, and then switch BigInteger over to the BCL |
Collaborator
|
One other thing: we can refer System.Memory nuget package when target to net462 and netstandard2.0 so that the code can use |
Collaborator
Author
|
Yes, it is on my slow-moving todo list, perhaps starting with reviving #1160. Feel free to get it started, I probably won't work on it in the immediate future |
scott-xu
approved these changes
Apr 16, 2024
src/Renci.SshNet/Security/Cryptography/CipherDigitalSignature.cs
Outdated
Show resolved
Hide resolved
WojciechNagorski
approved these changes
May 10, 2024
Rob-Hague
added a commit
to Rob-Hague/SSH.NET
that referenced
this pull request
Jul 27, 2024
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
Rob-Hague
added a commit
to Rob-Hague/SSH.NET
that referenced
this pull request
Aug 1, 2024
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
Rob-Hague
added a commit
to Rob-Hague/SSH.NET
that referenced
this pull request
Aug 10, 2024
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
Rob-Hague
added a commit
to Rob-Hague/SSH.NET
that referenced
this pull request
Aug 10, 2024
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
WojciechNagorski
pushed a commit
that referenced
this pull request
Aug 11, 2024
* Use System.Security.Cryptography for DSA This is the analogue of the RSA change #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 * Appease mono * test experiment * Revert "Appease mono" This reverts commit 881eefe.
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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.
And remove the hand-rolled implementation
Before:
After:
closes #1388