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

Minor fixes #160

Merged
merged 1 commit into from Jan 31, 2023
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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
3 changes: 2 additions & 1 deletion src/Crypto/WebAuthn/AttestationStatementFormat/AndroidKey.hs
Expand Up @@ -264,7 +264,8 @@ instance M.AttestationStatementFormat Format where
(CBOR.TString "alg", CBOR.TInt $ Cose.fromCoseSignAlg $ Cose.signAlg pubKeyAndAlg),
( CBOR.TString "x5c",
CBOR.TList $
map (CBOR.TBytes . X509.encodeSignedObject) $ toList x5c
map (CBOR.TBytes . X509.encodeSignedObject) $
toList x5c
)
]

Expand Down
Expand Up @@ -94,7 +94,7 @@ data Response = Response
}
deriving (Eq, Show, Generic, Aeson.FromJSON, Aeson.ToJSON)

-- | Milliseconds represented as an 'Integer', used for 'timestampMs'
-- | Milliseconds represented as an 'Integer', used for @timestampMs@
newtype Milliseconds = Milliseconds Integer
deriving (Eq, Show)
deriving newtype (Aeson.FromJSON, Aeson.ToJSON)
Expand Down Expand Up @@ -180,7 +180,8 @@ instance M.AttestationStatementFormat Format where
(Just (TString ver), Just (TBytes responseRaw)) -> do
jws <-
first (("Failed to decode compact JWT response blob: " <>) . Text.pack . show) $
runExcept @JOSE.Error $ JOSE.decodeCompact (LBS.fromStrict responseRaw)
runExcept @JOSE.Error $
JOSE.decodeCompact (LBS.fromStrict responseRaw)
response <-
first (("Failed to verify/decode JWT payload: " <>) . Text.pack . show) $
JOSE.verifyJWSWithPayload
Expand Down Expand Up @@ -256,7 +257,8 @@ instance M.AttestationStatementFormat Format where
(True, False) -> BasicIntegrity
(False, False) -> NoIntegrity
unless (integrity >= requiredIntegrity) $
failure $ IntegrityCheckFailed integrity
failure $
IntegrityCheckFailed integrity

-- 5. If successful, return implementation-specific values representing attestation type Basic and attestation trust
-- path x5c.
Expand Down
3 changes: 2 additions & 1 deletion src/Crypto/WebAuthn/AttestationStatementFormat/FidoU2F.hs
Expand Up @@ -136,7 +136,8 @@ instance M.AttestationStatementFormat Format where
Just (xb, yb) -> do
-- We decode the x and y values in an earlier stage of the process. In order to construct the publicKeyU2F, we have to reencode the value.
unless (BS.length xb == 32 && BS.length yb == 32) $
failure $ CoordinateSizeInvalid (BS.length xb) (BS.length yb)
failure $
CoordinateSizeInvalid (BS.length xb) (BS.length yb)
-- 4.c Let publicKeyU2F be the concatenation 0x04 || x || y.
let publicKeyU2F = BS.singleton 0x04 <> xb <> yb

Expand Down
2 changes: 1 addition & 1 deletion src/Crypto/WebAuthn/Cose/Internal/Verify.hs
Expand Up @@ -116,7 +116,7 @@ verify
public_q = ECC.Point ecdsaX ecdsaY

-- This check is already done in checkPublicKey
--unless (ECC.isPointValid public_curve public_q) $
-- unless (ECC.isPointValid public_curve public_q) $
-- Left $ "ECDSA point is not valid for curve " <> Text.pack (show curveName) <> ": " <> Text.pack (show public_q)
let key = ECDSA.PublicKey {..}

Expand Down
28 changes: 14 additions & 14 deletions src/Crypto/WebAuthn/Cose/PublicKey.hs
Expand Up @@ -131,26 +131,26 @@ checkPublicKey :: UncheckedPublicKey -> Either Text PublicKey
checkPublicKey key@PublicKeyEdDSA {..}
| actualSize == expectedSize = Right $ CheckedPublicKey key
| otherwise =
Left $
"EdDSA public key for curve "
<> Text.pack (show eddsaCurve)
<> " didn't have the expected size of "
<> Text.pack (show expectedSize)
<> " bytes, it has "
<> Text.pack (show actualSize)
<> " bytes instead: "
<> Text.decodeUtf8 (Base16.encode eddsaX)
Left $
"EdDSA public key for curve "
<> Text.pack (show eddsaCurve)
<> " didn't have the expected size of "
<> Text.pack (show expectedSize)
<> " bytes, it has "
<> Text.pack (show actualSize)
<> " bytes instead: "
<> Text.decodeUtf8 (Base16.encode eddsaX)
where
actualSize = BS.length eddsaX
expectedSize = coordinateSizeEdDSA eddsaCurve
checkPublicKey key@PublicKeyECDSA {..}
| ECC.isPointValid curve point = Right $ CheckedPublicKey key
| otherwise =
Left $
"ECDSA public key point is not valid for curve "
<> Text.pack (show ecdsaCurve)
<> ": "
<> Text.pack (show point)
Left $
"ECDSA public key point is not valid for curve "
<> Text.pack (show ecdsaCurve)
<> ": "
<> Text.pack (show point)
where
curve = ECC.getCurveByName (toCryptCurveECDSA ecdsaCurve)
point = ECC.Point ecdsaX ecdsaY
Expand Down
26 changes: 14 additions & 12 deletions src/Crypto/WebAuthn/Cose/PublicKeyWithSignAlg.hs
Expand Up @@ -237,23 +237,24 @@ os2ipWithSize :: MonadFail m => Int -> BS.ByteString -> m Integer
os2ipWithSize size bytes
| BS.length bytes == size = pure $ os2ip bytes
| otherwise =
fail $
"bytes have length " <> show (BS.length bytes)
<> " when length "
<> show size
<> " was expected"
fail $
"bytes have length "
<> show (BS.length bytes)
<> " when length "
<> show size
<> " was expected"

-- | Same as 'os2ip', but throws an error if there are leading zero bytes. Thus any successful result of this function will give the same 'BS.ByteString' back if encoded with 'i2osp'.
os2ipNoLeading :: MonadFail m => BS.ByteString -> m Integer
os2ipNoLeading bytes
| leadingZeroCount == 0 = pure $ os2ip bytes
| otherwise =
fail $
"bytes of length "
<> show (BS.length bytes)
<> " has "
<> show leadingZeroCount
<> " leading zero bytes when none were expected"
fail $
"bytes of length "
<> show (BS.length bytes)
<> " has "
<> show leadingZeroCount
<> " leading zero bytes when none were expected"
where
leadingZeroCount = BS.length (BS.takeWhile (== 0) bytes)

Expand All @@ -262,7 +263,8 @@ decodeExpected :: (Show a, Eq a, Serialise a) => a -> Decoder s ()
decodeExpected expected = do
actual <- decode
unless (expected == actual) $
fail $ "Expected " <> show expected <> " but got " <> show actual
fail $
"Expected " <> show expected <> " but got " <> show actual

fromCurveEdDSA :: P.CoseCurveEdDSA -> R.CoseEllipticCurveOKP
fromCurveEdDSA P.CoseCurveEd25519 = R.CoseEllipticCurveEd25519
Expand Down
30 changes: 15 additions & 15 deletions src/Crypto/WebAuthn/Encoding/Binary.hs
Expand Up @@ -77,9 +77,9 @@ import GHC.Generics (Generic)
The functions in this module are grouped and named according to the
following conventions:

* If the type is parametrized by @raw@, there should be @stripRaw@ and
* If the type is parametrized by @raw@, there should be @stripRaw@ and
@encodeRaw@ functions
* If the type is serializable there should be a @decode@
* If the type is serializable there should be a @decode@
* In addition, if the type has a raw field for its own encoding (implying
that it's parametrized by @raw@), no other function needs to be provided
* Alternatively, there should be an @encode@ that encodes the type, using
Expand All @@ -88,9 +88,9 @@ following conventions:
If the type is parametrized by @raw@, this module should guarantee these
invariants for any @value :: TheType (raw ~ False)@:

* @stripRaw@ doesn't change any fields: @stripRaw value = value@
* @encodeRaw@ doesn't change any fields: @stripRaw (encodeRaw value) = value@
* If the type is also serializable:
* @stripRaw@ doesn't change any fields: @stripRaw value = value@
* @encodeRaw@ doesn't change any fields: @stripRaw (encodeRaw value) = value@
* If the type is also serializable:
* If the type has a raw field, @decode@ inverses @encodeRaw@ and
@getEncoded@: @stripRaw (decode (getEncoded (encodeRaw value))) = value@
* Alternatively, @decode@ inverses @encodeRaw@ and @encode@:
Expand All @@ -99,7 +99,7 @@ invariants for any @value :: TheType (raw ~ False)@:
If the type is only serializable then this invariant should hold for any
@value :: TheType@

* @decode@ inverses @encode@: @decode (encode value) = value@
* @decode@ inverses @encode@: @decode (encode value) = value@

If any such functions are expected to be used only internally, they may not be
exported
Expand All @@ -111,13 +111,13 @@ exported
-- custom binary format the
-- [binary](https://hackage.haskell.org/package/binary) library. However these
-- two libraries don't interact nicely with each other. Because of this we are
-- specifying decoders that don't consume all input as a 'PartialBinaryDecoder
-- DecodingError', which is just a function that can partially consume a
-- 'LBS.ByteString'. Using this we can somewhat easily flip between the two
-- specifying decoders that don't consume all input as a @PartialBinaryDecoder@,
-- which is just a state monad transformer over an 'LBS.ByteString'.
-- Using this we can somewhat easily flip between the two
-- libraries while decoding without too much nastiness.
type PartialBinaryDecoder a = StateT LBS.ByteString (Either Text) a

-- | Runs a 'PartialBinaryDecoder' using a strict bytestring. Afterwards it
-- | Runs a @PartialBinaryDecoder@ using a strict bytestring. Afterwards it
-- makes sure that no bytes are left, otherwise returns an error.
runPartialBinaryDecoder ::
BS.ByteString ->
Expand All @@ -129,11 +129,11 @@ runPartialBinaryDecoder bytes decoder =
Right (result, rest)
| LBS.null rest -> return result
| otherwise ->
Left $
"Not all binary input used, rest in base64 format is: "
<> decodeUtf8 (Base64.encode $ LBS.toStrict rest)
Left $
"Not all binary input used, rest in base64 format is: "
<> decodeUtf8 (Base64.encode $ LBS.toStrict rest)

-- | A 'PartialBinaryDecoder' for a binary encoding specified using
-- | A @PartialBinaryDecoder@ for a binary encoding specified using
-- 'Binary.Get'.
runBinary ::
Binary.Get a ->
Expand All @@ -147,7 +147,7 @@ runBinary decoder = do
put rest
pure result

-- | A 'PartialBinaryDecoder' for a CBOR encoding specified using the given
-- | A @PartialBinaryDecoder@ for a CBOR encoding specified using the given
-- 'CBOR.Decoder'.
runCBOR ::
(forall s. CBOR.Decoder s a) ->
Expand Down
3 changes: 3 additions & 0 deletions src/Crypto/WebAuthn/Encoding/Internal/WebAuthnJson.hs
Expand Up @@ -23,6 +23,9 @@ module Crypto.WebAuthn.Encoding.Internal.WebAuthnJson
PublicKeyCredentialRpEntity (..),
PublicKeyCredentialUserEntity (..),
PublicKeyCredentialParameters (..),
AuthenticationExtensionsClientInputs (..),
AuthenticationExtensionsClientOutputs (..),
CredentialPropertiesOutput (..),
COSEAlgorithmIdentifier,
PublicKeyCredentialDescriptor (..),
AuthenticatorSelectionCriteria (..),
Expand Down
17 changes: 10 additions & 7 deletions src/Crypto/WebAuthn/Operation/Authentication.hs
Expand Up @@ -245,17 +245,17 @@ verifyAuthenticationResponse origin rpIdHash midentifiedUser entry options crede
case (midentifiedUser, mUserHandler) of
(Just identifiedUser, Just userHandle)
| identifiedUser /= owner ->
failure $ AuthenticationIdentifiedUserHandleMismatch identifiedUser owner
failure $ AuthenticationIdentifiedUserHandleMismatch identifiedUser owner
| userHandle /= owner ->
failure $ AuthenticationCredentialUserHandleMismatch userHandle owner
failure $ AuthenticationCredentialUserHandleMismatch userHandle owner
| otherwise -> pure ()
(Just identifiedUser, Nothing)
| identifiedUser /= owner ->
failure $ AuthenticationIdentifiedUserHandleMismatch identifiedUser owner
failure $ AuthenticationIdentifiedUserHandleMismatch identifiedUser owner
| otherwise -> pure ()
(Nothing, Just userHandle)
| userHandle /= owner ->
failure $ AuthenticationCredentialUserHandleMismatch userHandle owner
failure $ AuthenticationCredentialUserHandleMismatch userHandle owner
| otherwise -> pure ()
(Nothing, Nothing) ->
failure AuthenticationCannotVerifyUserHandle
Expand Down Expand Up @@ -286,11 +286,13 @@ verifyAuthenticationResponse origin rpIdHash midentifiedUser entry options crede

-- 12. Verify that the value of C.challenge equals the base64url encoding of options.challenge.
unless (M.ccdChallenge c == M.coaChallenge options) $
failure $ AuthenticationChallengeMismatch (M.coaChallenge options) (M.ccdChallenge c)
failure $
AuthenticationChallengeMismatch (M.coaChallenge options) (M.ccdChallenge c)

-- 13. Verify that the value of C.origin matches the Relying Party's origin.
unless (M.ccdOrigin c == origin) $
failure $ AuthenticationOriginMismatch origin (M.ccdOrigin c)
failure $
AuthenticationOriginMismatch origin (M.ccdOrigin c)

-- 14. Verify that the value of C.tokenBinding.status matches the state of
-- Token Binding for the TLS connection over which the attestation was
Expand All @@ -305,7 +307,8 @@ verifyAuthenticationResponse origin rpIdHash midentifiedUser entry options crede
-- Note: If using the appid extension, this step needs some special logic.
-- See § 10.1 FIDO AppID Extension (appid) for details.
unless (M.adRpIdHash authData == rpIdHash) $
failure $ AuthenticationRpIdHashMismatch rpIdHash (M.adRpIdHash authData)
failure $
AuthenticationRpIdHashMismatch rpIdHash (M.adRpIdHash authData)

-- 16. Verify that the User Present bit of the flags in authData is set.
unless (M.adfUserPresent (M.adFlags authData)) $
Expand Down
12 changes: 8 additions & 4 deletions src/Crypto/WebAuthn/Operation/Registration.hs
Expand Up @@ -334,11 +334,13 @@ verifyRegistrationResponse
-- 8. Verify that the value of C.challenge equals the base64url encoding of
-- options.challenge.
unless (corChallenge == M.ccdChallenge c) $
failure $ RegistrationChallengeMismatch corChallenge (M.ccdChallenge c)
failure $
RegistrationChallengeMismatch corChallenge (M.ccdChallenge c)

-- 9. Verify that the value of C.origin matches the Relying Party's origin.
unless (rpOrigin == M.ccdOrigin c) $
failure $ RegistrationOriginMismatch rpOrigin (M.ccdOrigin c)
failure $
RegistrationOriginMismatch rpOrigin (M.ccdOrigin c)

-- 10. Verify that the value of C.tokenBinding.status matches the state of
-- Token Binding for the TLS connection over which the assertion was
Expand All @@ -363,7 +365,8 @@ verifyRegistrationResponse
-- 13. Verify that the rpIdHash in authData is the SHA-256 hash of the RP
-- ID expected by the Relying Party.
unless (rpIdHash == M.adRpIdHash authData) $
failure $ RegistrationRpIdHashMismatch rpIdHash (M.adRpIdHash authData)
failure $
RegistrationRpIdHashMismatch rpIdHash (M.adRpIdHash authData)

-- 14. Verify that the User Present bit of the flags in authData is set.
unless (M.adfUserPresent (M.adFlags authData)) $
Expand Down Expand Up @@ -391,7 +394,8 @@ verifyRegistrationResponse
let acdAlg = Cose.signAlg acdCredentialPublicKey
desiredAlgs = map M.cpAlg corPubKeyCredParams
unless (acdAlg `elem` desiredAlgs) $
failure $ RegistrationPublicKeyAlgorithmDisallowed desiredAlgs acdAlg
failure $
RegistrationPublicKeyAlgorithmDisallowed desiredAlgs acdAlg

-- 17. Verify that the values of the client extension outputs in
-- clientExtensionResults and the authenticator extension outputs in the
Expand Down
11 changes: 6 additions & 5 deletions tests/Main.hs
Expand Up @@ -345,18 +345,19 @@ isExpectedAttestationResponse _ _ _ (Left _) = False -- We should never receive
isExpectedAttestationResponse M.Credential {..} M.CredentialOptionsRegistration {..} verifiable (Right O.RegistrationResult {..}) =
rrEntry == expectedCredentialEntry
&& not verifiable
|| ( case rrAttestationStatement of
O.SomeAttestationStatement _ O.VerifiedAuthenticator {} -> True
_ -> False
)
|| ( case rrAttestationStatement of
O.SomeAttestationStatement _ O.VerifiedAuthenticator {} -> True
_ -> False
)
where
expectedCredentialEntry :: O.CredentialEntry
expectedCredentialEntry =
O.CredentialEntry
{ ceCredentialId = cIdentifier,
ceUserHandle = M.cueId corUser,
cePublicKeyBytes =
M.PublicKeyBytes . M.unRaw
M.PublicKeyBytes
. M.unRaw
. M.acdCredentialPublicKeyBytes
. M.adAttestedCredentialData
. M.aoAuthData
Expand Down
16 changes: 8 additions & 8 deletions tests/Spec/Types.hs
Expand Up @@ -119,11 +119,11 @@ instance Arbitrary (M.CollectedClientData c 'False) where
arbitrary =
M.CollectedClientData
<$> arbitrary
<*> arbitrary
-- The crossOrigin value can't be roundtripped with Nothing values,
-- so let's just not generate Nothing values here
<*> (Just <$> arbitrary)
<*> arbitrary
<*> arbitrary
-- The crossOrigin value can't be roundtripped with Nothing values,
-- so let's just not generate Nothing values here
<*> (Just <$> arbitrary)
<*> arbitrary

instance Arbitrary (M.AttestationObject 'False) where
arbitrary = do
Expand All @@ -142,9 +142,9 @@ instance Arbitrary ArbitraryAttestationStatementFormat where
arbitrary =
elements
[ ArbitraryAttestationStatementFormat None.Format
--ArbitraryAttestationStatementFormat Packed.Format,
--ArbitraryAttestationStatementFormat FidoU2F.Format,
--ArbitraryAttestationStatementFormat AndroidKey.Format
-- ArbitraryAttestationStatementFormat Packed.Format,
-- ArbitraryAttestationStatementFormat FidoU2F.Format,
-- ArbitraryAttestationStatementFormat AndroidKey.Format
]

instance Arbitrary M.SignatureCounter where
Expand Down