From a6877af532aa062fa554d488a88b0f941cc449d0 Mon Sep 17 00:00:00 2001 From: Kevin Jones Date: Thu, 23 Feb 2023 18:40:17 -0500 Subject: [PATCH 1/4] Implement RSA.GetMaxOutputSize --- .../ref/System.Security.Cryptography.cs | 1 + .../src/System/Security/Cryptography/RSA.cs | 26 +++++++++- .../RSACryptoServiceProvider.Windows.cs | 2 +- .../tests/RSATests.cs | 49 +++++++++++++++++++ 4 files changed, 76 insertions(+), 2 deletions(-) diff --git a/src/libraries/System.Security.Cryptography/ref/System.Security.Cryptography.cs b/src/libraries/System.Security.Cryptography/ref/System.Security.Cryptography.cs index 914f180f5df61d..4e4e3e4313aa08 100644 --- a/src/libraries/System.Security.Cryptography/ref/System.Security.Cryptography.cs +++ b/src/libraries/System.Security.Cryptography/ref/System.Security.Cryptography.cs @@ -1873,6 +1873,7 @@ protected RSA() { } public virtual byte[] ExportRSAPublicKey() { throw null; } public string ExportRSAPublicKeyPem() { throw null; } public override void FromXmlString(string xmlString) { } + public int GetMaxOutputSize() { throw null; } protected virtual byte[] HashData(byte[] data, int offset, int count, System.Security.Cryptography.HashAlgorithmName hashAlgorithm) { throw null; } protected virtual byte[] HashData(System.IO.Stream data, System.Security.Cryptography.HashAlgorithmName hashAlgorithm) { throw null; } public override void ImportEncryptedPkcs8PrivateKey(System.ReadOnlySpan passwordBytes, System.ReadOnlySpan source, out int bytesRead) { throw null; } diff --git a/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/RSA.cs b/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/RSA.cs index 66b19175eca8be..8e9c9118434db2 100644 --- a/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/RSA.cs +++ b/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/RSA.cs @@ -59,6 +59,30 @@ public static RSA Create(RSAParameters parameters) } } + /// + /// The maximum output of an RSA operation, in bytes. + /// + /// + /// The maximum output size is defined by the RSA modulus, or key size. The key size, in bytes, is the maximum + /// output size. If the key size is not an even number of bytes, then it is rounded up to the nearest number of + /// whole bytes for purposes of determining the maximum output size. + /// + /// + /// returned a value that is not a possible RSA key size. + /// + public int GetMaxOutputSize() + { + if (KeySize <= 0) + { + throw new CryptographicException(SR.Cryptography_InvalidKeySize); + } + + // KeySize is in bits. Add 7 to round up to nearest byte, then divide by 8. + // There is no reality in which we will have a 2 GB RSA key. However, since KeySize is virtual, + // perform an unsigned shift so that we end up with the right value if the addition overflows. + return (KeySize + 7) >>> 3; + } + public abstract RSAParameters ExportParameters(bool includePrivateParameters); public abstract void ImportParameters(RSAParameters parameters); public virtual byte[] Encrypt(byte[] data, RSAEncryptionPadding padding) => throw DerivedClassMustOverride(); @@ -1384,7 +1408,7 @@ private byte[] TryWithKeyBuffer( // In normal circumstances, the signing and encryption size is the key size. // In the case of decryption, it will be at most the size of the key, but the final output size is not // deterministic, so start with the key size. - int resultSize = (KeySize + 7) / 8; + int resultSize = GetMaxOutputSize(); int written; // For scenarios where we are confident that we can get the output side right on the first try, we allocate diff --git a/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/RSACryptoServiceProvider.Windows.cs b/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/RSACryptoServiceProvider.Windows.cs index 53d88f8818c261..5a7655933f5b62 100644 --- a/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/RSACryptoServiceProvider.Windows.cs +++ b/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/RSACryptoServiceProvider.Windows.cs @@ -322,7 +322,7 @@ public byte[] Encrypt(byte[] rgb, bool fOAEP) if (fOAEP) { - int rsaSize = (KeySize + 7) / 8; + int rsaSize = GetMaxOutputSize(); const int OaepSha1Overhead = 20 + 20 + 2; // Normalize the Windows 7 and Windows 8.1+ exception diff --git a/src/libraries/System.Security.Cryptography/tests/RSATests.cs b/src/libraries/System.Security.Cryptography/tests/RSATests.cs index f484aca578297b..7b55391916c587 100644 --- a/src/libraries/System.Security.Cryptography/tests/RSATests.cs +++ b/src/libraries/System.Security.Cryptography/tests/RSATests.cs @@ -1125,6 +1125,37 @@ static bool TryDecrypt( } } + [Theory] + [InlineData(1, 1)] + [InlineData(1024, 128)] + [InlineData(1025, 129)] + [InlineData(1031, 129)] + [InlineData(1032, 129)] + [InlineData(2048, 256)] + [InlineData(3072, 384)] + [InlineData(int.MaxValue, 268_435_456)] + public static void GetMaxOutputSize_IsModulusSizeToNearestByte(int keySize, int expectedMaxOutputSize) + { + using (DelegateRSA rsa = new DelegateRSA()) + { + rsa.KeySizeGetDelegate = () => keySize; + Assert.Equal(expectedMaxOutputSize, rsa.GetMaxOutputSize()); + } + } + + [Theory] + [InlineData(0)] + [InlineData(-1)] + [InlineData(int.MinValue)] + public static void GetMaxOutputSize_InvalidKeySizes(int keySize) + { + using (DelegateRSA rsa = new DelegateRSA()) + { + rsa.KeySizeGetDelegate = () => keySize; + Assert.Throws(() => rsa.GetMaxOutputSize()); + } + } + private sealed class EmptyRSA : RSA { public override RSAParameters ExportParameters(bool includePrivateParameters) => throw new NotImplementedException(); @@ -1180,6 +1211,8 @@ public delegate bool TryDecryptFunc( public TrySignHashFunc TrySignHashDelegate = null; public TryEncryptFunc TryEncryptDelegate = null; public TryDecryptFunc TryDecryptDelegate = null; + public Func KeySizeGetDelegate = null; + public Action KeySizeSetDelegate = null; public override byte[] Encrypt(byte[] data, RSAEncryptionPadding padding) => EncryptDelegate(data, padding); @@ -1263,6 +1296,22 @@ public override bool TryDecrypt(ReadOnlySpan data, Span destination, public override RSAParameters ExportParameters(bool includePrivateParameters) => throw new NotImplementedException(); public override void ImportParameters(RSAParameters parameters) => throw new NotImplementedException(); + + public override int KeySize + { + get => KeySizeGetDelegate is not null ? KeySizeGetDelegate() : base.KeySize; + set + { + if (KeySizeSetDelegate is not null) + { + KeySizeSetDelegate(value); + } + else + { + base.KeySize = value; + } + } + } } } } From f64433ac5de5bb9ae8e851e3e0d0bb7d2bde615e Mon Sep 17 00:00:00 2001 From: Kevin Jones Date: Thu, 23 Feb 2023 18:42:42 -0500 Subject: [PATCH 2/4] Try to improve docs --- .../src/System/Security/Cryptography/RSA.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/RSA.cs b/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/RSA.cs index 8e9c9118434db2..b9f197f843a511 100644 --- a/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/RSA.cs +++ b/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/RSA.cs @@ -60,7 +60,7 @@ public static RSA Create(RSAParameters parameters) } /// - /// The maximum output of an RSA operation, in bytes. + /// The maximum number of bytes an RSA operation can produce. /// /// /// The maximum output size is defined by the RSA modulus, or key size. The key size, in bytes, is the maximum From ba673b1e5b3cf3174b6b3371473d5b7a7f3e0fc0 Mon Sep 17 00:00:00 2001 From: Kevin Jones Date: Thu, 23 Feb 2023 20:32:38 -0500 Subject: [PATCH 3/4] Apply suggestions from code review Co-authored-by: Jeremy Barton --- .../src/System/Security/Cryptography/RSA.cs | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/RSA.cs b/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/RSA.cs index b9f197f843a511..833abbd3856af0 100644 --- a/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/RSA.cs +++ b/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/RSA.cs @@ -60,8 +60,11 @@ public static RSA Create(RSAParameters parameters) } /// - /// The maximum number of bytes an RSA operation can produce. + /// Gets the maximum number of bytes an RSA operation can produce. /// + /// + /// The maximum number of bytes an RSA operation can produce. + /// /// /// The maximum output size is defined by the RSA modulus, or key size. The key size, in bytes, is the maximum /// output size. If the key size is not an even number of bytes, then it is rounded up to the nearest number of @@ -77,7 +80,7 @@ public int GetMaxOutputSize() throw new CryptographicException(SR.Cryptography_InvalidKeySize); } - // KeySize is in bits. Add 7 to round up to nearest byte, then divide by 8. + // KeySize is in bits. Add 7 before dividing by 8 to get ceil() instead of floor(). // There is no reality in which we will have a 2 GB RSA key. However, since KeySize is virtual, // perform an unsigned shift so that we end up with the right value if the addition overflows. return (KeySize + 7) >>> 3; From c40f5cf655455eb0b343b43cbeaf8e76bfb0a235 Mon Sep 17 00:00:00 2001 From: Kevin Jones Date: Thu, 23 Feb 2023 20:35:57 -0500 Subject: [PATCH 4/4] Replace BitsToBytes with GetMaxOutputSize where possible --- .../src/System/Security/Cryptography/RSABCrypt.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/RSABCrypt.cs b/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/RSABCrypt.cs index 4daea665de19f7..c5ac24ab029b2e 100644 --- a/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/RSABCrypt.cs +++ b/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/RSABCrypt.cs @@ -120,7 +120,7 @@ public override byte[] Encrypt(byte[] data, RSAEncryptionPadding padding) ArgumentNullException.ThrowIfNull(data); ArgumentNullException.ThrowIfNull(padding); - byte[] ret = new byte[AsymmetricAlgorithmHelpers.BitsToBytes(KeySize)]; + byte[] ret = new byte[GetMaxOutputSize()]; int written = Encrypt(new ReadOnlySpan(data), ret.AsSpan(), padding); VerifyWritten(ret, written); @@ -144,7 +144,7 @@ public override byte[] SignHash( ArgumentException.ThrowIfNullOrEmpty(hashAlgorithm.Name, nameof(hashAlgorithm)); ArgumentNullException.ThrowIfNull(padding); - byte[] ret = new byte[AsymmetricAlgorithmHelpers.BitsToBytes(KeySize)]; + byte[] ret = new byte[GetMaxOutputSize()]; int written = SignHash( new ReadOnlySpan(hash),