acmesh-official / acme.sh

A pure Unix shell script implementing ACME client protocol
https://acme.sh
GNU General Public License v3.0
39.44k stars 4.98k forks source link

ec-256 keytype as default breaks manual mode #4416

Open ianw opened 1 year ago

ianw commented 1 year ago

Change https://github.com/acmesh-official/acme.sh/commit/ec0e871592d286b8cd4e1d407411f0ba17c775e6 appears to break manual mode renewals.

What happens is that we run the issue step, and see

DOMAIN_PATH='/etc/letsencrypt-certs/<domain>_ecc'

I think what happens is _initpath sets that up by using the isEccKey function

    if _isEccKey "$_ilength"; then
      DOMAIN_PATH="$domainhomeecc"
    else

With https://github.com/acmesh-official/acme.sh/commit/ec0e871592d286b8cd4e1d407411f0ba17c775e6 the length is now marking this as a ECC key, and thus changing the DOMAIN_PATH.

This issue is made, we get our TXT records to install into DNS and acme.sh exits. We install the records using our automation tools, then come back to run the renew step.

What should happen then is that the domain .conf file should be read, where the Le_Vlist will be set with the required tokens, and acme.sh will then attempt to authenticate the records and issue the certificate.

However, the renew function gets called from the command-line parser

  renew)
    renew "$_domain" "$_ecc" "$_server"

where $_ecc is only set from the command-line flag; e.g.

    --ecc)
      _ecc="isEcc"
      ;;

Thus the renewal path is now ends up setting the DOMAIN_PATH with no _ecc; we see in the logs for the renewal

DOMAIN_PATH='/etc/letsencrypt-certs/<domain>'

The renewal then falls into

  vlist="$Le_Vlist"
  _cleardomainconf "Le_Vlist"
  _info "Getting domain auth token for each domain"
  sep='#'
  dvsep=','
  if [ -z "$vlist" ]; then
    #make new order request

Now the Le_Vlist is empty -- because it's using the wrong DOMAIN_PATH directory, and it falls into the make new order request part and tries to re-authenticate the domain again.

In general, I'm not sure this change is backwards-compatible because of the way ecc keys have been separated out into a separate directory. It seems like this will mean that everyone will start creating keys in <domain>_ecc by default, which is not where they are expected to be?

ianw commented 1 year ago

The logs for the issue (called by https://opendev.org/opendev/system-config/src/branch/master/playbooks/roles/letsencrypt-acme-sh-install/files/driver.sh#L45) and the renewal (called by https://opendev.org/opendev/system-config/src/branch/master/playbooks/roles/letsencrypt-acme-sh-install/files/driver.sh#L108) are below

airshipit-issue.txt airshipit-renew.txt

hoewer commented 1 year ago

This issue also breaks deploy scripts which currently rely on the correct folder name (folder name = certname). When using ECC-Certificates the folder name is changed to 'certname_ecc'. The deploy scripts report

[Fri Jan  6 12:54:50 UTC 2023] The domain 'example.com' is not a cert name. You must use the cert name to specify 
the cert to install.                                                                                                     
[Fri Jan  6 12:54:50 UTC 2023] Can not find path:'/acme.sh/example.com'

At a first glance manually renaming the folder name seems to solve this part of the issue.

Neilpang commented 1 year ago

@hoewer which deploy script did you use ?

Neilpang commented 1 year ago

@ianw sorry, but can you please try again with the latest dev code, I have fixed such kind of bugs recently.

acme.sh  --upgrade -b dev
hoewer commented 1 year ago

@Neilpang Sorry for the missing information. I'm using the Synology DSM deploy hook.

joshbartley commented 1 year ago

I have acme.sh running in a github action and because of the file path changes it almost broke our renewal pipeline. It's a fresh install of acme.sh each time and it started to default to ecc scripts in a different directory which didn't get packaged up correctly.

Install into the github action container is wget -O acme.sh https://raw.githubusercontent.com/acmesh-official/acme.sh/master/acme.sh

Had to add a -k 2048 to force it back to RSA to get a renewal going.

fungi commented 1 year ago

@ianw sorry, but can you please try again with the latest dev code, I have fixed such kind of bugs recently.

acme.sh  --upgrade -b dev

Apologies for the lengthy delay in response. I help manage the same systems as @ianw originally found this in, and we believe we're still seeing the same behavior which is blocking our attempts to upgrade from 3.0.5 to 3.0.6. Is running from the dev branch still a useful test at this point?

Roy-Orbison commented 2 months ago

@hoewer Obtaining a list of domains should be done via acme.sh --list possibly using specific info from acme.sh --info --domain domain.example. However, one should limit reliance on acme.sh's internal structure, as it's changed before and may again. If you mainly need the certificate files, use an acme.sh --install-cert ... --reloadcmd ... command to get and deploy them, even if that's only to temporary location before their actual deployment.