[tor-bugs] #5543 [Tor Directory Authority]: BridgePassword would be insecure if anybody used it

[Tor Bug Tracker & Wiki]

#5543: BridgePassword would be insecure if anybody used it
-------------------------------------+--------------------------------------
 Reporter:  nickm                    |          Owner:                    
     Type:  defect                   |         Status:  needs_review      
 Priority:  major                    |      Milestone:  Tor: 0.2.2.x-final
Component:  Tor Directory Authority  |        Version:                    
 Keywords:                           |         Parent:                    
   Points:                           |   Actualpoints:                    
-------------------------------------+--------------------------------------
Changes (by nickm):

  * status:  needs_revision => needs_review


Comment:

 Replying to [comment:2 rransom]:
 > Replying to [comment:1 nickm]:
 > > Please review branch "bridgepassword" on 0.2.2.x in my public
 repository.
 >
 > `base64_encode` is probably not protected against side-channel leaks.  I
 don't know whether that's a problem; leaks there can only be exploited by
 observing the bridge authority while someone who knows BridgePassword
 fetches the consensus from it.

 I'm missing something there.  I thought we no longer called base64_encode
 in response to incoming authenticators.  At least, I hope we don't?

 > If `alloc_http_authenticator` fails, `BridgePassword_AuthDigest` is
 silently not set.  That would be a royal PITA to debug if it could ever
 happen.

 Ick, yeah.  Better fix that.

 > Storing BridgePassword as a digest isn't what prevents timing attacks,
 it's what allows you to use a timing-attack-resistant comparison function
 with it.  (That's quite a subtle distinction, but still important enough
 to justify correcting the comment.)

 There too.  Please see branch now?

 > Other than that, looks good.
 >
 >
 > > For fun, you can also see branch "di_strcmp" in my public repository:
 that's how you do a one-sided-data-independent strcmp, I think.  But the
 approach in "bridgepassword" is more solid, I think.
 >
 > `di_strcmp` is broken: it uses secret information (the length of
 `target`) to determine what memory location (`ba`) to read from.

 Argh, you're right.  Perhaps if seymour cray rises from the grave and
 abolishes all cache everywhere, it will be a good idea.  Force-pushed a
 version with an updated commit message to indicate that it is broken.

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