From fa6c78af12b82557831263f9ad2702231bfa0d6e Mon Sep 17 00:00:00 2001 From: Kevin Jones Date: Thu, 10 Mar 2022 15:18:32 -0500 Subject: [PATCH 01/15] Initial implementation --- .../src/System/Security/Cryptography/Oids.cs | 6 + .../ref/System.Security.Cryptography.cs | 19 ++ .../ref/System.Security.Cryptography.csproj | 1 + .../src/Resources/Strings.resx | 9 + .../src/System.Security.Cryptography.csproj | 1 + .../X500DistinguishedNameBuilder.cs | 222 ++++++++++++++++++ 6 files changed, 258 insertions(+) create mode 100644 src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/X509Certificates/X500DistinguishedNameBuilder.cs diff --git a/src/libraries/Common/src/System/Security/Cryptography/Oids.cs b/src/libraries/Common/src/System/Security/Cryptography/Oids.cs index 2562b15501de1..8b530cd8e8814 100644 --- a/src/libraries/Common/src/System/Security/Cryptography/Oids.cs +++ b/src/libraries/Common/src/System/Security/Cryptography/Oids.cs @@ -92,6 +92,9 @@ internal static partial class Oids // X500 Names internal const string CommonName = "2.5.4.3"; + internal const string CountryOrRegionName = "2.5.4.6"; + internal const string LocalityName = "2.5.4.7"; + internal const string StateOrProvinceName = "2.5.4.8"; internal const string Organization = "2.5.4.10"; internal const string OrganizationalUnit = "2.5.4.11"; internal const string EmailAddress = "1.2.840.113549.1.9.1"; @@ -151,5 +154,8 @@ internal static partial class Oids internal const string secp256r1 = "1.2.840.10045.3.1.7"; internal const string secp384r1 = "1.3.132.0.34"; internal const string secp521r1 = "1.3.132.0.35"; + + // LDAP + internal const string DomainComponent = "0.9.2342.19200300.100.1.25"; } } 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 ab98152e3398d..9595d5aa28bf1 100644 --- a/src/libraries/System.Security.Cryptography/ref/System.Security.Cryptography.cs +++ b/src/libraries/System.Security.Cryptography/ref/System.Security.Cryptography.cs @@ -2525,6 +2525,25 @@ public X500DistinguishedName(string distinguishedName, System.Security.Cryptogra public string Decode(System.Security.Cryptography.X509Certificates.X500DistinguishedNameFlags flag) { throw null; } public override string Format(bool multiLine) { throw null; } } + public sealed partial class X500DistinguishedNameBuilder + { + public X500DistinguishedNameBuilder() { } + public void Add(System.Security.Cryptography.Oid oid, string value, System.Formats.Asn1.UniversalTagNumber? stringEncodingType = default(System.Formats.Asn1.UniversalTagNumber?)) { } + public void Add(string oidValue, string value, System.Formats.Asn1.UniversalTagNumber? stringEncodingType = default(System.Formats.Asn1.UniversalTagNumber?)) { } + public void AddCommonName(string commonName) { } + public void AddCountryOrRegion(string twoLetterCode) { } + public void AddDomainComponent(string domainComponent) { } + public void AddEmailAddress(string emailAddress) { } + public void AddEncoded(System.Security.Cryptography.Oid oid, byte[] encodedValue) { } + public void AddEncoded(System.Security.Cryptography.Oid oid, System.ReadOnlySpan encodedValue) { } + public void AddEncoded(string oidValue, byte[] encodedValue) { } + public void AddEncoded(string oidValue, System.ReadOnlySpan encodedValue) { } + public void AddLocalityName(string localityName) { } + public void AddOrganizationalUnitName(string organizationalUnitName) { } + public void AddOrganizationName(string organizationName) { } + public void AddStateOrProvinceName(string stateOrProvinceName) { } + public System.Security.Cryptography.X509Certificates.X500DistinguishedName Build() { throw null; } + } [System.FlagsAttribute] public enum X500DistinguishedNameFlags { diff --git a/src/libraries/System.Security.Cryptography/ref/System.Security.Cryptography.csproj b/src/libraries/System.Security.Cryptography/ref/System.Security.Cryptography.csproj index 3020364482b9e..de6f358ac721e 100644 --- a/src/libraries/System.Security.Cryptography/ref/System.Security.Cryptography.csproj +++ b/src/libraries/System.Security.Cryptography/ref/System.Security.Cryptography.csproj @@ -11,6 +11,7 @@ + diff --git a/src/libraries/System.Security.Cryptography/src/Resources/Strings.resx b/src/libraries/System.Security.Cryptography/src/Resources/Strings.resx index 02431e49a9bd4..a1d26af4ba766 100644 --- a/src/libraries/System.Security.Cryptography/src/Resources/Strings.resx +++ b/src/libraries/System.Security.Cryptography/src/Resources/Strings.resx @@ -78,6 +78,9 @@ Invalid type. + + The specified UniversalTagNumber must be a string type. + Only single dimensional arrays are supported for the requested action. @@ -93,6 +96,12 @@ The value of 'nameType' is invalid. + + The country or region code must be exactly two characters. + + + The email address cannot exceed 255 characters. + Offset and length were out of bounds for the array or count is greater than the number of elements from index to the end of the source collection. diff --git a/src/libraries/System.Security.Cryptography/src/System.Security.Cryptography.csproj b/src/libraries/System.Security.Cryptography/src/System.Security.Cryptography.csproj index 0cb9fd6ec30fe..d78ce152036b4 100644 --- a/src/libraries/System.Security.Cryptography/src/System.Security.Cryptography.csproj +++ b/src/libraries/System.Security.Cryptography/src/System.Security.Cryptography.csproj @@ -437,6 +437,7 @@ + diff --git a/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/X509Certificates/X500DistinguishedNameBuilder.cs b/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/X509Certificates/X500DistinguishedNameBuilder.cs new file mode 100644 index 0000000000000..8b14c27b0d4c1 --- /dev/null +++ b/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/X509Certificates/X500DistinguishedNameBuilder.cs @@ -0,0 +1,222 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System.Collections.Generic; +using System.Formats.Asn1; + +namespace System.Security.Cryptography.X509Certificates +{ + public sealed class X500DistinguishedNameBuilder + { + private readonly List _encodedComponents = new List(); + + public void Add(string oidValue!!, string value!!, UniversalTagNumber? stringEncodingType = null) + { + UniversalTagNumber tag = GetAndValidateTagNumber(stringEncodingType); + EncodeComponent(oidValue, value, tag); + } + + public void Add(Oid oid!!, string value!!, UniversalTagNumber? stringEncodingType = null) + { + if (oid.Value is null) + throw new ArgumentException(SR.Format(SR.Arg_EmptyOrNullString_Named, "oid.Value"), nameof(oid)); + + UniversalTagNumber tag = GetAndValidateTagNumber(stringEncodingType); + EncodeComponent(oid.Value, value, tag); + } + + public void AddEncoded(string oidValue!!, byte[] encodedValue!!) + { + EncodeComponent(oidValue, encodedValue); + } + + public void AddEncoded(string oidValue!!, ReadOnlySpan encodedValue) + { + EncodeComponent(oidValue, encodedValue); + } + + public void AddEncoded(Oid oid!!, byte[] encodedValue!!) + { + if (oid.Value is null) + throw new ArgumentException(SR.Format(SR.Arg_EmptyOrNullString_Named, "oid.Value"), nameof(oid)); + + EncodeComponent(oid.Value, encodedValue); + } + + public void AddEncoded(Oid oid!!, ReadOnlySpan encodedValue) + { + if (oid.Value is null) + throw new ArgumentException(SR.Format(SR.Arg_EmptyOrNullString_Named, "oid.Value"), nameof(oid)); + + EncodeComponent(oid.Value, encodedValue); + } + + public void AddEmailAddress(string emailAddress) + { + //RFC 5912: + // id-emailAddress AttributeType ::= { pkcs-9 1 } + // at-emailAddress ATTRIBUTE ::= {TYPE IA5String + // (SIZE (1..ub-emailaddress-length)) IDENTIFIED BY + // id-emailAddress } + // ub-emailaddress-length INTEGER ::= 255 + + ArgumentException.ThrowIfNullOrEmpty(emailAddress); + + if (emailAddress.Length > 255) + { + throw new ArgumentException(SR.Argument_X500_EmailTooLong, nameof(emailAddress)); + } + + EncodeComponent(Oids.EmailAddress, emailAddress, UniversalTagNumber.IA5String); + } + + public void AddCommonName(string commonName) + { + // ITU T-REC X.520 Annex A: + // id-at-commonName + // WITH SYNTAX UnboundedDirectoryString + + ArgumentException.ThrowIfNullOrEmpty(commonName); + EncodeComponent(Oids.CommonName, commonName, UniversalTagNumber.UTF8String); + } + + public void AddLocalityName(string localityName) + { + // ITU T-REC X.520 Annex A: + // id-at-commonName + // WITH SYNTAX UnboundedDirectoryString + + ArgumentException.ThrowIfNullOrEmpty(localityName); + EncodeComponent(Oids.LocalityName, localityName, UniversalTagNumber.UTF8String); + } + + public void AddCountryOrRegion(string twoLetterCode) + { + // ITU T-REC X.520 Annex A: + // id-at-countryName + // WITH SYNTAX CountryName + // CountryName ::= PrintableString(SIZE (2)) + + ArgumentException.ThrowIfNullOrEmpty(twoLetterCode); + + // This could be a surrogate pair, but since we are encoding as a PrintableString, + // those will be prohibited, so "Length" should be fine for checking the length of + // the string. + if (twoLetterCode.Length != 2) + { + throw new ArgumentException(SR.Argument_X500_InvalidCountryOrRegion, nameof(twoLetterCode)); + } + + EncodeComponent(Oids.CountryOrRegionName, twoLetterCode, UniversalTagNumber.PrintableString); + } + + public void AddOrganizationName(string organizationName) + { + // ITU T-REC X.520 Annex A: + // id-at-organizationName + // WITH SYNTAX UnboundedDirectoryString + + ArgumentException.ThrowIfNullOrEmpty(organizationName); + EncodeComponent(Oids.Organization, organizationName, UniversalTagNumber.UTF8String); + } + + public void AddOrganizationalUnitName(string organizationalUnitName) + { + // ITU T-REC X.520 Annex A: + // id-at-organizationalUnitName + // WITH SYNTAX UnboundedDirectoryString + + ArgumentException.ThrowIfNullOrEmpty(organizationalUnitName); + EncodeComponent(Oids.OrganizationalUnit, organizationalUnitName, UniversalTagNumber.UTF8String); + } + + public void AddStateOrProvinceName(string stateOrProvinceName) + { + // ITU T-REC X.520 Annex A: + // id-at-stateOrProvinceName + // WITH SYNTAX UnboundedDirectoryString + + ArgumentException.ThrowIfNullOrEmpty(stateOrProvinceName); + EncodeComponent(Oids.StateOrProvinceName, stateOrProvinceName, UniversalTagNumber.UTF8String); + } + + public void AddDomainComponent(string domainComponent) + { + // RFC 5912 + // id-domainComponent AttributeType ::= + // { itu-t(0) data(9) pss(2342) ucl(19200300) pilot(100) + // pilotAttributeType(1) 25 } + // at-domainComponent ATTRIBUTE ::= {TYPE IA5String + // IDENTIFIED BY id-domainComponent } + + ArgumentException.ThrowIfNullOrEmpty(domainComponent); + EncodeComponent(Oids.DomainComponent, domainComponent, UniversalTagNumber.IA5String); + } + + public X500DistinguishedName Build() + { + AsnWriter writer = new AsnWriter(AsnEncodingRules.DER); + + using (writer.PushSequence()) + { + foreach (byte[] component in _encodedComponents) + { + writer.WriteEncodedValue(component); + } + } + + byte[] rented = CryptoPool.Rent(writer.GetEncodedLength()); + int encoded = writer.Encode(rented); + X500DistinguishedName name = new X500DistinguishedName(rented.AsSpan(0, encoded)); + CryptoPool.Return(rented, clearSize: 0); // Distinguished Names do not contain sensitive information. + return name; + } + + private void EncodeComponent(string oid, string value, UniversalTagNumber stringEncodingType) + { + AsnWriter writer = new AsnWriter(AsnEncodingRules.DER); + + using (writer.PushSetOf()) + using (writer.PushSequence()) + { + writer.WriteObjectIdentifier(oid); + writer.WriteCharacterString(stringEncodingType, value); + } + + _encodedComponents.Add(writer.Encode()); + } + + private void EncodeComponent(string oid, ReadOnlySpan value) + { + AsnWriter writer = new AsnWriter(AsnEncodingRules.DER); + + using (writer.PushSetOf()) + using (writer.PushSequence()) + { + writer.WriteObjectIdentifier(oid); + writer.WriteEncodedValue(value); + } + + _encodedComponents.Add(writer.Encode()); + } + + private static UniversalTagNumber GetAndValidateTagNumber(UniversalTagNumber? stringEncodingType) + { + switch (stringEncodingType) + { + case null: + return UniversalTagNumber.UTF8String; + case UniversalTagNumber.UTF8String: + case UniversalTagNumber.NumericString: + case UniversalTagNumber.PrintableString: + case UniversalTagNumber.IA5String: + case UniversalTagNumber.VisibleString: + case UniversalTagNumber.BMPString: + case UniversalTagNumber.T61String: + return stringEncodingType.GetValueOrDefault(); + default: + throw new ArgumentException(SR.Argument_Asn1_InvalidCharacterString, nameof(stringEncodingType)); + } + } + } +} From 92ef98e95f8c0f7beacacd6eabcf4a97649f24e8 Mon Sep 17 00:00:00 2001 From: Kevin Jones Date: Thu, 10 Mar 2022 19:07:00 -0500 Subject: [PATCH 02/15] Tests --- ...Cryptography.X509Certificates.Tests.csproj | 1 + .../X500DistinguishedNameBuilderTests.cs | 196 ++++++++++++++++++ .../src/Resources/Strings.resx | 3 + .../X500DistinguishedNameBuilder.cs | 18 +- 4 files changed, 216 insertions(+), 2 deletions(-) create mode 100644 src/libraries/System.Security.Cryptography.X509Certificates/tests/X500DistinguishedNameBuilderTests.cs diff --git a/src/libraries/System.Security.Cryptography.X509Certificates/tests/System.Security.Cryptography.X509Certificates.Tests.csproj b/src/libraries/System.Security.Cryptography.X509Certificates/tests/System.Security.Cryptography.X509Certificates.Tests.csproj index 3fce6c8a16ea7..2a25d072edc76 100644 --- a/src/libraries/System.Security.Cryptography.X509Certificates/tests/System.Security.Cryptography.X509Certificates.Tests.csproj +++ b/src/libraries/System.Security.Cryptography.X509Certificates/tests/System.Security.Cryptography.X509Certificates.Tests.csproj @@ -48,6 +48,7 @@ + diff --git a/src/libraries/System.Security.Cryptography.X509Certificates/tests/X500DistinguishedNameBuilderTests.cs b/src/libraries/System.Security.Cryptography.X509Certificates/tests/X500DistinguishedNameBuilderTests.cs new file mode 100644 index 0000000000000..9e6ee9ea1767a --- /dev/null +++ b/src/libraries/System.Security.Cryptography.X509Certificates/tests/X500DistinguishedNameBuilderTests.cs @@ -0,0 +1,196 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using Xunit; + +namespace System.Security.Cryptography.X509Certificates.Tests +{ + public static class X500DistinguishedNameBuilderTests + { + private const string TestOid = "2.25.77135202736018529853602245419149860647"; + + [Fact] + public static void AddEmailAddress_Success_EncodedToIA5() + { + AssertBuilder( + "30223120301E06092A864886F70D01090116116B6576696E406578616D706C652E636F6D", + builder => builder.AddEmailAddress("kevin@example.com")); + } + + [Fact] + public static void AddEmailAddress_ExceedsMaximumLength_Fails() + { + X500DistinguishedNameBuilder builder = new(); + string bigEmail = new string('a', 244) + "@example.com"; + AssertExtensions.Throws("emailAddress", () => builder.AddEmailAddress(bigEmail)); + } + + [Fact] + public static void AddEmailAddress_InvalidIA5_Fails() + { + X500DistinguishedNameBuilder builder = new(); + ArgumentException ex = AssertExtensions.Throws( + "emailAddress", + () => builder.AddEmailAddress("\u043A@example.com")); + Assert.Contains("'IA5String'", ex.Message); + } + + [Fact] + public static void AddEmailAddress_NullOrEmpty_Fails() + { + AssertAddThrowsOnNullAndEmpty("emailAddress", builder => builder.AddEmailAddress); + } + + [Fact] + public static void AddCommonName_Success_EncodeToUTF8String() + { + AssertBuilder("300D310B300906035504030C024B4A", builder => builder.AddCommonName("KJ")); + } + + [Fact] + public static void AddCommonName_NullOrEmpty_Fails() + { + AssertAddThrowsOnNullAndEmpty("commonName", builder => builder.AddCommonName); + } + + [Fact] + public static void AddLocalityName_Success_EncodeToUTF8String() + { + AssertBuilder("300F310D300B06035504070C04486F6D65", builder => builder.AddLocalityName("Home")); + } + + [Fact] + public static void AddLocalityName_NullOrEmpty_Fails() + { + AssertAddThrowsOnNullAndEmpty("localityName", builder => builder.AddLocalityName); + } + + [Fact] + public static void AddCountryOrRegion_Success_EncodeToPrintableString() + { + AssertBuilder("300D310B3009060355040613025553", builder => builder.AddCountryOrRegion("US")); + } + + [Theory] + [InlineData("A")] + [InlineData("AAA")] + public static void AddCountryOrRegion_InvalidLength_Fails(string countryOrRegion) + { + X500DistinguishedNameBuilder builder = new(); + AssertExtensions.Throws( + "twoLetterCode", + () => builder.AddCountryOrRegion(countryOrRegion)); + } + + [Fact] + public static void AddCountryOrRegion_InvalidContents_Fails() + { + X500DistinguishedNameBuilder builder = new(); + ArgumentException ex = AssertExtensions.Throws( + "twoLetterCode", + () => builder.AddCountryOrRegion("``")); + Assert.Contains("'PrintableString'", ex.Message); + } + + [Fact] + public static void AddOrganizationName_Success_EncodeToUTF8String() + { + AssertBuilder("3011310F300D060355040A0C06476974487562", builder => builder.AddOrganizationName("GitHub")); + } + + [Fact] + public static void AddOrganizationName_NullOrEmpty_Fails() + { + AssertAddThrowsOnNullAndEmpty("organizationName", builder => builder.AddOrganizationName); + } + + [Fact] + public static void AddOrganizationalUnitName_Success_EncodeToUTF8String() + { + AssertBuilder("300E310C300A060355040B0C03505345", builder => builder.AddOrganizationalUnitName("PSE")); + } + + [Fact] + public static void AddOrganizationalUnitName_NullOrEmpty_Fails() + { + AssertAddThrowsOnNullAndEmpty("organizationalUnitName", builder => builder.AddOrganizationalUnitName); + } + + [Fact] + public static void AddStateOrProvinceName_Success_EncodeToUTF8String() + { + AssertBuilder("300D310B300906035504080C024341", builder => builder.AddStateOrProvinceName("CA")); + } + + [Fact] + public static void AddStateOrProvinceName_NullOrEmpty_Fails() + { + AssertAddThrowsOnNullAndEmpty("stateOrProvinceName", builder => builder.AddStateOrProvinceName); + } + + [Fact] + public static void AddDomainComponent_Success_EncodeToIA5String() + { + AssertBuilder( + "301A31183016060A0992268993F22C6401191608494E5445524E414C", + builder => builder.AddDomainComponent("INTERNAL")); + } + + [Fact] + public static void AddDomainComponent_NullOrEmpty_Fails() + { + AssertAddThrowsOnNullAndEmpty("domainComponent", builder => builder.AddDomainComponent); + } + + [Fact] + public static void AddDomainComponent_InvalidIA5_Fails() + { + X500DistinguishedNameBuilder builder = new(); + ArgumentException ex = AssertExtensions.Throws( + "domainComponent", + () => builder.AddDomainComponent("\u043Aevin")); + Assert.Contains("'IA5String'", ex.Message); + } + + [Fact] + public static void AddEncoded_StringOid_Success() + { + AssertBuilder( + "301C311A3018061369F487D9C7B5D0AEA2CCCBE8C8C7F2F6F2BE27130141", + builder => builder.AddEncoded(TestOid, new byte[] { 0x13, 0x01, 65 })); + + AssertBuilder( + "301C311A3018061369F487D9C7B5D0AEA2CCCBE8C8C7F2F6F2BE27130141", + builder => builder.AddEncoded(TestOid, stackalloc byte[] { 0x13, 0x01, 65 })); + } + + [Fact] + public static void AddEncoded_Oid_Success() + { + AssertBuilder( + "301C311A3018061369F487D9C7B5D0AEA2CCCBE8C8C7F2F6F2BE27130141", + builder => builder.AddEncoded(new Oid(TestOid), new byte[] { 0x13, 0x01, 65 })); + + AssertBuilder( + "301C311A3018061369F487D9C7B5D0AEA2CCCBE8C8C7F2F6F2BE27130141", + builder => builder.AddEncoded(new Oid(TestOid), stackalloc byte[] { 0x13, 0x01, 65 })); + } + + private static void AssertAddThrowsOnNullAndEmpty(string paramName, Func> adder) + { + AssertExtensions.Throws( + paramName, () => adder(new X500DistinguishedNameBuilder())(null)); + AssertExtensions.Throws( + paramName, () => adder(new X500DistinguishedNameBuilder())(string.Empty)); + } + + private static X500DistinguishedName AssertBuilder(string expectedHex, Action callback) + { + X500DistinguishedNameBuilder builder = new(); + callback(builder); + X500DistinguishedName dn = builder.Build(); + Assert.Equal(expectedHex, Convert.ToHexString(dn.RawData)); + return dn; + } + } +} diff --git a/src/libraries/System.Security.Cryptography/src/Resources/Strings.resx b/src/libraries/System.Security.Cryptography/src/Resources/Strings.resx index a1d26af4ba766..0132a72b60e32 100644 --- a/src/libraries/System.Security.Cryptography/src/Resources/Strings.resx +++ b/src/libraries/System.Security.Cryptography/src/Resources/Strings.resx @@ -81,6 +81,9 @@ The specified UniversalTagNumber must be a string type. + + The contents of the string are not valid for the '{0}' ASN.1 encoding. + Only single dimensional arrays are supported for the requested action. diff --git a/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/X509Certificates/X500DistinguishedNameBuilder.cs b/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/X509Certificates/X500DistinguishedNameBuilder.cs index 8b14c27b0d4c1..d645e5ea1ba84 100644 --- a/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/X509Certificates/X500DistinguishedNameBuilder.cs +++ b/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/X509Certificates/X500DistinguishedNameBuilder.cs @@ -3,6 +3,8 @@ using System.Collections.Generic; using System.Formats.Asn1; +using System.Runtime.CompilerServices; +using System.Text; namespace System.Security.Cryptography.X509Certificates { @@ -172,7 +174,11 @@ public X500DistinguishedName Build() return name; } - private void EncodeComponent(string oid, string value, UniversalTagNumber stringEncodingType) + private void EncodeComponent( + string oid, + string value, + UniversalTagNumber stringEncodingType, + [CallerArgumentExpression("value")] string? paramName = null) { AsnWriter writer = new AsnWriter(AsnEncodingRules.DER); @@ -180,7 +186,15 @@ private void EncodeComponent(string oid, string value, UniversalTagNumber string using (writer.PushSequence()) { writer.WriteObjectIdentifier(oid); - writer.WriteCharacterString(stringEncodingType, value); + + try + { + writer.WriteCharacterString(stringEncodingType, value); + } + catch (EncoderFallbackException) + { + throw new ArgumentException(SR.Format(SR.Argument_Asn1_InvalidStringContents, stringEncodingType), paramName); + } } _encodedComponents.Add(writer.Encode()); From 6501b8e8170a9d01a87c852f9420e30e43a2c3f9 Mon Sep 17 00:00:00 2001 From: Kevin Jones Date: Fri, 11 Mar 2022 11:15:42 -0500 Subject: [PATCH 03/15] Finish up tests --- ...Cryptography.X509Certificates.Tests.csproj | 1 - .../X500DistinguishedNameBuilder.cs | 6 +- .../System.Security.Cryptography.Tests.csproj | 1 + .../X500DistinguishedNameBuilderTests.cs | 141 +++++++++++++++++- 4 files changed, 143 insertions(+), 6 deletions(-) rename src/libraries/{System.Security.Cryptography.X509Certificates/tests => System.Security.Cryptography/tests/X509Certificates}/X500DistinguishedNameBuilderTests.cs (55%) diff --git a/src/libraries/System.Security.Cryptography.X509Certificates/tests/System.Security.Cryptography.X509Certificates.Tests.csproj b/src/libraries/System.Security.Cryptography.X509Certificates/tests/System.Security.Cryptography.X509Certificates.Tests.csproj index 2a25d072edc76..3fce6c8a16ea7 100644 --- a/src/libraries/System.Security.Cryptography.X509Certificates/tests/System.Security.Cryptography.X509Certificates.Tests.csproj +++ b/src/libraries/System.Security.Cryptography.X509Certificates/tests/System.Security.Cryptography.X509Certificates.Tests.csproj @@ -48,7 +48,6 @@ - diff --git a/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/X509Certificates/X500DistinguishedNameBuilder.cs b/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/X509Certificates/X500DistinguishedNameBuilder.cs index d645e5ea1ba84..b4a11a7fcca73 100644 --- a/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/X509Certificates/X500DistinguishedNameBuilder.cs +++ b/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/X509Certificates/X500DistinguishedNameBuilder.cs @@ -20,7 +20,7 @@ public void Add(string oidValue!!, string value!!, UniversalTagNumber? stringEnc public void Add(Oid oid!!, string value!!, UniversalTagNumber? stringEncodingType = null) { - if (oid.Value is null) + if (string.IsNullOrEmpty(oid.Value)) throw new ArgumentException(SR.Format(SR.Arg_EmptyOrNullString_Named, "oid.Value"), nameof(oid)); UniversalTagNumber tag = GetAndValidateTagNumber(stringEncodingType); @@ -39,7 +39,7 @@ public void AddEncoded(string oidValue!!, ReadOnlySpan encodedValue) public void AddEncoded(Oid oid!!, byte[] encodedValue!!) { - if (oid.Value is null) + if (string.IsNullOrEmpty(oid.Value)) throw new ArgumentException(SR.Format(SR.Arg_EmptyOrNullString_Named, "oid.Value"), nameof(oid)); EncodeComponent(oid.Value, encodedValue); @@ -47,7 +47,7 @@ public void AddEncoded(Oid oid!!, byte[] encodedValue!!) public void AddEncoded(Oid oid!!, ReadOnlySpan encodedValue) { - if (oid.Value is null) + if (string.IsNullOrEmpty(oid.Value)) throw new ArgumentException(SR.Format(SR.Arg_EmptyOrNullString_Named, "oid.Value"), nameof(oid)); EncodeComponent(oid.Value, encodedValue); diff --git a/src/libraries/System.Security.Cryptography/tests/System.Security.Cryptography.Tests.csproj b/src/libraries/System.Security.Cryptography/tests/System.Security.Cryptography.Tests.csproj index 3fd47902df503..7410509cbc00b 100644 --- a/src/libraries/System.Security.Cryptography/tests/System.Security.Cryptography.Tests.csproj +++ b/src/libraries/System.Security.Cryptography/tests/System.Security.Cryptography.Tests.csproj @@ -275,6 +275,7 @@ + diff --git a/src/libraries/System.Security.Cryptography.X509Certificates/tests/X500DistinguishedNameBuilderTests.cs b/src/libraries/System.Security.Cryptography/tests/X509Certificates/X500DistinguishedNameBuilderTests.cs similarity index 55% rename from src/libraries/System.Security.Cryptography.X509Certificates/tests/X500DistinguishedNameBuilderTests.cs rename to src/libraries/System.Security.Cryptography/tests/X509Certificates/X500DistinguishedNameBuilderTests.cs index 9e6ee9ea1767a..07c7d96ee86aa 100644 --- a/src/libraries/System.Security.Cryptography.X509Certificates/tests/X500DistinguishedNameBuilderTests.cs +++ b/src/libraries/System.Security.Cryptography/tests/X509Certificates/X500DistinguishedNameBuilderTests.cs @@ -1,6 +1,8 @@ // Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. +using System.Collections.Generic; +using System.Formats.Asn1; using Xunit; namespace System.Security.Cryptography.X509Certificates.Tests @@ -169,11 +171,146 @@ public static void AddEncoded_Oid_Success() { AssertBuilder( "301C311A3018061369F487D9C7B5D0AEA2CCCBE8C8C7F2F6F2BE27130141", - builder => builder.AddEncoded(new Oid(TestOid), new byte[] { 0x13, 0x01, 65 })); + builder => builder.AddEncoded(new Oid(TestOid, null), new byte[] { 0x13, 0x01, 65 })); AssertBuilder( "301C311A3018061369F487D9C7B5D0AEA2CCCBE8C8C7F2F6F2BE27130141", - builder => builder.AddEncoded(new Oid(TestOid), stackalloc byte[] { 0x13, 0x01, 65 })); + builder => builder.AddEncoded(new Oid(TestOid, null), stackalloc byte[] { 0x13, 0x01, 65 })); + } + + [Fact] + public static void AddEncoded_StringOid_NullOrEmpty_Fails() + { + X500DistinguishedNameBuilder builder = new(); + AssertExtensions.Throws( + "oidValue", + () => builder.AddEncoded((string)null, Array.Empty())); + AssertExtensions.Throws( + "oidValue", + () => builder.AddEncoded("", Array.Empty())); + } + + [Fact] + public static void Add_InvalidUniversalTagNumber() + { + X500DistinguishedNameBuilder builder = new(); + AssertExtensions.Throws( + "stringEncodingType", + () => builder.Add(TestOid, "True", UniversalTagNumber.Boolean)); + AssertExtensions.Throws( + "stringEncodingType", + () => builder.Add(new Oid(TestOid, null), "True", UniversalTagNumber.Boolean)); + } + + [Fact] + public static void AddEncoded_Oid_NullOrEmptyValue_Fails() + { + X500DistinguishedNameBuilder builder = new(); + AssertExtensions.Throws( + "oid", + () => builder.AddEncoded((Oid)null, Array.Empty())); + AssertExtensions.Throws( + "oid", + () => builder.AddEncoded(new Oid(), Array.Empty())); + AssertExtensions.Throws( + "oid", + () => builder.AddEncoded(new Oid("", null), Array.Empty())); + } + + [Fact] + public static void AddEncoded_Value_Null_Fails() + { + X500DistinguishedNameBuilder builder = new(); + AssertExtensions.Throws( + "encodedValue", + () => builder.AddEncoded(TestOid, (byte[])null)); + AssertExtensions.Throws( + "encodedValue", + () => builder.AddEncoded(new Oid(TestOid, null), (byte[])null)); + } + + [Theory] + [MemberData(nameof(AddStringTheories))] + public static void Add_StringOid_Success(string oid, string value, UniversalTagNumber? tag, string expectedHex) + { + AssertBuilder(expectedHex, builder => builder.Add(oid, value, tag)); + } + + [Theory] + [MemberData(nameof(AddStringTheories))] + public static void Add_Oid_Success(string oid, string value, UniversalTagNumber? tag, string expectedHex) + { + AssertBuilder(expectedHex, builder => builder.Add(new Oid(oid, null), value, tag)); + } + + public static IEnumerable AddStringTheories + { + get + { + yield return new object[] + { + TestOid, + "banana", + UniversalTagNumber.UTF8String, + "3021311F301D061369F487D9C7B5D0AEA2CCCBE8C8C7F2F6F2BE270C0662616E616E61", + }; + yield return new object[] + { + TestOid, + "banana", + null, + "3021311F301D061369F487D9C7B5D0AEA2CCCBE8C8C7F2F6F2BE270C0662616E616E61", + }; + yield return new object[] + { + TestOid, + "banana", + UniversalTagNumber.PrintableString, + "3021311F301D061369F487D9C7B5D0AEA2CCCBE8C8C7F2F6F2BE27130662616E616E61", + }; + yield return new object[] + { + TestOid, + "banana", + UniversalTagNumber.VisibleString, + "3021311F301D061369F487D9C7B5D0AEA2CCCBE8C8C7F2F6F2BE271A0662616E616E61", + }; + yield return new object[] + { + TestOid, + "banana", + UniversalTagNumber.IA5String, + "3021311F301D061369F487D9C7B5D0AEA2CCCBE8C8C7F2F6F2BE27160662616E616E61", + }; + yield return new object[] + { + TestOid, + "banana", + UniversalTagNumber.PrintableString, + "3021311F301D061369F487D9C7B5D0AEA2CCCBE8C8C7F2F6F2BE27130662616E616E61", + }; + yield return new object[] + { + TestOid, + "\u0411\u0430\u043d\u0430\u043d", + UniversalTagNumber.BMPString, + "302531233021061369F487D9C7B5D0AEA2CCCBE8C8C7F2F6F2BE271E0A04110430043D0430043D", + }; + yield return new object[] + { + TestOid, + "\u0411\u0430\u043d\u0430\u043d", + UniversalTagNumber.UTF8String, + "302531233021061369F487D9C7B5D0AEA2CCCBE8C8C7F2F6F2BE270C0AD091D0B0D0BDD0B0D0BD", + }; + yield return new object[] + { + TestOid, + "0123456789", + UniversalTagNumber.NumericString, + "302531233021061369F487D9C7B5D0AEA2CCCBE8C8C7F2F6F2BE27120A30313233343536373839", + }; + } } private static void AssertAddThrowsOnNullAndEmpty(string paramName, Func> adder) From db93a2f09c8af03c6c95bd72ea472205a90bf653 Mon Sep 17 00:00:00 2001 From: Kevin Jones Date: Fri, 11 Mar 2022 11:18:30 -0500 Subject: [PATCH 04/15] Make Windows like me --- .../tests/System.Security.Cryptography.Tests.csproj | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libraries/System.Security.Cryptography/tests/System.Security.Cryptography.Tests.csproj b/src/libraries/System.Security.Cryptography/tests/System.Security.Cryptography.Tests.csproj index 7410509cbc00b..889dca4802564 100644 --- a/src/libraries/System.Security.Cryptography/tests/System.Security.Cryptography.Tests.csproj +++ b/src/libraries/System.Security.Cryptography/tests/System.Security.Cryptography.Tests.csproj @@ -275,7 +275,7 @@ - + From c019ab6792c5e8b78baf539def8cd93c30b9d2ae Mon Sep 17 00:00:00 2001 From: Kevin Jones Date: Fri, 11 Mar 2022 12:38:55 -0500 Subject: [PATCH 05/15] Docs and better input validation --- .../src/Resources/Strings.resx | 3 + .../X500DistinguishedNameBuilder.cs | 253 +++++++++++++++++- .../X500DistinguishedNameBuilderTests.cs | 70 ++++- 3 files changed, 309 insertions(+), 17 deletions(-) diff --git a/src/libraries/System.Security.Cryptography/src/Resources/Strings.resx b/src/libraries/System.Security.Cryptography/src/Resources/Strings.resx index 0132a72b60e32..2a6f9d7aafa52 100644 --- a/src/libraries/System.Security.Cryptography/src/Resources/Strings.resx +++ b/src/libraries/System.Security.Cryptography/src/Resources/Strings.resx @@ -84,6 +84,9 @@ The contents of the string are not valid for the '{0}' ASN.1 encoding. + + The contents are not valid ASN.1 under Distinguished Encoding Rules. + Only single dimensional arrays are supported for the requested action. diff --git a/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/X509Certificates/X500DistinguishedNameBuilder.cs b/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/X509Certificates/X500DistinguishedNameBuilder.cs index b4a11a7fcca73..bfbb2096293f4 100644 --- a/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/X509Certificates/X500DistinguishedNameBuilder.cs +++ b/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/X509Certificates/X500DistinguishedNameBuilder.cs @@ -8,16 +8,71 @@ namespace System.Security.Cryptography.X509Certificates { + /// + /// This class facilitates building a distinguished name for an X.509 certificate. + /// public sealed class X500DistinguishedNameBuilder { private readonly List _encodedComponents = new List(); - public void Add(string oidValue!!, string value!!, UniversalTagNumber? stringEncodingType = null) + /// + /// Adds a Relative Distinguished Name attribute identified by an OID. + /// + /// The OID of the attribute. + /// The value of the attribute. + /// + /// The encoding type to use when encoding the + /// in to the attribute. + /// + /// + /// or is . + /// + /// + ///

+ /// is an empty string or not a valid OID. + ///

+ ///

-or-

+ ///

+ /// is not a type for character strings. + ///

+ ///

-or-

+ ///

+ /// is not encodable as defined by . + ///

+ ///
+ public void Add(string oidValue, string value!!, UniversalTagNumber? stringEncodingType = null) { + ArgumentException.ThrowIfNullOrEmpty(oidValue); + UniversalTagNumber tag = GetAndValidateTagNumber(stringEncodingType); EncodeComponent(oidValue, value, tag); } + /// + /// Adds a Relative Distinguished Name attribute identified by an OID. + /// + /// The OID of the attribute. + /// The value of the attribute. + /// + /// The encoding type to use when encoding the + /// in to the attribute. + /// + /// + /// or is . + /// + /// + ///

+ /// does not contain a valid OID. + ///

+ ///

-or-

+ ///

+ /// is not a type for character strings. + ///

+ ///

-or-

+ ///

+ /// is not encodable as defined by . + ///

+ ///
public void Add(Oid oid!!, string value!!, UniversalTagNumber? stringEncodingType = null) { if (string.IsNullOrEmpty(oid.Value)) @@ -27,16 +82,69 @@ public void Add(Oid oid!!, string value!!, UniversalTagNumber? stringEncodingTyp EncodeComponent(oid.Value, value, tag); } - public void AddEncoded(string oidValue!!, byte[] encodedValue!!) + /// + /// Adds a Relative Distinguished Name attribute identified by an OID. + /// + /// The OID of the attribute. + /// The pre-encoded value of the attribute. + /// + /// or is . + /// + /// + ///

+ /// is an empty string or not a valid OID. + ///

+ ///

-or-

+ ///

+ /// does not contain valid ASN.1 as defined by the Distinguished Encoding Rules (DER). + ///

+ ///
+ public void AddEncoded(string oidValue, byte[] encodedValue!!) { + ArgumentException.ThrowIfNullOrEmpty(oidValue); EncodeComponent(oidValue, encodedValue); } - public void AddEncoded(string oidValue!!, ReadOnlySpan encodedValue) + /// + /// Adds a Relative Distinguished Name attribute identified by an OID. + /// + /// The OID of the attribute. + /// The pre-encoded value of the attribute. + /// + /// is . + /// + /// + ///

+ /// is an empty string or not a valid OID. + ///

+ ///

-or-

+ ///

+ /// does not contain valid ASN.1 as defined by the Distinguished Encoding Rules (DER). + ///

+ ///
+ public void AddEncoded(string oidValue, ReadOnlySpan encodedValue) { + ArgumentException.ThrowIfNullOrEmpty(oidValue); EncodeComponent(oidValue, encodedValue); } + /// + /// Adds a Relative Distinguished Name attribute identified by an OID. + /// + /// The OID of the attribute. + /// The pre-encoded value of the attribute. + /// + /// or is . + /// + /// + ///

+ /// does not contain a valid OID. + ///

+ ///

-or-

+ ///

+ /// does not contain valid ASN.1 as defined by the Distinguished Encoding Rules (DER). + ///

+ ///
public void AddEncoded(Oid oid!!, byte[] encodedValue!!) { if (string.IsNullOrEmpty(oid.Value)) @@ -45,6 +153,23 @@ public void AddEncoded(Oid oid!!, byte[] encodedValue!!) EncodeComponent(oid.Value, encodedValue); } + /// + /// Adds a Relative Distinguished Name attribute identified by an OID. + /// + /// The OID of the attribute. + /// The pre-encoded value of the attribute. + /// + /// is . + /// + /// + ///

+ /// does not contain a valid OID. + ///

+ ///

-or-

+ ///

+ /// does not contain valid ASN.1 as defined by the Distinguished Encoding Rules (DER). + ///

+ ///
public void AddEncoded(Oid oid!!, ReadOnlySpan encodedValue) { if (string.IsNullOrEmpty(oid.Value)) @@ -53,6 +178,19 @@ public void AddEncoded(Oid oid!!, ReadOnlySpan encodedValue) EncodeComponent(oid.Value, encodedValue); } + /// + /// Adds an email address attribute. + /// + /// The email address to add. + /// + /// is . + /// + /// + /// is empty or exceeds 255 characters. + /// + /// + /// This encodes an attribute with the OID 1.2.840.113549.1.9.1 as an IA5String. + /// public void AddEmailAddress(string emailAddress) { //RFC 5912: @@ -72,6 +210,19 @@ public void AddEmailAddress(string emailAddress) EncodeComponent(Oids.EmailAddress, emailAddress, UniversalTagNumber.IA5String); } + /// + /// Adds a common name attribute. + /// + /// The common name to add. + /// + /// is . + /// + /// + /// is empty. + /// + /// + /// This encodes an attribute with the OID 2.5.4.3 as a UTF8String. + /// public void AddCommonName(string commonName) { // ITU T-REC X.520 Annex A: @@ -82,16 +233,42 @@ public void AddCommonName(string commonName) EncodeComponent(Oids.CommonName, commonName, UniversalTagNumber.UTF8String); } + /// + /// Adds a locality name attribute. + /// + /// The locality name to add. + /// + /// is . + /// + /// + /// is empty. + /// + /// + /// This encodes an attribute with the OID 2.5.4.7 as a UTF8String. + /// public void AddLocalityName(string localityName) { // ITU T-REC X.520 Annex A: - // id-at-commonName + // id-at-localityName // WITH SYNTAX UnboundedDirectoryString ArgumentException.ThrowIfNullOrEmpty(localityName); EncodeComponent(Oids.LocalityName, localityName, UniversalTagNumber.UTF8String); } + /// + /// Adds a country or region attribute. + /// + /// The two letter code of the country or region. + /// + /// is . + /// + /// + /// is not exactly two characters. + /// + /// + /// This encodes an attribute with the OID 2.5.4.6 as a PrintableString. + /// public void AddCountryOrRegion(string twoLetterCode) { // ITU T-REC X.520 Annex A: @@ -112,6 +289,19 @@ public void AddCountryOrRegion(string twoLetterCode) EncodeComponent(Oids.CountryOrRegionName, twoLetterCode, UniversalTagNumber.PrintableString); } + /// + /// Adds an organization name attribute. + /// + /// The organization name to add. + /// + /// is . + /// + /// + /// is empty. + /// + /// + /// This encodes an attribute with the OID 2.5.4.10 as a UTF8String. + /// public void AddOrganizationName(string organizationName) { // ITU T-REC X.520 Annex A: @@ -122,6 +312,19 @@ public void AddOrganizationName(string organizationName) EncodeComponent(Oids.Organization, organizationName, UniversalTagNumber.UTF8String); } + /// + /// Adds an organizational unit name attribute. + /// + /// The organizational unit name to add. + /// + /// is . + /// + /// + /// is empty. + /// + /// + /// This encodes an attribute with the OID 2.5.4.11 as a UTF8String. + /// public void AddOrganizationalUnitName(string organizationalUnitName) { // ITU T-REC X.520 Annex A: @@ -132,6 +335,19 @@ public void AddOrganizationalUnitName(string organizationalUnitName) EncodeComponent(Oids.OrganizationalUnit, organizationalUnitName, UniversalTagNumber.UTF8String); } + /// + /// Adds a state or province name attribute. + /// + /// The state or province name to add. + /// + /// is . + /// + /// + /// is empty. + /// + /// + /// This encodes an attribute with the OID 2.5.4.8 as a UTF8String. + /// public void AddStateOrProvinceName(string stateOrProvinceName) { // ITU T-REC X.520 Annex A: @@ -142,6 +358,19 @@ public void AddStateOrProvinceName(string stateOrProvinceName) EncodeComponent(Oids.StateOrProvinceName, stateOrProvinceName, UniversalTagNumber.UTF8String); } + /// + /// Adds a domain component attribute. + /// + /// The domain component to add. + /// + /// is . + /// + /// + /// is empty. + /// + /// + /// This encodes an attribute with the OID 0.9.2342.19200300.100.1.25 as an IA5String. + /// public void AddDomainComponent(string domainComponent) { // RFC 5912 @@ -155,6 +384,12 @@ public void AddDomainComponent(string domainComponent) EncodeComponent(Oids.DomainComponent, domainComponent, UniversalTagNumber.IA5String); } + /// + /// Builds an that represents the encoded attributes. + /// + /// + /// An that represents the encoded attributes. + /// public X500DistinguishedName Build() { AsnWriter writer = new AsnWriter(AsnEncodingRules.DER); @@ -200,8 +435,16 @@ private void EncodeComponent( _encodedComponents.Add(writer.Encode()); } - private void EncodeComponent(string oid, ReadOnlySpan value) + private void EncodeComponent( + string oid, + ReadOnlySpan value, + [CallerArgumentExpression("value")] string? valueParamName = null) { + if (!AsnDecoder.TryReadEncodedValue(value, AsnEncodingRules.DER, out _, out _, out _, out _)) + { + throw new ArgumentException(SR.Argument_Asn1_InvalidDer, valueParamName); + } + AsnWriter writer = new AsnWriter(AsnEncodingRules.DER); using (writer.PushSetOf()) diff --git a/src/libraries/System.Security.Cryptography/tests/X509Certificates/X500DistinguishedNameBuilderTests.cs b/src/libraries/System.Security.Cryptography/tests/X509Certificates/X500DistinguishedNameBuilderTests.cs index 07c7d96ee86aa..3526b522a5437 100644 --- a/src/libraries/System.Security.Cryptography/tests/X509Certificates/X500DistinguishedNameBuilderTests.cs +++ b/src/libraries/System.Security.Cryptography/tests/X509Certificates/X500DistinguishedNameBuilderTests.cs @@ -179,27 +179,33 @@ public static void AddEncoded_Oid_Success() } [Fact] - public static void AddEncoded_StringOid_NullOrEmpty_Fails() + public static void AddEncoded_InvalidDer() { X500DistinguishedNameBuilder builder = new(); - AssertExtensions.Throws( - "oidValue", - () => builder.AddEncoded((string)null, Array.Empty())); AssertExtensions.Throws( - "oidValue", - () => builder.AddEncoded("", Array.Empty())); + "encodedValue", + () => builder.AddEncoded(TestOid, new byte[] { 0xFF, 0xFF, 0xFF })); + AssertExtensions.Throws( + "encodedValue", + () => builder.AddEncoded(TestOid, stackalloc byte[] { 0xFF, 0xFF, 0xFF })); + AssertExtensions.Throws( + "encodedValue", + () => builder.AddEncoded(new Oid(TestOid), new byte[] { 0xFF, 0xFF, 0xFF })); + AssertExtensions.Throws( + "encodedValue", + () => builder.AddEncoded(new Oid(TestOid), stackalloc byte[] { 0xFF, 0xFF, 0xFF })); } [Fact] - public static void Add_InvalidUniversalTagNumber() + public static void AddEncoded_StringOid_NullOrEmpty_Fails() { X500DistinguishedNameBuilder builder = new(); + AssertExtensions.Throws( + "oidValue", + () => builder.AddEncoded((string)null, Array.Empty())); AssertExtensions.Throws( - "stringEncodingType", - () => builder.Add(TestOid, "True", UniversalTagNumber.Boolean)); - AssertExtensions.Throws( - "stringEncodingType", - () => builder.Add(new Oid(TestOid, null), "True", UniversalTagNumber.Boolean)); + "oidValue", + () => builder.AddEncoded("", Array.Empty())); } [Fact] @@ -229,6 +235,39 @@ public static void AddEncoded_Value_Null_Fails() () => builder.AddEncoded(new Oid(TestOid, null), (byte[])null)); } + [Fact] + public static void Add_InvalidUniversalTagNumber() + { + X500DistinguishedNameBuilder builder = new(); + AssertExtensions.Throws( + "stringEncodingType", + () => builder.Add(TestOid, "True", UniversalTagNumber.Boolean)); + AssertExtensions.Throws( + "stringEncodingType", + () => builder.Add(new Oid(TestOid, null), "True", UniversalTagNumber.Boolean)); + } + + [Fact] + public static void Add_OidString_NullOrEmpty_Fails() + { + X500DistinguishedNameBuilder builder = new(); + AssertExtensions.Throws( + "oidValue", + () => builder.Add((string)null, "banana")); + AssertExtensions.Throws( + "oidValue", + () => builder.Add("", "banana")); + } + + [Fact] + public static void Add_OidString_NotAnOid_Fails() + { + X500DistinguishedNameBuilder builder = new(); + AssertExtensions.Throws( + "oidValue", + () => builder.Add("strawberry", "banana")); + } + [Theory] [MemberData(nameof(AddStringTheories))] public static void Add_StringOid_Success(string oid, string value, UniversalTagNumber? tag, string expectedHex) @@ -310,6 +349,13 @@ public static IEnumerable AddStringTheories UniversalTagNumber.NumericString, "302531233021061369F487D9C7B5D0AEA2CCCBE8C8C7F2F6F2BE27120A30313233343536373839", }; + yield return new object[] + { + TestOid, + "", + UniversalTagNumber.UTF8String, + "301B31193017061369F487D9C7B5D0AEA2CCCBE8C8C7F2F6F2BE270C00", + }; } } From 8f832fd47fed55f2898f02b7d8e68e516ff803de Mon Sep 17 00:00:00 2001 From: Kevin Jones Date: Fri, 11 Mar 2022 14:20:18 -0500 Subject: [PATCH 06/15] p to para --- .../X500DistinguishedNameBuilder.cs | 72 +++++++++---------- 1 file changed, 36 insertions(+), 36 deletions(-) diff --git a/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/X509Certificates/X500DistinguishedNameBuilder.cs b/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/X509Certificates/X500DistinguishedNameBuilder.cs index bfbb2096293f4..bc2342cffbc88 100644 --- a/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/X509Certificates/X500DistinguishedNameBuilder.cs +++ b/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/X509Certificates/X500DistinguishedNameBuilder.cs @@ -28,17 +28,17 @@ public sealed class X500DistinguishedNameBuilder /// or is . /// /// - ///

+ /// /// is an empty string or not a valid OID. - ///

- ///

-or-

- ///

+ /// + /// -or- + /// /// is not a type for character strings. - ///

- ///

-or-

- ///

+ /// + /// -or- + /// /// is not encodable as defined by . - ///

+ /// ///
public void Add(string oidValue, string value!!, UniversalTagNumber? stringEncodingType = null) { @@ -61,17 +61,17 @@ public void Add(string oidValue, string value!!, UniversalTagNumber? stringEncod /// or is . /// /// - ///

+ /// /// does not contain a valid OID. - ///

- ///

-or-

- ///

+ /// + /// -or- + /// /// is not a type for character strings. - ///

- ///

-or-

- ///

+ /// + /// -or- + /// /// is not encodable as defined by . - ///

+ /// ///
public void Add(Oid oid!!, string value!!, UniversalTagNumber? stringEncodingType = null) { @@ -91,13 +91,13 @@ public void Add(Oid oid!!, string value!!, UniversalTagNumber? stringEncodingTyp /// or is . /// /// - ///

+ /// /// is an empty string or not a valid OID. - ///

- ///

-or-

- ///

+ /// + /// -or- + /// /// does not contain valid ASN.1 as defined by the Distinguished Encoding Rules (DER). - ///

+ /// ///
public void AddEncoded(string oidValue, byte[] encodedValue!!) { @@ -114,13 +114,13 @@ public void AddEncoded(string oidValue, byte[] encodedValue!!) /// is . /// /// - ///

+ /// /// is an empty string or not a valid OID. - ///

- ///

-or-

- ///

+ /// + /// -or- + /// /// does not contain valid ASN.1 as defined by the Distinguished Encoding Rules (DER). - ///

+ /// ///
public void AddEncoded(string oidValue, ReadOnlySpan encodedValue) { @@ -137,13 +137,13 @@ public void AddEncoded(string oidValue, ReadOnlySpan encodedValue) /// or is . /// /// - ///

+ /// /// does not contain a valid OID. - ///

- ///

-or-

- ///

+ /// + /// -or- + /// /// does not contain valid ASN.1 as defined by the Distinguished Encoding Rules (DER). - ///

+ /// ///
public void AddEncoded(Oid oid!!, byte[] encodedValue!!) { @@ -162,13 +162,13 @@ public void AddEncoded(Oid oid!!, byte[] encodedValue!!) /// is . /// /// - ///

+ /// /// does not contain a valid OID. - ///

- ///

-or-

- ///

+ /// + /// -or- + /// /// does not contain valid ASN.1 as defined by the Distinguished Encoding Rules (DER). - ///

+ /// ///
public void AddEncoded(Oid oid!!, ReadOnlySpan encodedValue) { From fbd6efc16da75c20e6b1701a01529f5958ba12fe Mon Sep 17 00:00:00 2001 From: Kevin Jones Date: Fri, 11 Mar 2022 21:34:26 -0500 Subject: [PATCH 07/15] Order strings resources, for sanity purposes --- .../src/Resources/Strings.resx | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/src/libraries/System.Security.Cryptography/src/Resources/Strings.resx b/src/libraries/System.Security.Cryptography/src/Resources/Strings.resx index 2a6f9d7aafa52..3ef8e2b1b4d66 100644 --- a/src/libraries/System.Security.Cryptography/src/Resources/Strings.resx +++ b/src/libraries/System.Security.Cryptography/src/Resources/Strings.resx @@ -78,6 +78,9 @@ Invalid type. + + Only single dimensional arrays are supported for the requested action. + The specified UniversalTagNumber must be a string type. @@ -87,9 +90,6 @@ The contents are not valid ASN.1 under Distinguished Encoding Rules. - - Only single dimensional arrays are supported for the requested action. - Destination is too short. @@ -102,12 +102,6 @@ The value of 'nameType' is invalid. - - The country or region code must be exactly two characters. - - - The email address cannot exceed 255 characters. - Offset and length were out of bounds for the array or count is greater than the number of elements from index to the end of the source collection. @@ -147,6 +141,12 @@ An encrypted key was found, but no password was provided. Use ImportFromEncryptedPem to import this key. + + The email address cannot exceed 255 characters. + + + The country or region code must be exactly two characters. + Index was out of range. Must be non-negative and less than the size of the collection. From d7a2bbe396cfb73b26099141b6fe2db163d0cd9d Mon Sep 17 00:00:00 2001 From: Kevin Jones Date: Fri, 11 Mar 2022 21:38:56 -0500 Subject: [PATCH 08/15] Use a better reference --- .../X500DistinguishedNameBuilder.cs | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/X509Certificates/X500DistinguishedNameBuilder.cs b/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/X509Certificates/X500DistinguishedNameBuilder.cs index bc2342cffbc88..d0005c1b5f875 100644 --- a/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/X509Certificates/X500DistinguishedNameBuilder.cs +++ b/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/X509Certificates/X500DistinguishedNameBuilder.cs @@ -193,12 +193,14 @@ public void AddEncoded(Oid oid!!, ReadOnlySpan encodedValue) /// public void AddEmailAddress(string emailAddress) { - //RFC 5912: - // id-emailAddress AttributeType ::= { pkcs-9 1 } - // at-emailAddress ATTRIBUTE ::= {TYPE IA5String - // (SIZE (1..ub-emailaddress-length)) IDENTIFIED BY - // id-emailAddress } - // ub-emailaddress-length INTEGER ::= 255 + //RFC 2585: + // emailAddress ATTRIBUTE ::= { + // WITH SYNTAX IA5String (SIZE(1..pkcs-9-ub-emailAddress)) + // EQUALITY MATCHING RULE pkcs9CaseIgnoreMatch + // ID pkcs-9-at-emailAdress + // } + // pkcs-9-ub-pkcs9String INTEGER ::= 255 + // pkcs-9-ub-emailAddress INTEGER ::= pkcs-9-ub-pkcs9String ArgumentException.ThrowIfNullOrEmpty(emailAddress); From e93b1a74f52b5218e67b5f2e711ab0eb102002fb Mon Sep 17 00:00:00 2001 From: Kevin Jones Date: Fri, 11 Mar 2022 22:29:48 -0500 Subject: [PATCH 09/15] Stricter checking of country or region --- .../src/Resources/Strings.resx | 2 +- .../X500DistinguishedNameBuilder.cs | 35 ++++++++++++++++--- .../X500DistinguishedNameBuilderTests.cs | 32 ++++++++--------- 3 files changed, 47 insertions(+), 22 deletions(-) diff --git a/src/libraries/System.Security.Cryptography/src/Resources/Strings.resx b/src/libraries/System.Security.Cryptography/src/Resources/Strings.resx index 3ef8e2b1b4d66..09ca702cc15a1 100644 --- a/src/libraries/System.Security.Cryptography/src/Resources/Strings.resx +++ b/src/libraries/System.Security.Cryptography/src/Resources/Strings.resx @@ -145,7 +145,7 @@ The email address cannot exceed 255 characters. - The country or region code must be exactly two characters. + The country or region code must be exactly two characters and contain characters between A through Z, inclusive. Index was out of range. Must be non-negative and less than the size of the collection. diff --git a/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/X509Certificates/X500DistinguishedNameBuilder.cs b/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/X509Certificates/X500DistinguishedNameBuilder.cs index d0005c1b5f875..fefb2ca8f573d 100644 --- a/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/X509Certificates/X500DistinguishedNameBuilder.cs +++ b/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/X509Certificates/X500DistinguishedNameBuilder.cs @@ -2,6 +2,7 @@ // The .NET Foundation licenses this file to you under the MIT license. using System.Collections.Generic; +using System.Diagnostics; using System.Formats.Asn1; using System.Runtime.CompilerServices; using System.Text; @@ -266,10 +267,15 @@ public void AddLocalityName(string localityName) /// is . /// /// - /// is not exactly two characters. + /// is not exactly two characters, or contains + /// characters that are not A through Z. /// /// - /// This encodes an attribute with the OID 2.5.4.6 as a PrintableString. + /// This encodes an attribute with the OID 2.5.4.6 as a PrintableString. + /// + /// should be a two letter ISO 3166 alpha-2 code, and + /// will be normalized to upper case characters. + /// /// public void AddCountryOrRegion(string twoLetterCode) { @@ -277,18 +283,37 @@ public void AddCountryOrRegion(string twoLetterCode) // id-at-countryName // WITH SYNTAX CountryName // CountryName ::= PrintableString(SIZE (2)) + // An attribute value for country name is a string chosen from ISO 3166-1 alpha-2 or ISO 3166-3 alpha-2. + // We can't reasonably enforce the ISO 3166 list, but we can ensure it's a two letter ISO code + // and consists of alpha characters. ArgumentException.ThrowIfNullOrEmpty(twoLetterCode); + ReadOnlySpan twoLetterCodeSpan = twoLetterCode; // This could be a surrogate pair, but since we are encoding as a PrintableString, // those will be prohibited, so "Length" should be fine for checking the length of // the string. - if (twoLetterCode.Length != 2) + // Input must be A-Z per ISO 3166. + if (twoLetterCode.Length != 2 || !IsAlpha(twoLetterCode[0]) || !IsAlpha(twoLetterCode[1])) { throw new ArgumentException(SR.Argument_X500_InvalidCountryOrRegion, nameof(twoLetterCode)); } - EncodeComponent(Oids.CountryOrRegionName, twoLetterCode, UniversalTagNumber.PrintableString); + Span fixupTwoLetterCode = stackalloc char[2]; + int written = twoLetterCodeSpan.ToUpperInvariant(fixupTwoLetterCode); + Debug.Assert(written == 2); + + EncodeComponent( + Oids.CountryOrRegionName, + fixupTwoLetterCode, + UniversalTagNumber.PrintableString, + nameof(twoLetterCode)); + + static bool IsAlpha(char ch) + { + uint c = (uint)ch; + return c - 'A' < 26 || c - 'a' < 26; + } } /// @@ -413,7 +438,7 @@ public X500DistinguishedName Build() private void EncodeComponent( string oid, - string value, + ReadOnlySpan value, UniversalTagNumber stringEncodingType, [CallerArgumentExpression("value")] string? paramName = null) { diff --git a/src/libraries/System.Security.Cryptography/tests/X509Certificates/X500DistinguishedNameBuilderTests.cs b/src/libraries/System.Security.Cryptography/tests/X509Certificates/X500DistinguishedNameBuilderTests.cs index 3526b522a5437..4a73da9f69b9f 100644 --- a/src/libraries/System.Security.Cryptography/tests/X509Certificates/X500DistinguishedNameBuilderTests.cs +++ b/src/libraries/System.Security.Cryptography/tests/X509Certificates/X500DistinguishedNameBuilderTests.cs @@ -67,16 +67,26 @@ public static void AddLocalityName_NullOrEmpty_Fails() AssertAddThrowsOnNullAndEmpty("localityName", builder => builder.AddLocalityName); } - [Fact] - public static void AddCountryOrRegion_Success_EncodeToPrintableString() + [Theory] + [InlineData("US", "300D310B3009060355040613025553")] + [InlineData("AA", "300D310B3009060355040613024141")] + [InlineData("ZZ", "300D310B3009060355040613025A5A")] + [InlineData("aa", "300D310B3009060355040613024141")] // Normalized to AA + [InlineData("zz", "300D310B3009060355040613025A5A")] // Normalized to ZZ. + public static void AddCountryOrRegion_Success_EncodeToPrintableString(string twoLetterCode, string expectedHex) { - AssertBuilder("300D310B3009060355040613025553", builder => builder.AddCountryOrRegion("US")); + AssertBuilder(expectedHex, builder => builder.AddCountryOrRegion(twoLetterCode)); } [Theory] - [InlineData("A")] - [InlineData("AAA")] - public static void AddCountryOrRegion_InvalidLength_Fails(string countryOrRegion) + [InlineData("A")] // Not two characters + [InlineData("AAA")] // Not two characters + [InlineData("01")] // PrintableString, but not alpha. + [InlineData("@@")] // Boundary case, one below 'A'. + [InlineData("[[")] // Boundary case, one above 'Z'. + [InlineData("``")] // Boundary case, one below 'a'. Also tests not a PrintableString + [InlineData("{{")] // Boundary case, one above 'z'. + public static void AddCountryOrRegion_Invalid_Fails(string countryOrRegion) { X500DistinguishedNameBuilder builder = new(); AssertExtensions.Throws( @@ -84,16 +94,6 @@ public static void AddCountryOrRegion_InvalidLength_Fails(string countryOrRegion () => builder.AddCountryOrRegion(countryOrRegion)); } - [Fact] - public static void AddCountryOrRegion_InvalidContents_Fails() - { - X500DistinguishedNameBuilder builder = new(); - ArgumentException ex = AssertExtensions.Throws( - "twoLetterCode", - () => builder.AddCountryOrRegion("``")); - Assert.Contains("'PrintableString'", ex.Message); - } - [Fact] public static void AddOrganizationName_Success_EncodeToUTF8String() { From 461cada38aa0c570d8c37bd5ad42945d8a8fdb0a Mon Sep 17 00:00:00 2001 From: Kevin Jones Date: Fri, 11 Mar 2022 22:34:07 -0500 Subject: [PATCH 10/15] Code review feedback. --- .../X500DistinguishedNameBuilder.cs | 5 +++++ .../X500DistinguishedNameBuilderTests.cs | 14 +++++++++----- 2 files changed, 14 insertions(+), 5 deletions(-) diff --git a/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/X509Certificates/X500DistinguishedNameBuilder.cs b/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/X509Certificates/X500DistinguishedNameBuilder.cs index fefb2ca8f573d..218851a68b614 100644 --- a/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/X509Certificates/X500DistinguishedNameBuilder.cs +++ b/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/X509Certificates/X500DistinguishedNameBuilder.cs @@ -203,6 +203,9 @@ public void AddEmailAddress(string emailAddress) // pkcs-9-ub-pkcs9String INTEGER ::= 255 // pkcs-9-ub-emailAddress INTEGER ::= pkcs-9-ub-pkcs9String + // We don't attempt to do any input validation that the email resembles an email + // address, only that it conforms to the ASN.1 syntax. + ArgumentException.ThrowIfNullOrEmpty(emailAddress); if (emailAddress.Length > 255) @@ -467,6 +470,8 @@ private void EncodeComponent( ReadOnlySpan value, [CallerArgumentExpression("value")] string? valueParamName = null) { + // WriteEncodedValue checks that there is no trailing data, so ignore the bytesConsumed here. + // We just want to give a more appropriate error that the contents are not DER. if (!AsnDecoder.TryReadEncodedValue(value, AsnEncodingRules.DER, out _, out _, out _, out _)) { throw new ArgumentException(SR.Argument_Asn1_InvalidDer, valueParamName); diff --git a/src/libraries/System.Security.Cryptography/tests/X509Certificates/X500DistinguishedNameBuilderTests.cs b/src/libraries/System.Security.Cryptography/tests/X509Certificates/X500DistinguishedNameBuilderTests.cs index 4a73da9f69b9f..c4a293686b3c3 100644 --- a/src/libraries/System.Security.Cryptography/tests/X509Certificates/X500DistinguishedNameBuilderTests.cs +++ b/src/libraries/System.Security.Cryptography/tests/X509Certificates/X500DistinguishedNameBuilderTests.cs @@ -11,12 +11,14 @@ public static class X500DistinguishedNameBuilderTests { private const string TestOid = "2.25.77135202736018529853602245419149860647"; - [Fact] - public static void AddEmailAddress_Success_EncodedToIA5() + [Theory] + [InlineData("kevin@example.com", "30223120301E06092A864886F70D01090116116B6576696E406578616D706C652E636F6D")] + [InlineData("totes not an email", "30233121301F06092A864886F70D0109011612746F746573206E6F7420616E20656D61696C")] + public static void AddEmailAddress_Success_EncodedToIA5(string emailAddress, string expectedHex) { AssertBuilder( - "30223120301E06092A864886F70D01090116116B6576696E406578616D706C652E636F6D", - builder => builder.AddEmailAddress("kevin@example.com")); + expectedHex, + builder => builder.AddEmailAddress(emailAddress)); } [Fact] @@ -121,7 +123,9 @@ public static void AddOrganizationalUnitName_NullOrEmpty_Fails() [Fact] public static void AddStateOrProvinceName_Success_EncodeToUTF8String() { - AssertBuilder("300D310B300906035504080C024341", builder => builder.AddStateOrProvinceName("CA")); + AssertBuilder( + "30153113301106035504080C0A43616C69666F726E6961", + builder => builder.AddStateOrProvinceName("California")); } [Fact] From 6526911edc3ba571e7b7f7a77341bfc442944a70 Mon Sep 17 00:00:00 2001 From: Kevin Jones Date: Sat, 12 Mar 2022 12:26:59 -0500 Subject: [PATCH 11/15] Use a shared writer --- .../X500DistinguishedNameBuilder.cs | 35 ++++++++++--------- .../X500DistinguishedNameBuilderTests.cs | 16 +++++++++ 2 files changed, 34 insertions(+), 17 deletions(-) diff --git a/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/X509Certificates/X500DistinguishedNameBuilder.cs b/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/X509Certificates/X500DistinguishedNameBuilder.cs index 218851a68b614..c7d4b47b6cbc6 100644 --- a/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/X509Certificates/X500DistinguishedNameBuilder.cs +++ b/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/X509Certificates/X500DistinguishedNameBuilder.cs @@ -15,6 +15,7 @@ namespace System.Security.Cryptography.X509Certificates public sealed class X500DistinguishedNameBuilder { private readonly List _encodedComponents = new List(); + private readonly AsnWriter _writer = new AsnWriter(AsnEncodingRules.DER); /// /// Adds a Relative Distinguished Name attribute identified by an OID. @@ -422,18 +423,18 @@ public void AddDomainComponent(string domainComponent) /// public X500DistinguishedName Build() { - AsnWriter writer = new AsnWriter(AsnEncodingRules.DER); + _writer.Reset(); - using (writer.PushSequence()) + using (_writer.PushSequence()) { foreach (byte[] component in _encodedComponents) { - writer.WriteEncodedValue(component); + _writer.WriteEncodedValue(component); } } - byte[] rented = CryptoPool.Rent(writer.GetEncodedLength()); - int encoded = writer.Encode(rented); + byte[] rented = CryptoPool.Rent(_writer.GetEncodedLength()); + int encoded = _writer.Encode(rented); X500DistinguishedName name = new X500DistinguishedName(rented.AsSpan(0, encoded)); CryptoPool.Return(rented, clearSize: 0); // Distinguished Names do not contain sensitive information. return name; @@ -445,16 +446,16 @@ private void EncodeComponent( UniversalTagNumber stringEncodingType, [CallerArgumentExpression("value")] string? paramName = null) { - AsnWriter writer = new AsnWriter(AsnEncodingRules.DER); + _writer.Reset(); - using (writer.PushSetOf()) - using (writer.PushSequence()) + using (_writer.PushSetOf()) + using (_writer.PushSequence()) { - writer.WriteObjectIdentifier(oid); + _writer.WriteObjectIdentifier(oid); try { - writer.WriteCharacterString(stringEncodingType, value); + _writer.WriteCharacterString(stringEncodingType, value); } catch (EncoderFallbackException) { @@ -462,7 +463,7 @@ private void EncodeComponent( } } - _encodedComponents.Add(writer.Encode()); + _encodedComponents.Add(_writer.Encode()); } private void EncodeComponent( @@ -477,16 +478,16 @@ private void EncodeComponent( throw new ArgumentException(SR.Argument_Asn1_InvalidDer, valueParamName); } - AsnWriter writer = new AsnWriter(AsnEncodingRules.DER); + _writer.Reset(); - using (writer.PushSetOf()) - using (writer.PushSequence()) + using (_writer.PushSetOf()) + using (_writer.PushSequence()) { - writer.WriteObjectIdentifier(oid); - writer.WriteEncodedValue(value); + _writer.WriteObjectIdentifier(oid); + _writer.WriteEncodedValue(value); } - _encodedComponents.Add(writer.Encode()); + _encodedComponents.Add(_writer.Encode()); } private static UniversalTagNumber GetAndValidateTagNumber(UniversalTagNumber? stringEncodingType) diff --git a/src/libraries/System.Security.Cryptography/tests/X509Certificates/X500DistinguishedNameBuilderTests.cs b/src/libraries/System.Security.Cryptography/tests/X509Certificates/X500DistinguishedNameBuilderTests.cs index c4a293686b3c3..378a87bd3dc83 100644 --- a/src/libraries/System.Security.Cryptography/tests/X509Certificates/X500DistinguishedNameBuilderTests.cs +++ b/src/libraries/System.Security.Cryptography/tests/X509Certificates/X500DistinguishedNameBuilderTests.cs @@ -286,6 +286,22 @@ public static void Add_Oid_Success(string oid, string value, UniversalTagNumber? AssertBuilder(expectedHex, builder => builder.Add(new Oid(oid, null), value, tag)); } + [Fact] + public static void Add_WithException_ValidState() + { + X500DistinguishedNameBuilder builder = new(); + + // Cause an exception to be raised, and handled. + AssertExtensions.Throws( + "emailAddress", + () => builder.AddEmailAddress("\u0411\u0430\u043d\u0430\u043d")); + + // Make sure the handled exception didn't put the builder in an invalid state. + builder.AddEmailAddress("k@example.com"); + X500DistinguishedName dn = builder.Build(); + Assert.Equal("301E311C301A06092A864886F70D010901160D6B406578616D706C652E636F6D", Convert.ToHexString(dn.RawData)); + } + public static IEnumerable AddStringTheories { get From 996e4d8cc1102d3b6d02843eea0681ee3dd6a5be Mon Sep 17 00:00:00 2001 From: Kevin Jones Date: Sat, 12 Mar 2022 12:28:53 -0500 Subject: [PATCH 12/15] Test empty builder --- .../X509Certificates/X500DistinguishedNameBuilderTests.cs | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/libraries/System.Security.Cryptography/tests/X509Certificates/X500DistinguishedNameBuilderTests.cs b/src/libraries/System.Security.Cryptography/tests/X509Certificates/X500DistinguishedNameBuilderTests.cs index 378a87bd3dc83..58f74f01a9bc9 100644 --- a/src/libraries/System.Security.Cryptography/tests/X509Certificates/X500DistinguishedNameBuilderTests.cs +++ b/src/libraries/System.Security.Cryptography/tests/X509Certificates/X500DistinguishedNameBuilderTests.cs @@ -302,6 +302,12 @@ public static void Add_WithException_ValidState() Assert.Equal("301E311C301A06092A864886F70D010901160D6B406578616D706C652E636F6D", Convert.ToHexString(dn.RawData)); } + [Fact] + public static void Build_Empty() + { + AssertBuilder("3000", builder => { }); + } + public static IEnumerable AddStringTheories { get From 80cee845913bd75ff7f37f3a725e7472999170bc Mon Sep 17 00:00:00 2001 From: Kevin Jones Date: Sat, 12 Mar 2022 13:59:04 -0500 Subject: [PATCH 13/15] More code review feedback --- .../X500DistinguishedNameBuilder.cs | 4 +- .../X500DistinguishedNameBuilderTests.cs | 53 +++++++++++++++++-- 2 files changed, 51 insertions(+), 6 deletions(-) diff --git a/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/X509Certificates/X500DistinguishedNameBuilder.cs b/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/X509Certificates/X500DistinguishedNameBuilder.cs index c7d4b47b6cbc6..fc7e598cf5c3c 100644 --- a/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/X509Certificates/X500DistinguishedNameBuilder.cs +++ b/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/X509Certificates/X500DistinguishedNameBuilder.cs @@ -427,9 +427,9 @@ public X500DistinguishedName Build() using (_writer.PushSequence()) { - foreach (byte[] component in _encodedComponents) + for (int i = _encodedComponents.Count - 1; i >= 0; i--) { - _writer.WriteEncodedValue(component); + _writer.WriteEncodedValue(_encodedComponents[i]); } } diff --git a/src/libraries/System.Security.Cryptography/tests/X509Certificates/X500DistinguishedNameBuilderTests.cs b/src/libraries/System.Security.Cryptography/tests/X509Certificates/X500DistinguishedNameBuilderTests.cs index 58f74f01a9bc9..344eda346f32a 100644 --- a/src/libraries/System.Security.Cryptography/tests/X509Certificates/X500DistinguishedNameBuilderTests.cs +++ b/src/libraries/System.Security.Cryptography/tests/X509Certificates/X500DistinguishedNameBuilderTests.cs @@ -88,6 +88,7 @@ public static void AddCountryOrRegion_Success_EncodeToPrintableString(string two [InlineData("[[")] // Boundary case, one above 'Z'. [InlineData("``")] // Boundary case, one below 'a'. Also tests not a PrintableString [InlineData("{{")] // Boundary case, one above 'z'. + [InlineData("\uD83C\uDF4C")] // Surrogate pair for U+1F34C "banana" public static void AddCountryOrRegion_Invalid_Fails(string countryOrRegion) { X500DistinguishedNameBuilder builder = new(); @@ -298,14 +299,52 @@ public static void Add_WithException_ValidState() // Make sure the handled exception didn't put the builder in an invalid state. builder.AddEmailAddress("k@example.com"); - X500DistinguishedName dn = builder.Build(); - Assert.Equal("301E311C301A06092A864886F70D010901160D6B406578616D706C652E636F6D", Convert.ToHexString(dn.RawData)); + AssertBuilder("301E311C301A06092A864886F70D010901160D6B406578616D706C652E636F6D", builder); } [Fact] public static void Build_Empty() { - AssertBuilder("3000", builder => { }); + AssertBuilder("3000", new X500DistinguishedNameBuilder()); + } + + [Fact] + public static void Build_UsableAfterBuild() + { + X500DistinguishedNameBuilder builder = new(); + builder.AddOrganizationName("GitHub"); + AssertBuilder("3011310F300D060355040A0C06476974487562", builder); + builder.AddOrganizationalUnitName("PSE"); + AssertBuilder("301F310C300A060355040B0C03505345310F300D060355040A0C06476974487562", builder); + } + + [Fact] + public static void Build_KitchenSink() + { + const string ExpectedHex = + "3081B9" + + "31133011060A0992268993F22C6401191603636F6D" + + "31163014060A0992268993F22C6401191606676974687562" + + "310C300A060355040B0C03505345" + + "310F300D060355040A0C06476974487562" + + "310B3009060355040613025553" + + "3111300F06035504080C0856697267696E6961" + + "3113301106035504070C0A416C6578616E64726961" + + "3120301E06092A864886F70D01090116116B6576696E406578616D706C652E636F6D" + + "3114301206035504030C0B4B6576696E204A6F6E6573"; + + AssertBuilder(ExpectedHex, builder => + { + builder.AddCommonName("Kevin Jones"); + builder.AddEmailAddress("kevin@example.com"); + builder.AddLocalityName("Alexandria"); + builder.AddStateOrProvinceName("Virginia"); + builder.AddCountryOrRegion("US"); + builder.AddOrganizationName("GitHub"); + builder.AddOrganizationalUnitName("PSE"); + builder.AddDomainComponent("github"); + builder.AddDomainComponent("com"); + }); } public static IEnumerable AddStringTheories @@ -397,8 +436,14 @@ private static X500DistinguishedName AssertBuilder(string expectedHex, Action Date: Sat, 12 Mar 2022 18:37:03 -0500 Subject: [PATCH 14/15] Additional code review feedback --- .../src/Resources/Strings.resx | 2 +- .../X500DistinguishedNameBuilder.cs | 22 ++++++++++++++++--- .../X500DistinguishedNameBuilderTests.cs | 14 +++++++----- 3 files changed, 28 insertions(+), 10 deletions(-) diff --git a/src/libraries/System.Security.Cryptography/src/Resources/Strings.resx b/src/libraries/System.Security.Cryptography/src/Resources/Strings.resx index 09ca702cc15a1..07e00bca2a31a 100644 --- a/src/libraries/System.Security.Cryptography/src/Resources/Strings.resx +++ b/src/libraries/System.Security.Cryptography/src/Resources/Strings.resx @@ -88,7 +88,7 @@ The contents of the string are not valid for the '{0}' ASN.1 encoding. - The contents are not valid ASN.1 under Distinguished Encoding Rules. + The contents are not valid ASN.1 under Distinguished Encoding Rules or contains trailing data. Destination is too short. diff --git a/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/X509Certificates/X500DistinguishedNameBuilder.cs b/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/X509Certificates/X500DistinguishedNameBuilder.cs index fc7e598cf5c3c..9f39ab222c484 100644 --- a/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/X509Certificates/X500DistinguishedNameBuilder.cs +++ b/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/X509Certificates/X500DistinguishedNameBuilder.cs @@ -12,6 +12,23 @@ namespace System.Security.Cryptography.X509Certificates /// /// This class facilitates building a distinguished name for an X.509 certificate. /// + /// + /// When constructing the SEQUENCE OF Relative Distinguished Names, this builder + /// constructs the SEQUENCE OF in the opposite order which they were added to the + /// builder. For example: + /// + /// builder.AddCommonName("Contoso"); + /// builder.AddCountryOrRegion("US"); + /// + /// Will produce a SEQUENCE OF with the country or region first, and the common + /// name second. Because follows RFC 4514 + /// when converting a Distinguished Name to a string, it starts with the last + /// Relative Distinuished Name component, moving backward toward the first. + /// Because the builder creates the SEQUENCE OF in reverse, and + /// is also in reverse, it gives the appearence + /// of the added attributes and the string representation of the Distinguished Name in + /// the same order. + /// public sealed class X500DistinguishedNameBuilder { private readonly List _encodedComponents = new List(); @@ -471,9 +488,8 @@ private void EncodeComponent( ReadOnlySpan value, [CallerArgumentExpression("value")] string? valueParamName = null) { - // WriteEncodedValue checks that there is no trailing data, so ignore the bytesConsumed here. - // We just want to give a more appropriate error that the contents are not DER. - if (!AsnDecoder.TryReadEncodedValue(value, AsnEncodingRules.DER, out _, out _, out _, out _)) + if (!AsnDecoder.TryReadEncodedValue(value, AsnEncodingRules.DER, out _, out _, out _, out int bytesConsumed) || + bytesConsumed != value.Length) { throw new ArgumentException(SR.Argument_Asn1_InvalidDer, valueParamName); } diff --git a/src/libraries/System.Security.Cryptography/tests/X509Certificates/X500DistinguishedNameBuilderTests.cs b/src/libraries/System.Security.Cryptography/tests/X509Certificates/X500DistinguishedNameBuilderTests.cs index 344eda346f32a..e77f1e2c94efc 100644 --- a/src/libraries/System.Security.Cryptography/tests/X509Certificates/X500DistinguishedNameBuilderTests.cs +++ b/src/libraries/System.Security.Cryptography/tests/X509Certificates/X500DistinguishedNameBuilderTests.cs @@ -183,22 +183,24 @@ public static void AddEncoded_Oid_Success() builder => builder.AddEncoded(new Oid(TestOid, null), stackalloc byte[] { 0x13, 0x01, 65 })); } - [Fact] - public static void AddEncoded_InvalidDer() + [Theory] + [InlineData(new byte[] { 0xFF, 0xFF, 0xFF })] /// Nonsense data + [InlineData(new byte[] { 0x0C, 0x04, 0x74, 0x65, 0x73, 0x74, 0x74 })] /// trailing data + public static void AddEncoded_InvalidDer(byte[] der) { X500DistinguishedNameBuilder builder = new(); AssertExtensions.Throws( "encodedValue", - () => builder.AddEncoded(TestOid, new byte[] { 0xFF, 0xFF, 0xFF })); + () => builder.AddEncoded(TestOid, der)); AssertExtensions.Throws( "encodedValue", - () => builder.AddEncoded(TestOid, stackalloc byte[] { 0xFF, 0xFF, 0xFF })); + () => builder.AddEncoded(TestOid, new ReadOnlySpan(der))); AssertExtensions.Throws( "encodedValue", - () => builder.AddEncoded(new Oid(TestOid), new byte[] { 0xFF, 0xFF, 0xFF })); + () => builder.AddEncoded(new Oid(TestOid), der)); AssertExtensions.Throws( "encodedValue", - () => builder.AddEncoded(new Oid(TestOid), stackalloc byte[] { 0xFF, 0xFF, 0xFF })); + () => builder.AddEncoded(new Oid(TestOid), new ReadOnlySpan(der))); } [Fact] From f1188900ae310c45ce216c293661a7d00165f306 Mon Sep 17 00:00:00 2001 From: Kevin Jones Date: Tue, 15 Mar 2022 00:23:24 +0000 Subject: [PATCH 15/15] Remove AddEncoded --- .../ref/System.Security.Cryptography.cs | 4 - .../src/Resources/Strings.resx | 3 - .../X500DistinguishedNameBuilder.cs | 119 ------------------ .../X500DistinguishedNameBuilderTests.cs | 83 ------------ 4 files changed, 209 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 9595d5aa28bf1..ee3a74724143d 100644 --- a/src/libraries/System.Security.Cryptography/ref/System.Security.Cryptography.cs +++ b/src/libraries/System.Security.Cryptography/ref/System.Security.Cryptography.cs @@ -2534,10 +2534,6 @@ public void AddCommonName(string commonName) { } public void AddCountryOrRegion(string twoLetterCode) { } public void AddDomainComponent(string domainComponent) { } public void AddEmailAddress(string emailAddress) { } - public void AddEncoded(System.Security.Cryptography.Oid oid, byte[] encodedValue) { } - public void AddEncoded(System.Security.Cryptography.Oid oid, System.ReadOnlySpan encodedValue) { } - public void AddEncoded(string oidValue, byte[] encodedValue) { } - public void AddEncoded(string oidValue, System.ReadOnlySpan encodedValue) { } public void AddLocalityName(string localityName) { } public void AddOrganizationalUnitName(string organizationalUnitName) { } public void AddOrganizationName(string organizationName) { } diff --git a/src/libraries/System.Security.Cryptography/src/Resources/Strings.resx b/src/libraries/System.Security.Cryptography/src/Resources/Strings.resx index 07e00bca2a31a..c96dca0819b91 100644 --- a/src/libraries/System.Security.Cryptography/src/Resources/Strings.resx +++ b/src/libraries/System.Security.Cryptography/src/Resources/Strings.resx @@ -87,9 +87,6 @@ The contents of the string are not valid for the '{0}' ASN.1 encoding. - - The contents are not valid ASN.1 under Distinguished Encoding Rules or contains trailing data. - Destination is too short. diff --git a/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/X509Certificates/X500DistinguishedNameBuilder.cs b/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/X509Certificates/X500DistinguishedNameBuilder.cs index 9f39ab222c484..f46596ab29884 100644 --- a/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/X509Certificates/X500DistinguishedNameBuilder.cs +++ b/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/X509Certificates/X500DistinguishedNameBuilder.cs @@ -101,102 +101,6 @@ public void Add(Oid oid!!, string value!!, UniversalTagNumber? stringEncodingTyp EncodeComponent(oid.Value, value, tag); } - /// - /// Adds a Relative Distinguished Name attribute identified by an OID. - /// - /// The OID of the attribute. - /// The pre-encoded value of the attribute. - /// - /// or is . - /// - /// - /// - /// is an empty string or not a valid OID. - /// - /// -or- - /// - /// does not contain valid ASN.1 as defined by the Distinguished Encoding Rules (DER). - /// - /// - public void AddEncoded(string oidValue, byte[] encodedValue!!) - { - ArgumentException.ThrowIfNullOrEmpty(oidValue); - EncodeComponent(oidValue, encodedValue); - } - - /// - /// Adds a Relative Distinguished Name attribute identified by an OID. - /// - /// The OID of the attribute. - /// The pre-encoded value of the attribute. - /// - /// is . - /// - /// - /// - /// is an empty string or not a valid OID. - /// - /// -or- - /// - /// does not contain valid ASN.1 as defined by the Distinguished Encoding Rules (DER). - /// - /// - public void AddEncoded(string oidValue, ReadOnlySpan encodedValue) - { - ArgumentException.ThrowIfNullOrEmpty(oidValue); - EncodeComponent(oidValue, encodedValue); - } - - /// - /// Adds a Relative Distinguished Name attribute identified by an OID. - /// - /// The OID of the attribute. - /// The pre-encoded value of the attribute. - /// - /// or is . - /// - /// - /// - /// does not contain a valid OID. - /// - /// -or- - /// - /// does not contain valid ASN.1 as defined by the Distinguished Encoding Rules (DER). - /// - /// - public void AddEncoded(Oid oid!!, byte[] encodedValue!!) - { - if (string.IsNullOrEmpty(oid.Value)) - throw new ArgumentException(SR.Format(SR.Arg_EmptyOrNullString_Named, "oid.Value"), nameof(oid)); - - EncodeComponent(oid.Value, encodedValue); - } - - /// - /// Adds a Relative Distinguished Name attribute identified by an OID. - /// - /// The OID of the attribute. - /// The pre-encoded value of the attribute. - /// - /// is . - /// - /// - /// - /// does not contain a valid OID. - /// - /// -or- - /// - /// does not contain valid ASN.1 as defined by the Distinguished Encoding Rules (DER). - /// - /// - public void AddEncoded(Oid oid!!, ReadOnlySpan encodedValue) - { - if (string.IsNullOrEmpty(oid.Value)) - throw new ArgumentException(SR.Format(SR.Arg_EmptyOrNullString_Named, "oid.Value"), nameof(oid)); - - EncodeComponent(oid.Value, encodedValue); - } - /// /// Adds an email address attribute. /// @@ -483,29 +387,6 @@ private void EncodeComponent( _encodedComponents.Add(_writer.Encode()); } - private void EncodeComponent( - string oid, - ReadOnlySpan value, - [CallerArgumentExpression("value")] string? valueParamName = null) - { - if (!AsnDecoder.TryReadEncodedValue(value, AsnEncodingRules.DER, out _, out _, out _, out int bytesConsumed) || - bytesConsumed != value.Length) - { - throw new ArgumentException(SR.Argument_Asn1_InvalidDer, valueParamName); - } - - _writer.Reset(); - - using (_writer.PushSetOf()) - using (_writer.PushSequence()) - { - _writer.WriteObjectIdentifier(oid); - _writer.WriteEncodedValue(value); - } - - _encodedComponents.Add(_writer.Encode()); - } - private static UniversalTagNumber GetAndValidateTagNumber(UniversalTagNumber? stringEncodingType) { switch (stringEncodingType) diff --git a/src/libraries/System.Security.Cryptography/tests/X509Certificates/X500DistinguishedNameBuilderTests.cs b/src/libraries/System.Security.Cryptography/tests/X509Certificates/X500DistinguishedNameBuilderTests.cs index e77f1e2c94efc..96c264ef4fc59 100644 --- a/src/libraries/System.Security.Cryptography/tests/X509Certificates/X500DistinguishedNameBuilderTests.cs +++ b/src/libraries/System.Security.Cryptography/tests/X509Certificates/X500DistinguishedNameBuilderTests.cs @@ -159,89 +159,6 @@ public static void AddDomainComponent_InvalidIA5_Fails() Assert.Contains("'IA5String'", ex.Message); } - [Fact] - public static void AddEncoded_StringOid_Success() - { - AssertBuilder( - "301C311A3018061369F487D9C7B5D0AEA2CCCBE8C8C7F2F6F2BE27130141", - builder => builder.AddEncoded(TestOid, new byte[] { 0x13, 0x01, 65 })); - - AssertBuilder( - "301C311A3018061369F487D9C7B5D0AEA2CCCBE8C8C7F2F6F2BE27130141", - builder => builder.AddEncoded(TestOid, stackalloc byte[] { 0x13, 0x01, 65 })); - } - - [Fact] - public static void AddEncoded_Oid_Success() - { - AssertBuilder( - "301C311A3018061369F487D9C7B5D0AEA2CCCBE8C8C7F2F6F2BE27130141", - builder => builder.AddEncoded(new Oid(TestOid, null), new byte[] { 0x13, 0x01, 65 })); - - AssertBuilder( - "301C311A3018061369F487D9C7B5D0AEA2CCCBE8C8C7F2F6F2BE27130141", - builder => builder.AddEncoded(new Oid(TestOid, null), stackalloc byte[] { 0x13, 0x01, 65 })); - } - - [Theory] - [InlineData(new byte[] { 0xFF, 0xFF, 0xFF })] /// Nonsense data - [InlineData(new byte[] { 0x0C, 0x04, 0x74, 0x65, 0x73, 0x74, 0x74 })] /// trailing data - public static void AddEncoded_InvalidDer(byte[] der) - { - X500DistinguishedNameBuilder builder = new(); - AssertExtensions.Throws( - "encodedValue", - () => builder.AddEncoded(TestOid, der)); - AssertExtensions.Throws( - "encodedValue", - () => builder.AddEncoded(TestOid, new ReadOnlySpan(der))); - AssertExtensions.Throws( - "encodedValue", - () => builder.AddEncoded(new Oid(TestOid), der)); - AssertExtensions.Throws( - "encodedValue", - () => builder.AddEncoded(new Oid(TestOid), new ReadOnlySpan(der))); - } - - [Fact] - public static void AddEncoded_StringOid_NullOrEmpty_Fails() - { - X500DistinguishedNameBuilder builder = new(); - AssertExtensions.Throws( - "oidValue", - () => builder.AddEncoded((string)null, Array.Empty())); - AssertExtensions.Throws( - "oidValue", - () => builder.AddEncoded("", Array.Empty())); - } - - [Fact] - public static void AddEncoded_Oid_NullOrEmptyValue_Fails() - { - X500DistinguishedNameBuilder builder = new(); - AssertExtensions.Throws( - "oid", - () => builder.AddEncoded((Oid)null, Array.Empty())); - AssertExtensions.Throws( - "oid", - () => builder.AddEncoded(new Oid(), Array.Empty())); - AssertExtensions.Throws( - "oid", - () => builder.AddEncoded(new Oid("", null), Array.Empty())); - } - - [Fact] - public static void AddEncoded_Value_Null_Fails() - { - X500DistinguishedNameBuilder builder = new(); - AssertExtensions.Throws( - "encodedValue", - () => builder.AddEncoded(TestOid, (byte[])null)); - AssertExtensions.Throws( - "encodedValue", - () => builder.AddEncoded(new Oid(TestOid, null), (byte[])null)); - } - [Fact] public static void Add_InvalidUniversalTagNumber() {