Skip to content
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

Feature/native security policygen regen #55

Merged
merged 142 commits into from
Nov 17, 2015

Conversation

mvo5
Copy link
Contributor

@mvo5 mvo5 commented Nov 4, 2015

This branch implements native apparmor and seccomp generation from snappy itself. Once this branch lands we can remove aa-clickhook from the image. Its unfortunately huge. It is split up:
feature/native-security-policygen
feature/native-security-policygen-add-hwaccess
feature/native-security-policygen-compare

  • feature/native-security-policygen-regen
    so I could split it into smaller chunks, but I'm not sure this helps much as the first branch is already pretty big :/

Anyway, early feedback would be great. I will also ask Jamie to see if there is anything missing. Please do not merge before Jamie had a chance to have a look.

Jamie Strandboge added 30 commits October 21, 2015 06:53
- fix lint issues
- implement loadAppArmorPolicy()
- implement security-policy generation
- go lint checks
- adjust findCaps and findTemplate to handle framework-policy
- adjust error reporting for not found template and caps
- fix a couple badly copy-pasted paths
- implement dbusPath() and use it in apparmor policy generation
fix lint issues
compat manifest (more cleanups needed)
cmd/policygen/main.go, snappy/security.go: adjust argument to
GenerateSecurityPolicyFromFile
snappy/security.go

detect Ubuntu system version

dirs/dirs.go: adjust for /var/lib/snappy/apparmor/profiles
snappy/snapp.go:
- remove JSON timestamp code
- update RequestAppArmorUpdate() for new profile generation
- use generatePolicy() instead of click wrapper code
generatePolicyForServiceBinary()
- refactor
- remove legacyIntegrateSecDef
snappy/security.go: implement security overrides in yaml
- set apparmor_parser up for mocking
- better handle overrides
@jdstrand
Copy link

mvo and I went through the additions and bugs and this is ok to merge with the latest commits. There is additional work that should be done, but it is minor and should not block this already large pull request. Thanks to everyone for the reviews and especially thanks to mvo for all the help on the remaining issues this week.

* `read-paths`: a list of additional paths that the app can read
* `write-paths`: a list of additional paths that the app can write
* `abstractions`: a list of additional abstractions for the app
* `syscalls`: a list of additional syscalls that the app can use
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is an area I really hope we can clean up with capabilities, but I'm not yet sure we'll have time for 16.04. It feels like way too many variations for something that should be internal to the system, and offered via an abstraction that we own.

@niemeyer
Copy link
Contributor

Okay, I admit partial defeat, as I don't want to block you on this anymore. The branch is big, dense and dependent on external logic (apparmor, seccomp details), which makes it hard to keep tracking in my mind the most relevant detail which is whether the logic works or not.. so please feel free to merge this once you're happy. Having more time using it will be much more useful.

@mvo5
Copy link
Contributor Author

mvo5 commented Nov 16, 2015

Thanks a lot @niemeyer for the review. I agree its a tricky branch. I think once we resolved the two open questions about the error handling with @jdstrand then we can merge this one and iterate on it to make it nicer. The subsequent branches should be easier to review as they will not introduce radical changes like this one (which effectively reimplements most of aa-clickhook).

@mvo5
Copy link
Contributor Author

mvo5 commented Nov 17, 2015

retest this please

@mvo5
Copy link
Contributor Author

mvo5 commented Nov 17, 2015

retest this please

mvo5 added a commit that referenced this pull request Nov 17, 2015
@mvo5 mvo5 merged commit eb45d7b into canonical:master Nov 17, 2015
zyga added a commit to zyga/snapd that referenced this pull request Nov 7, 2016
flocko-motion pushed a commit to ML-PA-Consulting-GmbH/snapd that referenced this pull request Feb 2, 2024
Add command line option to print the calculated states
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants