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

Carsten's Review for jose-cose-guidance #90

Open
hannestschofenig opened this issue Jan 4, 2024 · 1 comment
Open

Carsten's Review for jose-cose-guidance #90

hannestschofenig opened this issue Jan 4, 2024 · 1 comment

Comments

@hannestschofenig
Copy link
Owner

(2) For COSE, the one subject discussed leads to a new MUST on a header parameter that defined in STD96 (kid, defined in RFC 9052, Section 3.1). I don't think a BCP can redefine a parameter defined in a standards-track document. RFC 9052 is quite explicit that »Applications MUST NOT assume that "kid" values are unique«:

kid: This header parameter identifies one piece of data that can be
used as input to find the needed cryptographic key. The value of
this header parameter can be matched against the "kid" member in a
COSE_Key structure. Other methods of key distribution can define
an equivalent field to be matched. Applications MUST NOT assume
that "kid" values are unique. There may be more than one key with
the same "kid" value, so all of the keys associated with this
"kid" may need to be checked. The internal structure of "kid"
values is not defined and cannot be relied on by applications.
Key identifier values are hints about which key to use. This is
not a security-critical field. For this reason, it can be placed
in the unprotected-header-parameters bucket.

(3) The rationale given in the draft for replacing the definition of this parameter with what essentially is a new definition is rather wobbly. It seems that the intention is not really to add security, but just to reduce the attack surface for attacks that continue to be possible if the "guidance" of the document is heeded.

(4) The term "globally unique" is not defined. I don't think it actually means anything in the specification as it is being used.

If the intention is to define a globally unique Key ID, please define a header parameter that has the desired properties (e.g, "gukid", globally unique kid). For this, please define "globally unique".

Or what we maybe should do here is define specific forms of the "kid" value that do have some desirable uniqueness properties. Proponents of the approach of this document can then recommend the use of one of these forms.

But what this document really tries to do has little to do with "kid", but can be summarized by a list of recommendations an expression of which I'll steal from an off-list discussion:

  1. try not to deserialize untrusted data needlessly (reducing attack surface, not actually increasing security) — this is not just about kid.

kid-related:
2. try not to use identifiers for keys that suck (yes, need an operational definition of what that means)
3. you are still responsible for establishing the actual authorization properties of the key ("trusting" it) before using it to verify / decrypt...
-- but did you read all the warnings about how headers can be misleading, and maybe you are holding the wrong key?

@yaronf
Copy link
Contributor

yaronf commented Feb 8, 2024

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

No branches or pull requests

2 participants