Mir

Merge lp:~vanvugt/mir/run-without-entropy into lp:mir

Proposed by Daniel van Vugt on 2016-02-29
Status: Merged
Approved by: Daniel van Vugt on 2016-03-23
Approved revision: 3366
Merged at revision: 3416
Proposed branch: lp:~vanvugt/mir/run-without-entropy
Merge into: lp:mir
Prerequisite: lp:~vanvugt/mir/test-without-entropy-more
Diff against target: 115 lines (+15/-54)
3 files modified
src/cookie/authority.cpp (+12/-51)
tests/acceptance-tests/test_server_startup.cpp (+1/-1)
tests/unit-tests/test_mir_cookie.cpp (+2/-2)
To merge this branch: bzr merge lp:~vanvugt/mir/run-without-entropy
Reviewer Review Type Date Requested Status
Mir CI Bot continuous-integration 2016-02-29 Approve on 2016-03-23
Daniel van Vugt Approve on 2016-03-23
Alan Griffiths 2016-02-29 Abstain on 2016-03-21
Alexandros Frantzis (community) 2016-02-29 Approve on 2016-03-21
Brandon Schaefer (community) 2016-02-29 Approve on 2016-03-08
Chris Halse Rogers 2016-02-29 Needs Information on 2016-03-02
Review via email: mp+287445@code.launchpad.net

This proposal supersedes a proposal from 2016-02-26.

Commit message

Avoid hanging and/or crashing the Mir server when gathering entropy
(fixes LP: #1536662 and LP: #1541188)

To post a comment you must log in.
Mir CI Bot (mir-ci-bot) wrote : Posted in a previous version of this proposal

FAILED: Continuous integration, rev:3347
https://mir-jenkins.ubuntu.com/job/mir-ci/411/
Executed test runs:
    FAILURE: https://mir-jenkins.ubuntu.com/job/build-mir/212/console
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-0-fetch/236
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=vivid+overlay/228
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=xenial/228
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=vivid+overlay/219
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=vivid+overlay/219/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=xenial/219
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=xenial/219/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=cross-armhf,compiler=gcc,platform=android,release=vivid+overlay/219
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=cross-armhf,compiler=gcc,platform=android,release=vivid+overlay/219/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=android,release=vivid+overlay/219
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=android,release=vivid+overlay/219/artifact/output/*zip*/output.zip
    FAILURE: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=mesa,release=xenial/219/console

Click here to trigger a rebuild:
https://mir-jenkins.ubuntu.com/job/mir-ci/411/rebuild

review: Needs Fixing (continuous-integration)
Alexandros Frantzis (afrantzis) wrote : Posted in a previous version of this proposal

Seems sensible (but of course I am not a security expert).

(ignoring the build failure in this approval)

review: Approve
Alan Griffiths (alan-griffiths) wrote : Posted in a previous version of this proposal

I am not a security expert either, but I know this won't pass audit

review: Disapprove
Brandon Schaefer (brandontschaefer) wrote : Posted in a previous version of this proposal

We cant do this with out a security team member approving this.

As this would RUIN our entire reasoning for even doing cookies to begin with. If we are having entropy issues to begin with on boot, then this is pretty much accepting that we will be generating a predictable secret. The best we can do is an attempted lazy create the secret we something is first requested vs doing it on boot.

review: Disapprove
Brandon Schaefer (brandontschaefer) wrote : Posted in a previous version of this proposal

The other thing we can do, is check the kernel version, and based on that use get_random(). Wont solve our issues for prev versions or the phone. This could fix the issue on the desktop though (which seems to be the only place we are getting hit by this so far)

Alexandros Frantzis (afrantzis) wrote : Posted in a previous version of this proposal

An interesting read (but once again I note that I don't have the expertise to judge):

http://www.2uo.de/myths-about-urandom/

Brandon Schaefer (brandontschaefer) wrote : Posted in a previous version of this proposal

Well i am far from a security expert :). If we can get a security review saying this is the fine way to go, then im 100% fine with it. Im just being overly cautious because I dont want to break security!

Daniel van Vugt (vanvugt) wrote : Posted in a previous version of this proposal

The logic is simple:

No other display server (or app?) hangs and/or crashes waiting for entropy. It's a bad user experience and obviously unacceptable.

I don't care if it "would fail audit", that misses the point. The bigger problem is that we currently fail basic usability. Mir needs to start up quickly and without crashing. Presently it does not do that.

Furthermore, Mir needs to keep responding quickly and without crashing. So to defer entropy collection only defers the problem without solving it.

This branch is the only way forward I can tell. If we've made heavy crypto a priority over a usable system, we've failed. It's not that the current design needs to pass security audit, but the current design is a failure if it can't pass a security audit without hanging indefinitely or crashing.

Chris Halse Rogers (raof) wrote : Posted in a previous version of this proposal

Mir - or something - needs to generate a key, because we're securing copy/paste with a secret-key system.

Using /dev/urandom is unsafe for something that'll run in early-boot, before the random pool is initialised; if /dev/urandom is used at that time there's no entropy so it'd relatively simple to break the key and hence the copy/paste security.

Since USC doesn't require copy/paste we can avoid the most likely trigger for the problem by USC not providing cookies.

If this is insufficient, we could not provide cookies until a source of randomness has been initialised.

We could also rework the copy/paste API to not require a secret key.

We *shouldn't* handle this by throwing away the check. If this check is failing, then we're in precisely the situation where urandom has insufficient entropy to provide security guarantees!

review: Disapprove
Alan Griffiths (alan-griffiths) wrote :

Resubmitting this MP because it collected three "Disapprove" votes isn't helpful.

I'm not happy with the current behaviour either, but there is work in progress to address it. The reasons this approach to fixing it is unacceptable have been stated above.

review: Disapprove
Mir CI Bot (mir-ci-bot) wrote :

FAILED: Continuous integration, rev:3350
https://mir-jenkins.ubuntu.com/job/mir-ci/430/
Executed test runs:
    FAILURE: https://mir-jenkins.ubuntu.com/job/build-mir/240/console
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-0-fetch/264
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=vivid+overlay/256
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=xenial/256
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=vivid+overlay/247
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=vivid+overlay/247/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=xenial/247
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=xenial/247/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=cross-armhf,compiler=gcc,platform=android,release=vivid+overlay/247
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=cross-armhf,compiler=gcc,platform=android,release=vivid+overlay/247/artifact/output/*zip*/output.zip
    FAILURE: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=android,release=vivid+overlay/247/console
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=android,release=vivid+overlay/247/artifact/output/*zip*/output.zip
    FAILURE: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=mesa,release=xenial/247/console

Click here to trigger a rebuild:
https://mir-jenkins.ubuntu.com/job/mir-ci/430/rebuild

review: Needs Fixing (continuous-integration)
Brandon Schaefer (brandontschaefer) wrote :

So this would be a good time to think of reasonable solutions You're right delaying the creation of the secret would delay the issue but at the same time is still likely to fail at some point.

How about, we check if entropy exists ie. check if we can read /dev/random if we cannot read from it, we fail to create valid cookies. This means we just send off cookies that are empty and wont pass any cookie check. From there, each time we generate a new cookie we attempt to read from /dev/random UNTIL we hit some entropy. From there we create the secret, and are done checking /dev/random then reading from /dev/urandom.

This would mean, we would never block/halt but cookies would fail until some sort of entropy exists on the system. Though starting a process requires entropy ... so I dont expect this to be a huge issue once we have booted.

Now this would bring up some other issues such as anything depending on the cookies to fail... but we can critical log or warn the issue that entropy does not exist and we cannot create any cookies.

Any other ideas are welcome.

Brandon Schaefer (brandontschaefer) wrote :

The other issue, is unity8 freaks out right away. ie. If we block on /dev/random for less then a second unity8 thinks USC failed to start right away and then brings the system down. We should make unity8 wait a little long to assume USC has failed.

Mir CI Bot (mir-ci-bot) wrote :

FAILED: Continuous integration, rev:3351
https://mir-jenkins.ubuntu.com/job/mir-ci/450/
Executed test runs:
    FAILURE: https://mir-jenkins.ubuntu.com/job/build-mir/266/console
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-0-fetch/290
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=vivid+overlay/282
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=xenial/282
    FAILURE: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=vivid+overlay/273/console
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=xenial/273
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=xenial/273/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=cross-armhf,compiler=gcc,platform=android,release=vivid+overlay/273
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=cross-armhf,compiler=gcc,platform=android,release=vivid+overlay/273/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=android,release=vivid+overlay/273
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=android,release=vivid+overlay/273/artifact/output/*zip*/output.zip
    FAILURE: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=mesa,release=xenial/273/console

Click here to trigger a rebuild:
https://mir-jenkins.ubuntu.com/job/mir-ci/450/rebuild

review: Needs Fixing (continuous-integration)
Daniel van Vugt (vanvugt) wrote :

Hmm, same Jenkins problem affecting other MPs too....

/tmp/hudson3861179810836943644.sh: line 100: syntax error near unexpected token `fi'

Daniel van Vugt (vanvugt) wrote :

Alan: Actually I resubmitted this MP only to change the prerequisite branch.

This branch was discussed in the meeting this morning. Brandon and Chris seem to be happy with the idea of using this branch together with an init script/systemd enhancement that primes the kernel entropy pool at boot time (a prerequisite to USC starting). That would give us the best of both worlds -- a cryptographically secure entropy seed, and also Mir logic that is guaranteed to not hang or crash.

Mir CI Bot (mir-ci-bot) wrote :

FAILED: Continuous integration, rev:3353
https://mir-jenkins.ubuntu.com/job/mir-ci/454/
Executed test runs:
    FAILURE: https://mir-jenkins.ubuntu.com/job/build-mir/273/console
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-0-fetch/297
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=vivid+overlay/289
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=xenial/289
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=vivid+overlay/281
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=vivid+overlay/281/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=xenial/281
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=xenial/281/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=cross-armhf,compiler=gcc,platform=android,release=vivid+overlay/281
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=cross-armhf,compiler=gcc,platform=android,release=vivid+overlay/281/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=android,release=vivid+overlay/281
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=android,release=vivid+overlay/281/artifact/output/*zip*/output.zip
    FAILURE: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=mesa,release=xenial/281/console

Click here to trigger a rebuild:
https://mir-jenkins.ubuntu.com/job/mir-ci/454/rebuild

review: Needs Fixing (continuous-integration)
Alan Griffiths (alan-griffiths) wrote :

OK, after discussion I'm happy that an application ought to be able to use urandom and that a lack of entropy should be considered an issue in a wider context to be fixed there (and not worked around here).

Any downstream that wants to check for entropy on startup has that option.

review: Approve
Chris Halse Rogers (raof) wrote :

I'm happy with this once we've got security-team buy-in for “Mir is not responsible for ensuring that urandom is initialised prior to the Mir server”. Do we have that?

review: Needs Information
Daniel van Vugt (vanvugt) wrote :

I had no idea there was a security team before yesterday. Who do we ask?

Daniel van Vugt (vanvugt) wrote :

Oh, it already exists in xenial (and probably earlier too):

$ systemctl show systemd-random-seed.service

https://www.freedesktop.org/software/systemd/man/systemd-random-seed.service.html

Chris Halse Rogers (raof) wrote :

/etc/init.d/urandom has existed basically forever; it *may* predate
Warty Warthog.

The relevant issue is ensuring Mir starts up after it :). And checking
that it actually runs on the phone!

Oh, and maybe the interactions with the device image.

Mir CI Bot (mir-ci-bot) wrote :

FAILED: Continuous integration, rev:3354
https://mir-jenkins.ubuntu.com/job/mir-ci/466/
Executed test runs:
    FAILURE: https://mir-jenkins.ubuntu.com/job/build-mir/290/console
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-0-fetch/315
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=vivid+overlay/307
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=xenial/307
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=vivid+overlay/298
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=vivid+overlay/298/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=xenial/298
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=xenial/298/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=cross-armhf,compiler=gcc,platform=android,release=vivid+overlay/298
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=cross-armhf,compiler=gcc,platform=android,release=vivid+overlay/298/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=android,release=vivid+overlay/298
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=android,release=vivid+overlay/298/artifact/output/*zip*/output.zip
    FAILURE: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=mesa,release=xenial/298/console

Click here to trigger a rebuild:
https://mir-jenkins.ubuntu.com/job/mir-ci/466/rebuild

review: Needs Fixing (continuous-integration)
Daniel van Vugt (vanvugt) wrote :

Looking to the long term when USC is in initramfs a prerequisite on start-up might not cut it any more (as USC is close to the first thing starting). But we could avoid that problem if we just implement delayed secret creation on top of this branch. Then no service dependencies are required (assuming nothing uses cookies till after login).

Mir CI Bot (mir-ci-bot) wrote :

PASSED: Continuous integration, rev:3355
https://mir-jenkins.ubuntu.com/job/mir-ci/467/
Executed test runs:
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-mir/291
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-0-fetch/316
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=vivid+overlay/308
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=xenial/308
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=vivid+overlay/299
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=vivid+overlay/299/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=xenial/299
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=xenial/299/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=cross-armhf,compiler=gcc,platform=android,release=vivid+overlay/299
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=cross-armhf,compiler=gcc,platform=android,release=vivid+overlay/299/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=android,release=vivid+overlay/299
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=android,release=vivid+overlay/299/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=mesa,release=xenial/299
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=mesa,release=xenial/299/artifact/output/*zip*/output.zip

Click here to trigger a rebuild:
https://mir-jenkins.ubuntu.com/job/mir-ci/467/rebuild

review: Approve (continuous-integration)
Brandon Schaefer (brandontschaefer) wrote :

So talking with Tyler Hicks, we should depend on systemd-random-seed.service. Ensure it runs before our server (ie. USC). From there, it would be perfectly fine to use urandom. Soo approving!

review: Approve
Alan Griffiths (alan-griffiths) wrote :

I think Chris's concern has been addressed

Daniel van Vugt (vanvugt) wrote :

Landing blocked waiting on reviews of the prerequisite branch

Brandon Schaefer (brandontschaefer) wrote :

The only remaining issue is, the phone doesnt have system.d. Sooo we will have to look into this.

Daniel van Vugt (vanvugt) wrote :

^ failures are just Jenkins infrastructure failures.

Daniel van Vugt (vanvugt) wrote :

Brandon: Here's the missing piece for upstart on the phone:
https://bugs.launchpad.net/ubuntu/+source/ubuntu-touch-session/+bug/1557984

Mir CI Bot (mir-ci-bot) wrote :

FAILED: Continuous integration, rev:3361
https://mir-jenkins.ubuntu.com/job/mir-ci/590/
Executed test runs:
    FAILURE: https://mir-jenkins.ubuntu.com/job/build-mir/490/console
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-0-fetch/520
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=vivid+overlay/512
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=xenial/512
    FAILURE: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=vivid+overlay/500/console
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=xenial/500
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=xenial/500/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=cross-armhf,compiler=gcc,platform=android,release=vivid+overlay/500
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=cross-armhf,compiler=gcc,platform=android,release=vivid+overlay/500/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=android,release=vivid+overlay/500
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=android,release=vivid+overlay/500/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=mesa,release=xenial/500
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=mesa,release=xenial/500/artifact/output/*zip*/output.zip

Click here to trigger a rebuild:
https://mir-jenkins.ubuntu.com/job/mir-ci/590/rebuild

review: Needs Fixing (continuous-integration)
Brandon Schaefer (brandontschaefer) wrote :

Sweet, this looks good then :). Just have to wait for jenkins to become happy again. Yay for removing code :)

Daniel van Vugt (vanvugt) wrote :

Although I'm not sure who to prod to ensure that init change happens soon.

Mir CI Bot (mir-ci-bot) wrote :

FAILED: Continuous integration, rev:3362
https://mir-jenkins.ubuntu.com/job/mir-ci/607/
Executed test runs:
    FAILURE: https://mir-jenkins.ubuntu.com/job/build-mir/510/console
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-0-fetch/540
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=vivid+overlay/532
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=xenial/532
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=vivid+overlay/520
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=vivid+overlay/520/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=xenial/520
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=xenial/520/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=cross-armhf,compiler=gcc,platform=android,release=vivid+overlay/520
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=cross-armhf,compiler=gcc,platform=android,release=vivid+overlay/520/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=android,release=vivid+overlay/520
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=android,release=vivid+overlay/520/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=mesa,release=xenial/520
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=mesa,release=xenial/520/artifact/output/*zip*/output.zip

Click here to trigger a rebuild:
https://mir-jenkins.ubuntu.com/job/mir-ci/607/rebuild

review: Needs Fixing (continuous-integration)
Alan Griffiths (alan-griffiths) wrote :

*Needs Discussion*

Having seen an entropy related failure in an unrelated job (lp:1558518) on the same box I'm concerned about the tests this activates. Specifically:

    // Flush the entropy pool
    int fd = open("/dev/random", O_RDONLY | O_NONBLOCK);
    ASSERT_THAT(fd, Ge(0));
    char buf[256];
    while (read(fd, buf, sizeof buf) > 0) {}
    close(fd);

Affects the whole machine not just the current test. While it could be argued that this also lands the fix and other builds then be unaffected we can't assume that will always be true.

I'd rather drop these tests.

review: Needs Information
Daniel van Vugt (vanvugt) wrote :

Bug 1558518 is a duplicate, and is 100% fixed by landing this branch on trunk. After it lands, nobody's branches will encounter any such problems. So it's not a big deal and will be entirely solved soon.

Mir CI Bot (mir-ci-bot) wrote :

PASSED: Continuous integration, rev:3363
https://mir-jenkins.ubuntu.com/job/mir-ci/617/
Executed test runs:
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-mir/529
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-0-fetch/559
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=vivid+overlay/551
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=xenial/551
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=vivid+overlay/539
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=vivid+overlay/539/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=xenial/539
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=xenial/539/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=cross-armhf,compiler=gcc,platform=android,release=vivid+overlay/539
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=cross-armhf,compiler=gcc,platform=android,release=vivid+overlay/539/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=android,release=vivid+overlay/539
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=android,release=vivid+overlay/539/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=mesa,release=xenial/539
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=mesa,release=xenial/539/artifact/output/*zip*/output.zip

Click here to trigger a rebuild:
https://mir-jenkins.ubuntu.com/job/mir-ci/617/rebuild

review: Approve (continuous-integration)
Alan Griffiths (alan-griffiths) wrote :

> After it lands, nobody's branches will encounter any such problems. So it's
> not a big deal and will be entirely solved soon.

That is exactly the contention I said needs discussing. ;)

Daniel van Vugt (vanvugt) wrote :

Well, some philosophers will argue that objectivity does not exist and subjectivity is all... :)

My assertion is that nobody will experience any entropy-related timeouts after this branch lands, because everybody will have the fix then. It's not a fault in this MP, but rather a bug in all other branches that is only resolved by landing this MP.

Daniel van Vugt (vanvugt) wrote :

Also worth noting; the bug does not extend beyond Mir branches. AFAIK, no other software is as silly as Mir has been with respect to /dev/random.

Alan Griffiths (alan-griffiths) wrote :

Causing problems does seem unlikely, especially in the short term.

OTOH It is not "nice" behaviour of a test to manipulate the state of the machine running the test (not just the test process) like this. All it needs is some unrelated code running on the system to do something that relies on really having entropy for this to cause a hard-to-reproduce, intermittent problem.

For an unlikely but possible scenario: suppose you run this test just when you need to generate a strong cryptographic key.

Alexandros Frantzis (afrantzis) wrote :

I am OK with this change. There is no way around exhausting entropy if we want to test our code at this level (i.e., integration with a real kernel).

I am not concerned about affecting other applications. Applications that depend on having high entropy (mainly key generators as Alan suggests) are designed to deal with low entropy situations elegantly, since such situations can arise for many reasons unrelated to our crafty tests.

Non-blocking concern:

I have one slight concern about the newly enabled tests, though: they assume without verifying that we are using a particular mechanism to get random data, the kernel /dev/*random devices. If we switch to a different mechanism (e.g. imagine a user-space random data service, or a different kernel interface) the tests will still pass but will not be testing anything useful. It would be good if we at least had a test that verified that we are using the mechanism we are testing (i.e., that we are using /dev/urandom).

review: Approve
Alan Griffiths (alan-griffiths) wrote :

I'm not sure the benefits of this testing outweigh the costs of this testing. But so long as the "Approves" have considered this question I won't block.

review: Abstain
Daniel van Vugt (vanvugt) wrote :

I appreciate Alan's caution. However I think his concern that other common software exists that would suffer from the same /dev/random mistakes is incorrect. As Alexandros points out, only software like ssh-keygen uses /dev/random, and it is designed to do so elegantly -- expecting long indefinite delays.

As for the non-blocking concern of switching entropy sources, that's not really a new problem. We will forever have the problem of developers changing code that reduces the efficacy and coverage of existing tests. It's just something we have to maintain.

Mir CI Bot (mir-ci-bot) wrote :

FAILED: Autolanding.
More details in the following jenkins job:
https://mir-jenkins.ubuntu.com/job/mir-autolanding/121/
Executed test runs:
    FAILURE: https://mir-jenkins.ubuntu.com/job/build-mir/550/console
    None: https://mir-jenkins.ubuntu.com/job/generic-land-mp/130/console
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-0-fetch/580
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=vivid+overlay/572
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=xenial/572
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=vivid+overlay/560
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=vivid+overlay/560/artifact/output/*zip*/output.zip
    FAILURE: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=xenial/560/console
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=cross-armhf,compiler=gcc,platform=android,release=vivid+overlay/560
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=cross-armhf,compiler=gcc,platform=android,release=vivid+overlay/560/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=android,release=vivid+overlay/560
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=android,release=vivid+overlay/560/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=mesa,release=xenial/560
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=mesa,release=xenial/560/artifact/output/*zip*/output.zip

review: Needs Fixing (continuous-integration)
Daniel van Vugt (vanvugt) wrote :

Same cluster of failures happening with other MPs. It's just that one machine for some reason....

04:20:01 The following tests FAILED:
04:20:01 11 - mir_acceptance_tests (Failed)
04:20:01 13 - mir_acceptance_tests---MIR_SERVER_NBUFFERS=0--- (Failed)
04:20:01 17 - mir_integration_tests (Failed)
04:20:01 25 - mir_umock_unit_tests (Failed)

Mir CI Bot (mir-ci-bot) wrote :

FAILED: Autolanding.
More details in the following jenkins job:
https://mir-jenkins.ubuntu.com/job/mir-autolanding/125/
Executed test runs:
    FAILURE: https://mir-jenkins.ubuntu.com/job/build-mir/556/console
    None: https://mir-jenkins.ubuntu.com/job/generic-land-mp/134/console
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-0-fetch/586
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=vivid+overlay/578
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=xenial/578
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=vivid+overlay/566
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=vivid+overlay/566/artifact/output/*zip*/output.zip
    FAILURE: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=xenial/566/console
    FAILURE: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=cross-armhf,compiler=gcc,platform=android,release=vivid+overlay/566/console
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=cross-armhf,compiler=gcc,platform=android,release=vivid+overlay/566/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=android,release=vivid+overlay/566
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=android,release=vivid+overlay/566/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=mesa,release=xenial/566
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=mesa,release=xenial/566/artifact/output/*zip*/output.zip

review: Needs Fixing (continuous-integration)
lp:~vanvugt/mir/run-without-entropy updated on 2016-03-23
3366. By Daniel van Vugt on 2016-03-23

Merge latest trunk

Daniel van Vugt (vanvugt) wrote :

Try again...

review: Approve
Mir CI Bot (mir-ci-bot) wrote :

PASSED: Continuous integration, rev:3366
https://mir-jenkins.ubuntu.com/job/mir-ci/632/
Executed test runs:
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-mir/564
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-0-fetch/600
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=vivid+overlay/592
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=xenial/592
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=vivid+overlay/574
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=vivid+overlay/574/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=xenial/574
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=xenial/574/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=cross-armhf,compiler=gcc,platform=android,release=vivid+overlay/574
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=cross-armhf,compiler=gcc,platform=android,release=vivid+overlay/574/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=android,release=vivid+overlay/574
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=android,release=vivid+overlay/574/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=mesa,release=xenial/574
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=mesa,release=xenial/574/artifact/output/*zip*/output.zip

Click here to trigger a rebuild:
https://mir-jenkins.ubuntu.com/job/mir-ci/632/rebuild

review: Approve (continuous-integration)
Mir CI Bot (mir-ci-bot) :
review: Approve (continuous-integration)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'src/cookie/authority.cpp'
2--- src/cookie/authority.cpp 2016-01-29 08:18:22 +0000
3+++ src/cookie/authority.cpp 2016-03-23 02:41:04 +0000
4@@ -41,9 +41,6 @@
5
6 namespace
7 {
8-std::string const random_device_path{"/dev/random"};
9-std::string const urandom_device_path{"/dev/urandom"};
10-uint32_t const wait_seconds{30};
11 uint32_t const mac_byte_size{20};
12
13 size_t cookie_size_from_format(mir::cookie::Format const& format)
14@@ -62,55 +59,19 @@
15
16 static mir::cookie::Secret get_random_data(unsigned size)
17 {
18+ int fd = open("/dev/urandom", O_RDONLY);
19+ if (fd < 0)
20+ BOOST_THROW_EXCEPTION(std::system_error(errno, std::system_category(),
21+ "open failed on urandom"));
22+
23 mir::cookie::Secret buffer(size);
24- int random_fd;
25- int retval;
26- fd_set rfds;
27-
28- struct timeval tv;
29- tv.tv_sec = wait_seconds;
30- tv.tv_usec = 0;
31-
32- if ((random_fd = open(random_device_path.c_str(), O_RDONLY)) == -1)
33- {
34- int error = errno;
35- BOOST_THROW_EXCEPTION(std::system_error(error, std::system_category(),
36- "open failed on device " + random_device_path));
37- }
38-
39- FD_ZERO(&rfds);
40- FD_SET(random_fd, &rfds);
41-
42- /* We want to block until *some* entropy exists on boot, then use urandom once we have some */
43- retval = select(random_fd + 1, &rfds, NULL, NULL, &tv);
44-
45- /* We are done with /dev/random at this point, and it is either an error or ready to be read */
46- if (close(random_fd) == -1)
47- {
48- int error = errno;
49- BOOST_THROW_EXCEPTION(std::system_error(error, std::system_category(),
50- "close failed on device " + random_device_path));
51- }
52-
53- if (retval == -1)
54- {
55- int error = errno;
56- BOOST_THROW_EXCEPTION(std::system_error(error, std::system_category(),
57- "select failed on file descriptor " + std::to_string(random_fd) +
58- " from device " + random_device_path));
59- }
60- else if (retval && FD_ISSET(random_fd, &rfds))
61- {
62- std::uniform_int_distribution<uint8_t> dist;
63- std::random_device rand_dev(urandom_device_path);
64-
65- std::generate(std::begin(buffer), std::end(buffer), [&]() { return dist(rand_dev); });
66- }
67- else
68- {
69- BOOST_THROW_EXCEPTION(std::runtime_error("Failed to read from device: " + random_device_path +
70- " after: " + std::to_string(wait_seconds) + " seconds"));
71- }
72+ unsigned got = read(fd, buffer.data(), size);
73+ int error = errno;
74+ close(fd);
75+
76+ if (got != size)
77+ BOOST_THROW_EXCEPTION(std::system_error(error, std::system_category(),
78+ "read failed on urandom"));
79
80 return buffer;
81 }
82
83=== modified file 'tests/acceptance-tests/test_server_startup.cpp'
84--- tests/acceptance-tests/test_server_startup.cpp 2016-03-17 03:38:36 +0000
85+++ tests/acceptance-tests/test_server_startup.cpp 2016-03-23 02:41:04 +0000
86@@ -68,7 +68,7 @@
87 });
88 }
89
90-TEST(ServerStartupReliability, DISABLED_can_start_with_low_entropy)
91+TEST(ServerStartupReliability, can_start_with_low_entropy)
92 { // Regression test for LP: #1536662 and LP: #1541188
93 using namespace ::testing;
94
95
96=== modified file 'tests/unit-tests/test_mir_cookie.cpp'
97--- tests/unit-tests/test_mir_cookie.cpp 2016-03-17 03:38:36 +0000
98+++ tests/unit-tests/test_mir_cookie.cpp 2016-03-23 02:41:04 +0000
99@@ -134,7 +134,7 @@
100 Ge(mir::cookie::Authority::minimum_secret_size));
101 }
102
103-TEST(MirCookieAuthority, DISABLED_given_low_entropy_does_not_hang_or_crash)
104+TEST(MirCookieAuthority, given_low_entropy_does_not_hang_or_crash)
105 { // Regression test for LP: #1536662 and LP: #1541188
106 using namespace testing;
107
108@@ -149,7 +149,7 @@
109 EXPECT_THAT(seconds, Lt(15));
110 }
111
112-TEST(MirCookieAuthority, DISABLED_makes_cookies_quickly)
113+TEST(MirCookieAuthority, makes_cookies_quickly)
114 { // Regression test for LP: #1536662 and LP: #1541188
115 using namespace testing;
116

Subscribers

People subscribed via source and target branches