Skip to content

Commit

Permalink
Make properties on S.S.C.Oid effectively init-only
Browse files Browse the repository at this point in the history
Do not permit OID properties to be changed after they have
been set to a non-null value.
  • Loading branch information
vcsjones authored Jun 25, 2020
1 parent 910b7fa commit 4a21529
Show file tree
Hide file tree
Showing 6 changed files with 90 additions and 37 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -69,8 +69,18 @@ public static Oid FromOidValue(string oidValue, OidGroup group)

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
Expand All @@ -86,16 +96,40 @@ public string? FriendlyName
}
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;
}
}
}
Expand Down
63 changes: 40 additions & 23 deletions src/libraries/System.Security.Cryptography.Encoding/tests/Oid.cs
Original file line number Diff line number Diff line change
Expand Up @@ -116,49 +116,66 @@ public static void TestValueProperty()
{
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]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -67,10 +67,6 @@ public static void GetBagIdIsFactory()
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]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -114,8 +114,7 @@ public static void InputDateTimeAsX509TimeBetween1950And2049_Utc()
[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;
Expand All @@ -125,8 +124,7 @@ public static void Pkcs9AttributeAsnEncodedDataCtorNullOidValue()
[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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1156,7 +1156,9 @@ public static void EnsureDataIsolation_NewDocument(bool detached)

// 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())
Expand Down

0 comments on commit 4a21529

Please sign in to comment.