Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use dns zone if available in resource ID #18

Merged
merged 6 commits into from
Mar 7, 2023

Conversation

mdhowle
Copy link
Contributor

@mdhowle mdhowle commented May 31, 2022

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
Copy link
Owner

Looks good, three things:

  • 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.
  • Can you update the docs with an example of using it with a bit of explanation so that others know it can be done and under what circumstances you'll need to use it?
  • Can you add a test to test this scenario

@terricain terricain added the enhancement New feature or request label Mar 5, 2023
@mdhowle
Copy link
Contributor Author

mdhowle commented Mar 6, 2023

  • 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
Copy link
Owner

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 😄

@terricain
Copy link
Owner

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

@terricain terricain merged commit 0b158bb into terricain:master Mar 7, 2023
@mdhowle mdhowle deleted the dns-alias branch March 12, 2023 02:47
@vidarw
Copy link

vidarw commented Mar 20, 2023

@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
Copy link
Owner

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

@vidarw
Copy link

vidarw commented Mar 23, 2023

@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 mdhowle restored the dns-alias branch March 23, 2023 15:36
@mdhowle mdhowle deleted the dns-alias branch March 23, 2023 15:37
@mdhowle
Copy link
Contributor Author

mdhowle commented Mar 23, 2023

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants