snap-confine

Merge lp://qastaging/~jdstrand/snap-confine/seccom-arg-filtering into lp://qastaging/~snappy-dev/snap-confine/trunk

  1. seccom-arg-filtering
  2. Merge into trunk
Proposed by Jamie Strandboge
Status: Rejected
Rejected by: Jamie Strandboge
Proposed branch: lp://qastaging/~jdstrand/snap-confine/seccom-arg-filtering
Merge into: lp://qastaging/~snappy-dev/snap-confine/trunk
Diff against target: 974 lines (+823/-31)
10 files modified
README (+82/-18)
debian/changelog (+1/-0)
src/seccomp.c (+313/-13)
tests/test_bad_seccomp_filter_args (+54/-0)
tests/test_bad_seccomp_filter_args_null (+51/-0)
tests/test_bad_seccomp_filter_args_prctl (+55/-0)
tests/test_bad_seccomp_filter_args_socket (+55/-0)
tests/test_restrictions_working_args (+96/-0)
tests/test_restrictions_working_args_prctl (+58/-0)
tests/test_restrictions_working_args_socket (+58/-0)
To merge this branch: bzr merge lp://qastaging/~jdstrand/snap-confine/seccom-arg-filtering
Reviewer Review Type Date Requested Status
Tyler Hicks Pending
Snappy Developers Pending
Review via email: mp+291069@code.qastaging.launchpad.net

Description of the change

Implement seccomp arg filtering. See README for a complete description of how policy is affected. Implementation-wise, adjust seccomp.c:
- add the seccomp_args struct for populating seccomp_rule_add_*array
- add hsearch map and a few simple wrapper functions
- add parse_line() which takes a string that went through validate_and_trim_line(). This will parse the string for arguments as described in the README file and setup the seccomp_args struct
- use seccomp_rule_add_exact_array/seccomp_rule_add_array with the seccomp_args struct data instead of seccomp_rule_add_exact/seccomp_rule_add

Currently parse_line can handle enums for:
- man 2 socket - domain
- man 2 socket - type
- man 2 prctl

More can easily be added as needed.

To post a comment you must log in.
lp://qastaging/~jdstrand/snap-confine/seccom-arg-filtering updated
148. By Jamie Strandboge

update README for prctl

Revision history for this message
Tyler Hicks (tyhicks) wrote :

Hi Jamie - could you update this PR with the changes that you made when you merged in the cgroups branch?

lp://qastaging/~jdstrand/snap-confine/seccom-arg-filtering updated
149. By Jamie Strandboge

merge from trunk
adjust filter tests for trunk changes
adjust strdup error message

Revision history for this message
Jamie Strandboge (jdstrand) wrote :

Hi Tyler, I've merged this and also tested with a real world example with electron-quick-start. I used this:

setpriority - 0 >=0

and electron-quick-start worked and then used this:

setpriority - 0 >=20

and it failed:
audit: type=1326 audit(1463522677.088:611): auid=4294967295 uid=1000 gid=1000 ses=4294967295 pid=17596 comm="electron" exe="/snap/electron-quick-start/100006/lib/node_modules/electron-prebuilt/dist/electron" sig=31 arch=c000003e syscall=141 compat=0 ip=0x7fc853d314a7 code=0x0

Note that for setpriority I'm going to need a followup MP for PRIO_PROCESS, PRIO_PGRP, and PRIO_USER from sys/resource.h since we are I think going to actually want this for setpriority in policy:

setpriority PRIO_PROCESS 0 >=0

Revision history for this message
Jamie Strandboge (jdstrand) wrote :

I forgot to mention, note that the logged seccomp denial doesn't give indication that it was an unmatched argument that caused the denial (ie, it looks that same as if 'setpriority' was missing). I'm not sure how this will impact devmode-- it would be nice to have the log show that it was an argument, but we can also handle this in snappy-debug (ie, mention that you have to use setpriority(PRIO_PROCESS, 0, >=0) or something).

This shouldn't impact this MP but thought it worth mentioning.

Revision history for this message
Jamie Strandboge (jdstrand) wrote :

Rejecting this in favor of https://github.com/ubuntu-core/snap-run/pull/7

Revision history for this message
Zygmunt Krynicki (zyga) wrote :

Hey, could you please re-target this pull request to
github.com/CanonicalLtd/snap-run

On Fri, May 13, 2016 at 1:12 AM, Tyler Hicks <email address hidden> wrote:
> Hi Jamie - could you update this PR with the changes that you made when you merged in the cgroups branch?
> --
> https://code.launchpad.net/~jdstrand/ubuntu-core-launcher/seccom-arg-filtering/+merge/291069
> Your team Snappy Developers is requested to review the proposed merge of lp:~jdstrand/ubuntu-core-launcher/seccom-arg-filtering into lp:ubuntu-core-launcher.

Unmerged revisions

149. By Jamie Strandboge

merge from trunk
adjust filter tests for trunk changes
adjust strdup error message

148. By Jamie Strandboge

update README for prctl

147. By Jamie Strandboge

cleanup some comments

146. By Jamie Strandboge

use hcreate_r(), hsearch_r() and hdestroy_r() instead of hcreate(), hsearch()
and hdestroy() respectively

145. By Jamie Strandboge

add some clarifying comments
fix a comment
free(buf_copy) as soon as possible instead of waiting

144. By Jamie Strandboge

clarify some comments
use strtoul() instead of sscanf()

143. By Jamie Strandboge

removes some parthesis that aren't needed
clarify a comment

142. By Jamie Strandboge

tests/test_bad_seccomp_filter_args_null: adjust for fgets() limitations

141. By Jamie Strandboge

add some embedded NULL tests

140. By Jamie Strandboge

add prctl PR_ mappings

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
The diff is not available at this time. You can reload the page or download it.

Subscribers

People subscribed via source and target branches