Skip to content

Commit

Permalink
refactor: apply several linting fixes (#10)
Browse files Browse the repository at this point in the history
Handle errs, simplify logic, remove checks that are not necessary.
  • Loading branch information
james-d-elliott committed Mar 1, 2022
1 parent df2e592 commit 6f7565d
Show file tree
Hide file tree
Showing 8 changed files with 60 additions and 18 deletions.
2 changes: 1 addition & 1 deletion protocol/attestation_androidkey.go
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ func verifyAndroidKeyFormat(att AttestationObject, clientDataHash []byte) (strin
}
e := pubKey.(webauthncose.EC2PublicKeyData)
valid, err = e.Verify(signatureData, sig)
if err != nil || valid != true {
if err != nil || !valid {
return androidAttestationKey, nil, ErrInvalidAttestation.WithDetails(fmt.Sprintf("Error parsing public key: %+v\n", err))
}
// §8.4.3. Verify that the attestationChallenge field in the attestation certificate extension data is identical to clientDataHash.
Expand Down
18 changes: 13 additions & 5 deletions protocol/attestation_packed.go
Original file line number Diff line number Diff line change
Expand Up @@ -174,14 +174,22 @@ func handleBasicAttestation(signature, clientDataHash, authData, aaguid []byte,
// AAGUID MUST be wrapped in two OCTET STRINGS to be valid.
if len(foundAAGUID) > 0 {
unMarshalledAAGUID := []byte{}
asn1.Unmarshal(foundAAGUID, &unMarshalledAAGUID)
_, err = asn1.Unmarshal(foundAAGUID, &unMarshalledAAGUID)
if err != nil {
return attestationType, x5c, ErrInvalidAttestation.WithDetails(fmt.Sprintf("AAGUID could not be unmarshalled: %v", err))
}

if !bytes.Equal(aaguid, unMarshalledAAGUID) {
return attestationType, x5c, ErrInvalidAttestation.WithDetails("Certificate AAGUID does not match Auth Data certificate")
}
}
uuid, err := uuid.FromBytes(aaguid)

if meta, ok := metadata.Metadata[uuid]; ok {
parsedAAGUID, err := uuid.FromBytes(aaguid)
if err != nil {
return attestationType, x5c, ErrInvalidAttestation.WithDetails(fmt.Sprintf("AAGUID could not be parsed: %v", err))
}

if meta, ok := metadata.Metadata[parsedAAGUID]; ok {
for _, s := range meta.StatusReports {
if metadata.IsUndesiredAuthenticatorStatus(metadata.AuthenticatorStatus(s.Status)) {
return attestationType, x5c, ErrInvalidAttestation.WithDetails("Authenticator with undesirable status encountered")
Expand All @@ -191,12 +199,12 @@ func handleBasicAttestation(signature, clientDataHash, authData, aaguid []byte,
if attCert.Subject.CommonName != attCert.Issuer.CommonName {
var hasBasicFull = false
for _, a := range meta.MetadataStatement.AttestationTypes {
if metadata.AuthenticatorAttestationType(a) == metadata.AuthenticatorAttestationType(metadata.BasicFull) {
if metadata.AuthenticatorAttestationType(a) == metadata.BasicFull {
hasBasicFull = true
}
}
if !hasBasicFull {
return attestationType, x5c, ErrInvalidAttestation.WithDetails("Attestation with full attestation from authentictor that does not support full attestation")
return attestationType, x5c, ErrInvalidAttestation.WithDetails("Attestation with full attestation from authenticator that does not support full attestation")
}
}
} else {
Expand Down
8 changes: 8 additions & 0 deletions protocol/attestation_safetynet.go
Original file line number Diff line number Diff line change
Expand Up @@ -73,11 +73,19 @@ func verifySafetyNetFormat(att AttestationObject, clientDataHash []byte) (string

token, err := jwt.Parse(string(response), func(token *jwt.Token) (interface{}, error) {
chain := token.Header["x5c"].([]interface{})

o := make([]byte, base64.StdEncoding.DecodedLen(len(chain[0].(string))))

n, err := base64.StdEncoding.Decode(o, []byte(chain[0].(string)))
if err != nil {
return nil, err
}

cert, err := x509.ParseCertificate(o[:n])

return cert.PublicKey, err
})

if err != nil {
return safetyNetAttestationKey, nil, ErrInvalidAttestation.WithDetails(fmt.Sprintf("Error finding cert issued to correct hostname: %+v", err))
}
Expand Down
2 changes: 1 addition & 1 deletion protocol/attestation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ func attestationTestUnpackResponse(t *testing.T, response string) ParsedCredenti
}

func TestPackedAttestationVerification(t *testing.T) {
t.Run(fmt.Sprintf("Testing Self Packed"), func(t *testing.T) {
t.Run("Testing Self Packed", func(t *testing.T) {
pcc := attestationTestUnpackResponse(t, testAttestationResponses[0])

// Test Packed Verification
Expand Down
15 changes: 12 additions & 3 deletions protocol/attestation_tpm.go
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,10 @@ func verifyTPMFormat(att AttestationObject, clientDataHash []byte) (string, []in
}

key, err := webauthncose.ParsePublicKey(att.AuthData.AttData.CredentialPublicKey)
if err != nil {
return tpmAttestationKey, nil, ErrAttestationFormat.WithDetails(fmt.Sprintf("Public Key could not be parsed in the COSE format: %v", err))
}

switch k := key.(type) {
case webauthncose.EC2PublicKeyData:
if pubArea.ECCParameters.CurveID != googletpm.EllipticCurve(k.Curve) ||
Expand All @@ -101,6 +105,10 @@ func verifyTPMFormat(att AttestationObject, clientDataHash []byte) (string, []in

// Validate that certInfo is valid:
certInfo, err := googletpm.DecodeAttestationData(certInfoBytes)
if err != nil {
return tpmAttestationKey, nil, ErrAttestationFormat.WithDetails(fmt.Sprintf("Could not decode Attestation data, %v", err))
}

// 1/4 Verify that magic is set to TPM_GENERATED_VALUE.
if certInfo.Magic != 0xff544347 {
return tpmAttestationKey, nil, ErrAttestationFormat.WithDetails("Magic is not set to TPM_GENERATED_VALUE")
Expand Down Expand Up @@ -174,7 +182,7 @@ func verifyTPMFormat(att AttestationObject, clientDataHash []byte) (string, []in
return tpmAttestationKey, nil, ErrAttestationFormat.WithDetails("Invalid SAN data in AIK certificate")
}

if false == isValidTPMManufacturer(manufacturer) {
if !isValidTPMManufacturer(manufacturer) {
return tpmAttestationKey, nil, ErrAttestationFormat.WithDetails("Invalid TPM manufacturer")
}

Expand All @@ -190,7 +198,7 @@ func verifyTPMFormat(att AttestationObject, clientDataHash []byte) (string, []in
ekuValid = true
}
}
if false == ekuValid {
if !ekuValid {
return tpmAttestationKey, nil, ErrAttestationFormat.WithDetails("AIK certificate missing EKU")
}

Expand All @@ -209,7 +217,8 @@ func verifyTPMFormat(att AttestationObject, clientDataHash []byte) (string, []in
}
}
}
if constraints.IsCA != false {

if constraints.IsCA {
return tpmAttestationKey, nil, ErrAttestationFormat.WithDetails("AIK certificate basic constraints missing or CA is true")
}
// 6/6 An Authority Information Access (AIA) extension with entry id-ad-ocsp and a CRL Distribution Point
Expand Down
4 changes: 1 addition & 3 deletions protocol/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -65,9 +65,7 @@ func (c *CollectedClientData) Verify(storedChallenge string, ceremony CeremonyTy

// Assertion Step 7. Verify that the value of C.type is the string webauthn.get.
if c.Type != ceremony {
err := ErrVerification.WithDetails("Error validating ceremony type")
err.WithInfo(fmt.Sprintf("Expected Value: %s\n Received: %s\n", ceremony, c.Type))
return err
return ErrVerification.WithDetails("Error validating ceremony type").WithInfo(fmt.Sprintf("Expected Value: %s\n Received: %s\n", ceremony, c.Type))
}

// Registration Step 4. Verify that the value of C.challenge matches the challenge
Expand Down
25 changes: 22 additions & 3 deletions protocol/webauthncose/webauthncose.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (
"crypto/x509"
"encoding/asn1"
"encoding/pem"
"fmt"
"hash"
"math/big"

Expand Down Expand Up @@ -163,18 +164,36 @@ func ParsePublicKey(keyBytes []byte) (interface{}, error) {
switch COSEKeyType(pk.KeyType) {
case OctetKey:
var o OKPPublicKeyData
webauthncbor.Unmarshal(keyBytes, &o)

err := webauthncbor.Unmarshal(keyBytes, &o)
if err != nil {
return nil, ErrUnsupportedKey.WithDetails(fmt.Sprintf("Could not unmarshall Public Key data: %v", err))
}

o.PublicKeyData = pk

return o, nil
case EllipticKey:
var e EC2PublicKeyData
webauthncbor.Unmarshal(keyBytes, &e)

err := webauthncbor.Unmarshal(keyBytes, &e)
if err != nil {
return nil, ErrUnsupportedKey.WithDetails(fmt.Sprintf("Could not unmarshall Public Key data: %v", err))
}

e.PublicKeyData = pk

return e, nil
case RSAKey:
var r RSAPublicKeyData
webauthncbor.Unmarshal(keyBytes, &r)

err := webauthncbor.Unmarshal(keyBytes, &r)
if err != nil {
return nil, ErrUnsupportedKey.WithDetails(fmt.Sprintf("Could not unmarshall key: %v", err))
}

r.PublicKeyData = pk

return r, nil
default:
return nil, ErrUnsupportedKey
Expand Down
4 changes: 2 additions & 2 deletions webauthn/login.go
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ func (webauthn *WebAuthn) BeginLogin(user User, opts ...LoginOption) (*protocol.
Extensions: requestOptions.Extensions,
}

response := protocol.CredentialAssertion{requestOptions}
response := protocol.CredentialAssertion{Response: requestOptions}

return &response, &newSessionData, nil
}
Expand Down Expand Up @@ -143,7 +143,7 @@ func (webauthn *WebAuthn) ValidateLogin(user User, session SessionData, parsedRe
// This is in part handled by our Step 1

userHandle := parsedResponse.Response.UserHandle
if userHandle != nil && len(userHandle) > 0 {
if len(userHandle) > 0 {
if !bytes.Equal(userHandle, user.WebAuthnID()) {
return nil, protocol.ErrBadRequest.WithDetails("userHandle and User ID do not match")
}
Expand Down

0 comments on commit 6f7565d

Please sign in to comment.