From 509e08fb364c78be30067a93d976730a8fe4a656 Mon Sep 17 00:00:00 2001 From: James Elliott Date: Sun, 12 Dec 2021 00:45:05 +1100 Subject: [PATCH] feat: appid extension (#7) This implements the appid extension for backwards compat with CTAP1 keys. --- metadata/metadata_test.go | 4 +- protocol/assertion.go | 28 +++++++++--- protocol/assertion_test.go | 9 ++-- protocol/attestation.go | 2 +- protocol/authenticator.go | 6 +-- protocol/authenticator_test.go | 2 +- protocol/credential.go | 62 +++++++++++++++++++++++++-- protocol/credential_test.go | 4 +- protocol/extensions.go | 4 +- protocol/webauthncose/webauthncose.go | 13 ++++++ webauthn/credential.go | 1 - webauthn/login.go | 8 +++- webauthn/registration.go | 20 ++++----- webauthn/session.go | 1 + 14 files changed, 128 insertions(+), 36 deletions(-) diff --git a/metadata/metadata_test.go b/metadata/metadata_test.go index 19d1f795..42288fda 100644 --- a/metadata/metadata_test.go +++ b/metadata/metadata_test.go @@ -52,7 +52,9 @@ func TestMetadataTOCParsing(t *testing.T) { b, _ := ioutil.ReadFile(tt.file) _, _, err := unmarshalMDSTOC(b, *httpClient) failed := true - if err != nil { + if tt.wantErr == nil { + failed = err != nil + } else if err != nil { failed = (err.Error() != tt.wantErr.Error()) } else { failed = tt.wantErr != nil diff --git a/protocol/assertion.go b/protocol/assertion.go index 67b9ac31..91712bc2 100644 --- a/protocol/assertion.go +++ b/protocol/assertion.go @@ -45,7 +45,7 @@ type ParsedAssertionResponse struct { // Parse the credential request response into a format that is either required by the specification // or makes the assertion verification steps easier to complete. This takes an http.Request that contains -// the attestation response data in a raw, mostly base64 encoded format, and parses the data into +// the assertion response data in a raw, mostly base64 encoded format, and parses the data into // manageable structures func ParseCredentialRequestResponse(response *http.Request) (*ParsedCredentialAssertionData, error) { if response == nil || response.Body == nil { @@ -56,7 +56,7 @@ func ParseCredentialRequestResponse(response *http.Request) (*ParsedCredentialAs // Parse the credential request response into a format that is either required by the specification // or makes the assertion verification steps easier to complete. This takes an io.Reader that contains -// the attestation response data in a raw, mostly base64 encoded format, and parses the data into +// the assertion response data in a raw, mostly base64 encoded format, and parses the data into // manageable structures func ParseCredentialRequestResponseBody(body io.Reader) (*ParsedCredentialAssertionData, error) { var car CredentialAssertionResponse @@ -77,7 +77,7 @@ func ParseCredentialRequestResponseBody(body io.Reader) (*ParsedCredentialAssert return nil, ErrBadRequest.WithDetails("CredentialAssertionResponse with bad type") } var par ParsedCredentialAssertionData - par.ID, par.RawID, par.Type = car.ID, car.RawID, car.Type + par.ID, par.RawID, par.Type, par.ClientExtensionResults = car.ID, car.RawID, car.Type, car.ClientExtensionResults par.Raw = car par.Response.Signature = car.AssertionResponse.Signature @@ -100,7 +100,7 @@ func ParseCredentialRequestResponseBody(body io.Reader) (*ParsedCredentialAssert // Follow the remaining steps outlined in §7.2 Verifying an authentication assertion // (https://www.w3.org/TR/webauthn/#verifying-assertion) and return an error if there // is a failure during each step. -func (p *ParsedCredentialAssertionData) Verify(storedChallenge string, relyingPartyID, relyingPartyOrigin string, verifyUser bool, credentialBytes []byte) error { +func (p *ParsedCredentialAssertionData) Verify(storedChallenge string, relyingPartyID, relyingPartyOrigin, appID string, verifyUser bool, credentialBytes []byte) error { // Steps 4 through 6 in verifying the assertion data (https://www.w3.org/TR/webauthn/#verifying-assertion) are // "assertive" steps, i.e "Let JSONtext be the result of running UTF-8 decode on the value of cData." @@ -116,8 +116,13 @@ func (p *ParsedCredentialAssertionData) Verify(storedChallenge string, relyingPa // Begin Step 11. Verify that the rpIdHash in authData is the SHA-256 hash of the RP ID expected by the RP. rpIDHash := sha256.Sum256([]byte(relyingPartyID)) + var appIDHash [32]byte + if appID != "" { + appIDHash = sha256.Sum256([]byte(appID)) + } + // Handle steps 11 through 14, verifying the authenticator data. - validError = p.Response.AuthenticatorData.Verify(rpIDHash[:], verifyUser) + validError = p.Response.AuthenticatorData.Verify(rpIDHash[:], appIDHash[:], verifyUser) if validError != nil { return ErrAuthData.WithInfo(validError.Error()) } @@ -132,7 +137,18 @@ func (p *ParsedCredentialAssertionData) Verify(storedChallenge string, relyingPa sigData := append(p.Raw.AssertionResponse.AuthenticatorData, clientDataHash[:]...) - key, err := webauthncose.ParsePublicKey(credentialBytes) + var ( + key interface{} + err error + ) + + // If the Session Data does not contain the appID extension or it wasn't reported as used by the Client/RP then we + // use the standard CTAP2 public key parser. + if appID == "" { + key, err = webauthncose.ParsePublicKey(credentialBytes) + } else { + key, err = webauthncose.ParseFIDOPublicKey(credentialBytes) + } valid, err := webauthncose.VerifySignature(key, sigData, p.Response.Signature) if !valid { diff --git a/protocol/assertion_test.go b/protocol/assertion_test.go index 5bf29606..fc178a86 100644 --- a/protocol/assertion_test.go +++ b/protocol/assertion_test.go @@ -94,8 +94,8 @@ func TestParseCredentialRequestResponse(t *testing.T) { t.Errorf("ParseCredentialRequestResponse() error = %v, wantErr %v", err, tt.wantErr) return } - if !reflect.DeepEqual(got.Extensions, tt.want.Extensions) { - t.Errorf("Extensions = %v \n want: %v", got, tt.want) + if !reflect.DeepEqual(got.ClientExtensionResults, tt.want.ClientExtensionResults) { + t.Errorf("ClientExtensionResults = %v \n want: %v", got, tt.want) } if !reflect.DeepEqual(got.ID, tt.want.ID) { t.Errorf("ID = %v \n want: %v", got.ID, tt.want.ID) @@ -104,7 +104,7 @@ func TestParseCredentialRequestResponse(t *testing.T) { t.Errorf("ParsedCredential = %v \n want: %v", got, tt.want) } if !reflect.DeepEqual(got.ParsedPublicKeyCredential, tt.want.ParsedPublicKeyCredential) { - t.Errorf("ParsedPublicKeyCredential = %v \n want: %v", got.ParsedPublicKeyCredential.Extensions, tt.want.ParsedPublicKeyCredential.Extensions) + t.Errorf("ParsedPublicKeyCredential = %v \n want: %v", got.ParsedPublicKeyCredential.ClientExtensionResults, tt.want.ParsedPublicKeyCredential.ClientExtensionResults) } if !reflect.DeepEqual(got.Raw, tt.want.Raw) { t.Errorf("Raw = %+v \n want: %+v", got.Raw, tt.want.Raw) @@ -172,7 +172,8 @@ func TestParsedCredentialAssertionData_Verify(t *testing.T) { Response: tt.fields.Response, Raw: tt.fields.Raw, } - if err := p.Verify(tt.args.storedChallenge.String(), tt.args.relyingPartyID, tt.args.relyingPartyOrigin, tt.args.verifyUser, tt.args.credentialBytes); (err != nil) != tt.wantErr { + + if err := p.Verify(tt.args.storedChallenge.String(), tt.args.relyingPartyID, tt.args.relyingPartyOrigin, "", tt.args.verifyUser, tt.args.credentialBytes); (err != nil) != tt.wantErr { t.Errorf("ParsedCredentialAssertionData.Verify() error = %v, wantErr %v", err, tt.wantErr) } }) diff --git a/protocol/attestation.go b/protocol/attestation.go index 979e16d6..9362c5f5 100644 --- a/protocol/attestation.go +++ b/protocol/attestation.go @@ -115,7 +115,7 @@ func (attestationObject *AttestationObject) Verify(relyingPartyID string, client // the SHA-256 hash of the RP ID expected by the RP. rpIDHash := sha256.Sum256([]byte(relyingPartyID)) // Handle Steps 9 through 12 - authDataVerificationError := attestationObject.AuthData.Verify(rpIDHash[:], verificationRequired) + authDataVerificationError := attestationObject.AuthData.Verify(rpIDHash[:], nil, verificationRequired) if authDataVerificationError != nil { return authDataVerificationError } diff --git a/protocol/authenticator.go b/protocol/authenticator.go index 287602b8..a003591c 100644 --- a/protocol/authenticator.go +++ b/protocol/authenticator.go @@ -177,7 +177,7 @@ func (a *AuthenticatorData) Unmarshal(rawAuthData []byte) error { a.ExtData = rawAuthData[len(rawAuthData)-remaining:] remaining -= len(a.ExtData) } else { - return ErrBadRequest.WithDetails("Extensions flag set but extensions data is missing") + return ErrBadRequest.WithDetails("ClientExtensionResults flag set but extensions data is missing") } } @@ -218,12 +218,12 @@ func ResidentKeyUnrequired() *bool { // Verify on AuthenticatorData handles Steps 9 through 12 for Registration // and Steps 11 through 14 for Assertion. -func (a *AuthenticatorData) Verify(rpIdHash []byte, userVerificationRequired bool) error { +func (a *AuthenticatorData) Verify(rpIdHash []byte, appIDHash []byte, userVerificationRequired bool) error { // Registration Step 9 & Assertion Step 11 // Verify that the RP ID hash in authData is indeed the SHA-256 // hash of the RP ID expected by the RP. - if !bytes.Equal(a.RPIDHash[:], rpIdHash) { + if !bytes.Equal(a.RPIDHash[:], rpIdHash) && appIDHash != nil && !bytes.Equal(a.RPIDHash[:], appIDHash) { return ErrVerification.WithInfo(fmt.Sprintf("RP Hash mismatch. Expected %+s and Received %+s\n", a.RPIDHash, rpIdHash)) } diff --git a/protocol/authenticator_test.go b/protocol/authenticator_test.go index bc42b04f..135b40bc 100644 --- a/protocol/authenticator_test.go +++ b/protocol/authenticator_test.go @@ -253,7 +253,7 @@ func TestAuthenticatorData_Verify(t *testing.T) { AttData: tt.fields.AttData, ExtData: tt.fields.ExtData, } - if err := a.Verify(tt.args.rpIdHash, tt.args.userVerificationRequired); (err != nil) != tt.wantErr { + if err := a.Verify(tt.args.rpIdHash, nil, tt.args.userVerificationRequired); (err != nil) != tt.wantErr { t.Errorf("AuthenticatorData.Verify() error = %v, wantErr %v", err, tt.wantErr) } }) diff --git a/protocol/credential.go b/protocol/credential.go index e10593f0..a461f85c 100644 --- a/protocol/credential.go +++ b/protocol/credential.go @@ -32,14 +32,14 @@ type ParsedCredential struct { type PublicKeyCredential struct { Credential - RawID URLEncodedBase64 `json:"rawId"` - Extensions AuthenticationExtensionsClientOutputs `json:"extensions,omitempty"` + RawID URLEncodedBase64 `json:"rawId"` + ClientExtensionResults AuthenticationExtensionsClientOutputs `json:"clientExtensionResults,omitempty"` } type ParsedPublicKeyCredential struct { ParsedCredential - RawID []byte `json:"rawId"` - Extensions AuthenticationExtensionsClientOutputs `json:"extensions,omitempty"` + RawID []byte `json:"rawId"` + ClientExtensionResults AuthenticationExtensionsClientOutputs `json:"clientExtensionResults,omitempty"` } type CredentialCreationResponse struct { @@ -160,3 +160,57 @@ func (pcc *ParsedCredentialCreationData) Verify(storedChallenge string, verifyUs return nil } + +// GetAppID takes a AuthenticationExtensions object or nil. It then performs the following checks in order: +// +// 1. Check that the Session Data's AuthenticationExtensions has been provided and if it hasn't return an error. +// 2. Check that the AuthenticationExtensionsClientOutputs contains the extensions output and return an empty string if it doesn't. +// 3. Check that the Credential AttestationType is `fido-u2f` and return an empty string if it isn't. +// 4. Check that the AuthenticationExtensionsClientOutputs contains the appid key and if it doesn't return an empty string. +// 5. Check that the AuthenticationExtensionsClientOutputs appid is a bool and if it isn't return an error. +// 6. Check that the appid output is true and if it isn't return an empty string. +// 7. Check that the Session Data has an appid extension defined and if it doesn't return an error. +// 8. Check that the appid extension in Session Data is a string and if it isn't return an error. +// 9. Return the appid extension value from the Session data. +func (ppkc ParsedPublicKeyCredential) GetAppID(authExt AuthenticationExtensions, credentialAttestationType string) (appID string, err error) { + var ( + value, clientValue interface{} + enableAppID, ok bool + ) + + if authExt == nil { + return "", nil + } + + if ppkc.ClientExtensionResults == nil { + return "", nil + } + + // If the credential does not have the correct attestation type it is assumed to NOT be a fido-u2f credential. + // https://w3c.github.io/webauthn/#sctn-fido-u2f-attestation + if credentialAttestationType != "fido-u2f" { + return "", nil + } + + if clientValue, ok = ppkc.ClientExtensionResults["appid"]; ok { + if enableAppID, ok = clientValue.(bool); !ok { + return "", ErrBadRequest.WithDetails("Client Output appid did not have the expected type") + } + + if !enableAppID { + return "", nil + } + + if value, ok = authExt["appid"]; !ok { + return "", ErrBadRequest.WithDetails("Session Data does not have an appid but Client Output indicates it should be set") + } + + if appID, ok = value.(string); !ok { + return "", ErrBadRequest.WithDetails("Session Data appid did not have the expected type") + } + + return appID, nil + } + + return "", nil +} diff --git a/protocol/credential_test.go b/protocol/credential_test.go index 41aa823b..6986265a 100644 --- a/protocol/credential_test.go +++ b/protocol/credential_test.go @@ -92,8 +92,8 @@ func TestParseCredentialCreationResponse(t *testing.T) { t.Errorf("ParseCredentialCreationResponse() error = %v, wantErr %v", err, tt.wantErr) return } - if !reflect.DeepEqual(got.Extensions, tt.want.Extensions) { - t.Errorf("Extensions = %v \n want: %v", got, tt.want) + if !reflect.DeepEqual(got.ClientExtensionResults, tt.want.ClientExtensionResults) { + t.Errorf("ClientExtensionResults = %v \n want: %v", got, tt.want) } if !reflect.DeepEqual(got.ID, tt.want.ID) { t.Errorf("ID = %v \n want: %v", got, tt.want) diff --git a/protocol/extensions.go b/protocol/extensions.go index 6d6ea7b0..0cfa2ed7 100644 --- a/protocol/extensions.go +++ b/protocol/extensions.go @@ -2,7 +2,7 @@ package protocol // Extensions are discussed in §9. WebAuthn Extensions (https://www.w3.org/TR/webauthn/#extensions). -// For a list of commonly supported extenstions, see §10. Defined Extensions +// For a list of commonly supported extensions, see §10. Defined Extensions // (https://www.w3.org/TR/webauthn/#sctn-defined-extensions). -type AuthenticationExtensionsClientOutputs map[interface{}]interface{} +type AuthenticationExtensionsClientOutputs map[string]interface{} diff --git a/protocol/webauthncose/webauthncose.go b/protocol/webauthncose/webauthncose.go index 37252a7c..835f0b72 100644 --- a/protocol/webauthncose/webauthncose.go +++ b/protocol/webauthncose/webauthncose.go @@ -181,6 +181,19 @@ func ParsePublicKey(keyBytes []byte) (interface{}, error) { } } +func ParseFIDOPublicKey(keyBytes []byte) (EC2PublicKeyData, error) { + x, y := elliptic.Unmarshal(elliptic.P256(), keyBytes) + + return EC2PublicKeyData{ + PublicKeyData: PublicKeyData{ + Algorithm: int64(AlgES256), + KeyType: int64(EllipticKey), + }, + XCoord: x.Bytes(), + YCoord: y.Bytes(), + }, nil +} + // COSEAlgorithmIdentifier From §5.10.5. A number identifying a cryptographic algorithm. The algorithm // identifiers SHOULD be values registered in the IANA COSE Algorithms registry // [https://www.w3.org/TR/webauthn/#biblio-iana-cose-algs-reg], for instance, -7 for "ES256" diff --git a/webauthn/credential.go b/webauthn/credential.go index a651f155..224bb7f6 100644 --- a/webauthn/credential.go +++ b/webauthn/credential.go @@ -21,7 +21,6 @@ type Credential struct { // MakeNewCredential will return a credential pointer on successful validation of a registration response func MakeNewCredential(c *protocol.ParsedCredentialCreationData) (*Credential, error) { - newCredential := &Credential{ ID: c.Response.AttestationObject.AuthData.AttData.CredentialID, PublicKey: c.Response.AttestationObject.AuthData.AttData.CredentialPublicKey, diff --git a/webauthn/login.go b/webauthn/login.go index d5545207..4ce3a88d 100644 --- a/webauthn/login.go +++ b/webauthn/login.go @@ -59,6 +59,7 @@ func (webauthn *WebAuthn) BeginLogin(user User, opts ...LoginOption) (*protocol. UserID: user.WebAuthnID(), AllowedCredentialIDs: requestOptions.GetAllowedCredentialIDs(), UserVerification: requestOptions.UserVerification, + Extensions: requestOptions.Extensions, } response := protocol.CredentialAssertion{requestOptions} @@ -169,8 +170,13 @@ func (webauthn *WebAuthn) ValidateLogin(user User, session SessionData, parsedRe rpID := webauthn.Config.RPID rpOrigin := webauthn.Config.RPOrigin + appID, err := parsedResponse.GetAppID(session.Extensions, loginCredential.AttestationType) + if err != nil { + return nil, err + } + // Handle steps 4 through 16 - validError := parsedResponse.Verify(session.Challenge, rpID, rpOrigin, shouldVerifyUser, loginCredential.PublicKey) + validError := parsedResponse.Verify(session.Challenge, rpID, rpOrigin, appID, shouldVerifyUser, loginCredential.PublicKey) if validError != nil { return nil, validError } diff --git a/webauthn/registration.go b/webauthn/registration.go index 5feed649..15223814 100644 --- a/webauthn/registration.go +++ b/webauthn/registration.go @@ -126,43 +126,43 @@ func (webauthn *WebAuthn) CreateCredential(user User, session SessionData, parse func defaultRegistrationCredentialParameters() []protocol.CredentialParameter { return []protocol.CredentialParameter{ - protocol.CredentialParameter{ + { Type: protocol.PublicKeyCredentialType, Algorithm: webauthncose.AlgES256, }, - protocol.CredentialParameter{ + { Type: protocol.PublicKeyCredentialType, Algorithm: webauthncose.AlgES384, }, - protocol.CredentialParameter{ + { Type: protocol.PublicKeyCredentialType, Algorithm: webauthncose.AlgES512, }, - protocol.CredentialParameter{ + { Type: protocol.PublicKeyCredentialType, Algorithm: webauthncose.AlgRS256, }, - protocol.CredentialParameter{ + { Type: protocol.PublicKeyCredentialType, Algorithm: webauthncose.AlgRS384, }, - protocol.CredentialParameter{ + { Type: protocol.PublicKeyCredentialType, Algorithm: webauthncose.AlgRS512, }, - protocol.CredentialParameter{ + { Type: protocol.PublicKeyCredentialType, Algorithm: webauthncose.AlgPS256, }, - protocol.CredentialParameter{ + { Type: protocol.PublicKeyCredentialType, Algorithm: webauthncose.AlgPS384, }, - protocol.CredentialParameter{ + { Type: protocol.PublicKeyCredentialType, Algorithm: webauthncose.AlgPS512, }, - protocol.CredentialParameter{ + { Type: protocol.PublicKeyCredentialType, Algorithm: webauthncose.AlgEdDSA, }, diff --git a/webauthn/session.go b/webauthn/session.go index d6dc51e1..d6c56436 100644 --- a/webauthn/session.go +++ b/webauthn/session.go @@ -9,4 +9,5 @@ type SessionData struct { UserID []byte `json:"user_id"` AllowedCredentialIDs [][]byte `json:"allowed_credentials,omitempty"` UserVerification protocol.UserVerificationRequirement `json:"userVerification"` + Extensions protocol.AuthenticationExtensions `json:"extensions,omitempty"` }