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

freedns: fix domain splitting for multi-part domains #1454

Closed
wants to merge 7 commits into from

Conversation

frebib
Copy link

@frebib frebib commented Mar 27, 2018

It seems that the way domains were split before was using some unnecessarily complex maths which was broken for domains >3 parts (e.g _acme-challenge.test.domain.com)

I assume what the correct behaviour should be is the following:

sub_domain='_acme-challenge'
top_domain='everything.after.the.first.dot.com'

This PR fixes for domains >3 parts and still works for simple sub.domain.com domains

@martgras
Copy link
Contributor

Does that also work if I have separate zone for a subdomain.
Let's say I have example.com. I need hostnames like www.devorg.example.com mail.devorg.example.com
www.sales.example.com etc.
Of course I can create the records in the example.com (like www.devorg) but the devorg team wants to manage entries in their subdomain . For security reasons I don't want them to have control over anything else than devorg.example.com. The solution is creating a new DNS zone for devorg.example.com
This becomes more important with the DNS alias mode where I can use a subdomain for the validation records without the need to grant access to the complete example.com zone

@frebib
Copy link
Author

frebib commented Mar 28, 2018

I'm not entirely sure, are you able to test it?
Did it work before this patch?

@martgras
Copy link
Contributor

I never used freedns.
Most implenations start splitting at the first dot. Then check if the domain is in the list of domains from your provider.
If not split the domain again until you have a match.

@frebib
Copy link
Author

frebib commented Mar 28, 2018

This addresses the issue in #1086.

As for checking each part in turn, that will need some testing.
I'd suggest a separate issue should be opened to track that? It looks like it would be reasonably significant rewrite of the code in this function

@frebib frebib closed this Aug 21, 2018
frebib added a commit to frebib/acme.sh that referenced this pull request Aug 21, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants