-
Notifications
You must be signed in to change notification settings - Fork 652
snap-confine, snap-seccomp: utilize new seccomp logging features #3998
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from all commits
Commits
Show all changes
21 commits
Select commit
Hold shift + click to select a range
0126a77
snap-confine, snap-seccomp: Default to SECCOMP_RET_ERRNO
tyhicks 25d3024
cmd/snap-seccomp: Use seccomp's SECCOMP_RET_LOG mode for devmode
tyhicks 3926506
snap-confine: Add curly braces to single line conditional blocks
tyhicks b1ba332
snap-seccomp: Add comment describing seccomp fallback behavior
tyhicks 1b97890
snap-seccomp: update tests for new ActErrno behaviour
mvo5 c09d060
snap-seccomp: simplify tests, remove use of main.SeccompRetKill
mvo5 34af16c
snap-seccomp: update github.com/mvo5/libseccomp-golang revision
mvo5 0e1758b
snap-seccomp: implement preprocessing suggestion from jdstrand
mvo5 c24e6d3
run `make fmt`
mvo5 b8bed79
Merge remote-tracking branch 'upstream/master' into HEAD
mvo5 32f2598
Merge remote-tracking branch 'upstream/master' into HEAD
mvo5 beb7751
Merge remote-tracking branch 'upstream/master' into HEAD
mvo5 bb359f1
debian: ensure we use the right libseccomp-dev version
mvo5 f77345f
cmd/snap-confine: add seccomp prototype (we build with -Wstrict-proto…
mvo5 dd0e16f
debian: drop version in seccomp-dev b-d
mvo5 570c9e3
Merge remote-tracking branch 'upstream/master' into HEAD
mvo5 3edf853
Merge remote-tracking branch 'origin/master' into seccomp-logging
tyhicks 00e4c9e
snap-seccomp: conditionally use ActLog for devmode
tyhicks 3ff75cd
address review feedback; implement system-key for seccomp logging, ad…
d496b76
small test setup cleanup
4bf7516
interfaces/seccomp: mock seccomp actions for testing
zyga File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -122,6 +122,9 @@ package main | |
| //#define SCMP_ARCH_S390X ARCH_BAD | ||
| //#endif | ||
| // | ||
| //#ifndef SECCOMP_RET_LOG | ||
| //#define SECCOMP_RET_LOG 0x7ffc0000U | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I looked up the value to this patch so +1 |
||
| //#endif | ||
| // | ||
| //typedef struct seccomp_data kernel_seccomp_data; | ||
| // | ||
|
|
@@ -390,6 +393,7 @@ var seccompResolver = map[string]uint64{ | |
| const ( | ||
| SeccompRetAllow = C.SECCOMP_RET_ALLOW | ||
| SeccompRetKill = C.SECCOMP_RET_KILL | ||
| SeccompRetLog = C.SECCOMP_RET_LOG | ||
| ) | ||
|
|
||
| // UbuntuArchToScmpArch takes a dpkg architecture and converts it to | ||
|
|
@@ -610,49 +614,93 @@ func addSecondaryArches(secFilter *seccomp.ScmpFilter) error { | |
| return nil | ||
| } | ||
|
|
||
| var errnoOnDenial int16 = C.EPERM | ||
|
|
||
| func preprocess(content []byte) (unrestricted, complain bool) { | ||
| scanner := bufio.NewScanner(bytes.NewBuffer(content)) | ||
| for scanner.Scan() { | ||
| line := strings.TrimSpace(scanner.Text()) | ||
| switch line { | ||
| case "@unrestricted": | ||
| unrestricted = true | ||
| case "@complain": | ||
| complain = true | ||
| } | ||
| } | ||
| return unrestricted, complain | ||
| } | ||
|
|
||
| func complainAction() seccomp.ScmpAction { | ||
| // XXX: Work around some distributions not having a new enough | ||
| // libseccomp-golang that declares ActLog. Instead, we'll guess at its | ||
| // value by adding one to ActAllow and then verify that the string | ||
| // representation is what we expect for ActLog. The value and string is | ||
| // defined in https://github.com/seccomp/libseccomp-golang/pull/29. | ||
| // | ||
| // Ultimately, the fix for this workaround is to be able to use the | ||
| // GetApi() function created in the PR above. It'll tell us if the | ||
| // kernel, libseccomp, and libseccomp-golang all support ActLog. | ||
| var actLog seccomp.ScmpAction = seccomp.ActAllow + 1 | ||
|
|
||
| if actLog.String() == "Action: Log system call" { | ||
| return actLog | ||
| } | ||
|
|
||
| // Because ActLog is functionally ActAllow with logging, if we don't | ||
| // support ActLog, fallback to ActLog. | ||
| return seccomp.ActAllow | ||
| } | ||
|
|
||
| func compile(content []byte, out string) error { | ||
| var err error | ||
| var secFilter *seccomp.ScmpFilter | ||
|
|
||
| secFilter, err = seccomp.NewFilter(seccomp.ActKill) | ||
| unrestricted, complain := preprocess(content) | ||
| switch { | ||
| case unrestricted: | ||
| return osutil.AtomicWrite(out, bytes.NewBufferString("@unrestricted\n"), 0644, 0) | ||
| case complain: | ||
| var complainAct seccomp.ScmpAction = complainAction() | ||
|
|
||
| secFilter, err = seccomp.NewFilter(complainAct) | ||
| if err != nil { | ||
| if complainAct != seccomp.ActAllow { | ||
| // ActLog is only supported in newer versions | ||
| // of the kernel, libseccomp, and | ||
| // libseccomp-golang. Attempt to fall back to | ||
| // ActAllow before erroring out. | ||
| complainAct = seccomp.ActAllow | ||
| secFilter, err = seccomp.NewFilter(complainAct) | ||
| } | ||
| } | ||
|
|
||
| // Set unrestricted to 'true' to fallback to the pre-ActLog | ||
| // behavior of simply setting the allow filter without adding | ||
| // any rules. | ||
| if complainAct == seccomp.ActAllow { | ||
| unrestricted = true | ||
| } | ||
| default: | ||
| secFilter, err = seccomp.NewFilter(seccomp.ActErrno.SetReturnCode(errnoOnDenial)) | ||
| } | ||
| if err != nil { | ||
| return fmt.Errorf("cannot create seccomp filter: %s", err) | ||
| } | ||
|
|
||
| if err := addSecondaryArches(secFilter); err != nil { | ||
| return err | ||
| } | ||
|
|
||
| scanner := bufio.NewScanner(bytes.NewBuffer(content)) | ||
| for scanner.Scan() { | ||
| line := strings.TrimSpace(scanner.Text()) | ||
|
|
||
| // special case: unrestricted means we stop early, we just | ||
| // write this special tag and evalulate in snap-confine | ||
| if line == "@unrestricted" { | ||
| return osutil.AtomicWrite(out, bytes.NewBufferString(line+"\n"), 0644, 0) | ||
| } | ||
| // complain mode is a "allow-all" filter for now until | ||
| // we can land https://github.com/snapcore/snapd/pull/3998 | ||
| if line == "@complain" { | ||
| secFilter, err = seccomp.NewFilter(seccomp.ActAllow) | ||
| if err != nil { | ||
| return fmt.Errorf("cannot create seccomp filter: %s", err) | ||
| } | ||
| if err := addSecondaryArches(secFilter); err != nil { | ||
| return err | ||
| if !unrestricted { | ||
| scanner := bufio.NewScanner(bytes.NewBuffer(content)) | ||
| for scanner.Scan() { | ||
| if err := parseLine(scanner.Text(), secFilter); err != nil { | ||
| return fmt.Errorf("cannot parse line: %s", err) | ||
| } | ||
| break | ||
| } | ||
|
|
||
| // look for regular syscall/arg rule | ||
| if err := parseLine(line, secFilter); err != nil { | ||
| return fmt.Errorf("cannot parse line: %s", err) | ||
| if scanner.Err(); err != nil { | ||
| return err | ||
| } | ||
| } | ||
| if scanner.Err(); err != nil { | ||
| return err | ||
| } | ||
|
|
||
| if osutil.GetenvBool("SNAP_SECCOMP_DEBUG") { | ||
| secFilter.ExportPFC(os.Stdout) | ||
|
|
||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is seccomp defined in the pre-processor usually?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, the
seccomp()syscall may not have a glibc wrapper:https://lwn.net/Articles/655028/
It is important that we wrap it in that situation.