From 0569f636c76a43ec0f5f7517f388ffb92988c381 Mon Sep 17 00:00:00 2001 From: Jeremy Barton Date: Mon, 6 Jun 2022 17:56:02 -0700 Subject: [PATCH] Early draft iterator for X.500 name AttributeTypeAndValue values --- .../Cryptography/Asn1Reader/AsnValueReader.cs | 3 +- .../tests/CertificateCreation/DontBeACA.cs | 67 ++++++++++++++-- .../ref/System.Security.Cryptography.cs | 1 + .../src/System.Security.Cryptography.csproj | 4 +- .../X500DictionaryStringHelper.cs | 27 +++++++ .../X509Certificates/X500DistinguishedName.cs | 80 ++++++++++++++++++- 6 files changed, 172 insertions(+), 10 deletions(-) diff --git a/src/libraries/Common/src/System/Security/Cryptography/Asn1Reader/AsnValueReader.cs b/src/libraries/Common/src/System/Security/Cryptography/Asn1Reader/AsnValueReader.cs index 67e9d869a68db..8c23228315df7 100644 --- a/src/libraries/Common/src/System/Security/Cryptography/Asn1Reader/AsnValueReader.cs +++ b/src/libraries/Common/src/System/Security/Cryptography/Asn1Reader/AsnValueReader.cs @@ -173,7 +173,7 @@ internal AsnValueReader ReadSequence(Asn1Tag? expectedTag = default) return new AsnValueReader(content, _ruleSet); } - internal AsnValueReader ReadSetOf(Asn1Tag? expectedTag = default) + internal AsnValueReader ReadSetOf(Asn1Tag? expectedTag = default, bool skipSortOrderValidation = false) { AsnDecoder.ReadSetOf( _span, @@ -181,6 +181,7 @@ internal AsnValueReader ReadSetOf(Asn1Tag? expectedTag = default) out int contentOffset, out int contentLength, out int bytesConsumed, + skipSortOrderValidation: skipSortOrderValidation, expectedTag: expectedTag); ReadOnlySpan content = _span.Slice(contentOffset, contentLength); diff --git a/src/libraries/System.Security.Cryptography.X509Certificates/tests/CertificateCreation/DontBeACA.cs b/src/libraries/System.Security.Cryptography.X509Certificates/tests/CertificateCreation/DontBeACA.cs index e4374f29e1253..d686215d0d41e 100644 --- a/src/libraries/System.Security.Cryptography.X509Certificates/tests/CertificateCreation/DontBeACA.cs +++ b/src/libraries/System.Security.Cryptography.X509Certificates/tests/CertificateCreation/DontBeACA.cs @@ -1,6 +1,7 @@ // 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 System.Net; using System.Net.Sockets; @@ -58,8 +59,6 @@ static CertificateRequest IngestRequest(byte[] pkcs10, X509Certificate2 issuerCe skipSignatureValidation: true, signerSignaturePadding: RSASignaturePadding.Pss); - // TODO: How to validate the Subject? - AsnEncodedData? challengePassword = null; foreach (AsnEncodedData otherAttribute in req.OtherRequestAttributes) @@ -396,9 +395,67 @@ static CertificateRequest IngestRequest(byte[] pkcs10, X509Certificate2 issuerCe new[] { "http://ocsp.issuer.ca.example/ocsp/" }, new[] { "http://issuer.ca.example/issuer.cer" }); - // Because of the above validation we technically know we only need to remove the SAN extension, - // but let's just build clean, for safety. - req.CertificateExtensions.Clear(); + + bool acceptSubject = true; + string? cn = null; + + if (san is not null) + { + if (req.SubjectName.RawData.Length == 2) + { + throw new InvalidOperationException("A name is required when no SAN extension is present"); + } + } + else + { + foreach ((Oid typeId, string value) in req.SubjectName.EnumerateSimpleAttributes()) + { + switch (typeId.Value) + { + case "2.5.4.3": + if (cn is not null) + { + throw new InvalidOperationException("CN was specified more than once"); + } + + if (value.Length == 0 || value.IndexOfAny(new[] { ' ', '*' }) > -1 || + !value.EndsWith(".fruit.example")) + { + throw new InvalidOperationException("CN is unauthorized"); + } + + cn = value; + break; + default: + acceptSubject = false; + break; + } + } + } + + if (!acceptSubject) + { + if (cn is null) + { + throw new InvalidOperationException("No CN provided"); + } + + // Rewrite the subject name. + X500DistinguishedNameBuilder nameBuilder = new X500DistinguishedNameBuilder(); + nameBuilder.AddCommonName(cn); + + req = new CertificateRequest( + nameBuilder.Build(), + req.PublicKey, + HashAlgorithmName.SHA256, + RSASignaturePadding.Pss); + } + else + { + // Because of the above validation we technically know we only need to remove the SAN extension, + // but let's just build clean, for safety. + req.CertificateExtensions.Clear(); + } // There may be a standard order these get written in. // This order is mostly arbitrary, except the conditional ones went last. 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 13e561351da46..2f8c5b1cae37a 100644 --- a/src/libraries/System.Security.Cryptography/ref/System.Security.Cryptography.cs +++ b/src/libraries/System.Security.Cryptography/ref/System.Security.Cryptography.cs @@ -2552,6 +2552,7 @@ public X500DistinguishedName(string distinguishedName) { } public X500DistinguishedName(string distinguishedName, System.Security.Cryptography.X509Certificates.X500DistinguishedNameFlags flag) { } public string Name { get { throw null; } } public string Decode(System.Security.Cryptography.X509Certificates.X500DistinguishedNameFlags flag) { throw null; } + public System.Collections.Generic.IEnumerable<(System.Security.Cryptography.Oid AttributeType, string Value)> EnumerateSimpleAttributes(bool reversed = true) { throw null; } public override string Format(bool multiLine) { throw null; } } public sealed partial class X500DistinguishedNameBuilder 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 c88c3b663af7c..de3f9bc10c64c 100644 --- a/src/libraries/System.Security.Cryptography/src/System.Security.Cryptography.csproj +++ b/src/libraries/System.Security.Cryptography/src/System.Security.Cryptography.csproj @@ -443,6 +443,7 @@ + @@ -850,7 +851,6 @@ - @@ -1042,7 +1042,6 @@ - @@ -1207,7 +1206,6 @@ - diff --git a/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/X509Certificates/X500DictionaryStringHelper.cs b/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/X509Certificates/X500DictionaryStringHelper.cs index 4edef81da00f2..2ef7a8cb4426b 100644 --- a/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/X509Certificates/X500DictionaryStringHelper.cs +++ b/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/X509Certificates/X500DictionaryStringHelper.cs @@ -33,5 +33,32 @@ internal static string ReadAnyAsnString(this AsnReader tavReader) throw new CryptographicException(SR.Cryptography_Der_Invalid_Encoding); } } + + internal static string ReadAnyAsnString(ref this AsnValueReader tavReader) + { + Asn1Tag tag = tavReader.PeekTag(); + + if (tag.TagClass != TagClass.Universal) + { + throw new CryptographicException(SR.Cryptography_Der_Invalid_Encoding); + } + + switch ((UniversalTagNumber)tag.TagValue) + { + case UniversalTagNumber.BMPString: + case UniversalTagNumber.IA5String: + case UniversalTagNumber.NumericString: + case UniversalTagNumber.PrintableString: + case UniversalTagNumber.UTF8String: + case UniversalTagNumber.T61String: + // .NET's string comparisons start by checking the length, so a trailing + // NULL character which was literally embedded in the DER would cause a + // failure in .NET whereas it wouldn't have with strcmp. + return tavReader.ReadCharacterString((UniversalTagNumber)tag.TagValue).TrimEnd('\0'); + + default: + throw new CryptographicException(SR.Cryptography_Der_Invalid_Encoding); + } + } } } diff --git a/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/X509Certificates/X500DistinguishedName.cs b/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/X509Certificates/X500DistinguishedName.cs index 1da4b68c8fa37..cc61b84820167 100644 --- a/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/X509Certificates/X500DistinguishedName.cs +++ b/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/X509Certificates/X500DistinguishedName.cs @@ -1,10 +1,16 @@ // 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 X500DistinguishedName : AsnEncodedData { + private volatile string? _lazyDistinguishedName; + private List<(Oid, string)>? _parsedAttributes; + public X500DistinguishedName(byte[] encodedDistinguishedName) : base(new Oid(null, null), encodedDistinguishedName) { @@ -69,6 +75,28 @@ public override string Format(bool multiLine) return X509Pal.Instance.X500DistinguishedNameFormat(RawData, multiLine); } + /// + /// Enumerates over the X500DistinguishedName, showing the attribute type identifier and attribute value + /// at each step in the enumeration. + /// + /// + /// to enumerate in the order used by ; + /// to enumerate in the declared order. + /// + /// + /// An enumerator that iterates over the attributes in the X.500 Dinstinguished Name. + /// + /// + /// The X.500 Name is not a proper DER-encoded X.500 Name value, or the X.500 Name contains + /// multiple-value Relative Distinguished Names. + /// + public IEnumerable<(Oid AttributeType, string Value)> EnumerateSimpleAttributes(bool reversed = true) + { + List<(Oid, string)> parsedAttributes = _parsedAttributes ??= ParseAttributes(RawData); + + return EnumerateParsedAttributes(parsedAttributes, reversed); + } + private static byte[] Encode(string distinguishedName, X500DistinguishedNameFlags flags) { ArgumentNullException.ThrowIfNull(distinguishedName); @@ -87,6 +115,56 @@ private static void ThrowIfInvalid(X500DistinguishedNameFlags flags) throw new ArgumentException(SR.Format(SR.Arg_EnumIllegalVal, "flag")); } - private volatile string? _lazyDistinguishedName; + private static IEnumerable<(Oid AttributeType, string AttributeValue)> EnumerateParsedAttributes( + List<(Oid, string)> parsedAttributes, + bool reversed) + { + if (reversed) + { + for (int i = parsedAttributes.Count - 1; i >= 0; i--) + { + yield return parsedAttributes[i]; + } + } + else + { + for (int i = 0; i < parsedAttributes.Count; i++) + { + yield return parsedAttributes[i]; + } + } + } + + private static List<(Oid, string)> ParseAttributes(byte[] rawData) + { + List<(Oid, string)>? parsedAttributes = null; + + try + { + AsnValueReader outer = new AsnValueReader(rawData, AsnEncodingRules.DER); + AsnValueReader sequence = outer.ReadSequence(); + outer.ThrowIfNotEmpty(); + + while (sequence.HasData) + { + // If the set has multiple values we're going to throw, so don't bother checking that they're sorted. + AsnValueReader set = sequence.ReadSetOf(skipSortOrderValidation: true); + AsnValueReader typeAndValue = set.ReadSequence(); + set.ThrowIfNotEmpty(); + + string type = typeAndValue.ReadObjectIdentifier(); + string value = typeAndValue.ReadAnyAsnString(); + typeAndValue.ThrowIfNotEmpty(); + + (parsedAttributes ??= new List<(Oid, string)>()).Add((new Oid(type, null), value)); + } + } + catch (AsnContentException e) + { + throw new CryptographicException(SR.Cryptography_Der_Invalid_Encoding, e); + } + + return parsedAttributes ?? new List<(Oid, string)>(); + } } }