From 065a6b86bbaf46fa94c4ea3534187485353394a2 Mon Sep 17 00:00:00 2001 From: James Elliott Date: Sun, 17 Nov 2024 16:35:18 +1100 Subject: [PATCH] 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) } }