From 99d717de8418795c09d5e0b346180584b74d064b Mon Sep 17 00:00:00 2001 From: James Elliott Date: Wed, 27 Nov 2024 20:09:02 +1100 Subject: [PATCH] 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) } }