go-acme / lego

Let's Encrypt/ACME client and library written in Go
https://go-acme.github.io/lego/
MIT License
7.45k stars 987 forks source link

P521 support for internal JWS signing algorithm #956

Open enugentdt opened 4 years ago

enugentdt commented 4 years ago

Hello,

Just a quick note. If I use a P521 curve for my signing key, it crashes with the following error:

failed to post JWS message -> failed to sign content -> failed to create jose signer -> square/go-jose: unknown/unsupported algorithm

The solution would be to add the P521 curve to acme/api/internal/secure/jws.go:

else if k.Curve == elliptic.P521() {
    alg = jose.ES512
}

Hope this helps. I'd be happy to PR it if needed.

ldez commented 4 years ago

Hello,

the current supported key types are: https://github.com/go-acme/lego/blob/f6230a26624ea6effd6e96925b5f3bf415b5d29f/certcrypto/crypto.go#L21-L28

I'm not sure that LE allow to use P521. I need to find the information.

ldez commented 4 years ago

https://letsencrypt.org/docs/integration-guide/#supported-key-algorithms

Let’s Encrypt accepts RSA keys from 2048 to 4096 bits in length, and P-256 and P-384 ECDSA keys. That’s true for both account keys and certificate keys. You can’t reuse an account key as a certificate key.

ldez commented 4 years ago

I checked the pebble code: https://github.com/letsencrypt/pebble/blob/08b2d7a6116126bda2fa1fab09dfb24800a79959/wfe/jose.go#L21-L30

And the boulder code: https://github.com/letsencrypt/boulder/blob/62db2d0cae397ad393ac30463049ea387e81f60e/wfe2/verify.go#L33-L42

The documentation seems outdated.

ldez commented 4 years ago

so @enugentdt you can do a PR :+1:

cpu commented 4 years ago

https://letsencrypt.org/docs/integration-guide/#supported-key-algorithms

Let’s Encrypt accepts RSA keys from 2048 to 4096 bits in length, and P-256 and P-384 ECDSA keys. That’s true for both account keys and certificate keys. You can’t reuse an account key as a certificate key.

This is correct. We don't allow P-521 ECDSA account keys or end-entity certificate keys.

And the boulder code: https://github.com/letsencrypt/boulder/blob/62db2d0cae397ad393ac30463049ea387e81f60e/wfe2/verify.go#L33-L42

The documentation seems outdated.

@ldez I think you're looking at the wrong part of Boulder. Pebble looks like it does support it (largely because we didn't port the majority of the goodkey logic to Pebble, not as an active choice) but Boulder does not.

In goodKeyECDSA we call out to goodCurve: https://github.com/letsencrypt/boulder/blob/7fc21382ebf8dea92803791188d3de1047c2a6f4/goodkey/good_key.go#L85-L93

goodCurve in turn only allows P-256 and P-384: https://github.com/letsencrypt/boulder/blob/7fc21382ebf8dea92803791188d3de1047c2a6f4/goodkey/good_key.go#L182-L194

so @enugentdt you can do a PR +1

Not allowing P-521 ECDSA account keys is technically a Let's Encrypt specific limitation and other RFC 8555 implementor may allow it. I'm not sure if any do in practice. From my perspective I'm very skeptical of the utility of using P-521 over the other curves but it's not forbidden for account keys.

W.r.t end-entity certificates the restriction is not specific to Let's Encrypt. Any CA that is participating in the Mozilla root program is forbidden from issuing end-entity certificates with an P-521 public key:

Root certificates in our root program, and any certificate which chains up to them, MUST use only algorithms and key sizes from the following set:

ECDSA keys using one of the following curve-hash pairs: P‐256 with SHA-256 P‐384 with SHA-384

Hope that background helps!

ldez commented 4 years ago

@cpu thanks, very interesting :+1:

@enugentdt So I think we will not implement P-521 ECDSA for now.

Thanks all!