[Feature Request] INWX: support null-target SRV

[rubgfx]

Dear DNSControl-maintainers,

I'm a thankful user of your amazing software.
Lately I had a little annoyance with the INWX-Provider and null-target
SRV-records.

Despite having little programming experience (and no experience with
Go), I was able to hack something together which seems to work.
Because this might be useful to others, I wanted to contribute that back
to DNSControl.

I therefore created myself a GitHub-Account and posted a feature-request
on the DNSControl issue-tracker (https://github.com/StackExchange/
dnscontrol/issues/3318) last week.
But I think my new GitHub-Account might be "shadow-banned", at least I
am only able to see the issue when logged in and I am unable to fork
DNSControl on GitHub.
So maybe no one could see my feature-request.

Therefore I'm posting the issue below again.
Sorry for the noise if the issue was already visible to you on GitHub.


Bye


-------
>
> # INWX: support null-target SRV
>
>
> ## Is your feature request related to a problem?
>
> I want to add a SRV-record with a null-target (".") (see [RFC 2782](https://datatracker.ietf.org/doc/html/rfc2782)) to my zone.
>
> `_pop3._tcp.example.org 0 0 0 .`
>
> I use the nameservers of INWX, so i also use the INWX-Provider with dnscontrol.
>
> Adding a SRV-Record with a null-target via the INWX website works.
>
> When i add `SRV("_pop3._tcp", 0, 0, 0, "."),` to my dnsconfig.js, dnscontrol produces a validation error:
>
> ```
> 2025/01/07 10:54:30 1 Validation errors:
> 2025/01/07 10:54:30 ERROR: INWX rejects domain example.org: srv has null target
> ```
>
> So until now i used a workaround and added the record with a two-dot-target (`".."`) to my dnsconfig.js:
> `SRV("_pop3._tcp", 0, 0, 0, ".."),`.
> This works and the INWX-nameservers serve the correct answer:
>
> ```
>> drill SRV _pop3._tcp.example.org @ns.inwx.de
> ;; ANSWER SECTION:
> _pop3._tcp.example.org. 3600 IN SRV 0 0 0 .
> ```
>
> Now i configured an additional nameserver with AXFRDDNS-Provider for my zone (for local name resolution, as in https://docs.dnscontrol.org/advanced-features/nameservers#shadow-nameservers).
> The update to this server fails with the following error-message:
>
> ```
> ******************** Domain: example.org
> 1 correction (ns-local)
> #1: DDNS UPDATES to 'example.org' (primary master: 'ns.local.example.org:53'). Changes:
> + CREATE _pop3._tcp.example.org SRV 0 0 0 .. ttl=3600
> FAILURE! dns: bad rdata
> Done. 1 corrections.
> completed with errors
> ```
>
> I think this is correct, as the two dots are just a workaround for the INWX-Provider.
>
> *(BTW, minor nitpick: it first says `1 correction` (singular), but `1 corrections.` (plural) at the end)*
>
> ## Describe the solution you'd like
>
> As INWX supports SRV-RRs with a "."-target, dnscontrol should too.
>
> I took some inspiration from #2643, #2687 and #2700 and changed some things i don't fully understand :).
>
> ```
> diff --git a/providers/inwx/auditrecords.go b/providers/inwx/auditrecords.go
> index 64dff42e..7a2853d1 100644
> --- a/providers/inwx/auditrecords.go
> +++ b/providers/inwx/auditrecords.go
> @@ -11,8 +11,6 @@ import (
> func AuditRecords(records []*models.RecordConfig) []error {
> a := rejectif.Auditor{}
>
> - a.Add("SRV", rejectif.SrvHasNullTarget) // Last verified 2020-12-28
> -
> a.Add("TXT", rejectif.TxtHasBackticks) // Last verified 2021-03-01
>
> a.Add("TXT", rejectif.TxtHasTrailingSpace) // Last verified 2021-03-01
> ```
>
> -> So dnscontrol doesn't prevent me from adding a null-target.
>
> ```
> diff --git a/providers/inwx/inwxProvider.go b/providers/inwx/inwxProvider.go
> index 3c5ca53f..d5a3c9a4 100644
> --- a/providers/inwx/inwxProvider.go
> +++ b/providers/inwx/inwxProvider.go
> @@ -195,7 +195,11 @@ func makeNameserverRecordRequest(domain string, rec *models.RecordConfig) *goinw
> }
> case "SRV":
> req.Priority = int(rec.SrvPriority)
> - req.Content = fmt.Sprintf("%d %d %v", rec.SrvWeight, rec.SrvPort, content[:len(content)-1])
> + if content == "." {
> + req.Content = fmt.Sprintf("%d %d %v", rec.SrvWeight, rec.SrvPort, content)
> + } else {
> + req.Content = fmt.Sprintf("%d %d %v", rec.SrvWeight, rec.SrvPort, content[:len(content)-1])
> + }
>
> ```
>
> -> For not removing the dot when sending an update?
>
> ```
> diff --git a/providers/inwx/inwxProvider.go b/providers/inwx/inwxProvider.go
> index 3c5ca53f..d5a3c9a4 100644
> --- a/providers/inwx/inwxProvider.go
> +++ b/providers/inwx/inwxProvider.go
> @@ -319,6 +323,8 @@ func (api *inwxAPI) GetZoneRecords(domain string, meta map[string]string) (model
> if rtypeAddDot[record.Type] {
> if record.Type == "MX" && record.Content == "." {
> // null records don't need to be modified
> + } else if record.Type == "SRV" && strings.HasSuffix(record.Content, ".") {
> + // null targets don't need to be modified
> } else {
> record.Content = record.Content + "."
> }
> ```
>
> -> For not adding an additonal dot while receiving a record with null-target from INWX?
> (I'm a bit unsure if `strings.HasSuffix(record.Content, ".")` is the best solution here.)
>
> And maybe some documentation/comment updates:
>
> ```
> diff --git a/providers/inwx/inwxProvider.go b/providers/inwx/inwxProvider.go
> index 3c5ca53f..d5a3c9a4 100644
> --- a/providers/inwx/inwxProvider.go
> +++ b/providers/inwx/inwxProvider.go
> @@ -53,7 +53,7 @@ var features = providers.DocumentationNotes{
> providers.CanUseLOC: providers.Unimplemented(),
> providers.CanUseNAPTR: providers.Can(),
> providers.CanUsePTR: providers.Can("PTR records with empty targets are not supported"),
> - providers.CanUseSRV: providers.Can("SRV records with empty targets are not supported."),
> + providers.CanUseSRV: providers.Can(),
> providers.CanUseSSHFP: providers.Can(),
> providers.CanUseSVCB: providers.Can(),
> providers.CanUseTLSA: providers.Can(),
> @@ -182,7 +182,7 @@ func makeNameserverRecordRequest(domain string, rec *models.RecordConfig) *goinw
> The API will not accept any target with a final dot but will
> instead always add this final dot internally.
> Records with empty targets (i.e. records with target ".")
> - are not allowed.
> + are allowed.
> */
> case "CNAME", "NS":
> req.Content = content[:len(content)-1]
> @@ -307,7 +311,7 @@ func (api *inwxAPI) GetZoneRecords(domain string, meta map[string]string) (model
> The API will not accept any target with a final dot but will
> instead always add this final dot internally.
> Records with empty targets (i.e. records with target ".")
> - are not allowed.
> + are allowed.
> */
> var rtypeAddDot = map[string]bool{
> "CNAME": true,
> ```
>
> This "works for me".™
> I can push the same zone to INWX and AFXRDDNS, and both times get the expected/correct answers.
>
> (I tried running the integration tests, but unfortunately they fail. Maybe becaus I'm using TOTP.
> ```
>> go test -v -verbose -profile INWX
> === RUN TestDNSProviders
> WARNING: -provider="" does not match profile TYPE="INWX". Using profile TYPE.
> Testing Profile="INWX" TYPE="INWX"
> integration_test.go:130: INWX: Unable to login
> --- FAIL: TestDNSProviders (3.22s)
> === RUN TestDualProviders
> Testing Profile="INWX" TYPE="INWX"
> integration_test.go:130: INWX: Unable to login
> --- FAIL: TestDualProviders (3.06s)
> === RUN TestNameserverDots
> Testing Profile="INWX" TYPE="INWX"
> integration_test.go:130: INWX: Unable to login
> --- FAIL: TestNameserverDots (3.08s)
> FAIL
> exit status 1
> FAIL github.com/StackExchange/dnscontrol/v4/integrationTest 9.391s
> ```
> )
>
> ### Describe alternatives you've considered
>
> - Special configurations for every provider i use, maybe with D_EXTEND() + some magic.
> - Wait for someone else to fix this.
>
> ## Additional context
>
> …

[Tom Limoncelli]

Hi there!

Interesting github issue. I, too, can not view that issue. I'm glad
you found the email list!

Anyway...

Here's the issue: Back in 2020 INWX wouldn't permit SRV records with
"." as the target. Therefore DNSControl won't try to create them.
This is enforced here:

https://github.com/StackExchange/dnscontrol/blob/2ef23621b5fbf0652a4eb5c7c86fba56444e3644/providers/inwx/auditrecords.go#L14

INWX no longer has that restriction. We just need to delete that
line (and run `gofmt -w providers/inwx/auditrecords.go`)

Would you like to submit a PR?

Tom

> --
> You received this message because you are subscribed to the Google Groups "DNSControl-discuss" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to dnscontrol-disc...@googlegroups.com.
> To view this discussion visit https://groups.google.com/d/msgid/dnscontrol-discuss/bfb2d0fc-55e5-4d80-91b7-32f83539c0e9%40mailbox.org.

[Christopher Hicks]

Tom:

There's a chance that the PR that github is hiding is because of the moderation settings on the repo or the org in github.  https://docs.github.com/en/communities/setting-up-your-project-for-healthy-contributions/about-community-management-and-moderation

To view this discussion visit https://groups.google.com/d/msgid/dnscontrol-discuss/CAOWqsAg4GE1CEwuq%2BH%2BOy4X%3DEu_b-Wx-LQ%3D-aK-umYyYXMRyzg%40mail.gmail.com.

--

Christopher Hicks ( he/him )
https://www.chicks.net/

[Tom Limoncelli]

I've created a PR. https://github.com/StackExchange/dnscontrol/pull/3377

This should be in the next release.

Tom

[rubgfx]

Hi Tom,

thank you very much for creating that PR!

Unfortunaly that change alone is not enough, due to how the INWX-API
handles dots.

See previous issue/pull requests for null-MX:
- https://github.com/StackExchange/dnscontrol/issues/2643
- https://github.com/StackExchange/dnscontrol/pull/2687
- https://github.com/StackExchange/dnscontrol/pull/2700

I have attached a diff that works for me.

Bye

[Tom Limoncelli]

Thanks!

I've turned your diff in to a PR:
https://github.com/StackExchange/dnscontrol/pull/3380

It will be in the next release.

Best,
Tom

On Thu, Jan 16, 2025 at 10:10 AM 'rubgfx' via DNSControl-discuss

> To view this discussion visit https://groups.google.com/d/msgid/dnscontrol-discuss/34e3e663-241a-445c-acd5-4f826691e00a%40mailbox.org.

[rubgfx]

Thanks for your work, much appreciated!

Bye