acme.sh
acme.sh copied to clipboard
Acme2 similar names
When using ACME2 with an order that contains multiple similar names (e.g. "api.example.com" and "api-example.com") the code that looks up the authorization state for a particular name can end up looking at the wrong entry due to the way grep was used without escaping the "." placeholder. I.e. the grep can end up returning multiple map entries:
[Mon 28 May 2018 15:48:27 NZST] Getting webroot for domain='video.alpha.example.com'
[Mon 28 May 2018 15:48:27 NZST] _w='dns_aws'
[Mon 28 May 2018 15:48:27 NZST] _currentRoot='dns_aws'
[Mon 28 May 2018 15:48:27 NZST] response='{"identifier":{"type":"dns","value":"video-alpha.example.com"},"status":"valid","expires":"2018-06-27T03:35:25Z","challenges":[{"type":"http-01","status":"pending","url":"https://acme-v02.api.letsencrypt.org/acme/challenge/WZ0RBOI0habN9pVqLX1G_h-TjTTcdjm5NRK1tlK_50o/4844224903","token":"aVdLaEuvhoAy7d6MHfwKT8eaz9q7afx7LqSmQCfqNLo"},{"type":"dns-01","status":"valid","url":"https://acme-v02.api.letsencrypt.org/acme/challenge/REDACTED/REDACTED","token":"REDACTED","validationRecord":[{"hostname":"video-alpha.example.com"}]}]}
{"identifier":{"type":"dns","value":"video.alpha.example.com"},"status":"pending","expires":"2018-06-04T03:34:35Z","challenges":[{"type":"dns-01","status":"pending","url":"https://acme-v02.api.letsencrypt.org/acme/challenge/REDACTED/REDACTED","token":"cdhmmNBqh9QWH1Dq6pDAEbZfUyeIQw6pHeETndepde4"},{"type":"http-01","status":"pending","url":"https://acme-v02.api.letsencrypt.org/acme/challenge/REDACTED/REDACTED","token":"REDACTED"}]}'
If one of these authorizations is valid while another is still pending, acme.sh will erroneously consider both of them valid, skip authorization, and then fail to issue the cert.
The dns_aws change is unrelated, but I found it while debugging this issue.
Hi, @ksperling Thanks for your PR, it's indeed a bug.
The following 2 case can both grep successfully:
root@localhost# echo "api.example.com" | grep "api.example.com"
api.example.com
root@localhost# echo "api-example.com" | grep "api.example.com"
api-example.com
The reason is that the dot('.') in the grep expression is explained as a regExp wildcard char, so it matches any char.
It seems that your code can not work for this case also.
I think we need to think about another fix.
Thanks. -Neil
I'm not sure what you mean, the fact that "." is a regex meta character is exactly what my patch fixes by escaping each "." in the domain name as "\." before passing the string to grep.
@ksperling can you please try these code ?
root@st:~# d=api.example.com
root@st:~# echo "api.example.com" | grep "^${d//./\.}"
api.example.com
root@st:~# echo "api-example.com" | grep "^${d//./\.}"
api-example.com
There is no difference.
Ah, apologies, when I read your initial response on github somehow the markdown parser must have swallowed some of the backslashes. Just tried out your code, this seems to be a difference in behaviour between GNU grep and BSD grep:
karsten@qualifier:~$ grep --version; echo -e "api.example.com\napi-example.com" | grep "^${d//./\.}"
grep (BSD grep) 2.5.1-FreeBSD
api.example.com
karsten@deep4:~$ grep --version; echo -e "api.example.com\napi-example.com" | grep "^${d//./\.}"
grep (GNU grep) 2.25
Copyright (C) 2016 Free Software Foundation, Inc.
License GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html>.
This is free software: you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law.
Written by Mike Haertel and others, see <http://git.sv.gnu.org/cgit/grep.git/tree/AUTHORS>.
api.example.com
api-example.com
I'll see what I can do to make this work with GNU grep as well... I had assumed escaping with backslash was defined by POSIX so didn't think to test multiple platforms.
Actually the issue seem to be with the pattern replacement, not grep itself, the following works on both platforms (extra backslash in the replacement):
grep --version; d=api.example.com; echo -e "api.example.com\napi-example.com" | grep "^${d//./\\.}"
I'll amend the pull request.
Could you please have a look at the updated PR? Cheers Karsten
please merge the latest code.