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

Signatures: "alg" for JWS signatures #1907

Closed
yaronf opened this issue Jan 30, 2022 · 4 comments · Fixed by #1964
Closed

Signatures: "alg" for JWS signatures #1907

yaronf opened this issue Jan 30, 2022 · 4 comments · Fixed by #1964

Comments

@yaronf
Copy link
Contributor

yaronf commented Jan 30, 2022

This sentence is correct:

There is no use of the explicit alg signature parameter when using JOSE signing algorithms, as they can be signaled using JSON Web Keys or other mechanisms.

But it's true for most reasonable uses of alg, and people will still be using this parameter. So why not add:

If the alg parameter is used anyway, its value MUST be the algorithm's name (Sec. 3.1 of RFC 7518), such as "ES256".

@jricher
Copy link
Contributor

jricher commented Jan 30, 2022

No, we explicitly did not want to conflate this with the jwa registry. We also didn't want to have a second tier choice like having "alg=jose" and then having the actual algorithm elsewhere, since a big problem with late editions of the Cavage draft were exactly that process. (See the mess around "hs2019").

The editors discussed this at length and decided the best compromise was to define a way that jwa algorithms could be used in an interoperable fashion, but not to allow runtime signaling with them. Better to have the choice of jwa be signaled within the application level.

Additionally, it's preferable to not use the runtime parameter to signal the algorithm any way, which is discussed at length in the security considerations.

@yaronf
Copy link
Contributor Author

yaronf commented Jan 30, 2022

All good, so why aren't you using normative language?

An explicit alg signature parameter SHOULD/MUST NOT be included when using JOSE signing algorithms, as they can be signaled using JSON Web Keys or other mechanisms.

@jricher
Copy link
Contributor

jricher commented Jan 30, 2022

It's not normative because this section defines the algorithms, not the alg parameter. We didn't want to make it seem like the alg parameter was the "right" way to signal algorithms here, and most certainly is not required. You'll notice that none of the other sections have "MUST use the alg parameter value of 'foo'" either. Older Cavage drafts relied on the alg parameter, albeit in a much less secure fashion, and implementations were susceptible to algorithm substitution in the same way that naive JOSE implementations tend to be -- the "alg: none" attack all over again, more or less.

The value of the alg parameter already MUST be something from the registry, and the registry does not (and will not) contain JWA values. So you already aren't allowed to do it, normatively, and the note is our attempt at explaining the why.

Ultimately, an implementation is allowed to use whatever algorithm they want and makes sense. What this whole section does is define a set of common usable algorithms as well as provide a pattern of applying crypto primitives that applications can follow for their own algorithms. If someone defines a new algorithm that's generally useful, like we recently did with the ed25519 here, it can be registered for use with the alg parameter. But if a system can base the algorithm selection off of the key material or another part of the protocol (like with GNAP), that's highly preferable and less vulnerable to substitution at runtime.

I'd be happy to know how we can make this clearer in the text, but I'm not comfortable with that kind of normative construct in that section.

@jricher
Copy link
Contributor

jricher commented Jan 31, 2022

@richanna -- would you like to chime in? I recall from our discussions you having another, more specific reason for this decision, but I can't remember the details beyond us agreeing to do it the way we have it in the spec right now.

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

Successfully merging a pull request may close this issue.

2 participants