Dev - Githubissues

AlvinSchiller commented 2 years ago

I implemented the fixes of the review, but these have a few implications.

as the hardcoded prefix for the matching must be removed, the SELFHOSTDNS_MAP must contain the fulldomain including the prefix ("_acme-challenge." or any other for DNS alias mode)
SELFHOSTDNS_RID and SELFHOSTDNS_RID2 variables have been removed as suggested. All entries must be contained in SELFHOSTDNS_MAP. Up to two RIDs per fulldomain are supported but at least one must be set, e.g. prefix.sub.domain.net:<RID>:<RID2> prefix.sub2.domain.net:<RID> prefix.sub3.domain.net:<RID>:<RID2>
SELFHOSTDNS_LAST_SLOT has been removed. For wildcard support the last used RID per domain will be stored internally (SELFHOSTDNS_MAP_LAST_USED_INTERNAL) to switch between them (if two RIDs are defined). The domain and last used RID will be stored in the format ";fulldomainA:lastUsedRidForFulldomainA;;fulldomainB:lastUsedRidForFulldomainB;;fulldomainC:lastUsedRidForFulldomainC;"

The usage of only SELFHOSTDNS_MAP has the sideeffect that now the issuing of multiple wildcard domain in one cert is supported, e.g. ... -d sub.example.com -d *.sub.example.com -d sub2.example.com -d *.sub2.example.com ...

Please review, as these changes probably break your supported plugins.

As the user has to add the TXT entries with the fulldomain incl. prefix manually anyway, i hope this is not a big drawback

Marvo2011 commented 2 years ago

Thanks. Lot of plugins must be recreated, because it was designed for SELFHOSTDNS_RID and SELFHOSTDNS_RID2. But no problem. Have you tested if there is a variable with the actual needed prefix. I think users can be run in some trouble when he don't know with prefix should be set / created in selfhost.

Marvo2011 commented 2 years ago

I think we can get the prefix with this: $(echo "$fulldomain" | sed -n -E "s/(^[^.]+)/\1:/p" | cut -d: -f1)

So the mapping can without the domain. But we should display it in the log that the user can create the right records in selfhost.

AlvinSchiller commented 2 years ago

Thanks. Lot of plugins must be recreated, because it was designed for SELFHOSTDNS_RID and SELFHOSTDNS_RID2. But no problem. Have you tested if there is a variable with the actual needed prefix. I think users can be run in some trouble when he don't know with prefix should be set / created in selfhost.

There isn't any variable for the prefix. The log prints the fulldomain which was used, before adding and if the adding failed (e.g. due to a missing entry in the MAP), so the user can see the needed name:

[...] Adding txt value: XXX for domain: _acme-challenge.test.example.de [...] Calling acme-dns on selfhost [...] SELFHOSTDNS_MAP must contain the fulldomain incl. prefix and at least one RID [...] Error add txt for domain:_acme-challenge.test.example.de`

The same in domain alias mode (but here the fulldomain of the alias 'myalias.test.example.de' has to be specified on calling acme.sh as parameter anyway):

[...] Adding txt value: XXX for domain: myalias.test.example.de [...] Calling acme-dns on selfhost [...] SELFHOSTDNS_MAP must contain the fulldomain incl. prefix and at least one RID [...] Error add txt for domain:myalias.test.example.de`

Is this sufficient in your opinion?

AlvinSchiller commented 2 years ago

I think we can get the prefix with this: $(echo "$fulldomain" | sed -n -E "s/(^[^.]+)/\1:/p" | cut -d: -f1)

So the mapping can without the domain. But we should display it in the log that the user can create the right records in selfhost.

Yes that would give us the prefix, but the prefix can't be just cut from fulldomain for the MAP lookup. This would probably work for the "_acme-challenge." prefix alone, but not for domain alias mode or mixed mode, where the prefix can be any subdomain. There could be ambiguous matches in the MAP, if mutiple domain alias differ only in the prefix.

e.g.: acme.sh --issue -d test1.example.de --domain-alias test1.validation.example.net -d test2.example.de --domain-alias test2.validation.example.net -d test3.example.de --challenge-alias _acme-challenge.validation.example.net

With prefix the MAP would look like this: SELFHOSTDNS_MAP=test1.validation.example.net:<RID1> test2.validation.example.net:<RID2> _acme-challenge.validation.example.net:<RID3> All alias are cleary associated with the defined RIDs, which the user added on his domain

If we would always cut the prefix programmatically for lookup, all entries would match and the MAP had to look like this: SELFHOSTDNS_MAP=validation.example.net:<RID>:<RID2>:<RID3> The problem is that now we cannot match the correct RID to the correct prefixed domain.

Marvo2011 commented 2 years ago

@AlvinSchiller Yes, you are right. I didn't know about the alias mode and mixed mode. I think the log output is fine. I will edit the existing plugins and merge it soon.

Marvo2011 commented 2 years ago

@AlvinSchiller I think we have truble with the last used RID on ubuntu DNS Test fail https://github.com/acmesh-official/acme.sh/pull/3873#issuecomment-1228068921


[Fri Aug 26 05:36:41 UTC 2022] Calling acme-dns on selfhost
[Fri Aug 26 05:36:41 UTC 2022] fulldomain='acmetestXyzRandomName.***'
[Fri Aug 26 05:36:41 UTC 2022] txtvalue='acmeTestTxtRecord'
[Fri Aug 26 05:36:41 UTC 2022] config file is empty, can not read ***_LAST_USED_INTERNAL
[Fri Aug 26 05:36:41 UTC 2022] mapEntry='acmetestXyzRandomName.***:XXXXXX1:XXXXXX2'
[Fri Aug 26 05:36:41 UTC 2022] lastUsedRidForDomainEntry
[Fri Aug 26 05:36:41 UTC 2022] Trying to add acmeTestTxtRecord on selfhost for rid: XXXXXX1
[Fri Aug 26 05:36:41 UTC 2022] GET
[Fri Aug 26 05:36:41 UTC 2022] url='https://selfhost.de/cgi-bin/api.pl?username=***&***'
[Fri Aug 26 05:36:41 UTC 2022] timeout=
[Fri Aug 26 05:36:41 UTC 2022] _CURL='curl --silent --dump-header /root/.acme.sh/http.header  -L '
[Fri Aug 26 05:36:43 UTC 2022] ret='0'
[Fri Aug 26 05:36:43 UTC 2022] config file is empty, can not save ***=***
[Fri Aug 26 05:36:43 UTC 2022] config file is empty, can not save ***_LAST_USED_INTERNAL=;acmetestXyzRandomName.***:XXXXXX1;
Run Failed

AlvinSchiller commented 2 years ago

@Marvo2011 I think there is a bug in the setup for this testcase. It just calles the dns skript without the issuing process. Therefore it seems that the domain.conf isn't initialized, which produces this error. I tested on ubuntu myself and all manuell test worked well.

AlvinSchiller commented 2 years ago

@Marvo2011 The Testcase is fixed. Please rerun.

Edit: As first try for a fix i changed the separetor of the Internal MAP entries to Space (equivalent to the export MAP). In the end this was not needed, but brings better readability in debug mode and the conf file. Maybe have a look if you would like to merge this too: https://github.com/AlvinSchiller/acme.sh/commit/f9320fff8ff71c9bc7a4d228bb8943ed9a1ae7eb

Marvo2011 commented 2 years ago

Yes, please create a PR.

Marvo2011 / acme.sh

Dev #6