PowerDNS / pdns

PowerDNS Authoritative, PowerDNS Recursor, dnsdist
https://www.powerdns.com/
GNU General Public License v2.0
3.67k stars 907 forks source link

auth api: consider turning off "Not in expected format" check - discussion ticket #9870

Open zeha opened 3 years ago

zeha commented 3 years ago

Short description

Today, the Auth API checks if the supplied RR Content field matches the output of getZoneRepresentation after parsing. Every now and then, there are sometimes valid, sometimes not so valid questions about this. One (IMO very valid) is in #9869.

Longer description

I've added the check because some of the content parsing code is quite lenient, and adds defaults for values that are not present. This can be quite surprising. On the other hand, the RFCs allow minor formatting differences in "complicated" types, like (C)DNSKEY, but probably also SVCB.

Proposal: remove the check, but always store the output of getZoneRepresentation.

This would mean that the data one puts in, is not necessarily what will be read back. It's a different tradeoff.

peterthomassen commented 3 years ago

If the check is removed and something else than given is stored, then it would be great if the response contained what was stored, so that a client can check whether it is what's intended. (I am not sure whether the API currently contains the accepted record contents in its response, but afair it doesn't.)

Habbie commented 3 years ago

I agree that the current check is stricter than is nice. This proposal (including Peter's comment!) makes sense to me.

zeha commented 3 years ago

Sending back the "as stored" record is a challenge. Right now, for PATCH, we just send a 204 - No Content response.

nils-wisiol commented 3 years ago

The API should allow any valid presentation format as input, as this is the only reliable, well-known common ground. Any further restriction would require extensive documentation in order to make the pdns API a reliable tool to work with. (Accepting more than just that is fine, e.g. as done in TXT records.)

RobinGeuze commented 3 years ago

The other side of the argument is that its potentially dangerous to have PDNS interpret some input from the API "wrong" and serving a completely different record than the user intended.

I think there was some discussion about partially reworking the API anyway, maybe its better to change the behavior on the new endpoints and just leave the old endpoints alone?

nils-wisiol commented 3 years ago

The other side of the argument is that its potentially dangerous to have PDNS interpret some input from the API "wrong" and serving a completely different record than the user intended.

Could you outline such a case? I thought that accepting any presentation format would be an extension of the inputs that would be accepted, rather than a re-interpretation of input that are already accepted. The latter, of course, needs to be avoided, in order to not break existing applications.

peterthomassen commented 3 years ago

Another finding: For CNAME targets, the API does not accept names with escaped characters such as foobar\@powerdns.example..

Based on RFC 1035 Sec. 5.1, it appears reasonable, however, to escape certain non-alphanumeric characters, namely "().;\@$. (These have special meaning in zone files.) dnspython indeed does escape these characters. As a result, certain CNAME targets preprocessed by dnspython are currently not accepted by pdns auth.

PenelopeFudd commented 5 months ago

I'd request that the error message be changed. Old:

{"error": "Record i1.jp1.netskrt.org./SOA 'ns1.example.com hostmaster.example.org 1 10800 3600 604800 3600':
 Not in expected format (parsed as 'ns1.example.com. hostmaster.example.org. 1 10800 3600 604800 3600')"}

New:

{"error": "Record i1.jp1.netskrt.org./SOA 'ns1.example.com hostmaster.example.org 1 10800 3600 604800 3600': 
Not in expected format (expecting exactly 'ns1.example.com. hostmaster.example.org. 1 10800 3600 604800 3600')"}

We've been staring at the error message and thinking "ok, it parsed it as XXX, so? Why doesn't it tell us what we should have sent instead?" There was a solid round of face-palming when we finally figured it out by reading this issue. 😵‍💫