Mir

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

Proposed by Daniel van Vugt
Status: Merged
Approved by: Daniel van Vugt
Approved revision: no longer in the source branch.
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 Approve
Daniel van Vugt Approve
Alan Griffiths Abstain
Alexandros Frantzis (community) Approve
Brandon Schaefer (community) Approve
Chris Halse Rogers Needs Information
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.
Revision history for this message
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)
Revision history for this message
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
Revision history for this message
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
Revision history for this message
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
Revision history for this message
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)

Revision history for this message
Brandon Schaefer (brandontschaefer) wrote : Posted in a previous version of this proposal
Revision history for this message
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/

Revision history for this message
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!

Revision history for this message
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.

Revision history for this message
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
Revision history for this message
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
Revision history for this message
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)
Revision history for this message
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.

Revision history for this message
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.

Revision history for this message
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)
Revision history for this message
Daniel van Vugt (vanvugt) wrote :

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

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

Revision history for this message
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.

Revision history for this message
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)
Revision history for this message
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
Revision history for this message
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
Revision history for this message
Daniel van Vugt (vanvugt) wrote :

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

Revision history for this message
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

Revision history for this message
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.

Revision history for this message
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)
Revision history for this message
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).

Revision history for this message
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)
Revision history for this message
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
Revision history for this message
Alan Griffiths (alan-griffiths) wrote :

I think Chris's concern has been addressed

Revision history for this message
Daniel van Vugt (vanvugt) wrote :

Landing blocked waiting on reviews of the prerequisite branch

Revision history for this message
Brandon Schaefer (brandontschaefer) wrote :

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

Revision history for this message
Mir CI Bot (mir-ci-bot) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
Daniel van Vugt (vanvugt) wrote :

^ failures are just Jenkins infrastructure failures.

Revision history for this message
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

Revision history for this message
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)
Revision history for this message
Brandon Schaefer (brandontschaefer) wrote :

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

Revision history for this message
Daniel van Vugt (vanvugt) wrote :

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

Revision history for this message
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)
Revision history for this message
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
Revision history for this message
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.

Revision history for this message
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)
Revision history for this message
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. ;)

Revision history for this message
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.

Revision history for this message
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.

Revision history for this message
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.

Revision history for this message
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
Revision history for this message
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
Revision history for this message
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.

Revision history for this message
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)
Revision history for this message
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)

Revision history for this message
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)
Revision history for this message
Daniel van Vugt (vanvugt) wrote :

Try again...

review: Approve
Revision history for this message
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)
Revision history for this message
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
=== modified file 'src/cookie/authority.cpp'
--- src/cookie/authority.cpp 2016-01-29 08:18:22 +0000
+++ src/cookie/authority.cpp 2016-03-23 02:41:04 +0000
@@ -41,9 +41,6 @@
4141
42namespace42namespace
43{43{
44std::string const random_device_path{"/dev/random"};
45std::string const urandom_device_path{"/dev/urandom"};
46uint32_t const wait_seconds{30};
47uint32_t const mac_byte_size{20};44uint32_t const mac_byte_size{20};
4845
49size_t cookie_size_from_format(mir::cookie::Format const& format)46size_t cookie_size_from_format(mir::cookie::Format const& format)
@@ -62,55 +59,19 @@
6259
63static mir::cookie::Secret get_random_data(unsigned size)60static mir::cookie::Secret get_random_data(unsigned size)
64{61{
62 int fd = open("/dev/urandom", O_RDONLY);
63 if (fd < 0)
64 BOOST_THROW_EXCEPTION(std::system_error(errno, std::system_category(),
65 "open failed on urandom"));
66
65 mir::cookie::Secret buffer(size);67 mir::cookie::Secret buffer(size);
66 int random_fd;68 unsigned got = read(fd, buffer.data(), size);
67 int retval;69 int error = errno;
68 fd_set rfds;70 close(fd);
6971
70 struct timeval tv;72 if (got != size)
71 tv.tv_sec = wait_seconds;73 BOOST_THROW_EXCEPTION(std::system_error(error, std::system_category(),
72 tv.tv_usec = 0;74 "read failed on urandom"));
73
74 if ((random_fd = open(random_device_path.c_str(), O_RDONLY)) == -1)
75 {
76 int error = errno;
77 BOOST_THROW_EXCEPTION(std::system_error(error, std::system_category(),
78 "open failed on device " + random_device_path));
79 }
80
81 FD_ZERO(&rfds);
82 FD_SET(random_fd, &rfds);
83
84 /* We want to block until *some* entropy exists on boot, then use urandom once we have some */
85 retval = select(random_fd + 1, &rfds, NULL, NULL, &tv);
86
87 /* We are done with /dev/random at this point, and it is either an error or ready to be read */
88 if (close(random_fd) == -1)
89 {
90 int error = errno;
91 BOOST_THROW_EXCEPTION(std::system_error(error, std::system_category(),
92 "close failed on device " + random_device_path));
93 }
94
95 if (retval == -1)
96 {
97 int error = errno;
98 BOOST_THROW_EXCEPTION(std::system_error(error, std::system_category(),
99 "select failed on file descriptor " + std::to_string(random_fd) +
100 " from device " + random_device_path));
101 }
102 else if (retval && FD_ISSET(random_fd, &rfds))
103 {
104 std::uniform_int_distribution<uint8_t> dist;
105 std::random_device rand_dev(urandom_device_path);
106
107 std::generate(std::begin(buffer), std::end(buffer), [&]() { return dist(rand_dev); });
108 }
109 else
110 {
111 BOOST_THROW_EXCEPTION(std::runtime_error("Failed to read from device: " + random_device_path +
112 " after: " + std::to_string(wait_seconds) + " seconds"));
113 }
11475
115 return buffer;76 return buffer;
116}77}
11778
=== modified file 'tests/acceptance-tests/test_server_startup.cpp'
--- tests/acceptance-tests/test_server_startup.cpp 2016-03-17 03:38:36 +0000
+++ tests/acceptance-tests/test_server_startup.cpp 2016-03-23 02:41:04 +0000
@@ -68,7 +68,7 @@
68 });68 });
69}69}
7070
71TEST(ServerStartupReliability, DISABLED_can_start_with_low_entropy)71TEST(ServerStartupReliability, can_start_with_low_entropy)
72{ // Regression test for LP: #1536662 and LP: #154118872{ // Regression test for LP: #1536662 and LP: #1541188
73 using namespace ::testing;73 using namespace ::testing;
7474
7575
=== modified file 'tests/unit-tests/test_mir_cookie.cpp'
--- tests/unit-tests/test_mir_cookie.cpp 2016-03-17 03:38:36 +0000
+++ tests/unit-tests/test_mir_cookie.cpp 2016-03-23 02:41:04 +0000
@@ -134,7 +134,7 @@
134 Ge(mir::cookie::Authority::minimum_secret_size));134 Ge(mir::cookie::Authority::minimum_secret_size));
135}135}
136136
137TEST(MirCookieAuthority, DISABLED_given_low_entropy_does_not_hang_or_crash)137TEST(MirCookieAuthority, given_low_entropy_does_not_hang_or_crash)
138{ // Regression test for LP: #1536662 and LP: #1541188138{ // Regression test for LP: #1536662 and LP: #1541188
139 using namespace testing;139 using namespace testing;
140140
@@ -149,7 +149,7 @@
149 EXPECT_THAT(seconds, Lt(15));149 EXPECT_THAT(seconds, Lt(15));
150}150}
151151
152TEST(MirCookieAuthority, DISABLED_makes_cookies_quickly)152TEST(MirCookieAuthority, makes_cookies_quickly)
153{ // Regression test for LP: #1536662 and LP: #1541188153{ // Regression test for LP: #1536662 and LP: #1541188
154 using namespace testing;154 using namespace testing;
155155

Subscribers

People subscribed via source and target branches