apparmor check version on deb install #17002
Conversation
jessfraz
added
area/security/apparmor
status/2-code-review
and removed
status/0-triage
labels
|
running across all versions now to test |
jessfraz
force-pushed
the
apparmor-check-version-on-deb-install
branch
2 times, most recently
from
be722d3 to
7d8df7c
Compare
|
Wish I thought of this :) Good job. |
jessfraz
force-pushed
the
apparmor-check-version-on-deb-install
branch
7 times, most recently
from
7b4aea1 to
9310c92
Compare
|
I added a test script |
|
also ping @tianon |
contrib/apparmor/main.go
Outdated
There was a problem hiding this comment.
Won't this mean that 3.9 > 3.10 if we just do the naïve float conversion here like this?
There was a problem hiding this comment.
oh shoot
On Fri, Oct 16, 2015 at 4:54 PM, Tianon Gravi [email protected]
wrote:
In contrib/apparmor/main.go
#17002 (comment):
- }
- // parse the version from the output
- // output is in the form of the following:
- // AppArmor parser version 2.9.1
- // Copyright (C) 1999-2008 Novell Inc.
- // Copyright 2009-2012 Canonical Ltd.
- lines := strings.SplitN(string(output), "\n", 2)
- words := strings.Split(lines[0], " ")
- version := words[len(words)-1]
- // get major minor version only
- if strings.Count(version, ".") > 1 {
version = version[:strings.LastIndex(version, ".")]- }
- v, err := strconv.ParseFloat(version, 64)
Won't this mean that 3.9 > 3.10 if we just do the naïve float conversion
here like this?—
Reply to this email directly or view it on GitHub
https://github.com/docker/docker/pull/17002/files#r42301473.
There was a problem hiding this comment.
facepalm was trying to avoid using a version package like ours
On Fri, Oct 16, 2015 at 4:57 PM, Jessie Frazelle [email protected] wrote:
oh shoot
On Fri, Oct 16, 2015 at 4:54 PM, Tianon Gravi [email protected]
wrote:In contrib/apparmor/main.go
#17002 (comment):
- }
- // parse the version from the output
- // output is in the form of the following:
- // AppArmor parser version 2.9.1
- // Copyright (C) 1999-2008 Novell Inc.
- // Copyright 2009-2012 Canonical Ltd.
- lines := strings.SplitN(string(output), "\n", 2)
- words := strings.Split(lines[0], " ")
- version := words[len(words)-1]
- // get major minor version only
- if strings.Count(version, ".") > 1 {
version = version[:strings.LastIndex(version, ".")]- }
- v, err := strconv.ParseFloat(version, 64)
Won't this mean that 3.9 > 3.10 if we just do the naïve float conversion
here like this?—
Reply to this email directly or view it on GitHub
https://github.com/docker/docker/pull/17002/files#r42301473.
There was a problem hiding this comment.
…parser version Signed-off-by: Jessica Frazelle <[email protected]>
Signed-off-by: Jessica Frazelle <[email protected]>
Signed-off-by: Jessica Frazelle <[email protected]>
Signed-off-by: Jessica Frazelle <[email protected]>
jessfraz
force-pushed
the
apparmor-check-version-on-deb-install
branch
from
9310c92 to
14e85c8
Compare
|
updated |
|
ping @tianon PTAL |
|
Cool, LGTM 👍 |
|
@jfrazelle looks good, but it will need some refactoring if we later reintroduce the container policies into contrib/apparmor. It's okay, but worth noting for readers of this thread / docs-folks that this is not changing current AppArmor config, but is installing a new policy which defines what the docker-engine itself can do (rather than containers). This policy is in complain-only mode, so there should be no harm in installing it except, perhaps, some additional warning messages in dmesg. |
|
@ewindisch @jfrazelle if it's in complain-only mode, would it be good to include it in 1.9? Then we can collect feedback and have a whole release to fine-tune if needed |
|
No we should wait and test more for sure no rushing apparmor fixes ;) |
|
but by all means lets merge it :P |
|
YOLO, LGTM, as a wise man once taught me; looks like Go and AppArmor, merging! we can refactor when the container policies land |
…b-install apparmor check version on deb install
5 participants
This allows us to use the apparmor profile we have in
contrib/apparmor/and solves the problems where certain functions are not apparent on older versions ofapparmor_parseron debian/ubuntu.Also runs
apparmor_parser --debug docker-engineon the generated profile to make sure that it actually works with the version in the build container.After this we can remove the docker-default from the engine code and generate it also like this. But I wanted this PR to be simple (or at least as simple as an apparmor PR gets ;).
ping @ewindisch