[Author Prev][Author Next][Thread Prev][Thread Next][Author Index][Thread Index]
Re: [tor-bugs] #1837 [BridgeDB]: bridgedb learns to load file of which bridges are blocked where
#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
_______________________________________________
tor-bugs mailing list
tor-bugs@xxxxxxxxxxxxxxxxxxxx
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs