Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(protocol): enhance errors #341

Merged
merged 3 commits into from
Jan 26, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
6 changes: 3 additions & 3 deletions metadata/const.go
Original file line number Diff line number Diff line change
Expand Up @@ -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",
}
Expand Down
2 changes: 1 addition & 1 deletion metadata/metadata_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ func TestConformanceMetadataTOCParsing(t *testing.T) {
var (
res *http.Response
blob *PayloadJSON
me *MetadataError
me *Error
)

for _, endpoint := range endpoints {
Expand Down
6 changes: 3 additions & 3 deletions metadata/status.go
Original file line number Diff line number Diff line change
Expand Up @@ -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, ", ")),
}
Expand Down
6 changes: 3 additions & 3 deletions metadata/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"`

Expand All @@ -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.
Expand Down
32 changes: 16 additions & 16 deletions protocol/assertion.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand All @@ -67,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()
Expand All @@ -79,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()
Expand All @@ -95,20 +97,18 @@ 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 != "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{
Expand All @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand Down
4 changes: 2 additions & 2 deletions protocol/assertion_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{
Expand Down Expand Up @@ -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,
Expand Down
117 changes: 10 additions & 107 deletions protocol/attestation.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ package protocol
import (
"context"
"crypto/sha256"
"crypto/x509"
"encoding/json"
"fmt"

Expand Down Expand Up @@ -95,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() {
Expand Down Expand Up @@ -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")
Expand All @@ -173,118 +172,22 @@ func (a *AttestationObject) VerifyAttestation(clientDataHash []byte, mds metadat

var (
aaguid uuid.UUID
entry *metadata.Entry
)

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)
}
}

if mds == nil {
return nil
}

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 == 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()))
}
var protoErr *Error

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
}

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).WithError(protoErr)
}

return nil
Expand Down
Loading
Loading