Fixing (and breaking) TXT records again

[Tom Limoncelli]

The way we handle TXT records internally is complex. I'm trying to see if I can simplify it.  This is tech debt that I've been meaning to clean up for a while.

How are txt records handled now?

The DNS protocols treat TXT records as a sequence of 255-byte chunks.  Some APIs send a TXT record's data in 255-byte chunks, but others send one long string.

DNSControl takes the data as the provider sends it in the API (one big string or chunks) and stores them as they were received. Later anything that works with TXT records must handle both cases.

Why is this complex?

By maintaining the strings as chunks, we get into complex situations later.  Sometimes it makes sense to join up the chunks, sometimes it makes sense to keep them separate.  Different code expects chunks and splits up big strings; some code watches for big strings and breaks them up.

What's the proposal?

DNSControl should only treat a TXT record's data as one long string.  This reduces the number of edge cases in the main code.  The individual providers are responsible for doing the right thing when talking to the API.  If the API deals with chunks, the provider must split long strings before sending create/modify commands; and must join the smaller chunks into one long string when downloading the zone's records.  Thus, the details of the API are hidden from DNSControl's main code.

Status so far?

  

The goal is to have a little pain now (converting all the providers) so that there is less pain in the future (writing new providers in the future).

I've been playing around with what this would look like here: https://github.com/StackExchange/dnscontrol/pull/2598  (it isn't complete yet. You'll see many integration tests fail).

I thought the change would be fairly simple on the providers. However, many of them didn't survive my somewhat mechanical changes.  Google DNS and a few others are proving to be difficult to debug.

The CI/CD pipeline only tests these providers: AZURE_DNS BIND CLOUDFLAREAPI CLOUDNS CSCGLOBAL DIGITALOCEAN GANDI_V5 GCLOUD HEDNS HEXONET NAMEDOTCOM POWERDNS ROUTE53 TRANSIP.  The other provider maintainers will need to test the code themselves.

Thoughts, comments, and bugfixes appreciated!

Tom

--

Tom Limoncelli (he/him)
SRE TPM, Stack Overflow, Inc.

[Phil Pennock]

On 2023-10-27 at 16:21 -0400, 'Tom Limoncelli' via DNSControl-discuss wrote:
> What's the proposal?
> DNSControl should only treat a TXT record's data as one long string.

The question is: are there any higher-level applications which ascribe
semantic value to the TXT string boundaries?

It's up to _everything_ using TXT records to describe what they think
should be done in the presence of:
1. Multiple TXT records
2. Multiple strings in a single TXT record.

Eg, SPF (RFC 7208) explicitly prohibits multiple TXT records (section 3)
and specifies (in 3.3) that when multiple strings are present, they
should be concatenated, with no space, and treated as one long string.

For DKIM, RFC 6376 has the same string-joining specification in
section 3.6.2.2.

The question is: are there any specifications or usages which do
anything else? If not, then it is a reasonable constraint of DNSControl
to say "you don't get to choose where the string boundaries are".

But if there are such other usages in the wild, then this change would
break them. So if it's not a hassle, then probably best to continue to
support the array of strings API.

IMO the safest approach is to always treat TXT records as an array of
strings, but treat the TXT constructor with one long string as
something which splits internally to multiple strings, long before
talking to the service provider's API.
But "safest" != "most pragmatic".

-Phil

[Tom Limoncelli]

Phil, as always you've provided some great analysis.

On Sun, Oct 29, 2023 at 4:55 PM Phil Pennock <dnscontrol-disc...@spodhuis.org> wrote:

The question is: are there any higher-level applications which ascribe
semantic value to the TXT string boundaries?

I think this is the key question.  

I believe that no applications do. My proof is not based on reading RFCs, but instead based on:

1. observing that I've never seen a DNS provider's control panel let you specify the boundaries.

2. I've never seen a FAQ or reddit post asking how to specify those boundaries.

From that, I conclude that no applications do this.  I suspect that if an IETF workgroup proposed such a thing, they'd get push-back from DNS providers.

 

IMO the safest approach is to always treat TXT records as an array of
strings, but treat the TXT constructor with one long string as
something which splits internally to multiple strings, long before
talking to the service provider's API.
But "safest" != "most pragmatic".

FYI: The current code does just that.  It maintains the substrings as they came from the provider (a.k.a. the existing config) and the user (dnsconfig.js, a.k.a. the desired config).  The user data ("desired config") is left alone unless a provider calls txtutil.SplitSingleLongTxt() to chop it up into 255-octet chunks.  Only some providers call SplitSingleLongTxt and I think that may be a source of confusion down the road.

I think the primary problem is that the differencing system doesn't do the right thing in some edge cases.  It's difficult to find a repro case.

Here is the PR I'm working on to fix this:

https://github.com/StackExchange/dnscontrol/pull/2610

(It needs work. It currently breaks many providers.)

Here are 2 situations where I think (I hope!) might be improved by the above PR.

https://github.com/StackExchange/dnscontrol/issues/2489

https://github.com/StackExchange/dnscontrol/issues/2272

Tom

[Julius Rickert]

The DNS protocols treat TXT records as a sequence of 255-byte chunks.

Small nit-pick: It's not sequences of 255-byte chunks but rather sequences of chunks of bytes, each individually sized and up to 255 bytes long.

I can't come up with an instance of any use case where the semantics of a sequence of strings differs from a single string either.

From that, I conclude that no applications do this.

And no one can introduce it to a broad audience as providers don't support it.

DNSControl's tag line is: "DNSControl is an opinionated platform"

We can be opinionated about this!

I would simplify the TXT interface to just a single string – no array – with a length of up to 65,280 bytes (the maximum possible TXT record size).

Maybe, if it's not too much effort, we can keep records that aren't "canonicalised" into 255-byte chunks (but the last one) and change them to the "canonicalised" representation on updates only.

If necessary, we could introduce an additional domain modifier that allows for the <character-string>s to be set individually for providers that support them (and throw an error otherwise).

— Julius

[Tom Limoncelli]

On Wed, Nov 1, 2023 at 12:35 PM Julius Rickert <m...@juliusrickert.de> wrote:

The DNS protocols treat TXT records as a sequence of 255-byte chunks.

Small nit-pick: It's not sequences of 255-byte chunks but rather sequences of chunks of bytes, each individually sized and up to 255 bytes long.

I can't come up with an instance of any use case where the semantics of a sequence of strings differs from a single string either.

From that, I conclude that no applications do this.

And no one can introduce it to a broad audience as providers don't support it.

DNSControl's tag line is: "DNSControl is an opinionated platform"

We can be opinionated about this!

I would simplify the TXT interface to just a single string – no array – with a length of up to 65,280 bytes (the maximum possible TXT record size).

That's what I was attempting to do in https://github.com/StackExchange/dnscontrol/pull/2598.  (the code is incomplete; many tests still fail).

 

I'd revise the opinion to have 2 parts:

* TXT records are just a single string – no array – with a length of up to 65,280 bytes (the maximum possible TXT record size).

* TXT records are 255-octet chunks except possibly the last one.  If a provider presents the strings broken up some other way, the provider might reset it to canonical form.  That is, if we get chunk sizes of 200+200+200, the code might leave it alone or it might change it to 255+255+90).

Maybe, if it's not too much effort, we can keep records that aren't "canonicalised" into 255-byte chunks (but the last one) and change them to the "canonicalised" representation on updates only.

Agreed.

 

If necessary, we could introduce an additional domain modifier that allows for the <character-string>s to be set individually for providers that support them (and throw an error otherwise).

Agreed.

Tom 

[Tom Limoncelli]

Well that was fun!  

I've reworked the TXT handling code to treat TXT records as one big string!

Modifying the providers appropriately was easier than expected.  If you look at #2631 you'll see that most providers were unmodified.  (Actually I did a little trick here: an earlier release changed all the providers to access TXT data one of two ways.  That future-proofed them in a way that made this PR a lot shorter!)

I've made a "pre-release" so there are binaries that people can test. Here's an easy way to access it:

    docker run --rm -it -v "$(pwd):/dns"  ghcr.io/stackexchange/dnscontrol:4.7.0 preview

Other binaries are here: https://github.com/StackExchange/dnscontrol/releases/tag/v4.7.0

The code is in the https://github.com/StackExchange/dnscontrol/tree/tlim_newtxt_minimal branch.

There should be no user-visible difference.  Please test this with your data (`preview` first, of course!).  If you maintain a provider, please try the integration tests again to verify everything works.

Feedback welcome!

Tom

[Tom Limoncelli]

Hi folks!

We're getting a lot closer to merging this PR!

Thanks to costasd for fixing NS1, imlonghao for fixing PORKBUN and CLOUDNS!

I haven't heard a lot of feedback from other maintainers.  Since all the officially supported providers work, I'm going to merge this in a day or two.  Please speak up soon if there are problems!

I've made a new set of binaries: v4.7.1-beta.   An easy way to test this change is by running the container:

    docker run --rm -it -v "$(pwd):/dns"  ghcr.io/stackexchange/dnscontrol:4.7.1-beta preview

NOTE: The CLOUDFLAREAPI provider currently rejects TXT records longer than 255 octets.  This is fixed in the branch, but now in the binaries above.  Sorry!

Tom