Tails

#1 Updated by anonym 2015-06-02 14:19:21

• blocks #8538 added

#2 Updated by anonym 2015-06-02 14:36:32

• related to Bug #9478: How to deal with transient network errors in the test suite? added

#3 Updated by intrigeri 2015-06-04 21:25:55

• related to deleted (Bug #9478: How to deal with transient network errors in the test suite?)

#4 Updated by intrigeri 2015-06-04 21:25:57

• blocked by deleted (#8538)

#5 Updated by intrigeri 2015-06-04 21:30:43

• related to Bug #9478: How to deal with transient network errors in the test suite? added

#6 Updated by intrigeri 2015-06-04 21:30:51

• blocks #8538 added

#7 Updated by kytv 2015-06-19 12:54:08

• Status changed from Confirmed to In Progress

#11 Updated by kytv 2015-06-20 03:59:09

• related to Bug #9632: restart-vidalia appears to occasionally fail to actually restart Vidalia added

#12 Updated by kytv 2015-06-20 06:43:36

• % Done changed from 0 to 20

My work in progress:

calling as root: . /usr/local/lib/tails-shell-library/tor.sh; tor_is_working
call returned: [0, "", ""]
spawning as root: restart-vidalia
spawn returned: [0, "", ""]
calling as amnesia: gpg --batch --keyserver-options import-clean --recv-key '10CC5BC7'
call returned: [2, "gpgkeys: HTTP fetch error 35: gnutls_handshake() failed: A TLS fatal alert has been received.\n", "gpg: requesting key 0x10CC5BC7 from hkps server hkps.pool.sks-keyservers.net\ngpg: no valid OpenPGP data found.\ngpg: Total number processed: 0\n"]
Forcing new Tor circuit... (attempt #1)
calling as root: pidof -x -o '%PPID' vidalia
call returned: [1, "", ""]
Vidalia wasn't running. Starting Vidalia...
spawning as root: restart-vidalia
spawn returned: [0, "", ""]
calling as root: pidof -x -o '%PPID' vidalia
call returned: [1, "", ""]
calling as root: pidof -x -o '%PPID' vidalia
call returned: [1, "", ""]
calling as root: pidof -x -o '%PPID' vidalia
call returned: [0, "5875\n", ""]
[log] RIGHT CLICK on (728,11)
[log] CLICK on (782,105)
calling as amnesia: gpg --batch --keyserver-options import-clean --recv-key '10CC5BC7'
call returned: [2, "gpgkeys: HTTP fetch error 35: gnutls_handshake() failed: A TLS fatal alert has been received.\n", "gpg: requesting key 0x10CC5BC7 from hkps server hkps.pool.sks-keyservers.net\ngpg: no valid OpenPGP data found.\ngpg: Total number processed: 0\n"]
Forcing new Tor circuit... (attempt #2)
calling as root: pidof -x -o '%PPID' vidalia
call returned: [0, "5875\n", ""]
[log] RIGHT CLICK on (728,11)
[log] CLICK on (782,105)
calling as amnesia: gpg --batch --keyserver-options import-clean --recv-key '10CC5BC7'
call returned: [2, "gpgkeys: HTTP fetch error 35: gnutls_handshake() failed: A TLS fatal alert has been received.\n", "gpg: requesting key 0x10CC5BC7 from hkps server hkps.pool.sks-keyservers.net\ngpg: no valid OpenPGP data found.\ngpg: Total number processed: 0\n"]
Forcing new Tor circuit... (attempt #3)
calling as root: pidof -x -o '%PPID' vidalia
call returned: [0, "5875\n", ""]
[log] RIGHT CLICK on (728,11)
[log] CLICK on (782,105)
calling as amnesia: gpg --batch --keyserver-options import-clean --recv-key '10CC5BC7'
call returned: [0, "", "gpg: requesting key 0x10CC5BC7 from hkps server hkps.pool.sks-keyservers.net\ngpg: key 0x1D84CCF010CC5BC7: public key \"anonym <anonym@riseup.net>\" imported\ngpg: no ultimately trusted keys found\ngpg: Total number processed: 1\ngpg:               imported: 1  (RSA: 1)\n"]
    Given I fetch the "10CC5BC7" OpenPGP key using the GnuPG CLI without any signatures                                 # features/step_definitions/torified_gnupg.rb:46
    And the GnuPG fetch is successful                                                                                   # features/step_definitions/torified_gnupg.rb:68

#13 Updated by kytv 2015-06-21 04:57:43

• % Done changed from 20 to 40

At this point I’m satisfied that my test is doing what it should. After a quick self-review of the branch I’ll submit for review’n’merge action.

I duplicated the contents of torified_gnupg.feature until there were 66 scenarios and started a test run:

^[[31mFailing Scenarios:^[[0m
^[[31mcucumber features/torified_gnupg.feature:296^[[0m^[[90m # Scenario: Fetching OpenPGP keys using GnuPG should work and be done over Tor.^[[0m

66 scenarios (^[[31m1 failed^[[0m, ^[[32m65 passed^[[0m)
957 steps (^[[32m957 passed^[[0m)
126m44.073s

Investigating the failure, it was unrelated to my work.

Full network capture available at: /home/kytv/git/tails/tmp/torified_gnupg_sniffer.pcap-2015-06-21T03:54:00+00:00
^[[31m      The following IPv4 TCP non-Tor Internet hosts were contacted:^[[0m
^[[31m      93.104.209.61 (RuntimeError)^[[0m
^[[31m      /home/kytv/git/tails/features/support/helpers/firewall_helper.rb:115:in `assert_no_leaks'^[[0m
^[[31m      /home/kytv/git/tails/features/support/hooks.rb:152:in `After'^[[0m
Scenario failed at time 01:25:32
Took screenshot "/home/kytv/git/tails/tmp/torified_gnupg-2015-06-21T03:54:00+00:00.png"

The tests had to be retried a few times:

# grep 'Forcing\|abort'  gpg.log 
Forcing new Tor circuit... (attempt #1)
Forcing new Tor circuit... (attempt #1)
Forcing new Tor circuit... (attempt #1)
Forcing new Tor circuit... (attempt #2)
Forcing new Tor circuit... (attempt #3)
Forcing new Tor circuit... (attempt #4)
Forcing new Tor circuit... (attempt #1)
Forcing new Tor circuit... (attempt #2)
Forcing new Tor circuit... (attempt #3)
Forcing new Tor circuit... (attempt #1)
Forcing new Tor circuit... (attempt #1)
Forcing new Tor circuit... (attempt #1)

and were successful. :)

#14 Updated by kytv 2015-06-22 12:46:29

• Assignee changed from kytv to intrigeri
• QA Check set to Ready for QA
• Feature Branch set to kytv:test/9518-gpg-retry-with-new-circuit

Note: This branch depends on the one in Feature #9517 (kytv:test/9517-retry-oftc).

#15 Updated by kytv 2015-06-22 12:46:56

• blocked by Feature #9517: Retry connecting to OFTC when it fails added

#17 Updated by kytv 2015-06-26 05:38:58

• Assignee changed from intrigeri to anonym

#18 Updated by intrigeri 2015-06-28 03:05:17

• Assignee changed from anonym to kytv

kytv, please reassign to me once this branch can actually be merged (that is, once Feature #9517 is completed and merged).

#20 Updated by kytv 2015-06-29 00:11:26

• Target version changed from Tails_1.4.1 to Tails_2.3

#21 Updated by intrigeri 2015-06-29 00:44:48

• Target version changed from Tails_2.3 to Tails_1.5

#22 Updated by kytv 2015-06-29 01:35:56

• Target version changed from Tails_1.5 to Tails_1.4.1

OK. Merged and doing another test run before pushing/re-assigning.

#23 Updated by kytv 2015-06-29 01:36:14

• Target version changed from Tails_1.4.1 to Tails_1.5

#25 Updated by kytv 2015-06-29 02:03:06

• Assignee changed from kytv to intrigeri

stable merged and the branch still does what I expect.

#26 Updated by kytv 2015-06-29 02:39:46

• Target version changed from Tails_1.5 to Tails_1.4.1

#27 Updated by intrigeri 2015-06-29 03:02:54

• Target version changed from Tails_1.4.1 to Tails_1.5

kytv wrote:
> Investigating the failure, it was unrelated to my work.
> […]

Was a ticket filed about this? If not, please do so.

#29 Updated by intrigeri 2015-07-01 10:35:05

• Assignee changed from intrigeri to kytv
• QA Check changed from Ready for QA to Info Needed

In commit:3be9ea5 it’s not clear to me if we’re re-throwing Test::Unit::AssertionFailedError after failing more than MAX_NEW_TOR_CIRCUIT_RETRIES tries. Are we missing a final assert like you added in commit:a7f6a9b, perhaps?

Also, if we’re starting catching exceptions thrown by vm.execute_successfully, perhaps it should throw its own exception class, instead of the generic Test::Unit::AssertionFailedError? Not sure if it’s a blocker.

Otherwise, looks good! Untested for now.

#30 Updated by kytv 2015-07-02 09:21:07

• QA Check changed from Info Needed to Dev Needed

intrigeri wrote:
> In commit:3be9ea5 it’s not clear to me if we’re re-throwing Test::Unit::AssertionFailedError after failing more than MAX_NEW_TOR_CIRCUIT_RETRIES tries. Are we missing a final assert like you added in commit:a7f6a9b, perhaps?

An earlier revision had an assertion here but I removed it because it felt redundant due to the the GnuPG fetch is successful step. Looking at it again I think it should indeed have the assertion.

> Also, if we’re starting catching exceptions thrown by vm.execute_successfully, perhaps it should throw its own exception class, instead of the generic Test::Unit::AssertionFailedError? Not sure if it’s a blocker.

This would probably be better.

#31 Updated by kytv 2015-07-02 12:57:10

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

Both issues have been addressed so maybe it’s acceptable now.

#32 Updated by kytv 2015-07-03 06:42:23

• blocks Feature #9668: Retry whois request on "WHOIS LIMIT EXCEEDED" added

#33 Updated by intrigeri 2015-07-04 09:00:14

• Assignee changed from intrigeri to kytv
• % Done changed from 40 to 50
• QA Check changed from Ready for QA to Dev Needed

kytv wrote:
> Both issues have been addressed so maybe it’s acceptable now.

Well, thanks for introducing a new exception class, but then it needs to be used consistently: at first glance, there are other places that seem to catch errors from execute_successfully by rescue’ing Test::Unit::AssertionFailedError. Do you want to go back to the drawing board for this, or should I instead merge everything except commit:2f25a195?

#34 Updated by kytv 2015-07-04 10:04:54

• Assignee changed from kytv to intrigeri
• QA Check changed from Dev Needed to Info Needed

intrigeri wrote:
> kytv wrote:
> > Both issues have been addressed so maybe it’s acceptable now.
>
> Well, thanks for introducing a new exception class, but then it needs to be used consistently: at first glance, there are other places that seem to catch errors from execute_successfully by rescue’ing Test::Unit::AssertionFailedError.

Interesting. I looked for other instances and didn’t see any. Looking again I still don’t see any.

Running git grep -B7 'rescue.*Test::Unit::AssertionFailedError' -- features in this branch (after temporarily merging in stable) I see

features/step_definitions/checks.rb-
features/step_definitions/checks.rb-Then /^the OpenPGP keys shipped with Tails will be valid for the next (\d+) months$/ do |months|
features/step_definitions/checks.rb-  next if @skip_steps_while_restoring_background
features/step_definitions/checks.rb-  invalid = Array.new
features/step_definitions/checks.rb-  shipped_openpgp_keys.each do |key|
features/step_definitions/checks.rb-    begin
features/step_definitions/checks.rb-      step "the shipped OpenPGP key #{key} will be valid for the next #{months} months"
features/step_definitions/checks.rb:    rescue Test::Unit::AssertionFailedError
--
features/step_definitions/common_steps.rb-end
features/step_definitions/common_steps.rb-
features/step_definitions/common_steps.rb-Then /^I force Tor to use a new circuit( in Vidalia)?$/ do |with_vidalia|
features/step_definitions/common_steps.rb-  if with_vidalia
features/step_definitions/common_steps.rb-    assert_equal('gnome', @theme, "Vidalia is not available in the #{@theme} theme.")
features/step_definitions/common_steps.rb-    begin
features/step_definitions/common_steps.rb-      step 'process "vidalia" is running'
features/step_definitions/common_steps.rb:    rescue Test::Unit::AssertionFailedError
--
features/step_definitions/pidgin.rb-    account = $config["Pidgin"]["Accounts"]["XMPP"][account_name]
features/step_definitions/pidgin.rb-    check_keys = ["username", "domain", "password"] + required_options
features/step_definitions/pidgin.rb-    for key in check_keys do
features/step_definitions/pidgin.rb-      assert(account.has_key?(key))
features/step_definitions/pidgin.rb-      assert_not_nil(account[key])
features/step_definitions/pidgin.rb-      assert(!account[key].empty?)
features/step_definitions/pidgin.rb-    end
features/step_definitions/pidgin.rb:  rescue NoMethodError, Test::Unit::AssertionFailedError
--
features/step_definitions/pidgin.rb-  next if @skip_steps_while_restoring_background
features/step_definitions/pidgin.rb-  expected_channel_entry = chan_image(account, default_chan(account), 'roster')
features/step_definitions/pidgin.rb-  tries = 0
features/step_definitions/pidgin.rb-  until tries == $config["MAX_NEW_TOR_CIRCUIT_RETRIES"] do
features/step_definitions/pidgin.rb-    # Sometimes the OFTC welcome notice window pops up over the buddy list one...
features/step_definitions/pidgin.rb-    begin
features/step_definitions/pidgin.rb-      @vm.focus_window('Buddy List')
features/step_definitions/pidgin.rb:    rescue Test::Unit::AssertionFailedError
--
features/step_definitions/tor.rb-  next if @skip_steps_while_restoring_background
features/step_definitions/tor.rb-  bridge_type.downcase!
features/step_definitions/tor.rb-  bridge_type.capitalize!
features/step_definitions/tor.rb-  begin
features/step_definitions/tor.rb-    @bridges = $config["Tor"]["Transports"][bridge_type]
features/step_definitions/tor.rb-    assert_not_nil(@bridges)
features/step_definitions/tor.rb-    assert(!@bridges.empty?)
features/step_definitions/tor.rb:  rescue NoMethodError, Test::Unit::AssertionFailedError
--
features/support/helpers/vm_helper.rb-    return VMCommand.new(self, cmd, { :user => user, :spawn => false })
features/support/helpers/vm_helper.rb-  end
features/support/helpers/vm_helper.rb-
features/support/helpers/vm_helper.rb-  def execute_successfully(cmd, user = "root")
features/support/helpers/vm_helper.rb-    p = execute(cmd, user)
features/support/helpers/vm_helper.rb-    begin
features/support/helpers/vm_helper.rb-      assert_vmcommand_success(p)
features/support/helpers/vm_helper.rb:    rescue Test::Unit::AssertionFailedError => e

The last line there is from my change in 2f25a195.

> Do you want to go back to the drawing board for this, or should I instead merge everything except commit:2f25a195?

If you can direct me to what I’m missing I’ll be happy to do it correctly across the board but I’m just not seeing it ATM. :(

#36 Updated by kytv 2015-07-04 10:12:09

• QA Check changed from Info Needed to Ready for QA

Yes, I think that pidgin step was missed earlier.

Branch updated again.

#37 Updated by intrigeri 2015-07-04 11:37:15

• % Done changed from 50 to 60

Code now looks good! Will test and hopefully merge :)

I’m just wondering if ExecutionFailed should perhaps be called ExecutionFailedInVM, or something that indicates more clearly that it’s about a failed execution in the system under test. What do you think?

#41 Updated by intrigeri 2015-07-05 09:27:42

• Status changed from In Progress to Fix committed
• % Done changed from 60 to 100

Applied in changeset commit:6c125cbf13baa69f92a2cb77aa36c338d5522187.

#42 Updated by intrigeri 2015-07-05 09:28:14

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

Merged!

#44 Updated by BitingBird 2015-08-11 10:46:34

• Status changed from Fix committed to Resolved