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

PublicKeyCredentialParameters can't select curve (E.g. ed448) #1446

Open
arianvp opened this issue Jun 27, 2020 · 9 comments
Open

PublicKeyCredentialParameters can't select curve (E.g. ed448) #1446

arianvp opened this issue Jun 27, 2020 · 9 comments
Assignees
Labels
@Risk Items that are at risk for L3 type:technical

Comments

@arianvp
Copy link

arianvp commented Jun 27, 2020

PublicKeyCredentialParameters Currently only allows you to choose the signature algorithm to use; but not which curve.

COSE standard suggests that ES256 should imply P-256, ES384 P-384 etc but this is only a suggestion.

From https://tools.ietf.org/html/rfc8152

In order to promote interoperability, it is suggested that SHA-256 be used only with curve P-256, SHA-384 be used only with curve P-384, and SHA-512 be used with curve P-521. This is aligned with the recommendation in Section 4 of [RFC5480].

The current Webauthn draft removes this ambiguity using the following text:

Keys with algorithm ES256 (-7) MUST specify P-256 (1) as the crv parameter.
Keys with algorithm ES384 (-35) MUST specify P-384 (2) as the crv parameter.
Keys with algorithm ES512 (-36) MUST specify P-521 (3) as the crv parameter.
Keys with algorithm EdDSA (-8) MUST specify Ed25519 (6) as the crv parameter.

Though this makes sense for the first three, (as it is "suggested" by RFC8512) it doesn't make a lot of sense for EdDSA where both Ed25519 and Ed448 are good choices. Now it is impossible to ask for an Ed448 curve using the Webauthn API.

Suggestion:

Add a field crv to the PublicKeyCredentialParameters object corresponding to crv in COSE. Such that PublicKeyCredentialParameters actually uniquely defines a public key type without needing extra restrictive text in the standard

Also we could potentially also add the kty parameter, to fully remove ambiguity; making PublicKeyCredentialParameters a proper subset of the COSE_key type. Which aids ergonomics and straightforwardness of implementation

arianvp added a commit to arianvp/haskell-fido2 that referenced this issue Jun 27, 2020
The current webauthn standard is rather ambigious again. So I think
there are some mistakes in the implementation that make our public
key decoder too crypto-agile.

The new draft of the webauthn standard is not ambigious, but also
has an issue I'd like to be clarified first:

w3c/webauthn#1446

it probably means `alg` should imply `crv` and we should remove
support for Ed448 or wait for the spec to be more fleshed out
arianvp added a commit to arianvp/haskell-fido2 that referenced this issue Jun 28, 2020
The current webauthn standard is rather ambigious again. So I think
there are some mistakes in the implementation that make our public
key decoder too crypto-agile.

The new draft of the webauthn standard is not ambigious, but also
has an issue I'd like to be clarified first:

w3c/webauthn#1446

it probably means `alg` should imply `crv` and we should remove
support for Ed448 or wait for the spec to be more fleshed out
@ve7jtb
Copy link
Contributor

ve7jtb commented Jul 1, 2020

Yes this is a known issue.

We would need to make a corresponding change to CTAP. Without that, it can't be passed to the authenticator.
There are other possible EdDSA curves as well as Ed448 that some people might want.

Remember that the alg are passed to the authenticator as a list ordered by the RP's preference, so we need to think about how the kty paramater would be applied by the authenticator. Is this just for edDSA or should it be more general?

@equalsJeffH
Copy link
Contributor

on 2020-07-01 call: given where we are in ctap and webauthn spec processes, we will punt this to L3.

@equalsJeffH equalsJeffH added this to the L3-WD-01 milestone Jul 1, 2020
@arianvp
Copy link
Author

arianvp commented Jul 1, 2020 via email

@agl
Copy link
Contributor

agl commented Jul 2, 2020

We punted this to level three (i.e. a future version):

The history is that JOSE started out fully specifying signature algorithms (i.e. JOSE defines ES256 as “ECDSA using P-256 and SHA-256”). On the assumption that COSE is just JOSE with CBOR, the underlying protocol between the client and security keys only expressed a preference list of COSE algorithm identifiers.

However, as noted, COSE subtly changed the meaning of ES256 to let the curve be an independent degree of freedom. (And JOSE has started doing that too for new algorithm registrations.)

We also have the reality that ECDSA with P-256 and SHA-256 dominates the WebAuthn ecosystem and any authenticator that tried to be technically correct and return an ES256 key that used a curve other than P-256 simply isn't going to function with most sites. Thus we've nailed down ES* to mean the obvious thing. I don't think there's sufficient utility in mismatching hash functions and curves to worry about that.

Ed25519 is newer but, as far as anyone in the group knows, all authenticators that support -8 always use Ed25519. On the server side, Ed25519 is more common than Ed448 and, if we didn't nail it down, we would just have the same problem: use of X448 wouldn't, practically speaking, work because sites would specify -8 but only accept Ed25519.

We could either define a different COSEAlgorithmIdentifier for X448 (which wouldn't need a WebAuthn change) or, as you suggest, support more fields in the parameters. But, since clients have to translate the parameters into the underlying security key protocol, that needs changes to that protocol too. It, too, is currently feature frozen for the next version.

Hopefully that explains why we punted this for now.

@arianvp
Copy link
Author

arianvp commented Jul 3, 2020

Yep. Makes sense!

I actually think less moving parts might be a good thing not a bad thing. Perhaps we should just close this. and leave the rigidity in place...

We could either define a different COSEAlgorithmIdentifier for X448

I'm not sure how realistic this is; as from the COSE-side of things they clearly have separated Algorithm from Curve and they already support alg = EdDSA, crv = Ed448 as a valid way to express this idea

@cyberphone
Copy link

cyberphone commented Jun 2, 2021

We could either define a different COSEAlgorithmIdentifier for X448

Yes, the sooner the better:
https://cyberphone.github.io/javaapi/org/webpki/cbor/package-summary.html#csf-algorithms
There is a rationale in the end of the algorithm table as well.

FIDO alliance have effectively redefined -8 to mean EdDSA with an Ed25519 key.

Using -9 (currently unassigned) for Ed448 would not break any existing code.

Related: I once had an argument with the author of RFC 8037 (when the damage was already done...) about using a family name like "EdDSA" instead of concrete signature algorithms. The cose-wg reference code is incompatible with existing crypto providers:
https://github.com/cose-wg/COSE-JAVA/blob/master/src/main/java/COSE/SignCommon.java#L45

@cyberphone
Copy link

How about the following pragmatic solution as a suggestion to the COSE-WG?

  • Enhancement: EdDSA (-8) does if no other information is available default to EdDSA for Ed25519 keys.
  • New: -9 denotes EdDSA for Ed448 keys.

Easy, somewhat ugly but backward compatible.

@selfissued
Copy link
Contributor

https://datatracker.ietf.org/doc/draft-jones-jose-fully-specified-algorithms/ was created to deal with this problem. It proposes new fully-specified algorithm identifiers that include the curve choice. See https://www.ietf.org/archive/id/draft-jones-jose-fully-specified-algorithms-01.html#name-elliptic-curve-digital-sign for specifics.

Once these are registered with IANA, we will be able to use them in WebAuthn implementations.

@selfissued
Copy link
Contributor

As an update, the IETF JOSE working group did adopt the fully-specified algorithms spec. Its current status can be viewed at https://datatracker.ietf.org/doc/draft-ietf-jose-fully-specified-algorithms/. I plan to ask for working group last call at IETF 119 in Brisbane two weeks from now.

@nadalin nadalin modified the milestones: L3-WD-02, Futures (catch-all) Sep 24, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
@Risk Items that are at risk for L3 type:technical
Projects
None yet
Development

No branches or pull requests

8 participants