(11:03:12 AM) jjohansen: cboltz, sbeattie, sarnold, rlee287, georgiag, mbelair_, iskunk: meeting time
(11:03:18 AM) rlee287: Hello
(11:03:25 AM) iskunk: yo
(11:03:53 AM) ahasenack_ left the room (quit: Quit: Leaving).
(11:05:04 AM) ***cboltz hides
(11:05:51 AM) ahasenack [~andreas@177.16.32.249] entered the room.
(11:05:52 AM) jjohansen: \o/
(11:06:45 AM) jjohansen: so did we want/are we ready to argue about policy dir handling again
(11:07:20 AM) jjohansen: whether we are consistent with all dirs, or treat existing locations special?
(11:09:51 AM) ***cboltz looks for the boxing gloves
(11:11:14 AM) jjohansen: my preference is still consistency. Doesn't matter on the location. That way people can just drop the config for a location into the policy config directory. Everything gets treated the same
(11:11:42 AM) jjohansen: yes that means we wither have to special case /etc/apparmor.d/ and /var/lib/apparmor/
(11:12:24 AM) jjohansen: or our dir scheme for lots of profiles, needs something to hint that a subdir is profiles
(11:12:52 AM) iskunk: e.g. /etc/apparmor/profile.d/ ?
(11:13:02 AM) jjohansen: uhmmm, and by special case /etc/apparmor.d/ I mean their existing layout scheme, so the existing dirs
(11:13:28 AM) jjohansen: iskunk: part of the point is to let people define multiple locations
(11:14:26 AM) iskunk: there would presumably also be e.g. /usr/local/etc/apparmor/profile.d/, /var/lib/apparmor/profile.d/ ?
(11:14:39 AM) jjohansen: so we have a dir in /etc/apparmor/policy.d/ or maybe config.d/ kind of like the /etc/apt/source.list.d/ directory
(11:15:00 AM) jjohansen: where we drop a config file, or multipe, one for each policy location
(11:16:04 AM) jjohansen: so yes you could have /etc/apparmor/profile.d/ and /var/lib/snap/apparmor/profiles/ and /var/lib/lxd/apparmor/profiles and /etc/apparmor.d and /var/lib/apparmor/
(11:16:20 AM) jjohansen: a system may have one, it could have many
(11:17:14 AM) iskunk: allowing multiple profile dirs is good, but does this also include handling profiles in subdirs (allowing a hierarchy)?
(11:17:19 AM) jjohansen: each one can specify if apparmor manages the policy (snapd, and lxd, both expect to fully manage their policy)
(11:18:05 AM) jjohansen: iskunk: it will, its a matter of defining how we now allow subdirs
(11:18:22 AM) iskunk: got it
(11:18:51 AM) jjohansen: there is already the tradition of using subdirs for tunables, abstractions, forrce-complain
(11:19:12 AM) jjohansen: to do none policy things, and just sticking profile files in the main dir
(11:19:59 AM) jjohansen: libvirt even creates profiles in /etc/apparmor.d/libvirt for the profiles it generates
(11:20:23 AM) ***georgiag is here
(11:20:51 AM) jjohansen: so if we allow existing layout, and we really need to for backwards compatibility reasons
(11:20:59 AM) sbeattie: sorry, also a late arriver
(11:21:13 AM) jjohansen: it becomes a question of doing a completely clean break
(11:21:39 AM) jjohansen: or just having some way to indicate its a profile subdir, and maybe what its contents should be
(11:21:58 AM) jjohansen: like profiles_A, profiles_B
(11:23:10 AM) sbeattie: also, another consideration is how to handle/represent apparmor namespaces
(11:23:40 AM) jjohansen: yeah, that is indeed another wrinkle
(11:23:40 AM) sbeattie: and the policy set that may exist in each one and keeping them distinct from other namespaces.
(11:24:15 AM) jjohansen: currently there is some ability to specify namespacing within the profiles
(11:25:06 AM) jjohansen: but at the policy set level, I think it makes sense to generally keep policy for different namespaces separate
(11:25:13 AM) jjohansen: so in their own directories
(11:25:51 AM) jjohansen: then the namespace info could potentially specified as part of the policy dir config, similar to its location, cache, ...
(11:30:31 AM) iskunk: I would argue for a clean break, moving away from /etc/apparmor.d (which has become a mess) and rebuilding our world using nice subdirs under /etc/apparmor/. this would mean we can keep the former around for a while during the transition period. being able to specify profile-dir metadata sounds nice, too!
(11:31:06 AM) jjohansen: uhhh, so just so you are aware a while in apparmor terms is 10+ years
(11:31:20 AM) jjohansen: we really strive for backwards compatability
(11:32:16 AM) cboltz: in this case I'd even say 50 years ;-) - in other words: just keep /etc/apparmor.d/ supported as it is
(11:32:41 AM) cboltz: (that doesn't stop us from adding other profile dirs which use a different layout)
(11:32:58 AM) iskunk: could just read profiles under there if the dir exists, but have a default/normal install not even have that dir present
(11:33:14 AM) jjohansen: no, but it does mean at least 4 special cased locations
(11:33:35 AM) iskunk: so if someone installs an old package, things will work, but for current stuff /etc/apparmor.d won't be a thing anymore
(11:34:13 AM) cboltz: instead of hardcoding these special cased locations, what about making that another config option so that the config for /etc/apparmor.d/ has layout=traditional ?
(11:35:30 AM) jjohansen: yes, I think that would be a requirement if we are having different policy dir layouts
(11:37:42 AM) iskunk: and allow the dir to be absent without an error
(11:39:24 AM) cboltz: another requirement will be some sort of directory stacking - usecase: if we move the upstream abstractions and tunables to a new directory, profiles installed to /etc/apparmor.d/ by another package still need to be able to include them
(11:40:08 AM) jjohansen: yes, overlays are a wip on the parser side
(11:40:40 AM) iskunk: is the whole "include <abstractions/blarg>" convention remaining the same?
(11:40:55 AM) jjohansen: so you could do something like
(11:40:55 AM) jjohansen: profiles: /etc/apparmor.d/:/var/lib/apparmor/
(11:41:19 AM) jjohansen: where /etc/apparmor.d/ are locally defined profiles that overlay package installed profiles
(11:41:21 AM) cboltz: iskunk: changing it would probably cause lots of breakage, so yes IMHO
(11:41:41 AM) jjohansen: * just choosing those 2 locations as they are already existing profile locations
(11:42:17 AM) jjohansen: yes, but the conflg could split the include location out
(11:42:19 AM) iskunk: okay, but then that would imply abstractions/ is a special-case subdir of wherever the profiles go (since it doesn't contain standalone profiles)
(11:42:27 AM) jjohansen: this is already supported by the parser
(11:42:55 AM) jjohansen: iskunk maybe but not necessarily
(11:43:19 AM) iskunk: hmm, so then "include <some-profile>" (where that is an existing profile) would error out as file-not-found?
(11:44:06 AM) jjohansen: like C
(11:44:06 AM) jjohansen: - "include" is relative to current dir
(11:44:06 AM) jjohansen: - <include> is relative to the specified include dirs. the default right now is to have the include dir be the same as the profiles but it doesn't have to be
(11:44:34 AM) jjohansen: if the include location is defined wrong yes
(11:44:55 AM) iskunk: okay, I see what you mean. this is a good direction, not conflating profiles + include dirs
(11:45:15 AM) jjohansen: and includes already support overlays, in that you can specify multiple include locations and they are searched in order.
(11:46:05 AM) jjohansen: it also allows sharing the abstractions between different policy sets. snapd and lxd are both using the default includes
(11:46:11 AM) jjohansen: well snapd sort of ...
(11:46:42 AM) jjohansen: now that it is vendoring apparmor it gets interesting ...
(11:48:06 AM) cboltz: "searched in order" reminds me of another point - we probably need a way to define in which order all the profile dirs are read (if two dirs contain a profile with the same name, which one should "win"? And should the parser temporarily load the "wrong one" and replace it later, or first check all dirs?)
(11:48:09 AM) jjohansen: they are vendoring, so they don't have to worry about apparmor versions on all the different systems a snap is supposed to support, like any containerized app its sucking in all its dependencies
(11:49:02 AM) iskunk: does that mean potentially loading a different-version kernel module, too?
(11:49:24 AM) jjohansen: so within a given profile set, its config will define the overlay/dir order. Same with config, and cache (which also already supports overlays)
(11:50:25 AM) cboltz: what if two (separate) profile sets both have a "profile foo"?
(11:50:43 AM) jjohansen: iskunk: no, apparmor isn't actually a module per say. Its a module in the linux infrastructure/coding initialization. But a special module that can only be built into the kernel. All LSMs are that way.
(11:51:37 AM) iskunk: right, I just wasn't clear on how you can have e.g. apparmor 3.0 + apparmor 4.0 at the same time
(11:51:39 AM) jjohansen: cboltz: that is a harder problem. we could ignore it and tell people, you do that its inconsistent ...
(11:52:09 AM) jjohansen: or we could do something like the sysctl approach where entries have a priority in their name
(11:52:33 AM) iskunk: cboltz: there has to be an order to these, right? defining multiple sets with all being equal would leave this kind of ambiguity open
(11:53:35 AM) cboltz: ignoring the issue might be problematic if both "profile foo" have different child profiles or hats - loading the wrong one might result in running childs loosing their confinement
(11:54:42 AM) jjohansen: iskunk: so 3.0 and 4.0 userspaces at the same time are fine. They will both use what kernel module is present, to the best of their abilities and the kernel will support them to the best of its abilities. its a 3 dimensional matrix, between userspace versions, kernel version, and policy version (which can be separate from the userspace)
(11:55:32 AM) jjohansen: iskunk: defining an order is preferred, being ambiguous in security is just bad
(11:56:03 AM) jjohansen: of course the ordering just may be, you have 2 of this policy that conflict and we fail instead of load
(11:56:18 AM) jjohansen: cboltz: indeed
(11:56:40 AM) iskunk: okay, so both can coexist with a >=4.0 kernel
(11:57:14 AM) iskunk: two of a given policy could be an intentional "override" use case
(11:58:14 AM) jjohansen: iskunk: doesn't even have to be a >= 4.0 kernel. AppArmor 4.0 userspace will support a kernel back to about 2.6.21 or so, or at least try to. certainly a 3.x kernel, so we are talking 2.10 userspace or even older
(11:58:32 AM) cboltz: with a defined order, no need to fail - "last one wins" is a sane solution (optionally with a warning printed)
(11:58:42 AM) jjohansen: but it might break on policy, and not all policy rules will be enforced on old kernels
(11:58:57 AM) jjohansen: warning, or error
(11:59:27 AM) jjohansen: /etc/sysctl.d/
(11:59:27 AM) jjohansen: does ##-name.conf
(11:59:29 AM) iskunk: jjohansen: yeah, the problem I ran into was 4.0 profiles not loading into a 3.x kernel module, because of the <abi/4.0> and "userns"
(12:00:11 PM) jjohansen: eg.
(12:00:11 PM) jjohansen: 10-ptrace.conf
(12:00:11 PM) jjohansen: 99-sysctl.conf
(12:00:27 PM) jjohansen: its a simple priority scheme but it works
(12:00:52 PM) jjohansen: iskunk: bug/issue?
(12:00:52 PM) iskunk: are numbers needed here, however? I thought the ordering was on directories, not files
(12:01:09 PM) cboltz: warning makes more sense - similar to how we warn about *.rpmnew etc.
(12:01:41 PM) iskunk: is that a bug? I figured the old parser just can't handle newer syntax
(12:01:41 PM) jjohansen: iskunk: maybe it depends on which dir or files you are talking about.
(12:02:04 PM) ***mbelair_ is here
(12:02:17 PM) jjohansen: The proposal allows for multiple policy sets. /etc/apparmor.d/ /var/lib/snap.d/apparmor/profiles/ /etc/apparmor/profiles/
(12:02:36 PM) jjohansen: a location can be defined to support subdirectories of profiles
(12:02:38 PM) iskunk: jjohansen: I thought the "two sets with the same profile foo" issue meant "two directories with the same profile"
(12:03:12 PM) iskunk: or is this about two differently-named profiles matching the same executable?
(12:03:42 PM) jjohansen: so there is two potential points of conflict on profiles. Between policy sets defining the same profile, and within a policy set different dirs redefining a profile
(12:04:23 PM) jjohansen: the first case I think should be prioritized
(12:05:01 PM) jjohansen: the second case is an error withing the policy set, and should ideally be caught and at the very least warned about
(12:05:06 PM) iskunk: or even just different files in the same dir. the concordance between profile file name, profile symbolic name, and matched executable path(s) is only by convention
(12:05:24 PM) jjohansen: yes
(12:06:05 PM) jjohansen: one of the things I want to do in general is get all the profiles compiling under a single profile run
(12:06:56 PM) jjohansen: well all the profiles within a policy set
(12:07:20 PM) iskunk: yeah, a disagreement within the same set seems more like a bug than a normal use case... assuming that a given profile set is typically coming from a single author, rather than pastiche-d together
(12:07:49 PM) jjohansen: policy dev, etc could still do partial/incremental but in general doing a whole set at a time, means being able to catch more errors and do more optimizations
(12:08:39 PM) jjohansen: well, maybe. there are actually quite a few packages that want to be in charge of their policy
(12:09:22 PM) jjohansen: which is another reason overlays would be good, those applications could install to a different location than system/reference policy
(12:09:24 PM) cboltz: disagreement within the same set can also be intentional - think /usr vs /etc, where the profile in /etc is expected to "win"
(12:09:37 PM) jjohansen: and overlays determine what policy is used
(12:10:19 PM) jjohansen: cboltz: yes, but that is a different overlay layer so that shouldn't conflict
(12:10:36 PM) jjohansen: you only choose 1 in the end
(12:10:39 PM) cboltz: right
(12:11:11 PM) jjohansen: compiling the whole profile set as one lets you detect multiple after the overlay, missing profiles, ...
(12:12:31 PM) jjohansen: okay, how about I take the task to try and write a first draft up, and post it to the ml, so we can dicker over it at the next meeting
(12:12:51 PM) cboltz: good idea :-)
(12:13:18 PM) jjohansen: next topic is rule ordering, brought up by this little discussion https://gitlab.com/apparmor/apparmor/-/merge_requests/1311#note_2070028065
(12:13:27 PM) iskunk: would be good to have something concrete to noodle over
(12:14:09 PM) jjohansen: summary: file rules support both permission first, and permission last layout in rules.
(12:14:19 PM) jjohansen: every other rule type is permission first
(12:14:32 PM) jjohansen: file rules have historically been permission last
(12:15:15 PM) jjohansen: ages ago, their was a discussion around permission ordering and it was decided to go with permission first for all new rule types
(12:15:30 PM) iskunk: if I were doing this from scratch, I would put the short bit first, and the variable-length bit last. works for "du", "ls -l", etc.
(12:15:59 PM) jjohansen: that is essentially the permissions first
(12:16:13 PM) jjohansen: rw /file/one,
(12:16:39 PM) jjohansen: for compatability reasons, the file class is also optional on files
(12:16:46 PM) jjohansen: where it isn't for other classes
(12:17:01 PM) iskunk: yup. while I like lining up the permission bits in a nice straight column, sometimes that requires adding a *lot* of spaces... that's not a great property of the format
(12:18:04 PM) jjohansen: true
(12:18:07 PM) cboltz: file rules typically are > 80% of the rules in a profile, therefore not needing the "file" keyword in every line is a nice feature IMHO (even if that feature only exists for historical reasons ;-)
(12:18:36 PM) jjohansen: same with allow being the default and optional
(12:18:46 PM) iskunk: would probably help to have some kind of block syntax for this... e.g. "file rw { path path path path }"
(12:18:48 PM) cboltz: I'd prefer not to have the "file" keyword used in every file rule we ship
(12:19:09 PM) jjohansen: it makes writing policy easier, but can mess with alignment and can make policy a little more confusing for newbies
(12:19:16 PM) cboltz: iskunk: we have that, just replace the spaces inside { } with commas ;-)
(12:19:31 PM) jjohansen: hahaha
(12:19:47 PM) iskunk: but you still have to repeat the permission bits, and possibly the "file" keyword...
(12:19:59 PM) jjohansen: so yes, there is some block formating and more will come eg.
(12:20:52 PM) jjohansen: it is possible (at the parser level, tooling is different) today to specify
(12:20:52 PM) jjohansen: deny { ... }
(12:20:52 PM) jjohansen: audit { ....}
(12:21:25 PM) iskunk: great! that sidesteps a lot of the problem, then
(12:21:30 PM) Talkless left the room (quit: Remote host closed the connection).
(12:22:59 PM) cboltz: unfortunately it comes with the problem that the aa-* tools will explode when you use this block syntax (supporting it is somewhere[tm] on the TODO list)
(12:23:11 PM) jjohansen: right
(12:23:27 PM) jjohansen: the goal is to close the gap
(12:24:47 PM) jjohansen: the question is do we work towards switching file rules in the reference (ie project supplied policy) to be by default the same as the other rules, permissions first
(12:24:50 PM) georgiag: I don't have a firm preference but I can appreciate the consistency of having for example: ptrace rw peer=foo, and file rw /foo,
(12:24:57 PM) georgiag: so, class + perms + conditionals
(12:25:23 PM) jjohansen: if we do, the other format will remain supported
(12:26:36 PM) cboltz: keep in mind that lots of people are used to /foo r, so r /foo, will look strange to them, and file r /foo, even more
(12:26:48 PM) cboltz: therefore I'd prefer to keep the current way
(12:27:37 PM) cboltz: (even if it's somewhat inconsistent with how other rule types look)
(12:27:43 PM) sbeattie: I think we should have some overriding goals that affect both directory layout + rules formatting: we want to support flexibility for our users to admin their policy how they like *but* we should have some preferred formats and nudges that push people towards better practices, for however you define better practices.
(12:28:39 PM) cboltz: aa-logprof --file-perms-first ? ;-)
(12:28:58 PM) iskunk: yeah, the path of least resistance is always the status quo... some discomfort is going to be unavoidable if we want to get to a better place
(12:31:23 PM) cboltz: so the question is: is there a really good reason to cause/take this discomfort?
(12:32:02 PM) cboltz: personally, I wouldn't call r /foo, "better" ;-)
(12:32:58 PM) iskunk: that's more window dressing than anything. I'd like to have a clear path forward with e.g. integrating the AppArmor project's work, for starters.
(12:33:14 PM) ***sbeattie has to drop for another meeting
(12:36:24 PM) georgiag: I wouldnt call it better either, just (with the file keyword) more consistent. but you're right, the status quo is the least resistant path. we should probably write some documentation with the expected rule/profile layout
(12:38:46 PM) jjohansen: so consistency I think becomes more important once we start throwing in more permissions and conditionals
(12:40:48 PM) jjohansen: eg.
(12:40:48 PM) jjohansen: file rules already allow controlling of link locations
(12:40:48 PM) jjohansen: /foo/bar l,
(12:41:09 PM) jjohansen: is actually
(12:41:09 PM) jjohansen: link subsert /foo/bar -> /**,
(12:41:23 PM) jjohansen: that is not possible with the trailing perm
(12:41:56 PM) jjohansen: its entirely possible to do similar restrictions around move and copy
(12:44:01 PM) jjohansen: with blocks (again yes not supported in the utils atm)
(12:44:28 PM) jjohansen: having perms first could allow factoring those out to the start of a block rule
(12:44:56 PM) jjohansen: file rw { /foo, /bar, }
(12:45:09 PM) jjohansen: yes, you can do something similar with alternations
(12:45:27 PM) jjohansen: but then it isn't consistent with doing it to other rule type
(12:45:39 PM) jjohansen: dbus rw { .. }
(12:46:20 PM) jjohansen: are we going to go to the above level of allowing to factor out the permissions? maybe I am not sure
(12:46:28 PM) georgiag: ahh good point
(12:46:55 PM) iskunk: factoring out permissions gets points for being DRY-friendly
(12:49:00 PM) cboltz: yes, but I'm not sure if it's really worth the effort and added complexity
(12:49:31 PM) cboltz: in worst case (with big blocks), you have to scroll up to find out the actual permissions ;-)
(12:52:21 PM) georgiag: that is something we can specify in the policy-writing-guidelines we will document (I'm not volunteering :) )
(12:54:16 PM) jjohansen: hahaha
(12:55:45 PM) jjohansen: to me perms first has some clear small advantages, weighed against the historical weight that people are use to the old format
(12:56:45 PM) jjohansen: today's meeting has already gone almost 2 hours and I need to prep for another meeting, so I move we table the issue and think about it some more.
(12:56:59 PM) jjohansen: cboltz: tooling wise, what would need to be done?
(12:57:24 PM) cboltz: quick summary:
(12:57:50 PM) cboltz: - when parsing profiles, count { and } - and don't always interprete } as "profile end"
(12:58:52 PM) cboltz: - profile storage needs support for blocks - easiest way to implement that would be a "blocks" sub-item in profile_storage that can contain multiple profile_storage
(12:59:10 PM) cboltz: - when writing a profile, write all the blocks
(01:00:11 PM) cboltz: sounds quite easy, but in practise it willl probably cause some fun...
(01:00:39 PM) jjohansen: when doesn't "easy" cause fun
(01:00:49 PM) jjohansen: alright is there anything else to discuss today
(01:01:29 PM) cboltz: the good thing is that parse_profile_data() is much shorter than it was before we had all the *Rule classes :-)
(01:01:46 PM) jjohansen: hahaha, indeed
(01:02:06 PM) cboltz: so even after adding support for handling blocks the function will stay readable
(01:03:14 PM) jjohansen: oh I am sure we could make it unreadable ;)
(01:03:43 PM) cboltz: ;-)
(01:06:08 PM) cboltz: ideally we should also remove the last traces of profile, hat in the internal storage so that the tools can support nested child profiles
(01:06:25 PM) cboltz: sounds different than block support, but actually is not that different
(01:06:54 PM) jjohansen: alright, meeting adjourned
Comments
Please register or sign in to add a comment.