acmesh-official / acme.sh

A pure Unix shell script implementing ACME client protocol
https://acme.sh
GNU General Public License v3.0
39.21k stars 4.95k forks source link

Update dns api to support v2 wildcard cert #1261

Open Neilpang opened 6 years ago

Neilpang commented 6 years ago

To support v2 wildcard cert, we need to add 2 txt records for the same domain. for example:

_acme-challenge.example.com   TXT   "this is txt value 1"
_acme-challenge.example.com   TXT   "this is txt value 2"

In many dns api hooks, in the dns_xx_add() function, they try to UPDATE the existing txt record, instead of ADD a new record. This was a good practice for ACME v1, but it's not good in ACME v2.

In ACME v2, we just need to add new txt record all the time in the dns_xx_add() function, And in the the dns_xx_rm() function, we must delete the txt record according to the specified txt value.

Test example:

acme.sh  --issue --test  -d example.com  -d *.example.com

Please make sure this works, and the 2 txt records are removed after the cert is issued.

See my changes:

https://github.com/Neilpang/acme.sh/commit/ea25492c2823a4971b3d2eb28fa0dcd2b5105db9#diff-51fe23dd1a90a481487dbca5b9c3ae24

https://github.com/Neilpang/acme.sh/commit/72f54ca6c13c33943aafbacaee03423907f5e738#diff-d48ca70b90232acffb2b5b9d1ec2938a

https://github.com/Neilpang/acme.sh/commit/584fb2904b6b63cf13d71ba20fa184ade62730a0#diff-f272833bc0ccf326ea343539e829f1d3

Neilpang commented 6 years ago

@wpk- Please update the rm function of dns_ad hook.

Neilpang commented 6 years ago

@baiyangliu please support the rm function for dns_ali hook.

Neilpang commented 6 years ago

@martgras please make sure dns_azure hook works in this way.

Neilpang commented 6 years ago

@boyanpeychev please update dns_cloudns hook

boyanpeychev commented 6 years ago

@Neilpang please accept this pull request and I will proceed with the wildcard implementation: https://github.com/Neilpang/acme.sh/pull/1094

Neilpang commented 6 years ago

@boyanpeychev accepted, please go ahead.

Neilpang commented 6 years ago

@pho3nixf1re please update dns_dnsimple hook

Neilpang commented 6 years ago

@dkerr64 please update dns_freedns.sh

Neilpang commented 6 years ago

@fcrozat please update dns_gandi_livedns.sh

Neilpang commented 6 years ago

@justmwa please update dns_me.sh

Neilpang commented 6 years ago

@justmwa please also update dns_nsone.sh

Neilpang commented 6 years ago

@magna-z please update dns_pdns.sh

Neilpang commented 6 years ago

@Aarup please update dns_unoeuro.sh

drybalkadk commented 6 years ago

aws work. Thanks a lot

martgras commented 6 years ago

I don't think any changes are needed but will double check next week when I'm back


From: drybalkadk notifications@github.com Sent: Wednesday, February 14, 2018 4:49:57 PM To: Neilpang/acme.sh Cc: Martin Grasruck; Mention Subject: Re: [Neilpang/acme.sh] Update dns api to support v2 wildcard cert (#1261)

aws work. Thanks a lot

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHubhttps://github.com/Neilpang/acme.sh/issues/1261#issuecomment-365649473, or mute the threadhttps://github.com/notifications/unsubscribe-auth/AYjgXW3X7a88xrFvZiuuvKzzoVXrt3oaks5tUwCkgaJpZM4SDoO5.

Neilpang commented 6 years ago

@martgras thank you.

boyanpeychev commented 6 years ago

no changes needed for dns_cloudns.sh

Neilpang commented 6 years ago

@boyanpeychev There is an updating logic in the dns_cloudns_add() function. Are you sure no changes are needed there?

Please make sure this case is passing:

acme.sh  --issue --test  -d example.com  -d *.example.com
justmwa commented 6 years ago

Works as is because there's only one TXT sent from what I can see in logs. And since LE accepts it, they changed their mind about the double TXT record ?

justmwa commented 6 years ago

Message to those who think it works "as is", try with a subdomain ;) (unless it's not officially supported?) i.e foo.example.com. Actually it's only an issue with one API so far, need to dig further.

Neilpang commented 6 years ago

@justmwa no, never. please use a new domain to test.

acme.sh --issue  -d  sub.domain.com  -d *.sub.domain.com
martgras commented 6 years ago

Fixed the Azure hook only 1 DNS record was added: #1287

boyanpeychev commented 6 years ago

@Neilpang, yes, it is tested with completely new domain and sub-domain for Let's Encrypt and it works both for the root domain name and with a sub-domain. The records are added successfully.

However, I do see one problem not related to the cloudns.net api - sometimes the given token is not accepted by Let's Encrypt, even it is resolved absolutely properly. Here is example log: test.log

The TXT records requested to be added is with token: GUFN5HSXlmothHwsZWbCZW-SfgCaQYCTTB874fZ6Ijw The same token is found by Let's Encrypt, but reported as invalid. Please check you too.

martgras commented 6 years ago

I don't understand the cloudns api well enough but I suspect you run into the the same problem I had with Azure in #1287 Instead of creating 2 txt entries my code was first creating _acme-challenge.test.cloudns.biz. 10 IN TXT "DyLeOblvCb1I9DqZ2BlB0fdQYsA3or4WVdZN139KaHg" and then the second call to dns_azure_add changed this txt record to
_acme-challenge.test.cloudns.biz. 10 IN TXT "GUFN5HSXlmothHwsZWbCZW-SfgCaQYCTTB874fZ6Ijw"

therefore the validation failed Make sure you really create 2 entries instead of updating the existing one

Basically I have to use a Json body like this for the Azure DNS REST API call

{
    "properties": {
        "TTL": 10,
        "TXTRecords": [
            {   "value": ["DyLeOblvCb1I9DqZ2BlB0fdQYsA3or4WVdZN139KaHg"] },
            {   "value": ["GUFN5HSXlmothHwsZWbCZW-SfgCaQYCTTB874fZ6Ijw"]  }
        ]
    }
}

you should see something like this when checking with dig

dig -t txt _acme-challenge.test.cloudns.biz
_acme-challenge.test.cloudns.biz. 10 IN TXT "DyLeOblvCb1I9DqZ2BlB0fdQYsA3or4WVdZN139KaHg"
_acme-challenge.test.cloudns.biz. 10 IN TXT "GUFN5HSXlmothHwsZWbCZW-SfgCaQYCTTB874fZ6Ijw"
boyanpeychev commented 6 years ago

Thanks for the hint. Fixed. Pull request submitted.

Neilpang commented 6 years ago

@boyanpeychev It's not enough yet. In the rm() function, you must remove the record by the exact txt value. In the rm() function:

record_id=$(_dns_cloudns_get_record_id "$zone" "$host")

But the _dns_cloudns_get_record_id() function seems not considering the txt value.

martgras commented 6 years ago

@Neilpang the same is true for Azure but why does it matter?

first call to dns_azure_rm : DELETE https://management.azure.com/subscriptions/../dnszones/example.com/TXT/_acme-challenge.somesub1.test?api-version=2017-09-01 http response code 200

second call: DELETE https://management.azure.com/subscriptions/../dnszones/example.com/TXT/_acme-challenge.somesub1.test?api-version=2017-09-01 http response code 204

so yes the first call removes the record already but I don't see the problem with this approach

Neilpang commented 6 years ago

@martgras Yes, it doesn't matter if it's only you that use the domain to verify the cert. If there are multiple people validating the same domain, you may remove the txt domain from others. Especially we will soon merge the support for dns alias mode: https://github.com/Neilpang/acme.sh/wiki/DNS-alias-mode

So, it's strongly recommended that you only remove the txt record added by you.

Neilpang commented 6 years ago

@martgras In the dns azure api, the rm() function:

body="{\"properties\": {\"TTL\": 3600, \"TXTRecords\": [{\"value\": [\"$txtvalue\"]}]}}"

I think the txt value is considered.

boyanpeychev commented 6 years ago

@Neilpang can I use awk, if not, let me know what you prefer for the parse of JSON?

Neilpang commented 6 years ago

@boyanpeychev Sorry, I don' want to import anymore dependencies, like: awk.

https://github.com/Neilpang/acme.sh/wiki/DNS-API-Dev-Guide#5-process-the-api-response

See examples I process json: https://github.com/Neilpang/acme.sh/blob/dev/dnsapi/dns_ali.sh#L188

https://github.com/Neilpang/acme.sh/blob/dev/dnsapi/dns_gd.sh#L52

It may help you.

Thanks.

martgras commented 6 years ago

@Neilpang this is a pain - lots of ugly Json parsing but I hope I got it now once I get rid of my usual travis format errors

martgras commented 6 years ago

@Neilpang the txt value was not considered this body was never sent and would have been ignored anyways - was just a leftover when I copied the code from add. Azure_dns is finally ok now. BTW: I understand you don't want new dependencies and it makes sense. jq would have been my preference

boyanpeychev commented 6 years ago

@Neilpang cloudns api is updated and tested with multiple TXT records and it deletes only the proper records.

Tungstene commented 6 years ago

@fcrozat @Neilpang - Regarding Gandi Live DNS (http://doc.livedns.gandi.net/#work-with-domains), it seems this API isn't able to remove records by value, all we can do is "DELETE /domains/{DOMAIN}/records/{NAME}/TXT" (remove all TXT records for a name). For adding records, indeed, method POST should do a better job than method PUT (update a record).

A possible workaround would be to first generate a list of all TXT records for {NAME} (GET), edit list by removing TXT records to delete, erase all TXT records for {NAME} (DELETE) and finally add (POST) edited list. But I believe it's a lot of work for not so much of a benefit (and we couldn't get atomicity/locking on this process, especially if user choose a long duration for Le_DNSSleep).

dkerr64 commented 6 years ago

I am starting to look at this but running into a issue. from debug....

[Wed Mar  7 10:12:41 EST 2018] Using ACME_DIRECTORY: https://acme-v02.api.letsencrypt.org/directory
[Wed Mar  7 10:12:41 EST 2018] _init api for server: https://acme-v02.api.letsencrypt.org/directory
[Wed Mar  7 10:12:41 EST 2018] GET
[Wed Mar  7 10:12:41 EST 2018] url='https://acme-v02.api.letsencrypt.org/directory'
[Wed Mar  7 10:12:41 EST 2018] timeout=
[Wed Mar  7 10:12:41 EST 2018] _CURL='curl -L --silent --dump-header /mnt/kd/acme/http.header  -g '
[Wed Mar  7 10:12:42 EST 2018] Please refer to https://curl.haxx.se/libcurl/c/libcurl-errors.html for error code: 6
[Wed Mar  7 10:12:42 EST 2018] ret='6'
[Wed Mar  7 10:12:42 EST 2018] Can not init api.

I am using OpenDNS as my DNS service and it reports that there is no A record for this URL. Any thoughts?

Try entering acme-v02.api.letsencrypt.org into https://www.ultratools.com/tools/dnsLookupResult and get "no record found" for everything.

Neilpang commented 6 years ago

@dkerr64 please add --test

dkerr64 commented 6 years ago

Okay, thank you. That got me past that issue. The freedns method is NOT working with the v2 version... the code made assumption that only one TXT record would be created at a time. Therefore I will need to work on fixing it. Not sure how easy that will be, will let you know.

dkerr64 commented 6 years ago

@Neilpang support added for FreeDNS API, see PR #1343 Thank you.

rihadik commented 6 years ago

Would wildcard certs work in --webroot mode? Also, are partial wildcards supported? Like foo-*.example.com Thanks.

FernandoMiguel commented 6 years ago

No. Let's Encrypt wildcard certs will only be validated via dns01 method

-- Fernando Miguel

On 12 Mar 2018 05:00, "rihadik" notifications@github.com wrote:

Would wildcard certs work in --webroot mode?

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/Neilpang/acme.sh/issues/1261#issuecomment-372194146, or mute the thread https://github.com/notifications/unsubscribe-auth/AAKRrsI8JTYgcJMN74qw1iWk4lHR91z-ks5tdgDlgaJpZM4SDoO5 .

rihadik commented 6 years ago

What about through dns_aws? Is it supported or will it be?

FernandoMiguel commented 6 years ago

@rihadik from my quick testing using LE staging servers and acme v2, it worked without issues.

rihadik commented 6 years ago

Great, thank you. I think partials are currently not possible: --issue --dns dns_aws -d foo-*.example.com according to logs acme.sh contacts LE v01 API and naturally gets an error due to invalid characters in domain name. Probably acme.sh expects a domain to start with * and have nothing more to trigger wildcard logic. But there's such a thing as a partial wildcard, please see here: https://en.wikipedia.org/wiki/Wildcard_certificate#Examples Is this a acme.sh limitation or LE's?

boyanpeychev commented 6 years ago

@rihadik limitation should be from the both sides. There is no dns provider in the list, who can give you a TXT record for foo-*.example.com and I do not know about any CA who is providing such partial ssl certificates.

rihadik commented 6 years ago

What about en.*.example.com? It isn't a partial wildcard if you ask me, but it is also not supported. update: just checked, no, this form is specifically disallowed.

Neilpang commented 6 years ago

@rihadik I had fixed aws api for wildcard.

Letsencrypt only supports one style of wildcard: *.example.com. No other format is supported.

FernandoMiguel commented 6 years ago

--issue --dns dns_aws -d *.example.com --test should work. you cant combine partials with wildcard, i dont think any browser would support that

FernandoMiguel commented 6 years ago

And it's live https://community.letsencrypt.org/t/acme-v2-and-wildcard-certificate-support-is-live/55579

zloster commented 6 years ago

@Tungstene @Neilpang @fcrozat I've just succeeded to get a wildcard certificate with gandi_live_dns.sh. It's deployed here: https://try.test.edno.moe/ The "normal" site is here: https://www.edno.moe/

acme.sh installed from github today:

~/.acme.sh/*.test.edno.moe$ acme.sh -v
https://github.com/Neilpang/acme.sh
v2.7.7

Latest in the commit log is this:

commit 14bb60c61ffd83779e3e18fe5ab79ad328edb7db
Merge: 8ab8a6e 0f120c4
Author: neil <github@byneil.com>
Date:   Wed Mar 14 20:11:55 2018 +0800

    Merge pull request #1370 from anabis/patch-1

    fix syntax error missing space

P.S. Great project! Thank you all for the effort.