Re: [DNSOP] Alexey Melnikov's Discuss on draft-ietf-dnsop-dns-capture-format-08: (with DISCUSS and COMMENT)

[Alexey Melnikov <[e-mail]>]

Hi Sara,

On Wed, Nov 21, 2018, at 2:36 PM, Sara Dickinson wrote:
>> On 19 Nov 2018, at 20:37, Alexey Melnikov
>> <[e-mail]> wrote:> 
> Hi Alexey, 
> 
> Thanks for the review.
> 
>> 
>> Alexey Melnikov has entered the following ballot position for
>> draft-ietf-dnsop-dns-capture-format-08: Discuss
>> ---------------------------------------------------------------
>> ------->> DISCUSS:
>> ---------------------------------------------------------------
>> ------->> 
>> Thank you for this document, it is a useful contribution to RFC
>> series. I>> enjoyed reading it.
>> 
>> I have a small list of issues that is hopefully easy to fix:
>> 
>> 1)
>> 
>> In 7.4.2:
>> 
>>    | filter           | O | T | "tcpdump" [pcap] style filter
>>    | for      |>>    |                  |   |   | input.
>>    |                  |   |   | |>> 
>> This makes the [pcap] reference Normative. If you don't want to do
>> that, please>> fully specify syntax in this document.
> 
> Is that true if it is an optional field? 
Yes, optionallity of a field doesn't make its full specification
optional.
> 
>> 
>> 2) I believe [I-D.ietf-cbor-cddl] reference needs to be Normative due
>>    to use of>> CDDL in Appendix A. If you don't think CDDL is normative, you need
>> to state>> that in Appendix A.
> 
> Yes indeed - it should be normative, will fix. 
> 
> 
>> 
>> ---------------------------------------------------------------
>> ------->> COMMENT:
>> ---------------------------------------------------------------
>> ------->> 
>> Was CDDL in Appendix A validated with a tool? This information is
>> missing from>> the shepherding write-up.
> 
> We (the authors) have used the CDDL tool described on
> http://cbor.io/tools.html to read the CDDL in Appendix A and ensured
> an example instance can be created.> 
> Did you have some other validation tool in mind?
No, this is fine. I just wanted to know.

> 
>> 
>> 6.2.3.  Storage flags
>> 
>>    The Storage Parameters also contains optional fields holding
>>    details>>    of the sampling method used and the anonymisation method used.
>>    It is>>    RECOMMENDED these fields contain URIs pointing to resources
>>    describing the methods used.
>> 
>> Please add a Normative Reference for URI spec here (RFC 3986).
> 
> Yes, will do. 
> 
>> 
>> 7.5.3.2.  "QueryResponseSignature"
>> 
>>    | qr-transport-flags | O | U | Bit flags describing the
>>    | transport   |>>    |                    |   |   | used to service the query.
>>    |                    |   |   | |>>    |                    |   |   | Bit 0. IP version. 0 if IPv4, 1 if
>>    |                    |   |   | |>>    |                    |   |   | IPv6
>>    |                    |   |   | |>>    |                    |   |   | Bit 1-4. Transport. 4 bit unsigned
>>    |                    |   |   | |>>    |                    |   |   | value where 0 = UDP, 1 = TCP, 2 =
>>    |                    |   |   | |>>    |                    |   |   | TLS, 3 = DTLS. Values 4-15 are
>>    |                    |   |   | |>>    |                    |   |   | reserved for future use.
>>    |                    |   |   | |>>    |                    |   |   | Bit 5. 1 if trailing bytes in query
>>    |                    |   |   | |>>    |                    |   |   | packet. See Section 11.2.
>>    |                    |   |   | |>> 
>> Would something like DoH appear as a separate transport choice?
> 
> No, we need to add DoH to this list (it didn’t exist when we started
> the draft!).> 
>> 
>> How can new values be added to the list if there are no IANA
>> Considerations?> 
> As described in response to the DISCUSS on this issue we propose IANA
> create a C-DNS registry with> subregistries with keys for each of the different maps used in C-DNS.> New entries in these subregistries would follow Expert Review 
Ok, great.

> 
>> 
>> 7.5.3.5.  "MalformedMessageData"
>> 
>>    |                    |   |   |                                      |
>>    | mm-transport-flags | O | U | Bit flags describing the
>>    | transport   |>>    |                    |   |   | used to service the query. Bit 0 is
>>    |                    |   |   | |>>    |                    |   |   | the least significant bit.
>>    |                    |   |   | |>>    |                    |   |   | Bit 0. IP version. 0 if IPv4, 1 if
>>    |                    |   |   | |>>    |                    |   |   | IPv6
>>    |                    |   |   | |>>    |                    |   |   | Bit 1-4. Transport. 4 bit unsigned
>>    |                    |   |   | |>>    |                    |   |   | value where 0 = UDP, 1 = TCP, 2 =
>>    |                    |   |   | |>>    |                    |   |   | TLS, 3 = DTLS. Values 4-15 are
>>    |                    |   |   | |>>    |                    |   |   | reserved for future use.
>>    |                    |   |   | |>> 
>> If the above bits supposed to be the same as for qr-transport-flags,>> maybe it is worth defining them once and referencing in all relevant
>> places?> 
> The qr-transport-flags and mm-transport-flags are different in that
> the qr-transport-flags include Bit 5, the trailing bytes indicator.> 
> In the CDDL a base ’TransportFlags’ type is defined and then
> 
> mm-transport-flags     => TransportFlags,
> 
> qr-transport-flags    => QueryResponseTransportFlags,
> 
>  QueryResponseTransportFlagValues = &(
>       query-trailingdata : 5,
>   ) / TransportFlagValues
>   QueryResponseTransportFlags = uint .bits
>   QueryResponseTransportFlagValues> 
> We can add some text to the table descriptions in sections 7.5.3.2 and
> 7.5.3.5 to clarify the relationship.Thank you for your clarification. Your explanation is sufficient.

> 
>> 
>> 7.6.  "QueryResponse"
>>    | query-size           | O | U | DNS query message size
>>    | (see        |>>    |                      |   |   | below).
>>    |                      |   |   | |>>    |                      |   |   |                                    |
>>    | response-size        | O | U | DNS query message size
>>    | (see        |>>    |                      |   |   | below).
>>    |                      |   |   | |>> 
>> I think "DNS response message size" for response-size.
>> 
>> It is odd to have RFC 2119 language in B.2, which is itself
>> informative.> 
> Good catch :-)
> 
> Many thanks
> 
> Sara.