improved documentation, minor refactorings,

Author: DennisDyallo

Yubico.Core/src/Yubico/Core/Tlv/TlvObject.cs

@@ -31,7 +31,7 @@ public class TlvObject
         public int Tag { get; }
 
         /// <summary>
-        /// Returns the value.
+        /// Returns a copy of the value.
         /// </summary>
         public Memory<byte> Value => _bytes.Skip(_offset).Take(Length).ToArray();
 

Yubico.YubiKey/src/Yubico/YubiKey/Cryptography/Curve25519PrivateKeyParameters.cs

@@ -20,33 +20,61 @@ namespace Yubico.YubiKey.Cryptography;
 
 public class Curve25519PrivateKeyParameters : IPrivateKeyParameters
 {
-    private KeyDefinition _keyDefinition { get; }
     private readonly Memory<byte> _privateKey;
+    
+    public KeyDefinition KeyDefinition { get; }
+    public KeyType KeyType => KeyDefinition.KeyType;
+    public ReadOnlyMemory<byte> PrivateKey => _privateKey;
+
     private Curve25519PrivateKeyParameters(
         ReadOnlyMemory<byte> privateKey,
-        KeyDefinition keyDefinition)
+        KeyType keyType)
     {
+        var keyDefinition = keyType.GetKeyDefinition();
         if (keyDefinition.AlgorithmOid == KeyDefinitions.CryptoOids.X25519)
         {
             AsnUtilities.VerifyX25519PrivateKey(privateKey.Span);
         }
 
         _privateKey = new byte[privateKey.Length];
-        _keyDefinition = keyDefinition;
+        KeyDefinition = keyDefinition;
 
         privateKey.CopyTo(_privateKey);
     }
 
+    /// <summary>
+    /// Exports the private key in PKCS#8 DER encoded format.
+    /// </summary>
+    /// <returns>The DER encoded private key.</returns>
     public byte[] ExportPkcs8PrivateKey() => AsnPrivateKeyWriter.EncodeToPkcs8(_privateKey, KeyType);
-    public KeyDefinition KeyDefinition => _keyDefinition;
-    public KeyType KeyType => _keyDefinition.KeyType;
-    public ReadOnlyMemory<byte> PrivateKey => _privateKey;
+
+    /// <summary>
+    /// Clears the private key.
+    /// </summary>
+    /// <remarks>
+    /// This method securely zeroes out the private key data.
+    /// </remarks>
     public void Clear() => CryptographicOperations.ZeroMemory(_privateKey.Span);
 
+    /// <summary>
+    /// Creates an instance of <see cref="Curve25519PrivateKeyParameters"/> from a PKCS#8
+    /// DER-encoded private key.
+    /// </summary>
+    /// <param name="encodedKey">
+    /// The DER-encoded private key.
+    /// </param>
+    /// <returns>
+    /// A new instance of <see cref="Curve25519PrivateKeyParameters"/>.
+    /// </returns>
+    /// <exception cref="ArgumentException">
+    /// Thrown if the algorithm OID is not X25519 or Ed25519.
+    /// </exception>
+    /// <exception cref="CryptographicException">
+    /// Thrown if the private key is invalid.
+    /// </exception>
     public static Curve25519PrivateKeyParameters CreateFromPkcs8(ReadOnlyMemory<byte> encodedKey)
     {
         var reader = new AsnReader(encodedKey, AsnEncodingRules.DER);
-        
         var seqPrivateKeyInfo = reader.ReadSequence();
         var version = seqPrivateKeyInfo.ReadInteger();
         if (version != 0)
@@ -55,16 +83,17 @@ public static Curve25519PrivateKeyParameters CreateFromPkcs8(ReadOnlyMemory<byte
         }
 
         var seqAlgorithmIdentifier = seqPrivateKeyInfo.ReadSequence();
-        string oidAlgorithm = seqAlgorithmIdentifier.ReadObjectIdentifier();
-        if (oidAlgorithm != KeyDefinitions.CryptoOids.X25519 &&
-            oidAlgorithm != KeyDefinitions.CryptoOids.Ed25519)
+        string algorithmOid = seqAlgorithmIdentifier.ReadObjectIdentifier();
+        if (algorithmOid != KeyDefinitions.CryptoOids.X25519 &&
+            algorithmOid != KeyDefinitions.CryptoOids.Ed25519)
         {
             throw new ArgumentException(
                 "Invalid curve OID. Must be: " + KeyDefinitions.CryptoOids.X25519 + " or " +
                 KeyDefinitions.CryptoOids.Ed25519);
         }
 
-        var seqPrivateKey = new AsnReader(seqPrivateKeyInfo.ReadOctetString(), AsnEncodingRules.DER);
+        using var privateKeyDataHandle = new ZeroingMemoryHandle(seqPrivateKeyInfo.ReadOctetString());
+        var seqPrivateKey = new AsnReader(privateKeyDataHandle.Data, AsnEncodingRules.DER);
         var tag = seqPrivateKey.PeekTag();
         if (tag.TagValue != 4 || tag.TagClass != TagClass.Universal)
         {
@@ -78,13 +107,17 @@ public static Curve25519PrivateKeyParameters CreateFromPkcs8(ReadOnlyMemory<byte
             throw new CryptographicException("Invalid Curve25519 private key: incorrect length");
         }
 
-        var keyDefinition = KeyDefinitions.GetByOid(oidAlgorithm);
-        return new Curve25519PrivateKeyParameters(privateKeyHandle.Data, keyDefinition);
+        var keyDefinition = KeyDefinitions.GetByOid(algorithmOid);
+        return new Curve25519PrivateKeyParameters(privateKeyHandle.Data, keyDefinition.KeyType);
 
     }
-    public static Curve25519PrivateKeyParameters CreateFromValue(ReadOnlyMemory<byte> privateKey, KeyType keyType)
-    {
-        var keyDefinition = KeyDefinitions.GetByKeyType(keyType);
-        return new Curve25519PrivateKeyParameters(privateKey, keyDefinition);
-    }
+    
+    /// <summary>
+    /// Creates an instance of <see cref="Curve25519PrivateKeyParameters"/> from the given
+    /// <paramref name="privateKey"/> and <paramref name="keyType"/>.
+    /// </summary>
+    /// <param name="privateKey">The raw private key data.</param>
+    /// <param name="keyType">The type of key this is.</param>
+    /// <returns>An instance of <see cref="Curve25519PrivateKeyParameters"/>.</returns>
+    public static Curve25519PrivateKeyParameters CreateFromValue(ReadOnlyMemory<byte> privateKey, KeyType keyType) => new(privateKey, keyType);
 }

Yubico.YubiKey/src/Yubico/YubiKey/Cryptography/Curve25519PublicKeyParameters.cs

@@ -21,25 +21,43 @@ namespace Yubico.YubiKey.Cryptography;
 
 public class Curve25519PublicKeyParameters : IPublicKeyParameters
 {
-    private KeyDefinition _keyDefinition { get; }
     private readonly Memory<byte> _publicPoint;
-    public KeyType KeyType => _keyDefinition.KeyType;
-    public KeyDefinition KeyDefinition => _keyDefinition;
+
+    public KeyDefinition KeyDefinition { get; }
+    public KeyType KeyType => KeyDefinition.KeyType;
     public ReadOnlyMemory<byte> PublicPoint => _publicPoint;
 
     private Curve25519PublicKeyParameters(
         ReadOnlyMemory<byte> publicPoint,
         KeyDefinition keyDefinition)
     {
-        _keyDefinition = keyDefinition;
+        KeyDefinition = keyDefinition;
         _publicPoint = new byte[publicPoint.Length];
 
         publicPoint.CopyTo(_publicPoint);
     }
-
+    
+    /// <summary>
+    /// Converts this public key to an ASN.1 DER encoded format (X.509 SubjectPublicKeyInfo).
+    /// </summary>
+    /// <returns>
+    /// A byte array containing the ASN.1 DER encoded public key.
+    /// </returns>
     public byte[] ExportSubjectPublicKeyInfo() =>
-        AsnPublicKeyWriter.EncodeToSubjectPublicKeyInfo(_publicPoint, _keyDefinition.KeyType);
+        AsnPublicKeyWriter.EncodeToSubjectPublicKeyInfo(_publicPoint, KeyDefinition.KeyType);
 
+    /// <summary>
+    /// Creates a new instance of <see cref="Curve25519PublicKeyParameters"/> from a DER-encoded public key.
+    /// </summary>
+    /// <param name="encodedKey">
+    /// The DER-encoded public key.
+    /// </param>
+    /// <returns>
+    /// A new instance of <see cref="Curve25519PublicKeyParameters"/>.
+    /// </returns>
+    /// <exception cref="CryptographicException">
+    /// Thrown if the public key is invalid.
+    /// </exception>
     public static Curve25519PublicKeyParameters CreateFromPkcs8(ReadOnlyMemory<byte> encodedKey)
     {
         var reader = new AsnReader(encodedKey, AsnEncodingRules.DER);
@@ -57,17 +75,27 @@ public static Curve25519PublicKeyParameters CreateFromPkcs8(ReadOnlyMemory<byte>
         return CreateFromValue(subjectPublicKey, keyType);
     }
 
-    public static Curve25519PublicKeyParameters CreateFromValue(ReadOnlyMemory<byte> publicKey, KeyType keyType)
+    /// <summary>
+    /// Creates an instance of <see cref="Curve25519PublicKeyParameters"/> from the given
+    /// <paramref name="publicPoint"/> and <paramref name="keyType"/>.
+    /// </summary>
+    /// <param name="publicPoint">The raw public key data, formatted as an compressed point.</param>
+    /// <param name="keyType">The type of key this is.</param>
+    /// <returns>An instance of <see cref="Curve25519PublicKeyParameters"/>.</returns>
+    /// <exception cref="ArgumentException">
+    /// Thrown if the public key data length is not valid.
+    /// </exception>
+    public static Curve25519PublicKeyParameters CreateFromValue(ReadOnlyMemory<byte> publicPoint, KeyType keyType)
     {
         var keyDefinition = KeyDefinitions.GetByKeyType(keyType);
-        if (publicKey.Length != keyDefinition.LengthInBytes)
+        if (publicPoint.Length != keyDefinition.LengthInBytes)
         {
             throw new ArgumentException(
                 string.Format(
                     CultureInfo.CurrentCulture,
                     ExceptionMessages.InvalidPublicKeyData));
         }
 
-        return new Curve25519PublicKeyParameters(publicKey, keyDefinition);
+        return new Curve25519PublicKeyParameters(publicPoint, keyDefinition);
     }
 }

Yubico.YubiKey/src/Yubico/YubiKey/Cryptography/ECKeyParameters.cs

@@ -24,14 +24,12 @@ namespace Yubico.YubiKey.Cryptography
     /// <remarks>
     /// This class encapsulates the parameters specific to EC private keys and
     /// contains the necessary private key data.
-    /// It extends the base <see cref="ECKeyParameters"/> class with additional validation for private key components.
     /// </remarks>
     public class ECPrivateKeyParameters : IPrivateKeyParameters
     {
         public ECParameters Parameters { get; }
         public KeyDefinition KeyDefinition { get; }
         public KeyType KeyType => KeyDefinition.KeyType;
-        public ReadOnlyMemory<byte> PrivateKey => Parameters.D.ToArray();
 
         /// <summary>
         /// Initializes a new instance of the <see cref="ECPrivateKeyParameters"/> class.
@@ -49,7 +47,7 @@ public ECPrivateKeyParameters(ECParameters parameters)
             {
                 throw new ArgumentException("Parameters must contain private key data (D value)", nameof(parameters));
             }
-            
+
             Parameters = parameters.DeepCopy();
             KeyDefinition = KeyDefinitions.GetByOid(parameters.Curve.Oid);
         }
@@ -62,7 +60,6 @@ public ECPrivateKeyParameters(ECParameters parameters)
         /// </remarks>
         /// <param name="ecdsaObject">The ECDsa object.</param>
         [Obsolete("Use factory methods instead")]
-        // TODO The constructor should be private, but not possible to have to constructors with the same signature
         public ECPrivateKeyParameters(ECDsa ecdsaObject)
         {
             if (ecdsaObject == null)
@@ -73,7 +70,7 @@ public ECPrivateKeyParameters(ECDsa ecdsaObject)
             Parameters = ecdsaObject.ExportParameters(true);
             KeyDefinition = KeyDefinitions.GetByOid(Parameters.Curve.Oid);
         }
-        
+
         public byte[] ExportPkcs8PrivateKey() => AsnPrivateKeyWriter.EncodeToPkcs8(Parameters);
 
         public void Clear()
@@ -83,37 +80,62 @@ public void Clear()
             CryptographicOperations.ZeroMemory(Parameters.D);
         }
 
+        /// <summary>
+        /// Creates a new instance of <see cref="ECPrivateKeyParameters"/> from a DER-encoded private key.
+        /// </summary>
+        /// <param name="encodedKey">
+        /// The DER-encoded private key.
+        /// </param>
+        /// <returns>
+        /// A new instance of <see cref="ECPrivateKeyParameters"/>.
+        /// </returns>
+        /// <exception cref="CryptographicException">
+        /// Thrown if the private key is invalid.
+        /// </exception>
         public static ECPrivateKeyParameters CreateFromPkcs8(ReadOnlyMemory<byte> encodedKey)
         {
             var parameters = AsnPrivateKeyReader.CreateECParameters(encodedKey);
             return CreateFromParameters(parameters);
         }
+        
         #pragma warning disable CS0618 // Type or member is obsolete.
         public static ECPrivateKeyParameters CreateFromParameters(ECParameters parameters) => new(parameters);
         #pragma warning restore CS0618 // Type or member is obsolete
+
+        /// <summary>
+        /// Creates a new instance of <see cref="ECPrivateKeyParameters"/> from the given
+        /// <paramref name="privateValue"/> and <paramref name="keyType"/>.
+        /// </summary>
+        /// <remarks>
+        /// The <paramref name="privateValue"/> is taken as the raw private key data (scalar value).
+        /// </remarks>
+        /// <param name="privateValue">The raw private key data.</param>
+        /// <param name="keyType">The type of key this is.</param>
+        /// <returns>A new instance of <see cref="ECPrivateKeyParameters"/>.</returns>
+        /// <exception cref="ArgumentException">
+        /// Thrown if the key type is not a valid EC key.
+        /// </exception>
         public static ECPrivateKeyParameters CreateFromValue(
             ReadOnlyMemory<byte> privateValue,
             KeyType keyType)
         {
-            if (keyType != KeyType.P256 &&
-                keyType != KeyType.P384 &&
-                keyType != KeyType.P521)
+            var keyDefinition = keyType.GetKeyDefinition();
+            if (keyDefinition.AlgorithmOid is not KeyDefinitions.CryptoOids.ECDSA)
             {
-                throw new ArgumentOutOfRangeException(nameof(keyType), keyType, null);
+                throw new ArgumentException("Only P-256, P-384 and P-521 are supported.", nameof(keyType));
             }
 
-            var keyDefinition = KeyDefinitions.GetByKeyType(keyType);
-            string oidValue = keyDefinition.CurveOid ?? throw new ArgumentException("Curve OID is null.");
+            string curveOid = keyDefinition.CurveOid ??
+                throw new ArgumentException("The key definition for this key type has no Curve OID is null.");
 
-            var curve = ECCurve.CreateFromOid(new Oid(oidValue));
+            var curve = ECCurve.CreateFromValue(curveOid);
             var parameters = new ECParameters
             {
                 Curve = curve,
                 D = privateValue.ToArray(),
             };
 
-            var ecdsa = ECDsa.Create(parameters);
-            return CreateFromParameters(ecdsa.ExportParameters(true));
+            return CreateFromParameters(parameters);
         }
     }
 }