Issue with reuse-key when updating domains

[urz-hgw]

Hi,

My operating system is (include version):

Debian 12

I installed Certbot with (snap, OS package manager, pip, certbot-auto, etc):

package manager (v2.1) and docker (v2.6) for testing this issue with the latest version.

I ran this command and it produced this output:

I created a certificate using a different elliptic curve than default:

certbot certonly --standalone --agree-tos --no-eff-email --email EMAIL --domain test1.example.com --key-type ecdsa --elliptic-curve secp384r1

Then I wanted to update the certificate by adding another domain but reuse the key:

certbot certonly --standalone --cert-name test1.example.com --domain test1.example.com,test2.example.com --reuse-key

The output the is:

Unable to change the --elliptic-curve of this certificate because --reuse-key is set. To stop reusing the private key, specify --no-reuse-key. To change the private key this one time and then reuse it in future, add --new-key.

Certbot's behavior differed from what I expected because:

I expect certbot to reuse the key. Furthermore the error message is IMHO a contradiction. It says I can't change the elliptic curve BECAUSE reuse-key is set. But certbot seems trying to use the default key settings here, which are different from my previously chosen settings for the key I want to reuse. When I explicitly add " --key-type ecdsa --elliptic-curve secp384r1" again.. a new key is created, so IMHO "--reuse-key" is ignored completely here.

Here is a Certbot log showing the issue (if available):

2023-07-12 09:36:31,543:DEBUG:certbot._internal.display.obj:Falling back to default True for the prompt:
You are updating certificate test1.example.com to include new domain(s):

• test2.example.com

You are also removing previously included domain(s):
(None)

Did you intend to make this change?
2023-07-12 09:36:31,543:DEBUG:certbot._internal.display.obj:Notifying user: Renewing an existing certificate for test1.example.com
2023-07-12 09:36:31,556:DEBUG:certbot._internal.plugins.selection:Requested authenticator standalone and installer <certbot._internal.cli.cli_utils._Default object at 0x7f40a7714ca0>
2023-07-12 09:36:31,556:DEBUG:certbot._internal.cli:Var reuse_key=True (set by user).
2023-07-12 09:36:31,557:DEBUG:certbot._internal.log:Exiting abnormally:
Traceback (most recent call last):
File "/usr/local/bin/certbot", line 33, in
sys.exit(load_entry_point('certbot', 'console_scripts', 'certbot')())
File "/opt/certbot/src/certbot/certbot/main.py", line 19, in main
return internal_main.main(cli_args)
File "/opt/certbot/src/certbot/certbot/_internal/main.py", line 1864, in main
return config.func(config, plugins)
File "/opt/certbot/src/certbot/certbot/_internal/main.py", line 1597, in certonly
lineage = _get_and_save_cert(le_client, config, domains, certname, lineage)
File "/opt/certbot/src/certbot/certbot/_internal/main.py", line 129, in _get_and_save_cert
renewal.renew_cert(config, domains, le_client, lineage)
File "/opt/certbot/src/certbot/certbot/_internal/renewal.py", line 385, in renew_cert
_avoid_reuse_key_conflicts(config, lineage)
File "/opt/certbot/src/certbot/certbot/_internal/renewal.py", line 372, in _avoid_reuse_key_conflicts
raise errors.Error(
certbot.errors.Error: Unable to change the --elliptic-curve of this certificate because --reuse-key is set. To stop reusing the private key, specify --no-reuse-key. To change the private key this one time and then reuse it in future, add --new-key.

Best regards
Daniel

[osirisinferi]

I think the part here where it checks the curve in the configuration with the key in the lineage

certbot/certbot/certbot/_internal/renewal.py

Lines 367 to 368 in e6572e6

	lambda: kt == "ecdsa" and lineage.elliptic_curve and \
	config.elliptic_curve.lower() != lineage.elliptic_curve.lower())

is not actually checking the correct thing in this specific situation.

I think that code should check for whatever has been entered on the command line using --elliptic-curve while currently it checks the default curve. Which can indeed be different than the curve set in the lineage, but we shouldn't care about that.

Some little testing makes me think a simple extra check with config.set_by_user('elliptic_curve') in the lambda should do the trick?

[alexzorin]

When I explicitly add " --key-type ecdsa --elliptic-curve secp384r1" again.. a new key is created, so IMHO "--reuse-key" is ignored completely here.

That doesn't look like the case to me.

I ran:

$ certbot certonly --standalone --agree-tos --no-eff-email --register-unsafely-without-email --domain test1.example.com --key-type ecdsa --elliptic-curve secp384r1

$ sudo openssl pkey -in /etc/letsencrypt/live/test1.example.com/privkey.pem -noout -text
Private-Key: (384 bit)
priv:
    84:49:ec:3d:2a:9d:9c:5d:fc:74:2f:fb:a2:c1:85:
    cc:04:64:d8:24:08:29:d6:89:a5:ae:ee:07:c6:11:
    b5:3e:da:af:9e:4a:22:a8:ff:3b:18:99:38:c0:67:
    cd:b8:1a
....

then

$ certbot certonly --standalone --cert-name test1.example.com --domain test1.example.com,test2.example.com --reuse-key --elliptic-curve secp384r1

and the key was the same:

$ sudo openssl pkey -in /etc/letsencrypt/live/test1.example.com/privkey.pem -noout -text
Private-Key: (384 bit)
priv:
    84:49:ec:3d:2a:9d:9c:5d:fc:74:2f:fb:a2:c1:85:
    cc:04:64:d8:24:08:29:d6:89:a5:ae:ee:07:c6:11:
    b5:3e:da:af:9e:4a:22:a8:ff:3b:18:99:38:c0:67:
    cd:b8:1a
....

I think what you have described is how certonly is intended to work.

The way certonly has always worked, is that it is issuing an entirely fresh certificate without migrating parameters from any existing certificate, even if --cert-name points to an existing certificate.

Therefore, you need to provide the full set of request parameters that you need, on the command line.

In the case of --reuse-key (and --key-type, actually), Certbot takes an extra measure to prevent users from accidentally shooting themselves in the foot, and requires them to be very explicit when there's a chance that a key parameter is changing unintentionally.

Typically, you would want to use certbot renew to change an existing certificate (while also migrating the certificate's previous settings), but this is not possible for the certificate's --domains. It's necessary to use certonly, keeping in mind all of the above.

I'm not going to be able to review Osiris' PR, but this design was (iirc) completely intentional and in my opinion is not a bug, but it's possible there might be some way to improve the UX or documentation around it.

I appreciate also that this is confusing as a user. --reuse-key is probably one of the parts of Certbot's UI with the most complex interactions, and that's reflected in how hard it is to use, I guess.

[urz-hgw]

Thanks for your responses and explanation.
I retested and can verify, that it is in fact the same key, sorry for not checking.

The way certonly has always worked, is that it is issuing an entirely fresh certificate without migrating parameters from any existing certificate, even if --cert-name points to an existing certificate.

In the case of --reuse-key (and --key-type, actually), Certbot takes an extra measure to prevent users from accidentally shooting themselves in the foot, and requires them to be very explicit when there's a chance that a key parameter is changing unintentionally.

But still certonlyis able to reuse the same key from an existing certificate I am pointing to, that's still a bit confusing for me, why this could be a problem so that all key parameters are needed. But I think I get it now and the error message is more clear to me.

$ certbot certonly --standalone --cert-name test1.example.com --domain test1.example.com,test2.example.com --reuse-key --elliptic-curve secp384r1

This creates an updated (or new certificate to be exact) with the additional domain, but uses the same private key - so it should be the same like

$ certbot certonly --standalone --cert-name test1.example.com --domain test1.example.com,test2.example.com --reuse-key

So it looks to me (and that would clarify the error message) that even if --reuse-key is used the key parameters --key-type and --elliptic-curve are still used mandatorily when using certonly and if they don't match to the already existing key, the error message appears. A solution could be that if --reuse-key is used the key parameters are no longer used for the request and if set manually and don't match said error message appears.

Best regards
Daniel

[bmw]

I agree this behavior seems annoying.

With that said, we/I felt that there were many potential foot guns for people around --reuse-key and the additional key types when we were adding them. We spent a lot of time thinking about and testing this (see the design doc or reuse-key scenarios for example). Because of this, I'm not personally interested in trying to change our behavior here unless someone on the Certbot team wants to do the work of thinking through all the different scenarios (or verifying that someone else has correctly done so). I'm personally not motivated to do it unless we get many more complaints about this.

I'm sure that's not what you wanted to hear, but Certbot is a well established project now with millions of people using it in all kinds of ways and for better or worse I'm much more worried about potentially breaking things for them than keeping some UX ugliness around. Hope that at least kind of makes sense.

With that said:

1. It seems like only erroring if these flags were set on the CLI may have actually been our intended behavior based on the design doc linked above and code comments like
   

   certbot/certbot/certbot/_internal/renewal.py

   Lines 357 to 360 in c31d3a2

   	# The remaining cases where conflicts are present:
   	# - --key-type is set on the CLI and doesn't match the stored private key
   	# - It's an RSA key and --rsa-key-size is set and doesn't match
   	# - It's an ECDSA key and --eliptic-curve is set and doesn't match

2. I would be open to improving the error message without changing Certbot's behavior in the meantime.

[urz-hgw]

Hi,

it was just not clear to me and as soon as I understood it, it felt a bit strange because of the (for me) unexpected behavior. But it looks like I am the first one who has a problem with that, so be it, it was not my intention to force you into a change you don't feel comfortable with.

But I would definitely appreciate an improved error message to make the issue more clear.

Thanks so far.
Daniel

[osirisinferi]

@bmw I think your "1" point in your post in the issue is what's bothering me: the current behaviour is not what was intended, which is also clear from the posted documents. (And a first issuance with a non-default setting was not in one of the scenarios listed in the spreadsheet from the looks of it, every scenario starts with the defaults and continues with non-default settings.)

I'd like to argue currently Certbot is "broken" and requires fixing. And I'm not sure if the impact of fixing this is actually that big? If renewals are succeeding now, they also will succeed after this is fixed. I'm assuming Certbot will actually reuse the key after the added CLI checks are added in my PR? This is of course something that requires testing.

[bmw]

the current behaviour is not what was intended

I'm honestly not sure if that's true or not. It sure seems like that wasn't what I intended when I wrote that doc and that's what the code comment seems to suggest, but this feature was developed over months by multiple developers and multiple PRs. I think confidently concluding that requires further digging into the history here and even if that behavior is what we initially wanted, I don't think it's obvious that we can change it now without problems.

We may be able to change it, but I'd encourage us to be very careful about doing so. This all may seem relatively straightforward, but this area was something that multiple members of the team found to be quite tricky to work on. We've also had bugs related to this kind of thing. Because of all that, I personally feel that doing anything other than changing the error message isn't worth the effort, but that's just my opinion. You are of course entitled to your own.

[no-identd]

This ticket feels like it sits in a quantum superposition until someone interacts with it to force a collapse of the wave function, so, here, take this otherwise superfluous comment