refactor: PivSession

- use AsnReader to validate x509 cert
- use negating operator in boolean expression
- removed parts about specific slots for attestation

Author: DennisDyallo

Yubico.YubiKey/src/Yubico/YubiKey/Piv/Commands/CreateAttestationStatementCommand.cs

@@ -76,7 +76,7 @@ public byte SlotNumber
             get => _slotNumber;
             set
             {
-                if (PivSlot.IsValidSlotNumberForSigning(value) == false)
+                if (!PivSlot.IsValidSlotNumberForSigning(value))
                 {
                     throw new ArgumentException(
                         string.Format(
@@ -152,7 +152,7 @@ public CreateAttestationStatementCommand()
         /// </exception>
         public CommandApdu CreateCommandApdu()
         {
-            if (PivSlot.IsValidSlotNumberForSigning(_slotNumber) == false)
+            if (!PivSlot.IsValidSlotNumberForSigning(_slotNumber))
             {
                 throw new InvalidOperationException(
                     string.Format(
@@ -161,7 +161,7 @@ public CommandApdu CreateCommandApdu()
                         _slotNumber));
             }
 
-            return new CommandApdu()
+            return new CommandApdu
             {
                 Ins = AttestInstruction,
                 P1 = SlotNumber,

Yubico.YubiKey/src/Yubico/YubiKey/Piv/PivSession.Attestation.cs

@@ -13,7 +13,9 @@
 // limitations under the License.
 
 using System;
+using System.Formats.Asn1;
 using System.Globalization;
+using System.Numerics;
 using System.Security;
 using System.Security.Cryptography.X509Certificates;
 using Yubico.Core.Tlv;
@@ -35,11 +37,6 @@ public sealed partial class PivSession : IDisposable
 
         /// <summary>
         /// Create an attestation statement for the private key in the given slot.
-        /// &gt; [!NOTE]
-        /// &gt; In version 1.0.0 of the SDK, it was not possible to get an
-        /// &gt; attestation statement for keys in slots 82 - 95 (retired key
-        /// &gt; slots). However, beginning with SDK 1.0.1, it is possible to get
-        /// &gt; attestation statements for keys in those slots.
         /// </summary>
         /// <remarks>
         /// See the User's Manual entry on
@@ -55,16 +52,7 @@ public sealed partial class PivSession : IDisposable
         /// </para>
         /// <para>
         /// It is possible to create attestation statements only for keys
-        /// generated on a YubiKey, and only for keys in the following slots:
-        /// <code>
-        ///   PivSlot.Authentication      = 9A
-        ///   PivSlot.Signing             = 9C
-        ///   PivSlot.KeyManagement       = 9D
-        ///   PivSlot.CardAuthentication  = 9E
-        ///   PivSlot.Retired1            = 82
-        ///     through
-        ///   PivSlot.Retired20           = 95
-        /// </code>
+        /// generated on a YubiKey.
         /// If the <c>slotNumber</c> argument is for any other slot, or if there
         /// is no key in the slot, or if the key in the slot was imported and not
         /// generated by the YubiKey, this method will throw an exception.
@@ -115,21 +103,22 @@ public sealed partial class PivSession : IDisposable
         /// </exception>
         public X509Certificate2 CreateAttestationStatement(byte slotNumber)
         {
-            if (YubiKey.HasFeature(YubiKeyFeature.PivAttestation))
+            if (!YubiKey.HasFeature(YubiKeyFeature.PivAttestation))
             {
-                // This call will throw an exception if the slot number is incorrect.
-                var command = new CreateAttestationStatementCommand(slotNumber);
-                var response = Connection.SendCommand(command);
-
-                // This call will throw an exception if there was a problem with
-                // attestation (imported, invalid cert, etc.).
-                return response.GetData();
+                throw new NotSupportedException(
+                    string.Format(
+                        CultureInfo.CurrentCulture,
+                        ExceptionMessages.NotSupportedByYubiKeyVersion));
             }
 
-            throw new NotSupportedException(
-                string.Format(
-                    CultureInfo.CurrentCulture,
-                    ExceptionMessages.NotSupportedByYubiKeyVersion));
+            // This call will throw an exception if the slot number is incorrect.
+            var command = new CreateAttestationStatementCommand(slotNumber);
+            var response = Connection.SendCommand(command);
+
+            // This call will throw an exception if there was a problem with
+            // attestation (imported, invalid cert, etc.).
+            return response.GetData();
+
         }
 
         /// <summary>
@@ -201,19 +190,20 @@ public X509Certificate2 GetAttestationCertificate()
                     ExceptionMessages.NotSupportedByYubiKeyVersion));
         }
 
-
         [Obsolete("Usage of PivEccPublic/PivEccPrivateKey is deprecated. Use IPublicKey, IPrivateKey instead", false)]
         public void ReplaceAttestationKeyAndCertificate(PivPrivateKey privateKey, X509Certificate2 certificate)
         {
-            byte[] certDer = CheckVersionKeyAndCertRequirements(privateKey.Algorithm.GetKeyType(), certificate);
+            CheckVersionKeyAndCertRequirements(privateKey.Algorithm.GetKeyType(), certificate);
 
+            byte[] certDer = certificate.GetRawCertData();
             var tlvWriter = new TlvWriter();
             using (tlvWriter.WriteNestedTlv(0x53))
             {
                 tlvWriter.WriteValue(0x70, certDer);
                 tlvWriter.WriteByte(0x71, 0);
                 tlvWriter.WriteValue(0xfe, null);
             }
+
             byte[] encodedCert = tlvWriter.Encode();
 
             ImportPrivateKey(PivSlot.Attestation, privateKey);
@@ -229,8 +219,8 @@ public void ReplaceAttestationKeyAndCertificate(PivPrivateKey privateKey, X509Ce
                         response.StatusWord.ToString("X4", CultureInfo.InvariantCulture)));
             }
         }
-        
-                /// <summary>
+
+        /// <summary>
         /// Replace the attestation key and certificate.
         /// </summary>
         /// <remarks>
@@ -363,21 +353,23 @@ public void ReplaceAttestationKeyAndCertificate(IPrivateKey privateKey, X509Cert
             {
                 throw new ArgumentNullException(nameof(privateKey));
             }
-            
+
             if (certificate is null)
             {
                 throw new ArgumentNullException(nameof(certificate));
             }
-            
-            byte[] certDer = CheckVersionKeyAndCertRequirements(privateKey.KeyType, certificate);
 
+            CheckVersionKeyAndCertRequirements(privateKey.KeyType, certificate);
+
+            byte[] certDer = certificate.GetRawCertData();
             var tlvWriter = new TlvWriter();
             using (tlvWriter.WriteNestedTlv(0x53))
             {
                 tlvWriter.WriteValue(0x70, certDer);
                 tlvWriter.WriteByte(0x71, 0);
                 tlvWriter.WriteValue(0xfe, null);
             }
+
             byte[] encodedCert = tlvWriter.Encode();
 
             ImportPrivateKey(PivSlot.Attestation, privateKey);
@@ -402,7 +394,7 @@ public void ReplaceAttestationKeyAndCertificate(IPrivateKey privateKey, X509Cert
         // This will throw an exception if a check fails, or if one or both
         // arguments are null, or the algorithm is unsupported.
         // Return the DER encoding of the certificate.
-        private byte[] CheckVersionKeyAndCertRequirements(KeyType keyType, X509Certificate2 certificate)
+        private void CheckVersionKeyAndCertRequirements(KeyType keyType, X509Certificate2 certificate)
         {
             if (!YubiKey.HasFeature(YubiKeyFeature.PivAttestation))
             {
@@ -412,126 +404,73 @@ private byte[] CheckVersionKeyAndCertRequirements(KeyType keyType, X509Certifica
                         ExceptionMessages.NotSupportedByYubiKeyVersion));
             }
 
-            var keyDefinition = KeyDefinitions.GetByKeyType(keyType);
-            int keySize = keyDefinition.LengthInBytes;
-
-            bool isValidCert = IsCert(certificate, out byte[] certDer);
-            isValidCert = IsCertSameAlgorithm(isValidCert, certificate, keySize, keyType);
-            isValidCert = IsCertNameAndValidity(isValidCert, certDer);
-
-            if (isValidCert == false)
+            bool isValidCert = IsSupportedCert(certificate, keyType);
+            if (!isValidCert)
             {
                 throw new ArgumentException(
                     string.Format(
                         CultureInfo.CurrentCulture,
                         ExceptionMessages.UnsupportedAttestationCert));
             }
-
-            return certDer;
         }
 
         // Is there really a cert in this variable? Is it > version 1?
         // If so, set certDer to the DER encoding of the cert.
-        private static bool IsCert(X509Certificate2 certificate, out byte[] certDer)
+        private static bool IsSupportedCert(X509Certificate2 certificate, KeyType keyType)
         {
-            certDer = Array.Empty<byte>();
+            byte[] certDer = certificate.GetRawCertData();
 
-            if (certificate is null)
-            {
-                throw new ArgumentNullException(nameof(certificate));
-            }
+            return certificate.Handle != IntPtr.Zero &&
+                certificate.Version > 1 &&
+                certDer.Length is > 0 and < MaximumCertDerLength &&
+                certificate.PublicKey.Oid.Value == keyType.GetAlgorithmOid() &&
+                IsValidCertNameAndValidity(certDer);
+        }
 
-            if (certificate.Handle != IntPtr.Zero)
-            {
-                if (certificate.Version > 1)
-                {
-                    certDer = certificate.GetRawCertData();
-                }
-            }
+        private static bool IsValidCertNameAndValidity(byte[] certDer)
+        {
+            var reader = new AsnReader(certDer, AsnEncodingRules.BER);
+            var seqCert = reader.ReadSequence();
+            var seqTbsCert = seqCert.ReadSequence();
+            _ = ReadVersion(seqTbsCert);
 
-            return certDer.Length > 0 && certDer.Length < MaximumCertDerLength;
+            // Read serial number integer, signature and issuer sequence
+            _ = seqTbsCert.ReadInteger();
+            _ = seqTbsCert.ReadSequence();
+            _ = seqTbsCert.ReadSequence();
+
+            var validity = seqTbsCert.ReadSequence();
+            int validityValue = GetSequenceLength(validity);
+
+            var subject = seqTbsCert.ReadSequence();
+            int subjectValue = GetSequenceLength(subject);
+
+            return validityValue < MaximumValidityValueLength && subjectValue < MaximumNameValueLength;
         }
 
-        // Does the cert in the object share the algorithm and key size?
-        // If the input arg isValidCert is false, don't check, just return false;
-        private static bool IsCertSameAlgorithm(bool isValidCert, X509Certificate2 certificate, int keySize, KeyType keyType)
+        private static BigInteger? ReadVersion(AsnReader seqTbsCert)
         {
-            bool returnValue = false;
-
-            if (isValidCert)
+            if (!seqTbsCert.HasData ||
+                seqTbsCert.PeekTag().TagClass != TagClass.ContextSpecific ||
+                seqTbsCert.PeekTag().TagValue != 0)
             {
-                // For ECC, the Certificate class's PublicKey property does not
-                // have a Key. But the public key is in the EncodedKeyValue
-                // property. An encoded ECC public key is a point:
-                //   04 || x-coord || y-coord   fixed length
-                // The length of each coordinate is the key size. Because keySize
-                // is given in bits, and the Length of the encoded key is given
-                // in bytes, compare ( 2 * (keySize / 8)) + 1.
-                // That's why the comparison is to (keySize / 4) + 1.
-                if (certificate.PublicKey.Oid.Value == keyType.GetAlgorithmOid())
-                {
-                    returnValue = keySize switch
-                    {
-                        256 => certificate.PublicKey.EncodedKeyValue.RawData.Length == (keySize / 4) + 1,
-                        384 => certificate.PublicKey.EncodedKeyValue.RawData.Length == (keySize / 4) + 1,
-                        2048 => certificate.PublicKey.Key.KeySize == keySize,
-                        _ => false,
-                    };
-                }
+                return null;
             }
 
-            return returnValue;
+            var versionReader = seqTbsCert.ReadSequence(new Asn1Tag(TagClass.ContextSpecific, 0));
+            return versionReader.ReadInteger();
         }
 
-        // Does the cert in the object have Validity and IssuerName encodings
-        // that are not too long?
-        // If the input arg isValidCert is false, don't check, just return false;
-        private static bool IsCertNameAndValidity(bool isValidCert, byte[] certDer)
+        private static int GetSequenceLength(AsnReader validity)
         {
-            bool returnValue = false;
-
-            if (isValidCert)
+            var clone = validity.Clone();
+            int length = 0;
+            while (clone.HasData)
             {
-                // Get some of the elements of a cert. We just want to verify
-                // their lengths.
-                // The cert is the DER encoding of
-                //   SEQ {
-                //     SEQ {
-                //       [0] EXPLICIT INTEGER OPTIONAL,
-                //       INTEGER,
-                //       AlgId (a SEQ)
-                //       IssuerName (a SEQ)
-                //       Validity (a SEQ)
-                //       SubjectName (a SEQ)
-                // We just want to know how long the SubjectName and Validity are,
-                // so "decode" them as full elements (don't decode the contents
-                // of the IssuerName and Validity SEQUENCEs).
-                var reader = new TlvReader(certDer);
-                if (reader.TryReadNestedTlv(out reader, 0x30))
-                {
-                    if (reader.TryReadNestedTlv(out reader, 0x30))
-                    {
-                        byte[] tags = new byte[] { 0xA0, 0x02, 0x30, 0x30, 0x30, 0x30 };
-                        var value = new ReadOnlyMemory<byte>[tags.Length];
-                        int index = 0;
-                        for (; index < tags.Length; index++)
-                        {
-                            if (reader.TryReadValue(out value[index], tags[index]) == false)
-                            {
-                                break;
-                            }
-                        }
-
-                        if (index >= tags.Length)
-                        {
-                            returnValue = value[4].Length < MaximumValidityValueLength &&
-                                          value[5].Length < MaximumNameValueLength;
-                        }
-                    }
-                }
+                length += clone.ReadEncodedValue().Length;
             }
 
-            return returnValue;
+            return length;
         }
     }
 }