Skip to content

Commit 99bbbdb

Browse files
fix(protocol): single signature encoding policy and canonical der (#744)
The signature encoding was carried by the attestation policy while assertions were governed by an experimental process global, so one deviation had two controls with different reach. Where both applied the global won, and a Relying Party which selected DER while the global was set had a BER signature accepted anyway. SignaturePolicy is now a member of webauthn.Config in its own right, threaded through both ceremonies, and the global and its setter are removed. The encoding is one decision which applies wherever a signature is verified. Verification of an ECDSA signature against a credential public key now requires the DER encoding to be canonical. The decoder discards data trailing the signature and elements trailing the two integers within it rather than reporting either, so a signature carrying an appended integer or arbitrary appended bytes verified against the same message as the signature it was built from. Re-encoding the decoded integers and requiring the result to equal the input rejects both, along with the non-minimal integers which previously depended on the global. A Relying Party which accepts the BER encoding normalizes the signature before it reaches this point, so the relaxation stays in one place rather than being repeated at each verifier. The helpers no longer wrap the errors they return, which the format handlers already describe in the terms of the attestation they were verifying, so a signature which fails to decode is reported once rather than twice. BREAKING CHANGE: ConfigProvider requires GetSignaturePolicy. AttestationObject.Verify, AttestationObject.VerifyAttestation, ParsedCredentialAssertionData.Verify, ParsedCredentialCreationData.Verify, Credential.Verify, Credential.VerifyAttestationType and the attestation format validation handler registered by RegisterAttestationFormat take a SignaturePolicy. AttestationPolicy no longer carries a Signature member and webauthncose.SetExperimentalInsecureAllowBERIntegers is removed.
1 parent 29404e9 commit 99bbbdb

37 files changed

Lines changed: 438 additions & 289 deletions

protocol/assertion.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -145,7 +145,7 @@ func (car CredentialAssertionResponse) Parse() (par *ParsedCredentialAssertionDa
145145
// documentation. It's important to note that the credentialBytes field is the CBOR representation of the credential.
146146
//
147147
// Specification: §7.2 Verifying an Authentication Assertion (https://www.w3.org/TR/webauthn/#sctn-verifying-assertion)
148-
func (p *ParsedCredentialAssertionData) Verify(storedChallenge string, relyingPartyID, appID string, rpOrigins, rpTopOrigins []string, rpTopOriginsVerify TopOriginVerificationMode, allowCrossOrigin, verifyUser, verifyUserPresence bool, credentialBytes []byte) error {
148+
func (p *ParsedCredentialAssertionData) Verify(storedChallenge string, relyingPartyID, appID string, rpOrigins, rpTopOrigins []string, rpTopOriginsVerify TopOriginVerificationMode, allowCrossOrigin, verifyUser, verifyUserPresence bool, credentialBytes []byte, signature SignaturePolicy) error {
149149
// Steps 4 through 6 in verifying the assertion data (https://www.w3.org/TR/webauthn/#verifying-assertion) are
150150
// "assertive" steps, i.e. "Let JSONtext be the result of running UTF-8 decode on the value of cData."
151151
// We handle these steps in part as we verify but also beforehand
@@ -196,7 +196,7 @@ func (p *ParsedCredentialAssertionData) Verify(storedChallenge string, relyingPa
196196
return ErrAssertionSignature.WithDetails(fmt.Sprintf("Error parsing the assertion public key: %+v", err)).WithError(err)
197197
}
198198

199-
valid, err := webauthncose.VerifySignature(key, sigData, p.Response.Signature)
199+
valid, err := keyVerifySignature(key, sigData, p.Response.Signature, signature)
200200
if !valid || err != nil {
201201
return ErrAssertionSignature.WithDetails(fmt.Sprintf("Error validating the assertion signature: %+v", err)).WithError(err)
202202
}

protocol/assertion_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -337,7 +337,7 @@ func TestParsedCredentialAssertionData_Verify(t *testing.T) {
337337

338338
for _, tc := range testCases {
339339
t.Run(tc.name, func(t *testing.T) {
340-
err := par.Verify(tc.challenge, tc.relyingPartyID, tc.appID, tc.rpOrigins, nil, TopOriginExplicitVerificationMode, false, false, true, tc.credentialBytes)
340+
err := par.Verify(tc.challenge, tc.relyingPartyID, tc.appID, tc.rpOrigins, nil, TopOriginExplicitVerificationMode, false, false, true, tc.credentialBytes, SignaturePolicy{})
341341

342342
if tc.err == "" {
343343
assert.NoError(t, err)

protocol/attestation.go

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -99,7 +99,7 @@ type NonCompoundAttestationObject struct {
9999
AttStatement map[string]any `json:"attStmt,omitempty"`
100100
}
101101

102-
type attestationFormatValidationHandler func(att AttestationObject, clientDataHash []byte, mds metadata.Provider, policy AttestationPolicy) (attestationType string, x5cs []any, err error)
102+
type attestationFormatValidationHandler func(att AttestationObject, clientDataHash []byte, mds metadata.Provider, policy AttestationPolicy, signature SignaturePolicy) (attestationType string, x5cs []any, err error)
103103

104104
var attestationRegistry = make(map[AttestationFormat]attestationFormatValidationHandler)
105105

@@ -149,7 +149,7 @@ func (ccr *AuthenticatorAttestationResponse) Parse() (p *ParsedAttestationRespon
149149
//
150150
// Steps 13 through 15 are verified against the auth data. These steps are identical to 15 through 18 for assertion so we
151151
// handle them with AuthData.
152-
func (a *AttestationObject) Verify(relyingPartyID string, clientDataHash []byte, userVerificationRequired bool, userPresenceRequired bool, mds metadata.Provider, credParams []CredentialParameter, policy AttestationPolicy) (err error) {
152+
func (a *AttestationObject) Verify(relyingPartyID string, clientDataHash []byte, userVerificationRequired bool, userPresenceRequired bool, mds metadata.Provider, credParams []CredentialParameter, policy AttestationPolicy, signature SignaturePolicy) (err error) {
153153
rpIDHash := sha256.Sum256([]byte(relyingPartyID))
154154

155155
// Begin Step 13 through 15. Verify that the rpIdHash in authData is the SHA-256 hash of the RP ID expected by the RP.
@@ -177,15 +177,15 @@ func (a *AttestationObject) Verify(relyingPartyID string, clientDataHash []byte,
177177
return ErrAttestationFormat.WithInfo("Credential public key algorithm not supported")
178178
}
179179

180-
return a.VerifyAttestation(clientDataHash, mds, policy)
180+
return a.VerifyAttestation(clientDataHash, mds, policy, signature)
181181
}
182182

183183
// VerifyAttestation only verifies the attestation object excluding the AuthData values. If you wish to also verify the
184184
// AuthData values you should use [Verify].
185185
//
186186
// The policy carries the Relying Party decisions which §8 leaves to the Relying Party. Its zero value selects the
187187
// most restrictive behavior available. See [AttestationPolicy].
188-
func (a *AttestationObject) VerifyAttestation(clientDataHash []byte, mds metadata.Provider, policy AttestationPolicy) (err error) {
188+
func (a *AttestationObject) VerifyAttestation(clientDataHash []byte, mds metadata.Provider, policy AttestationPolicy, signature SignaturePolicy) (err error) {
189189
// Step 18. Determine the attestation statement format by performing a
190190
// USASCII case-sensitive match on fmt against the set of supported
191191
// WebAuthn Attestation Statement Format Identifier values. The up-to-date
@@ -226,7 +226,7 @@ func (a *AttestationObject) VerifyAttestation(clientDataHash []byte, mds metadat
226226
// Step 19. Verify that attStmt is a correct attestation statement, conveying a valid attestation signature, by using
227227
// the attestation statement format fmt’s verification procedure given attStmt, authData and the hash of the serialized
228228
// client data computed in step 7.
229-
if attestationType, x5cs, err = handler(*a, clientDataHash, mds, policy); err != nil {
229+
if attestationType, x5cs, err = handler(*a, clientDataHash, mds, policy, signature); err != nil {
230230
var e *Error
231231

232232
if errors.As(err, &e) {

protocol/attestation_androidkey.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@ import (
3737
// See: https://www.w3.org/TR/webauthn/#sctn-android-key-attestation
3838
//
3939
//nolint:gocyclo
40-
func attestationFormatValidationHandlerAndroidKey(att AttestationObject, clientDataHash []byte, _ metadata.Provider, policy AttestationPolicy) (attestationType string, x5cs []any, err error) {
40+
func attestationFormatValidationHandlerAndroidKey(att AttestationObject, clientDataHash []byte, _ metadata.Provider, policy AttestationPolicy, signature SignaturePolicy) (attestationType string, x5cs []any, err error) {
4141
var (
4242
alg int64
4343
sig []byte
@@ -83,7 +83,7 @@ func attestationFormatValidationHandlerAndroidKey(att AttestationObject, clientD
8383

8484
if sigAlg := webauthncose.SigAlgFromCOSEAlg(webauthncose.COSEAlgorithmIdentifier(alg)); sigAlg == x509.UnknownSignatureAlgorithm {
8585
return "", nil, ErrInvalidAttestation.WithDetails(fmt.Sprintf("Unsupported COSE alg: %d", alg))
86-
} else if err = attestationCertCheckSignature(credCert, sigAlg, signatureData, sig, policy.Signature); err != nil {
86+
} else if err = certCheckSignature(credCert, sigAlg, signatureData, sig, signature); err != nil {
8787
return "", nil, ErrInvalidAttestation.WithDetails(fmt.Sprintf("Signature validation error: %+v", err)).WithError(err)
8888
}
8989

@@ -99,7 +99,7 @@ func attestationFormatValidationHandlerAndroidKey(att AttestationObject, clientD
9999
// as each selects the digest from the algorithm it carries.
100100
var valid bool
101101

102-
if valid, err = attestationKeyVerifySignature(credentialPublicKey, signatureData, sig, policy.Signature); err != nil {
102+
if valid, err = keyVerifySignature(credentialPublicKey, signatureData, sig, signature); err != nil {
103103
return "", nil, ErrInvalidAttestation.WithDetails(fmt.Sprintf("Error verifying the signature with the credential public key: %+v", err)).WithError(err)
104104
} else if !valid {
105105
return "", nil, ErrInvalidAttestation.WithDetails("Signature is not valid for the credential public key")

protocol/attestation_androidkey_test.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -100,13 +100,13 @@ func TestVerifyAndroidKeyFormat(t *testing.T) {
100100
tc.setup(t, mds)
101101
}
102102

103-
attestationType, x5cs, err = attestationFormatValidationHandlerAndroidKey(tc.args.att, tc.args.clientDataHash, mds, AttestationPolicy{})
103+
attestationType, x5cs, err = attestationFormatValidationHandlerAndroidKey(tc.args.att, tc.args.clientDataHash, mds, AttestationPolicy{}, SignaturePolicy{})
104104
} else {
105105
if tc.setup != nil {
106106
tc.setup(t, nil)
107107
}
108108

109-
attestationType, x5cs, err = attestationFormatValidationHandlerAndroidKey(tc.args.att, tc.args.clientDataHash, nil, AttestationPolicy{})
109+
attestationType, x5cs, err = attestationFormatValidationHandlerAndroidKey(tc.args.att, tc.args.clientDataHash, nil, AttestationPolicy{}, SignaturePolicy{})
110110
}
111111

112112
if tc.err != "" {
@@ -283,7 +283,7 @@ func TestAndroidKeyFormat_HandlerErrors(t *testing.T) {
283283

284284
for _, tc := range testCases {
285285
t.Run(tc.name, func(t *testing.T) {
286-
_, _, err := attestationFormatValidationHandlerAndroidKey(tc.att, tc.clientDataHash, nil, AttestationPolicy{})
286+
_, _, err := attestationFormatValidationHandlerAndroidKey(tc.att, tc.clientDataHash, nil, AttestationPolicy{}, SignaturePolicy{})
287287

288288
assert.EqualError(t, err, tc.err)
289289
})

protocol/attestation_apple.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ import (
2727
// Specification: §8.8. Apple Anonymous Attestation Statement Format
2828
//
2929
// See : https://www.w3.org/TR/webauthn/#sctn-apple-anonymous-attestation
30-
func attestationFormatValidationHandlerAppleAnonymous(att AttestationObject, clientDataHash []byte, _ metadata.Provider, _ AttestationPolicy) (attestationType string, x5cs []any, err error) {
30+
func attestationFormatValidationHandlerAppleAnonymous(att AttestationObject, clientDataHash []byte, _ metadata.Provider, _ AttestationPolicy, _ SignaturePolicy) (attestationType string, x5cs []any, err error) {
3131
// Step 1. Verify that attStmt is valid CBOR conforming to the syntax defined above and perform CBOR decoding on it
3232
// to extract the contained fields.
3333
var (

protocol/attestation_apple_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ func Test_VerifyAppleFormat(t *testing.T) {
4141

4242
for _, tc := range testCases {
4343
t.Run(tc.name, func(t *testing.T) {
44-
attestationType, x5cs, err := attestationFormatValidationHandlerAppleAnonymous(tc.args.att, tc.args.clientDataHash, nil, AttestationPolicy{})
44+
attestationType, x5cs, err := attestationFormatValidationHandlerAppleAnonymous(tc.args.att, tc.args.clientDataHash, nil, AttestationPolicy{}, SignaturePolicy{})
4545

4646
assert.Equal(t, tc.attestationType, attestationType)
4747
assert.Equal(t, tc.x5cs, x5cs)

protocol/attestation_compound.go

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ func init() {
3434
// Specification: §8.9. Compound Attestation Statement Forma
3535
//
3636
// See: https://www.w3.org/TR/webauthn-3/#sctn-compound-attestation
37-
func attestationFormatValidationHandlerCompound(att AttestationObject, clientDataHash []byte, mds metadata.Provider, policy AttestationPolicy) (attestationType string, x5cs []any, err error) {
37+
func attestationFormatValidationHandlerCompound(att AttestationObject, clientDataHash []byte, mds metadata.Provider, policy AttestationPolicy, signature SignaturePolicy) (attestationType string, x5cs []any, err error) {
3838
var (
3939
aaguid uuid.UUID
4040
raw any
@@ -96,20 +96,20 @@ func attestationFormatValidationHandlerCompound(att AttestationObject, clientDat
9696
// for more than one sub-statement, and the paths of independent sub-statements joined together describe no real
9797
// chain.
9898
if policy.Compound.SubStatementScope.any() {
99-
return compoundVerifySubStatementsAny(att, attStmts, clientDataHash, mds, policy, aaguid)
99+
return compoundVerifySubStatementsAny(att, attStmts, clientDataHash, mds, policy, signature, aaguid)
100100
}
101101

102-
return compoundVerifySubStatementsAll(att, attStmts, clientDataHash, mds, policy, aaguid)
102+
return compoundVerifySubStatementsAll(att, attStmts, clientDataHash, mds, policy, signature, aaguid)
103103
}
104104

105105
// compoundVerifySubStatementsAll verifies every sub-statement, rejecting the attestation on the first which fails.
106106
//
107107
// This is the behavior of [CompoundSubStatementScopeAll].
108-
func compoundVerifySubStatementsAll(att AttestationObject, attStmts []NonCompoundAttestationObject, clientDataHash []byte, mds metadata.Provider, policy AttestationPolicy, aaguid uuid.UUID) (attestationType string, x5cs []any, err error) {
108+
func compoundVerifySubStatementsAll(att AttestationObject, attStmts []NonCompoundAttestationObject, clientDataHash []byte, mds metadata.Provider, policy AttestationPolicy, signature SignaturePolicy, aaguid uuid.UUID) (attestationType string, x5cs []any, err error) {
109109
for i, attStmt := range attStmts {
110110
var subAttType string
111111

112-
if subAttType, err = compoundVerifySubStatement(att, attStmt, clientDataHash, mds, policy, aaguid); err != nil {
112+
if subAttType, err = compoundVerifySubStatement(att, attStmt, clientDataHash, mds, policy, signature, aaguid); err != nil {
113113
return "", nil, err
114114
}
115115

@@ -129,7 +129,7 @@ func compoundVerifySubStatementsAll(att AttestationObject, attStmts []NonCompoun
129129
// reached is not lost.
130130
//
131131
// This is the behavior of [CompoundSubStatementScopeAny].
132-
func compoundVerifySubStatementsAny(att AttestationObject, attStmts []NonCompoundAttestationObject, clientDataHash []byte, mds metadata.Provider, policy AttestationPolicy, aaguid uuid.UUID) (attestationType string, x5cs []any, err error) {
132+
func compoundVerifySubStatementsAny(att AttestationObject, attStmts []NonCompoundAttestationObject, clientDataHash []byte, mds metadata.Provider, policy AttestationPolicy, signature SignaturePolicy, aaguid uuid.UUID) (attestationType string, x5cs []any, err error) {
133133
var (
134134
errs = make([]error, 0, len(attStmts))
135135
reasons = make([]string, 0, len(attStmts))
@@ -138,7 +138,7 @@ func compoundVerifySubStatementsAny(att AttestationObject, attStmts []NonCompoun
138138
for _, attStmt := range attStmts {
139139
var subAttType string
140140

141-
if subAttType, err = compoundVerifySubStatement(att, attStmt, clientDataHash, mds, policy, aaguid); err != nil {
141+
if subAttType, err = compoundVerifySubStatement(att, attStmt, clientDataHash, mds, policy, signature, aaguid); err != nil {
142142
errs = append(errs, err)
143143
reasons = append(reasons, fmt.Sprintf("%s: %s", attStmt.Format, compoundSubStatementFailureReason(err)))
144144

@@ -179,7 +179,7 @@ func compoundSubStatementFailureReason(err error) string {
179179
// path it produces against the Metadata Service. A sub-statement is verified in full or not at all, so a scope which
180180
// tolerates a failure treats a sub-statement whose trust path the Metadata Service rejects the same as one whose
181181
// verification procedure fails.
182-
func compoundVerifySubStatement(att AttestationObject, attStmt NonCompoundAttestationObject, clientDataHash []byte, mds metadata.Provider, policy AttestationPolicy, aaguid uuid.UUID) (attestationType string, err error) {
182+
func compoundVerifySubStatement(att AttestationObject, attStmt NonCompoundAttestationObject, clientDataHash []byte, mds metadata.Provider, policy AttestationPolicy, signature SignaturePolicy, aaguid uuid.UUID) (attestationType string, err error) {
183183
object := AttestationObject{
184184
Format: attStmt.Format,
185185
AttStatement: attStmt.AttStatement,
@@ -189,7 +189,7 @@ func compoundVerifySubStatement(att AttestationObject, attStmt NonCompoundAttest
189189

190190
var cx5cs []any
191191

192-
if attestationType, cx5cs, err = attestationRegistry[AttestationFormat(object.Format)](object, clientDataHash, mds, policy); err != nil {
192+
if attestationType, cx5cs, err = attestationRegistry[AttestationFormat(object.Format)](object, clientDataHash, mds, policy, signature); err != nil {
193193
return "", err
194194
}
195195

0 commit comments

Comments
 (0)