refactor: handle errors and linting fixes (#10)

This ensures several errors that are not checked are properly checked, simplifies some logic, and removes several ineffectual checks.

protocol/attestation_androidkey.go

@@ -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.

protocol/attestation_packed.go

@@ -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")
@@ -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 {

protocol/attestation_safetynet.go

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

protocol/attestation_test.go

@@ -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

protocol/attestation_tpm.go

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

protocol/attestation_u2f.go

@@ -5,6 +5,7 @@ import (
  "crypto/ecdsa"
  "crypto/elliptic"
  "crypto/x509"
+ "fmt"
 
  "github.com/go-webauthn/webauthn/protocol/webauthncbor"
  "github.com/go-webauthn/webauthn/protocol/webauthncose"
@@ -25,7 +26,11 @@ func verifyU2FFormat(att AttestationObject, clientDataHash []byte) (string, []in
  // Signing procedure step - If the credential public key of the given credential is not of
  // algorithm -7 ("ES256"), stop and return an error.
  key := webauthncose.EC2PublicKeyData{}
- webauthncbor.Unmarshal(att.AuthData.AttData.CredentialPublicKey, &key)
+
+ err := webauthncbor.Unmarshal(att.AuthData.AttData.CredentialPublicKey, &key)
+ if err != nil {
+ return u2fAttestationKey, nil, ErrAttestationFormat.WithDetails(fmt.Sprintf("Could not unmarshal Credential Public Key: %v", err))
+ }
 
  if webauthncose.COSEAlgorithmIdentifier(key.PublicKeyData.Algorithm) != webauthncose.AlgES256 {
  return u2fAttestationKey, nil, ErrUnsupportedAlgorithm.WithDetails("Non-ES256 Public Key algorithm used")

protocol/authenticator.go

@@ -195,28 +195,45 @@ func (a *AuthenticatorData) Unmarshal(rawAuthData []byte) error {
 }
 
 // If Attestation Data is present, unmarshall that into the appropriate public key structure
-func (a *AuthenticatorData) unmarshalAttestedData(rawAuthData []byte) error {
+func (a *AuthenticatorData) unmarshalAttestedData(rawAuthData []byte) (err error) {
  a.AttData.AAGUID = rawAuthData[37:53]
+
  idLength := binary.BigEndian.Uint16(rawAuthData[53:55])
  if len(rawAuthData) < int(55+idLength) {
  return ErrBadRequest.WithDetails("Authenticator attestation data length too short")
  }
+
  a.AttData.CredentialID = rawAuthData[55 : 55+idLength]
- a.AttData.CredentialPublicKey = unmarshalCredentialPublicKey(rawAuthData[55+idLength:])
+
+ 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 nil
 }
 
 // Unmarshall the credential's Public Key into CBOR encoding
-func unmarshalCredentialPublicKey(keyBytes []byte) []byte {
+func unmarshalCredentialPublicKey(keyBytes []byte) ([]byte, error) {
  var m interface{}
- webauthncbor.Unmarshal(keyBytes, &m)
- rawBytes, _ := webauthncbor.Marshal(m)
- return rawBytes
+
+ err := webauthncbor.Unmarshal(keyBytes, &m)
+ if err != nil {
+ return nil, err
+ }
+
+ rawBytes, err := webauthncbor.Marshal(m)
+ if err != nil {
+ return nil, err
+ }
+
+ return rawBytes, nil
 }
 
 // ResidentKeyRequired - Require that the key be private key resident to the client device
 func ResidentKeyRequired() *bool {
  required := true
+
  return &required
 }
 

protocol/authenticator_test.go

@@ -220,7 +220,11 @@ func Test_unmarshalCredentialPublicKey(t *testing.T) {
  }
  for _, tt := range tests {
  t.Run(tt.name, func(t *testing.T) {
- if got := unmarshalCredentialPublicKey(tt.args.keyBytes); !reflect.DeepEqual(got, tt.want) {
+ got, err := unmarshalCredentialPublicKey(tt.args.keyBytes)
+
+ if err != nil {
+ t.Errorf("unmarshalCredentialPublicKey() returned err %v", err)
+ } else if !reflect.DeepEqual(got, tt.want) {
  t.Errorf("unmarshalCredentialPublicKey() = %v, want %v", got, tt.want)
  }
  })

protocol/client.go

@@ -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

protocol/webauthncose/webauthncose.go

@@ -8,6 +8,7 @@ import (
  "crypto/x509"
  "encoding/asn1"
  "encoding/pem"
+ "fmt"
  "hash"
  "math/big"
 
@@ -159,22 +160,45 @@ func HasherFromCOSEAlg(coseAlg COSEAlgorithmIdentifier) func() hash.Hash {
 // Figure out what kind of COSE material was provided and create the data for the new key
 func ParsePublicKey(keyBytes []byte) (interface{}, error) {
  pk := PublicKeyData{}
- webauthncbor.Unmarshal(keyBytes, &pk)
+
+ err := webauthncbor.Unmarshal(keyBytes, &pk)
+ if err != nil {
+ return nil, ErrUnsupportedKey.WithDetails(fmt.Sprintf("Could not unmarshall Public Key data: %v", err))
+ }
+
  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 OK 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 EC2 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 RSA Public Key data: %v", err))
+ }
+
  r.PublicKeyData = pk
+
  return r, nil
  default:
  return nil, ErrUnsupportedKey

webauthn/login.go

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