Clean up https://github.com/adulau/pdns-qof/wiki/Additional-Fields
djw1149 opened this issue · 15 comments
The additional fields registry at https://github.com/adulau/pdns-qof/wiki/Additional-Fields contains three fields that have long since been added to the COF draft: sensor_id, zone_time_first, and zone_time_last. I suggest that they be removed from the registry or an additional column be added to the registry table showing the draft version number or RFC number in which each additional field was added to the I-D or RFC (as/if becomes applicable).
For the master file format, perhaps we add a reference to RFC1035 section "5.1. Format" in our 3.5.2 and 3.5.3? We already reference "DNS master file RFC 1035..." in our section "3.3.3 rdata".
For the milli/nano resolution, are you saying to add additional variations of all time based fields with the higher resolution data format? i.e. a new time_first, time_last, zone_time_first, and zone_time_last? We actually probably don't need it for zone_time_first and zone_time_last.
Perhaps:
time_first_hires
This field returns the first time that the record / unique tuple (rrname, rrtype, rdata) has been seen by the passive DNS with a higher resolution than the standard time_first field. The timestamp is expressed in milliseconds (decimal) since 1st of January 1970 00:00:00.0000 (the Unix epoch), but this is 1000 times larger than a Unix timestamp. The time zone MUST be UTC. This field is represented as a JSON [RFC4627] number. If BOTH time_first and time_first_hires are present in a record, then INTEGER(time_first_hires / 1000) MUST equal time_first, otherwise the behavior is undefined.
time_last_hires
(Similar to above: s/time_first/time_last/g)
See version 07. Good enough?
Could you clarify:
if time_first_hires is present then time_first need not be present. (and the implementation is still "conforming")
if time_last_hires is present then time_last need not be present.
Aaron, your response makes sense, but a COF object can validly have zone_time_first but not time_first.
Implementation MUST support all the mandatory fields. does not mean that all the mandatory fields MUST be present in every COF object.
i would still prefer that the _hires times be just the fraction, thus not duplicating the time_first and time_last, which should remain mandatory. if we had time_first_nano and time_last_nano it ought to be future proof. sensor_id is of course optional.
okay, hmm... but
a) AFAIK that's what came out of the FIRST pDNS SIG.
b) are we OK to keep time_first, time_last no matter what now? (i.e. no matter if _hires is fraction or not or full or nanoseconds?)In case (b) is OK for you, I can add a sentence (thanks for pointing it out!) about the mandatory-ness of time_first and time_last (even in the presence of _hires). Idea behind it: don't break existing clients.
The pattern that is currently in service should be supported without a compelling reason to break backwards compatibility. In current implementations the records contain either a pair of time_* or a pair of zone_time_* fields. Duplicating timestamps is unnecessary. I suggest updating the mandatory field list as follows:
(time_first AND time_last) OR (zone_time_first AND zone_time_last)
Sounds like a good compromise, but ... w.r.t to backwards compatibility, I am not really sure if users of the format implement the zone_time_* fields.
Farsight's DNSDB uses this model today. We send either time_[first|last] or zone_time_[first|last], but not both.
The only fields which are mandatory are the time_first, time_last fields.
Hm... a bit of a conundrum because time_first, time_last are mandatory, zone_time_first, zone_time_last (as well as the *_hires ones) are optional. I wonder if it's better to solve this before it goes to the IETF or during that process.... What do you think?
I agree we should get this solved.
I propose restructuring the mandatory fields section such that the zone_time_[first|last] are allowed to appear in place of time_[first|last], as long as one of the pairs is always present.
Added an additional note. I would still discourage this since it's really confusing for implementers.