Re: [tor-bugs] #9199 [BridgeDB]: Rethink the logging of BridgeDB

["Tor Bug Tracker & Wiki" <blackhole@xxxxxxxxxxxxxx>]

#9199: Rethink the logging of BridgeDB
--------------------------+----------------------------
     Reporter:  asn       |      Owner:  isis
         Type:  task      |     Status:  needs_revision
     Priority:  normal    |  Milestone:
    Component:  BridgeDB  |    Version:
   Resolution:            |   Keywords:
Actual Points:            |  Parent ID:
       Points:            |
--------------------------+----------------------------

Comment (by sysrqb):

 Replying to [comment:21 isis]:
 > Just jotting down two more bugs/problems that I've noticed:
 >
 >  1) At L435 (or thereabouts) in lib/bridgedb/EmailServer.py:
 > {{{
 > $ ack-grep log | ack-grep body
 > lib/bridgedb/EmailServer.py:435:    logging.debug("Email body:\n%s" %
 f.read())
 > }}}
 > This doesn't actually log the ''body'' of the email, it logs the body of
 the email ''and'' the full headers, including the original client headers,
 which include the client's email address and IP address.
 >

 This is okay because it is at debug, however we must 1) figure out a way
 to scrub all PII by default, and 2) either adjust the message label to
 reflect the output or 3) adjust the output to reflect the message label
 :). If we need to troubleshoot mail issues in the future, I prefer to have
 the logging already in place. I guess one option for this is to build two
 copies of the outgoing mail, one we send and the other has PII scrubbed.
 Another option is that we manually log the headers and scrub them by
 default, e.g.
 {{{
 w.addheader("To", clientAddr)
 logging.debug("To: %s" % util.logSafely(clientAddr))
 }}}

 Are you opposed to any/all of these ideas?

 >  2) At L207 (ish) in lib/bridgedb/Main.py:
 > {{{
 > 206                 logging.warn("Parsed bridge that we've already
 added. Skipping")
 > 207                 logging.debug("  Bridge: %s" % bridge.getID())
 > }}}
 > This is borking the logfiles, because `getID()` returns the raw SHA-1
 digest, not the hex representation (the latter is already stored in
 `bridge.fingerprint`).
 >

 Borked is such a strong word :) But yeah, I noticed this also. It will be
 best to make this change.

 > I wasn't sure whether or not to make separate bugs for these, since the
 fixes are only like +6/-2 changes.

 You can simply make it a separate commit in the patchset, that should be
 fine.

-- 
Ticket URL: <https://trac.torproject.org/projects/tor/ticket/9199#comment:22>
Tor Bug Tracker & Wiki <https://trac.torproject.org/>
The Tor Project: anonymity online
_______________________________________________
tor-bugs mailing list
tor-bugs@xxxxxxxxxxxxxxxxxxxx
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs