apparmor

mirror of https://gitlab.com/apparmor/apparmor synced 2025-08-22 01:57:43 +00:00
(11:00:40 AM) jjohansen: georgiag, mbelair, rlee287, sbeattie, sarnold, cboltz, iskunk, roddhjav: meeting time
(11:00:53 AM) ***georgiag is here
(11:01:05 AM) rlee287: hello
(11:01:21 AM) ***cboltz hides
(11:02:06 AM) roddhjav: hello
(11:04:15 AM) jjohansen: well that will have to do, lets get started
(11:05:20 AM) jjohansen: First topic 5.0-alpha: didn't happen, we have just been too busy. Going to try and make it happen this week. We need it to support upstream af_unix in apparmor-next
(11:05:49 AM) jjohansen: policy layout: again I am not ready so will punt to next meeting
(11:06:27 AM) jjohansen: policy rule order consistency: I propose we punt to next meeting as well.
(11:06:54 AM) jjohansen: rlee287: Review of CONTRIBUTING.md
(11:07:08 AM) rlee287: Punt: haven't had a chance to think about it at all since last time
(11:07:14 AM) jjohansen: ack
(11:07:49 AM) jjohansen: next up: file extension. I was really hoping we would have a few more people for the vote
(11:08:23 AM) jjohansen: we can
(11:08:23 AM) jjohansen: 1. vote anyways
(11:08:23 AM) jjohansen: 2. punt to next meeting
(11:08:23 AM) jjohansen: 3. take it to the ml for feedback and a vote
(11:09:19 AM) rlee287: Modified 3: solicit commentary on the mailing list first and decide how we want to vote depending on what feedback we get there
(11:11:41 AM) roddhjav: Modified 3 sound good to me.
(11:11:56 AM) georgiag: I like both 3s too
(11:12:11 AM) jjohansen: okay, I will send an email to the list today
(11:12:44 AM) roddhjav: Personally, I still don't have an opinion of the best extension...
(11:15:01 AM) jjohansen: sure, I expect most people don't as long as there is one, and it doesn't collide with other extensions. But its worth getting feedback so we don't have to revert what ever we decide, because we missed something
(11:16:53 AM) jjohansen: next up: proposal for re extension. I propose we punt it down the road. For a couple of reasons
(11:16:53 AM) jjohansen: 1. The code isn't ready to expose this yet, so it isn't a critical decision
(11:16:53 AM) jjohansen: 2. I am not ready to discuss, so will lean on 1 as an excuse :)
(11:17:11 AM) rlee287: Am fine with punting on that
(11:17:58 AM) jjohansen: it is something I would like to see us get to an experimental level for the 5.0 release, but we have so many other things to land ...
(11:18:25 AM) jjohansen: next up policy split and reorg
(11:19:09 AM) jjohansen: so this is a big one, it partly depends on some of the stuff (ie. dir reorg) that we skipped
(11:19:54 AM) jjohansen: with that said, I want to
(11:19:54 AM) jjohansen: 1. get at least an experiemental repo up this month
(11:19:54 AM) jjohansen: 2. have it as part of the 5.0 release
(11:20:31 AM) jjohansen: As discussed last time the goal of the split is to separate policy and code updates after release
(11:21:11 AM) jjohansen: with that in mind the policy repo and code repo are planned to be dependent, and at a minimum sync at major release points
(11:22:49 AM) jjohansen: * we still need to figure out how we want to make the repos depend on each other
(11:22:49 AM) jjohansen:   - could be just manual
(11:22:49 AM) jjohansen:   - could be CI requires it be checked out and locations specified
(11:22:49 AM) jjohansen:   - potentially could be something like a git sub project
(11:23:42 AM) jjohansen: * part of the goal here is to integrate as much of the apparmor.d project (hey roddhjav) as possible
(11:23:56 AM) cboltz: speaking of dependencies - the profiles tests use parser and utils, and IIRC at least some of the utils tests also use the profiles
(11:24:30 AM) jjohansen: - for the 5.0 release, we are probably not going to get everything, and it is expected apparmor.d at least in the short term will live on carrying a diff.
(11:24:42 AM) rlee287: From what I remember the utils tests try to parse the profile set to check that the Python rules parsing works
(11:25:21 AM) jjohansen: - roddhjav will be have maintainer rights on on the new profile repo, as well as continuing to mainatin apparmor.d
(11:25:38 AM) cboltz: right (and IIRC there are a few other tests that also use the profiles and abstractions)
(11:26:37 AM) jjohansen: cboltz: correct on the tests. We need to figure out how best to allow those dependencies to exist with the split. It is expected that those tests will continue to use the profiles in the repo
(11:26:56 AM) rlee287: One idea that came to mind was to have the CI for the profiles repo check out a specific tag of the main apparmor repo, build the tooling, and then go from there
(11:27:00 AM) jjohansen: rlee287: correct
(11:27:21 AM) roddhjav: I had a tough about the links, so yes, we need direct links between the parser and the profiles. We may have to handle it in a few way:
(11:27:21 AM) roddhjav: - Using release, for everything that has been released
(11:27:21 AM) roddhjav: - During dev, sub project is the obvious solution. However, it is a pain, therefore using something like go mod version tracking (or similar) could do it too
(11:27:21 AM) eittaebs_: with the proposed split, are we looking to move abstractions and tunables as well?
(11:27:33 AM) roddhjav: eittaebs_yes
(11:28:41 AM) jjohansen: yeah I think that moving abstractions and tunables is probably a hard requirement
(11:29:25 AM) cboltz: rlee287: the dependency is in both ways, so besides profiles CI needing parser and utils, the CI doing utils tests needs a checkout of the profiles
(11:29:34 AM) eittaebs_: rlee287: I think you'd want both: CI for the profiles repo to check out the core project and ensure that it parses the existing state of the profiles, and also that the core project checks out the policy repo to ensure that changes there haven't broken parsing/enforcement of existing profiles.
(11:30:08 AM) rlee287: Right: didn't mention the other direction because I'm not sure which commit of the profiles repo we'd want to check out for that
(11:30:20 AM) rlee287: Just the latest master/main? Or a tag
(11:30:23 AM) rlee287: Or something else
(11:30:25 AM) jjohansen: I don't think we want to use a tag
(11:30:33 AM) jjohansen: I think we want to use a branch
(11:30:47 AM) jjohansen: that corresponds to the release in common/Version
(11:31:08 AM) jjohansen: or a new var set there
(11:31:28 AM) jjohansen: that way each major branch, can find its corresponding branch and do the check
(11:31:31 AM) eittaebs_: we'd potentially need something if we ever deprecate a language feature (prefix/postfix perms comes to mind) and but still have older supported versions.
(11:31:58 AM) eittaebs_: or actually the other way round; added language features would break older lts branches.
(11:32:07 AM) eittaebs_: -ENOBRAIN today
(11:32:28 AM) jjohansen: right, again branches based on major versions should mostly cover that
(11:32:43 AM) eittaebs_: some of that could be predicated on ABI versioning.
(11:32:46 AM) jjohansen: as long as such breaking features are only done in major versions
(11:33:28 AM) cboltz: if you do such a breaking change in a minor version, you have a bug - at least in the version number ;-)
(11:33:28 AM) jjohansen: yes, abi/features could be brought in
(11:34:41 AM) eittaebs_: but yeah, you would need branches, because as the primary policy development branch moves along, you might want (and downstreams might prefer) bug fixes to policy backported.
(11:35:05 AM) eittaebs_: s/you would need/we probably want/
(11:36:01 AM) eittaebs_: which points to coming up with a broader release policy plan for the profiles repo.
(11:36:39 AM) jjohansen: right
(11:37:40 AM) roddhjav: Regarding the tests:... (full message at <https://matrix.org/oftc/media/v1/media/download/AWRfprNI2dUR04Qy7mckdaK6VLwgwiwz56g72EZDW_9lVUM2o-chkjoPjeW7UqUp97Hc40OcTVitns6d01CTTzZCeYM21WkQAG1hdHJpeC5vcmcvQlZEYmdUbk1OV3RLT0ZsVkdYRE9Hd0do>)
       Regarding the tests:
       - Profiles checks need to move. apparmor.d already include some profiles checker, they could get integrated.
       - Profiles needed in the parser' units tests of the parser: they could probably be copied in apparmor (ie not the profiles project) such that they won't change a lot.
       - Integration tests of "ensuring apparmor still works with the profiles" could be done by the profile project (such as we don't need a link between the project in both direction).

(11:39:45 AM) jjohansen: so most of the parser tests will indeed be static and remain part of the code project
(11:39:56 AM) jjohansen: eg. simple_tests/
(11:40:46 AM) jjohansen: basically anything that is a feature parse test etc, anything that is not real policy that will be loaded and used on a non-dev system
(11:42:20 AM) cboltz: I see why you consider a (static) copy of the profiles so that parser and utils tests don't need the profiles repo - but OTOH, I'd argue that testing with the latest profiles is a feature
(11:42:51 AM) eittaebs_: Yeah, for sure. But real world profiles have a way of shaking out edge case bugs that can then be minimized and incorporated into the static profile tests.
(11:43:40 AM) eittaebs_: It's probably a failure on the core project that we haven't had compiling the apparmor.d repo as part of the CI, frankly.
(11:44:42 AM) cboltz: I remember some of the tests helped to uncover a few issues in profile changes (like "why does this abstraction suddenly get proposed for $file?) - and I'd really like to keep getting such "surprise failures"
(11:45:02 AM) roddhjav: Regarding release policy. I think we should be able to release fix twice a week (ie: as often as needed). While new feature (in this context: new profiles/abstraction/tunables, profile rewrite) could have a less frequent update. (Very) fast fix release will be needed, this could lead to full system breakage otherwise.
(11:45:05 AM) cboltz: and yes, testing against apparmor.d sounds like another good idea ;-)
(11:45:07 AM) rlee287: The concern I have is making sure that, when a profile fails to parse during testing, we can easily isolate whether it was a tooling change regression or a profile regression
(11:45:50 AM) rlee287: roddhjav: I see why you'd like that but such rolling profile releases would be hard to cleanly bring back into distro packaging
(11:46:53 AM) rlee287: On the Ubuntu side: with every new profile regression bug that comes in I have to also triage which releases it affects
(11:46:53 AM) cboltz: nobody forces distros to update their package every week ;-)
(11:47:38 AM) rlee287: cboltz: would that mean then that e.g. debian selects the latest profiles release that occurs before the relevant freeze and then does backports onto that release
(11:47:39 AM) cboltz: OTOH, I often have to add upstream patches to the package - having more releases would avoid that
(11:48:13 AM) cboltz: rlee287: that would indeed be an option
(11:48:43 AM) cboltz: another option would be to decide on a branch, and regularly take the latest release from that branch
(11:49:05 AM) cboltz: depends on how strict the policy on backporting vs. taking new minor releases is
(11:49:16 AM) jjohansen: so, I don't think there is anything wrong with having a large set of static profiles in the code repo, but having CI against the profile repo really is a hard requirement
(11:50:16 AM) jjohansen: part of the reason for the split is to make it easier for a distro to decide what to take
(11:50:29 AM) jjohansen: code and policy updates are cleanly separated
(11:50:40 AM) roddhjav: rlee287Well, there are two kind of `fix`:... (full message at <https://matrix.org/oftc/media/v1/media/download/AXjjo8IQHRfCTiguHDDLtxgdiE6tJlYEKzJlCb5WPD3jRSvpZ4NM57UruevPqNK9GiYHUfd21mxToS3m8fEHatNCeYM3k-cwAG1hdHJpeC5vcmcvdGFyRUVhekZsV2lLaGdXbU9JRWhuaGVv>)
       rlee287Well, there are two kind of `fix`: 
       - First the real fix: issue in the profile that would break any stable distro. They would need to update
       - Then the false fix: update the profile to ensure the new feature X from a given program works. This one is usually less of an issue for stable release and they kind of break rolling distro first (but it is not as simple as this: some update require/allow full profile rewrite).
       To us, it can be hard to make differences, hence why I said "fix"

(11:51:30 AM) cboltz: jjohansen: if "static profiles" means what we currently have in profiles/, I don't see why we should add/keep a copy of bitrotting profiles - just always test against the latest profiles
(11:51:33 AM) rlee287: What I had in mind more was programs changing e.g. paths over time
(11:52:09 AM) rlee287: For example one of the patches I was re-examining again today related to KDE plasmashell changing the location of a child binary and having to update the profile accordingly
(11:52:16 AM) rlee287: Having version conditionals would help, though
(11:52:18 AM) eittaebs_: cboltz: he meant static things like the parser/simple_tests/ 
(11:53:50 AM) cboltz: ok, those testcases will indeed stay where they are
(11:54:59 AM) jjohansen: cboltz: static profiles that test certain, features/combination of features are fine. Static profiles that are a snapshot in time that we want to ensure we can parse, and handle are fine. Basically there needs to be a reason for the static profile to exist as a test. We are testing X with this. Which could be even be we are testing a known snap shot with a known abi (note we aren't currently doing this).
(11:54:59 AM) jjohansen: But we also definitely need to be checking against the repo profiles
(11:56:19 AM) cboltz: ok, sounds good :-)
(11:57:57 AM) roddhjav: I would also propose that we need to strictly enforce some rules regarding the profiles we merge in the new repo, I could try to propose a draft of this later (but most is already in apparmor.d docs). The goal is to:... (full message at <https://matrix.org/oftc/media/v1/media/download/AQrOzRXen-uOrIsUzuuoHhlzdMhsfC3812vW1uWEf-ugjJTHMN1CqH_gk_oW7dcyFb6asLwWaPpXjiwLzavKDp5CeYM3_oqAAG1hdHJpeC5vcmcvZHVFQU5tR2VnQ2VBam1yTUlRUHF6V1hM>)
       I would also propose that we need to strictly enforce some rules regarding the profiles we merge in the new repo, I could try to propose a draft of this later (but most is already in apparmor.d docs). The goal is to:
       1. Ensure all profiles have unit tests
       2. Ensure some general guideline are respected (try maintaining thousand of profile that don't look the same at all...)
       3. Ensure some basic linter & security check
       4. Ensure they use common methods (ex: always use an abstraction when it is proposed, use subprofile for X, never transition to profile Y...)

(11:58:40 AM) rlee287: For unit tests: that would be a nice goal but there are profiles that interact with hardware that would be very hard to test in a container/VM
(11:59:03 AM) rlee287: Not saying we shouldn't, but saying that good testing wpa_supplicant or bluetoothctl would be difficult
(11:59:09 AM) rlee287: *good testing for e.g.
(11:59:13 AM) roddhjav: rlee287I agree, it won't be possible for all
(11:59:40 AM) roddhjav: We also need... to define a security model the profile are expected to implement
(12:02:15 PM) rlee287: Do you have something more precise in mind than "allow normal usage and block all else"?
(12:02:28 PM) rlee287: The big issue we've run into before is with stuff like command-line specified paths
(12:06:10 PM) roddhjav: Well, for most profiles the security basis is separation of privs to limit the consequences of attacks, data leak... so "block all else". Some usage could do with different model, such as RBAC, anonimity (not alone obviously)
(12:08:23 PM) Talkless left the room (quit: Quit: Konversation terminated!).
(12:10:49 PM) jjohansen: so, yes all good goals
(12:11:06 PM) jjohansen: but I also think we aren't going to get that all met in a first pass
(12:11:09 PM) roddhjav: The question of generic cli specified paths that can change a lot.
(12:11:09 PM) roddhjav: 1. As we mentioned in some PR, we need to define strong/weak confinement to not allow transition from strong to weak.
(12:11:09 PM) roddhjav: 2. When the question is more: allow this program to open this resource (url, video, path, a pager, sudo, mknod...) this is usually well-defined in apparmor.d: For example, you have multiple child-open-* profile that allow you to precise what kind of resource a program can open. The general solution is to provide general methods for all this.
(12:11:34 PM) jjohansen: For the first pass, I expect to turn on complain mode for all the new profiles being integrated
(12:11:58 PM) jjohansen: And I want to get where the program and data requirements are easy to separate
(12:12:38 PM) jjohansen: As long term, I would like to see delegation dealing with the data/rbac side of things
(12:13:55 PM) roddhjav: jjohansenDon't hesitate to ping me when you are setting up the new project.
(12:14:02 PM) jjohansen: defining strong/weak confinement is just hard. And there are cases where we want to explicitly allow that
(12:14:12 PM) jjohansen: roddhjav: ack
(12:14:46 PM) jjohansen: Its a good goal to define/document expected confinement
(12:15:01 PM) jjohansen: long term we need tooling support
(12:15:06 PM) roddhjav: strong/weak: we simply need to keep track of the weak one and to detect Px in CI
(12:15:26 PM) jjohansen: this is a wip, so that we will be able to have the tooling spit out where privs are being increased
(12:15:58 PM) roddhjav: (and we need a better name than weak, they are actually very useful)
(12:16:26 PM) eittaebs_: (project ideas for anyone looking for Something To Do: semantic policy diff tool, useful for backporting policy fixes; policy overlap walker/viewer, useful for determining execution transitions and data movement between policies)
(12:17:45 PM) cboltz: eittaebs_: diff tool shouldn't be too hard to implement, just base it on the code of aa-mergeprof (and display the diff instead of asking one-by-one)
(12:21:20 PM) cboltz: (it will become more interesting if you also want to check against possible changes in abstractions in $branch, but that sounds like a luxory problem ;-)
(12:22:11 PM) jjohansen: right, hence semantic diff, which is an ugly problem
(12:22:15 PM) jjohansen: :)
(12:23:31 PM) cboltz: well, I see two options:
(12:23:57 PM) jjohansen: okay, so resolution here is, lets try to get some docs, together and a test repo. We won't move the profiles out of the main repo yet, and we can discuss things more next meeting
(12:24:00 PM) cboltz: a) have an easy-to-implement, good-enough solution (basically based on aa-mergeprof)
(12:24:10 PM) cboltz: b) have a perfect solution, and hope someone writes it ;-)
(12:24:33 PM) cboltz: I'd start with a)  ;-)
(12:24:38 PM) rlee287: jjohansen: sounds good
(12:24:42 PM) jjohansen: why can't I have a and b and some cake too ;-)
(12:25:11 PM) cboltz: you can, you just have to write - and bake - it ;-)
(12:25:14 PM) rlee287: Whenever I design stuff I design around reaching a) first but then being able to go from a) to b) without having to re-architect everything :p
(12:26:20 PM) jjohansen: alright that is everything on the agenda, does anyone have anything else that needs to be raised?
(12:26:50 PM) cboltz: any news about the 4.1.x release notes? ;-)
(12:27:16 PM) cboltz: (actually I'm surprised that the package was accepted in Tumbleweed while they only say "TODO")
(12:28:45 PM) jjohansen: well they are still waiting on anyone to have time to do them
(12:29:23 PM) jjohansen: sadly I am very backed-up atm, and don't see getting to them this week, and probably not next either
(12:30:31 PM) cboltz: the package got accepted, so - no hurry ;-)
(12:33:22 PM) cboltz: next topic: the libapparmor license issue (a few files with GPL instead of LGPL) that was raised
(12:33:38 PM) cboltz: I don't have an issue with changing the affected files to LGPL
(12:33:51 PM) cboltz: but IIRC there are plans to move more parts of the parser into a library
(12:34:26 PM) cboltz: so it might be a good/better idea to create a separate GPL library for those parts
(12:35:04 PM) cboltz: (just as a quick input on this topic - no need for a big discussion now ;-)
(12:37:28 PM) cboltz: another good reason for a separate library would be that programs that only need things like change_hat don't need to load a "big" library with lots of stuff they don't need
(12:39:19 PM) cboltz: and yet another topic - is someone bored enough to review https://gitlab.com/apparmor/apparmor/-/merge_requests/1728 ? (touches lots of utils tests, therefore I'd like to merge it before conflicting changes come in)
(12:40:00 PM) jjohansen: so,yes I have been planning a libaaparser or such
(12:40:45 PM) jjohansen: this particular set of code, however would be best if it could be in the current lib as it is used in loading policy
(12:41:01 PM) georgiag: cboltz: I'll try to review it later today
(12:41:11 PM) roddhjav: That would be nice to have, even more if the libaaparser could share the parser AST....
(12:41:17 PM) jjohansen: and the current lib ideally is an interface layer to the kernel interfaces, and policy load
(12:42:30 PM) jjohansen: roddhjav: that is the goal, the parser gets split into a frontend, and backend libs. Frontend is the AST and parse, backend is the state machine
(12:42:49 PM) jjohansen: the parser becomes just a wrapper/driver around the libs
(12:42:55 PM) roddhjav: :+1:
(12:49:30 PM) jjohansen: alright next meeting is scheduled for Aug 12, 11:00 UTC.
(12:49:30 PM) jjohansen: feel free to update the agenda items, or send an email so we can update them
(12:49:30 PM) jjohansen: meeting adjourned.