antevens / letsencrypt-freeipa

Scripts to automate installation, configuration and renewal of LetsEncrypt certificates on FreeIPA Servers.
MIT License
59 stars 17 forks source link

Several minors #4

Closed rudgab closed 5 years ago

rudgab commented 6 years ago

Hi,

your code (/usr/sbin/renew_letsencrypt_cert.sh has the following bug: If your domain consists on 4 parts like host.sub.example.com then the line dns_domain_name="$(echo ${host} | awk -F. '{OFS="."; print $(NF-1), $NF; }')" will fail and in this case should be dns_domain_name="$(echo ${host} | awk -F. '{OFS="."; print $(NF-2),$(NF-1), $NF; }')"

So maybe the evaluation should be changed to something more stable. Also host is evaluated from a fqdn hostname which is not always the case and maybe has to be corrected, because at least kerberos keytabs are often done with fqdn host/....

We don't use a freeipa integrated dns because our DNS environment is more complicated and is not managable by the freeipa one. We use nsupdate for zones and then your code maybe work with the following (in our case we are using it in this way):

certbot certonly --manual \ --preferred-challenges dns \ --manual-public-ip-logging-ok \ --manual-auth-hook 'echo -e "server your-dns-server\nupdate delete _acme-challenge.${CERTBOT_DOMAIN} TXT\nupdate add _acme-challenge.${CERTBOT_DOMAIN} 320 IN TXT \"${CERTBOT_VALIDATION}\"\nsend" | nsupdate -v -y"nsupdate-key"' \ ${domain_args} \ --agree-tos \ --email "${email}" \ --expand \ -n

Regards,

Rudi G.

antevens commented 5 years ago

Thanks for the bug report and sorry for the late reply, GitHub notifications were turned off.

  1. I fixed the code to get the proper domain from the host.
  2. Added a more generic nsupdate implementation including a couple of customization hooks.

Authorization can be set using NSUPDATE_KEY_NAME and NSUPDATE_KEY_SECRET or NSUPDATE_KEY_FILE, the authorization hook itself can be customized using AUTH_HOOK.

If you've got time check out the newly created develop branch and let me know what you think, note that this is untested at this time!

antevens commented 5 years ago

These changes are now merged into master, please test at your own leisure and if you still have issues feel free to re-open this issue.