dehydrated-io / dehydrated

letsencrypt/acme client implemented as a shell-script – just add water
https://dehydrated.io
MIT License
5.97k stars 717 forks source link

When there is an error, it removes domains.txt #597

Open bortzmeyer opened 6 years ago

bortzmeyer commented 6 years ago

I ran dehydrated and there was a error in my request ("status": 400). As a result, dehydrated removed domains.txt.

Running it again, same issue with my request, it tries again to remove it and prints: rm: cannot remove '/etc/dehydrated/domains.txt': No such file or directory

lukas2511 commented 6 years ago

Mh... Are you using the --signcsr function combined with --domain parameters? Those are not designed to be used with this function and seem to be the trigger for this bug.

bortzmeyer commented 6 years ago

Mh... Are you using the --signcsr function combined with --domain parameters? Those are not designed to be used with this function and seem to be the trigger for this bug.

I do. OK, I'll try without --domain (it is in the CSR after all) but the bug is still really damaging.

ilyaevseev commented 4 years ago

The problem still here, /etc/dehydrated/domains.txt is removed on error:

# bash -x /usr/local/sbin/dehydrated -c -d test.org
+ set -e
+ set -u
+ set -o pipefail
+ [[ -n '' ]]
+ [[ -z '' ]]
+ shopt -s nullglob
+ set -f
+ umask 077
+ exec
+ exec
+ VERSION=0.6.5
+ SOURCE=/usr/local/sbin/dehydrated
+ '[' -h /usr/local/sbin/dehydrated ']'
+++ dirname /usr/local/sbin/dehydrated
++ cd -P /usr/local/sbin
++ pwd
+ SCRIPTDIR=/usr/local/sbin
+ BASEDIR=/usr/local/sbin
+ ORIGARGS='-c -d test.org'
++ uname
+ OSTYPE=Linux
+ [[ ! '' = \N\O\O\P ]]
+ main -c -d test.org
+ COMMAND=
+ [[ -z -c -d test.org ]]
+ ((  3  ))
+ case "${1}" in
+ set_command sign_domains
+ [[ -z '' ]]
+ COMMAND=sign_domains
+ shift 1
+ ((  2  ))
+ case "${1}" in
+ shift 1
+ check_parameters test.org
+ [[ -z test.org ]]
+ [[ t = \- ]]
+ [[ -z '' ]]
+ PARAM_DOMAIN=test.org
+ shift 1
+ ((  0  ))
+ case "${COMMAND}" in
+ command_sign_domains
+ init_system
+ load_config
+ [[ -z '' ]]
+ for check_config in "/etc/dehydrated" "/usr/local/etc/dehydrated" "${PWD}" "${SCRIPTDIR}"
+ [[ -f /etc/dehydrated/config ]]
+ BASEDIR=/etc/dehydrated
+ CONFIG=/etc/dehydrated/config
+ break
+ CA=https://acme-v02.api.letsencrypt.org/directory
+ OLDCA=
+ CERTDIR=
+ ALPNCERTDIR=
+ ACCOUNTDIR=
+ CHALLENGETYPE=http-01
+ CONFIG_D=
+ CURL_OPTS=
+ DOMAINS_D=
+ DOMAINS_TXT=
+ HOOK=
+ HOOK_CHAIN=no
+ RENEW_DAYS=30
+ KEYSIZE=4096
+ WELLKNOWN=
+ PRIVATE_KEY_RENEW=yes
+ PRIVATE_KEY_ROLLOVER=no
+ KEY_ALGO=rsa
+ OPENSSL=openssl
+ OPENSSL_CNF=
+ CONTACT_EMAIL=
+ LOCKFILE=
+ OCSP_MUST_STAPLE=no
+ OCSP_FETCH=no
+ OCSP_DAYS=5
+ IP_VERSION=
+ CHAINCACHE=
+ AUTO_CLEANUP=no
+ DEHYDRATED_USER=
+ DEHYDRATED_GROUP=
+ API=auto
+ [[ -z /etc/dehydrated/config ]]
+ [[ -f /etc/dehydrated/config ]]
+ echo '# INFO: Using main config file /etc/dehydrated/config'
# INFO: Using main config file /etc/dehydrated/config
++ dirname /etc/dehydrated/config
+ BASEDIR=/etc/dehydrated
+ . /etc/dehydrated/config
++ CONFIG_D=/etc/dehydrated/conf.d
++ BASEDIR=/var/lib/dehydrated
++ WELLKNOWN=/var/lib/dehydrated/acme-challenges
++ DOMAINS_TXT=/etc/dehydrated/domains.txt
+ [[ -n /etc/dehydrated/conf.d ]]
+ [[ ! -d /etc/dehydrated/conf.d ]]
+ [[ -n '' ]]
+ set +f
+ [[ -n '' ]]
+ set -f
+ [[ -n '' ]]
+ [[ -n '' ]]
+ check_dependencies
+ openssl version
+ _sed ''
+ command -v grep
+ command -v mktemp
+ command -v diff
+ set +e
++ curl -V
++ head -n1
++ awk '{print $2}'
+ CURL_VERSION=7.58.0
+ retcode=0
+ set -e
+ [[ ! 0 = \0 ]]
+ [[ /var/lib/dehydrated != \/ ]]
+ BASEDIR=/var/lib/dehydrated
+ [[ -d /var/lib/dehydrated ]]
+ [[ -z '' ]]
+ [[ https://acme-v02.api.letsencrypt.org/directory = \h\t\t\p\s\:\/\/\a\c\m\e\-\v\0\2\.\a\p\i\.\l\e\t\s\e\n\c\r\y\p\t\.\o\r\g\/\d\i\r\e\c\t\o\r\y ]]
+ OLDCA=https://acme-v01.api.letsencrypt.org/directory
++ echo https://acme-v02.api.letsencrypt.org/directory
++ urlbase64
++ openssl base64 -e
++ tr -d '\n\r'
++ _sed -e 's:=*$::g' -e y:+/:-_:
++ [[ Linux = \L\i\n\u\x ]]
++ sed -r -e 's:=*$::g' -e y:+/:-_:
+ CAHASH=aHR0cHM6Ly9hY21lLXYwMi5hcGkubGV0c2VuY3J5cHQub3JnL2RpcmVjdG9yeQo
+ [[ -z '' ]]
+ ACCOUNTDIR=/var/lib/dehydrated/accounts
+ [[ ! -e /var/lib/dehydrated/accounts/aHR0cHM6Ly9hY21lLXYwMi5hcGkubGV0c2VuY3J5cHQub3JnL2RpcmVjdG9yeQo ]]
+ [[ -f /var/lib/dehydrated/accounts/aHR0cHM6Ly9hY21lLXYwMi5hcGkubGV0c2VuY3J5cHQub3JnL2RpcmVjdG9yeQo/config ]]
+ ACCOUNT_KEY=/var/lib/dehydrated/accounts/aHR0cHM6Ly9hY21lLXYwMi5hcGkubGV0c2VuY3J5cHQub3JnL2RpcmVjdG9yeQo/account_key.pem
+ ACCOUNT_KEY_JSON=/var/lib/dehydrated/accounts/aHR0cHM6Ly9hY21lLXYwMi5hcGkubGV0c2VuY3J5cHQub3JnL2RpcmVjdG9yeQo/registration_info.json
+ ACCOUNT_ID_JSON=/var/lib/dehydrated/accounts/aHR0cHM6Ly9hY21lLXYwMi5hcGkubGV0c2VuY3J5cHQub3JnL2RpcmVjdG9yeQo/account_id.json
+ [[ -f /var/lib/dehydrated/private_key.pem ]]
+ [[ -f /var/lib/dehydrated/private_key.json ]]
+ [[ -z '' ]]
+ CERTDIR=/var/lib/dehydrated/certs
+ [[ -z '' ]]
+ ALPNCERTDIR=/var/lib/dehydrated/alpn-certs
+ [[ -z '' ]]
+ CHAINCACHE=/var/lib/dehydrated/chains
+ [[ -z /etc/dehydrated/domains.txt ]]
+ [[ -z /var/lib/dehydrated/acme-challenges ]]
+ [[ -z '' ]]
+ LOCKFILE=/var/lib/dehydrated/lock
+ [[ -z '' ]]
++ openssl version -d
++ cut '-d"' -f2
+ OPENSSL_CNF=/usr/lib/ssl/openssl.cnf
+ [[ -n '' ]]
+ [[ -n '' ]]
+ [[ -n '' ]]
+ [[ -n '' ]]
+ [[ -n '' ]]
+ [[ -n '' ]]
+ [[ -n '' ]]
+ [[ -n '' ]]
+ [[ -n '' ]]
+ '[' '!' '' = noverify ']'
+ verify_config
+ [[ http-01 == \h\t\t\p\-\0\1 ]]
+ [[ http-01 = \d\n\s\-\0\1 ]]
+ [[ http-01 = \h\t\t\p\-\0\1 ]]
+ [[ ! -d /var/lib/dehydrated/acme-challenges ]]
+ [[ rsa == \r\s\a ]]
+ [[ -n '' ]]
+ [[ auto == \a\u\t\o ]]
+ [[ 5 =~ ^[0-9]+$ ]]
+ store_configvars
+ __KEY_ALGO=rsa
+ __OCSP_MUST_STAPLE=no
+ __PRIVATE_KEY_RENEW=yes
+ __KEYSIZE=4096
+ __CHALLENGETYPE=http-01
+ __HOOK=
+ __WELLKNOWN=/var/lib/dehydrated/acme-challenges
+ __HOOK_CHAIN=no
+ __OPENSSL_CNF=/usr/lib/ssl/openssl.cnf
+ __RENEW_DAYS=30
+ __IP_VERSION=
+ [[ -n /var/lib/dehydrated/lock ]]
++ dirname /var/lib/dehydrated/lock
+ LOCKDIR=/var/lib/dehydrated
+ [[ -w /var/lib/dehydrated ]]
+ trap remove_lock EXIT
++ http_request get https://acme-v02.api.letsencrypt.org/directory
+++ _mktemp
+++ mktemp /tmp/dehydrated-XXXXXX
++ tempcont=/tmp/dehydrated-VulQhV
+++ _mktemp
+++ mktemp /tmp/dehydrated-XXXXXX
++ tempheaders=/tmp/dehydrated-o9OFO6
++ [[ -n '' ]]
++ set +e
++ [[ get = \h\e\a\d ]]
++ [[ get = \g\e\t ]]
+++ curl -A 'dehydrated/0.6.5 curl/7.58.0' -L -s -w '%{http_code}' -o /tmp/dehydrated-VulQhV -D /tmp/dehydrated-o9OFO6 https://acme-v02.api.letsencrypt.org/directory
++ statuscode=503
++ curlret=0
++ set -e
++ [[ ! 0 = \0 ]]
++ [[ ! 5 = \2 ]]
++ [[ auto = \1 ]]
++ [[ -n '' ]]
++ echo '  + ERROR: An error occurred while sending get-request to https://acme-v02.api.letsencrypt.org/directory (Status 503)'
  + ERROR: An error occurred while sending get-request to https://acme-v02.api.letsencrypt.org/directory (Status 503)
++ echo

++ echo Details:
Details:
++ cat /tmp/dehydrated-o9OFO6
HTTP/2 503 
server: nginx
date: Sat, 29 Feb 2020 05:21:40 GMT
content-type: application/problem+json
content-length: 178
etag: "5d9e23f1-b2"

++ cat /tmp/dehydrated-VulQhV
{
  "type": "urn:acme:error:serverInternal",
  "detail": "The service is down for maintenance or had an internal error. Check https://letsencrypt.status.io/ for more details."
}
++ echo

++ echo

++ [[ -n '' ]]
++ rm -f /tmp/dehydrated-VulQhV
++ rm -f /tmp/dehydrated-o9OFO6
++ [[ sign_domains = \s\i\g\n\_\d\o\m\a\i\n\s ]]
++ [[ -n test.org ]]
++ [[ -n /etc/dehydrated/domains.txt ]]
++ rm /etc/dehydrated/domains.txt
++ exit 1
+ CA_DIRECTORY=
+ remove_lock
+ rm -f /var/lib/dehydrated/lock
txr13 commented 4 years ago

I found the error.

command_sign_domains calls init_system first, and checks for PARAM_DOMAIN afterward (creating a temp file if present).

In init_system, line 298, get CA URLs by calling http_request.

In http_request, if the CA is experiencing a temporary issue, we fail through to lines 559-581, where we remove the "temporary" DOMAINS_TXT file. But at this point in the chain, we have not yet created a temporary domains file, and so the actual domains.txt is removed instead.

This is a bug, but it's an extremely specific bug that would only ever be seen if you were using the -c argument, and the -d argument, and had an actual domains.txt file as well, and the CA experienced a temporary issue during the initial request for the directory URLs. Tricky!

usev6 commented 4 years ago

Since I've wondered if this might become a problem for my setup, I've looked at the related code. If I'm not mistaken the bug was introduced a long time ago when the call to ìnit_system was added before the temporary domains.txt was defined.

However, from the perspective of separation of concerns it seems strange to have http_request taking care of cleaning up a temporary domains.txt file. Maybe it would be best to just remove these lines: https://github.com/dehydrated-io/dehydrated/blob/dbb0ef1ce1/dehydrated#L577-L578. I guess there are other code pathes that let the script exit without hitting one of the two statements that remove the temporary domains.txt file.

Instead the cleanup of the temporary domains.txt could perhaps be done with a trap. (I'm aware of this suggestion being a bit handwavy ;-) E.g. there already is a trap command in init_system, so this would need some coordination.)