Improve Code Quality (#405)

* Make UafVersion a readonly struct

* Fix UndesiredMetadataStatusFido2VerificationException class name

* Make RgbPaletteEntry a readonly struct

* Make AuthenticationExtensionsDevicePublicKeyOutputs immutable

* Make PublicKeyCredentialDescriptor immutable

* Fix typo

* Remove unused namespace

* Use new IBufferWriter.WriteGuidBigEndian helper

* Add AsnHelper.GetAaguidBlob

* Fix tests

Author: iamcarbon

Src/Fido2.Ctap2/Cbor/CborHelper.cs

@@ -7,22 +7,23 @@ internal sealed class CborHelper
 {
     public static PublicKeyCredentialDescriptor DecodePublicKeyCredentialDescriptor(CborMap map)
     {
-        var result = new PublicKeyCredentialDescriptor();
+        byte[]? id = null;
+        PublicKeyCredentialType type = default;
 
         foreach (var (key, value) in map)
         {
             switch ((string)key)
             {
                 case "id":
-                    result.Id = (byte[])value;
+                    id = (byte[])value;
                     break;
                 case "type" when (value is CborTextString { Value: "public-key" }):
-                    result.Type = PublicKeyCredentialType.PublicKey;
+                    type = PublicKeyCredentialType.PublicKey;
                     break;
             }
         }
 
-        return result;
+        return new PublicKeyCredentialDescriptor(type, id!, null);
     }
 
     public static PublicKeyCredentialUserEntity DecodePublicKeyCredentialUserEntity(CborMap map)

Src/Fido2.Models/Metadata/RgbPaletteEntry.cs

@@ -1,4 +1,4 @@
-using System.ComponentModel.DataAnnotations;
+using System;
 using System.Text.Json.Serialization;
 
 namespace Fido2NetLib;
@@ -9,23 +9,58 @@ namespace Fido2NetLib;
 /// <remarks>
 /// <see href="https://fidoalliance.org/specs/fido-v2.0-rd-20180702/fido-metadata-statement-v2.0-rd-20180702.html#rgbpaletteentry-dictionary"/>
 /// </remarks>
-public class RgbPaletteEntry
+public readonly struct RgbPaletteEntry : IEquatable<RgbPaletteEntry>
 {
+    [JsonConstructor]
+    public RgbPaletteEntry(ushort r, ushort g, ushort b)
+    {
+        R = r;
+        G = g;
+        B = b;
+    }
+
     /// <summary>
     /// Gets or sets the red channel sample value.
     /// </summary>
-    [JsonPropertyName("r"), Required]
-    public ushort R { get; set; }
+    [JsonPropertyName("r")]
+    public ushort R { get; }
 
     /// <summary>
     /// Gets or sets the green channel sample value.
     /// </summary>
-    [JsonPropertyName("g"), Required]
-    public ushort G { get; set; }
+    [JsonPropertyName("g")]
+    public ushort G { get; }
 
     /// <summary>
     /// Gets or sets the blue channel sample value.
     /// </summary>
-    [JsonPropertyName("b"), Required]
-    public ushort B { get; set; }
+    [JsonPropertyName("b")]
+    public ushort B { get; }
+
+    public bool Equals(RgbPaletteEntry other)
+    {
+        return R == other.R
+            && G == other.G
+            && B == other.B;
+    }
+
+    public override bool Equals(object obj)
+    {
+        return obj is RgbPaletteEntry other && Equals(other);
+    }
+
+    public static bool operator ==(RgbPaletteEntry left, RgbPaletteEntry right)
+    {
+        return left.Equals(right);
+    }
+
+    public static bool operator !=(RgbPaletteEntry left, RgbPaletteEntry right)
+    {
+        return !left.Equals(right);
+    }
+
+    public override int GetHashCode()
+    {
+        return HashCode.Combine(R, G, B);
+    }
 }

Src/Fido2.Models/Metadata/UafVersion.cs

@@ -1,4 +1,5 @@
-using System.Text.Json.Serialization;
+using System;
+using System.Text.Json.Serialization;
 
 namespace Fido2NetLib;
 
@@ -8,16 +9,50 @@ namespace Fido2NetLib;
 /// <remarks>
 /// https://fidoalliance.org/specs/fido-uaf-v1.2-rd-20171128/fido-uaf-protocol-v1.2-rd-20171128.html#version-interface
 /// </remarks>
-public class UafVersion
+public readonly struct UafVersion : IEquatable<UafVersion>
 {
+    [JsonConstructor]
+    public UafVersion(ushort major, ushort minor)
+    {
+        Major = major;
+        Minor = minor;
+    }
+
     /// <summary>
     /// Major version
     /// </summary>
     [JsonPropertyName("major")]
-    public ushort Major { get; set; }
+    public ushort Major { get; }
+
     /// <summary>
     /// Minor version
     /// </summary>
     [JsonPropertyName("minor")]
-    public ushort Minor { get; set; }
+    public ushort Minor { get; }
+
+    public bool Equals(UafVersion other)
+    {
+        return Major == other.Major
+            && Minor == other.Minor;
+    }
+
+    public override bool Equals(object obj)
+    {
+        return obj is UafVersion other && Equals(other);
+    }
+
+    public override int GetHashCode()
+    {
+        return HashCode.Combine(Major, Minor);
+    }
+
+    public static bool operator ==(UafVersion left, UafVersion right)
+    {
+        return left.Equals(right);
+    }
+
+    public static bool operator !=(UafVersion left, UafVersion right)
+    {
+        return !left.Equals(right);
+    }
 }

Src/Fido2.Models/Objects/AuthenticationExtensionsDevicePublicKeyOutputs.cs

@@ -1,14 +1,23 @@
-namespace Fido2NetLib.Objects;
+#nullable enable
+
+namespace Fido2NetLib.Objects;
 
 using System.Text.Json.Serialization;
 
 public sealed class AuthenticationExtensionsDevicePublicKeyOutputs
 {
+    [JsonConstructor]
+    public AuthenticationExtensionsDevicePublicKeyOutputs(byte[] authenticatorOutput, byte[] signature)
+    {
+        AuthenticatorOutput = authenticatorOutput;
+        Signature = signature;
+    }
+
     [JsonConverter(typeof(Base64UrlConverter))]
     [JsonPropertyName("authenticatorOutput")]
-    public byte[] AuthenticatorOutput { get; set; }
+    public byte[] AuthenticatorOutput { get; }
 
     [JsonConverter(typeof(Base64UrlConverter))]
     [JsonPropertyName("signature")]
-    public byte[] Signature { get; set; }
+    public byte[] Signature { get; }
 }

Src/Fido2.Models/Objects/PublicKeyCredentialDescriptor.cs

@@ -1,4 +1,7 @@
-using System.Text.Json.Serialization;
+#nullable enable
+
+using System;
+using System.Text.Json.Serialization;
 
 namespace Fido2NetLib.Objects;
 
@@ -7,37 +10,38 @@ namespace Fido2NetLib.Objects;
 /// Lazy implementation of https://www.w3.org/TR/webauthn/#dictdef-publickeycredentialdescriptor
 /// todo: Should add validation of values as specified in spec
 /// </summary>
-public class PublicKeyCredentialDescriptor
+public sealed class PublicKeyCredentialDescriptor
 {
-    public PublicKeyCredentialDescriptor(byte[] credentialId)
-    {
-        Id = credentialId;
-    }
+    public PublicKeyCredentialDescriptor(byte[] id)
+        : this(PublicKeyCredentialType.PublicKey, id, null) { }
 
-    public PublicKeyCredentialDescriptor()
+    [JsonConstructor]
+    public PublicKeyCredentialDescriptor(PublicKeyCredentialType type, byte[] id, AuthenticatorTransport[]? transports = null)
     {
+        ArgumentNullException.ThrowIfNull(id);
 
+        Type = type;
+        Id = id;
+        Transports = transports;
     }
 
     /// <summary>
     /// This member contains the type of the public key credential the caller is referring to.
     /// </summary>
     [JsonPropertyName("type")]
-    public PublicKeyCredentialType? Type { get; set; } = PublicKeyCredentialType.PublicKey;
+    public PublicKeyCredentialType Type { get; }
 
     /// <summary>
     /// This member contains the credential ID of the public key credential the caller is referring to.
     /// </summary>
     [JsonConverter(typeof(Base64UrlConverter))]
     [JsonPropertyName("id")]
-    public byte[] Id { get; set; }
-
-#nullable enable
+    public byte[] Id { get; }
 
     /// <summary>
     /// This OPTIONAL member contains a hint as to how the client might communicate with the managing authenticator of the public key credential the caller is referring to.
     /// </summary>
     [JsonPropertyName("transports")]
     [JsonIgnore(Condition = JsonIgnoreCondition.WhenWritingNull)]
-    public AuthenticatorTransport[]? Transports { get; set; }
+    public AuthenticatorTransport[]? Transports { get; }
 };

…atataStatusFido2VerificationException.cs …adataStatusFido2VerificationException.csSrc/Fido2.Models/UndesiredMetdatataStatusFido2VerificationException.cs renamed to Src/Fido2.Models/UndesiredMetadataStatusFido2VerificationException.cs Src/Fido2.Models/UndesiredMetdatataStatusFido2VerificationException.cs renamed to Src/Fido2.Models/UndesiredMetadataStatusFido2VerificationException.cs

@@ -7,14 +7,14 @@ namespace Fido2NetLib;
 /// Exception thrown when a new attestation comes from an authenticator with a current reported security issue.
 /// </summary>
 [Serializable]
-public class UndesiredMetdatataStatusFido2VerificationException : Fido2VerificationException
+public class UndesiredMetadataStatusFido2VerificationException : Fido2VerificationException
 {
-    public UndesiredMetdatataStatusFido2VerificationException(StatusReport statusReport) : base($"Authenticator found with undesirable status. Was {statusReport.Status}")
+    public UndesiredMetadataStatusFido2VerificationException(StatusReport statusReport) : base($"Authenticator found with undesirable status. Was {statusReport.Status}")
     {
         StatusReport = statusReport;
     }
 
-    protected UndesiredMetdatataStatusFido2VerificationException(SerializationInfo info, StreamingContext context) : base(info, context) { }
+    protected UndesiredMetadataStatusFido2VerificationException(SerializationInfo info, StreamingContext context) : base(info, context) { }
 
     /// <summary>
     /// Status report from the authenticator that caused the attestation to be rejected.

Src/Fido2/AuthenticatorAssertionResponse.cs

@@ -50,7 +50,7 @@ public static AuthenticatorAssertionResponse Parse(AuthenticatorAssertionRawResp
     }
 
     /// <summary>
-    /// Implements alghoritm from https://www.w3.org/TR/webauthn/#verifying-assertion
+    /// Implements algorithm from https://www.w3.org/TR/webauthn/#verifying-assertion
     /// </summary>
     /// <param name="options">The assertionoptions that was sent to the client</param>
     /// <param name="fullyQualifiedExpectedOrigins">

Src/Fido2/AuthenticatorAttestationResponse.cs

@@ -176,11 +176,11 @@ public async Task<AttestationVerificationSuccess> VerifyAsync(
         //     If ECDAA was used, verify that the identifier of the ECDAA-Issuer public key used is included in the set of acceptable trust anchors obtained in step 15.
         //     Otherwise, use the X.509 certificates returned by the verification procedure to verify that the attestation public key correctly chains up to an acceptable root certificate.
 
-        // Check status resports for authenticator with undesirable status
+        // Check status reports for authenticator with undesirable status
         var latestStatusReport = metadataEntry?.GetLatestStatusReport();
         if (latestStatusReport != null && config.UndesiredAuthenticatorMetadataStatuses.Contains(latestStatusReport.Status))
         {
-            throw new UndesiredMetdatataStatusFido2VerificationException(latestStatusReport);
+            throw new UndesiredMetadataStatusFido2VerificationException(latestStatusReport);
         }
 
         // 23. Verify that the credentialId is ≤ 1023 bytes.
@@ -274,7 +274,7 @@ byte[] hash
         var latestStatusReport = metadataEntry?.GetLatestStatusReport();
         if (latestStatusReport != null && _config.UndesiredAuthenticatorMetadataStatuses.Contains(latestStatusReport.Status))
         {
-            throw new UndesiredMetdatataStatusFido2VerificationException(latestStatusReport);
+            throw new UndesiredMetadataStatusFido2VerificationException(latestStatusReport);
         }
 
         return devicePublicKeyAuthenticatorOutput.GetBytes();

Src/Fido2/Objects/AttestedCredentialData.cs

@@ -123,21 +123,6 @@ public static Guid FromBigEndian(byte[] aaGuid)
         return new Guid(aaGuid);
     }
 
-    /// <summary>
-    /// AAGUID is sent as big endian byte array, this converter is for little endian systems.
-    /// </summary>
-    public static byte[] AaGuidToBigEndian(Guid aaGuid)
-    {
-        var aaguid = aaGuid.ToByteArray();
-
-        SwapBytes(aaguid, 0, 3);
-        SwapBytes(aaguid, 1, 2);
-        SwapBytes(aaguid, 4, 5);
-        SwapBytes(aaguid, 6, 7);
-
-        return aaguid;
-    }
-
     public override string ToString()
     {
         return $"AAGUID: {AaGuid}, CredentialID: {Convert.ToHexString(CredentialID)}, CredentialPublicKey: {CredentialPublicKey}";
@@ -154,16 +139,7 @@ public byte[] ToByteArray()
 
     public void WriteTo(IBufferWriter<byte> writer)
     {
-        // Write the aaguid as big endian bytes
-        if (BitConverter.IsLittleEndian)
-        {
-            writer.Write(AaGuidToBigEndian(AaGuid));
-        }
-        else
-        {
-            _ = AaGuid.TryWriteBytes(writer.GetSpan(16));
-            writer.Advance(16);
-        }
+        writer.WriteGuidBigEndian(AaGuid);
 
         // Write the length of credential ID, as big endian bytes of a 16-bit unsigned integer
         writer.WriteUInt16BigEndian((ushort)CredentialID.Length);

Src/Fido2/Objects/DevicePublicKeyAuthenticatorOutput.cs

@@ -2,8 +2,8 @@
 namespace Fido2NetLib.Objects;
 
 using System;
+
 using Fido2NetLib.Cbor;
-using NSec.Cryptography;
 
 
 public sealed class DevicePublicKeyAuthenticatorOutput