Feature #9087

Write tests for connecting to hosts on a LAN via SSH

Added by kytv 2015-03-21 17:50:24 . Updated 2016-06-26 10:39:57 .

Status:
Resolved
Priority:
Normal
Assignee:
Category:
Test suite
Target version:
Tails_2.0
Start date:
2015-03-21
Due date:
% Done:

100%

Feature Branch:
test/9087-lan-ssh-tests
Type of work:
Code
Blueprint:

Starter:
0
Affected tool:
Deliverable for:

Description

Once Feature #6308 is merged we’ll have tests for SSH to hosts through Tor. We should also have tests for LAN hosts.

Subtasks

Related issues

Related to Tails - Feature #9520: Investigate the Shadow network simulator for use in our test suite Rejected 2015-06-02
Copied from Tails - Feature #6308: Write tests for SSH Resolved 2013-09-26

History

#1 Updated by kytv 2015-03-21 17:50:24

#2 Updated by kytv 2015-03-21 17:50:45

  • Status changed from New to Confirmed
  • Start date changed from 2013-09-26 to 2015-03-21

#3 Updated by kytv 2015-03-21 17:50:56

  • Subject changed from Write tests for conneting to hosts on a LAN via SSH to Write tests for connecting to hosts on a LAN via SSH

#4 Updated by kytv 2015-03-21 17:54:20

Feature #6308#note-13 offers guidance as to how to tackle this.

#5 Updated by intrigeri 2015-03-21 18:19:08

  • blocks #8538 added

#6 Updated by kytv 2015-08-04 04:01:55

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

#7 Updated by kytv 2015-09-22 03:49:32

  • Target version changed from Tails_1.6 to Tails_1.7

#8 Updated by intrigeri 2015-09-22 06:03:15

  • Priority changed from Normal to Elevated

Postponed, raising priority.

#9 Updated by anonym 2015-09-26 07:24:04

  • related to Feature #10252: Test that FTP connections to LAN IPs works in Unsafe Browser added

#10 Updated by anonym 2015-09-26 07:27:41

It might be that we could come up (or find an existing one) with a simple framework for starting/monitoring/managing sub-processes like sshd that can be reused for the FTP server needed in Feature #10252. Such a framework will likely be very helpful for the “simulate the Internet services” part of Feature #9520.

#11 Updated by anonym 2015-09-26 07:27:53

  • related to Feature #9520: Investigate the Shadow network simulator for use in our test suite added

#12 Updated by anonym 2015-09-26 07:31:36

  • related to deleted (Feature #10252: Test that FTP connections to LAN IPs works in Unsafe Browser)

#13 Updated by anonym 2015-09-26 07:31:47

#14 Updated by kytv 2015-11-04 10:46:06

  • Target version changed from Tails_1.7 to Tails_1.8

#15 Updated by kytv 2015-11-28 14:11:44

  • Status changed from Confirmed to In Progress
  • Assignee changed from kytv to anonym
  • QA Check set to Info Needed

Since one can only login using SSH with an existing user account, would it be acceptable to consider seeing the fingerprint verification prompt as a successful connection?

#16 Updated by kytv 2015-11-28 18:09:04

  • % Done changed from 0 to 30
  • Feature Branch set to kytv/test/9087-lan-ssh-tests

If what I propose is OK, perhaps the included branch is also acceptable.

#17 Updated by intrigeri 2015-12-05 09:24:08

  • QA Check changed from Info Needed to Ready for QA

kytv wrote:
> Since one can only login using SSH with an existing user account, would it be acceptable to consider seeing the fingerprint verification prompt as a successful connection?

I think it’s OK => ready for QA.

#18 Updated by anonym 2015-12-16 14:06:13

  • Target version changed from Tails_1.8 to Tails_2.0

#19 Updated by kytv 2015-12-30 21:47:01

I merged devel into the branch, fixed merge conflicts, tested that things still worked, and pushed to my repository.

#20 Updated by anonym 2016-01-19 16:26:53

  • Assignee changed from anonym to intrigeri
  • % Done changed from 30 to 50
  • Feature Branch changed from kytv/test/9087-lan-ssh-tests to test/9087-lan-ssh-tests

I feel responsible for this branch since I have let the review slide for so long. I’ve fixed it up. intrigeri, what do you think of my commits?

#21 Updated by anonym 2016-01-19 17:12:27

I couldn’t help myself and pushed some other semi-related cleanups. Feel free to only merge until commit:ce011a00186e37cd9ef9b85cb85d664341e0f619, and then I file a new ticket with a new branch for the rest, targeting 2.2.

#22 Updated by intrigeri 2016-01-21 16:27:24

  • Assignee changed from intrigeri to anonym
  • % Done changed from 50 to 80
  • QA Check changed from Ready for QA to Dev Needed

I think that ssh_host is not what you mean in:

+def assert_not_ipaddr(s)
+  err_msg = "#{@ssh_host} looks like a LAN IP address."

I guess it should be #{s} instead.

Also, it seems that the dependency on openssh-server is not documented nor encoded in the usual places. Can you please fix this?

Feel free to merge into testing after fixing these two small issues: code review and test passes for me, yeah :)

Also, if I got it right, “I connect to an SSH server on the […]” now creates the needed “Given” (a running sshd on the LAN) in case we’re in the “on the LAN” case, but relies on that “Given” to exist otherwise. I’m not sure I like it much. I think I’d rather see the sshd startup done via an explicit “Given” step. What do you think? (Not a blocker IMO, this can be dealt with in another iteration, on another ticket, if we agree it should be changed.)

#23 Updated by anonym 2016-01-21 20:35:59

intrigeri wrote:
> I think that ssh_host is not what you mean in:
>
> […]
>
> I guess it should be #{s} instead.

True; copy-paste error while refactoring. Thanks! Fixed in commit:dc06223.

> Also, it seems that the dependency on openssh-server is not documented nor encoded in the usual places. Can you please fix this?

Fixed in commit:de642cc.

> Feel free to merge into testing after fixing these two small issues: code review and test passes for me, yeah :)

I’ll do that, then.

> Also, if I got it right, “I connect to an SSH server on the […]” now creates the needed “Given” (a running sshd on the LAN) in case we’re in the “on the LAN” case, but relies on that “Given” to exist otherwise. I’m not sure I like it much. I think I’d rather see the sshd startup done via an explicit “Given” step. What do you think? (Not a blocker IMO, this can be dealt with in another iteration, on another ticket, if we agree it should be changed.)

I agree. I didn’t care about this since it’s far from the only place we offend in the same way. Any way, let’s try to do better, so I fixed it in commit:61f2928 (which also fixes two other very minor issues with that code).

#24 Updated by anonym 2016-01-21 20:47:30

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

Applied in changeset commit:d0da5db6d4a090290177064f375c29f5b882de46.

#25 Updated by anonym 2016-01-21 21:12:12

  • Assignee deleted (anonym)
  • QA Check changed from Dev Needed to Pass

Merged. If you have the time, please look at the unreview commits I merged:

61f2928 Refactor ssh server setup part into its own step.
de642cc The automated test suite depends on openssh-server.
dc06223 Fix bad variable after copy-paste mistake.

#26 Updated by anonym 2016-01-27 13:31:26

  • Status changed from Fix committed to Resolved

#27 Updated by intrigeri 2016-02-08 15:17:27

> 61f2928 Refactor ssh server setup part into its own step.

Good.

> de642cc The automated test suite depends on openssh-server.

Good, and so I’ve done the same in our tails::tester Puppet class.

> dc06223 Fix bad variable after copy-paste mistake.

Good.

#28 Updated by BitingBird 2016-06-26 10:39:57

  • Priority changed from Elevated to Normal