Skip to content

Commit 86b6c6d

Browse files
authored
Improve Code Quality (#421)
* Rename DeserializeAndValidateBlob -> DeserializeAndValidateBlobAsync * Make AuthenticatorAttestationResponse immutable * Expose {PubArea,CertInfo}.Raw over a ReadOnlySpan<byte> * Make MDSGetEndpointResponse immutable * Make DevicePublicKeyAuthenticatorOutput immutable, and change properties that allocate to methods * Make AttestedCredentialData immutable * Add oid note * Use Concat helper * Rename ecparams -> ecParams * Make AuthenticatorAssertionResponse immutable and improve nullability annotations * Make AuthenticatorAssertionResponse.Signature a ReadOnlySpan * Improve error message in AndroidKey * Use Concat helper * Breakout TrustAnchor logic from AuthenticatorAttestationResponse * Pass {config,metadataService, cancellationToken} to DevicePublicKeyRegistrationAsync * Move CryptoUtils -> /Extensions * Introduce VerifyAttestationRequest to remove state from AttestationVerifier * Move CoseKeyTypeFromOid helper to COSE
1 parent bf75254 commit 86b6c6d

31 files changed

+601
-557
lines changed

Src/Fido2.Models/COSETypes.cs

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,6 @@
1-
namespace Fido2NetLib.Objects;
1+
using System;
2+
3+
namespace Fido2NetLib.Objects;
24

35
/// <summary>
46
/// CBOR Object Signing and Encryption RFC8152 https://tools.ietf.org/html/rfc8152
@@ -188,4 +190,14 @@ public enum EllipticCurve
188190
/// </summary>
189191
P256K = 8
190192
}
193+
194+
public static KeyType GetKeyTypeFromOid(string oid)
195+
{
196+
return oid switch
197+
{
198+
"1.2.840.10045.2.1" => KeyType.EC2, // ecPublicKey
199+
"1.2.840.113549.1.1.1" => KeyType.RSA,
200+
_ => throw new Exception($"Unknown oid. Was {oid}")
201+
};
202+
}
191203
}

Src/Fido2/AttestationFormat/AndroidKey.cs

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -130,22 +130,22 @@ public static bool IsPurposeSign(byte[] attExtBytes)
130130
return (softwareEnforcedPurposeValue is 2 && teeEnforcedPurposeValue is 2);
131131
}
132132

133-
public override (AttestationType, X509Certificate2[]) Verify()
133+
public override (AttestationType, X509Certificate2[]) Verify(VerifyAttestationRequest request)
134134
{
135135
// 1. Verify that attStmt is valid CBOR conforming to the syntax defined above and perform CBOR decoding on it to extract the contained fields
136136
// (handled in base class)
137-
if (_attStmt.Count is 0)
137+
if (request.AttStmt.Count is 0)
138138
throw new Fido2VerificationException(Fido2ErrorCode.InvalidAttestation, Fido2ErrorMessages.MissingAndroidKeyAttestationStatement);
139139

140-
if (!TryGetSig(out byte[]? sig))
140+
if (!request.TryGetSig(out byte[]? sig))
141141
throw new Fido2VerificationException(Fido2ErrorCode.InvalidAttestation, Fido2ErrorMessages.InvalidAndroidKeyAttestationSignature);
142142

143143
// 2. Verify that sig is a valid signature over the concatenation of authenticatorData and clientDataHash
144144
// using the attestation public key in attestnCert with the algorithm specified in alg
145-
if (!(X5c is CborArray { Length: > 0 } x5cArray))
145+
if (!(request.X5c is CborArray { Length: > 0 } x5cArray))
146146
throw new Fido2VerificationException(Fido2ErrorCode.InvalidAttestation, Fido2ErrorMessages.MalformedX5c_AndroidKeyAttestation);
147147

148-
if (!TryGetAlg(out var alg))
148+
if (!request.TryGetAlg(out var alg))
149149
throw new Fido2VerificationException(Fido2ErrorCode.InvalidAttestation, Fido2ErrorMessages.InvalidAndroidKeyAttestationAlgorithm);
150150

151151
var trustPath = new X509Certificate2[x5cArray.Length];
@@ -172,21 +172,21 @@ public override (AttestationType, X509Certificate2[]) Verify()
172172
X509Certificate2 androidKeyCert = trustPath[0];
173173
ECDsa androidKeyPubKey = androidKeyCert.GetECDsaPublicKey()!; // attestation public key
174174

175-
byte[] ecsig;
175+
byte[] ecSignature;
176176
try
177177
{
178-
ecsig = CryptoUtils.SigFromEcDsaSig(sig, androidKeyPubKey.KeySize);
178+
ecSignature = CryptoUtils.SigFromEcDsaSig(sig, androidKeyPubKey.KeySize);
179179
}
180180
catch (Exception ex)
181181
{
182182
throw new Fido2VerificationException(Fido2ErrorCode.InvalidAttestation, "Failed to decode android key attestation signature from ASN.1 encoded form", ex);
183183
}
184184

185-
if (!androidKeyPubKey.VerifyData(Data, ecsig, CryptoUtils.HashAlgFromCOSEAlg(alg)))
186-
throw new Fido2VerificationException(Fido2ErrorCode.InvalidAttestation, "Invalid android key attestation signature");
185+
if (!androidKeyPubKey.VerifyData(request.Data, ecSignature, CryptoUtils.HashAlgFromCOSEAlg(alg)))
186+
throw new Fido2VerificationException(Fido2ErrorCode.InvalidAttestation, Fido2ErrorMessages.InvalidAndroidKeyAttestationSignature);
187187

188188
// 3. Verify that the public key in the first certificate in x5c matches the credentialPublicKey in the attestedCredentialData in authenticatorData.
189-
if (!AuthData.AttestedCredentialData!.CredentialPublicKey.Verify(Data, sig))
189+
if (!request.AuthData.AttestedCredentialData!.CredentialPublicKey.Verify(request.Data, sig))
190190
throw new Fido2VerificationException(Fido2ErrorCode.InvalidAttestation, "Incorrect credentialPublicKey in android key attestation");
191191

192192
// 4. Verify that the attestationChallenge field in the attestation certificate extension data is identical to clientDataHash
@@ -197,7 +197,7 @@ public override (AttestationType, X509Certificate2[]) Verify()
197197
try
198198
{
199199
var attestationChallenge = GetAttestationChallenge(attExtBytes);
200-
if (!_clientDataHash.AsSpan().SequenceEqual(attestationChallenge))
200+
if (!request.ClientDataHash.SequenceEqual(attestationChallenge))
201201
throw new Fido2VerificationException(Fido2ErrorCode.InvalidAttestation, "Mismatch between attestationChallenge and hashedClientDataJson verifying android key attestation certificate extension");
202202
}
203203
catch (Exception)

Src/Fido2/AttestationFormat/AndroidSafetyNet.cs

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -20,22 +20,22 @@ internal sealed class AndroidSafetyNet : AttestationVerifier
2020
{
2121
private const int _driftTolerance = 0;
2222

23-
public override (AttestationType, X509Certificate2[]) Verify()
23+
public override (AttestationType, X509Certificate2[]) Verify(VerifyAttestationRequest request)
2424
{
2525
// 1. Verify that attStmt is valid CBOR conforming to the syntax defined above and perform
2626
// CBOR decoding on it to extract the contained fields
2727
// (handled in base class)
2828

2929
// 2. Verify that response is a valid SafetyNet response of version ver
30-
if (!TryGetVer(out string? ver))
30+
if (!request.TryGetVer(out string? ver))
3131
{
3232
throw new Fido2VerificationException("Invalid version in SafetyNet data");
3333
}
3434

35-
if (!(_attStmt["response"] is CborByteString { Length: > 0 }))
35+
if (!(request.AttStmt["response"] is CborByteString { Length: > 0 }))
3636
throw new Fido2VerificationException("Invalid response in SafetyNet data");
3737

38-
var response = (byte[])_attStmt["response"]!;
38+
var response = (byte[])request.AttStmt["response"]!;
3939
var responseJWT = Encoding.UTF8.GetString(response);
4040

4141
if (string.IsNullOrWhiteSpace(responseJWT))
@@ -153,7 +153,7 @@ public override (AttestationType, X509Certificate2[]) Verify()
153153
}
154154

155155
Span<byte> dataHash = stackalloc byte[32];
156-
SHA256.HashData(Data, dataHash);
156+
SHA256.HashData(request.Data, dataHash);
157157

158158
if (!dataHash.SequenceEqual(nonceHash))
159159
{

Src/Fido2/AttestationFormat/Apple.cs

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
using System.Linq;
44
using System.Security.Cryptography;
55
using System.Security.Cryptography.X509Certificates;
6+
67
using Fido2NetLib.Cbor;
78
using Fido2NetLib.Exceptions;
89
using Fido2NetLib.Objects;
@@ -39,10 +40,10 @@ public static byte[] GetAppleAttestationExtensionValue(X509ExtensionCollection e
3940
}
4041
}
4142

42-
public override (AttestationType, X509Certificate2[]) Verify()
43+
public override (AttestationType, X509Certificate2[]) Verify(VerifyAttestationRequest request)
4344
{
4445
// 1. Verify that attStmt is valid CBOR conforming to the syntax defined above and perform CBOR decoding on it to extract the contained fields.
45-
if (!(X5c is CborArray { Length: >= 2 } x5cArray && x5cArray[0] is CborByteString { Length: > 0 }))
46+
if (!(request.X5c is CborArray { Length: >= 2 } x5cArray && x5cArray[0] is CborByteString { Length: > 0 }))
4647
{
4748
throw new Fido2VerificationException(Fido2ErrorCode.InvalidAttestation, Fido2ErrorMessages.MalformedX5c_AppleAttestation);
4849
}
@@ -61,7 +62,7 @@ public override (AttestationType, X509Certificate2[]) Verify()
6162
var credCert = trustPath[0];
6263

6364
// 3. Concatenate authenticatorData and clientDataHash to form nonceToHash.
64-
ReadOnlySpan<byte> nonceToHash = Data;
65+
ReadOnlySpan<byte> nonceToHash = request.Data;
6566

6667
// 4. Perform SHA-256 hash of nonceToHash to produce nonce.
6768
Span<byte> nonce = stackalloc byte[32];
@@ -75,13 +76,13 @@ public override (AttestationType, X509Certificate2[]) Verify()
7576

7677
// 6. Verify credential public key matches the Subject Public Key of credCert.
7778
// First, obtain COSE algorithm being used from credential public key
78-
var coseAlg = (COSE.Algorithm)(int)CredentialPublicKey[COSE.KeyCommonParameter.Alg];
79+
var coseAlg = (COSE.Algorithm)(int)request.CredentialPublicKey[COSE.KeyCommonParameter.Alg];
7980

8081
// Next, build temporary CredentialPublicKey for comparison from credCert and COSE algorithm
8182
var cpk = new CredentialPublicKey(credCert, coseAlg);
8283

8384
// Finally, compare byte sequence of CredentialPublicKey built from credCert with byte sequence of CredentialPublicKey from AttestedCredentialData from authData
84-
if (!cpk.GetBytes().AsSpan().SequenceEqual(AuthData.AttestedCredentialData!.CredentialPublicKey.GetBytes()))
85+
if (!cpk.GetBytes().AsSpan().SequenceEqual(request.AuthData.AttestedCredentialData!.CredentialPublicKey.GetBytes()))
8586
throw new Fido2VerificationException(Fido2ErrorCode.InvalidAttestation, "Credential public key in Apple attestation does not match subject public key of credCert");
8687

8788
// 7. If successful, return implementation-specific values representing attestation type Anonymous CA and attestation trust path x5c.

Src/Fido2/AttestationFormat/AppleAppAttest.cs

Lines changed: 11 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
using System.Linq;
44
using System.Security.Cryptography;
55
using System.Security.Cryptography.X509Certificates;
6+
67
using Fido2NetLib.Cbor;
78
using Fido2NetLib.Objects;
89

@@ -46,10 +47,10 @@ public static byte[] GetAppleAppIdFromCredCertExtValue(X509ExtensionCollection e
4647
// 61707061-7474-6573-7400-000000000000
4748
public static readonly Guid prodAaguid = new("61707061-7474-6573-7400-000000000000");
4849

49-
public override (AttestationType, X509Certificate2[]) Verify()
50+
public override (AttestationType, X509Certificate2[]) Verify(VerifyAttestationRequest request)
5051
{
5152
// 1. Verify that the x5c array contains the intermediate and leaf certificates for App Attest, starting from the credential certificate in the first data buffer in the array (credcert).
52-
if (!(X5c is CborArray { Length: 2 } x5cArray && x5cArray[0] is CborByteString { Length: > 0 } && x5cArray[1] is CborByteString { Length: > 0 }))
53+
if (!(request.X5c is CborArray { Length: 2 } x5cArray && x5cArray[0] is CborByteString { Length: > 0 } && x5cArray[1] is CborByteString { Length: > 0 }))
5354
{
5455
throw new Fido2VerificationException("Malformed x5c in Apple AppAttest attestation");
5556
}
@@ -64,7 +65,7 @@ public override (AttestationType, X509Certificate2[]) Verify()
6465
chain.ChainPolicy.ExtraStore.Add(intermediateCert);
6566

6667
X509Certificate2 credCert = new((byte[])x5cArray[0]);
67-
if (AuthData.AttestedCredentialData!.AaGuid.Equals(devAaguid))
68+
if (request.AuthData.AttestedCredentialData!.AaGuid.Equals(devAaguid))
6869
{
6970
// Allow expired leaf cert in development environment
7071
chain.ChainPolicy.VerificationTime = credCert.NotBefore.AddSeconds(1);
@@ -79,13 +80,13 @@ public override (AttestationType, X509Certificate2[]) Verify()
7980
// 3. Generate a new SHA256 hash of the composite item to create nonce.
8081
// 4. Obtain the value of the credCert extension with OID 1.2.840.113635.100.8.2, which is a DER - encoded ASN.1 sequence.Decode the sequence and extract the single octet string that it contains. Verify that the string equals nonce.
8182
// Steps 2 - 4 done in the "apple" format verifier
82-
Apple apple = new();
83-
(var attType, var trustPath) = apple.Verify(_attStmt, _authenticatorData, _clientDataHash);
83+
var apple = new Apple();
84+
(var attType, var trustPath) = apple.Verify(request);
8485

8586
// 5. Create the SHA256 hash of the public key in credCert, and verify that it matches the key identifier from your app.
8687
Span<byte> credCertPKHash = stackalloc byte[32];
8788
SHA256.HashData(credCert.GetPublicKey(), credCertPKHash);
88-
var keyIdentifier = Convert.FromHexString(credCert.GetNameInfo(X509NameType.SimpleName, false));
89+
ReadOnlySpan<byte> keyIdentifier = Convert.FromHexString(credCert.GetNameInfo(X509NameType.SimpleName, false));
8990
if (!credCertPKHash.SequenceEqual(keyIdentifier))
9091
{
9192
throw new Fido2VerificationException("Public key hash does not match key identifier in Apple AppAttest attestation");
@@ -95,25 +96,25 @@ public override (AttestationType, X509Certificate2[]) Verify()
9596
var appId = GetAppleAppIdFromCredCertExtValue(credCert.Extensions);
9697
Span<byte> appIdHash = stackalloc byte[32];
9798
SHA256.HashData(appId, appIdHash);
98-
if (!appIdHash.SequenceEqual(AuthData.RpIdHash))
99+
if (!appIdHash.SequenceEqual(request.AuthData.RpIdHash))
99100
{
100101
throw new Fido2VerificationException("App ID hash does not match RP ID hash in Apple AppAttest attestation");
101102
}
102103

103104
// 7. Verify that the authenticator data's counter field equals 0.
104-
if (AuthData.SignCount != 0)
105+
if (request.AuthData.SignCount != 0)
105106
{
106107
throw new Fido2VerificationException("Sign count does not equal 0 in Apple AppAttest attestation");
107108
}
108109

109110
// 8. Verify that the authenticator data's aaguid field is either appattestdevelop if operating in the development environment, or appattest followed by seven 0x00 bytes if operating in the production environment.
110-
if (!AuthData.AttestedCredentialData.AaGuid.Equals(devAaguid) && !AuthData.AttestedCredentialData.AaGuid.Equals(prodAaguid))
111+
if (!request.AuthData.AttestedCredentialData.AaGuid.Equals(devAaguid) && !request.AuthData.AttestedCredentialData.AaGuid.Equals(prodAaguid))
111112
{
112113
throw new Fido2VerificationException("Invalid aaguid encountered in Apple AppAttest attestation");
113114
}
114115

115116
// 9. Verify that the authenticator data's credentialId field is the same as the key identifier.
116-
if (!keyIdentifier.SequenceEqual(AuthData.AttestedCredentialData.CredentialID))
117+
if (!keyIdentifier.SequenceEqual(request.AuthData.AttestedCredentialData.CredentialID))
117118
{
118119
throw new Fido2VerificationException("Mismatch between credentialId and keyIdentifier in Apple AppAttest attestation");
119120
}

0 commit comments

Comments
 (0)