snap-confine

Merge lp://qastaging/~mterry/snap-confine/fix-tests into lp://qastaging/~snappy-dev/snap-confine/trunk

fix-tests
Merge into trunk

Proposed by Michael Terry on 2015-05-29

Status:	Merged
Approved by:	John Lenton on 2015-06-03
Approved revision:	69
Merged at revision:	67
Proposed branch:	lp://qastaging/~mterry/snap-confine/fix-tests
Merge into:	lp://qastaging/~snappy-dev/snap-confine/trunk
Diff against target:	55 lines (+15/-13) 2 files modified Makefile (+2/-0) src/main.c (+13/-13)
To merge this branch:	bzr merge lp://qastaging/~mterry/snap-confine/fix-tests
Related bugs:	Link a bug report

Reviewer	Date Requested	Status
Michael Vogt (community)	2015-05-29	Approve on 2015-06-03
Tyler Hicks (community)		Approve on 2015-05-29
Review via email: mp+260618@code.qastaging.launchpad.net

Commit message

Fix tests to work again.

Description of the change

Fix tests to work again.

They all rely on passing a command like /bin/true or similar in. But these fail the check that the executable must be in /apps, /frameworks, or /oem.

I fixed the tests by ignoring that check when SNAPPY_LAUNCHER_INSIDE_TESTS is defined. Is doing so a security issue? (i.e. is that check actually important, or just a helpful sanity check?)

I've also enabled tests/test_whitelist to actually run by making it executable. And added a quick 'check' target to the top Makefile.

Revision history for this message

Tyler Hicks (tyhicks) wrote on 2015-05-29:

How about moving the original strstr() tests inside of the if(geteuid() == 0) { ... } block? That will result in the binary path restrictions being in place when the launcher is installed as a setuid root binary. Those restrictions will not be in place during test runs.

Revision history for this message

Michael Terry (mterry) wrote on 2015-05-29:

That doesn't help with the security side of things (since a malicious user can also skip that block with UBUNTU_CORE_LAUNCHER_NO_ROOT=1).

And checking the binary path isn't otherwise a root-user-thing, right?. So I'm leery of throwing it in that block just to avoid a check for SNAPPY_LAUNCHER_INSIDE_TESTS.

Revision history for this message

Tyler Hicks (tyhicks) wrote on 2015-05-29:

That does help with the security side of things. A malicious user cannot bypass that block because the binary will be setuid root. geteuid() will always return 0 in production.

lp://qastaging/~mterry/snap-confine/fix-tests updated on 2015-05-29

67. By Michael Terry on 2015-05-29: And add test target, so that debuild will automatically run tests
68. By Michael Terry on 2015-05-29: Undo test target addition, move path-check block inside uid==0 check