[tor-bugs] #21934 [Metrics/metrics-lib]: Allow extra arguments in lines containing comma-separated key-value lists

[Tor Bug Tracker & Wiki]

#21934: Allow extra arguments in lines containing comma-separated key-value lists
---------------------------------+------------------------------
 Reporter:  karsten              |          Owner:  metrics-team
     Type:  defect               |         Status:  needs_review
 Priority:  Medium               |      Milestone:
Component:  Metrics/metrics-lib  |        Version:
 Severity:  Normal               |     Resolution:
 Keywords:                       |  Actual Points:
Parent ID:                       |         Points:
 Reviewer:                       |        Sponsor:
---------------------------------+------------------------------
Changes (by karsten):

 * status:  new => needs_review


Comment:

 So, I spent more time on this ticket than I should have. :)  Here's what I
 found:

 According to the specification it's valid to not only add a new response
 status, but also to append one or more entirely new parameters.  Example:

 {{{
 dirreq-v3-resp ok=88,not-modified=24,busy=8,not-enough-sigs=0,not-
 found=0,unavailable=0 delta_f=1 epsilon=0.30 bin_size=8
 }}}

 If this were not permitted, there'd have to be a `[No extra arguments]`
 tag.

 The same applies to `cell-circuits-per-decile` which could indeed be
 extended to include another parameter than the number.  For example:

 {{{
 cell-circuits-per-decile 105472 delta_f=10240 epsilon=0.30 bin_size=1024
 }}}

 In total, I found 17 lines that ''don't'' allow extra arguments:

  - Server descriptors:
    - `"identity-ed25519"`
    - `"onion-key"`
    - `"onion-key-crosscert"`
    - `"ntor-onion-key-crosscert"`
    - `"signing-key"`
    - `"router-signature"`
    - `"caches-extra-info"`
    - `"allow-single-hop-exits"`
    - `"tunnelled-dir-server"`
  - Extra-info descriptors:
    - `"router-signature"`
  - Key certificates/votes:
    - `"dir-identity-key"`
    - `"dir-signing-key"`
    - `"dir-key-crosscert"`
    - `"dir-key-certification"`
  - Microdescriptors:
    - `"onion-key"`
  - Consensuses:
    - `"consensus-method"`
  - Votes and consensuses:
    - `"directory-footer"`

 The list of lines that ''do'' allow extra arguments is, of course, much
 longer.  Now, I don't want to touch all code in metrics-lib for this
 relatively minor change, and I don't think we have to.  The worst thing
 that can happen when an extra argument is added that metrics-lib doesn't
 understand is that it rejects the whole descriptor.  When we find out we
 can still update metrics-lib and reprocess the descriptor.  But we don't
 lose data, because CollecTor stores descriptors anyway.

 I suggest we focus on a subset that we care most about, which is extra-
 info descriptor lines related to statistics.  Please review
 [https://gitweb.torproject.org/user/karsten/metrics-
 lib.git/log/?h=task-21934 my branch task-21934].

 Forward compatibility, yay!

--
Ticket URL: <https://trac.torproject.org/projects/tor/ticket/21934#comment:2>
Tor Bug Tracker & Wiki <https://trac.torproject.org/>
The Tor Project: anonymity online