JSON improvements, revisited

[lykahb]

This is based on #144

Compared to that PR, this branch:

• is based on master
• replaces JPEGBytes with PNGBytes
• fixes build errors

It seems that @infinisil's concerns about the COSE signature and the spec are related to the choice of algorihtm, and are orthogonal to the refactoring that introduces newtypes CoseMessage and CoseSignature.

[infinisil]

We don't have it verified in CI, but everything should be documented. Please run cabal haddock and make sure it doesn't give any warnings.

Also please add an entry to the changelog under a new 0.8.0.0 section.

[infinisil]

This type might need a better name. It's not that it contains a base16 bytestring, but rather it's the same as ByteString but with ToJSON and Show instances that print it as base16 for debugging purposes. This should be in this types documentation. How about calling it PrintableByteString instead.

[lykahb]

Thanks for the review @infinisil. I could see how Base16ByteString could be misinterpreted that way. Instead of PrintableByteString I chose a name PrettyHexByteString instead - it both shows that its main purpose is pretty-printing and that it displays it as hex. Let me know what you think.

The documentation for the COSE newtypes points to the rfc8152. I am not deeply familiar how COSE should work, so can you take a look at that?

The haddock has full coverage with no warnings.

[infinisil]

PrettyHexByteString sounds good to me :)

[infinisil]

Taking a closer look at the specs again, I don't think this is right. The encoding of these depends on the signature algorithm used, see here. It particular it doesn't seem to be using the COSE_Messages and COSE_Signature structures at all.

Suggested change

	-- | Binary representation of the COSE_Messages structure defined in the [spec](https://datatracker.ietf.org/doc/html/rfc8152#section-2).
	newtype CoseMessage = CoseMessage {unCoseMessage :: BS.ByteString}
	deriving newtype (Eq, Show)
	deriving (ToJSON) via PrettyHexByteString
	-- | Binary representation of the COSE_Signature structure in the [spec](https://datatracker.ietf.org/doc/html/rfc8152#section-4.1).
	newtype CoseSignature = CoseSignature {unCoseSignature :: BS.ByteString}
	deriving newtype (Eq, Show)
	deriving (ToJSON) via PrettyHexByteString
	-- | A wrapper for the bytes of a message that should be verified.
	-- This is used for both assertion and assertion.
	newtype Message = Message {unMessage :: BS.ByteString}
	deriving newtype (Eq, Show)
	deriving (ToJSON) via PrettyHexByteString
	-- | [(spec)](https://www.w3.org/TR/webauthn-2/#sctn-signature-attestation-types)
	-- A wrapper for the bytes of a signature that can be used to verify a 'Message'.
	-- The encoding is specific to webauthn and depends on the 'CoseSignAlg' used.
	newtype Signature = Signature {unSignature :: BS.ByteString}
	deriving newtype (Eq, Show)
	deriving (ToJSON) via PrettyHexByteString

[lykahb]

Applied the suggestions. I'm still confused about these two types - isn't the top-level COSE structure the same for any signing algorithm? The spec rfc8152 seems only to define the positions of the signed payload and the signature.

These two specs https://datatracker.ietf.org/doc/html/rfc8152 and https://www.w3.org/TR/webauthn-2 seem to complement each other rather than repeat what COSE standards are.

Also, based on the verify function implementation it seems that this library doesn't support multiple signatures (the COSE_Sign struct in rfc8152). Maybe it doesn't matter because we don't get any errors about it in practice, but I wonder how we could test that and assert that we get an unambiguous error message that it's unsupported.

[infinisil]

From what I understand, webauthn mainly relies on the COSE spec just for the public key and signing algorithm (see COSE_Key here), that's really all that's implemented by the Crypto.WebAuthn.Cose modules. Other than that, the COSE spec isn't needed, the COSE_Sign and co. are never used for webauthn.

[infinisil]

Looking good!