icing / mod_md

Let's Encrypt (ACME) in Apache httpd
https://icing.github.io/mod_md/
Apache License 2.0
334 stars 28 forks source link

WIP: Allow MDChallengeDns01 directive globally or by domain #306

Closed bilhackmac closed 1 year ago

bilhackmac commented 1 year ago

Allow to define MDChallengeDns01 for a domain set or fallback to global one.

MR status :

Fix #305

icing commented 1 year ago

Thanks for the PR. Merged it via #307 with a test and some fixes.

Please verify if this works in your environment.

bilhackmac commented 1 year ago

Oh thanks !!

I had planned to continue working on it this week.

I'll test it as soon as I can

bilhackmac commented 1 year ago

It works great !

Thanks

bilhackmac commented 1 year ago

After some deeper test I found a little mistake in store and I don't understand why, but teardown is not called anymore

icing commented 1 year ago

It might be the effect of the JSON mistakes you found. Could you verify it that resolves the issue?

bilhackmac commented 1 year ago

No it don't fix.

When I correct md_core.c, md.json is fixed from

{
  …
  "proto": {
    "acme-tls/1": []
  },
  "cmd-dns-01": "bin/acme-dns01-ovh tld /auth-ovh"
}

to

{
  …
  "proto": {
    "acme-tls/1": []
  },
  "stapling": true,
  "cmd-dns-01": "bin/acme-dns01-ovh tld /auth-ovh"
}

But teardown not called anymore, even after server restart

icing commented 1 year ago

Hmm, a LogLevel md:trace2 could reveal what is happening, e.g. if the function cha_dns_01_teardown() is called and what it find the challenge command to be.

bilhackmac commented 1 year ago

Relevant part

2023-02-28 11:18:00.698373 md:debug - order teardown setup tls-alpn-01:www.example.com

https://github.com/icing/mod_md/blob/master/src/md_acme_order.c#L241 setup_token is identified as tls-alpn-01 instead dns-01

bilhackmac commented 1 year ago

Found !

https://github.com/icing/mod_md/blob/master/src/md_acme_authz.c#L637

It's

- challenge_setup = CHA_TYPES[i].name;
+ challenge_setup = CHA_TYPES[j].name;
icing commented 1 year ago

Nice catch!

icing commented 1 year ago

Fixed it in master. Is that all we needed?

bilhackmac commented 1 year ago

There remains the review on #307 and we're all good

icing commented 1 year ago

Thanks, added now.