Opened 7 years ago

Closed 6 years ago

Last modified 6 years ago

#8614 closed defect (worksforme)

BridgeDB should be able to return multiple transport types at the same time

Reported by: asn Owned by: sysrqb
Priority: Medium Milestone:
Component: Circumvention/BridgeDB Version:
Severity: Keywords: bridgedb-dist
Cc: isis Actual Points:
Parent ID: #8615 Points:
Reviewer: Sponsor:

Description

In https://lists.torproject.org/pipermail/tor-dev/2013-March/004505.html we decided to add a "Get Bridges" button to the HTTP interface of BridgeDB, which will provide users with bridges compatible with the latest obfsbundle.

This means that if the latest obfsbundle supports both obfs2 and obfs3 bridges, the "Get Bridges" button should return both obfs2 and obfs3 bridges (and some normal ones too).

The current BridgeDB code does not allow to return multiple types of bridges at the same time.

Child Tickets

Change History (21)

comment:1 Changed 7 years ago by asn

Parent ID: #8615

comment:2 Changed 7 years ago by sysrqb

First stab at this.

In summary, BridgeDB now accepts a line in the configuration file which specifies how many bridges it should try to return which support given transports. With this, it attempts to satisfy the requirement(s) with the set of bridges it selects from the ring. I also increased the number of bridges BridgeDB will retrieve from the ring by a factor of 3. This should allow us to return regular, obfs2 and obfs3 bridges, on an average request. Normally it only selects the exact number of bridges it needs, but we really need to grab a few extra so that we can hopefully get at least one of each.

branch bug8614 in https://github.com/sysrqb/bridgedb.git

comment:3 Changed 7 years ago by asn

Status: newneeds_revision

Quick review:

  • The code of BridgeDB is largely undocumented; when you change an undocumented function, can you add a tiny amount of documentation on what it does? In the long run, this process is going to improve the code quality considerably.
  • Why did you teach parseORAddressLine about transports? Isn't it supposed to just parse OR addresses? It seems that you are feeding it the output of getConfigLine(), that's why. Either the function name is wrong, or that function does more than it's supposed to do.
  • Try to do more helpful documentation. Examples:
    # Order the list of bridges such that some prefix satisfies a requirement
    # for certain pluggable transports
    

What is the "prefix" in this case?

# Check that we are returning the specified number of transports
# in this list of bridges

what is the "specified number"? what is the 'remain' argument?

        # Let's grab a few extra, just in case

"just in case" what?

Sorry for bothering you about comments, but the BridgeDB code is already undocumented and confusing, and we shouldn't convolute it further (if we want it to remain maintainable in the future). If you are looking for tips on Python function documentation, check out other projects (like 'stem' if you want a paranoid approach). IMO, documentation is extremely important in dynamically-typed languages, otherwise the code becomes unmaintainable very easily.

  • In checkTransportRequirement() you do trans = transports and work with trans from that point and on. That's an unusual pattern.
  • In the needTransports tuple, while you can specify a nonexistent maxcount, you can't do the same for mincount. Is this a feature or a bug? Also, DOCDOC.
  • f032596fc9e320d87b18f3cd88a7559cd8eccf7f might be an overkill. The user never requests a specific number of bridges; bridgedb returns a number of bridges in a best-effort basis.

Even if you wanted to notify the user that we don't have enough bridges in the database, the exception-based approach you are taking might be a bit too messy. While your approach is not wrong, if in the future you wanted to notify the user, I think you could have achieved the same result with a warnTheUserAboutBridgesScarcity() callback from within getNumBridgesPerAnswer(). In any case, if you like your approach, stick with it.

  • In the beginning of testMeetRequirementWithMixedSet2() you do some things with the types dictionary and then you initialize it back to the empty dictionary again.

Hopefully Aaron can take a look at your branch too.

Thanks for all the code!

comment:4 Changed 7 years ago by asn

(Oh also you have some trailing whitespace in your unit tests. Maybe tweak your editor so that it highlights or erases trailing whitespace?)

comment:5 in reply to:  3 ; Changed 6 years ago by sysrqb

Status: needs_revisionneeds_review

Replying to asn:

Quick review:

Thanks again for the review.

  • The code of BridgeDB is largely undocumented; when you change an undocumented function, can you add a tiny amount of documentation on what it does? In the long run, this process is going to improve the code quality considerably.

Yeah, I completely agree.

  • Why did you teach parseORAddressLine about transports? Isn't it supposed to just parse OR addresses? It seems that you are feeding it the output of getConfigLine(), that's why. Either the function name is wrong, or that function does more than it's supposed to do.

Heh, yeah, I should have been more careful about that. Tests started failing when I started
testing the PT code. This was a sloppy solution. It's better now.

  • Try to do more helpful documentation. Examples:
    # Order the list of bridges such that some prefix satisfies a requirement
    # for certain pluggable transports
    

What is the "prefix" in this case?

# Check that we are returning the specified number of transports
# in this list of bridges

what is the "specified number"? what is the 'remain' argument?

        # Let's grab a few extra, just in case

"just in case" what?

I thought those comments were more than sufficiently descriptive :)

Sorry for bothering you about comments, but the BridgeDB code is already undocumented and confusing, and we shouldn't convolute it further (if we want it to remain maintainable in the future). If you are looking for tips on Python function documentation, check out other projects (like 'stem' if you want a paranoid approach). IMO, documentation is extremely important in dynamically-typed languages, otherwise the code becomes unmaintainable very easily.

Nope, you're completely justified in this. I guess we're following stem's path.

  • In checkTransportRequirement() you do trans = transports and work with trans from that point and on. That's an unusual pattern.

It reduced line wraps, but that's silly, so now that function only uses trans

  • In the needTransports tuple, while you can specify a nonexistent maxcount, you can't do the same for mincount. Is this a feature or a bug? Also, DOCDOC.

Both? It's actually a TODO. I was more concerned with verifying the minimum than I was/am about the max. I'll implement it, but it's not essential for this patch. We can entirely pull it out if I don't finish it.

  • f032596fc9e320d87b18f3cd88a7559cd8eccf7f might be an overkill. The user never requests a specific number of bridges; bridgedb returns a number of bridges in a best-effort basis.

Even if you wanted to notify the user that we don't have enough bridges in the database, the exception-based approach you are taking might be a bit too messy. While your approach is not wrong, if in the future you wanted to notify the user, I think you could have achieved the same result with a warnTheUserAboutBridgesScarcity() callback from within getNumBridgesPerAnswer(). In any case, if you like your approach, stick with it.

I agree. It seemed like a good idea at first...gone.

  • In the beginning of testMeetRequirementWithMixedSet2() you do some things with the types dictionary and then you initialize it back to the empty dictionary again.

Result of not removing debugging stuff.

Hopefully Aaron can take a look at your branch too.

Thanks for all the code!

Thanks! New branch bug8614_3, rebased and such.

Also, re: whitespace in Tests, I fixed those too.

comment:6 in reply to:  5 Changed 6 years ago by sysrqb

Replying to sysrqb:

Replying to asn:

Quick review:

  • In the needTransports tuple, while you can specify a nonexistent maxcount, you can't do the same for mincount. Is this a feature or a bug? Also, DOCDOC.

Both? It's actually a TODO. I was more concerned with verifying the minimum than I was/am about the max. I'll implement it, but it's not essential for this patch. We can entirely pull it out if I don't finish it.

Ok, I just pushed an update as branch bug8614_4 that implements this.

comment:7 Changed 6 years ago by sysrqb

Please test/review branch bug8614_5_rebased. It's rebased on top of current master.

One thing to note, when a user applies different filters they will (likely) receive different bridges. I think we should decide that this is ok before this is merged.

comment:8 in reply to:  7 Changed 6 years ago by sysrqb

Replying to sysrqb:

Please test/review branch bug8614_5_rebased. It's rebased on top of current master.

I forgot to mention this adds another unit test and corrects the selection function so that when a bridge is selected to meet a requirement because it supports a certain PT, we now make a deepcopy of the bridge and modify it such that it only supports the single transport we need. The previous version guessed (randomly) which transport we wanted to return. This was silly because we chose the bridge because it supported a specific transport, so that is the transport we should use.

comment:9 Changed 6 years ago by sysrqb

Keywords: important added

comment:10 in reply to:  7 Changed 6 years ago by sysrqb

Replying to sysrqb:

One thing to note, when a user applies different filters they will (likely) receive different bridges. I think we should decide that this is ok before this is merged.

I don't think this is ok, but I also don't think we can accomplish this goal without creating a temporary cache containing the mapping area->distributed bridges. This has numerous consequences, but it will also provide a significant improvement with regard to how long it takes to scrape new bridges. I propose the cache be indexed by an hmac or salted double hash of the ip addr area and we rotate the key/salt and clear the cache every three hours (so that the rate-limiting occurs in the same time period as the email dist).

Advantage: everyone within the same /24 receives the same bridges
Disadvantage: Every request sourced from within a small geographical location receives the same bridges. =\

comment:11 Changed 6 years ago by sysrqb

Please review bug8614_5_rebased_r4 in my public repo.

I think I want to split the first commit and I need to rebase the set so that the merge conflict resolution is prettier, but these are minor things.

Also some lines are too long...I'll fix that too.

Last edited 6 years ago by sysrqb (previous) (diff)

comment:12 Changed 6 years ago by sysrqb

Prettierer in bug8614_5_rebased_r7.

I also opened #9652 because even though this branch now caches the responses (to prevent significant leakage), it only caches the bridges so if any filters change between requests then we may return a different PT for the same bridge. From a usability standpoint this is...more useful, compared to "I asked for obfs3 bridges, but I'm still getting the obfs2 bridges I got last time! What am I doing wrong?". However, it doesn't help as much with preventing enumeration.

comment:13 Changed 6 years ago by asn

Partial review of bug8614_5_rebased_r4 follows. Haven't seen bug8614_5_rebased_r7 yet.

  • 0e91c34375804e0f42ece1b89f26f0db7c1405d4 seems a bit messy. Why would transports be True? Isn't it supposed to be a pluggable transport method name (string?)?
  • Thanks for the extra docs! Here are two places where I would appreciate some more docs:
  • I think that there should be some docs on the body of getBridgesForIP() giving an overview of the algorithm used. That function is pretty important but long, borderline messy (at least there are tests) and its logic is not apparent.
  • Would be nice to have an example of what we are trying to parse in the docstring of findORAddrSubstringInBridgeLine(). Also, what's up with the v variable? Is it like a debug variable? Why not just use logging.debug() or the bridgedb equivalent?
  • if clauses in testDistWithFilterTransport() seem to be the same.

comment:14 in reply to:  13 Changed 6 years ago by sysrqb

Replying to asn:

Partial review of bug8614_5_rebased_r4 follows. Haven't seen bug8614_5_rebased_r7 yet.

Thanks

  • 0e91c34375804e0f42ece1b89f26f0db7c1405d4 seems a bit messy. Why would transports be True? Isn't it supposed to be a pluggable transport method name (string?)?

I was overloading the variable, I'm not doing this anymore, hopefully it is more understandable now.

  • Thanks for the extra docs! Here are two places where I would appreciate some more docs:
  • I think that there should be some docs on the body of getBridgesForIP() giving an overview of the algorithm used. That function is pretty important but long, borderline messy (at least there are tests) and its logic is not apparent.

Done, hopefully it explains the process well enough.

  • Would be nice to have an example of what we are trying to parse in the docstring of findORAddrSubstringInBridgeLine(). Also, what's up with the v variable? Is it like a debug variable? Why not just use logging.debug() or the bridgedb equivalent?

Those v things are gone, I was using them for debugging but I forgot to remove them before I pushed. I also added an example of how to use this function and its output.

  • if clauses in testDistWithFilterTransport() seem to be the same.

Indeed, I don't remember why this was done. I removed the duplication.

New and improved: see bug8614_6_rebased_1

comment:15 Changed 6 years ago by sysrqb

Cc: isis added
Owner: set to sysrqb
Status: needs_reviewassigned

(I guess I might as well own this while I'm at it)

comment:16 Changed 6 years ago by sysrqb

Status: assignedneeds_review

comment:17 Changed 6 years ago by isis

In lib/bridgedb/Bridges.py, in function reorderBridgesByTransportRequirement from commit 8641da059e2e925d6f5a45bd1ecf23f7e44b3d36, the minimum required transport number is decremented, and then the maximum (if not None) is also decremented:

+    for tp,minnum,maxnum in transports:
+        trans[tp] = [minnum, maxnum]
[...]
+    for bridge in bridges:
+        for tpt in bridge.transports:
+            tp = tpt.methodname
+            if tp in trans:
+                if trans[tp][0] > 0:
[...]
+                    bridges2.insert(0, bridge2)
+                    trans[tp][0] -= 1
+                    if trans[tp][1]:
+                        trans[tp][1] -= 1
+                        assert trans[tp][0] <= trans[tp][1]
+                    break

It seems like minnum is not needed...for example, if the config file specified FORCE_TRANS = [('obfs2, 2, 2)] then after the first obfs2 bridge is found, both counters are decremented to ('obfs2, 1, 1) and then the assertion fails. If FORCE_TRANS = [('obfs2', 1, 3)] then we grab the first obfs2 bridge, put it in the bridges2 list, and jump to this block:

+                elif trans[tp][1] and trans[tp][1] > 0:
+                    if len(bridge.transports) > 1:
+                        bridge2 = deepcopy(bridge)
+                        bridge2.transports = [
+                            PluggableTransport(bridge2, tp, tpt.address, tpt.port)
+                        ]
+                    else:
+                        bridge2 = bridge
+                    bridges2.insert(0, bridge2)

I think I'm not understanding why minnum is needed...wouldn't it just be easier to only have a maximum, and then try to add PluggableTransports until either maxnum is reached or we can't find any more?


There is a still a bit of code duplication which could be functionalised (which might also improve readability). For example, this block in reorderBridgesByTransportRequirement happens twice:

+                    if len(bridge.transports) > 1:
+                        bridge2 = deepcopy(bridge)
+                        bridge2.transports = [
+                            PluggableTransport(bridge2, tp, tpt.address, tpt.port)
+                        ]
+                    else:
+                        bridge2 = bridge
+                    bridges2.insert(0, bridge2)

Perhaps this could go into a new function like copyBridgeIntoBridgesList or something similar? (Doing this might depend on how the previously mentioned minnum thing is dealt with.)


This check, in reorderBridgesByTransportRequirement, to make sure that the lists of bridge fingerprints are the same:

+    copied = False
+    num_not_copied = 0
+    for bridge in bridges:
+        for bridge2 in bridges2:
+            if bridge.fingerprint == bridge2.fingerprint:
+                copied = True
+                break
+        if not copied:
+            num_not_copied += 1
+            bridges2.append(bridge)
+        copied = False
+
+    assert len(bridges2) == len(bridges), "Should be %d, but is %d. Appended %d" % (len(bridges), len(bridges2), num_not_copied)

seems a bit expensive, especially since you're already iterated over the bridges right before this block. Rather then indexing again to add the fingerprint here, you could do it during the previous loop. Or, in the previous loop, you could remove a bridge from bridges once done processing it's bridge.transports, and then just keep going until there aren't any `bridges left.

The other thing which is less expensive would be to check like this:

bridgeSet1 = set(bridges)
bridgeSet2 = set(bridges2)
try:
     # All the bridges which are in one set or the other, but not in both
     assert bridgeSet1.symmetric_difference(bridgeSet2) == 0
except AssertionError:
     # All the bridges in `bridges` which didn't end up in `bridges2`:
     not_copied = bridgeSet1.difference(bridgeSet2)

and then just iterate over / deal with the not_copied ones. (Set operations are way faster.)


The documentation looks amazing! :D


Also, I am now noticing, again in reorderBridgesByTransportRequirement, that tpt is (or, at least, it should be) an instance of the PluggableTransport class:

+    for bridge in bridges:
+        for tpt in bridge.transports:
+            tp = tpt.methodname
+            if tp in trans:
+                if trans[tp][0] > 0:
+                    if len(bridge.transports) > 1:
+                        bridge2 = deepcopy(bridge)
+                        bridge2.transports = [
+                            PluggableTransport(bridge2, tp, tpt.address, tpt.port)
+                        ]
+                    else:
+                        bridge2 = bridge
+                    bridges2.insert(0, bridge2)

If that is the case, then I don't understand why the whole bridge must be copied, since :attr:PluggableTransport.bridge already is the bridge. In other words, why not just do:

+    for bridge in bridges:
+        for tpt in bridge.transports:
+            tp = tpt.methodname
+            if tp in trans:
+                if trans[tp][0] > 0:
-                    if len(bridge.transports) > 1:
-                        bridge2 = deepcopy(bridge)
-                        bridge2.transports = [
-                            PluggableTransport(bridge2, tp, tpt.address, tpt.port)
-                        ]
+                    bridge2 = deepcopy(tpt.bridge)
+                    bridge2.transports.append(PluggableTransport(bridge2, tp, tpt.address, tpt.port))
+                    bridges2.insert(0, bridge2)

Appending the PluggableTransport to bridge2.transports might not even be necessary...since it's a deepcopy, and the Bridge and PluggableTransport classes circularly reference each other, the deepcopy might already include the PluggableTransport. However, since the PluggableTransport class references the instance of the Bridge class in its constructor, the deepcopied one would reference the bridge instance of Bridge, not the bridge2 instance.

Jesus fatherfucking shitpile son of a politician FUCK. grumble grumble...should just rewrite the whole damned program...grumble...

Either way, this one is a small problem, and probably not worth wasting your time with the effort to figure out what's going on with the circular references, especially if your version works.

Ugh...note to self: XXX FIXME for the BridgePluggableTransport references.


More later.

comment:18 in reply to:  17 ; Changed 6 years ago by sysrqb

Replying to isis:

Thanks for the review!

In lib/bridgedb/Bridges.py, in function reorderBridgesByTransportRequirement from commit 8641da059e2e925d6f5a45bd1ecf23f7e44b3d36, the minimum required transport number is decremented, and then the maximum (if not None) is also decremented:

+    for tp,minnum,maxnum in transports:
+        trans[tp] = [minnum, maxnum]
[...]
+    for bridge in bridges:
+        for tpt in bridge.transports:
+            tp = tpt.methodname
+            if tp in trans:
+                if trans[tp][0] > 0:
[...]
+                    bridges2.insert(0, bridge2)
+                    trans[tp][0] -= 1
+                    if trans[tp][1]:
+                        trans[tp][1] -= 1
+                        assert trans[tp][0] <= trans[tp][1]
+                    break

It seems like minnum is not needed...for example, if the config file specified FORCE_TRANS = [('obfs2, 2, 2)] then after the first obfs2 bridge is found, both counters are decremented to ('obfs2, 1, 1) and then the assertion fails.

Maybe I'm only seeing what I want to see, but why does the assertion fail?

If FORCE_TRANS = [('obfs2', 1, 3)] then we grab the first obfs2 bridge, put it in the bridges2 list, and jump to this block:

+                elif trans[tp][1] and trans[tp][1] > 0:
+                    if len(bridge.transports) > 1:
+                        bridge2 = deepcopy(bridge)
+                        bridge2.transports = [
+                            PluggableTransport(bridge2, tp, tpt.address, tpt.port)
+                        ]
+                    else:
+                        bridge2 = bridge
+                    bridges2.insert(0, bridge2)

I think I'm not understanding why minnum is needed...wouldn't it just be easier to only have a maximum, and then try to add PluggableTransports until either maxnum is reached or we can't find any more?

Yes, this makes more sense, done. Originally I only had a min and we stopped searching when we hit it, but then it seemed like a good idea to return more if we have them, so I added in a max.


There is a still a bit of code duplication which could be functionalised (which might also improve readability). For example, this block in reorderBridgesByTransportRequirement happens twice:

+                    if len(bridge.transports) > 1:
+                        bridge2 = deepcopy(bridge)
+                        bridge2.transports = [
+                            PluggableTransport(bridge2, tp, tpt.address, tpt.port)
+                        ]
+                    else:
+                        bridge2 = bridge
+                    bridges2.insert(0, bridge2)

Perhaps this could go into a new function like copyBridgeIntoBridgesList or something similar? (Doing this might depend on how the previously mentioned minnum thing is dealt with.)

Yup, gone.


This check, in reorderBridgesByTransportRequirement, to make sure that the lists of bridge fingerprints are the same:

+    copied = False
+    num_not_copied = 0
+    for bridge in bridges:
+        for bridge2 in bridges2:
+            if bridge.fingerprint == bridge2.fingerprint:
+                copied = True
+                break
+        if not copied:
+            num_not_copied += 1
+            bridges2.append(bridge)
+        copied = False
+
+    assert len(bridges2) == len(bridges), "Should be %d, but is %d. Appended %d" % (len(bridges), len(bridges2), num_not_copied)

seems a bit expensive, especially since you're already iterated over the bridges right before this block. Rather then indexing again to add the fingerprint here, you could do it during the previous loop. Or, in the previous loop, you could remove a bridge from bridges once done processing it's bridge.transports, and then just keep going until there aren't any `bridges left.

The other thing which is less expensive would be to check like this:

bridgeSet1 = set(bridges)
bridgeSet2 = set(bridges2)
try:
     # All the bridges which are in one set or the other, but not in both
     assert bridgeSet1.symmetric_difference(bridgeSet2) == 0
except AssertionError:
     # All the bridges in `bridges` which didn't end up in `bridges2`:
     not_copied = bridgeSet1.difference(bridgeSet2)

and then just iterate over / deal with the not_copied ones. (Set operations are way faster.)

Hm, sets, that could work. I admit it is expensive, but in reality we're talking about a dozen bridges, so there performance penalty is minimal. Regardless, I agree we shouldn't be needlessly wasteful.


The documentation looks amazing! :D

You had to find something positive to comment on, huh. :)


Also, I am now noticing, again in reorderBridgesByTransportRequirement, that tpt is (or, at least, it should be) an instance of the PluggableTransport class:

+    for bridge in bridges:
+        for tpt in bridge.transports:
+            tp = tpt.methodname
+            if tp in trans:
+                if trans[tp][0] > 0:
+                    if len(bridge.transports) > 1:
+                        bridge2 = deepcopy(bridge)
+                        bridge2.transports = [
+                            PluggableTransport(bridge2, tp, tpt.address, tpt.port)
+                        ]
+                    else:
+                        bridge2 = bridge
+                    bridges2.insert(0, bridge2)

If that is the case, then I don't understand why the whole bridge must be copied, since :attr:PluggableTransport.bridge already is the bridge. In other words, why not just do:

+    for bridge in bridges:
+        for tpt in bridge.transports:
+            tp = tpt.methodname
+            if tp in trans:
+                if trans[tp][0] > 0:
-                    if len(bridge.transports) > 1:
-                        bridge2 = deepcopy(bridge)
-                        bridge2.transports = [
-                            PluggableTransport(bridge2, tp, tpt.address, tpt.port)
-                        ]
+                    bridge2 = deepcopy(tpt.bridge)
+                    bridge2.transports.append(PluggableTransport(bridge2, tp, tpt.address, tpt.port))
+                    bridges2.insert(0, bridge2)

I don't quite understand this. What's the different between these?

                    bridge2 = deepcopy(bridge)
                    bridge2 = deepcopy(tpt.bridge)

they should be identical, afaict, yes? The reasoning for

                        bridge2.transports = [
                            PluggableTransport(bridge2, tp, tpt.address, tpt.port)
                        ]

was to overwrite the reference to the deepcopied list (because that's not what we want), and this is what you hint at below. The reasoning for doing this probably isn't clear, though. If we select a bridge specifically because it supports a PT, but we don't modify the bridge, then when we return it to the distributor it does not know which PT to tell the user. By making a copy of the bridge that only contains the needed PT we increase the chance that the distributor will tell the user the correct PT (verses the OR Port, this is where #9652 came from).

Appending the PluggableTransport to bridge2.transports might not even be necessary...since it's a deepcopy, and the Bridge and PluggableTransport classes circularly reference each other, the deepcopy might already include the PluggableTransport. However, since the PluggableTransport class references the instance of the Bridge class in its constructor, the deepcopied one would reference the bridge instance of Bridge, not the bridge2 instance.

Jesus fatherfucking shitpile son of a politician FUCK. grumble grumble...should just rewrite the whole damned program...grumble...

Either way, this one is a small problem, and probably not worth wasting your time with the effort to figure out what's going on with the circular references, especially if your version works.

Ugh...note to self: XXX FIXME for the BridgePluggableTransport references.

aaaaand deep breaths!


More later.

Thanks for this, so far!

comment:19 in reply to:  18 Changed 6 years ago by sysrqb

Replying to sysrqb:

I think I'm not understanding why minnum is needed...wouldn't it just be easier to only have a maximum, and then try to add PluggableTransports until either maxnum is reached or we can't find any more?

Yes, this makes more sense, done. Originally I only had a min and we stopped searching when we hit it, but then it seemed like a good idea to return more if we have them, so I added in a max.

Hm, actually, think about this when you review checkTransportRequirement(). If we do away with min, should we always check remain unless we hit max?

comment:20 Changed 6 years ago by sysrqb

Ok, I decided that it was a good idea to simply the logic, thus removed the minimum option.
New branch is bug8614_7_rebase_2.

comment:21 Changed 6 years ago by isis

Keywords: bridgedb-dist added; important removed
Resolution: worksforme
Status: needs_reviewclosed

It seemed decided at the PTTBB breakout session at the 2014 Winter Dev meeting that BridgeDB should not hand out multiple transports at the same time, in order to prevent a trivially fingerprintable transport from increasing an adversary's ability to fingerprint other transport types. Also, PTTBB launches PTs when they are requested, so firing them all up at once is bit of a resource drain.

Last edited 6 years ago by isis (previous) (diff)
Note: See TracTickets for help on using tickets.