Join GitHub today
GitHub is home to over 20 million developers working together to host and review code, manage projects, and build software together.
interfaces/default: allow mknod for regular files, pipes and sockets (LP: #1636540) #2749
Conversation
Some checks were not successful
|
@tyhicks - can you take a look at the policy syntax changes? |
| @@ -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) |
zyga
Jan 30, 2017
Contributor
Can you please explain the reasoning behind this? I'm just unfamiliar and I'd like to understand it better.
jdstrand
Jan 30, 2017
Contributor
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'.
|
Closing for the moment since the spread test failure needs to be investigated. |
jdstrand
closed this
Jan 31, 2017
jdstrand
added some commits
Jan 31, 2017
Some checks were not successful
jdstrand
reopened this
Jan 31, 2017
jdstrand
added some commits
Jan 31, 2017
Some checks were not successful
Some checks were not successful
Some checks were not successful
|
The final logged seccomp event is about system call 201 which on x86 is |
|
@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. |
|
And of course, this seems to be an intermittent failure: |
|
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? |
jdstrand
added some commits
Feb 1, 2017
All checks have failed
Some checks were not successful
|
This is annoying. The autopkgtests fail with tests/main/gccgo and so does continuous-integration via travis, but locally and running |
Some checks were not successful
|
Ok, finally got gccgo test to work. Now unrelated tests are failing: |
|
After a couple of tries, the unrelated tests (finally) passed. |
jdstrand
changed the title from
interfaces/default: allow mknod for regular files, pipes and sockets
to
interfaces/default: allow mknod for regular files, pipes and sockets (LP: #1636540)
Feb 2, 2017
jdstrand
added some commits
Feb 2, 2017
Some checks were not successful
Some checks were not successful
|
FYI, @tyhicks and I discussed the syntax changes over IRC and hangout and we agreed the syntax changes in this PR are ok. |
|
The zesty-amd64 test failures are unrelated: |
Some checks were not successful
|
The continuous integration test is unrelated: |
All checks have passed
|
@mvo5 - fyi, not sure what happened if anything, but the CI tests got restarted and just passed. |
zyga
requested changes
Feb 14, 2017
Let's please wait until snap-confine is executed from the core before landing this
mvo5
added this to the
2.23 milestone
Feb 14, 2017
niemeyer
approved these changes
Feb 14, 2017
Seems fine, but please make sure tests are actually running and verifying what they're supposed to together with the remaining of our automated spread tests.
| @@ -18,7 +18,7 @@ execute: | | ||
| ! /snap/bin/snapd-hacker-toolbelt.busybox true | ||
| sysctl -w kernel.printk_ratelimit=$orig_ratelimit | ||
| echo "Not only the command failed because snap-confine failed, we see why!" | ||
| - dmesg | grep 'apparmor="DENIED" operation="mount" info="failed srcname match" error=-13 profile="/usr/lib/snapd/snap-confine" name="/snap/bin/" pid=[0-9]\+ comm="ubuntu-core-lau" srcname="/snap/snapd-hacker-toolbelt/[0-9]\+/mnt/" flags="rw, bind"' | ||
| + dmesg --ctime | grep 'apparmor="DENIED" operation="mount" info="failed srcname match" error=-13 profile="/usr/lib/snapd/snap-confine" name="/snap/bin/" pid=[0-9]\+ comm="ubuntu-core-lau" srcname="/snap/snapd-hacker-toolbelt/[0-9]\+/mnt/" flags="rw, bind"' |
niemeyer
Feb 14, 2017
Contributor
I don't think these tests are even running. Actual tests live in /tests by now.
We need to kill these for good @zyga.
| + # apparmor | ||
| + dmesg --ctime | grep DENIED || true | ||
| + # seccomp | ||
| + dmesg --ctime | grep type=1326 || true |
mvo5
removed this from the
2.23 milestone
Feb 16, 2017
|
Yeah, we can not land this before #2810 has landed |
Some checks were not successful
mvo5
approved these changes
Mar 3, 2017
We re-generate seccomp/apparmor in 2.23 now on startup. So this can land now AFAICT.
Some checks were not successful
|
@mvo5 - "We re-generate seccomp/apparmor in 2.23 now on startup. So this can land now AFAICT." We did? Part of that change was supposed to revert the revert of the quotactl and 'ioctl - !TIOCSTI' rules, but that isn't in trunk now. |
|
@jdstrand Uh, sorry. We do re-generate the profiles now but that is not sufficient because we do not yet run the matching snap-confine. So This can not quite be merged :/ |
chipaca
added
the
Blocked
label
Mar 15, 2017
jdstrand
and others
added some commits
Mar 29, 2017
Some checks were not successful
Some checks were not successful
|
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. |
jdstrand
closed this
Apr 17, 2017
jdstrand
removed
the
Blocked
label
Apr 17, 2017
jdstrand
added some commits
Apr 18, 2017
|
I'm told that we can start landing the seccomp changes since snap-confine now is re-exec'd as necessary on classic. |
jdstrand
reopened this
Apr 18, 2017
mvo5
and others
added some commits
Apr 19, 2017
Some checks were not successful
Some checks were not successful
elopio
referenced this pull request
in bigchaindb/bigchaindb
Apr 20, 2017
Merged
Add the packaging metadata to build the bigchaindb snap #1415
Some checks were not successful
| @@ -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 |
jdstrand
added some commits
Apr 27, 2017
Some checks were not successful
| @@ -1 +1 @@ | ||
| -VERSION=unknown | ||
| +VERSION=2.24 |
Some checks were not successful
| @@ -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 |
jdstrand
Apr 27, 2017
•
Edited 1 time
-
jdstrand
Apr 27, 2017
Contributor
-
Apr 27, 2017
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.
Some checks were not successful
|
The xenial-i386 failure is unrelated: |
zyga
merged commit 25e1239
into
snapcore:master
Apr 27, 2017
5 of 6 checks passed
|
Thanks for the reviews and merge! :) |
jdstrand commentedJan 30, 2017
•
Edited 1 time
-
jdstrand
Feb 2, 2017
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.