FIDO Server conformance tests now passing 100% (#38)

* Moved COSE related things to their own package * move assertion to cose verify * Server-ServerPublicKeyCredentialCreationOptions-Req-1 * Update login.go * Fix packed attestation signature verification and added ServerResponse structure * Conformance testing fixes for MakeCredential * Conformance tests nearly complete * Initial metadata layout * Metadata progress * Further progress on metadata * Resolving conflict * Production and conformance metadata now load * Move SafetyNet to jwt-go and add sanity check for timestamp * Certificate checks on metadata TOC * Restrict timestamp check in safetynet to conformance only * Don't return safetynet x5c * Metadata (#1) * Moved COSE related things to their own package * move assertion to cose verify * Server-ServerPublicKeyCredentialCreationOptions-Req-1 * Update login.go * Fix packed attestation signature verification and added ServerResponse structure * Conformance testing fixes for MakeCredential * Conformance tests nearly complete * Initial metadata layout * Metadata progress * Further progress on metadata * Resolving conflict * Production and conformance metadata now load * Move SafetyNet to jwt-go and add sanity check for timestamp * Certificate checks on metadata TOC * Restrict timestamp check in safetynet to conformance only * Don't return safetynet x5c * SafetyNet timestamp check * Added metadata tests * Update metadata tests
duo-labs · May 8, 2019 · aa748d7 · aa748d7 · unrealperson666 · Dec 4, 2021
1 parent b10828b
commit aa748d7
Show file tree

Hide file tree

Showing 10 changed files with 342 additions and 50 deletions.
diff --git a/metadata/metadata.go b/metadata/metadata.go
@@ -3,19 +3,24 @@ package metadata
 import (
 	"bytes"
 	"crypto"
+	"crypto/x509"
 	"encoding/base64"
 	"encoding/json"
 	"errors"
 	"io/ioutil"
 	"net/http"
 
+	"github.com/cloudflare/cfssl/revoke"
 	"github.com/mitchellh/mapstructure"
 	uuid "github.com/satori/go.uuid"
 
 	jwt "github.com/dgrijalva/jwt-go"
 )
 
+// Metadata is a map of authenticator AAGUIDs to corresponding metadata statements
 var Metadata = make(map[uuid.UUID]MetadataTOCPayloadEntry)
+
+// Conformance indicates if test metadata is currently being used
 var Conformance = false
 
 // AuthenticatorAttestationType - The ATTESTATION constants are 16 bit long integers indicating the specific attestation that authenticator supports.
@@ -382,52 +387,189 @@ type MDSGetEndpointsResponse struct {
 	Result []string `json:"result"`
 }
 
+// ProcessMDSTOC processes a FIDO metadata table of contents object per §3.1.8, steps 1 through 5
+// FIDO Authenticator Metadata Service
+// https://fidoalliance.org/specs/fido-v2.0-rd-20180702/fido-metadata-service-v2.0-rd-20180702.html#metadata-toc-object-processing-rules
 func ProcessMDSTOC(url string, suffix string, c http.Client) (MetadataTOCPayload, string, error) {
 	var tocAlg string
 	var payload MetadataTOCPayload
-
+	// 1. The FIDO Server MUST be able to download the latest metadata TOC object from the well-known URL, when appropriate.
 	body, err := downloadBytes(url+suffix, c)
 	if err != nil {
 		return payload, tocAlg, err
 	}
-	var parser = new(jwt.Parser)
-	token, _, err := parser.ParseUnverified(string(body), jwt.MapClaims{})
+	// Steps 2 - 4 done in unmarshalMDSTOC.  Caller is responsible for step 5.
+	return unmarshalMDSTOC(body, c)
+}
+
+func unmarshalMDSTOC(body []byte, c http.Client) (MetadataTOCPayload, string, error) {
+	var tocAlg string
+	var payload MetadataTOCPayload
+	token, err := jwt.Parse(string(body), func(token *jwt.Token) (interface{}, error) {
+		// 2. If the x5u attribute is present in the JWT Header, then
+		if _, ok := token.Header["x5u"].([]interface{}); ok {
+			// never seen an x5u here, although it is in the spec
+			return nil, errors.New("x5u encountered in header of metadata TOC payload")
+		}
+		var chain []interface{}
+		// 3. If the x5u attribute is missing, the chain should be retrieved from the x5c attribute.
+
+		if x5c, ok := token.Header["x5c"].([]interface{}); !ok {
+			// If that attribute is missing as well, Metadata TOC signing trust anchor is considered the TOC signing certificate chain.
+			root, err := getMetdataTOCSigningTrustAnchor(c)
+			if nil != err {
+				return nil, err
+			}
+			chain[0] = root
+		} else {
+			chain = x5c
+		}
+
+		// The certificate chain MUST be verified to properly chain to the metadata TOC signing trust anchor
+		valid, err := validateChain(chain, c)
+		if !valid || err != nil {
+			return nil, err
+		}
+		// chain validated, extract the TOC signing certificate from the chain
+
+		// create a buffer large enough to hold the certificate bytes
+		o := make([]byte, base64.StdEncoding.DecodedLen(len(chain[0].(string))))
+		// base64 decode the certificate into the buffer
+		n, err := base64.StdEncoding.Decode(o, []byte(chain[0].(string)))
+		// parse the certificate from the buffer
+		cert, err := x509.ParseCertificate(o[:n])
+		if err != nil {
+			return nil, err
+		}
+		// 4. Verify the signature of the Metadata TOC object using the TOC signing certificate chain
+		// jwt.Parse() uses the TOC signing certificate public key internally to verify the signature
+		return cert.PublicKey, err
+	})
 	if err != nil {
 		return payload, tocAlg, err
 	}
+
 	tocAlg = token.Header["alg"].(string)
 	err = mapstructure.Decode(token.Claims, &payload)
+
 	return payload, tocAlg, err
 }
 
+func getMetdataTOCSigningTrustAnchor(c http.Client) ([]byte, error) {
+	rooturl := ""
+	if Conformance {
+		rooturl = "https://fidoalliance.co.nz/mds/pki/MDSROOT.crt"
+	} else {
+		rooturl = "https://mds.fidoalliance.org/Root.cer"
+	}
+
+	return downloadBytes(rooturl, c)
+}
+
+func validateChain(chain []interface{}, c http.Client) (bool, error) {
+	root, err := getMetdataTOCSigningTrustAnchor(c)
+	if err != nil {
+		return false, err
+	}
+
+	roots := x509.NewCertPool()
+
+	ok := roots.AppendCertsFromPEM(root)
+	if !ok {
+		return false, err
+	}
+
+	o := make([]byte, base64.StdEncoding.DecodedLen(len(chain[1].(string))))
+	n, err := base64.StdEncoding.Decode(o, []byte(chain[1].(string)))
+	if err != nil {
+		return false, err
+	}
+	intcert, err := x509.ParseCertificate(o[:n])
+	if err != nil {
+		return false, err
+	}
+
+	if revoked, ok := revoke.VerifyCertificate(intcert); !ok {
+		return false, errCRLUnavailable
+	} else if revoked {
+		return false, errIntermediateCertRevoked
+	}
+
+	ints := x509.NewCertPool()
+	ints.AddCert(intcert)
+
+	l := make([]byte, base64.StdEncoding.DecodedLen(len(chain[0].(string))))
+	n, err = base64.StdEncoding.Decode(l, []byte(chain[0].(string)))
+	if err != nil {
+		return false, err
+	}
+	leafcert, err := x509.ParseCertificate(l[:n])
+	if err != nil {
+		return false, err
+	}
+	if revoked, ok := revoke.VerifyCertificate(leafcert); !ok {
+		return false, errCRLUnavailable
+	} else if revoked {
+		return false, errLeafCertRevoked
+	}
+
+	opts := x509.VerifyOptions{
+		Roots:         roots,
+		Intermediates: ints,
+	}
+	_, err = leafcert.Verify(opts)
+	return err == nil, err
+}
+
+// GetMetadataStatement iterates through a list of payload entries within a FIDO metadata table of contents object per §3.1.8, step 6
+// FIDO Authenticator Metadata Service
+// https://fidoalliance.org/specs/fido-v2.0-rd-20180702/fido-metadata-service-v2.0-rd-20180702.html#metadata-toc-object-processing-rules
 func GetMetadataStatement(entry MetadataTOCPayloadEntry, suffix string, alg string, c http.Client) (MetadataStatement, error) {
 	var statement MetadataStatement
+	// 1. Ignore the entry if the AAID, AAGUID or attestationCertificateKeyIdentifiers is not relevant to the relying party (e.g. not acceptable by any policy)
+	// Caller is responsible for determining if entry is relevant.
 
+	// 2. Download the metadata statement from the URL specified by the field url.
 	body, err := downloadBytes(entry.URL+suffix, c)
 	if err != nil {
 		return statement, err
 	}
+	// 3. Check whether the status report of the authenticator model has changed compared to the cached entry by looking at the fields timeOfLastStatusChange and statusReport.
+	// Caller is responsible for cache
 
-	hasher := crypto.SHA256.New()
-	_, _ = hasher.Write(body)
-	hashed := hasher.Sum(nil)
-	entryHash, err := base64.URLEncoding.DecodeString(entry.Hash)
+	// step 4 done in unmarshalMetadataStatement, caller is responsible for step 5
+	return unmarshalMetadataStatement(body, entry.Hash)
+}
+
+func unmarshalMetadataStatement(body []byte, hash string) (MetadataStatement, error) {
+	// 4. Compute the hash value of the metadata statement downloaded from the URL and verify the hash value to the hash specified in the field hash of the metadata TOC object.
+	var statement MetadataStatement
+
+	entryHash, err := base64.URLEncoding.DecodeString(hash)
 	if err != nil {
-		entryHash, err = base64.RawURLEncoding.DecodeString(entry.Hash)
+		entryHash, err = base64.RawURLEncoding.DecodeString(hash)
 	}
 	if err != nil {
 		return statement, err
 	}
+
+	// TODO: Get hasher based on MDS TOC alg instead of assuming SHA256
+	hasher := crypto.SHA256.New()
+	_, _ = hasher.Write(body)
+	hashed := hasher.Sum(nil)
+	// Ignore the downloaded metadata statement if the hash value doesn't match.
 	if !bytes.Equal(hashed, entryHash) {
-		return statement, errors.New("Hash value mismatch between entry.Hash and downloaded bytes")
+		return statement, errHashValueMismatch
 	}
 
+	// Extract the metadata statement from base64 encoded form
 	n := base64.URLEncoding.DecodedLen(len(body))
 	out := make([]byte, n)
 	m, err := base64.URLEncoding.Decode(out, body)
 	if err != nil {
 		return statement, err
 	}
+	// Unmarshal the metadata statement into a MetadataStatement structure and return it to caller
 	err = json.Unmarshal(out[:m], &statement)
 	return statement, err
 }
@@ -441,3 +583,35 @@ func downloadBytes(url string, c http.Client) ([]byte, error) {
 	body, _ := ioutil.ReadAll(res.Body)
 	return body, err
 }
+
+type MetadataError struct {
+	// Short name for the type of error that has occurred
+	Type string `json:"type"`
+	// Additional details about the error
+	Details string `json:"error"`
+	// Information to help debug the error
+	DevInfo string `json:"debug"`
+}
+
+var (
+	errHashValueMismatch = &MetadataError{
+		Type:    "hash_mismatch",
+		Details: "Hash value mismatch between entry.Hash and downloaded bytes",
+	}
+	errIntermediateCertRevoked = &MetadataError{
+		Type:    "intermediate_revoked",
+		Details: "Intermediate certificate is on issuers revocation list",
+	}
+	errLeafCertRevoked = &MetadataError{
+		Type:    "leaf_revoked",
+		Details: "Leaf certificate is on issuers revocation list",
+	}
+	errCRLUnavailable = &MetadataError{
+		Type:    "crl_unavailable",
+		Details: "Certificate revocation list is unavailable",
+	}
+)
+
+func (err *MetadataError) Error() string {
+	return err.Details
+}
diff --git a/metadata/metadata_test.go b/metadata/metadata_test.go
@@ -0,0 +1,97 @@
+package metadata
+
+import (
+	"io/ioutil"
+	"net/http"
+	"testing"
+	"time"
+
+	jwt "github.com/dgrijalva/jwt-go"
+)
+
+func TestMetadataTOCParsing(t *testing.T) {
+	Conformance = true
+	httpClient := &http.Client{
+		Timeout: time.Second * 30,
+	}
+
+	tests := []struct {
+		name    string
+		file    string
+		wantErr error
+	}{
+		{
+			"success",
+			"../testdata/MetadataTOCParsing-P1.jwt",
+			nil,
+		},
+		{
+			"verification_failure",
+			"../testdata/MetadataTOCParsing-F1.jwt",
+			errIntermediateCertRevoked,
+		},
+		{
+			"intermediate_revoked",
+			"../testdata/MetadataTOCParsing-F2.jwt",
+			jwt.ErrECDSAVerification,
+		},
+		{
+			"leaf_revoked",
+			"../testdata/MetadataTOCParsing-F3.jwt",
+			errLeafCertRevoked,
+		},
+		{
+			"asn1_parse_error",
+			"../testdata/MetadataTOCParsing-F4.jwt",
+			errCRLUnavailable,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			b, _ := ioutil.ReadFile(tt.file)
+			_, _, err := unmarshalMDSTOC(b, *httpClient)
+			failed := true
+			if err != nil {
+				failed = (err.Error() != tt.wantErr.Error())
+			} else {
+				failed = tt.wantErr != nil
+			}
+			if failed {
+				t.Errorf("unmarshalMDSTOC() got %v, wanted %v", err, tt.wantErr)
+			}
+		})
+	}
+}
+
+func TestMetadataStatementParsing(t *testing.T) {
+	tests := []struct {
+		name    string
+		file    string
+		hash    string
+		wantErr error
+	}{
+		{
+			"success",
+			"../testdata/TestMetadataStatementParsing-P1.json",
+			"bEtEyoVkc-X-ypuFoAIj8s4xKKTZw3wzD7IuDnoBUE8",
+			nil,
+		},
+		{
+			"hash_value_mismatch",
+			"../testdata/TestMetadataStatementParsing-F1.json",
+			"eq28frELluGyBesOw_xE_10Tj25NG0pDS7Oa0DP2kVk",
+			errHashValueMismatch,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			b, _ := ioutil.ReadFile(tt.file)
+			_, err := unmarshalMetadataStatement(b, tt.hash)
+			if err != tt.wantErr {
+				t.Errorf("unmarshalMetadataStatement() error %v, wantErr %v", err, tt.wantErr)
+			}
+		})
+	}
+}