Project

General

Profile

Feature #6305

Feature #5479: Bridge support

Feature #6839: Tor bridges support: complete phase two

Write tests for Tor bridges

Added by bertagaz over 6 years ago. Updated almost 5 years ago.

Status:
Resolved
Priority:
Normal
Assignee:
-
Category:
Test suite
Target version:
Start date:
09/26/2013
Due date:
% Done:

100%

Feature Branch:
test/6305-tor-bridges
Type of work:
Code
Blueprint:
Starter:
No
Affected tool:

Description

Write tests to ensure that the bridge functionnality is well supported in Tails.


Related issues

Blocked by Tails - Feature #7821: Automatic tests for Tor Resolved 08/26/2014

Associated revisions

Revision 6deca5af (diff)
Added by Tails developers almost 5 years ago

The Tor bridges tests has now been automated (Will-fix: #6305).

Revision 9faac2c6
Added by Tails developers almost 5 years ago

Merge branch 'test/6305-tor-bridges' into testing

Fix-committed: #6305

History

#1 Updated by intrigeri over 6 years ago

  • Target version set to Sustainability_M1

#2 Updated by intrigeri over 6 years ago

  • Subject changed from Write bridges tests to Write tests for Tor bridges

#3 Updated by intrigeri almost 6 years ago

  • Parent task changed from #6298 to #6839

#4 Updated by intrigeri over 5 years ago

#5 Updated by intrigeri over 5 years ago

#6 Updated by intrigeri over 5 years ago

  • Assignee set to anonym
  • Target version changed from Sustainability_M1 to Tails_1.2.2

#10 Updated by anonym about 5 years ago

  • Status changed from Confirmed to In Progress
  • % Done changed from 0 to 30
  • Feature Branch set to test/6305-tor-bridges

Did some minimal work on this today, which actually may be enough:

  • While it only tests obfs3 bridges, I think that's enough (until we support scramblesuit, i.e. #7909), since "vanilla" and obfs2 bridges are obsolete. Adding scenarios for them would be little more than a trivial copy-paste away, but IMHO that'd only make the test suite run slower.
  • Also, in the manual test suite's bridge section we have the vague "Use the Internet" step, to generate some network activity. The current tests only activity is the actual Tor bootstrap and the upgrade check. If that isn't enough I suppose can, quite arbitrarily, throw in a "check that Tails' website loads in the Tor Browser" step too.

#11 Updated by intrigeri about 5 years ago

anonym wrote:

Did some minimal work on this today, which actually may be enough:

Congrats!

  • While it only tests obfs3 bridges, I think that's enough (until we support scramblesuit, i.e. #7909), since "vanilla" and obfs2 bridges are obsolete. Adding scenarios for them would be little more than a trivial copy-paste away, but IMHO that'd only make the test suite run slower.

I don't see regular bridges as obsolete (people for whom they work should not consume resources from the smaller PT bridge pools IMO), and I see value in testing both bridges that are handled by Tor itself in addition to bridges that are managed by PTs.

  • Also, in the manual test suite's bridge section we have the vague "Use the Internet" step, to generate some network activity. The current tests only activity is the actual Tor bootstrap and the upgrade check.

Good enough.

#12 Updated by anonym about 5 years ago

  • Target version changed from Tails_1.2.2 to Tails_1.2.3

#13 Updated by anonym about 5 years ago

  • Target version changed from Tails_1.2.3 to Tails_1.3

#14 Updated by Tails almost 5 years ago

Applied in changeset commit:2c420ac45ea9a92d4fec5b81071adbfbcc14d539.

#15 Updated by anonym almost 5 years ago

#16 Updated by anonym almost 5 years ago

#17 Updated by anonym almost 5 years ago

  • Assignee changed from anonym to kytv
  • % Done changed from 30 to 50
  • QA Check set to Ready for QA

Forced pushed a completely re-worked version of the branch. It's based on test/7821-tor, which isn't merged as of writing this, so I added a blocker to #7821. I'd still appreciate an initial code review even before test/7821-tor is merged, is convenient.

#18 Updated by anonym almost 5 years ago

anonym wrote:

I'd still appreciate an initial code review even before test/7821-tor is merged, is convenient.

Tip for this:

git log --cc --reverse -p  origin/testing..origin/test/6305-tor-bridges ^origin/test/7821-tor

#19 Updated by kytv almost 5 years ago

Initial comments:

One problem, so far, perhaps with wiki/src/contribute/release_process/test/usage.mdwn or features/step_definitions/tor.rb.

Running the time_syncing.feature failed with

 Scenario: Clock with host's time in bridge mode                        # features/time_syncing.feature:54
    When the network is plugged                                          # features/step_definitions/common_steps.rb:112
    And the Tor Launcher autostarts                                      # features/step_definitions/tor.rb:322
    And I configure some Bridge pluggable transports in Tor Launcher     # features/step_definitions/tor.rb:327
      uninitialized constant AssertionFailedError (NameError)
      ./features/step_definitions/tor.rb:335:in `rescue in block in <top (required)>'
      ./features/step_definitions/tor.rb:331:in `/^I configure some (\w+) pluggable transports in Tor Launcher$/'
      features/time_syncing.feature:57:in `And I configure some Bridge pluggable transports in Tor Launcher'
    And Tor is ready                                                     # features/step_definitions/common_steps.rb:306
    Then Tails clock is less than 5 minutes incorrect                    # features/step_definitions/time_syncing.rb:35

My features/config/local.yml contained lines like

Tor:
  Transports:
    bridge:
      - ipv4_address: "xxx.yyy.zzz.aaa" 
        ipv4_port: xxxx
        fingerprint: "SOMEFINGERPRINT" 
        extra:
 .....

and wiki/src/contribute/release_process/test/usage.mdwn contains

where the type `$TYPE` (and `$ANOTHER_TYPE`) should be something like
`obfs4` or `bridge` (the first type) or whatever Tor calls them. ...

When I changed the yml file to contain Bridge the test proceeded.

To be continued...

#20 Updated by kytv almost 5 years ago

  • Assignee changed from kytv to anonym
  • QA Check changed from Ready for QA to Dev Needed

The only problem that I saw was with respect to the config file handling (and maybe the documentation).

If we want to mandate that the bridge_type is upcase!'d and the config file specifies obfs3 I should see the warning

It seems no '#{bridge_type}' pluggable transports are defined in your local configuration file

but instead I saw

uninitialized constant AssertionFailedError (NameError)

When bridge_type is in the correct format in the yaml file the tests--AFAICT--test what should be tested.

If bridge_type should be specified with a leading capital letter:

diff --git a/wiki/src/contribute/release_process/test/usage.mdwn b/wiki/src/contribute/release_process/test/usage.mdwn
index 813d677..9376cee 100644
--- a/wiki/src/contribute/release_process/test/usage.mdwn
+++ b/wiki/src/contribute/release_process/test/usage.mdwn
@@ -103,12 +103,12 @@ The format is:
           [...]

 where the type `$TYPE` (and `$ANOTHER_TYPE`) should be something like
-`obfs4` or `bridge` (the first type) or whatever Tor calls them. Both
+`Obfs4` or `Bridge` (the first type) or whatever Tor calls them. Both
 `fingerprint` and `extra` are optional and can be left empty (or
-skipped completely), but e.g. `extra` is necessary for `obfs4` type
+skipped completely), but e.g. `extra` is necessary for `Obfs4` type
 bridges, for the `cert=... iat-mode=...` stuff, and the same for
-`scramblesuite`'s `password=...`.
+`Scramblesuite`'s `password=...`.

 This setting is required for `tor_bridges.feature` (requires types
-`bridge`, `obfs2`, `obfs3` and `obfs4`) and `time_syncing.feature`
-(requires type `bridge` only).
+`Bridge`, `Obfs2`, `Obfs3` and `Obfs4`) and `time_syncing.feature`
+(requires type `Bridge` only).

I'm not sure that's desired but I can see how it'd make sense to have the scalars capitalized! with the sequences downcased!.

#21 Updated by anonym almost 5 years ago

  • Assignee changed from anonym to kytv
  • QA Check changed from Dev Needed to Ready for QA

kytv wrote:

@uninitialized constant AssertionFailedError (NameError)|

Oops, now fixed.

If bridge_type should be specified with a leading capital letter:

[...]

Thanks for the patch! Now applied.

#22 Updated by intrigeri almost 5 years ago

Code review passes. Not tested.

#23 Updated by Tails almost 5 years ago

  • Status changed from In Progress to 11
  • % Done changed from 50 to 100

Applied in changeset commit:4bb35fc063a9fe695a3da7df1062b11d9fc50157.

#24 Updated by intrigeri almost 5 years ago

  • Assignee deleted (kytv)
  • QA Check changed from Ready for QA to Pass

#25 Updated by BitingBird almost 5 years ago

  • Status changed from 11 to Resolved

Also available in: Atom PDF