-
Notifications
You must be signed in to change notification settings - Fork 582
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
i/p/requestrules,o/i/apparmorprompting: move request matching logic into requestrules #14635
Open
olivercalder
wants to merge
3
commits into
canonical:master
Choose a base branch
from
olivercalder:prompting-move-request-matching-logic-down
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
i/p/requestrules,o/i/apparmorprompting: move request matching logic into requestrules #14635
olivercalder
wants to merge
3
commits into
canonical:master
from
olivercalder:prompting-move-request-matching-logic-down
Commits on Nov 7, 2024
-
many: support mixed outcomes for permissions in prompting constraints
Let rule content constraints have heterogeneous outcomes and lifespans for different permissions in the constraints. As such, convert the list of permissions to a map from permission to permission entry, where the entry holds the outcome, lifespan, and duration/expiration for that particular permission, where previous those were global to the containing rule, rule contents, or patch contents. However, the existing concept of replying "allow"/"deny" to a particular set of requested permisisons is clear and simple. We want to keep outcome, lifespan, and duration as reply-scoped values, not permission-specific, when accepting prompt replies. So we need different types of constraints for prompt replies vs. rule contents. The motivation behind this is so that we can have only a single rule for any given path pattern. We may have a situation where the user previously replied with "allow read `/path/to/foo`" and they're now prompted for write access, they need to be able to respond with "deny read `/path/to/foo`". If we only support a single outcome for any given rule, then we'd need two rules for the same path `/path/to/foo`. Thus, we need rules to support different outcomes for different permissions. The same logic applies for lifetimes and expirations, though this adds additional complexity now that the concept of rule expiration is shifted to being permission-specific. We care about expired rules in two primary places: when loading rules from disk, we want to discard any expired rules, and when adding a new rule, we want to discard any expired permisison entry for a rule which shares a pattern variant with the new rule. For cases where that expired permission entry had a conflicting outcome, we clearly can't have that, and we want to remove the expired permission entry from its containing rule as well, so as to avoid confusion for the user without them needing to check expiration timestamps. Even if the outcome of the expired entry matches that of the new rule's entry for the same permission, we still want to prune the expired permission from the old rule to avoid confusion. The complexity is around when a notice is recorded for a rule for which some permissions have expired. At the moment, the logic is that a notice is recorded in these cases: - when a rule is loaded from disk - data may be `"removed": "expired"` if all permissions are expired - when a rule is added - when a rule is patched - when a rule is removed (with data `"removed": "removed"`) - when a rule is found to be expired when attempting to add a new rule Notably, a notice is not recorded automatically when a permission entry expires. Nor is a notice recorded when a permission is found to be expired, so long as its associated rule still has at least one non-expired permission. Neither pruning an expired permission entry from the rule tree nor from the entry's containing rule results in a notice, even though technically the rule data has changed, since the expired permission has been erased. The rationale is that the semantics of the rule have not changed, since the expiration of that permission was part of the semantics of the rule. Since durations are used when adding/patching a rule and expirations are used when retrieving a rule, in addition to the differences for prompt replies vs. rule contents, we now need several different variants of constraints: - `promptConstraints`: - path, requested permissions list, available permissions list - internal to `requestprompts`, unchanged - `ReplyConstraints`: - path pattern, list of permissions - containing `PromptReply` holds outcome/lifespan/expiration - unchanged from before, though under a new name - converted to a `Constraints` if reply warrants a new rule - `Constraints`: - path pattern, map from permission to outcome, lifespan, duration - used when adding rule to the rule DB - converted to `RuleConstraints` when the new rule is created - `RuleConstraints`: - path pattern, map from permisison to outcome, lifespan, expiration - used when retrieving rules from the rule DB - never used when POSTing to the API - `PatchConstraints`: - identical to `Constraints`, but with omitempty fields - converted to `RuleConstraints` when the patched rule is created To support this, we define some new types, including `{,Rule}PermissionMap` and `{,Rule}PermissionEntry`. The latter of these is used in the leaves of the rule DB tree in place of the previous set of rule IDs of rules whose patterns render to a given pattern variant. Whenever possible, logic surrounding constraints, permissions, and expiration is pushed down to methods on these new types, thus simplifiying the logic of their callers. Signed-off-by: Oliver Calder <[email protected]>
Configuration menu - View commit details
-
Copy full SHA for 0b880ea - Browse repository at this point
Copy the full SHA 0b880eaView commit details -
i/prompting: expire entry on (not after) timestamp and improve unit t…
…ests Signed-off-by: Oliver Calder <[email protected]>
Configuration menu - View commit details
-
Copy full SHA for b2d5acc - Browse repository at this point
Copy the full SHA b2d5accView commit details
Commits on Nov 8, 2024
-
i/p/requestrules,o/i/apparmorprompting: move request matching logic i…
…nto requestrules Signed-off-by: Oliver Calder <[email protected]>
Configuration menu - View commit details
-
Copy full SHA for c00fe34 - Browse repository at this point
Copy the full SHA c00fe34View commit details
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.