acme.sh icon indicating copy to clipboard operation
acme.sh copied to clipboard

Acme2 similar names

Open ksperling opened this issue 7 years ago • 7 comments

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.

ksperling avatar May 28 '18 04:05 ksperling

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

Neilpang avatar Jun 08 '18 10:06 Neilpang

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 avatar Jun 08 '18 11:06 ksperling

@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.

Neilpang avatar Jun 08 '18 13:06 Neilpang

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.

ksperling avatar Jun 09 '18 23:06 ksperling

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.

ksperling avatar Jun 09 '18 23:06 ksperling

Could you please have a look at the updated PR? Cheers Karsten

ksperling avatar Jul 04 '18 11:07 ksperling

please merge the latest code.

Neilpang avatar Aug 21 '18 13:08 Neilpang