terricain / certbot-dns-azure

AzureDNS Certbot plugin
Other
33 stars 16 forks source link

Use dns zone if available in resource ID #18

Closed mdhowle closed 1 year ago

mdhowle commented 2 years ago

We have a subdomain, acme.example.com, in Azure DNS that handles the DNS validation. The DNS zone for example.com is hosted outside of Azure DNS. It has a CNAME that points to acme.example.com: _acme-challenge.foo.example.com => _acme-challenge.foo.acme.example.com that will be followed by the ACME server to validate (DNS aliasing).

The current implementation of certbot-dns-azure does not take this setup into account. Registering foo.example.com with the following configuration will result in certbot-dns-azure writing to a non-existent Azure DNS zone for example.com

dns_azure_zone1 = example.com:/subscriptions/<subid>/resourceGroups/<rgname>

Also, adjusting the domain name prefix to acme.example.com:/subscriptions/... leads to an error in the plugin: Domain foo.example.com does not have a valid domain to resource group id mapping. This makes sense because foo.example.com does not end with acme.example.com. Removing this check would probably fix it in my case, but it's also not the most common case.

With this PR, one can supply the full resource ID with the DNS zone in the configuration file as displayed in Azure DNS. image

dns_azure_zone1 = example.com:/subscriptions/<subid>/resourceGroups/<rgname>/providers/Microsoft.Network/dnszones/acme.example.com

The code will then attempt to index the DNS zone from resource ID. In this instance, acme.example.com will be returned. Otherwise, if a resource ID without the dnszone is supplied, it will return example.com as defined in the domain prefix and continue to work as before as currently documented.

terricain commented 1 year ago

Looks good, three things:

mdhowle commented 1 year ago
  • Can we check if Microsoft.Network/dnszones is in resource_group variable and then split, otherwise if one comes back to that code in 6 months, no-one'll remember what the parts of that split are.

Agreed. I've added the check.

Alternatively, I've used this function in other projects to parse Azure resource IDs.

from azure.core.utils import CaseInsensitiveDict

def parse_azure_resource_id(resource_id: str):
    if resource_id.startswith('/'):
        resource_id = resource_id[1:]

    if resource_id.endswith('/'):
        resource_id = resource_id[:-1]

    if '/' not in resource_id:
        raise ValueError('Invalid resource ID')

    parts = resource_id.split('/')
    if (len(parts) % 2) != 0 or '' in parts:
        raise ValueError('Invalid resource ID')
    return CaseInsensitiveDict(zip(parts[0::2], parts[1::2]))

r1 = parse_azure_resource_id("/subscriptions/c135abce-d87d-48df-936c-15596c6968a5/resourceGroups/dns1")
r2 = parse_azure_resource_id("/subscriptions/c135abce-d87d-48df-936c-15596c6968a5/resourceGroups/dns2/providers/Microsoft.Network/dnszones/acme.example.com")

print(r1)
{'subscriptions': 'c135abce-d87d-48df-936c-15596c6968a5', 'resourceGroups': 'dns1'}

print(r2) 
{'subscriptions': 'c135abce-d87d-48df-936c-15596c6968a5', 'resourceGroups': 'dns2', 'providers': 'Microsoft.Network', 'dnszones': 'acme.example.com'}

"dnszones" in r1  # False
r1.get("subscriptions")  # c135abce-d87d-48df-936c-15596c6968a5
r1.get("resourceGroups")  # dns1
r1.get("dnsZones")  # None

"dnszones" in r2  # True
r2.get("dnsZones")  # acme.example.com
terricain commented 1 year ago

Nice that function looks good. Excellent use of case insensitive dict too.

PR looks good, happy to wait if you want to use that function instead though, i reckon it'll look nicer :smile:

terricain commented 1 year ago

Nice that looks great, will give it a test over the weekend on my tenant and then do a release

vidarw commented 1 year ago

@terrycain Would it be possible to ask for this to be released to a new version at pypi anytime soon so it works in the version installed by pip3?

terricain commented 1 year ago

Hey, is on my todo list, hopefully get it done soon :)

vidarw commented 1 year ago

@mdhowle @terrycain I tried to install the version from GitHub master locally pip3 install -e ..

Given CNAME from _acme-challenge.site1.example.com to _acme-challenge.site1.certs.example.com. Given azure_dns_zone1 to example.com:/subscriptions/e15007b0-9d6d-4637-8273-ca788df20000/resourceGroups/dns-rg/providers/Microsoft.Network/dnszones/certs.example.com

As far as I can understand the version in the master branch creates the TXT lookup under _acme-challenge.site1.example.com.certs.example.com to verify. Is this the intended design? Based on the documentation _acme-challenge.site1.certs.example.com would make sense to me.

mdhowle commented 1 year ago

The documentation is incorrect. Using your example, if you are requesting a certificate for site1.example.com using certs.example.com for DNS delegation of example.com, the CNAME record on example.com needs to be _acme-challenge.site1.example.com = _acme-challenge.site1.example.com.certs.example.com.

The TXT record certbot creates would be _acme-challenge.site1.example.com on certs.example.com.

I'll create a new PR to fix the documentation. Thank you for pointing this out.