Structured Error Data for Filtered DNS
draft-ietf-dnsop-structured-dns-error-25

# Andy Newton, ART AD, comments for draft-ietf-dnsop-structured-dns-error-24
CC @anewton1998

* line numbers:
  - https://author-tools.ietf.org/api/idnits?url=https://www.ietf.org/archive/id/draft-ietf-dnsop-structured-dns-error-24.txt&submitcheck=True

* comment syntax:
  - https://github.com/mnot/ietf-comments/blob/main/format.md

* "Handling Ballot Positions":
  - https://ietf.org/about/groups/iesg/statements/handling-ballot-positions/

## Thanks to the Reviewers

Thanks to Paul Kyzivat for the ARTART review.

And many thanks to the authors for addressing Paul's review comments.

## Comments

### New JSON Names

442        New JSON names may be defined in the future.  This section specifies
443        the requirements to take into account for such names.

IMHO, a reference to the IANA considerations section for this registry would be helpful.

Thanks to Paul Kyzivat for multiple ARTART reviews and to the authors for resolving all the concerns.

Thanks to Benno Overeinder for the particularly helpful shepherd writeup.

Thank you for addressing my discuss (although I would have appreciated an actual response so I didn't have to go searching for the change).  

My comment on Section 10.1 still stands.  While QUIC relies on the TLS handshake, it is not normally specified as 'use TLS 1.3' or later.  

Also, consider changing the RFC8446 reference to RFC9846, as it was published early this week. 

-------------------------------------------
Thanks to Joe Salowey for their secdir review - and follow up messages.

Section 10.1, para 1:  DOQ is mentioned parenthetically, is QUIC (RFC9000) an acceptable transport mechanism?

Section 11.1, Structured DNS Error EDNS Option, and Section 11.5's RFC Editor note:

821 >       Notes to the RFC Editor: Please replace RFCXXXX with the RFC
822 >       number assigned to this document and "TBA1" with the value
823 >       assigned by IANA, and replace "TBD1" in Figure 3 with the value
824 >       assigned by IANA.

...

832 >    Value:  TBD

This is more of an NIT.

The placeholder in Section 11.1 is "TBD" but Figure 3 uses "TBD1" for the same
EDNS(0) Option Code, and the RFC Editor note only instructs the editor to replace
"TBD1" in Figure 3. As written, the note gives the RFC Editor no instruction to
replace the "TBD" value in Section 11.1 itself.

---

Section 11.4, New Registry for Extended DNS Sub-Error Codes, sub-error codes 1 and 2:

960 >        |   1    | Malware  | "Blocked", "Blocked | Section 5.5 of |
961 >        |        |          | by Upstream DNS     | [RFC5901]      |
962 >        |        |          | Server", "Filtered" |                |
963 >        +--------+----------+---------------------+----------------+
964 >        |   2    | Phishing | "Blocked", "Blocked | Section 5.5 of |
965 >        |        |          | by Upstream DNS     | [RFC5901]      |
966 >        |        |          | Server", "Filtered" |                |

I checked RFC 5901 Section 5.5, and it defines the FraudType attribute of an
IODEF PhraudReport, whose enumerated values happen to include "malware distribution"
and "phishing" among nine fraud-reporting categories. That section is defining values
for an incident-reporting data model, not originating general definitions of
"malware" or "phishing" as DNS-filtering categories. I am not asking for a DISCUSS
here, since the terms are in ordinary use and the registry entries are intelligible
without RFC 5901, but I would suggest the authors reconsider whether RFC 5901 is
really the right reference for these two rows, since a reader who follows the
citation to understand what "Malware" or "Phishing" means for this registry will find
an unrelated reporting schema instead.

---

Section 11.4, same table, sub-error codes 3 and 4:

968 >        |   3    | Spam     | "Blocked", "Blocked | Page 289 of    |
969 >        |        |          | by Upstream DNS     | [RFC4949]      |
970 >        |        |          | Server", "Filtered" |                |
971 >        +--------+----------+---------------------+----------------+
972 >        |   4    | Spyware  | "Blocked", "Blocked | Page 291 of    |
973 >        |        |          | by Upstream DNS     | [RFC4949]      |
974 >        |        |          | Server", "Filtered" |                |

RFC 4949 is a flat, alphabetically ordered glossary rather than a sectioned
document, which is presumably why these two rows cite page numbers instead. I was
not able to confirm pages 289 and 291 against the current RFC Editor rendering of
RFC 4949 to check that they still land on the "Spam" and "Spyware" entries. Page
numbers are tied to a specific paginated rendering and are a less stable locator
than the rest of this document's references, which all cite sections. I would ask
the authors not to use page numbers as references.

----------------------------------------------------------------------
NIT
----------------------------------------------------------------------

All comments below are about very minor potential issues that you may
choose to address in some way - or ignore - as you see fit. Some were
flagged by automated tools (via
https://github.com/larseggert/ietf-reviewtool), so there will likely
be some false positives. There is no need to let me know what you did
with these suggestions.

Section 4.2, Future JSON Names Requirements:

451 >    size.  Refer to [RFC9715] for a discusson on IP fragmentation
s/discusson/discussion/

Thanks for your edits to address my previous DISCUSS ballot. A few of my comments still stand, but not enough to block the document.

### Section 4, paragraph 5
```
        Contact URIs conveyed in the "c" field MUST use URI schemes
        registered in Section 11.3.
```
I continue to think it's a bit
odd to create a registry of entries that are already in another
registry, and where client behavior is entirely gated on what values they
support in the field or not.

### Section 4, paragraph 26

Are clients required to enforce these requirements on names they don't
recognize? If so, what behavior is a client required to take when processing a
non-compliant error response? If not, why are these normative requirements?

The additional text makes clear that these requirements are intended to apply to
future specifications, but there's no enforcement mechanism or client behavior
specified here.