[tor-bugs] #1837 [BridgeDB]: bridgedb learns to load file of which bridges are blocked where

[Tor Bug Tracker & Wiki]

#1837: bridgedb learns to load file of which bridges are blocked where
----------------------+-----------------------------------------------------
 Reporter:  arma      |          Owner:              
     Type:  defect    |         Status:  needs_review
 Priority:  normal    |      Milestone:              
Component:  BridgeDB  |        Version:              
 Keywords:            |         Parent:  #1608       
   Points:            |   Actualpoints:              
----------------------+-----------------------------------------------------

Comment(by aagbsn):

 Replying to [comment:5 karsten]:
 > Code looks good.  A few comments:
 >
 >  - Can you rebase this branch to origin/master, too?

 this seems to be replaying the entire branch over origin/master (again).
 Perhaps it makes more sense to apply the diff as a patch than to replay
 the branch on top of origin/master again?

 >
 >  - What's the format of the `blocked-bridges` file?  I think this should
 go into the bridge spec (which is not in origin/master, yet).

 1 block entry per line: (do not include < >)
 fingerprint <bridge fingerprint> country-code <country code>

 I've also updated the README accordingly
 >
 >  - I found that you're removing blocked bridges from the result, similar
 to the /16 filtering.  I'm not sure if this is the right thing to do.
 Maybe it is.  I'm adding this comment mostly for myself to think about it
 when looking at your branch again.

 I believe this is the correct approach -- if bridgedb always returns 3
 good bridges, an adversary could iteratively request and block bridges
 until there are none left.
 >
 >  - You have the line `self.db = db = bridgedb.Storage.getDB()` in both
 BridgeBlock and CountryBlock.  I think BridgeBlock doesn't need it.  Also,
 what's the `db` in the middle doing?
 >
 >  - Can you add two log statements for successfully loading and failing
 to load the GeoIP database?

 done
 >
 >  - You have at least two instances of `SELECT * FROM` in your code where
 you reference columns by their index.  This is fragile, because someone
 might overlook these places when changing the schema.  Better put the
 column names in here.  Also, is the `(fingerprint,)` and `(countryCode,)`
 syntax correct?
 >
 yes, the extra ',' is correct.

 I have replaced the SELECT *, and added another test for
 getBlockingCountries to Tests.py.

 >  - How would I migrate the schema from version 1 to 2?  Can you add a
 sentence or two to the README for that?

 there are a couple extra tables that need to be created, and the schema-
 version needs to be updated:

 CREATE TABLE BlockedBridges ( id INTEGER PRIMARY KEY NOT NULL, hex_key,
 blocking_country);
 CREATE INDEX BlockedBridgesBlockingCountry on BlockedBridges(hex_key);
 REPLACE INTO Config VALUES ( 'schema-version', 2 );

 README also updated.

 >
 > Thanks!

 Thanks for your feedback!

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