Skip to content

Commit 5c5e93a

Browse files
committed
refactor: minor work on Piv Attestation and RSAPublicKey
- lookup of key definitions - determine valid attestation cert
1 parent 46999f0 commit 5c5e93a

File tree

4 files changed

+61
-33
lines changed

4 files changed

+61
-33
lines changed

Yubico.YubiKey/src/Yubico/YubiKey/Cryptography/KeyDefinitions.cs

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -107,6 +107,8 @@ public static KeyDefinition GetByRSALength(int keySizeBits)
107107

108108
throw new NotSupportedException($"Unsupported RSA length: {keySizeBits}");
109109
}
110+
111+
public static KeyDefinition GetByRSAModulusLength(byte[] modulus) => GetByRSALength(modulus.Length * 8);
110112

111113
/// <summary>
112114
/// Gets a key definition by its curve type.
@@ -162,6 +164,12 @@ public static KeyDefinition GetByOid(string oid)
162164
throw new NotSupportedException(
163165
"RSA keys are not supported by this method as all RSA keys share the same OID.");
164166
}
167+
168+
if (string.Equals(oid, Oids.ECDSA, StringComparison.OrdinalIgnoreCase))
169+
{
170+
throw new NotSupportedException(
171+
"All ECDSA keys (P-256, P-384, P-521) share the same OID. Use the Curve OID instead.");
172+
}
165173

166174
var keyDefinition = AllDefinitions.Values.FirstOrDefault(d => d.AlgorithmOid == oid || d.CurveOid == oid);
167175
return keyDefinition ?? throw new NotSupportedException(

Yubico.YubiKey/src/Yubico/YubiKey/Cryptography/RSAPublicKey.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@ public sealed class RSAPublicKey : PublicKey
4545
private RSAPublicKey(RSAParameters parameters)
4646
{
4747
Parameters = parameters.DeepCopy();
48-
KeyDefinition = KeyDefinitions.GetByRSALength(parameters.Modulus.Length * 8);
48+
KeyDefinition = KeyDefinitions.GetByRSAModulusLength(parameters.Modulus);
4949
}
5050

5151
/// <inheritdoc />

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

Lines changed: 15 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,9 @@
1414

1515
using System;
1616
using System.Globalization;
17+
using System.Linq;
1718
using Yubico.Core.Iso7816;
19+
using Yubico.YubiKey.Cryptography;
1820

1921
namespace Yubico.YubiKey.Piv.Commands
2022
{
@@ -90,13 +92,6 @@ public sealed class AuthenticateSignCommand : AuthenticateCommand, IYubiKeyComma
9092
{
9193
private const byte DigestTag = 0x81;
9294

93-
private const int Rsa1024DigestDataLength = 128;
94-
private const int Rsa2048DigestDataLength = 256;
95-
private const int Rsa3072DigestDataLength = 384;
96-
private const int Rsa4096DigestDataLength = 512;
97-
private const int EccP256DigestDataLength = 32;
98-
private const int EccP384DigestDataLength = 48;
99-
10095
// The default constructor explicitly defined. We don't want it to be
10196
// used.
10297
private AuthenticateSignCommand()
@@ -166,25 +161,21 @@ public AuthenticateSignCommand(ReadOnlyMemory<byte> digestData, byte slotNumber)
166161
DataTag = DigestTag;
167162
Data = digestData;
168163
SlotNumber = slotNumber;
164+
Algorithm = GetPivAlgorithm(digestData);
165+
}
169166

170-
// Determine the algorithm based on the length of the digest data.
171-
// Currently, the length of the data must be 128 (RSA-1024), 256
172-
// (RSA-2048), 384 (RSA-3072), 512 (RSA-4096), 32 (ECC-P256), or 48 (ECC-P384).
173-
// Return the PivAlgorithm, or if the length is not supported, throw an
174-
// exception.
175-
Algorithm = digestData.Length switch
167+
private static PivAlgorithm GetPivAlgorithm(ReadOnlyMemory<byte> digestData)
168+
{
169+
if (KeyDefinitions.All.Values
170+
.Where(k => k.IsRSA || k.IsEllipticCurve)
171+
.Where(k => !k.KeyType.IsCurve25519())
172+
.SingleOrDefault(k => k.LengthInBytes == digestData.Length) is { } keyDefinition)
176173
{
177-
Rsa1024DigestDataLength => PivAlgorithm.Rsa1024,
178-
Rsa2048DigestDataLength => PivAlgorithm.Rsa2048,
179-
Rsa3072DigestDataLength => PivAlgorithm.Rsa3072,
180-
Rsa4096DigestDataLength => PivAlgorithm.Rsa4096,
181-
EccP256DigestDataLength => PivAlgorithm.EccP256,
182-
EccP384DigestDataLength => PivAlgorithm.EccP384,
183-
_ => throw new ArgumentException(
184-
string.Format(
185-
CultureInfo.CurrentCulture,
186-
ExceptionMessages.IncorrectDigestLength)),
187-
};
174+
return keyDefinition.KeyType.GetPivAlgorithm();
175+
}
176+
177+
throw new ArgumentException(
178+
string.Format(CultureInfo.CurrentCulture, ExceptionMessages.IncorrectDigestLength));
188179
}
189180

190181
/// <summary>

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

Lines changed: 37 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
using System.Globalization;
1818
using System.Numerics;
1919
using System.Security;
20+
using System.Security.Cryptography;
2021
using System.Security.Cryptography.X509Certificates;
2122
using Yubico.Core.Tlv;
2223
using Yubico.YubiKey.Cryptography;
@@ -118,7 +119,6 @@ public X509Certificate2 CreateAttestationStatement(byte slotNumber)
118119
// This call will throw an exception if there was a problem with
119120
// attestation (imported, invalid cert, etc.).
120121
return response.GetData();
121-
122122
}
123123

124124
/// <summary>
@@ -404,8 +404,7 @@ private void CheckVersionKeyAndCertRequirements(KeyType keyType, X509Certificate
404404
ExceptionMessages.NotSupportedByYubiKeyVersion));
405405
}
406406

407-
bool isValidCert = IsSupportedCert(certificate, keyType);
408-
if (!isValidCert)
407+
if (!IsSupportedCert(certificate, keyType))
409408
{
410409
throw new ArgumentException(
411410
string.Format(
@@ -414,19 +413,48 @@ private void CheckVersionKeyAndCertRequirements(KeyType keyType, X509Certificate
414413
}
415414
}
416415

417-
// Is there really a cert in this variable? Is it > version 1?
418-
// If so, set certDer to the DER encoding of the cert.
419416
private static bool IsSupportedCert(X509Certificate2 certificate, KeyType keyType)
420417
{
421-
byte[] certDer = certificate.GetRawCertData();
418+
string oidValue = certificate.PublicKey.Oid.Value;
419+
bool isRsa = oidValue == Oids.RSA;
420+
if (isRsa && certificate.PublicKey.Key.KeySize == KeyDefinitions.RSA1024.LengthInBits)
421+
{
422+
// RSA 1024 is not supported for attestation.
423+
return false;
424+
}
422425

426+
var certKeyType = oidValue switch
427+
{
428+
Oids.ECDSA => GetKeyTypeForECDsa(certificate.GetECDsaPublicKey()),
429+
Oids.RSA => GetKeyTypeForRSA(certificate.GetRSAPublicKey()),
430+
Oids.Ed25519 => KeyType.Ed25519,
431+
_ => throw new ArgumentException($"Unsupported key type: {keyType}")
432+
};
433+
434+
// var certKeyType = isRsa
435+
// ? KeyDefinitions.GetByRSALength(certificate.PublicKey.Key.KeySize).KeyType
436+
// : GetKeyTypeForECDsa(certificate);
437+
bool isSameAlgorithm = certKeyType == keyType;
438+
byte[] certDer = certificate.GetRawCertData();
423439
return certificate.Handle != IntPtr.Zero &&
424440
certificate.Version > 1 &&
425441
certDer.Length is > 0 and < MaximumCertDerLength &&
426-
certificate.PublicKey.Oid.Value == keyType.GetAlgorithmOid() &&
442+
isSameAlgorithm &&
427443
IsValidCertNameAndValidity(certDer);
428444
}
429445

446+
private static KeyType GetKeyTypeForECDsa(ECDsa ecdsa)
447+
{
448+
var parameters = ecdsa.ExportParameters(false);
449+
return KeyDefinitions.GetByOid(parameters.Curve.Oid).KeyType;
450+
}
451+
452+
private static KeyType GetKeyTypeForRSA(RSA rsa)
453+
{
454+
var parameters = rsa.ExportParameters(false);
455+
return KeyDefinitions.GetByRSAModulusLength(parameters.Modulus).KeyType;
456+
}
457+
430458
private static bool IsValidCertNameAndValidity(byte[] certDer)
431459
{
432460
var reader = new AsnReader(certDer, AsnEncodingRules.BER);
@@ -445,7 +473,8 @@ private static bool IsValidCertNameAndValidity(byte[] certDer)
445473
var subject = seqTbsCert.ReadSequence();
446474
int subjectValue = GetSequenceLength(subject);
447475

448-
return validityValue < MaximumValidityValueLength && subjectValue < MaximumNameValueLength;
476+
return validityValue < MaximumValidityValueLength &&
477+
subjectValue < MaximumNameValueLength;
449478
}
450479

451480
private static BigInteger? ReadVersion(AsnReader seqTbsCert)

0 commit comments

Comments
 (0)