1 IRC_meeting_2024 08 13

John Johansen edited this page 2024-08-13 19:56:03 +00:00

(11:01:02 AM) jjohansen: georgiag, cboltz, sarnold, mbelair: meeting time
(11:01:41 AM) ***cboltz is here
(11:01:46 AM) jjohansen: oops soory missed you rlee287
(11:01:50 AM) jjohansen: hey cboltz
(11:02:34 AM) rlee287: Hello
(11:03:27 AM) jjohansen: so we only have policy layout on the agenda today
(11:04:39 AM) jjohansen: do we want to discuss it with so many people out? (ie. sbeattie, sarnold, georgia, mbelair, ...)
(11:05:46 AM) cboltz: we probably can't make a final decision, but looking at it can't hurt ;-)
(11:06:14 AM) jjohansen: okay
(11:06:29 AM) jjohansen: so policy layout is a pita
(11:08:51 AM) jjohansen: what I would like to do is go with a layout config dir. Where we can drop a file for each location that policy is at with additional meta data, like whether its managed by apparmor (so remove unknown etc won't strip lxd, snapd, etc profiles), and possibly so compiler flags
(11:09:06 AM) jjohansen: maybe even the abstraction and include locations
(11:09:57 AM) carnil: hello jjohansen :)
(11:10:23 AM) carnil: (ah sorry, missed that is meeting time apologies)
(11:10:23 AM) jjohansen: there could also be a master config, which provides the default that the individual policy files can override
(11:10:56 AM) jjohansen: hey carnil welcome, so I have thursday and friday off. Guess what is at the top of the todo list
(11:11:23 AM) carnil: jjohansen: sorry I did not want ot disturb the meeting, I will ask later
(11:12:19 AM) jjohansen: carnil: no, its all good. like I said my goal is to get the stable update submitted thursday
(11:12:55 AM) jjohansen: maybe even a tiny bit sooner
(11:13:09 AM) jjohansen: cboltz: what do you want to see changed in the layout
(11:13:33 AM) carnil: jjohansen: thanks! you will have a happy downstream then (for context: https://bugs.debian.org/1050256)
(11:13:44 AM) cboltz: splitting autogenerated profiles to a separate directory sounds like a good idea
(11:14:27 AM) cboltz: maybe we could also have a separate directory for upstream profiles - at the risk of users no longer finding these profiles in /etc/apparmor.d/
(11:14:51 AM) jjohansen: carnil: yes sorry, I am sort of aware I haven't managed to follow or read the messages (I am so behind on messages) but I am should be starting to catch up now
(11:15:03 AM) cboltz: I guess an important detail we'll need is a priority setting - basically /etc/apparmor.d/ should override everything
(11:15:27 AM) jjohansen: well no it shouldn't override everything
(11:15:59 AM) jjohansen: I forgot to mention, that I want the policy overlay support as part of this
(11:16:28 AM) cboltz: what does that mean if two directories have a profile with the same profile name?
(11:16:29 AM) jjohansen: so a location can be set as an overlay with a higher priority
(11:17:16 AM) jjohansen: you can have multiple overlay directories, I assume like overlayfs, we treat the top as the writable location
(11:17:57 AM) cboltz: sounds similar to the handling of multiple cache directories we have now?
(11:18:05 AM) jjohansen: but its possible we could us metadata to say the top writable location, and distinguish between the readable and writable
(11:18:10 AM) jjohansen: yes
(11:18:42 AM) cboltz: ok, sounds good :-)
(11:18:56 AM) jjohansen: configs should be able to specify a cache location, and overlays should share the cache location
(11:19:15 AM) jjohansen: my intention is to not make the parser smart about this
(11:20:02 AM) jjohansen: it will have the base functionality, so pass in overlay locations it will do the work, and compile and load based on the overlay
(11:20:43 AM) jjohansen: but parsing the config, and setup, is the init/unit or a new tool that the init unit calls
(11:22:52 AM) jjohansen: cboltz: I do kind of assume that genprof/logprof will have to parse the config dir
(11:23:16 AM) cboltz: most boring solution on the parsing/loading side would be apparmor@whatever.service - with the little disadvantage that it makes it hard to configure priorities (besides the obvious Before=apparmor.service for /etc/apparmor.d/ )
(11:23:31 AM) cboltz: yes, this will make things interesting[tm] for the tools
(11:24:33 AM) cboltz: doesn't sound impossible, but depending on how much separation we need, it can be quite easy or quite hard (separate abstractions are a candidate towards "hard")
(11:24:34 AM) sbeattie [~steve@173.17.41.51] entered the room.
(11:24:44 AM) jjohansen: hrmmm, I am not sure the boring solution is the best, but I haven't really explored what we want to do implementation wise
(11:24:57 AM) ***georgiag is here, sorry for the tardiness 
(11:25:09 AM) jjohansen: I want to land the base parser support first
(11:25:28 AM) jjohansen: I propose /etc/apparmor/config.d/
(11:25:37 AM) jjohansen: as the config dir location
(11:27:03 AM) jjohansen: cboltz: I will start a wiki page, to use as a spec. please feel free to contribute
(11:27:25 AM) iskunk: shouldn't that be .../conf.d/, since the individual files end in .conf?
(11:28:01 AM) cboltz: if it's only meant for profile locations, the name sounds a bit too generic - quick (and probably not perfect) idea: /etc/apparmor/profiledirs.d/
(11:28:29 AM) iskunk: oh, this is for profiles, not something like parser.conf?
(11:28:43 AM) cboltz: (optionally also parser.conf.d, logprof.conf.d and severity.db.d to customize these files)
(11:29:34 AM) jjohansen: empty doc https://gitlab.com/apparmor/apparmor/-/wikis/apparmor-5-config-layout-spec
(11:29:41 AM) jjohansen: iskunk: possibly
(11:31:15 AM) jjohansen: cboltz: hrmmm, its for the config information about policy, location, who manages it, which abstractions to use, if its an overlay, etc. The actually policy will be in a different directory/directories
(11:31:40 AM) jjohansen: which could very well /etc/apparmor.d/  /var/lib/apparmor/  etc
(11:32:35 AM) cboltz: right, I get that - which is why I didn't propose "profiles.d"
(11:34:48 AM) jjohansen: iskunk: so parser.conf etc will remain as a global config, the configs within the .d/ will be overrides if there are any needed for a specific policy location
(11:35:23 AM) iskunk: ah, understood, so it will be easier for packages to override stuff just by dropping files in there
(11:35:36 AM) jjohansen: so you could have a different compiler flags for different locations if you need, but as long as you don't the setting in the global location will affect everything without the override
(11:35:42 AM) jjohansen: yes
(11:36:47 AM) jjohansen: and for policy to have overlays so a local version of a profile can override a packaged shipped version easily without messing up the packaging system
(11:37:34 AM) jjohansen: ideally, the policy language will pick up some extensions to allow sharing from a package specified profile
(11:38:12 AM) jjohansen: something like
(11:38:12 AM) jjohansen:    inherit X from /file,
(11:38:24 AM) jjohansen: (syntax made up on the spot)
(11:38:56 AM) iskunk: but this is meant to be more advanced than just adding stuff to files under local/ ?
(11:39:08 AM) jjohansen: yes
(11:39:46 AM) iskunk: e.g. you can say, "I want these rules, but not these other rules?"
(11:39:49 AM) jjohansen: iskunk: it is partly to deal with current config issues, partly to make policy a lot more flexible
(11:40:45 AM) cboltz: a real-world example: replacing an ix rule with Px won't work with a local include
(11:41:04 AM) jjohansen: iskunk: yes, I am actively working on being able to use boolean operators  AND, OR, and MINUS in policy so you can do things like
(11:41:04 AM) jjohansen: { rule 1, rule 2, rule 3, } - { rule 4, rule 5, }
(11:41:50 AM) jjohansen: there is also priority coming to the syntax. Current rules are priority 0, so you can specify rules with higher priority
(11:42:08 AM) sbeattie left the room (quit: Ping timeout: 480 seconds).
(11:42:21 AM) jjohansen: https://gitlab.com/apparmor/apparmor/-/merge_requests/1261
(11:42:22 AM) cboltz: I guess priority would solve the "change exec mode" problem
(11:42:39 AM) iskunk: interesting... sounds pretty powerful, though raises the complexity of writing profiles by a bit
(11:42:58 AM) jjohansen: yes, and I intend to make file, and implied ix being a default priority of -1
(11:43:46 AM) cboltz: now I wonder how many very special cases will remain - basically: are they worth the effort for another "special handling"?
(11:43:47 AM) jjohansen: yes it can make profiles much more complex if used poorly, but also it can simplify them if used well
(11:44:04 AM) jjohansen: cboltz: ?
(11:44:53 AM) cboltz: well, we have the priority draft, you mentioned AND, OR and MINUS, you came up with "inherit X from /file" - and I wonder if we really need all of them
(11:45:26 AM) jjohansen: no and yes
(11:46:18 AM) jjohansen: priority will fix a lot of unfixable problems atm, but it doesn't make doing so necessarily easy
(11:47:02 AM) jjohansen: expressing some patterns like /** except blah  is still ridiculous
(11:47:03 AM) jjohansen: ie.
(11:48:21 AM) jjohansen: /[^b]**
(11:48:21 AM) jjohansen: /b[^l]**
(11:48:21 AM) jjohansen: /bl[^a]**
(11:48:21 AM) jjohansen: /bla[^h]**
(11:49:13 AM) jjohansen: except/minus is the one I expect to be used the most, internally there are uses for AND, and OR during the compile
(11:49:56 AM) jjohansen: it will allow us to precompile individual units and combine them together, later
(11:50:15 AM) jjohansen: so eventually should lead to some compilation time improvements
(11:50:50 AM) jjohansen: whether we end up exposing AND and OR in policy beyond how they are currently exposed (I am not sure)
(11:51:42 AM) jjohansen: ie. each rule is effectively ORed with the other rules in a block
(11:52:11 AM) jjohansen: so we already have a form of OR
(11:52:37 AM) jjohansen: but internally, the way things are setup, I can't pre-compile the includes
(11:52:55 AM) iskunk: so does this include symbolic blocks? e.g. define a set of rules, with a name attached to that set, then do operations between multiple sets?
(11:53:06 AM) jjohansen: and OR that precompiled unit to the block of rules that they are included into
(11:54:13 AM) jjohansen: iskunk: the policy syntax for how this is exposed is not finalized but, we want to support it at both the rule expr level and the block level
(11:54:46 AM) cboltz: the idea sounds good in theory, but I'm not too sure about the real-world gain - even with > 1000 profiles, I'd say it's "fast enough" ;-)
(11:54:52 AM) iskunk: sounds good, it would be nice to have the existing (commented) rule groups in profiles be reflected in actual parsing/semantic structure
(11:54:59 AM) jjohansen: so
(11:54:59 AM) jjohansen:   rw /** except blah,   # note except was proposed as preferred over using the - symbol in a previous discussion of this
(11:55:57 AM) cboltz: also, if you pre-compile something like abstractions/base, keep in mind that it depends on the content of variables
(11:56:54 AM) jjohansen: cboltz: yes, I get that. Its going to take some tuning to figure out what is worth precompiling. Some things likely won't be worth it
(11:57:52 AM) cboltz: I'm quite sure I'll find more than "some things" that will make precompiling hard ;-)
(11:58:05 AM) jjohansen: some larger blocks will. Ideally we will be not compiling a single profile at a time but the whole directory. And pieces that are shared can add up
(11:58:51 AM) cboltz: do you have a rough estimate how much compile time this could save?
(11:58:57 AM) jjohansen: iskunk: can you explain what you want a little more
(12:00:11 PM) iskunk: The idea is just to have a set of rules (e.g. "access to files in app config") grouped together in a named block (that is not a subprofile), rather than only being able to deal with the individual rules
(12:01:32 PM) iskunk: so a derived profile could e.g. disable that block in one go
(12:04:51 PM) iskunk: it sounded for a bit like you were going in this direction, with the mention of "units". I assume that was referring to files
(12:05:54 PM) jjohansen: cboltz: so I have messed with this only a little. In degenerate cases it can be more than 1/N where N is the number of profiles and C is some fudge factor constant. Realistically in real world use cases I expect it will be maybe 20-25% if we get it tuned right. It really depends on what is shared and how
(12:05:54 PM) jjohansen: dfa builds are interesting, in that we have a potentially exponential state explosion during the build phase. Followed by a O(n logn) minimization phase, followed by the compression phase which is ~N^2
(12:05:54 PM) jjohansen: precompiling reduces the state explosion chance, and even when you hit it you reduce the number of states going into the minimization phased
(12:05:54 PM) jjohansen: so given to blocks of rules A + B, the build and minimization of them as one unit can be exponentially more than  doing A, and then B and then combining them
(12:06:40 PM) jjohansen: the trick is finding the sets of rules where this is happening, and that it is worth doing it on
(12:07:42 PM) jjohansen: overall the good thing is precompiling parts shouldn't generally result in a slowdown, even if it isn't a performance win
(12:09:19 PM) jjohansen: iskunk: yes, we do want to be able to do that with blocks, how that is expressed in syntax still needs to be finalized
(12:09:40 PM) iskunk: sounds great
(12:09:50 PM) jjohansen: eg. using include syntax
(12:09:50 PM) jjohansen:   { include <abstractions/blah } except { rule A, }
(12:10:51 PM) cboltz: ok, now let's apply that to real-world numbers: I have ~1600 profiles (thanks to the apparmor.d project), and after deleting the cache it takes 31 seconds to recompile and load them all on my 8 years old laptop
(12:10:53 PM) jjohansen: but its entirely possible we will end up with a way to name blocks for rules, if we can ever finish bikeshedding about it
(12:10:55 PM) jjohansen: :)
(12:11:16 PM) cboltz: a 25% improvement would bring that down to 23 seconds - which is of course nice, but I'm still not sure if it's worth it
(12:11:27 PM) cboltz: besides that - how many people have 1600 profiles? ;-)
(12:12:18 PM) cboltz: I'm afraid people who have 50 or 100 profiles probably won't notice the difference
(12:12:24 PM) jjohansen: cboltz: maybe not for you. But I (well Canonical) has customers who have compiles that are take multiple minutes and 25% would be huge to them
(12:12:49 PM) iskunk: is the intention still to import all of AppArmor.d eventually, and package them up? (even if not installed by default?)
(12:13:00 PM) cboltz: ok, multiple minutes indeed sound interesting[tm]
(12:13:20 PM) jjohansen: cboltz: yes, the average user likely won't except Canonical is looking to be shipping something like the 16000 profile set by default in a couple of years
(12:13:53 PM) iskunk: I'm still confused on what the plans/relationship is with that project
(12:13:58 PM) jjohansen: the cache does a lot, but recompiles happen
(12:14:10 PM) rlee287: Do we have benchmarking infrastructure for the parser in order to measure precise compile times for typical profiles?
(12:14:34 PM) rlee287: I think that would be useful data to have if we don't have that already
(12:16:20 PM) jjohansen: iskunk: so apparmor is an open source project, with different communities contributing, when I am doing work for Canonical its based on Canonical priorities. I also try to contribute some on personal time, around more upstream needs
(12:17:47 PM) iskunk: I understand that, what I mean is, do we intend to import all those AppArmor.d profiles and ship them. e.g. I've been told some of the profiles I've submitted in MRs conflict with theirs, and my implicit question is, "why do we care about that?"
(12:17:50 PM) jjohansen: but with that being said, Canonical does want to make sure its contributions are useful to the upstream project, its just not always the same priorities I would personally choose to work on if I was choosing what I thought was best for apparmor
(12:19:07 PM) jjohansen: policy side is harder, I think the upstream project should carry a reference policy that is easy to adapt to different distros
(12:19:18 PM) iskunk: I think they're doing good work, but if we want our profiles not to conflict with theirs, there should probably be some statement of intention to bring their work into our fold
(12:19:19 PM) cboltz: personally I care because I have the apparmor.d profiles installed, and will have to deal with any conflicts ;-)
(12:20:09 PM) jjohansen: something I don't think we have today, and on the upstream side I am more than willing to carry distro specific bits if they can easily made conditional, updated variables etc
(12:20:39 PM) cboltz: besides that, I'm a fan of avoiding duplicate work
(12:20:45 PM) jjohansen: iskunk: ah got it.
(12:20:54 PM) iskunk: right, and I understand that, it's just that it has to be formalized a bit for apparmor proper (the project, as opposed to its users) for that to be something that can be brought up in review
(12:21:33 PM) jjohansen: iskunk: I can try to get a more formal statement, but for now, yes Canonical wants its policy to work with the upstream project
(12:22:12 PM) acidsys: I think it's also not clear as a contributor wanting to make an official profile whether one should submit it to apparmor upstream or apparmor.d
(12:22:22 PM) jjohansen: one of the priorities is to improve, things to make that possible. There is work to improve the conditionals and variables to make them more useful
(12:22:26 PM) iskunk: Canonical's policy is fine. I'm worried about AppArmor.d, and the potential for duplication/conflicts
(12:23:12 PM) iskunk: acidsys: that's a good point too
(12:23:59 PM) jjohansen: well from talking with them, they would like to get at least some of it upstream. There are improvements that are needed to support that
(12:24:20 PM) jjohansen: but whether apparmor.d wants to get everything upstream I am not sure
(12:24:53 PM) jjohansen: I certainly intend to try working with them, to get stuff upstream
(12:24:59 PM) iskunk: especially helpful will be syntax improvements that allow some degree of merging. many AppArmor.d profiles are written pretty paranoically, making the security > robustness tradeoff. It would be nice if they can just override the parts of the "base" profile that they want to tighten up (or replace altogether)
(12:25:30 PM) jjohansen: right, so speaking more long term here
(12:25:58 PM) jjohansen: we want better conditionals, variables, and some form of parameterized macro/include
(12:26:19 PM) jjohansen: rule composition (boolean operators previously mentioned)
(12:26:34 PM) iskunk: yep, so if they don't get all their stuff into our repos, there are still ways to minimize conflicts and play well together
(12:26:53 PM) jjohansen: some way to indicate inheritance or shared block. Like giving a child all the rules of the parent with a single rule
(12:27:27 PM) jjohansen: yes
(12:28:07 PM) iskunk: it would be good to see their stuff packaged up into a(n optional) package that users can install, obviously without breaking the existing profiles
(12:29:16 PM) acidsys: it already exists on openSUSE (though only in cboltz' home project)
(12:29:56 PM) jjohansen: yeah, it would be good to see that be done more generically
(12:30:54 PM) jjohansen: sadly I don't have the time atm, but if anyone wants to volunteer ...
(12:31:49 PM) iskunk: we need to work out the policy layout thing first, however. even just allowing for a directory hierarchy... putting 1600+ files in a single directory is just not user/admin-friendly
(12:31:56 PM) cboltz: I have a feeling that some profiles need a few additions before I want to submit them to an official repo ;-)
(12:32:38 PM) jjohansen: iskunk: indeed, suggestions on layout are welcome
(12:33:13 PM) jjohansen: I assume the profiles dir, is going to turn into a directory hierarchy
(12:33:17 PM) cboltz: as long as these 1600 profiles are "hidden" somewhere in /usr (and only modified profiles end up in /etc), I don't think that's a real problem
(12:33:27 PM) iskunk: we should be able to copy in the AppArmor.d git tree as-is to an appropriate location and have it work. no flattening
(12:34:19 PM) iskunk: (maybe a subtree of it, but still, no flattening)
(12:34:20 PM) cboltz: "as is" might not work - AFAIK the apparmor.d install magic does a bit of magic on some profiles (for example distro-specific rules)
(12:34:34 PM) cboltz: but I get your point
(12:34:51 PM) iskunk: okay, that bit I'm not familiar with, but that sounds like a job for a tunable
(12:35:21 PM) jjohansen: hrmmm, so unless I am remembering wrong, that is going to take some tooling work. I don't think genprof/logprof nor the parser support automatic traversal into subdirs 
(12:35:37 PM) iskunk: maybe conditional blocks too? e.g. parse this block of rules only if this variable/expression is true
(12:35:55 PM) cboltz: that, and traditionally subdirs mean "include file, _not_ a standalone profile"
(12:36:07 PM) jjohansen: iskunk: yes, that is planned as part of the conditional improvements
(12:36:11 PM) cboltz: so blindly loading files from subdirs is not an option
(12:36:14 PM) iskunk: yeah, the single-directory assumption is baked into a lot of tooling
(12:37:13 PM) jjohansen: it wouldn't be hard to add subdir directory traversal with an exclusion list of known current dirs (configurable) that need to be skipped
(12:37:30 PM) jjohansen: or some such
(12:37:55 PM) iskunk: not ideal, but probably necessary given the kitchen-sink nature of /etc/apparmor.d/
(12:38:22 PM) cboltz: ... and then someone comes up with an   $application.d   subdir with profile sniplets for the $application profile
(12:39:01 PM) cboltz: so an exclusion list will explode pretty quickly (at least in /etc/apparmor.d/)
(12:39:27 PM) jjohansen: cboltz: do you have another solution?
(12:39:34 PM) iskunk: just handle *.d specially? not much different than e.g. *.dpkg-old
(12:39:35 PM) acidsys: /etc/apparmor/{profiles,snippets}.d
(12:39:53 PM) cboltz: I wouldn't add subdir support for /etc/apparmor.d
(12:40:33 PM) cboltz: for "new" locations, we could for example allow wildcards in the path configuration
(12:40:52 PM) jjohansen: and by that we need to add /var/lib/apparmor and some of the other locations profiles have lived
(12:41:23 PM) jjohansen: cboltz: the plan isn't to have a new location per say but to allow for them
(12:42:00 PM) cboltz: I get that, but I still think that existing locations shouldn't change their behaviour
(12:42:10 PM) jjohansen: and I would really rather be consistent with how we treat locations
(12:42:40 PM) Talkless left the room (quit: Remote host closed the connection).
(12:43:20 PM) jjohansen: how about we think about it, and come back to this next month
(12:43:27 PM) iskunk: fwiw, moving away from /etc/apparmor.d to /etc/apparmor wouldn't be bad. the ".d" in the former has lost any semblance of its traditional meaning
(12:43:53 PM) cboltz: jjohansen: good idea, we won't solve everything today ;-)
(12:44:19 PM) jjohansen: okay does anyone have anything else they want to bring up
(12:44:21 PM) cboltz: iskunk: the problem is that we already have /etc/apparmor which contains some config files
(12:44:42 PM) iskunk: right, but you could do e.g. /etc/apparmor/profiles (or profiles.d) without issue
(12:45:05 PM) iskunk: there's way less baggage
(12:45:16 PM) iskunk: jjohansen: one more thing
(12:45:25 PM) cboltz: indeed, but that sounds like a "change just for a change", which is annoying for users IMHO
(12:45:43 PM) iskunk: what are the prospects for moving in the profiles from the apparmor-profiles repo?
(12:46:02 PM) iskunk: cboltz: well, this is to get around the issue of not changing behavior in apparmor.d ...
(12:46:29 PM) jjohansen: iskunk: so, yes it is going to happen. Just not yet
(12:46:44 PM) iskunk: okay, good enough for me :)
(12:46:57 PM) jjohansen: sorry, I want to get the 4.1 release cut first
(12:47:19 PM) iskunk: understood, the MR is lower-priority... just want to make sure it'll happen
(12:47:21 PM) jjohansen: and with that. I plan on cuttng the 4.1-beta1 tonight or tomorrow morning
(12:47:56 PM) cboltz: ok, then you better don't wait for a working lastlog2 support in aa-notify ;-)
(12:48:35 PM) jjohansen: cboltz: I am willing to land that as an FFe if you get it working before release
(12:48:47 PM) cboltz: I'll probably rewrite it to read the database directly - that's not the official way, but should avoid issues with parsing the dates and times
(12:49:12 PM) cboltz: which is not really hard, I just need some time to do it
(12:49:20 PM) jjohansen: maybe I should take back the bit about landing it as an FFe ... :-)
(12:50:03 PM) acidsys: sorry, what's a FF= (I can think of FireFox, Feature Flag and Fast & Furious)
(12:50:14 PM) jjohansen: Feature Freeze exception
(12:50:20 PM) acidsys: ah, thanks!
(12:51:05 PM) jjohansen: anything else to discuss today?
(12:51:21 PM) cboltz: nothing from me
(12:51:27 PM) iskunk: all good here
(12:55:27 PM) jjohansen: all right next meeting is scheduled for Tuesday Sept 10, 18:00 UTC
(12:55:35 PM) jjohansen: meeting adjourned, thanks everyone