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

header: fix authentication when protected header is zero-length map #98

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

pulsastrix
Copy link

@pulsastrix pulsastrix commented Jul 28, 2024

COSE allows an empty protected header to be encoded as a zero-length map, even though the standard encourages encoding empty protected headers as a zero-length string (RFC 2119 SHOULD according to RFC 9052, Section 3).

However, according to RFC 9052, Section 4.4, 5.3 and 6.3, even if the header is encoded as a zero-length map, the structure used for authentication should not include the empty map if the protected header is empty ("if there are no protected attributes, a zero-length byte string is used").

Due to this, authentication of some of the official examples in the cose-wg/Examples repository was not possible using coset.
An example of this is the CoseSign1 object provided in
sign1-tests/sign-pass-03.json sign1-tests/sign1-pass-01.json, which uses the protected header encoding as a zero-length map.

This PR ensures that the correct behavior is implemented in coset by no longer including a zero-length map protected header during authentication.

Note: I am unsure as to how to proceed with the tests that now fail, as it seems like those explicitly expect a behavior different from the RFC.

COSE allows an empty protected header to be encoded as a zero-length
map, even though the standard encourages encoding empty protected
headers as a zero-length string (RFC 2119 SHOULD according to RFC 9052,
Section 3).

However, according to RFC 9052, Section 4.4, 5.3 and 6.3, even if the
header is encoded as a zero-length map, the structure used for
authentication should not include the empty map if the protected header
is empty ("if there are no protected attributes, a zero-length byte
string is used").

This commit ensures that this behavior is implemented in coset, which
previously did include the zero length map (encoded as h'a0') in
signature calculation.
This previously caused signature verification failures, e.g. when
verifying the CoseSign1 object provided in
https://github.com/cose-wg/Examples/blob/master/sign1-tests/sign-pass-03.json
using coset.
// protected header might have been encoded as a zero length map, only containing
// the byte 0xA0 (see RFC 9052, Section 3).
// However, this byte should not be included during authentication (RFC 9052, Section 4.4,
// 5.3 and 6.3), so we need to return an empty byte string here instead.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When would ProtectedHeader::cbor_bstr() ever return an (encoded) empty map?

I think the is_empty() check would mean that this could only happen if the original input (in ProtectedHeader::original_data) had the non-minimal encoding, and that object then got re-encoded using the original data. Or is there another way it can happen?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That should be the only way that this can happen, yes.

For actually re-encoding an existing COSE object this should not be an issue (hence why cbor_bstr() is unchanged).

However, as far as I understand the COSE specification for generating the structures that are provided to the signature/encryption/MAC-function (e.g. Sig_structure), this non-minimal encoding needs to be explicitly replaced with a zero-length byte string if no attributes are set, regardless of whether the actual encoding of the protected header is minimal or not.

The aforementioned examples/test cases explicitly check for this behavior (e.g. sign1-tests/sign-pass-01 and sign-tests/sign-pass-01), and in my own tests verification of these (supposedly valid) COSE structures only works with the changes made in this PR.

@daviddrysdale
Copy link
Collaborator

Coming back to this (apologies for the delay), I notice that there's an open issue that possibly indicates that the sign1-tests/sign1-pass-01.json test case is possibly expected to be invalid?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants