Add optional validation of attestation root certs (#164)

Resolves #159. LGTM, thanks!!!
passwordless-lib · May 21, 2020 · fe1c30b · fe1c30b
1 parent 6d9db95
commit fe1c30b
Show file tree

Hide file tree

Showing 7 changed files with 47 additions and 24 deletions.
diff --git a/Demo/Startup.cs b/Demo/Startup.cs
@@ -49,6 +49,7 @@ public void ConfigureServices(IServiceCollection services)
                 options.TimestampDriftTolerance = Configuration.GetValue<int>("fido2:timestampDriftTolerance");
                 options.MDSAccessKey = Configuration["fido2:MDSAccessKey"];
                 options.MDSCacheDirPath = Configuration["fido2:MDSCacheDirPath"];
+                options.RequireValidAttestationRoot = Configuration.GetValue<bool>("fido2:requireValidAttestationRoot");
             })
             .AddCachedMetadataService(config =>
             {
@@ -60,22 +61,7 @@ public void ConfigureServices(IServiceCollection services)
                 }
             });
 
-            services.AddFido2(options =>
-            {
-                options.ServerDomain = Configuration["fido2:serverDomain"];
-                options.ServerName = "FIDO2 Test";
-                options.Origin = Configuration["fido2:origin"];
-                options.TimestampDriftTolerance = Configuration.GetValue<int>("fido2:timestampDriftTolerance");
-            })
-            .AddCachedMetadataService(config =>
-            {
-                //They'll be used in a "first match wins" way in the order registered
-                config.AddStaticMetadataRepository();
-                if (!string.IsNullOrWhiteSpace(Configuration["fido2:MDSAccessKey"]))
-                {
-                    config.AddFidoMetadataRepository(Configuration["fido2:MDSAccessKey"]);
-                }
-            });
+
         }
 
         // This method gets called by the runtime. Use this method to configure the HTTP request pipeline.

diff --git a/Demo/appsettings.json b/Demo/appsettings.json
@@ -3,7 +3,8 @@
     "serverDomain": "localhost",
     "origin": "https://localhost:44329",
     "timestampDriftTolerance": 300000,
-    "MDSAccessKey": null
+    "MDSAccessKey": null,
+    "requireValidAttestationRoot":  false
   },  
   "Logging": {
     "IncludeScopes": false,

diff --git a/Src/Fido2.Models/Fido2Configuration.cs b/Src/Fido2.Models/Fido2Configuration.cs
@@ -15,6 +15,11 @@ public class Fido2Configuration
         /// </summary>
         public int TimestampDriftTolerance { get; set; } = 0; //Pretty sure 0 will never work - need a better default?
 
+        /// <summary>
+        /// When checking attestation, require the attestation to chain to a known root
+        /// </summary>
+        public bool RequireValidAttestationRoot { get; set; } = false;
+
         /// <summary>
         /// The size of the challenges sent to the client
         /// </summary>

diff --git a/Src/Fido2/AttestationFormat/FidoU2f.cs b/Src/Fido2/AttestationFormat/FidoU2f.cs
@@ -11,9 +11,11 @@ namespace Fido2NetLib.AttestationFormat
     internal class FidoU2f : AttestationFormat
     {
         private readonly IMetadataService _metadataService;
-        public FidoU2f(CBORObject attStmt, byte[] authenticatorData, byte[] clientDataHash, IMetadataService metadataService) : base(attStmt, authenticatorData, clientDataHash)
+        private readonly bool _requireValidAttestationRoot;
+        public FidoU2f(CBORObject attStmt, byte[] authenticatorData, byte[] clientDataHash, IMetadataService metadataService, bool requireValidAttestationRoot) : base(attStmt, authenticatorData, clientDataHash)
         {
             _metadataService = metadataService;
+            _requireValidAttestationRoot = requireValidAttestationRoot;
         }
         public override void Verify()
         {
@@ -63,6 +65,13 @@ public override void Verify()
                         valid = true;
                     }
 
+                    if (_requireValidAttestationRoot)
+                    {
+                        // because we are using AllowUnknownCertificateAuthority we have to verify that the root matches ourselves
+                        var chainRoot = chain.ChainElements[chain.ChainElements.Count - 1].Certificate;
+                        valid = valid && chainRoot.RawData.SequenceEqual(root.RawData);
+                    }
+
                     if (false == valid)
                     {
                         throw new Fido2VerificationException("Invalid certificate chain in U2F attestation");

diff --git a/Src/Fido2/AttestationFormat/Packed.cs b/Src/Fido2/AttestationFormat/Packed.cs
@@ -25,11 +25,13 @@ internal enum MetadataAttestationType
     internal class Packed : AttestationFormat
     {
         private readonly IMetadataService _metadataService;
+        private readonly bool _requireValidAttestationRoot;
 
-        public Packed(CBORObject attStmt, byte[] authenticatorData, byte[] clientDataHash, IMetadataService metadataService)
+        public Packed(CBORObject attStmt, byte[] authenticatorData, byte[] clientDataHash, IMetadataService metadataService, bool requireValidAttestationRoot)
             : base(attStmt, authenticatorData, clientDataHash)
         {
             _metadataService = metadataService;
+            _requireValidAttestationRoot = requireValidAttestationRoot;
         }
 
         public static bool IsValidPackedAttnCertSubject(string attnCertSubj)
@@ -142,6 +144,14 @@ public override void Verify()
                         }
                     }
                     var valid = chain.Build(trustPath[0]);
+
+                    if (_requireValidAttestationRoot)
+                    {
+                        // because we are using AllowUnknownCertificateAuthority we have to verify that the root matches ourselves
+                        var chainRoot = chain.ChainElements[chain.ChainElements.Count - 1].Certificate;
+                        valid = valid && chainRoot.RawData.SequenceEqual(root.RawData);
+                    }
+
                     if (false == valid)
                     {
                         throw new Fido2VerificationException("Invalid certificate chain in packed attestation");

diff --git a/Src/Fido2/AttestationFormat/Tpm.cs b/Src/Fido2/AttestationFormat/Tpm.cs
@@ -11,9 +11,12 @@ namespace Fido2NetLib.AttestationFormat
 {
     internal class Tpm : AttestationFormat
     {
-        public Tpm(CBORObject attStmt, byte[] authenticatorData, byte[] clientDataHash) 
+        private readonly bool _requireValidAttestationRoot;
+
+        public Tpm(CBORObject attStmt, byte[] authenticatorData, byte[] clientDataHash, bool requireValidAttestationRoot) 
             : base(attStmt, authenticatorData, clientDataHash)
         {
+            _requireValidAttestationRoot = requireValidAttestationRoot;
         }
 
         public static readonly Dictionary<string, X509Certificate2[]> TPMManufacturerRootMap = new Dictionary<string, X509Certificate2[]>
@@ -555,7 +558,7 @@ public override void Verify()
                 {
                     var chain = new X509Chain();
                     chain.ChainPolicy.ExtraStore.Add(tpmRoots[i]);
-                    i++;
+
                     chain.ChainPolicy.RevocationMode = X509RevocationMode.NoCheck;
                     if (tpmManufacturer == "id:FFFFF1D0")
                     {
@@ -573,6 +576,15 @@ public override void Verify()
                         }
                     }
                     valid = chain.Build(new X509Certificate2(X5c.Values.First().GetByteString()));
+
+                    if (_requireValidAttestationRoot)
+                    {
+                        // because we are using AllowUnknownCertificateAuthority we have to verify that the root matches ourselves
+                        var chainRoot = chain.ChainElements[chain.ChainElements.Count - 1].Certificate;
+                        valid = valid && chainRoot.RawData.SequenceEqual(tpmRoots[i].RawData);
+                    }
+
+                    i++;
                 }
                 if (false == valid)
                     throw new Fido2VerificationException("TPM attestation failed chain validation");

diff --git a/Src/Fido2/AuthenticatorAttestationResponse.cs b/Src/Fido2/AuthenticatorAttestationResponse.cs
@@ -135,7 +135,7 @@ public async Task<AttestationVerificationSuccess> VerifyAsync(CredentialCreateOp
 
                 case "tpm":
                     // https://www.w3.org/TR/webauthn/#tpm-attestation
-                    verifier = new Tpm(AttestationObject.AttStmt, AttestationObject.AuthData, clientDataHash);
+                    verifier = new Tpm(AttestationObject.AttStmt, AttestationObject.AuthData, clientDataHash, config.RequireValidAttestationRoot);
                     break;
 
                 case "android-key":
@@ -150,12 +150,12 @@ public async Task<AttestationVerificationSuccess> VerifyAsync(CredentialCreateOp
 
                 case "fido-u2f":
                     // https://www.w3.org/TR/webauthn/#fido-u2f-attestation
-                    verifier = new FidoU2f(AttestationObject.AttStmt, AttestationObject.AuthData, clientDataHash, metadataService);
+                    verifier = new FidoU2f(AttestationObject.AttStmt, AttestationObject.AuthData, clientDataHash, metadataService, config.RequireValidAttestationRoot);
                     break;
 
                 case "packed":
                     // https://www.w3.org/TR/webauthn/#packed-attestation
-                    verifier = new Packed(AttestationObject.AttStmt, AttestationObject.AuthData, clientDataHash, metadataService);
+                    verifier = new Packed(AttestationObject.AttStmt, AttestationObject.AuthData, clientDataHash, metadataService, config.RequireValidAttestationRoot);
                     break;
 
                 default: throw new Fido2VerificationException("Missing or unknown attestation type");