[tor-bugs] #20548 [Metrics]: Handle bad input more consistently in metrics code bases

[Tor Bug Tracker & Wiki]

#20548: Handle bad input more consistently in metrics code bases
-----------------------------+-----------------
     Reporter:  karsten      |      Owner:
         Type:  enhancement  |     Status:  new
     Priority:  Medium       |  Milestone:
    Component:  Metrics      |    Version:
     Severity:  Normal       |   Keywords:
Actual Points:               |  Parent ID:
       Points:               |   Reviewer:
      Sponsor:               |
-----------------------------+-----------------
 We started thinking about handling bad input while sanitizing bridge
 descriptors in CollecTor (#19834) and while reading relay descriptors in
 Onionoo (#20412).  But before we implement any changes we should
 generalize our strategies for handling bad input to avoid solving the same
 problem over and over.  The result of this discussion can also serve as
 guide for future code.

 What we're not covering here (but what we should think about anyway) is
 how we're handling issues during processing that are not directly related
 to bad input, like problems with sanitizing bridge IP addresses.

 So, we can distinguish a couple of use cases where we're handling
 descriptors as input:
  1. CollecTor downloads descriptors from the directory or TorDNSEL or
 Torperf or processes previously downloaded descriptors.
  2. CollecTor synchronizes descriptors from other CollecTor instances.
  3. CollecTor reads previously uploaded bridge descriptors and produces
 sanitized versions of them.
  4. Metrics, Onionoo, ExoneraTor, and other applications download
 descriptors from CollecTor and use them locally.

 The requirements on input data are quite different for these four use
 cases.  Let's go through them:
  1. As of a few weeks ago we're storing and serving descriptors even if
 metrics-lib cannot parse them.  The idea is that CollecTor shouldn't
 decide what goes into the archive, but the directory authorities (or
 TorDNSEL and Torperf) should.  As long as we can detect the descriptor
 type, extract the publication date, and possibly calculate the digest, we
 can accept a descriptor.
  2. The requirements are pretty much the same as for 1.
  3. We need to be very picky about bridge descriptors, in particular about
 unknown parts, because those might contain sensitive information that we'd
 rather not copy over to sanitized bridge descriptors.
  4. Most applications would want to skip bad descriptors and not bother
 much about it.  As of a few days ago, they all use metrics-lib for parsing
 descriptors.

 So much about the differences.  Let's also list the commonalities or
 possible goals for common behavior:
  - Regardless of the chosen strategy, we should apply it to all variants
 of descriptor badness.  This may sound obvious, but we're currently not
 doing this.  For example, if we encounter an invalid "a" line in a bridge
 descriptor, we skip that line, whereas an invalid "r" line makes us skip
 the entire descriptor.  This is part of what we're trying to streamline in
 #19834, even though this case is not explicitly listed there.
  - Whenever we encounter an error in processing a descriptor, we should
 attempt to recover and continue with subsequent good descriptors.  The
 reason is that descriptors in a common source can come from different
 original sources, and we cannot blame valid descriptors for following a
 faulty descriptor.  This is the cause for #20412.

 This is just the start.  Let's add more thoughts to this ticket and
 assemble a guide that we can apply to existing tickets and future code
 changes.

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