Make properties on S.S.C.Oid effectively init-only

Do not permit OID properties to be changed after they have
been set to a non-null value.

diff --git a/src/libraries/System.Security.Cryptography.Encoding/src/Resources/Strings.resx b/src/libraries/System.Security.Cryptography.Encoding/src/Resources/Strings.resx
index 3b48346..d7ce70c 100644 (file)
--- a/src/libraries/System.Security.Cryptography.Encoding/src/Resources/Strings.resx
+++ b/src/libraries/System.Security.Cryptography.Encoding/src/Resources/Strings.resx
@@ -63,6 +63,12 @@
   <data name="Cryptography_Oid_InvalidName" xml:space="preserve">
     <value>No OID value matches this name.</value>
   </data>
+  <data name="Cryptography_Oid_SetOnceValue" xml:space="preserve">
+    <value>The OID value cannot be changed from its current value.</value>
+  </data>
+  <data name="Cryptography_Oid_SetOnceFriendlyName" xml:space="preserve">
+    <value>The OID friendly name cannot be changed from its current value.</value>
+  </data>
   <data name="Cryptography_SSE_InvalidDataSize" xml:space="preserve">
     <value>NoLength of the data to encrypt is invalid.</value>
   </data>

diff --git a/src/libraries/System.Security.Cryptography.Encoding/src/System/Security/Cryptography/Oid.cs b/src/libraries/System.Security.Cryptography.Encoding/src/System/Security/Cryptography/Oid.cs
index c22390b..19245f3 100644 (file)
--- a/src/libraries/System.Security.Cryptography.Encoding/src/System/Security/Cryptography/Oid.cs
+++ b/src/libraries/System.Security.Cryptography.Encoding/src/System/Security/Cryptography/Oid.cs
@@ -69,8 +69,18 @@ namespace System.Security.Cryptography
 
         public string? Value
         {
-            get { return _value; }
-            set { _value = value; }
+            get => _value;
+            set
+            {
+                // If _value has not been set, permit it to be set once, or to
+                // the same value for "initialize once" behavior.
+                if (_value != null && !_value.Equals(value, StringComparison.Ordinal))
+                {
+                    throw new PlatformNotSupportedException(SR.Cryptography_Oid_SetOnceValue);
+                }
+
+                _value = value;
+            }
         }
 
         public string? FriendlyName
@@ -86,16 +96,40 @@ namespace System.Security.Cryptography
             }
             set
             {
-                _friendlyName = value;
+                // If _friendlyName has not been set, permit it to be set once, or to
+                // the same value for "initialize once" behavior.
+                if (_friendlyName != null && !_friendlyName.Equals(value, StringComparison.Ordinal))
+                {
+                    throw new PlatformNotSupportedException(SR.Cryptography_Oid_SetOnceFriendlyName);
+                }
+
                 // If we can find the matching OID value, then update it as well
-                if (_friendlyName != null)
+                if (_friendlyName == null && value != null)
                 {
                     // If FindOidInfo fails, we return a null String
-                    string? oidValue = OidLookup.ToOid(_friendlyName, _group, fallBackToAllGroups: true);
+                    string? oidValue = OidLookup.ToOid(value, _group, fallBackToAllGroups: true);
+
                     if (oidValue != null)
                     {
-                        _value = oidValue;
+                        // If the OID value has not been initialized, set it
+                        // to the lookup value.
+                        if (_value == null)
+                        {
+                            _value = oidValue;
+                        }
+
+                        // The friendly name resolves to an OID value other than the
+                        // current one, which is not permitted under "initialize once"
+                        // behavior.
+                        else if (!_value.Equals(oidValue, StringComparison.Ordinal))
+                        {
+                            throw new PlatformNotSupportedException(SR.Cryptography_Oid_SetOnceValue);
+                        }
                     }
+
+                    // Ensure we don't mutate _friendlyName until we are sure we can
+                    // set _value if we are going to.
+                    _friendlyName = value;
                 }
             }
         }

diff --git a/src/libraries/System.Security.Cryptography.Encoding/tests/Oid.cs b/src/libraries/System.Security.Cryptography.Encoding/tests/Oid.cs
index cafaa03..f15b3c0 100644 (file)
--- a/src/libraries/System.Security.Cryptography.Encoding/tests/Oid.cs
+++ b/src/libraries/System.Security.Cryptography.Encoding/tests/Oid.cs
@@ -116,49 +116,66 @@ namespace System.Security.Cryptography.Encoding.Tests
         {
             Oid oid = new Oid(null, null);
 
-            // Value property is just a field exposed as a property - no extra policy at all.
+            // Should allow setting null
+            oid.Value = null;
+
+            // Value property permits initializing once if null, or to its current value
 
             oid.Value = "BOGUS";
             Assert.Equal("BOGUS", oid.Value);
 
-            oid.Value = null;
-            Assert.Null(oid.Value);
+            Assert.Throws<PlatformNotSupportedException>(() => oid.Value = null);
+            Assert.Throws<PlatformNotSupportedException>(() => oid.Value = "SUGOB");
+            Assert.Throws<PlatformNotSupportedException>(() => oid.Value = "bogus");
+
+            oid.Value = "BOGUS";
+            Assert.Equal("BOGUS", oid.Value);
         }
 
         [Fact]
         public static void TestFriendlyNameProperty()
         {
-            Oid oid;
+            // Setting the FriendlyName can be initialized once, and may update
+            // the value as well. If the value updates, it must match the current
+            // value, if any.
 
-            oid = new Oid(null, null);
-
-            // Friendly name property can initialize itself from the Value (but only
-            // if it was originally null.)
-
-            oid.Value = SHA1_Oid;
-            Assert.Equal(SHA1_Name, oid.FriendlyName);
-
-            oid.Value = SHA256_Oid;
+            Oid oid = new Oid
+            {
+                FriendlyName = SHA1_Name
+            };
             Assert.Equal(SHA1_Name, oid.FriendlyName);
+            Assert.Equal(SHA1_Oid, oid.Value);
 
-            oid.Value = null;
-            Assert.Equal(SHA1_Name, oid.FriendlyName);
+            Assert.Throws<PlatformNotSupportedException>(() => oid.FriendlyName = "BOGUS");
 
-            oid.Value = Bogus_Name;
+            oid.FriendlyName = SHA1_Name;
             Assert.Equal(SHA1_Name, oid.FriendlyName);
+        }
 
-            // Setting the FriendlyName can also updates the value if there a valid OID for the new name.
-            oid.FriendlyName = Bogus_Name;
-            Assert.Equal(Bogus_Name, oid.FriendlyName);
-            Assert.Equal(Bogus_Name, oid.Value);
+        [Fact]
+        public static void TestFriendlyNameWithExistingValue()
+        {
+            Oid oid = new Oid(SHA1_Oid, null);
+            Assert.Equal(SHA1_Oid, oid.Value);
+            // Do not assert the friendly name here since the getter will populate it
 
-            oid.FriendlyName = SHA1_Name;
-            Assert.Equal(SHA1_Name, oid.FriendlyName);
+            // FriendlyName will attempt to set the Value if the FriendlyName is known.
+            // If the new Value does not match, do not permit mutating the value.
+            Assert.Throws<PlatformNotSupportedException>(() => oid.FriendlyName = SHA256_Name);
             Assert.Equal(SHA1_Oid, oid.Value);
+            Assert.Equal(SHA1_Name, oid.FriendlyName);
+        }
 
+        [Fact]
+        public static void TestFriendlyNameWithMismatchedValue()
+        {
+            Oid oid = new Oid(SHA1_Oid, SHA256_Name);
+            Assert.Equal(SHA1_Oid, oid.Value);
+            
             oid.FriendlyName = SHA256_Name;
+            
             Assert.Equal(SHA256_Name, oid.FriendlyName);
-            Assert.Equal(SHA256_Oid, oid.Value);
+            Assert.Equal(SHA1_Oid, oid.Value);
         }
 
         [Theory]

diff --git a/src/libraries/System.Security.Cryptography.Pkcs/tests/Pkcs12/Pkcs12SafeBagTests.cs b/src/libraries/System.Security.Cryptography.Pkcs/tests/Pkcs12/Pkcs12SafeBagTests.cs
index ccf814b..fa3d4b1 100644 (file)
--- a/src/libraries/System.Security.Cryptography.Pkcs/tests/Pkcs12/Pkcs12SafeBagTests.cs
+++ b/src/libraries/System.Security.Cryptography.Pkcs/tests/Pkcs12/Pkcs12SafeBagTests.cs
@@ -67,10 +67,6 @@ namespace System.Security.Cryptography.Pkcs.Tests.Pkcs12
             Assert.NotSame(firstCall, secondCall);
             Assert.Equal(Oids.Aes192, firstCall.Value);
             Assert.Equal(firstCall.Value, secondCall.Value);
-
-            secondCall.Value = Oids.Cms3DesWrap;
-            Assert.NotEqual(firstCall.Value, secondCall.Value);
-            Assert.Equal(Oids.Aes192, firstCall.Value);
         }
 
         [Fact]

diff --git a/src/libraries/System.Security.Cryptography.Pkcs/tests/Pkcs9AttributeTests.cs b/src/libraries/System.Security.Cryptography.Pkcs/tests/Pkcs9AttributeTests.cs
index 4474c06..8f216c4 100644 (file)
--- a/src/libraries/System.Security.Cryptography.Pkcs/tests/Pkcs9AttributeTests.cs
+++ b/src/libraries/System.Security.Cryptography.Pkcs/tests/Pkcs9AttributeTests.cs
@@ -114,8 +114,7 @@ namespace System.Security.Cryptography.Pkcs.Tests
         [Fact]
         public static void Pkcs9AttributeAsnEncodedDataCtorNullOidValue()
         {
-            Oid oid = new Oid(Oids.Aes128);
-            oid.Value = null;
+            Oid oid = new Oid(null, null);
 
             AsnEncodedData a = new AsnEncodedData(oid, new byte[3]);
             object ign;
@@ -125,8 +124,7 @@ namespace System.Security.Cryptography.Pkcs.Tests
         [Fact]
         public static void Pkcs9AttributeAsnEncodedDataCtorEmptyOidValue()
         {
-            Oid oid = new Oid(Oids.Aes128);
-            oid.Value = string.Empty;
+            Oid oid = new Oid(string.Empty, null);
 
             AsnEncodedData a = new AsnEncodedData(oid, new byte[3]);
             object ign;

diff --git a/src/libraries/System.Security.Cryptography.Pkcs/tests/SignedCms/SignedCmsTests.cs b/src/libraries/System.Security.Cryptography.Pkcs/tests/SignedCms/SignedCmsTests.cs
index efd1c16..5d76a30 100644 (file)
--- a/src/libraries/System.Security.Cryptography.Pkcs/tests/SignedCms/SignedCmsTests.cs
+++ b/src/libraries/System.Security.Cryptography.Pkcs/tests/SignedCms/SignedCmsTests.cs
@@ -1156,7 +1156,9 @@ namespace System.Security.Cryptography.Pkcs.Tests
 
             // CheckSignature doesn't read the public mutable data
             contentInfo.Content[0] ^= 0xFF;
+#if !NETCOREAPP
             contentInfo.ContentType.Value = Oids.Pkcs7Hashed;
+#endif
             cms.CheckSignature(true);
 
             using (X509Certificate2 signerCert = Certificates.RSA2048SignatureOnly.TryGetCertificateWithPrivateKey())