interfaces/default: allow mknod for regular files, pipes and sockets (LP: #1636540) #2749

jdstrand · 2017-01-30T21:12:33Z

This adds SCMP_CMP_MASKED_EQ support for doing things like '|S_IFIFO' in the policy to support
mknod which uses bitmasks for the permissions and file type. Developing this PR uncovered a number of testsuite rough edges, so fix those along the way.

This add SCMP_CMP_MASKED_EQ support for doing |S_IFIFO in the policy to support mknod which uses bitmasks for the permissions and file type.

jdstrand · 2017-01-30T21:14:01Z

@tyhicks - can you take a look at the policy syntax changes?

zyga · 2017-01-30T21:16:22Z

cmd/snap-confine/seccomp-support.c

@@ -423,7 +435,12 @@ static int parse_line(char *line, struct seccomp_args *sargs)
 		if (errno != 0)
 			return PARSE_ERROR;

-		sargs->arg_cmp[sargs->length] = SCMP_CMP(pos, op, value);
+		if (op == SCMP_CMP_MASKED_EQ)


Can you please explain the reasoning behind this? I'm just unfamiliar and I'd like to understand it better.

SCMP_CMP_MASKED_EQ is what you use to check bitmasks (eg, '|S_IFREG'). Since for mknod(2) the second argument is a bitmask for mode_t, we need to see if S_IFREG is set within the mode_t, and you do that with SCMP_CMP_MASKED_EQ. Because SCMP_CMP takes an additional argument for the mask when op is 'SCMP_CMP_MASKED_EQ', we check if the op is 'SCMP_CMP_MASKED_EQ' and give that extra argument, otherwise we do like we've always done. It is sufficient for us to use 'value' for both the mask and the datum because if we have have rule mknod - |S_IFREG - then <mode_t>|S_IFREG matches S_IFREG when using something like mknod(..., S_IFREG|S_IRUSR|S_IWUSR, ...).

For more information, see 'man 3 seccomp_rule_add'.

jdstrand · 2017-01-31T13:51:04Z

Closing for the moment since the spread test failure needs to be investigated.

…filtering

- adjust common.sh to create a temp dir in /run/shm that is auto-cleaned - put the pipes in /run/shm since run is shared between global and app mount namespace - adjust logic of perms test to use 'stat' instead of 'test -w' for when the testsuite is run as root (as with spread) - adjust common.sh to add more helpful debug output as part of FAIL

… FAIL

- disable kernel rate limiting - also search for seccomp denials in debug output

zyga · 2017-02-01T09:04:12Z

The final logged seccomp event is about system call 201 which on x86 is geteuid32 but AFAIR it was added to common.sh when i386 support was fixed.

jdstrand · 2017-02-01T12:19:37Z

@zyga - the geteuid32 is in the default policy but the denial is not from this test. I think it is from test_restrictions, but I'll make this test clearer so the geteuid32/geteuid stops confusing people.

If you look at the latest travis failure, it is in linode:ubuntu-16.04-32:tests/main/gccgo. c-unit-tests passes everywhere now. The failure in linode:ubuntu-16.04-32:tests/main/gccgo is confusing since c-unit-tests is passing and so are the autopkgtests (minus ppc64el). I'll take a look at that test now.

jdstrand · 2017-02-01T12:41:36Z

And of course, this seems to be an intermittent failure:

$ spread -reuse -resend -debug linode:ubuntu-16.04-32:tests/main/gccgo
2017/02/01 12:21:39 Allocating linode:ubuntu-16.04-32...
2017/02/01 12:22:36 Allocated linode:ubuntu-16.04-32 (Spread-X).
2017/02/01 12:22:36 Connecting to linode:ubuntu-16.04-32 (Spread-X)...
2017/02/01 12:22:53 Connected to linode:ubuntu-16.04-32 (Spread-X) at 45.79.186.250.
2017/02/01 12:22:53 Sending project content to linode:ubuntu-16.04-32 (Spread-X)...
2017/02/01 12:35:09 Successful tasks: 1
2017/02/01 12:35:09 Aborted tasks: 0
2017/02/01 12:35:09 Keeping linode:ubuntu-16.04-32 (Spread-X) at 45.79.186.250

jdstrand · 2017-02-01T12:43:42Z

I checked the original log and saw that linode:ubuntu-16.04-32 went to Spread-C and Spread-D. Perhaps Spread-C or Spread-D is out of date?

Seccomp logs might show up as: audit: type=1326 audit(1485875879.395:1909): auid=1000 uid=0 gid=0 ses=14 pid=13916 comm="snap-confine" exe=".../snap-confine" sig=31 arch=c000003e syscall=107 compat=0 ip=0x7f5a332e46a7 code=0x0 Some tests have expected failures but these tests are all logged as comm="snap-confine" exe="/path/to/snap-confine" which leads to confusion when debugging (eg, in the above example syscall 107 (geteuid) was blocked, but it is in the list of common syscalls. This denial is from test_restrictions which is expected to have geteuid blocked). The fix is to copy snap-confine to the name of the test so that exe and comm use the name of the test. Eg: audit: type=1326 audit(1485956974.614:58308): auid=4294967295 uid=1000 gid=1000 ses=4294967295 pid=3472 comm="test_restrictio" exe="/tmp/tmp.dsDGaRXxWx/test_restrictions" sig=31 arch=c000003e syscall=107 compat=0 ip=0x7f5bd35446a7 code=0x0

…ing off)

jdstrand · 2017-02-01T19:21:33Z

This is annoying. The autopkgtests fail with tests/main/gccgo and so does continuous-integration via travis, but locally and running spread -reuse -resend -debug linode:ubuntu-16.04-32:tests/main/gccgo with spread 27 runs fine. I suspect something wrong with the tests/main/gccgo but I can't get it to reproduce to debug it.

jdstrand · 2017-02-01T22:52:16Z

Ok, finally got gccgo test to work. Now unrelated tests are failing:

2017/02/01 21:45:42 Successful tasks: 520
2017/02/01 21:45:42 Aborted tasks: 1
2017/02/01 21:45:42 Failed tasks: 3
    - linode:ubuntu-14.04-64:tests/main/snap-run-alias:testsnapdtoolscat
    - linode:ubuntu-14.04-64:tests/regression/lp-1595444
    - linode:ubuntu-16.04-32:tests/main/revert-devmode:remote
2017/02/01 21:45:42 Failed task prepare: 1
    - linode:ubuntu-14.04-64:tests/main/interfaces-cups-control

jdstrand · 2017-02-02T03:50:52Z

After a couple of tries, the unrelated tests (finally) passed.

jdstrand · 2017-02-07T23:27:29Z

FYI, @tyhicks and I discussed the syntax changes over IRC and hangout and we agreed the syntax changes in this PR are ok.

jdstrand · 2017-04-17T13:29:01Z

It is my understanding that this is now unblocked, so removing that tag, but closing until I have a chance to review and retest everything.

…p-mknod

jdstrand · 2017-04-18T14:36:45Z

I'm told that we can start landing the seccomp changes since snap-confine now is re-exec'd as necessary on classic.

…omp-mknod

…p-mknod

zyga

+1

zyga · 2017-04-27T15:32:13Z

tests/unit/c-unit-tests/task.yaml

@@ -37,4 +37,4 @@ debug: |
    # Show the test suite failure log if there's one
    cat $SPREAD_PATH/cmd/autogarbage/test-suite.log || true
    # Show seccomp audit messages
-    tail /var/log/kern.log | grep -F type=1326


This is now in default debug-each so we could drop this copy.

…p-mknod

zyga · 2017-04-27T16:06:12Z

data/info

@@ -1 +1 @@
-VERSION=unknown
+VERSION=2.24


You don't want this change.

Hrmm, that came in as a result of the merge from trunk. Removing.

jdstrand · 2017-04-27T18:39:29Z

tests/lib/prepare.sh

@@ -383,4 +391,6 @@ prepare_all_snap() {
        tar czf $SPREAD_PATH/snapd-state.tar.gz /var/lib/snapd $BOOT
        systemctl start snapd.socket
    fi
+
+    disable_kernel_rate_limiting


I think it is these that are causing the issues with the testsuite. There are tons of apparmor denials in the snapd-reexec test for snap.network-bind-consumer.network-consumer:

2017-04-19 13:36:01 Error executing linode:ubuntu-14.04-64:tests/main/snapd-reexec : ----- + '[' '' = 0 ']' + echo 'Ensure we re-exec by default' Ensure we re-exec by default + snap list 2017/04/19 13:35:59.295076 main.go:237: WARNING: cannot create syslog logger 2017/04/19 13:35:59.431711 main.go:237: WARNING: cannot create syslog logger Name Version Rev Developer Notes core 16-2 1736 canonical - + MATCH 'DEBUG: restarting into' + journalctl error: pattern not found, got: -- Logs begin at Wed 2017-04-19 13:31:24 UTC, end at Wed 2017-04-19 13:31:25 UTC. -- Apr 19 13:31:24 ubuntu kernel: audit: type=1400 audit(1492608684.195:6094): apparmor="DENIED" operation="accept" profile="snap.network-bind-consumer.network-consumer" pid=16384 comm="python3" laddr=127.0.0.1 lport=8081 family="inet" sock_type="stream" protocol=6 requested_mask="accept" denied_mask="accept" Apr 19 13:31:24 ubuntu kernel: audit: type=1400 audit(1492608684.195:6095): apparmor="DENIED" operation="accept" profile="snap.network-bind-consumer.network-consumer" pid=16384 comm="python3" laddr=127.0.0.1 lport=8081 family="inet" sock_type="stream" protocol=6 requested_mask="accept" denied_mask="accept"

I'm going to revert the disabling of rate limiting for now so this PR will pass and add a forum topic that the tests should be able to run with this enabled.

jdstrand · 2017-04-27T21:15:51Z

The xenial-i386 failure is unrelated:

2017-04-27 20:16:45 Failed tasks: 1
    - autopkgtest:ubuntu-16.04-i386:tests/main/completion

jdstrand · 2017-04-27T21:23:50Z

Thanks for the reviews and merge! :)

zyga reviewed Jan 30, 2017

View reviewed changes

jdstrand closed this Jan 31, 2017

Jamie Strandboge added 4 commits January 31, 2017 14:50

add 'write' to tests/common.sh for stderr output

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.

GPG key ID: 4AEE18F83AFDEB23
Expired

Verified
Learn about vigilant mode

8a9d75c

default: allow mknod and mkfifo by default since we have seccomp arg …

cb2dd0c

…filtering

test_restrictions_working_args_: remove redundant 'dmesg|tail -n1' on…

97be6b8

… FAIL

jdstrand reopened this Jan 31, 2017

Jamie Strandboge added 4 commits January 31, 2017 20:05

mkfifo needs 'uname' on i386

20450e9

no need to use -F with grep

5ce7e11

increase debug verbosity for security policy denials

e42ceb2

- disable kernel rate limiting - also search for seccomp denials in debug output

common.sh: also allow futex for mkfifo on linode

22d836b

Jamie Strandboge added 4 commits February 1, 2017 13:50

always use dmesg when checking logs as syslogger may skip some messages

a6f8420

use 'dmesg --ctime' for human readable timestamps

392a3f8

don't rely on dmesg since kernel annoying drops (even with rate limit…

f82c805

…ing off)

remove a no longer needed debugging 'strace'

225263e

jdstrand changed the title ~~interfaces/default: allow mknod for regular files, pipes and sockets~~ interfaces/default: allow mknod for regular files, pipes and sockets (LP: #1636540) Feb 2, 2017

Jamie Strandboge added 2 commits February 2, 2017 16:29

Merge remote-tracking branch 'upstream/master' into seccomp-mknod

97e1bad

Merge remote-tracking branch 'upstream/master' into seccomp-mknod

920508a

chipaca added the ⛔ Blocked label Mar 15, 2017

Jamie Strandboge and others added 2 commits March 29, 2017 20:21

Merge remote-tracking branch 'upstream/master' into seccomp-mknod

8a2e1e2

Merge branch 'master' of github.com:snapcore/snapd into seccomp-mknod

d3b4d24

jdstrand closed this Apr 17, 2017

jdstrand removed the ⛔ Blocked label Apr 17, 2017

Jamie Strandboge added 2 commits April 18, 2017 14:32

Merge remote-tracking branch 'upstream/master' into seccomp-mknod

785ba29

Merge branch 'seccomp-mknod' of github.com:jdstrand/snapd into seccom…

53b5ff3

…p-mknod

jdstrand reopened this Apr 18, 2017

mvo5 and others added 3 commits April 19, 2017 08:33

Merge remote-tracking branch 'upstream/master' into HEAD

867619a

Merge remote-tracking branch 'refs/remotes/upstream/master' into secc…

f5b3863

…omp-mknod

Merge branch 'seccomp-mknod' of github.com:jdstrand/snapd into seccom…

9b58b07

…p-mknod

come-maiz mentioned this pull request Apr 20, 2017

Add the packaging metadata to build the bigchaindb snap bigchaindb/bigchaindb#1415

Merged

zyga approved these changes Apr 27, 2017

View reviewed changes

Merge branch 'master' of github.com:snapcore/snapd into seccomp-mknod

4987fda

zyga approved these changes Apr 27, 2017

View reviewed changes

zyga reviewed Apr 27, 2017

View reviewed changes

Jamie Strandboge added 3 commits April 27, 2017 15:58

Merge remote-tracking branch 'upstream/master' into seccomp-mknod

a94cca4

remove extra seccomp debug output (thanks zyga)

c857fd4

Merge branch 'seccomp-mknod' of github.com:jdstrand/snapd into seccom…

58e03e5

…p-mknod

zyga requested changes Apr 27, 2017

View reviewed changes

revert change in previous commit to data/info

dd0088c

jdstrand commented Apr 27, 2017

View reviewed changes

disable the disabling of kernel rate limiting and reference forum

45a4b0b

zyga approved these changes Apr 27, 2017

View reviewed changes

zyga merged commit 25e1239 into canonical:master Apr 27, 2017

jdstrand deleted the seccomp-mknod branch May 3, 2017 21:28

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

interfaces/default: allow mknod for regular files, pipes and sockets (LP: #1636540) #2749

interfaces/default: allow mknod for regular files, pipes and sockets (LP: #1636540) #2749

jdstrand commented Jan 30, 2017 •

edited

Loading

jdstrand commented Jan 30, 2017

zyga Jan 30, 2017

jdstrand Jan 30, 2017

jdstrand commented Jan 31, 2017

zyga commented Feb 1, 2017

jdstrand commented Feb 1, 2017

jdstrand commented Feb 1, 2017

jdstrand commented Feb 1, 2017 •

edited

Loading

jdstrand commented Feb 1, 2017 •

edited

Loading

jdstrand commented Feb 1, 2017

jdstrand commented Feb 2, 2017

jdstrand commented Feb 7, 2017

jdstrand commented Apr 17, 2017

jdstrand commented Apr 18, 2017

zyga left a comment

zyga Apr 27, 2017

zyga Apr 27, 2017

jdstrand Apr 27, 2017

jdstrand Apr 27, 2017 •

edited

Loading

jdstrand commented Apr 27, 2017

jdstrand commented Apr 27, 2017

		@@ -1 +1 @@
		VERSION=unknown
		VERSION=2.24

interfaces/default: allow mknod for regular files, pipes and sockets (LP: #1636540) #2749

interfaces/default: allow mknod for regular files, pipes and sockets (LP: #1636540) #2749

Conversation

jdstrand commented Jan 30, 2017 • edited Loading

jdstrand commented Jan 30, 2017

zyga Jan 30, 2017

Choose a reason for hiding this comment

jdstrand Jan 30, 2017

Choose a reason for hiding this comment

jdstrand commented Jan 31, 2017

zyga commented Feb 1, 2017

jdstrand commented Feb 1, 2017

jdstrand commented Feb 1, 2017

jdstrand commented Feb 1, 2017 • edited Loading

jdstrand commented Feb 1, 2017 • edited Loading

jdstrand commented Feb 1, 2017

jdstrand commented Feb 2, 2017

jdstrand commented Feb 7, 2017

jdstrand commented Apr 17, 2017

jdstrand commented Apr 18, 2017

zyga left a comment

Choose a reason for hiding this comment

zyga Apr 27, 2017

Choose a reason for hiding this comment

zyga Apr 27, 2017

Choose a reason for hiding this comment

jdstrand Apr 27, 2017

Choose a reason for hiding this comment

jdstrand Apr 27, 2017 • edited Loading

Choose a reason for hiding this comment

jdstrand commented Apr 27, 2017

jdstrand commented Apr 27, 2017

jdstrand commented Jan 30, 2017 •

edited

Loading

jdstrand commented Feb 1, 2017 •

edited

Loading

jdstrand commented Feb 1, 2017 •

edited

Loading

jdstrand Apr 27, 2017 •

edited

Loading