From f68288982f397ebdb4d3e79e96dfa64492a7f7a6 Mon Sep 17 00:00:00 2001 From: James Elliott Date: Sun, 17 Nov 2024 16:35:18 +1100 Subject: [PATCH 1/3] feat(protocol): enhance mds errors This adds significant improvements to the MDS3 errors helping identify the root cause of a validation error. --- protocol/attestation.go | 36 ++++-------------------------------- protocol/errors.go | 4 ++++ protocol/metadata.go | 34 +++++++++++++++++++++++++--------- webauthn/login.go | 6 ++++-- 4 files changed, 37 insertions(+), 43 deletions(-) diff --git a/protocol/attestation.go b/protocol/attestation.go index be30e92a..b8f90b3f 100644 --- a/protocol/attestation.go +++ b/protocol/attestation.go @@ -186,46 +186,18 @@ func (a *AttestationObject) VerifyAttestation(clientDataHash []byte, mds metadat return nil } + var protoErr *Error + ctx := context.Background() - if entry, err = mds.GetEntry(ctx, aaguid); err != nil { - return ErrInvalidAttestation.WithInfo(fmt.Sprintf("Error occurred retrieving metadata entry during attestation validation: %+v", err)).WithDetails(fmt.Sprintf("Error occurred looking up entry for AAGUID %s", aaguid.String())) + if entry, protoErr = ValidateMetadata(context.Background(), aaguid, attestationType, mds); protoErr != nil { + return ErrInvalidAttestation.WithInfo(fmt.Sprintf("Error occurred validating metadata during attestation validation: %+v", err)).WithDetails(protoErr.DevInfo) } if entry == nil { - if aaguid == uuid.Nil && mds.GetValidateEntryPermitZeroAAGUID(ctx) { - return nil - } - - if mds.GetValidateEntry(ctx) { - return ErrInvalidAttestation.WithDetails(fmt.Sprintf("AAGUID %s not found in metadata during attestation validation", aaguid.String())) - } - return nil } - if mds.GetValidateAttestationTypes(ctx) { - found := false - - for _, atype := range entry.MetadataStatement.AttestationTypes { - if string(atype) == attestationType { - found = true - - break - } - } - - if !found { - return ErrInvalidAttestation.WithDetails(fmt.Sprintf("Authenticator with invalid attestation type encountered during attestation validation. The attestation type '%s' is not known to be used by AAGUID '%s'", attestationType, aaguid.String())) - } - } - - if mds.GetValidateStatus(ctx) { - if err = mds.ValidateStatusReports(ctx, entry.StatusReports); err != nil { - return ErrInvalidAttestation.WithDetails(fmt.Sprintf("Authenticator with invalid status encountered during attestation validation. %s", err.Error())) - } - } - if mds.GetValidateTrustAnchor(ctx) { if x5cs == nil { return nil diff --git a/protocol/errors.go b/protocol/errors.go index 6d134d63..ceda6f79 100644 --- a/protocol/errors.go +++ b/protocol/errors.go @@ -40,6 +40,10 @@ var ( Type: "invalid_attestation", Details: "Invalid attestation data", } + ErrMetadata = &Error{ + Type: "invalid_metadata", + Details: "", + } ErrAttestationFormat = &Error{ Type: "invalid_attestation", Details: "Invalid attestation format", diff --git a/protocol/metadata.go b/protocol/metadata.go index a7e66515..edb9744a 100644 --- a/protocol/metadata.go +++ b/protocol/metadata.go @@ -9,36 +9,52 @@ import ( "github.com/go-webauthn/webauthn/metadata" ) -func ValidateMetadata(ctx context.Context, aaguid uuid.UUID, mds metadata.Provider) (err error) { +func ValidateMetadata(ctx context.Context, aaguid uuid.UUID, attestationType string, mds metadata.Provider) (entry *metadata.Entry, protoErr *Error) { if mds == nil { - return nil + return nil, nil } var ( - entry *metadata.Entry + err error ) if entry, err = mds.GetEntry(ctx, aaguid); err != nil { - return err + return nil, ErrMetadata.WithInfo(fmt.Sprintf("Failed to validate authenticator metadata for Authenticator Attestation GUID '%s'. Error occurred retreiving the metadata entry: %+v", aaguid, err)) } if entry == nil { if aaguid == uuid.Nil && mds.GetValidateEntryPermitZeroAAGUID(ctx) { - return nil + return nil, nil } if mds.GetValidateEntry(ctx) { - return fmt.Errorf("error occurred performing authenticator entry validation: AAGUID entry has not been registered with the metadata service") + return nil, ErrMetadata.WithInfo(fmt.Sprintf("Failed to validate authenticator metadata for Authenticator Attestation GUID '%s'. The authenticator has no registered metadata.", aaguid)) } - return nil + return nil, nil + } + + if mds.GetValidateAttestationTypes(ctx) { + found := false + + for _, atype := range entry.MetadataStatement.AttestationTypes { + if string(atype) == attestationType { + found = true + + break + } + } + + if !found { + return entry, ErrMetadata.WithInfo(fmt.Sprintf("Failed to validate authenticator metadata for Authenticator Attestation GUID '%s'. The attestation type '%s' is not known to be used by this authenticator.", aaguid.String(), attestationType)) + } } if mds.GetValidateStatus(ctx) { if err = mds.ValidateStatusReports(ctx, entry.StatusReports); err != nil { - return fmt.Errorf("error occurred performing authenticator status validation: %w", err) + return entry, ErrMetadata.WithInfo(fmt.Sprintf("Failed to validate authenticator metadata for Authenticator Attestation GUID '%s'. Error occurred validating the authenticator status: %+v", aaguid, err)) } } - return nil + return entry, nil } diff --git a/webauthn/login.go b/webauthn/login.go index acd1e26a..36e86e88 100644 --- a/webauthn/login.go +++ b/webauthn/login.go @@ -347,8 +347,10 @@ func (webauthn *WebAuthn) validateLogin(user User, session SessionData, parsedRe return nil, protocol.ErrBadRequest.WithDetails("Failed to decode AAGUID").WithInfo(fmt.Sprintf("Error occurred decoding AAGUID from the credential record: %s", err)) } - if err = protocol.ValidateMetadata(context.Background(), aaguid, webauthn.Config.MDS); err != nil { - return nil, protocol.ErrBadRequest.WithDetails("Failed to validate credential record metadata").WithInfo(fmt.Sprintf("Error occurred validating authenticator metadata from the credential record: %s", err)) + var protoErr *protocol.Error + + if _, protoErr = protocol.ValidateMetadata(context.Background(), aaguid, credential.AttestationType, webauthn.Config.MDS); protoErr != nil { + return nil, protocol.ErrBadRequest.WithDetails("Failed to validate credential record metadata").WithInfo(protoErr.DevInfo) } } From 961325b7703edc10b9287d7ef56444ae7add88fe Mon Sep 17 00:00:00 2001 From: James Elliott Date: Wed, 27 Nov 2024 20:09:02 +1100 Subject: [PATCH 2/3] feat: enhancements --- README.md | 2 +- protocol/assertion.go | 20 +++--- protocol/assertion_test.go | 4 +- protocol/attestation.go | 79 ++--------------------- protocol/attestation_androidkey_test.go | 6 +- protocol/attestation_apple.go | 6 +- protocol/attestation_packed.go | 9 +-- protocol/attestation_packed_test.go | 2 +- protocol/attestation_safetynet.go | 1 + protocol/attestation_tpm.go | 69 ++++++++++++++------ protocol/attestation_tpm_test.go | 1 + protocol/client.go | 8 ++- protocol/credential.go | 2 +- protocol/credential_test.go | 8 +-- protocol/metadata.go | 86 ++++++++++++++++++++++--- protocol/options_test.go | 2 +- webauthn/login.go | 4 +- 17 files changed, 165 insertions(+), 144 deletions(-) diff --git a/README.md b/README.md index 202c1775..82cd186d 100644 --- a/README.md +++ b/README.md @@ -237,7 +237,7 @@ func beginLogin() { allowList := make([]protocol.CredentialDescriptor, 1) allowList[0] = protocol.CredentialDescriptor{ CredentialID: credentialToAllowID, - Type: protocol.CredentialType("public-key"), + Type: protocol.PublicKeyCredentialType, } user := datastore.GetUser() // Get the user diff --git a/protocol/assertion.go b/protocol/assertion.go index 50cb5008..b055e01b 100644 --- a/protocol/assertion.go +++ b/protocol/assertion.go @@ -54,8 +54,10 @@ func ParseCredentialRequestResponse(response *http.Request) (*ParsedCredentialAs return nil, ErrBadRequest.WithDetails("No response given") } - defer response.Body.Close() - defer io.Copy(io.Discard, response.Body) + defer func(request *http.Request) { + _, _ = io.Copy(io.Discard, request.Body) + _ = request.Body.Close() + }(response) return ParseCredentialRequestResponseBody(response.Body) } @@ -98,17 +100,15 @@ func (car CredentialAssertionResponse) Parse() (par *ParsedCredentialAssertionDa return nil, ErrBadRequest.WithDetails("CredentialAssertionResponse with ID not base64url encoded") } - if car.Type != "public-key" { + if car.Type != string(PublicKeyCredentialType) { return nil, ErrBadRequest.WithDetails("CredentialAssertionResponse with bad type") } var attachment AuthenticatorAttachment - switch car.AuthenticatorAttachment { - case "platform": - attachment = Platform - case "cross-platform": - attachment = CrossPlatform + switch att := AuthenticatorAttachment(car.AuthenticatorAttachment); att { + case Platform, CrossPlatform: + attachment = att } par = &ParsedCredentialAssertionData{ @@ -143,9 +143,9 @@ func (p *ParsedCredentialAssertionData) Verify(storedChallenge string, relyingPa // 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." // We handle these steps in part as we verify but also beforehand - + // // Handle steps 7 through 10 of assertion by verifying stored data against the Collected Client Data - // returned by the authenticator + // returned by the authenticator. validError := p.Response.CollectedClientData.Verify(storedChallenge, AssertCeremony, rpOrigins, rpTopOrigins, rpTopOriginsVerify) if validError != nil { return validError diff --git a/protocol/assertion_test.go b/protocol/assertion_test.go index 2731850c..c93f467e 100644 --- a/protocol/assertion_test.go +++ b/protocol/assertion_test.go @@ -44,7 +44,7 @@ func TestParseCredentialRequestResponse(t *testing.T) { ParsedPublicKeyCredential: ParsedPublicKeyCredential{ ParsedCredential: ParsedCredential{ ID: "AI7D5q2P0LS-Fal9ZT7CHM2N5BLbUunF92T8b6iYC199bO2kagSuU05-5dZGqb1SP0A0lyTWng", - Type: "public-key", + Type: string(PublicKeyCredentialType), }, RawID: byteID, ClientExtensionResults: map[string]any{ @@ -74,7 +74,7 @@ func TestParseCredentialRequestResponse(t *testing.T) { Raw: CredentialAssertionResponse{ PublicKeyCredential: PublicKeyCredential{ Credential: Credential{ - Type: "public-key", + Type: string(PublicKeyCredentialType), ID: "AI7D5q2P0LS-Fal9ZT7CHM2N5BLbUunF92T8b6iYC199bO2kagSuU05-5dZGqb1SP0A0lyTWng", }, RawID: byteID, diff --git a/protocol/attestation.go b/protocol/attestation.go index b8f90b3f..83a004ad 100644 --- a/protocol/attestation.go +++ b/protocol/attestation.go @@ -3,7 +3,6 @@ package protocol import ( "context" "crypto/sha256" - "crypto/x509" "encoding/json" "fmt" @@ -144,12 +143,12 @@ func (a *AttestationObject) VerifyAttestation(clientDataHash []byte, mds metadat // list of registered WebAuthn Attestation Statement Format Identifier // values is maintained in the IANA registry of the same name // [WebAuthn-Registries] (https://www.w3.org/TR/webauthn/#biblio-webauthn-registries). - + // // Since there is not an active registry yet, we'll check it against our internal // Supported types. - + // // But first let's make sure attestation is present. If it isn't, we don't need to handle - // any of the following steps + // any of the following steps. if AttestationFormat(a.Format) == AttestationFormatNone { if len(a.AttStatement) != 0 { return ErrAttestationFormat.WithInfo("Attestation format none with attestation present") @@ -173,7 +172,6 @@ func (a *AttestationObject) VerifyAttestation(clientDataHash []byte, mds metadat var ( aaguid uuid.UUID - entry *metadata.Entry ) if len(a.AuthData.AttData.AAGUID) != 0 { @@ -188,75 +186,8 @@ func (a *AttestationObject) VerifyAttestation(clientDataHash []byte, mds metadat var protoErr *Error - ctx := context.Background() - - if entry, protoErr = ValidateMetadata(context.Background(), aaguid, attestationType, mds); protoErr != nil { - return ErrInvalidAttestation.WithInfo(fmt.Sprintf("Error occurred validating metadata during attestation validation: %+v", err)).WithDetails(protoErr.DevInfo) - } - - if entry == nil { - return nil - } - - if mds.GetValidateTrustAnchor(ctx) { - if x5cs == nil { - return nil - } - - var ( - x5c, parsed *x509.Certificate - x5cis []*x509.Certificate - raw []byte - ok bool - ) - - if len(x5cs) == 0 { - return ErrInvalidAttestation.WithDetails("Unable to parse attestation certificate from x5c during attestation validation").WithInfo("The attestation had no certificates") - } - - for _, x5cAny := range x5cs { - 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])) - } - - 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 = parsed - } else { - x5cis = append(x5cis, parsed) - } - } - - if attestationType == string(metadata.AttCA) { - if err = tpmParseSANExtension(x5c); err != nil { - return err - } - - if err = tpmRemoveEKU(x5c); err != nil { - return err - } - - for _, parent := range x5cis { - if err = tpmRemoveEKU(parent); err != nil { - return err - } - } - } - - 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(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)) - } - } + if protoErr = ValidateMetadata(context.Background(), mds, aaguid, attestationType, x5cs); protoErr != nil { + return ErrInvalidAttestation.WithInfo(fmt.Sprintf("Error occurred validating metadata during attestation validation: %+v", protoErr)).WithDetails(protoErr.DevInfo) } return nil diff --git a/protocol/attestation_androidkey_test.go b/protocol/attestation_androidkey_test.go index bdec9bcd..ba9d94d8 100644 --- a/protocol/attestation_androidkey_test.go +++ b/protocol/attestation_androidkey_test.go @@ -54,14 +54,10 @@ func TestVerifyAndroidKeyFormat(t *testing.T) { t.Errorf("verifyAndroidKeyFormat() error = %v, wantErr %v", err, tt.wantErr) return } + if got != tt.want { t.Errorf("verifyAndroidKeyFormat() got = %v, want %v", got, tt.want) } - /* - if !reflect.DeepEqual(got1, tt.want1) { - t.Errorf("verifySafetyNetFormat() got1 = %v, want %v", got1, tt.want1) - } - */ }) } } diff --git a/protocol/attestation_apple.go b/protocol/attestation_apple.go index 9b80b288..a69232ab 100644 --- a/protocol/attestation_apple.go +++ b/protocol/attestation_apple.go @@ -34,8 +34,7 @@ func init() { func verifyAppleFormat(att AttestationObject, clientDataHash []byte, _ metadata.Provider) (string, []any, error) { // Step 1. Verify that attStmt is valid CBOR conforming to the syntax defined // above and perform CBOR decoding on it to extract the contained fields. - - // If x5c is not present, return an error + // If x5c is not present, return an error. x5c, x509present := att.AttStatement[stmtX5C].([]any) if !x509present { // Handle Basic Attestation steps for the x509 Certificate @@ -104,7 +103,8 @@ func verifyAppleFormat(att AttestationObject, clientDataHash []byte, _ metadata. return string(metadata.AnonCA), x5c, nil } -// Apple has not yet publish schema for the extension(as of JULY 2021.) +// AppleAnonymousAttestation represents the attestation format for Apple, who have not yet published a schema for the +// extension (as of JULY 2021.) type AppleAnonymousAttestation struct { Nonce []byte `asn1:"tag:1,explicit"` } diff --git a/protocol/attestation_packed.go b/protocol/attestation_packed.go index a46a47f1..ddd17ee2 100644 --- a/protocol/attestation_packed.go +++ b/protocol/attestation_packed.go @@ -37,10 +37,8 @@ func init() { func verifyPackedFormat(att AttestationObject, clientDataHash []byte, _ metadata.Provider) (string, []any, error) { // Step 1. Verify that attStmt is valid CBOR conforming to the syntax defined // above and perform CBOR decoding on it to extract the contained fields. - // Get the alg value - A COSEAlgorithmIdentifier containing the identifier of the algorithm // used to generate the attestation signature. - alg, present := att.AttStatement[stmtAlgorithm].(int64) if !present { return string(AttestationFormatPacked), nil, ErrAttestationFormat.WithDetails("Error retrieving alg value") @@ -202,10 +200,6 @@ func handleECDAAAttestation(signature, clientDataHash, ecdaaKeyID []byte) (strin } func handleSelfAttestation(alg int64, pubKey, authData, clientDataHash, signature []byte) (string, []any, error) { - // §4.1 Validate that alg matches the algorithm of the credentialPublicKey in authenticatorData. - - // §4.2 Verify that sig is a valid signature over the concatenation of authenticatorData and - // clientDataHash using the credential public key with alg. verificationData := append(authData, clientDataHash...) key, err := webauthncose.ParsePublicKey(pubKey) @@ -213,6 +207,7 @@ func handleSelfAttestation(alg int64, pubKey, authData, clientDataHash, signatur return "", nil, ErrAttestationFormat.WithDetails(fmt.Sprintf("Error parsing the public key: %+v\n", err)) } + // §4.1 Validate that alg matches the algorithm of the credentialPublicKey in authenticatorData. switch k := key.(type) { case webauthncose.OKPPublicKeyData: err = verifyKeyAlgorithm(k.Algorithm, alg) @@ -228,6 +223,8 @@ func handleSelfAttestation(alg int64, pubKey, authData, clientDataHash, signatur return "", nil, err } + // §4.2 Verify that sig is a valid signature over the concatenation of authenticatorData and + // clientDataHash using the credential public key with alg. valid, err := webauthncose.VerifySignature(key, verificationData, signature) if !valid && err == nil { return "", nil, ErrInvalidAttestation.WithDetails("Unable to verify signature") diff --git a/protocol/attestation_packed_test.go b/protocol/attestation_packed_test.go index 7f222668..11ad9b2e 100644 --- a/protocol/attestation_packed_test.go +++ b/protocol/attestation_packed_test.go @@ -67,10 +67,10 @@ func Test_verifyPackedFormat(t *testing.T) { return } + // TODO: Consider doing something with the second return value from verifyPackedFormat, x5c. if got != tt.want { t.Errorf("verifyPackedFormat() got = %v, want %v", got, tt.want) } - // TODO: Consider doing something with the second return value from verifyPackedFormat, x5c. }) } } diff --git a/protocol/attestation_safetynet.go b/protocol/attestation_safetynet.go index 849cda18..7fa49ca1 100644 --- a/protocol/attestation_safetynet.go +++ b/protocol/attestation_safetynet.go @@ -85,6 +85,7 @@ func verifySafetyNetFormat(att AttestationObject, clientDataHash []byte, mds met } cert, err := x509.ParseCertificate(o[:n]) + return cert.PublicKey, err }) diff --git a/protocol/attestation_tpm.go b/protocol/attestation_tpm.go index 873e8640..881b8aa1 100644 --- a/protocol/attestation_tpm.go +++ b/protocol/attestation_tpm.go @@ -115,6 +115,7 @@ func verifyTPMFormat(att AttestationObject, clientDataHash []byte, _ metadata.Pr // 3/4 Verify that extraData is set to the hash of attToBeSigned using the hash algorithm employed in "alg". h := webauthncose.HasherFromCOSEAlg(coseAlg) h.Write(attToBeSigned) + if !bytes.Equal(certInfo.ExtraData, h.Sum(nil)) { return "", nil, ErrAttestationFormat.WithDetails("ExtraData is not set to hash of attToBeSigned") } @@ -219,23 +220,24 @@ func verifyTPMFormat(att AttestationObject, clientDataHash []byte, _ metadata.Pr return string(metadata.AttCA), x5c, err } +// forEachSAN loops through the TPM SAN extension. +// +// RFC 5280, 4.2.1.6 +// SubjectAltName ::= GeneralNames +// +// GeneralNames ::= SEQUENCE SIZE (1..MAX) OF GeneralName +// +// GeneralName ::= CHOICE { +// otherName [0] OtherName, +// rfc822Name [1] IA5String, +// dNSName [2] IA5String, +// x400Address [3] ORAddress, +// directoryName [4] Name, +// ediPartyName [5] EDIPartyName, +// uniformResourceIdentifier [6] IA5String, +// iPAddress [7] OCTET STRING, +// registeredID [8] OBJECT IDENTIFIER } func forEachSAN(extension []byte, callback func(tag int, data []byte) error) error { - // RFC 5280, 4.2.1.6 - - // SubjectAltName ::= GeneralNames - // - // GeneralNames ::= SEQUENCE SIZE (1..MAX) OF GeneralName - // - // GeneralName ::= CHOICE { - // otherName [0] OtherName, - // rfc822Name [1] IA5String, - // dNSName [2] IA5String, - // x400Address [3] ORAddress, - // directoryName [4] Name, - // ediPartyName [5] EDIPartyName, - // uniformResourceIdentifier [6] IA5String, - // iPAddress [7] OCTET STRING, - // registeredID [8] OBJECT IDENTIFIER } var seq asn1.RawValue rest, err := asn1.Unmarshal(extension, &seq) @@ -284,13 +286,16 @@ func parseSANExtension(value []byte) (manufacturer string, model string, version case nameTypeDN: tpmDeviceAttributes := pkix.RDNSequence{} _, err := asn1.Unmarshal(data, &tpmDeviceAttributes) + if err != nil { return err } + for _, rdn := range tpmDeviceAttributes { if len(rdn) == 0 { continue } + for _, atv := range rdn { value, ok := atv.Value.(string) if !ok { @@ -300,15 +305,18 @@ func parseSANExtension(value []byte) (manufacturer string, model string, version if atv.Type.Equal(tcgAtTpmManufacturer) { manufacturer = strings.TrimPrefix(value, "id:") } + if atv.Type.Equal(tcgAtTpmModel) { model = value } + if atv.Type.Equal(tcgAtTpmVersion) { version = strings.TrimPrefix(value, "id:") } } } } + return nil }) @@ -359,21 +367,40 @@ func isValidTPMManufacturer(id string) bool { return false } -func tpmParseSANExtension(attestation *x509.Certificate) (err error) { +func tpmParseAIKAttCA(x5c *x509.Certificate, x5cis []*x509.Certificate) (err *Error) { + if err = tpmParseSANExtension(x5c); err != nil { + return err + } + + if err = tpmRemoveEKU(x5c); err != nil { + return err + } + + for _, parent := range x5cis { + if err = tpmRemoveEKU(parent); err != nil { + return err + } + } + + return nil +} + +func tpmParseSANExtension(attestation *x509.Certificate) (protoErr *Error) { var ( manufacturer, model, version string + err error ) for _, ext := range attestation.Extensions { if ext.Id.Equal(oidExtensionSubjectAltName) { if manufacturer, model, version, err = parseSANExtension(ext.Value); err != nil { - return ErrInvalidAttestation.WithDetails("Authenticator with invalid AIK SAN data encountered during attestation validation.").WithInfo(fmt.Sprintf("Error occurred parsing SAN extension: %s", err.Error())) + return ErrInvalidAttestation.WithDetails("Authenticator with invalid Authenticator Identity Key SAN data encountered during attestation validation.").WithInfo(fmt.Sprintf("Error occurred parsing SAN extension: %s", err.Error())) } } } if manufacturer == "" || model == "" || version == "" { - return ErrAttestationFormat.WithDetails("Invalid SAN data in AIK certificate") + return ErrAttestationFormat.WithDetails("Invalid SAN data in AIK certificate.") } var unhandled []asn1.ObjectIdentifier @@ -404,7 +431,7 @@ type tpmBasicConstraints struct { } // Remove extension key usage to avoid ExtKeyUsage check failure. -func tpmRemoveEKU(x5c *x509.Certificate) error { +func tpmRemoveEKU(x5c *x509.Certificate) *Error { var ( unknown []asn1.ObjectIdentifier hasAiK bool @@ -425,7 +452,7 @@ func tpmRemoveEKU(x5c *x509.Certificate) error { } if !hasAiK { - return ErrAttestationFormat.WithDetails("AIK certificate missing EKU") + return ErrAttestationFormat.WithDetails("Attestation Identity Key certificate missing required Extended Key Usage.") } x5c.UnknownExtKeyUsage = unknown diff --git a/protocol/attestation_tpm_test.go b/protocol/attestation_tpm_test.go index b7848827..f122924f 100644 --- a/protocol/attestation_tpm_test.go +++ b/protocol/attestation_tpm_test.go @@ -30,6 +30,7 @@ func TestTPMAttestationVerificationSuccess(t *testing.T) { attestationType, _, err := verifyTPMFormat(pcc.Response.AttestationObject, clientDataHash[:], nil) require.NoError(t, err) + if err != nil { t.Fatalf("Not valid: %+v", err) } diff --git a/protocol/client.go b/protocol/client.go index 7e8356a3..7aa0d28d 100644 --- a/protocol/client.go +++ b/protocol/client.go @@ -43,13 +43,15 @@ type TokenBinding struct { type TokenBindingStatus string const ( - // Indicates token binding was used when communicating with the + // Present indicates token binding was used when communicating with the // Relying Party. In this case, the id member MUST be present. Present TokenBindingStatus = "present" - // Indicates token binding was used when communicating with the + + // Supported indicates token binding was used when communicating with the // negotiated when communicating with the Relying Party. Supported TokenBindingStatus = "supported" - // Indicates token binding not supported + + // NotSupported indicates token binding not supported // when communicating with the Relying Party. NotSupported TokenBindingStatus = "not-supported" ) diff --git a/protocol/credential.go b/protocol/credential.go index 5c9f6b00..a36b2068 100644 --- a/protocol/credential.go +++ b/protocol/credential.go @@ -115,7 +115,7 @@ func (ccr CredentialCreationResponse) Parse() (pcc *ParsedCredentialCreationData return nil, ErrBadRequest.WithDetails("Parse error for Registration").WithInfo("Missing type") } - if ccr.PublicKeyCredential.Credential.Type != "public-key" { + if ccr.PublicKeyCredential.Credential.Type != string(PublicKeyCredentialType) { return nil, ErrBadRequest.WithDetails("Parse error for Registration").WithInfo("Type not public-key") } diff --git a/protocol/credential_test.go b/protocol/credential_test.go index b4e7ba8b..17febb14 100644 --- a/protocol/credential_test.go +++ b/protocol/credential_test.go @@ -41,7 +41,7 @@ func TestParseCredentialCreationResponse(t *testing.T) { ParsedPublicKeyCredential: ParsedPublicKeyCredential{ ParsedCredential: ParsedCredential{ ID: "6xrtBhJQW6QU4tOaB4rrHaS2Ks0yDDL_q8jDC16DEjZ-VLVf4kCRkvl2xp2D71sTPYns-exsHQHTy3G-zJRK8g", - Type: "public-key", + Type: string(PublicKeyCredentialType), }, RawID: byteID, ClientExtensionResults: AuthenticationExtensionsClientOutputs{ @@ -74,7 +74,7 @@ func TestParseCredentialCreationResponse(t *testing.T) { Raw: CredentialCreationResponse{ PublicKeyCredential: PublicKeyCredential{ Credential: Credential{ - Type: "public-key", + Type: string(PublicKeyCredentialType), ID: "6xrtBhJQW6QU4tOaB4rrHaS2Ks0yDDL_q8jDC16DEjZ-VLVf4kCRkvl2xp2D71sTPYns-exsHQHTy3G-zJRK8g", }, RawID: byteID, @@ -182,7 +182,7 @@ func TestParsedCredentialCreationData_Verify(t *testing.T) { ParsedPublicKeyCredential: ParsedPublicKeyCredential{ ParsedCredential: ParsedCredential{ ID: "6xrtBhJQW6QU4tOaB4rrHaS2Ks0yDDL_q8jDC16DEjZ-VLVf4kCRkvl2xp2D71sTPYns-exsHQHTy3G-zJRK8g", - Type: "public-key", + Type: string(PublicKeyCredentialType), }, RawID: byteID, }, @@ -210,7 +210,7 @@ func TestParsedCredentialCreationData_Verify(t *testing.T) { Raw: CredentialCreationResponse{ PublicKeyCredential: PublicKeyCredential{ Credential: Credential{ - Type: "public-key", + Type: string(PublicKeyCredentialType), ID: "6xrtBhJQW6QU4tOaB4rrHaS2Ks0yDDL_q8jDC16DEjZ-VLVf4kCRkvl2xp2D71sTPYns-exsHQHTy3G-zJRK8g", }, RawID: byteID, diff --git a/protocol/metadata.go b/protocol/metadata.go index edb9744a..670ff871 100644 --- a/protocol/metadata.go +++ b/protocol/metadata.go @@ -2,6 +2,7 @@ package protocol import ( "context" + "crypto/x509" "fmt" "github.com/google/uuid" @@ -9,29 +10,30 @@ import ( "github.com/go-webauthn/webauthn/metadata" ) -func ValidateMetadata(ctx context.Context, aaguid uuid.UUID, attestationType string, mds metadata.Provider) (entry *metadata.Entry, protoErr *Error) { +func ValidateMetadata(ctx context.Context, mds metadata.Provider, aaguid uuid.UUID, attestationType string, x5cs []any) (protoErr *Error) { if mds == nil { - return nil, nil + return nil } var ( - err error + entry *metadata.Entry + err error ) if entry, err = mds.GetEntry(ctx, aaguid); err != nil { - return nil, ErrMetadata.WithInfo(fmt.Sprintf("Failed to validate authenticator metadata for Authenticator Attestation GUID '%s'. Error occurred retreiving the metadata entry: %+v", aaguid, err)) + return ErrMetadata.WithInfo(fmt.Sprintf("Failed to validate authenticator metadata for Authenticator Attestation GUID '%s'. Error occurred retreiving the metadata entry: %+v", aaguid, err)) } if entry == nil { if aaguid == uuid.Nil && mds.GetValidateEntryPermitZeroAAGUID(ctx) { - return nil, nil + return nil } if mds.GetValidateEntry(ctx) { - return nil, ErrMetadata.WithInfo(fmt.Sprintf("Failed to validate authenticator metadata for Authenticator Attestation GUID '%s'. The authenticator has no registered metadata.", aaguid)) + return ErrMetadata.WithInfo(fmt.Sprintf("Failed to validate authenticator metadata for Authenticator Attestation GUID '%s'. The authenticator has no registered metadata.", aaguid)) } - return nil, nil + return nil } if mds.GetValidateAttestationTypes(ctx) { @@ -46,15 +48,79 @@ func ValidateMetadata(ctx context.Context, aaguid uuid.UUID, attestationType str } if !found { - return entry, ErrMetadata.WithInfo(fmt.Sprintf("Failed to validate authenticator metadata for Authenticator Attestation GUID '%s'. The attestation type '%s' is not known to be used by this authenticator.", aaguid.String(), attestationType)) + return ErrMetadata.WithInfo(fmt.Sprintf("Failed to validate authenticator metadata for Authenticator Attestation GUID '%s'. The attestation type '%s' is not known to be used by this authenticator.", aaguid.String(), attestationType)) } } if mds.GetValidateStatus(ctx) { if err = mds.ValidateStatusReports(ctx, entry.StatusReports); err != nil { - return entry, ErrMetadata.WithInfo(fmt.Sprintf("Failed to validate authenticator metadata for Authenticator Attestation GUID '%s'. Error occurred validating the authenticator status: %+v", aaguid, err)) + return ErrMetadata.WithInfo(fmt.Sprintf("Failed to validate authenticator metadata for Authenticator Attestation GUID '%s'. Error occurred validating the authenticator status: %+v", aaguid, err)) } } - return entry, nil + if mds.GetValidateTrustAnchor(ctx) { + if len(x5cs) == 0 { + return nil + } + + var ( + x5c, parsed *x509.Certificate + x5cis []*x509.Certificate + raw []byte + ok bool + ) + + for i, x5cAny := range x5cs { + if raw, ok = x5cAny.([]byte); !ok { + return ErrMetadata.WithDetails(fmt.Sprintf("Failed to parse attestation certificate from x5c during attestation validation for Authenticator Attestation GUID '%s'.", aaguid)).WithInfo(fmt.Sprintf("The %s certificate in the attestation was type '%T' but '[]byte' was expected", loopOrdinalNumber(i), x5cAny)) + } + + if parsed, err = x509.ParseCertificate(raw); err != nil { + return ErrMetadata.WithDetails(fmt.Sprintf("Failed to parse attestation certificate from x5c during attestation validation for Authenticator Attestation GUID '%s'.", aaguid)).WithInfo(fmt.Sprintf("Error returned from x509.ParseCertificate: %+v", err)) + } + + if x5c == nil { + x5c = parsed + } else { + x5cis = append(x5cis, parsed) + } + } + + if attestationType == string(metadata.AttCA) { + if protoErr = tpmParseAIKAttCA(x5c, x5cis); protoErr != nil { + return ErrMetadata.WithDetails(protoErr.Details).WithInfo(protoErr.DevInfo) + } + } + + if x5c != nil && x5c.Subject.CommonName != x5c.Issuer.CommonName { + if !entry.MetadataStatement.AttestationTypes.HasBasicFull() { + return ErrMetadata.WithDetails(fmt.Sprintf("Failed to validate attestation statement signature during attestation validation for Authenticator Attestation GUID '%s'. Attestation was provided in the full format but the authenticator doesn't support the full attestation format.", aaguid)) + } + + if _, err = x5c.Verify(entry.MetadataStatement.Verifier(x5cis)); err != nil { + return ErrMetadata.WithDetails(fmt.Sprintf("Failed to validate attestation statement signature during attestation validation for Authenticator Attestation GUID '%s'. The attestation certificate could not be verified due to an error validating the trust chain agaisnt the Metadata Service.", aaguid)) + } + } + } + + return nil +} + +func loopOrdinalNumber(n int) string { + n++ + + if n > 9 && n < 20 { + return fmt.Sprintf("%dth", n) + } + + switch n % 10 { + case 1: + return fmt.Sprintf("%dst", n) + case 2: + return fmt.Sprintf("%dnd", n) + case 3: + return fmt.Sprintf("%drd", n) + default: + return fmt.Sprintf("%dth", n) + } } diff --git a/protocol/options_test.go b/protocol/options_test.go index e3fe3e76..456a59f8 100644 --- a/protocol/options_test.go +++ b/protocol/options_test.go @@ -27,7 +27,7 @@ func TestPublicKeyCredentialRequestOptions_GetAllowedCredentialIDs(t *testing.T) Timeout: 60, AllowedCredentials: []CredentialDescriptor{ { - Type: "public-key", CredentialID: []byte("1234"), Transport: []AuthenticatorTransport{"usb"}, + Type: PublicKeyCredentialType, CredentialID: []byte("1234"), Transport: []AuthenticatorTransport{"usb"}, }, }, RelyingPartyID: "test.org", diff --git a/webauthn/login.go b/webauthn/login.go index 36e86e88..bfa1e884 100644 --- a/webauthn/login.go +++ b/webauthn/login.go @@ -186,7 +186,7 @@ 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. +// 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 { @@ -349,7 +349,7 @@ func (webauthn *WebAuthn) validateLogin(user User, session SessionData, parsedRe var protoErr *protocol.Error - if _, protoErr = protocol.ValidateMetadata(context.Background(), aaguid, credential.AttestationType, webauthn.Config.MDS); protoErr != nil { + if protoErr = protocol.ValidateMetadata(context.Background(), webauthn.Config.MDS, aaguid, credential.AttestationType, nil); protoErr != nil { return nil, protocol.ErrBadRequest.WithDetails("Failed to validate credential record metadata").WithInfo(protoErr.DevInfo) } } From e582e2c8f51ad97a34c8ebf81dc9bf95052c4245 Mon Sep 17 00:00:00 2001 From: James Elliott Date: Sat, 25 Jan 2025 11:55:15 +1100 Subject: [PATCH 3/3] feat: refactor errors --- metadata/const.go | 6 ++-- metadata/metadata_test.go | 2 +- metadata/status.go | 6 ++-- metadata/types.go | 6 ++-- protocol/assertion.go | 12 +++---- protocol/attestation.go | 10 +++--- protocol/attestation_androidkey.go | 12 +++---- protocol/attestation_apple.go | 6 ++-- protocol/attestation_packed.go | 6 ++-- protocol/attestation_safetynet.go | 12 +++---- protocol/attestation_tpm.go | 4 +-- protocol/attestation_u2f.go | 2 +- protocol/authenticator.go | 2 +- protocol/client.go | 4 +-- protocol/credential.go | 4 +-- protocol/errors.go | 50 +++++++++++++++++++----------- protocol/metadata.go | 6 ++-- webauthn/login.go | 6 ++-- 18 files changed, 85 insertions(+), 71 deletions(-) diff --git a/metadata/const.go b/metadata/const.go index 97a5ad42..76e6a332 100644 --- a/metadata/const.go +++ b/metadata/const.go @@ -20,15 +20,15 @@ const ( ) var ( - errIntermediateCertRevoked = &MetadataError{ + errIntermediateCertRevoked = &Error{ Type: "intermediate_revoked", Details: "Intermediate certificate is on issuers revocation list", } - errLeafCertRevoked = &MetadataError{ + errLeafCertRevoked = &Error{ Type: "leaf_revoked", Details: "Leaf certificate is on issuers revocation list", } - errCRLUnavailable = &MetadataError{ + errCRLUnavailable = &Error{ Type: "crl_unavailable", Details: "Certificate revocation list is unavailable", } diff --git a/metadata/metadata_test.go b/metadata/metadata_test.go index b720fdf0..b0bfb805 100644 --- a/metadata/metadata_test.go +++ b/metadata/metadata_test.go @@ -82,7 +82,7 @@ func TestConformanceMetadataTOCParsing(t *testing.T) { var ( res *http.Response blob *PayloadJSON - me *MetadataError + me *Error ) for _, endpoint := range endpoints { diff --git a/metadata/status.go b/metadata/status.go index 158e37c8..f20d716f 100644 --- a/metadata/status.go +++ b/metadata/status.go @@ -44,17 +44,17 @@ func ValidateStatusReports(reports []StatusReport, desired, undesired []Authenti case len(present) == 0 && len(absent) == 0: return nil case len(present) != 0 && len(absent) == 0: - return &MetadataError{ + return &Error{ Type: "invalid_status", Details: fmt.Sprintf("The following undesired status reports were present: %s", strings.Join(present, ", ")), } case len(present) == 0 && len(absent) != 0: - return &MetadataError{ + return &Error{ Type: "invalid_status", Details: fmt.Sprintf("The following desired status reports were absent: %s", strings.Join(absent, ", ")), } default: - return &MetadataError{ + return &Error{ Type: "invalid_status", Details: fmt.Sprintf("The following undesired status reports were present: %s; the following desired status reports were absent: %s", strings.Join(present, ", "), strings.Join(absent, ", ")), } diff --git a/metadata/types.go b/metadata/types.go index 1562b637..dcc89136 100644 --- a/metadata/types.go +++ b/metadata/types.go @@ -299,7 +299,7 @@ const ( ALG_KEY_COSE PublicKeyAlgAndEncoding = "cose" ) -type MetadataError struct { +type Error struct { // Short name for the type of error that has occurred. Type string `json:"type"` @@ -310,8 +310,8 @@ type MetadataError struct { DevInfo string `json:"debug"` } -func (err *MetadataError) Error() string { - return err.Details +func (e *Error) Error() string { + return e.Details } // Clock is an interface used to implement clock functionality in various metadata areas. diff --git a/protocol/assertion.go b/protocol/assertion.go index b055e01b..f3caa918 100644 --- a/protocol/assertion.go +++ b/protocol/assertion.go @@ -69,7 +69,7 @@ func ParseCredentialRequestResponseBody(body io.Reader) (par *ParsedCredentialAs var car CredentialAssertionResponse if err = decodeBody(body, &car); err != nil { - return nil, ErrBadRequest.WithDetails("Parse error for Assertion").WithInfo(err.Error()) + return nil, ErrBadRequest.WithDetails("Parse error for Assertion").WithInfo(err.Error()).WithError(err) } return car.Parse() @@ -81,7 +81,7 @@ func ParseCredentialRequestResponseBytes(data []byte) (par *ParsedCredentialAsse var car CredentialAssertionResponse if err = decodeBytes(data, &car); err != nil { - return nil, ErrBadRequest.WithDetails("Parse error for Assertion").WithInfo(err.Error()) + return nil, ErrBadRequest.WithDetails("Parse error for Assertion").WithInfo(err.Error()).WithError(err) } return car.Parse() @@ -97,7 +97,7 @@ func (car CredentialAssertionResponse) Parse() (par *ParsedCredentialAssertionDa } if _, err = base64.RawURLEncoding.DecodeString(car.ID); err != nil { - return nil, ErrBadRequest.WithDetails("CredentialAssertionResponse with ID not base64url encoded") + return nil, ErrBadRequest.WithDetails("CredentialAssertionResponse with ID not base64url encoded").WithError(err) } if car.Type != string(PublicKeyCredentialType) { @@ -129,7 +129,7 @@ func (car CredentialAssertionResponse) Parse() (par *ParsedCredentialAssertionDa } if err = par.Response.AuthenticatorData.Unmarshal(car.AssertionResponse.AuthenticatorData); err != nil { - return nil, ErrParsingData.WithDetails("Error unmarshalling auth data") + return nil, ErrParsingData.WithDetails("Error unmarshalling auth data").WithError(err) } return par, nil @@ -189,12 +189,12 @@ func (p *ParsedCredentialAssertionData) Verify(storedChallenge string, relyingPa } if err != nil { - return ErrAssertionSignature.WithDetails(fmt.Sprintf("Error parsing the assertion public key: %+v", err)) + return ErrAssertionSignature.WithDetails(fmt.Sprintf("Error parsing the assertion public key: %+v", err)).WithError(err) } valid, err := webauthncose.VerifySignature(key, sigData, p.Response.Signature) if !valid || err != nil { - return ErrAssertionSignature.WithDetails(fmt.Sprintf("Error validating the assertion signature: %+v", err)) + return ErrAssertionSignature.WithDetails(fmt.Sprintf("Error validating the assertion signature: %+v", err)).WithError(err) } return nil diff --git a/protocol/attestation.go b/protocol/attestation.go index 83a004ad..144fd7ca 100644 --- a/protocol/attestation.go +++ b/protocol/attestation.go @@ -94,18 +94,18 @@ func (ccr *AuthenticatorAttestationResponse) Parse() (p *ParsedAttestationRespon p = &ParsedAttestationResponse{} if err = json.Unmarshal(ccr.ClientDataJSON, &p.CollectedClientData); err != nil { - return nil, ErrParsingData.WithInfo(err.Error()) + return nil, ErrParsingData.WithInfo(err.Error()).WithError(err) } if err = webauthncbor.Unmarshal(ccr.AttestationObject, &p.AttestationObject); err != nil { - return nil, ErrParsingData.WithInfo(err.Error()) + return nil, ErrParsingData.WithInfo(err.Error()).WithError(err) } // Step 8. Perform CBOR decoding on the attestationObject field of the AuthenticatorAttestationResponse // structure to obtain the attestation statement format fmt, the authenticator data authData, and // the attestation statement attStmt. if err = p.AttestationObject.AuthData.Unmarshal(p.AttestationObject.RawAuthData); err != nil { - return nil, fmt.Errorf("error decoding auth data: %v", err) + return nil, err } if !p.AttestationObject.AuthData.Flags.HasAttestedCredentialData() { @@ -176,7 +176,7 @@ func (a *AttestationObject) VerifyAttestation(clientDataHash []byte, mds metadat if len(a.AuthData.AttData.AAGUID) != 0 { if aaguid, err = uuid.FromBytes(a.AuthData.AttData.AAGUID); err != nil { - return ErrInvalidAttestation.WithInfo("Error occurred parsing AAGUID during attestation validation").WithDetails(err.Error()) + return ErrInvalidAttestation.WithInfo("Error occurred parsing AAGUID during attestation validation").WithDetails(err.Error()).WithError(err) } } @@ -187,7 +187,7 @@ func (a *AttestationObject) VerifyAttestation(clientDataHash []byte, mds metadat var protoErr *Error if protoErr = ValidateMetadata(context.Background(), mds, aaguid, attestationType, x5cs); protoErr != nil { - return ErrInvalidAttestation.WithInfo(fmt.Sprintf("Error occurred validating metadata during attestation validation: %+v", protoErr)).WithDetails(protoErr.DevInfo) + return ErrInvalidAttestation.WithInfo(fmt.Sprintf("Error occurred validating metadata during attestation validation: %+v", protoErr)).WithDetails(protoErr.DevInfo).WithError(protoErr) } return nil diff --git a/protocol/attestation_androidkey.go b/protocol/attestation_androidkey.go index 6e387b34..7bfff7ea 100644 --- a/protocol/attestation_androidkey.go +++ b/protocol/attestation_androidkey.go @@ -65,25 +65,25 @@ func verifyAndroidKeyFormat(att AttestationObject, clientDataHash []byte, _ meta attCert, err := x509.ParseCertificate(attCertBytes) if err != nil { - return "", nil, ErrAttestationFormat.WithDetails(fmt.Sprintf("Error parsing certificate from ASN.1 data: %+v", err)) + return "", nil, ErrAttestationFormat.WithDetails(fmt.Sprintf("Error parsing certificate from ASN.1 data: %+v", err)).WithError(err) } coseAlg := webauthncose.COSEAlgorithmIdentifier(alg) if err = attCert.CheckSignature(webauthncose.SigAlgFromCOSEAlg(coseAlg), signatureData, sig); err != nil { - return "", nil, ErrInvalidAttestation.WithDetails(fmt.Sprintf("Signature validation error: %+v\n", err)) + return "", nil, ErrInvalidAttestation.WithDetails(fmt.Sprintf("Signature validation error: %+v\n", err)).WithError(err) } // Verify that the public key in the first certificate in x5c matches the credentialPublicKey in the attestedCredentialData in authenticatorData. pubKey, err := webauthncose.ParsePublicKey(att.AuthData.AttData.CredentialPublicKey) if err != nil { - return "", nil, ErrInvalidAttestation.WithDetails(fmt.Sprintf("Error parsing public key: %+v\n", err)) + return "", nil, ErrInvalidAttestation.WithDetails(fmt.Sprintf("Error parsing public key: %+v\n", err)).WithError(err) } e := pubKey.(webauthncose.EC2PublicKeyData) valid, err = e.Verify(signatureData, sig) if err != nil || !valid { - return "", nil, ErrInvalidAttestation.WithDetails(fmt.Sprintf("Error parsing public key: %+v\n", err)) + return "", nil, ErrInvalidAttestation.WithDetails(fmt.Sprintf("Error parsing public key: %+v\n", err)).WithError(err) } // §8.4.3. Verify that the attestationChallenge field in the attestation certificate extension data is identical to clientDataHash. @@ -105,11 +105,11 @@ func verifyAndroidKeyFormat(att AttestationObject, clientDataHash []byte, _ meta decoded := keyDescription{} if _, err = asn1.Unmarshal(attExtBytes, &decoded); err != nil { - return "", nil, ErrAttestationFormat.WithDetails("Unable to parse Android key attestation certificate extensions") + return "", nil, ErrAttestationFormat.WithDetails("Unable to parse Android key attestation certificate extensions").WithError(err) } // Verify that the attestationChallenge field in the attestation certificate extension data is identical to clientDataHash. - if 0 != bytes.Compare(decoded.AttestationChallenge, clientDataHash) { + if bytes.Compare(decoded.AttestationChallenge, clientDataHash) != 0 { return "", nil, ErrAttestationFormat.WithDetails("Attestation challenge not equal to clientDataHash") } diff --git a/protocol/attestation_apple.go b/protocol/attestation_apple.go index a69232ab..e2b3e9ed 100644 --- a/protocol/attestation_apple.go +++ b/protocol/attestation_apple.go @@ -48,7 +48,7 @@ func verifyAppleFormat(att AttestationObject, clientDataHash []byte, _ metadata. credCert, err := x509.ParseCertificate(credCertBytes) if err != nil { - return "", nil, ErrAttestationFormat.WithDetails(fmt.Sprintf("Error parsing certificate from ASN.1 data: %+v", err)) + return "", nil, ErrAttestationFormat.WithDetails(fmt.Sprintf("Error parsing certificate from ASN.1 data: %+v", err)).WithError(err) } // Step 2. Concatenate authenticatorData and clientDataHash to form nonceToHash. @@ -73,7 +73,7 @@ func verifyAppleFormat(att AttestationObject, clientDataHash []byte, _ metadata. decoded := AppleAnonymousAttestation{} if _, err = asn1.Unmarshal(attExtBytes, &decoded); err != nil { - return "", nil, ErrAttestationFormat.WithDetails("Unable to parse apple attestation certificate extensions") + return "", nil, ErrAttestationFormat.WithDetails("Unable to parse apple attestation certificate extensions").WithError(err) } if !bytes.Equal(decoded.Nonce, nonce[:]) { @@ -84,7 +84,7 @@ func verifyAppleFormat(att AttestationObject, clientDataHash []byte, _ metadata. // TODO: Probably move this part to webauthncose.go pubKey, err := webauthncose.ParsePublicKey(att.AuthData.AttData.CredentialPublicKey) if err != nil { - return "", nil, ErrInvalidAttestation.WithDetails(fmt.Sprintf("Error parsing public key: %+v\n", err)) + return "", nil, ErrInvalidAttestation.WithDetails(fmt.Sprintf("Error parsing public key: %+v\n", err)).WithError(err) } credPK := pubKey.(webauthncose.EC2PublicKeyData) diff --git a/protocol/attestation_packed.go b/protocol/attestation_packed.go index ddd17ee2..8f299eb9 100644 --- a/protocol/attestation_packed.go +++ b/protocol/attestation_packed.go @@ -81,7 +81,7 @@ func handleBasicAttestation(signature, clientDataHash, authData, aaguid []byte, ct, err := x509.ParseCertificate(cb) if err != nil { - return "", x5c, ErrAttestationFormat.WithDetails(fmt.Sprintf("Error parsing certificate from ASN.1 data: %+v", err)) + return "", x5c, ErrAttestationFormat.WithDetails(fmt.Sprintf("Error parsing certificate from ASN.1 data: %+v", err)).WithError(err) } if ct.NotBefore.After(time.Now()) || ct.NotAfter.Before(time.Now()) { @@ -98,12 +98,12 @@ func handleBasicAttestation(signature, clientDataHash, authData, aaguid []byte, attCert, err := x509.ParseCertificate(attCertBytes) if err != nil { - return "", x5c, ErrAttestationFormat.WithDetails(fmt.Sprintf("Error parsing certificate from ASN.1 data: %+v", err)) + return "", x5c, ErrAttestationFormat.WithDetails(fmt.Sprintf("Error parsing certificate from ASN.1 data: %+v", err)).WithError(err) } coseAlg := webauthncose.COSEAlgorithmIdentifier(alg) if err = attCert.CheckSignature(webauthncose.SigAlgFromCOSEAlg(coseAlg), signatureData, signature); err != nil { - return "", x5c, ErrInvalidAttestation.WithDetails(fmt.Sprintf("Signature validation error: %+v\n", err)) + return "", x5c, ErrInvalidAttestation.WithDetails(fmt.Sprintf("Signature validation error: %+v\n", err)).WithError(err) } // Step 2.2 Verify that attestnCert meets the requirements in §8.2.1 Packed attestation statement certificate requirements. diff --git a/protocol/attestation_safetynet.go b/protocol/attestation_safetynet.go index 7fa49ca1..7355c97d 100644 --- a/protocol/attestation_safetynet.go +++ b/protocol/attestation_safetynet.go @@ -90,14 +90,14 @@ func verifySafetyNetFormat(att AttestationObject, clientDataHash []byte, mds met }) if err != nil { - return "", nil, ErrInvalidAttestation.WithDetails(fmt.Sprintf("Error finding cert issued to correct hostname: %+v", err)) + return "", nil, ErrInvalidAttestation.WithDetails(fmt.Sprintf("Error finding cert issued to correct hostname: %+v", err)).WithError(err) } // marshall the JWT payload into the safetynet response json var safetyNetResponse SafetyNetResponse if err = mapstructure.Decode(token.Claims, &safetyNetResponse); err != nil { - return "", nil, ErrAttestationFormat.WithDetails(fmt.Sprintf("Error parsing the SafetyNet response: %+v", err)) + return "", nil, ErrAttestationFormat.WithDetails(fmt.Sprintf("Error parsing the SafetyNet response: %+v", err)).WithError(err) } // §8.5.3 Verify that the nonce in the response is identical to the Base64 encoding of the SHA-256 hash of the concatenation @@ -106,7 +106,7 @@ func verifySafetyNetFormat(att AttestationObject, clientDataHash []byte, mds met nonceBytes, err := base64.StdEncoding.DecodeString(safetyNetResponse.Nonce) if !bytes.Equal(nonceBuffer[:], nonceBytes) || err != nil { - return "", nil, ErrInvalidAttestation.WithDetails("Invalid nonce for in SafetyNet response") + return "", nil, ErrInvalidAttestation.WithDetails("Invalid nonce for in SafetyNet response").WithError(err) } // §8.5.4 Let attestationCert be the attestation certificate (https://www.w3.org/TR/webauthn/#attestation-certificate) @@ -115,18 +115,18 @@ func verifySafetyNetFormat(att AttestationObject, clientDataHash []byte, mds met n, err := base64.StdEncoding.Decode(l, []byte(certChain[0].(string))) if err != nil { - return "", nil, ErrInvalidAttestation.WithDetails(fmt.Sprintf("Error finding cert issued to correct hostname: %+v", err)) + return "", nil, ErrInvalidAttestation.WithDetails(fmt.Sprintf("Error finding cert issued to correct hostname: %+v", err)).WithError(err) } attestationCert, err := x509.ParseCertificate(l[:n]) if err != nil { - return "", nil, ErrInvalidAttestation.WithDetails(fmt.Sprintf("Error finding cert issued to correct hostname: %+v", err)) + return "", nil, ErrInvalidAttestation.WithDetails(fmt.Sprintf("Error finding cert issued to correct hostname: %+v", err)).WithError(err) } // §8.5.5 Verify that attestationCert is issued to the hostname "attest.android.com" err = attestationCert.VerifyHostname("attest.android.com") if err != nil { - return "", nil, ErrInvalidAttestation.WithDetails(fmt.Sprintf("Error finding cert issued to correct hostname: %+v", err)) + return "", nil, ErrInvalidAttestation.WithDetails(fmt.Sprintf("Error finding cert issued to correct hostname: %+v", err)).WithError(err) } // §8.5.6 Verify that the ctsProfileMatch attribute in the payload of response is true. diff --git a/protocol/attestation_tpm.go b/protocol/attestation_tpm.go index 881b8aa1..fb71c7a5 100644 --- a/protocol/attestation_tpm.go +++ b/protocol/attestation_tpm.go @@ -72,7 +72,7 @@ func verifyTPMFormat(att AttestationObject, clientDataHash []byte, _ metadata.Pr // is identical to the credentialPublicKey in the attestedCredentialData in authenticatorData. pubArea, err := tpm2.DecodePublic(pubAreaBytes) if err != nil { - return "", nil, ErrAttestationFormat.WithDetails("Unable to decode TPMT_PUBLIC in attestation statement") + return "", nil, ErrAttestationFormat.WithDetails("Unable to decode TPMT_PUBLIC in attestation statement").WithError(err) } key, err := webauthncose.ParsePublicKey(att.AuthData.AttData.CredentialPublicKey) @@ -394,7 +394,7 @@ func tpmParseSANExtension(attestation *x509.Certificate) (protoErr *Error) { for _, ext := range attestation.Extensions { if ext.Id.Equal(oidExtensionSubjectAltName) { if manufacturer, model, version, err = parseSANExtension(ext.Value); err != nil { - return ErrInvalidAttestation.WithDetails("Authenticator with invalid Authenticator Identity Key SAN data encountered during attestation validation.").WithInfo(fmt.Sprintf("Error occurred parsing SAN extension: %s", err.Error())) + return ErrInvalidAttestation.WithDetails("Authenticator with invalid Authenticator Identity Key SAN data encountered during attestation validation.").WithInfo(fmt.Sprintf("Error occurred parsing SAN extension: %s", err.Error())).WithError(err) } } } diff --git a/protocol/attestation_u2f.go b/protocol/attestation_u2f.go index 211aab78..3b858f5f 100644 --- a/protocol/attestation_u2f.go +++ b/protocol/attestation_u2f.go @@ -76,7 +76,7 @@ func verifyU2FFormat(att AttestationObject, clientDataHash []byte, _ metadata.Pr attCert, err := x509.ParseCertificate(asn1Bytes) if err != nil { - return "", nil, ErrAttestationFormat.WithDetails("Error parsing certificate from ASN.1 data into certificate") + return "", nil, ErrAttestationFormat.WithDetails("Error parsing certificate from ASN.1 data into certificate").WithError(err) } // Step 2.3 diff --git a/protocol/authenticator.go b/protocol/authenticator.go index 82bd9104..4cd636a2 100644 --- a/protocol/authenticator.go +++ b/protocol/authenticator.go @@ -349,7 +349,7 @@ func (a *AuthenticatorData) unmarshalAttestedData(rawAuthData []byte) (err error a.AttData.CredentialPublicKey, err = unmarshalCredentialPublicKey(rawAuthData[55+idLength:]) if err != nil { - return ErrBadRequest.WithDetails(fmt.Sprintf("Could not unmarshal Credential Public Key: %v", err)) + return ErrBadRequest.WithDetails(fmt.Sprintf("Could not unmarshal Credential Public Key: %v", err)).WithError(err) } return nil diff --git a/protocol/client.go b/protocol/client.go index 7aa0d28d..e5c21672 100644 --- a/protocol/client.go +++ b/protocol/client.go @@ -111,7 +111,7 @@ func (c *CollectedClientData) Verify(storedChallenge string, ceremony CeremonyTy var fqOrigin string if fqOrigin, err = FullyQualifiedOrigin(c.Origin); err != nil { - return ErrParsingData.WithDetails("Error decoding clientData origin as URL") + return ErrParsingData.WithDetails("Error decoding clientData origin as URL").WithError(err) } found := false @@ -146,7 +146,7 @@ func (c *CollectedClientData) Verify(storedChallenge string, ceremony CeremonyTy ) if fqTopOrigin, err = FullyQualifiedOrigin(c.TopOrigin); err != nil { - return ErrParsingData.WithDetails("Error decoding clientData topOrigin as URL") + return ErrParsingData.WithDetails("Error decoding clientData topOrigin as URL").WithError(err) } switch rpTopOriginsVerify { diff --git a/protocol/credential.go b/protocol/credential.go index a36b2068..7b18a1d2 100644 --- a/protocol/credential.go +++ b/protocol/credential.go @@ -79,7 +79,7 @@ func ParseCredentialCreationResponseBody(body io.Reader) (pcc *ParsedCredentialC var ccr CredentialCreationResponse if err = decodeBody(body, &ccr); err != nil { - return nil, ErrBadRequest.WithDetails("Parse error for Registration").WithInfo(err.Error()) + return nil, ErrBadRequest.WithDetails("Parse error for Registration").WithInfo(err.Error()).WithError(err) } return ccr.Parse() @@ -91,7 +91,7 @@ func ParseCredentialCreationResponseBytes(data []byte) (pcc *ParsedCredentialCre var ccr CredentialCreationResponse if err = decodeBytes(data, &ccr); err != nil { - return nil, ErrBadRequest.WithDetails("Parse error for Registration").WithInfo(err.Error()) + return nil, ErrBadRequest.WithDetails("Parse error for Registration").WithInfo(err.Error()).WithError(err) } return ccr.Parse() diff --git a/protocol/errors.go b/protocol/errors.go index ceda6f79..d09b80eb 100644 --- a/protocol/errors.go +++ b/protocol/errors.go @@ -9,6 +9,38 @@ type Error struct { // Information to help debug the error. DevInfo string `json:"debug"` + + // Inner error. + Err error `json:"-"` +} + +func (e *Error) Error() string { + return e.Details +} + +func (e *Error) Unwrap() error { + return e.Err +} + +func (e *Error) WithDetails(details string) *Error { + err := *e + err.Details = details + + return &err +} + +func (e *Error) WithInfo(info string) *Error { + err := *e + err.DevInfo = info + + return &err +} + +func (e *Error) WithError(err error) *Error { + errCopy := *e + errCopy.Err = err + + return &errCopy } var ( @@ -73,21 +105,3 @@ var ( Details: "This field is not yet supported by this library", } ) - -func (e *Error) Error() string { - return e.Details -} - -func (e *Error) WithDetails(details string) *Error { - err := *e - err.Details = details - - return &err -} - -func (e *Error) WithInfo(info string) *Error { - err := *e - err.DevInfo = info - - return &err -} diff --git a/protocol/metadata.go b/protocol/metadata.go index 670ff871..da48cb89 100644 --- a/protocol/metadata.go +++ b/protocol/metadata.go @@ -76,7 +76,7 @@ func ValidateMetadata(ctx context.Context, mds metadata.Provider, aaguid uuid.UU } if parsed, err = x509.ParseCertificate(raw); err != nil { - return ErrMetadata.WithDetails(fmt.Sprintf("Failed to parse attestation certificate from x5c during attestation validation for Authenticator Attestation GUID '%s'.", aaguid)).WithInfo(fmt.Sprintf("Error returned from x509.ParseCertificate: %+v", err)) + return ErrMetadata.WithDetails(fmt.Sprintf("Failed to parse attestation certificate from x5c during attestation validation for Authenticator Attestation GUID '%s'.", aaguid)).WithInfo(fmt.Sprintf("Error returned from x509.ParseCertificate: %+v", err)).WithError(err) } if x5c == nil { @@ -88,7 +88,7 @@ func ValidateMetadata(ctx context.Context, mds metadata.Provider, aaguid uuid.UU if attestationType == string(metadata.AttCA) { if protoErr = tpmParseAIKAttCA(x5c, x5cis); protoErr != nil { - return ErrMetadata.WithDetails(protoErr.Details).WithInfo(protoErr.DevInfo) + return ErrMetadata.WithDetails(protoErr.Details).WithInfo(protoErr.DevInfo).WithError(protoErr) } } @@ -98,7 +98,7 @@ func ValidateMetadata(ctx context.Context, mds metadata.Provider, aaguid uuid.UU } if _, err = x5c.Verify(entry.MetadataStatement.Verifier(x5cis)); err != nil { - return ErrMetadata.WithDetails(fmt.Sprintf("Failed to validate attestation statement signature during attestation validation for Authenticator Attestation GUID '%s'. The attestation certificate could not be verified due to an error validating the trust chain agaisnt the Metadata Service.", aaguid)) + return ErrMetadata.WithDetails(fmt.Sprintf("Failed to validate attestation statement signature during attestation validation for Authenticator Attestation GUID '%s'. The attestation certificate could not be verified due to an error validating the trust chain agaisnt the Metadata Service.", aaguid)).WithError(err) } } } diff --git a/webauthn/login.go b/webauthn/login.go index bfa1e884..9c73b343 100644 --- a/webauthn/login.go +++ b/webauthn/login.go @@ -250,7 +250,7 @@ func (webauthn *WebAuthn) ValidatePasskeyLogin(handler DiscoverableUserHandler, } if user, err = handler(parsedResponse.RawID, parsedResponse.Response.UserHandle); err != nil { - return nil, nil, protocol.ErrBadRequest.WithDetails(fmt.Sprintf("Failed to lookup Client-side Discoverable Credential: %s", err)) + return nil, nil, protocol.ErrBadRequest.WithDetails(fmt.Sprintf("Failed to lookup Client-side Discoverable Credential: %s", err)).WithError(err) } if credential, err = webauthn.validateLogin(user, session, parsedResponse); err != nil { @@ -344,13 +344,13 @@ func (webauthn *WebAuthn) validateLogin(user User, session SessionData, parsedRe var aaguid uuid.UUID if aaguid, err = uuid.FromBytes(credential.Authenticator.AAGUID); err != nil { - return nil, protocol.ErrBadRequest.WithDetails("Failed to decode AAGUID").WithInfo(fmt.Sprintf("Error occurred decoding AAGUID from the credential record: %s", err)) + return nil, protocol.ErrBadRequest.WithDetails("Failed to decode AAGUID").WithInfo(fmt.Sprintf("Error occurred decoding AAGUID from the credential record: %s", err)).WithError(err) } var protoErr *protocol.Error if protoErr = protocol.ValidateMetadata(context.Background(), webauthn.Config.MDS, aaguid, credential.AttestationType, nil); protoErr != nil { - return nil, protocol.ErrBadRequest.WithDetails("Failed to validate credential record metadata").WithInfo(protoErr.DevInfo) + return nil, protocol.ErrBadRequest.WithDetails("Failed to validate credential record metadata").WithInfo(protoErr.DevInfo).WithError(protoErr) } }