Improve Code Quality

Src/Fido2/AttestationFormat/AndroidSafetyNet.cs

@@ -20,19 +20,6 @@ internal sealed class AndroidSafetyNet : AttestationVerifier
 {
     private const int _driftTolerance = 0;
 
-    private static X509Certificate2 GetX509Certificate(string certString)
-    {
-        try
-        {
-            var certBytes = Convert.FromBase64String(certString);
-            return new X509Certificate2(certBytes);
-        }
-        catch (Exception ex)
-        {
-            throw new ArgumentException("Could not parse X509 certificate", ex);
-        }
-    }
-
     public override (AttestationType, X509Certificate2[]) Verify()
     {
         // 1. Verify that attStmt is valid CBOR conforming to the syntax defined above and perform 
@@ -45,7 +32,7 @@ public override (AttestationType, X509Certificate2[]) Verify()
             throw new Fido2VerificationException("Invalid version in SafetyNet data");
         }
 
-        if (!(_attStmt["response"] is CborByteString { Length: > 0}))
+        if (!(_attStmt["response"] is CborByteString { Length: > 0 }))
             throw new Fido2VerificationException("Invalid response in SafetyNet data");
 
         var response = (byte[])_attStmt["response"]!;
@@ -64,19 +51,27 @@ public override (AttestationType, X509Certificate2[]) Verify()
         using var jwtHeaderJsonDoc = JsonDocument.Parse(Base64Url.Decode(jwtHeaderString));
         var jwtHeaderJson = jwtHeaderJsonDoc.RootElement;
 
-        string[] x5cStrings = jwtHeaderJson.TryGetProperty("x5c", out var x5cEl) && x5cEl.ValueKind is JsonValueKind.Array
-            ? x5cEl.ToStringArray()
-            : throw new Fido2VerificationException("SafetyNet response JWT header missing x5c");
+        if (!jwtHeaderJson.TryGetProperty("x5c", out var x5cEl))
+        {
+            throw new Fido2VerificationException("SafetyNet response JWT header missing x5c");
+        }
 
-        if (x5cStrings.Length is 0)
+        if (!x5cEl.TryDecodeArrayOfBase64EncodedBytes(out var x5cRawKeys))
+        {        
+            throw new Fido2VerificationException("SafetyNet response JWT header has a malformed x5c value");
+        }
+
+        if (x5cRawKeys.Length is 0)
+        {
             throw new Fido2VerificationException("No keys were present in the TOC header in SafetyNet response JWT");
+        }
 
-        var certs = new X509Certificate2[x5cStrings.Length];
+        var certs = new X509Certificate2[x5cRawKeys.Length];
         var keys = new List<SecurityKey>(certs.Length);
 
         for (int i = 0; i < certs.Length; i++)
         {
-            var cert = GetX509Certificate(x5cStrings[i]);
+            var cert = X509CertificateHelper.CreateFromRawData(x5cRawKeys[i]);
             certs[i] = cert;
 
             if (cert.GetECDsaPublicKey() is ECDsa ecdsaPublicKey)

Src/Fido2/AttestationFormat/FidoU2f.cs

@@ -27,7 +27,7 @@ public override (AttestationType, X509Certificate2[]) Verify()
                 throw new Fido2VerificationException(Fido2ErrorCode.InvalidAttestation, Fido2ErrorMessages.MalformedX5c_FidoU2fAttestation);
             }
 
-            var attCert = new X509Certificate2((byte[])X5c[0]);
+            var attCert = new X509Certificate2((byte[])x5cArray[0]);
 
             // TODO : Check why this variable isn't used. Remove it or use it.
             var u2ftransports = U2FTransportsFromAttnCert(attCert.Extensions);

Src/Fido2/AttestationFormat/Packed.cs

@@ -59,7 +59,7 @@ public override (AttestationType, X509Certificate2[]?) Verify()
 
             for (int i = 0; i < trustPath.Length; i++)
             {
-                if (X5c[i] is CborByteString { Length: > 0 } x5cObject)
+                if (x5cArray[i] is CborByteString { Length: > 0 } x5cObject)
                 {
                     var x5cCert = new X509Certificate2(x5cObject.Value);
 

Src/Fido2/Extensions/BinaryWriterExtensions.cs → Src/Fido2/Extensions/IBufferWriterExtensions.cs

@@ -1,12 +1,12 @@
 using System;
+using System.Buffers;
 using System.Buffers.Binary;
-using System.IO;
 
 namespace Fido2NetLib;
 
-internal static class BinaryWriterExtensions
+internal static class IBufferWriterExtensions
 {
-    public static void WriteUInt16BigEndian(this BinaryWriter writer, ushort value)
+    public static void WriteUInt16BigEndian(this IBufferWriter<byte> writer, ushort value)
     {
         Span<byte> buffer = stackalloc byte[2];
 
@@ -15,7 +15,7 @@ public static void WriteUInt16BigEndian(this BinaryWriter writer, ushort value)
         writer.Write(buffer);
     }
 
-    public static void WriteUInt32BigEndian(this BinaryWriter writer, uint value)
+    public static void WriteUInt32BigEndian(this IBufferWriter<byte> writer, uint value)
     {
         Span<byte> buffer = stackalloc byte[4];
 

Src/Fido2/Extensions/JsonElementExtensions.cs

@@ -1,4 +1,5 @@
-using System.Text.Json;
+using System.Diagnostics.CodeAnalysis;
+using System.Text.Json;
 
 namespace Fido2NetLib;
 
@@ -19,4 +20,29 @@ public static string[] ToStringArray(this in JsonElement el)
 
         return result;
     }
+
+    public static bool TryDecodeArrayOfBase64EncodedBytes(this in JsonElement el, [NotNullWhen(true)] out byte[][]? result)
+    {
+        if (el.ValueKind is JsonValueKind.Array)
+        {
+            result = new byte[el.GetArrayLength()][];
+
+            int i = 0;
+
+            try
+            {
+                foreach (var item in el.EnumerateArray())
+                {
+                    result[i++] = item.GetBytesFromBase64()!;
+                }
+
+                return true;
+            }
+            catch { }
+        }
+
+        result = null;
+
+        return false;
+    }
 }

Src/Fido2/Extensions/X509CertificateHelper.cs

@@ -0,0 +1,35 @@
+using System;
+using System.Security.Cryptography.X509Certificates;
+
+namespace Fido2NetLib;
+
+internal static class X509CertificateHelper
+{
+    public static X509Certificate2 CreateFromBase64String(string base64String)
+    {
+        byte[] rawData;
+
+        try
+        {
+            rawData = Convert.FromBase64String(base64String);
+        }
+        catch 
+        {
+            throw new Exception("Invalid base64 data found parsing X509 certificate");
+        }
+
+        return CreateFromRawData(rawData);
+    }
+
+    public static X509Certificate2 CreateFromRawData(byte[] rawData)
+    {
+        try
+        {
+            return new X509Certificate2(rawData);
+        }
+        catch (Exception ex)
+        {
+            throw new Exception("Could not parse X509 certificate", ex);
+        }
+    }
+}

Src/Fido2/Metadata/ConformanceMetadataRepository.cs

@@ -112,19 +112,6 @@ private Task<byte[]> DownloadDataAsync(string url, CancellationToken cancellatio
         return _httpClient.GetByteArrayAsync(url, cancellationToken);
     }
 
-    private X509Certificate2 GetX509Certificate(string key)
-    {
-        try
-        {
-            var certBytes = Convert.FromBase64String(key);
-            return new X509Certificate2(certBytes);
-        }
-        catch (Exception ex)
-        {
-            throw new ArgumentException("Could not parse X509 certificate.", ex);
-        }
-    }
-
     public async Task<MetadataBLOBPayload> DeserializeAndValidateBlob(string rawBLOBJwt, CancellationToken cancellationToken = default)
     {
         if (string.IsNullOrWhiteSpace(rawBLOBJwt))
@@ -133,27 +120,33 @@ public async Task<MetadataBLOBPayload> DeserializeAndValidateBlob(string rawBLOB
         var jwtParts = rawBLOBJwt.Split('.');
 
         if (jwtParts.Length != 3)
-            throw new ArgumentException("The JWT does not have the 3 expected components");
+            throw new Fido2MetadataException("The JWT does not have the 3 expected components");
 
         var blobHeader = jwtParts[0];
         using var jsonDoc = JsonDocument.Parse(Base64Url.Decode(blobHeader));
         var tokenHeader = jsonDoc.RootElement;
 
         var blobAlg = tokenHeader.TryGetProperty("alg", out var algEl)
             ? algEl.GetString()!
-            : throw new ArgumentNullException("No alg value was present in the BLOB header.");
+            : throw new Fido2MetadataException("No alg value was present in the BLOB header.");
+
+        if (!tokenHeader.TryGetProperty("x5c", out var x5cEl))
+        {
+            throw new Fido2MetadataException("No x5c array was present in the BLOB header.");
+        }
 
-        var blobCertStrings = tokenHeader.TryGetProperty("x5c", out var x5cEl) && x5cEl.ValueKind is JsonValueKind.Array
-            ? x5cEl.ToStringArray()
-            : throw new ArgumentException("No x5c array was present in the BLOB header.");
+        if (!x5cEl.TryDecodeArrayOfBase64EncodedBytes(out var x5cRawKeys))
+        {
+            throw new Fido2MetadataException("Malformed x5c array in the BLOB header.");
+        }
 
-        var rootCert = GetX509Certificate(ROOT_CERT);
-        var blobCertificates = new X509Certificate2[blobCertStrings.Length]; 
-        var blobPublicKeys = new List<SecurityKey>(blobCertStrings.Length);
+        var rootCert = X509CertificateHelper.CreateFromBase64String(ROOT_CERT);
+        var blobCertificates = new X509Certificate2[x5cRawKeys.Length]; 
+        var blobPublicKeys = new List<SecurityKey>(x5cRawKeys.Length);
 
-        for (int i = 0; i < blobCertStrings.Length; i++)
+        for (int i = 0; i < x5cRawKeys.Length; i++)
         {
-            var cert = GetX509Certificate(blobCertStrings[i]);
+            var cert = X509CertificateHelper.CreateFromRawData(x5cRawKeys[i]);
             blobCertificates[i] = cert;
 
             if (cert.GetECDsaPublicKey() is ECDsa ecdsaPublicKey)
@@ -213,7 +206,7 @@ public async Task<MetadataBLOBPayload> DeserializeAndValidateBlob(string rawBLOB
             // otherwise we have to manually validate that the root in the chain we are testing is the root we downloaded
             if (rootCert.Thumbprint.Equals(certChain.ChainElements[^1].Certificate.Thumbprint, StringComparison.Ordinal) &&
                 // and that the number of elements in the chain accounts for what was in x5c plus the root we added
-                certChain.ChainElements.Count == (blobCertStrings.Length + 1) &&
+                certChain.ChainElements.Count == (x5cRawKeys.Length + 1) &&
                 // and that the root cert has exactly one status listed against it
                 certChain.ChainElements[^1].ChainElementStatus.Length == 1 &&
                 // and that that status is a status of exactly UntrustedRoot

Src/Fido2/Metadata/Fido2MetadataServiceRepository.cs

@@ -76,22 +76,8 @@ private async Task<byte[]> DownloadDataAsync(string url, CancellationToken cance
             .GetByteArrayAsync(url, cancellationToken);
     }
 
-    private X509Certificate2 GetX509Certificate(string certString)
-    {
-        try
-        {
-            var certBytes = Convert.FromBase64String(certString);
-            return new X509Certificate2(certBytes);
-        }
-        catch (Exception ex)
-        {
-            throw new ArgumentException("Could not parse X509 certificate.", ex);
-        }
-    }
-
     private async Task<MetadataBLOBPayload> DeserializeAndValidateBlobAsync(string rawBLOBJwt, CancellationToken cancellationToken)
     {
-
         if (string.IsNullOrWhiteSpace(rawBLOBJwt))
             throw new ArgumentNullException(nameof(rawBLOBJwt));
 
@@ -106,22 +92,31 @@ private async Task<MetadataBLOBPayload> DeserializeAndValidateBlobAsync(string r
 
         string blobAlg = blobHeader.TryGetProperty("alg", out var algEl)
             ? algEl.GetString()!
-            : throw new ArgumentNullException("No alg value was present in the BLOB header.");
+            : throw new Fido2MetadataException("No alg value was present in the BLOB header");
 
-        string[] keyStrings = blobHeader.TryGetProperty("x5c", out var x5cEl) && x5cEl.ValueKind is JsonValueKind.Array
-            ? x5cEl.ToStringArray()
-            : throw new ArgumentNullException("No x5c array was present in the BLOB header.");
 
-        if (keyStrings.Length is 0)
-            throw new ArgumentException("No keys were present in the BLOB header.");
+        if (!blobHeader.TryGetProperty("x5c", out var x5cEl))
+        {
+            throw new Fido2MetadataException("No x5c value was present in the BLOB header");
+        }
+
+        if (!x5cEl.TryDecodeArrayOfBase64EncodedBytes(out var x5cRawKeys))
+        {
+            throw new Fido2MetadataException("The x5c value in the BLOB header is malformed");
+        }
+
+        if (x5cRawKeys.Length is 0)
+        {
+            throw new Fido2MetadataException("No x5c keys were present in the BLOB header");
+        }
 
-        var rootCert = GetX509Certificate(ROOT_CERT);
-        var blobCerts = new X509Certificate2[keyStrings.Length];
-        var keys = new SecurityKey[keyStrings.Length];
+        var rootCert = X509CertificateHelper.CreateFromBase64String(ROOT_CERT);
+        var blobCerts = new X509Certificate2[x5cRawKeys.Length];
+        var keys = new SecurityKey[x5cRawKeys.Length];
 
         for (int i = 0; i < blobCerts.Length; i++)
         {
-            var cert = GetX509Certificate(keyStrings[i]);
+            var cert = X509CertificateHelper.CreateFromRawData(x5cRawKeys[i]);
 
             blobCerts[i] = cert;
 
@@ -188,7 +183,7 @@ private async Task<MetadataBLOBPayload> DeserializeAndValidateBlobAsync(string r
             // otherwise we have to manually validate that the root in the chain we are testing is the root we downloaded
             if (rootCert.Thumbprint == certChain.ChainElements[^1].Certificate.Thumbprint &&
                 // and that the number of elements in the chain accounts for what was in x5c plus the root we added
-                certChain.ChainElements.Count == (keyStrings.Length + 1) &&
+                certChain.ChainElements.Count == (x5cRawKeys.Length + 1) &&
                 // and that the root cert has exactly one status listed against it
                 certChain.ChainElements[^1].ChainElementStatus.Length == 1 &&
                 // and that that status is a status of exactly UntrustedRoot