Code review feedback

dotnet · Oct 22, 2024 · 3811dde · 3811dde
1 parent a1a5be2
commit 3811dde
Show file tree

Hide file tree

Showing 9 changed files with 24 additions and 32 deletions.
diff --git a/src/libraries/Common/src/System/Security/Cryptography/DSACng.ImportExport.cs b/src/libraries/Common/src/System/Security/Cryptography/DSACng.ImportExport.cs
@@ -310,7 +310,9 @@ private static unsafe void GenerateV2DsaBlob(out byte[] blob, DSAParameters para
 
         public override DSAParameters ExportParameters(bool includePrivateParameters)
         {
-            if (includePrivateParameters && EncryptedOnlyExport)
+            bool encryptedOnlyExport = CngPkcs8.AllowsOnlyEncryptedExport(Key);
+
+            if (includePrivateParameters && encryptedOnlyExport)
             {
                 const string TemporaryExportPassword = "DotnetExportPhrase";
                 byte[] exported = ExportEncryptedPkcs8(TemporaryExportPassword, 1);
@@ -435,14 +437,5 @@ private static void CheckMagicValueOfKey(KeyBlobMagicNumber magic, bool includeP
                     throw new CryptographicException(SR.Cryptography_NotValidPublicOrPrivateKey);
             }
         }
-
-        private bool EncryptedOnlyExport
-        {
-            get
-            {
-                const CngExportPolicies Exportable = CngExportPolicies.AllowPlaintextExport | CngExportPolicies.AllowExport;
-                return (Key.ExportPolicy & Exportable) == CngExportPolicies.AllowExport;
-            }
-        }
     }
 }
diff --git a/src/libraries/Common/src/System/Security/Cryptography/RSACng.ImportExport.cs b/src/libraries/Common/src/System/Security/Cryptography/RSACng.ImportExport.cs
@@ -180,7 +180,9 @@ public override bool TryExportEncryptedPkcs8PrivateKey(
         /// </summary>
         public override RSAParameters ExportParameters(bool includePrivateParameters)
         {
-            if (includePrivateParameters && EncryptedOnlyExport)
+            bool encryptedOnlyExport = CngPkcs8.AllowsOnlyEncryptedExport(Key);
+
+            if (includePrivateParameters && encryptedOnlyExport)
             {
                 const string TemporaryExportPassword = "DotnetExportPhrase";
                 byte[] exported = ExportEncryptedPkcs8(TemporaryExportPassword, 1);
@@ -197,14 +199,5 @@ public override RSAParameters ExportParameters(bool includePrivateParameters)
             rsaParams.FromBCryptBlob(rsaBlob, includePrivateParameters);
             return rsaParams;
         }
-
-        private bool EncryptedOnlyExport
-        {
-            get
-            {
-                const CngExportPolicies Exportable = CngExportPolicies.AllowPlaintextExport | CngExportPolicies.AllowExport;
-                return (Key.ExportPolicy & Exportable) == CngExportPolicies.AllowExport;
-            }
-        }
     }
 }
diff --git a/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/CngPkcs8.cs b/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/CngPkcs8.cs
@@ -43,5 +43,11 @@ private static Pkcs8Response ImportPkcs8(
                 Key = key,
             };
         }
+
+        internal static bool AllowsOnlyEncryptedExport(CngKey key)
+        {
+            const CngExportPolicies Exportable = CngExportPolicies.AllowPlaintextExport | CngExportPolicies.AllowExport;
+            return (key.ExportPolicy & Exportable) == CngExportPolicies.AllowExport;
+        }
     }
 }
diff --git a/...ries/System.Security.Cryptography/src/System/Security/Cryptography/DSACng.ImportExport.cs b/...ries/System.Security.Cryptography/src/System/Security/Cryptography/DSACng.ImportExport.cs
@@ -50,7 +50,9 @@ private void AcceptImport(CngPkcs8.Pkcs8Response response)
 
         public override bool TryExportPkcs8PrivateKey(Span<byte> destination, out int bytesWritten)
         {
-             if (EncryptedOnlyExport)
+            bool encryptedOnlyExport = CngPkcs8.AllowsOnlyEncryptedExport(Key);
+
+            if (encryptedOnlyExport)
             {
                 const string TemporaryExportPassword = "DotnetExportPhrase";
                 byte[] exported = ExportEncryptedPkcs8(TemporaryExportPassword, 1);

diff --git a/...aries/System.Security.Cryptography/src/System/Security/Cryptography/ECCng.ImportExport.cs b/...aries/System.Security.Cryptography/src/System/Security/Cryptography/ECCng.ImportExport.cs
@@ -95,8 +95,6 @@ internal static ECParameters ExportExplicitParameters(CngKey key, bool includePr
 
         internal static ECParameters ExportParameters(CngKey key, bool includePrivateParameters)
         {
-            const CngExportPolicies Exportable = CngExportPolicies.AllowPlaintextExport | CngExportPolicies.AllowExport;
-            bool encryptedOnlyExport = (key.ExportPolicy & Exportable) == CngExportPolicies.AllowExport;
             ECParameters ecparams = default;
 
             const string TemporaryExportPassword = "DotnetExportPhrase";
@@ -116,6 +114,8 @@ internal static ECParameters ExportParameters(CngKey key, bool includePrivatePar
             }
             else
             {
+                bool encryptedOnlyExport = CngPkcs8.AllowsOnlyEncryptedExport(key);
+
                 if (includePrivateParameters && encryptedOnlyExport)
                 {
                     byte[] exported = key.ExportPkcs8KeyBlob(TemporaryExportPassword, 1);
@@ -138,8 +138,7 @@ internal static ECParameters ExportParameters(CngKey key, bool includePrivatePar
 
         private static ECParameters ExportPrivateExplicitParameters(CngKey key)
         {
-            const CngExportPolicies Exportable = CngExportPolicies.AllowPlaintextExport | CngExportPolicies.AllowExport;
-            bool encryptedOnlyExport = (key.ExportPolicy & Exportable) == CngExportPolicies.AllowExport;
+            bool encryptedOnlyExport = CngPkcs8.AllowsOnlyEncryptedExport(key);
 
             ECParameters ecparams = default;
 

diff --git a/...aries/System.Security.Cryptography/src/System/Security/Cryptography/ECDiffieHellmanCng.cs b/...aries/System.Security.Cryptography/src/System/Security/Cryptography/ECDiffieHellmanCng.cs
@@ -206,8 +206,7 @@ private void AcceptImport(CngPkcs8.Pkcs8Response response)
 
         public override bool TryExportPkcs8PrivateKey(Span<byte> destination, out int bytesWritten)
         {
-            const CngExportPolicies Exportable = CngExportPolicies.AllowPlaintextExport | CngExportPolicies.AllowExport;
-            bool encryptedOnlyExport = (Key.ExportPolicy & Exportable) == CngExportPolicies.AllowExport;
+            bool encryptedOnlyExport = CngPkcs8.AllowsOnlyEncryptedExport(Key);
 
             if (encryptedOnlyExport)
             {

diff --git a/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/ECDsaCng.cs b/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/ECDsaCng.cs
@@ -151,8 +151,7 @@ private void AcceptImport(CngPkcs8.Pkcs8Response response)
 
         public override bool TryExportPkcs8PrivateKey(Span<byte> destination, out int bytesWritten)
         {
-            const CngExportPolicies Exportable = CngExportPolicies.AllowPlaintextExport | CngExportPolicies.AllowExport;
-            bool encryptedOnlyExport = (Key.ExportPolicy & Exportable) == CngExportPolicies.AllowExport;
+            bool encryptedOnlyExport = CngPkcs8.AllowsOnlyEncryptedExport(Key);
 
             if (encryptedOnlyExport)
             {

diff --git a/...ries/System.Security.Cryptography/src/System/Security/Cryptography/RSACng.ImportExport.cs b/...ries/System.Security.Cryptography/src/System/Security/Cryptography/RSACng.ImportExport.cs
@@ -52,7 +52,9 @@ private byte[] ExportKeyBlob(bool includePrivateParameters)
 
         public override bool TryExportPkcs8PrivateKey(Span<byte> destination, out int bytesWritten)
         {
-            if (EncryptedOnlyExport)
+            bool encryptedOnlyExport = CngPkcs8.AllowsOnlyEncryptedExport(Key);
+
+            if (encryptedOnlyExport)
             {
                 const string TemporaryExportPassword = "DotnetExportPhrase";
                 byte[] exported = ExportEncryptedPkcs8(TemporaryExportPassword, 1);

diff --git a/src/libraries/System.Security.Cryptography/tests/X509Certificates/ExportTests.cs b/src/libraries/System.Security.Cryptography/tests/X509Certificates/ExportTests.cs
@@ -539,8 +539,7 @@ private static (byte[] Pkcs12, TKey key) CreateSimplePkcs12<TKey>() where TKey :
                     ca,
                     HashAlgorithmName.SHA256);
 
-                issuerRequest.CertificateExtensions.Add(
-                    new X509BasicConstraintsExtension(true, false, 0, true));
+                issuerRequest.CertificateExtensions.Add(X509BasicConstraintsExtension.CreateForCertificateAuthority());
 
                 DateTimeOffset notBefore = DateTimeOffset.UtcNow;
                 DateTimeOffset notAfter = notBefore.AddDays(30);