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

Add option to explicitly delegate FQDN's #27

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

TimeToogo
Copy link

@TimeToogo TimeToogo commented Apr 27, 2023

Following on from #24 I ended up implementing a workaround for this issue that exposes a new argument which can be used to delegate FQDN's to other nameservers. This has enabled me to successfully issue wildcard letsencrypt certificates on the sslip domain. Here's how I use it:

sslip.io-dns-server -delegate _acme-challenge.sslip.example.domain=ns-XXX.awsdns-XX.com.

Where Route53 has a hosted zone for sslip.example.domain and will answer authoritatively for _acme-challenge.sslip.example.domain, which enables certbot to pass the acme dns-01 challenge. You could likely substitute Route53 for other nameservers using this approach although I haven't tested any others.
I'm not sure if this is something you want to bring into the mainline but thought it was worth checking.

NOTE: I learnt the hard way that the dns01 challenge issues mixed case queries, hence the case-insensitive match.

@cunnie
Copy link
Owner

cunnie commented May 2, 2023

Thanks for the contribution, @TimeToogo. It might take me a while to get to this PR, but I'm intrigued by your idea.

@cunnie
Copy link
Owner

cunnie commented Jul 10, 2023

Hi @TimeToogo , thanks in advance for the PR. Here are some changes I'd like to see:

  • Squash your commits. I'd like to see one self-contained commit, not three, especially when one of the commit is a fix on an earlier commit (i.e. "fix binding")
  • Commit message should describe the "why". Your commit messages describe the "what" (e.g. "Add option to explicitly delegate FQDN's") but the "what" isn't as important as the "why". What problem are we trying to fix? For inspiration, look at some of the commit messages in the Linux kernel. Here's an example. Note: I appreciate how hard it is to write a good commit message—it takes effort to be clear and concise.
  • Use consistent argument parsing (see comment in code).
  • Write tests. Here's the file you want to write your tests in. Pattern it after the -addresses tests.
  • Documentation: this page needs to be updated to describe how to use your feature to procure a wildcard certificate.

Thanks for putting the time to make sslip.io better. —Brian

@@ -17,6 +17,7 @@ func main() {
var wg sync.WaitGroup
var blocklistURL = flag.String("blocklistURL", "https://raw.githubusercontent.com/cunnie/sslip.io/main/etc/blocklist.txt", `URL containing a list of "forbidden" names/CIDRs`)
var nameservers = flag.String("nameservers", "ns-aws.sslip.io.,ns-azure.sslip.io.,ns-gce.sslip.io.", "comma-separated list of nameservers")
var delegated = flag.String("delegate", "", "delegate specific FQDN's to comma-separated list of nameservers, seperate multiple mappings with ';'")
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"separate" is spelled with one "e". It was spelled correctly the first time, incorrectly the second.

Using ";" as a separator is tricky because it's a meaningful character to the sh/bash/zsh shells—it may trip up an unsophisticated user.

Also, the pre-existing -addresses flag uses a different way to set multiple records (= and ,). Is there a reason you chose to use a completely different paradigm (, and ;)?

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.

2 participants