Repository: go-webauthn/webauthn (go)

Issue:
A WithChallenge option
### Description

Currently, the challenges are randomly genererated automatically.
I'd like to see a `WithChallenge(challenge []byte)` option that simply creates the session with given challenge.

This makes less sense when registering new keys, more for login / auth

### Use Case

This could be used for document signing, by generating the challenge from document hashes and other document identifying material.

### Documentation

https://developers.yubico.com/WebAuthn/Concepts/Using_WebAuthn_for_Signing.html

Pull request:
feat(webauthn): add WithChallenge login option
This option customizes the challenge sent to the client. 
Can be used as a form of document signing method.

Closes #353

Patch:
diff --git a/metadata/metadata.go b/metadata/metadata.go
index 0a43e68..d6e9183 100644
--- a/metadata/metadata.go
+++ b/metadata/metadata.go
@@ -319,15 +319,26 @@ type Statement struct {
 	AuthenticatorGetInfo AuthenticatorGetInfo
 }
 
-func (s *Statement) Verifier() (opts x509.VerifyOptions) {
+func (s *Statement) Verifier(x5cis []*x509.Certificate) (opts x509.VerifyOptions) {
 	roots := x509.NewCertPool()
 
 	for _, root := range s.AttestationRootCertificates {
 		roots.AddCert(root)
 	}
 
+	var intermediates *x509.CertPool
+
+	if len(x5cis) > 0 {
+		intermediates = x509.NewCertPool()
+
+		for _, x5c := range x5cis {
+			intermediates.AddCert(x5c)
+		}
+	}
+
 	return x509.VerifyOptions{
-		Roots: roots,
+		Roots:         roots,
+		Intermediates: intermediates,
 	}
 }
 
diff --git a/webauthn/login.go b/webauthn/login.go
index 2132c8d..7122e97 100644
--- a/webauthn/login.go
+++ b/webauthn/login.go
@@ -56,14 +56,8 @@ func (webauthn *WebAuthn) beginLogin(userID []byte, allowedCredentials []protoco
 		return nil, nil, fmt.Errorf(errFmtConfigValidate, err)
 	}
 
-	challenge, err := protocol.CreateChallenge()
-	if err != nil {
-		return nil, nil, err
-	}
-
 	assertion = &protocol.CredentialAssertion{
 		Response: protocol.PublicKeyCredentialRequestOptions{
-			Challenge:          challenge,
 			RelyingPartyID:     webauthn.Config.RPID,
 			UserVerification:   webauthn.Config.AuthenticatorSelection.UserVerification,
 			AllowedCredentials: allowedCredentials,
@@ -74,6 +68,18 @@ func (webauthn *WebAuthn) beginLogin(userID []byte, allowedCredentials []protoco
 		opt(&assertion.Response)
 	}
 
+	if len(assertion.Response.Challenge) == 0 {
+		challenge, err := protocol.CreateChallenge()
+		if err != nil {
+			return nil, nil, err
+		}
+		assertion.Response.Challenge = challenge
+	}
+
+	if len(assertion.Response.Challenge) < 16 {
+		return nil, nil, fmt.Errorf("error generating assertion: the challenge must be at least 16 bytes")
+	}
+
 	if len(assertion.Response.RelyingPartyID) == 0 {
 		return nil, nil, fmt.Errorf("error generating assertion: the relying party id must be provided via the configuration or a functional option for a login")
 	} else if _, err = url.Parse(assertion.Response.RelyingPartyID); err != nil {
@@ -90,7 +96,7 @@ func (webauthn *WebAuthn) beginLogin(userID []byte, allowedCredentials []protoco
 	}
 
 	session = &SessionData{
-		Challenge:            challenge.String(),
+		Challenge:            assertion.Response.Challenge.String(),
 		RelyingPartyID:       assertion.Response.RelyingPartyID,
 		UserID:               userID,
 		AllowedCredentialIDs: assertion.Response.GetAllowedCredentialIDs(),
@@ -165,6 +171,18 @@ func WithLoginRelyingPartyID(id string) LoginOption {
 	}
 }
 
+// WithChallenge overrides the default random challenge with a user supplied value.
+// In order to prevent replay attacks, the challenges MUST contain enough entropy to make guessing them infeasible.
+// Challenges SHOULD therefore be at least 16 bytes long.
+// This function is EXPERIMENTAL and can be removed without warning. 
+//
+// Specification: §13.4.3. Cryptographic Challenges (https://www.w3.org/TR/webauthn/#sctn-cryptographic-challenges)
+func WithChallenge(challenge []byte) LoginOption {
+	return func(cco *protocol.PublicKeyCredentialRequestOptions) {
+		cco.Challenge = challenge
+	}
+}
+
 // FinishLogin takes the response from the client and validate it against the user credentials and stored session data.
 func (webauthn *WebAuthn) FinishLogin(user User, session SessionData, response *http.Request) (*Credential, error) {
 	parsedResponse, err := protocol.ParseCredentialRequestResponse(response)


Test patch:
diff --git a/protocol/attestation.go b/protocol/attestation.go
index 72a64f0..be30e92 100644
--- a/protocol/attestation.go
+++ b/protocol/attestation.go
@@ -232,8 +232,10 @@ func (a *AttestationObject) VerifyAttestation(clientDataHash []byte, mds metadat
 		}
 
 		var (
-			x5c     *x509.Certificate
-			parents []*x509.Certificate
+			x5c, parsed *x509.Certificate
+			x5cis       []*x509.Certificate
+			raw         []byte
+			ok          bool
 		)
 
 		if len(x5cs) == 0 {
@@ -241,18 +243,18 @@ func (a *AttestationObject) VerifyAttestation(clientDataHash []byte, mds metadat
 		}
 
 		for _, x5cAny := range x5cs {
-			x5cRaw, ok := x5cAny.([]byte)
-			if !ok {
+			if raw, ok = x5cAny.([]byte); !ok {
 				return ErrInvalidAttestation.WithDetails("Unable to parse attestation certificate from x5c during attestation validation").WithInfo(fmt.Sprintf("The first certificate in the attestation was type '%T' but '[]byte' was expected", x5cs[0]))
 			}
-			x5cParsed, err := x509.ParseCertificate(x5cRaw)
-			if err != nil {
+
+			if parsed, err = x509.ParseCertificate(raw); err != nil {
 				return ErrInvalidAttestation.WithDetails("Unable to parse attestation certificate from x5c during attestation validation").WithInfo(fmt.Sprintf("Error returned from x509.ParseCertificate: %+v", err))
 			}
+
 			if x5c == nil {
-				x5c = x5cParsed
+				x5c = parsed
 			} else {
-				parents = append(parents, x5cParsed)
+				x5cis = append(x5cis, parsed)
 			}
 		}
 
@@ -260,27 +262,25 @@ func (a *AttestationObject) VerifyAttestation(clientDataHash []byte, mds metadat
 			if err = tpmParseSANExtension(x5c); err != nil {
 				return err
 			}
+
 			if err = tpmRemoveEKU(x5c); err != nil {
 				return err
 			}
-			for _, parent := range parents {
+
+			for _, parent := range x5cis {
 				if err = tpmRemoveEKU(parent); err != nil {
 					return err
 				}
 			}
 		}
 
-		if x5c.Subject.CommonName != x5c.Issuer.CommonName {
+		if x5c != nil && x5c.Subject.CommonName != x5c.Issuer.CommonName {
 			if !entry.MetadataStatement.AttestationTypes.HasBasicFull() {
 				return ErrInvalidAttestation.WithDetails("Unable to validate attestation statement signature during attestation validation: attestation with full attestation from authenticator that does not support full attestation")
 			}
-			verifier := entry.MetadataStatement.Verifier()
-			if len(parents) != 0 {
-				verifier.Intermediates = x509.NewCertPool()
-				for _, parent := range parents {
-					verifier.Intermediates.AddCert(parent)
-				}
-			}
+
+			verifier := entry.MetadataStatement.Verifier(x5cis)
+
 			if _, err = x5c.Verify(verifier); err != nil {
 				return ErrInvalidAttestation.WithDetails(fmt.Sprintf("Unable to validate attestation signature statement during attestation validation: invalid certificate chain from MDS: %v", err))
 			}
diff --git a/protocol/attestation_tpm.go b/protocol/attestation_tpm.go
index 9251ae3..873e864 100644
--- a/protocol/attestation_tpm.go
+++ b/protocol/attestation_tpm.go
@@ -395,10 +395,7 @@ var (
 	oidExtensionSubjectAltName   = []int{2, 5, 29, 17}
 	oidExtensionExtendedKeyUsage = []int{2, 5, 29, 37}
 	oidExtensionBasicConstraints = []int{2, 5, 29, 19}
-
-	// From wincrypt.h of Windows SDK.
-	// Enhanced Key Usage for Privacy CA encryption certificate
-	oidKpPrivacyCA = []int{1, 3, 6, 1, 4, 1, 311, 21, 36}
+	oidKpPrivacyCA               = []int{1, 3, 6, 1, 4, 1, 311, 21, 36}
 )
 
 type tpmBasicConstraints struct {
@@ -406,24 +403,32 @@ type tpmBasicConstraints struct {
 	MaxPathLen int  `asn1:"optional,default:-1"`
 }
 
-// remove extension key usage to avoid ExtKeyUsage check failure
-// see also https://github.com/go-webauthn/webauthn/issues/342
+// Remove extension key usage to avoid ExtKeyUsage check failure.
 func tpmRemoveEKU(x5c *x509.Certificate) error {
-	var unknown []asn1.ObjectIdentifier
-	hasAiK := false
+	var (
+		unknown []asn1.ObjectIdentifier
+		hasAiK  bool
+	)
+
 	for _, eku := range x5c.UnknownExtKeyUsage {
 		if eku.Equal(tcgKpAIKCertificate) {
 			hasAiK = true
+
 			continue
 		}
+
 		if eku.Equal(oidKpPrivacyCA) {
 			continue
 		}
+
 		unknown = append(unknown, eku)
 	}
+
 	if !hasAiK {
 		return ErrAttestationFormat.WithDetails("AIK certificate missing EKU")
 	}
+
 	x5c.UnknownExtKeyUsage = unknown
+
 	return nil
 }
diff --git a/webauthn/login_test.go b/webauthn/login_test.go
index a869cf9..098c913 100644
--- a/webauthn/login_test.go
+++ b/webauthn/login_test.go
@@ -32,11 +32,12 @@ func TestLogin_FinishLoginFailure(t *testing.T) {
 
 func TestWithLoginRelyingPartyID(t *testing.T) {
 	testCases := []struct {
-		name       string
-		have       *Config
-		opts       []LoginOption
-		expectedID string
-		err        string
+		name              string
+		have              *Config
+		opts              []LoginOption
+		expectedID        string
+		expectedChallenge []byte
+		err               string
 	}{
 		{
 			name: "OptionDefinedInConfig",
@@ -84,6 +85,27 @@ func TestWithLoginRelyingPartyID(t *testing.T) {
 			opts: nil,
 			err:  "error generating assertion: the relying party id must be provided via the configuration or a functional option for a login",
 		},
+		{
+			name: "TooShortWithChallengeOption",
+			have: &Config{
+				RPID:          "https://example.com",
+				RPOrigins:     []string{"https://example.com"},
+				RPDisplayName: "Test Display Name",
+			},
+			opts: []LoginOption{WithChallenge([]byte("1234567890"))},
+			err:  "error generating assertion: the challenge must be at least 16 bytes",
+		},
+		{
+			name: "WithChallengeOption",
+			have: &Config{
+				RPID:          "https://example.com",
+				RPOrigins:     []string{"https://example.com"},
+				RPDisplayName: "Test Display Name",
+			},
+			opts:              []LoginOption{WithChallenge([]byte("00000000000000000000000000000000"))},
+			expectedID:        "https://example.com",
+			expectedChallenge: []byte("00000000000000000000000000000000"),
+		},
 	}
 
 	for _, tc := range testCases {
@@ -104,6 +126,9 @@ func TestWithLoginRelyingPartyID(t *testing.T) {
 				assert.NoError(t, err)
 				require.NotNil(t, creation)
 				assert.Equal(t, tc.expectedID, creation.Response.RelyingPartyID)
+				if len(tc.expectedChallenge) > 0 {
+					assert.Equal(t, protocol.URLEncodedBase64(tc.expectedChallenge).String(), creation.Response.Challenge.String())
+				}
 			}
 		})
 	}