tests/lib: introduce helpers for setting up /dev/random using /dev/urandom in project prepare #4354

Merged
merged 10 commits into from Dec 11, 2017

Conversation

Projects
None yet
4 participants
Contributor

bboozzoo commented Dec 5, 2017

The hosts used for testing may run out of entropy in /dev/random, thus causing
any crypto operations to potentially block. Since we do not need a high quality
RNG source for the tests, set up /dev/random to be the same as /dev/urandom in
project prepare and restore it back to the proper state in project restore
phase.

tests/lib: introduce helpers for setting up /dev/random using /dev/ur…
…andom in project prepare

The hosts used for testing may run out of entropy in /dev/random, thus causing
any crypto operations to potentially block. Since we do not need a high quality
RNG source for the tests, set up /dev/random to be the same as /dev/urandom in
project prepare and restore it back to the proper state in project restore
phase.

Signed-off-by: Maciej Borzecki <maciej.zenon.borzecki@canonical.com>

zyga approved these changes Dec 5, 2017

+1 with note, please check before merging

tests/lib/random.sh
+ mknod /dev/random c 1 9
+}
+
+restore_dev_random() {
@zyga

zyga Dec 5, 2017

Contributor

This is probably not correct since some distributions don't use proper random devices. We should probably mv /dev/random /dev/random.bak or .orig and then mv them back.

In any case, this is my suggestion, LGTM

tests/lib/random: keep the original /dev/random and restore it
Signed-off-by: Maciej Borzecki <maciej.zenon.borzecki@canonical.com>

codecov-io commented Dec 5, 2017

Codecov Report

Merging #4354 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #4354   +/-   ##
=======================================
  Coverage   78.05%   78.05%           
=======================================
  Files         450      450           
  Lines       30899    30899           
=======================================
  Hits        24118    24118           
  Misses       4772     4772           
  Partials     2009     2009

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 9544634...84cfe22. Read the comment docs.

tests/lib/prepare: do not setup rng-tools on Ubuntu & Debian
We have set up /dev/urandom device as /dev/random. No improve entropy anymore.

Signed-off-by: Maciej Borzecki <maciej.zenon.borzecki@canonical.com>

zyga approved these changes Dec 7, 2017

LGTM

@@ -252,30 +252,6 @@ EOF
systemctl start snapd.socket
fi
- if [[ "$SPREAD_SYSTEM" == debian-* || "$SPREAD_SYSTEM" == ubuntu-* ]]; then
@zyga

zyga Dec 7, 2017

Contributor

Ha, much much nicer :)

+ # keep the original /dev/random around
+ mv /dev/random /dev/random.orig
+ # same as /dev/urandom
+ mknod /dev/random c 1 9
@zyga

zyga Dec 7, 2017

Contributor

The device numbers are valid for urandom

mvo5 approved these changes Dec 7, 2017

bboozzoo added some commits Dec 8, 2017

tests/main: source mkpinentry.sh
mkpinentry.sh script is using MATCH, but this is not defined when running in a
subshell.

Signed-off-by: Maciej Borzecki <maciej.zenon.borzecki@canonical.com>
tests/lib/random: kill gpg-agent once /dev/random fixup is applied
Signed-off-by: Maciej Borzecki <maciej.zenon.borzecki@canonical.com>
tests/lib/random: use SIGKILL rather than SIGTERM when killing gpg-agent
gpg-agent stuck in blocking read from /dev/random does not react to SIGTERM, use
SIGKILL instead.

Signed-off-by: Maciej Borzecki <maciej.zenon.borzecki@canonical.com>
tests/lib/random: helper for dumping debug information for errors rel…
…ated to /dev/random

Signed-off-by: Maciej Borzecki <maciej.zenon.borzecki@canonical.com>
tests/main/{completion,create-key,snap-sign}: kill gpg-agent in prepare
Avoid potential issues with gpg-agent being stuck by tests that ran previously
and make sure that we get a freshly started agent.

Signed-off-by: Maciej Borzecki <maciej.zenon.borzecki@canonical.com>
tests/lib/prepare-restore: run /dev/random fixups before and after ea…
…ch task

Signed-off-by: Maciej Borzecki <maciej.zenon.borzecki@canonical.com>
Contributor

bboozzoo commented Dec 11, 2017

Well, at least it didn't timeout this time. The tasks that were getting stuck before, all passed now. What has failed is:

2017-12-11 13:35:04 Successful tasks: 1581
2017-12-11 13:35:04 Aborted tasks: 1
2017-12-11 13:35:04 Failed tasks: 1
    - linode:ubuntu-core-16-64:tests/main/interfaces-snapd-control-with-manage
2017-12-11 13:35:04 Failed task prepare: 1
    - linode:fedora-26-64:tests/main/revert-sideload
error: unsuccessful run

Looks unrelated, so I've restarted the build.

Contributor

bboozzoo commented Dec 11, 2017

FWIW, the travis job was filing before as the create-key/snap-sign/completion prepare were taking too long. Collected debug output suggests that the /dev/random node was not changed at all. This is either the fixup_dev_random applied in prepare was reverted (although I did not manage to track down the code where this could happen), didn't run, or project restore was run (?).

Debug log in question:

2017-12-11 11:27:34 Error executing linode:debian-9-64:tests/main/snap-sign : 
-----
+ echo 'Creating a new key without a password'
Creating a new key without a password
+ expect -f create-key.exp
spawn snap create-key

Passphrase: 

Confirm passphrase: 
<kill-timeout reached>
-----
travis_time:end:a4df1e52:start=1512990455705312850,finish=1512991654836096494,duration=1199130783644
travis_fold:end:fold-a4df1e52
.
travis_time:end:aaff80bf:start=1512991654103974079,finish=1512991655057084868,duration=953110789
travis_time:start:e3cf5337
2017-12-11 11:27:35 Restoring linode:ubuntu-16.04-32:tests/main/known-remote...
travis_fold:start:fold-d6a4eb69
travis_time:start:d6a4eb69
2017-12-11 11:27:35 Debug output for linode:debian-9-64:tests/main/snap-sign : 
-----
+ . /home/gopath/src/github.com/snapcore/snapd/tests/lib/random.sh
+ debug_random
+ sysctl kernel.random.entropy_avail
kernel.random.entropy_avail = 44
+ ls -l /dev/random /dev/urandom
crw-rw-rw- 1 root root 1, 8 Dec 11 11:06 /dev/random
crw-rw-rw- 1 root root 1, 9 Dec 11 11:06 /dev/urandom
++ pidof gpg-agent
+ pids=9700
+ for p in $pids
+ ps -q 9700
  PID TTY          TIME CMD
 9700 ?        00:00:00 gpg-agent
+ ls -l /proc/9700/fd
total 0
lr-x------ 1 root root 64 Dec 11 11:27 0 -> /dev/null
l-wx------ 1 root root 64 Dec 11 11:27 1 -> /dev/null
l-wx------ 1 root root 64 Dec 11 11:27 2 -> /dev/null
lrwx------ 1 root root 64 Dec 11 11:27 3 -> socket:[42935]
lrwx------ 1 root root 64 Dec 11 11:27 4 -> socket:[42937]
lrwx------ 1 root root 64 Dec 11 11:27 5 -> socket:[42939]
lrwx------ 1 root root 64 Dec 11 11:27 6 -> socket:[42941]
lr-x------ 1 root root 64 Dec 11 11:27 7 -> anon_inode:inotify
lrwx------ 1 root root 64 Dec 11 11:27 8 -> socket:[42946]
lr-x------ 1 root root 64 Dec 11 11:27 9 -> /dev/random
+ '[' 1 = 1 ']'
+ echo '# journal messages for snapd'

mvo5 approved these changes Dec 11, 2017

@mvo5 mvo5 merged commit 87e197c into snapcore:master Dec 11, 2017

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment