Re: [DNSOP] I-D Action: draft-ietf-dnsop-dns-capture-format-05.txt

[Jim Hague <[e-mail]>]

On 28/02/2018 06:36, Richard Gibson wrote:
> Thank you so much for the changes, I am confident that this format is
> now in a state where my organization could build on top of it to suit
> our needs.

Thanks for the feedback. Much appreciated!
> * Section 6.2.1 and 7.5.3.4: At minimum, RR field "ttl" should be
> optional in order to accommodate aggregation of resolver responses in
> which that field is decremented over time. But probably "rdata-index"
> should be optional as well.

We anticipated possibly doing this if anyone asked. We'll certainly go
back over that.

> * Section 7.1: Negative integer map keys are concise, but they come with
> the potential for interoperability-hostile collisions. I think the
> document should encourage restricting their use to tightly-controlled
> closed systems, and then additionally propose a particular namespacing
> pattern (e.g., "{namespaceName}-{keyName}") for use of
> implementation-specific /string/ keys in other cases.

We'll discuss this. We absolutely meant for negative keys to be
restricted to closed systems. Allowing string keys in addition is an
interesting idea; off the cuff, I wonder whether folk will want to pay
the file space penalty.

> Further, it should explicitly reserve positive integer keys for future standardized
> extensions to the C-DNS format.

That is absolutely the intention. We'll add some text making that clear.

> * Section 7.4.1: Why is BlockParameters an array instead of a map?

That's a typo. It should be a map, as it is in the CDDL.

> * Section 7.4.1.1: StorageParameters fields "opcodes" and "rrtypes"
> could be defined better... are they values /recorded/ (i.e., that were
> actually observed) or values that are /recordable/ (i.e., that the
> collection application was looking for)? And shouldn't they be optional
> (in either case, but *especially* the latter, lest implementations write
> out every possible rrtype value).

The intention is that they should be /recordable/ values - in line with
the rest of the StorageParameters fields, they let a reader distinguish
values that aren't present because the writer didn't understand them and
values not present because they didn't occur in the data stream. And
yes, we're thinking implementations should explicitly write each opcode
and rrtype they understand. And possibly should treat opcodes and
rrtypes they don't understand as malformed. The thinking here is that
if, say, an rrtype the writer does not know how to decode contains a
compressed label, it's not going to be correctly handled, and we should
not be recording potentially known-broken data in the file as not malformed.

> * Section 7.5.3: It's very good that the BlockTables "ip-address" items
> can be truncated by prefix length, but we have actually run into cases
> that call for /variable/ truncation (i.e., more precision in some
> subnets than in others), especially true given that a single table holds
> both client and server address data. What would you think about
> representing addresses not as direct byte strings, but rather as
> variable-length arrays or maps—index 0 encoding the prefix as a byte
> string, and optional index 1 (when present) encoding a prefix-length
> override? Or, if the extra size per address is too much, at least
> allowing such representations via polymorphism (byte string for default
> prefix-length, array/map for non-default)?

Variable truncation is a use case I for one was not anticipating. We'll
discuss this. Allowing an address to be a bstr OR an array or map with
optional prefix sounds an interesting idea.

> * Section 7.5.3.2: QueryResponseSignature field "qr-transport-flags"
> seems to reserve 1 bit /each/ for UDP, TCP, TLS, and DTLS, but it seems
> to me that UDP is mutually exclusive with TCP and that TLS is mutually
> exclusive with DTLS. Couldn't that data be represented in two bits (one
> in which 0 ⇒ UDP and 1 ⇒ TCP, the other in which 0 ⇒ plain-text and 1 ⇒
> TLS)?

We need to clarify the language. qr-transport-flags is a single bit
IPv4/v6, a 4 bit number indicating the protocol, and a final single bit
indicating the presence of trailing data in the query. The protocol
values are (currently) UDP=0, TCP=1, TLS=2, DTLS=3. We did consider a
UDP/TCP bit and a !TLS/TLS bit, but felt this (a) might imply a closer
connection between TLS and DTLS than is the case, and (b) would not
easily extend to possible future schemes like DOH and DNS over QUIC. So
we went for a number with sufficient range to add another 12 options
before trouble happens. We considered breaking the number out to a
separate integer, but felt that this was likely, in practice, to be
unnecessary and so space efficiency considerations took precedence.
-- 
Jim Hague - [e-mail]          Never trust a computer you can't lift.