From 6c9cc3fad1979ebc4bc7dce95a19262937ccb79c Mon Sep 17 00:00:00 2001 From: Oliver Calder Date: Mon, 16 Dec 2024 17:30:14 -0600 Subject: [PATCH 1/9] many: rename prompt remaining permissions to outstanding permissions Signed-off-by: Oliver Calder --- .../prompting/requestprompts/export_test.go | 10 +-- .../requestprompts/requestprompts.go | 73 ++++++++++--------- .../requestprompts/requestprompts_test.go | 26 +++---- .../ifacestate/apparmorprompting/prompting.go | 12 +-- .../apparmorprompting/prompting_test.go | 8 +- 5 files changed, 65 insertions(+), 64 deletions(-) diff --git a/interfaces/prompting/requestprompts/export_test.go b/interfaces/prompting/requestprompts/export_test.go index 2cc1dc1232d..f89f3c79df4 100644 --- a/interfaces/prompting/requestprompts/export_test.go +++ b/interfaces/prompting/requestprompts/export_test.go @@ -33,12 +33,12 @@ const ( MaxOutstandingPromptsPerUser = maxOutstandingPromptsPerUser ) -func NewPrompt(id prompting.IDType, timestamp time.Time, snap string, iface string, path string, remainingPermissions []string, availablePermissions []string, originalPermissions []string) *Prompt { +func NewPrompt(id prompting.IDType, timestamp time.Time, snap string, iface string, path string, outstandingPermissions []string, availablePermissions []string, originalPermissions []string) *Prompt { constraints := &promptConstraints{ - path: path, - remainingPermissions: remainingPermissions, - availablePermissions: availablePermissions, - originalPermissions: originalPermissions, + path: path, + outstandingPermissions: outstandingPermissions, + availablePermissions: availablePermissions, + originalPermissions: originalPermissions, } return &Prompt{ ID: id, diff --git a/interfaces/prompting/requestprompts/requestprompts.go b/interfaces/prompting/requestprompts/requestprompts.go index 281c3df16ae..2350052c660 100644 --- a/interfaces/prompting/requestprompts/requestprompts.go +++ b/interfaces/prompting/requestprompts/requestprompts.go @@ -86,7 +86,7 @@ type jsonPromptConstraints struct { func (p *Prompt) MarshalJSON() ([]byte, error) { constraints := &jsonPromptConstraints{ Path: p.Constraints.path, - RequestedPermissions: p.Constraints.remainingPermissions, + RequestedPermissions: p.Constraints.outstandingPermissions, AvailablePermissions: p.Constraints.availablePermissions, } toMarshal := &jsonPrompt{ @@ -109,10 +109,10 @@ func (p *Prompt) sendReply(outcome prompting.OutcomeType) error { // If outcome is allow, then reply by allowing all originally-requested // permissions. If outcome is deny, only allow permissions which were // originally requested but have since been allowed by rules, and deny any - // remaining permissions. + // outstanding permissions. var deniedPermissions []string if !allow { - deniedPermissions = p.Constraints.remainingPermissions + deniedPermissions = p.Constraints.outstandingPermissions } allowedPermission := p.Constraints.buildResponse(p.Interface, deniedPermissions) return p.sendReplyWithPermission(allowedPermission) @@ -135,7 +135,7 @@ var sendReply = (*listener.Request).Reply // promptConstraints store the path which was requested, along with three // lists of permissions: the original permissions associated with the request, -// the remaining unsatisfied permissions (as rules may satisfy some of the +// the outstanding unsatisfied permissions (as rules may satisfy some of the // permissions from a prompt before the prompt is fully resolved), and the // available permissions for the interface associated with the prompt, so that // the client may reply with a broader set of permissions than was originally @@ -143,9 +143,9 @@ var sendReply = (*listener.Request).Reply type promptConstraints struct { // path is the path to which the application is requesting access. path string - // remainingPermissions are the remaining unsatisfied permissions for which - // the application is requesting access. - remainingPermissions []string + // outstandingPermissions are the outstanding unsatisfied permissions for + // which the application is requesting access. + outstandingPermissions []string // availablePermissions are the permissions which are supported by the // interface associated with the prompt to which the constraints apply. availablePermissions []string @@ -176,7 +176,7 @@ func (pc *promptConstraints) equals(other *promptConstraints) bool { return true } -// applyRuleConstraints modifies the prompt constraints, removing any remaining +// applyRuleConstraints modifies the prompt constraints, removing any outstanding // permissions which are matched by the given rule constraints. // // Returns whether the prompt constraints were affected by the rule constraints, @@ -186,7 +186,7 @@ func (pc *promptConstraints) equals(other *promptConstraints) bool { // other return values can be ignored. // // If the path pattern does not match the prompt path, or the permissions in -// the rule constraints do not include any of the remaining prompt permissions, +// the rule constraints do not include any of the outstanding prompt permissions, // then affectedByRule is false, and no changes are made to the prompt // constraints. func (pc *promptConstraints) applyRuleConstraints(constraints *prompting.RuleConstraints) (affectedByRule, respond bool, deniedPermissions []string, err error) { @@ -202,13 +202,13 @@ func (pc *promptConstraints) applyRuleConstraints(constraints *prompting.RuleCon // Path pattern matched, now check if any permissions match - newRemainingPermissions := make([]string, 0, len(pc.remainingPermissions)) - for _, perm := range pc.remainingPermissions { + newOutstandingPermissions := make([]string, 0, len(pc.outstandingPermissions)) + for _, perm := range pc.outstandingPermissions { entry, exists := constraints.Permissions[perm] if !exists { // Permission not covered by rule constraints, so permission - // should continue to be in remainingPermissions. - newRemainingPermissions = append(newRemainingPermissions, perm) + // should continue to be in outstandingPermissions. + newOutstandingPermissions = append(newOutstandingPermissions, perm) continue } affectedByRule = true @@ -227,9 +227,9 @@ func (pc *promptConstraints) applyRuleConstraints(constraints *prompting.RuleCon return false, false, nil, nil } - pc.remainingPermissions = newRemainingPermissions + pc.outstandingPermissions = newOutstandingPermissions - if len(pc.remainingPermissions) == 0 || len(deniedPermissions) > 0 { + if len(pc.outstandingPermissions) == 0 || len(deniedPermissions) > 0 { // All permissions allowed or at least one permission denied, so tell // the caller to send a response back to the kernel. respond = true @@ -269,10 +269,10 @@ func (pc *promptConstraints) Path() string { return pc.path } -// Permissions returns the remaining unsatisfied permissions associated with -// the prompt. -func (pc *promptConstraints) RemainingPermissions() []string { - return pc.remainingPermissions +// OutstandingPermissions returns the outstanding unsatisfied permissions +// associated with the prompt. +func (pc *promptConstraints) OutstandingPermissions() []string { + return pc.outstandingPermissions } // userPromptDB maps prompt IDs to prompts for a single user. @@ -435,7 +435,7 @@ var timeAfterFunc = func(d time.Duration, f func()) timeutil.Timer { // // The caller must ensure that the given permissions are in the order in which // they appear in the available permissions list for the given interface. -func (pdb *PromptDB) AddOrMerge(metadata *prompting.Metadata, path string, requestedPermissions []string, remainingPermissions []string, listenerReq *listener.Request) (*Prompt, bool, error) { +func (pdb *PromptDB) AddOrMerge(metadata *prompting.Metadata, path string, requestedPermissions []string, outstandingPermissions []string, listenerReq *listener.Request) (*Prompt, bool, error) { availablePermissions, err := prompting.AvailablePermissions(metadata.Interface) if err != nil { // Error should be impossible, since caller has already validated that @@ -464,10 +464,10 @@ func (pdb *PromptDB) AddOrMerge(metadata *prompting.Metadata, path string, reque } constraints := &promptConstraints{ - path: path, - remainingPermissions: remainingPermissions, - availablePermissions: availablePermissions, - originalPermissions: requestedPermissions, + path: path, + outstandingPermissions: outstandingPermissions, + availablePermissions: availablePermissions, + originalPermissions: requestedPermissions, } // Search for an identical existing prompt, merge if found @@ -487,7 +487,7 @@ func (pdb *PromptDB) AddOrMerge(metadata *prompting.Metadata, path string, reque if len(userEntry.prompts) >= maxOutstandingPromptsPerUser { logger.Noticef("WARNING: too many outstanding prompts for user %d; auto-denying new one", metadata.User) // Deny all permissions which are not already allowed by existing rules - allowedPermission := constraints.buildResponse(metadata.Interface, constraints.remainingPermissions) + allowedPermission := constraints.buildResponse(metadata.Interface, constraints.outstandingPermissions) sendReply(listenerReq, allowedPermission) return nil, false, prompting_errors.ErrTooManyPrompts } @@ -594,14 +594,14 @@ func (pdb *PromptDB) Reply(user uint32, id prompting.IDType, outcome prompting.O // contents and, if so, sends back a decision to their listener requests. // // A prompt is satisfied by the given rule contents if the user, snap, -// interface, and path of the prompt match those of the rule, and all remaining -// permissions are covered by permissions in the rule constraints or at least -// one of the remaining permissions is covered by a permission which has an -// outcome of "deny". +// interface, and path of the prompt match those of the rule, and all +// outstanding permissions are covered by permissions in the rule constraints +// or at least one of the outstanding permissions is covered by a rule +// permission which has an outcome of "deny". // // Records a notice for any prompt which was satisfied, or which had some of // its permissions satisfied by the rule contents. In the future, only the -// remaining unsatisfied permissions of a partially-satisfied prompt must be +// outstanding unsatisfied permissions of a partially-satisfied prompt must be // satisfied for the prompt as a whole to be satisfied. // // Returns the IDs of any prompts which were fully satisfied by the given rule @@ -649,17 +649,18 @@ func (pdb *PromptDB) HandleNewRule(metadata *prompting.Metadata, constraints *pr // back to the kernel, and record a notice that the prompt was satisfied. if len(deniedPermissions) > 0 { // At least one permission was denied by new rule, and we want to - // send a response immediately, so include any remaining + // send a response immediately, so include any outstanding // permissions as denied as well. // // This could be done as part of applyRuleConstraints instead, but // it seems semantically clearer to only return the permissions // which were explicitly denied by the rule, rather than all - // remaining permissions because at least one was denied. It's the - // prorogative of the caller (this function) to treat the remaining - // permissions as denied since we want to send a response without - // waiting for future rules to satisfy the remaining permissions. - deniedPermissions = append(deniedPermissions, prompt.Constraints.remainingPermissions...) + // outstanding permissions because at least one was denied. It's + // the prorogative of the caller (this function) to treat the + // outstanding permissions as denied since we want to send a + // response without waiting for future rules to satisfy the + // outstanding permissions. + deniedPermissions = append(deniedPermissions, prompt.Constraints.outstandingPermissions...) } // Build and send a response with any permissions which were allowed, // either by this new rule or by previous rules. diff --git a/interfaces/prompting/requestprompts/requestprompts_test.go b/interfaces/prompting/requestprompts/requestprompts_test.go index b14d28dba99..7f2afc44ad7 100644 --- a/interfaces/prompting/requestprompts/requestprompts_test.go +++ b/interfaces/prompting/requestprompts/requestprompts_test.go @@ -309,7 +309,7 @@ func (s *requestpromptsSuite) TestAddOrMerge(c *C) { c.Check(prompt1.Snap, Equals, metadata.Snap) c.Check(prompt1.Interface, Equals, metadata.Interface) c.Check(prompt1.Constraints.Path(), Equals, path) - c.Check(prompt1.Constraints.RemainingPermissions(), DeepEquals, permissions) + c.Check(prompt1.Constraints.OutstandingPermissions(), DeepEquals, permissions) stored, err = pdb.Prompts(metadata.User, clientActivity) c.Assert(err, IsNil) @@ -553,9 +553,9 @@ func (s *requestpromptsSuite) TestReply(c *C) { // Check that permissions in response map to prompt's permissions abstractPermissions, err := prompting.AbstractPermissionsFromAppArmorPermissions(prompt1.Interface, allowedPermission) c.Check(err, IsNil) - c.Check(abstractPermissions, DeepEquals, prompt1.Constraints.RemainingPermissions()) + c.Check(abstractPermissions, DeepEquals, prompt1.Constraints.OutstandingPermissions()) // Check that prompt's permissions map to response's permissions - expectedPerm, err := prompting.AbstractPermissionsToAppArmorPermissions(prompt1.Interface, prompt1.Constraints.RemainingPermissions()) + expectedPerm, err := prompting.AbstractPermissionsToAppArmorPermissions(prompt1.Interface, prompt1.Constraints.OutstandingPermissions()) c.Check(err, IsNil) c.Check(allowedPermission, DeepEquals, expectedPerm) } else { @@ -695,7 +695,7 @@ func (s *requestpromptsSuite) TestHandleNewRule(c *C) { c.Check(promptIDListContains(satisfied, prompt1.ID), Equals, true) c.Check(promptIDListContains(satisfied, prompt3.ID), Equals, true) - // Read permissions of prompt2 satisfied, but it has one remaining + // Read permissions of prompt2 satisfied, but it has one outstanding // permission, so notice re-issued. prompt1 satisfied because at least // one permission was denied, and prompt3 permissions fully satisfied. e1 := ¬iceInfo{promptID: prompt1.ID, data: map[string]string{"resolved": "satisfied"}} @@ -1028,9 +1028,9 @@ func (s *requestpromptsSuite) TestPromptMarshalJSON(c *C) { } path := "/home/test/foo" requestedPermissions := []string{"read", "write", "execute"} - remainingPermissions := []string{"write", "execute"} + outstandingPermissions := []string{"write", "execute"} - prompt, merged, err := pdb.AddOrMerge(metadata, path, requestedPermissions, remainingPermissions, nil) + prompt, merged, err := pdb.AddOrMerge(metadata, path, requestedPermissions, outstandingPermissions, nil) c.Assert(err, IsNil) c.Assert(merged, Equals, false) @@ -1074,7 +1074,7 @@ func (s *requestpromptsSuite) TestPromptExpiration(c *C) { } path := "/home/test/foo" requestedPermissions := []string{"read", "write", "execute"} - remainingPermissions := []string{"write", "execute"} + outstandingPermissions := []string{"write", "execute"} noticeChan := make(chan noticeInfo, 1) pdb, err := requestprompts.New(func(userID uint32, promptID prompting.IDType, data map[string]string) error { @@ -1090,7 +1090,7 @@ func (s *requestpromptsSuite) TestPromptExpiration(c *C) { // Add prompt listenerReq := &listener.Request{} - prompt, merged, err := pdb.AddOrMerge(metadata, path, requestedPermissions, remainingPermissions, listenerReq) + prompt, merged, err := pdb.AddOrMerge(metadata, path, requestedPermissions, outstandingPermissions, listenerReq) c.Assert(err, IsNil) c.Assert(merged, Equals, false) checkCurrentNotices(c, noticeChan, prompt.ID, nil) @@ -1105,7 +1105,7 @@ func (s *requestpromptsSuite) TestPromptExpiration(c *C) { // Add another prompt, check that it does not bump the activity timeout listenerReq = &listener.Request{} otherPath := "/home/test/bar" - prompt2, merged, err := pdb.AddOrMerge(metadata, otherPath, requestedPermissions, remainingPermissions, listenerReq) + prompt2, merged, err := pdb.AddOrMerge(metadata, otherPath, requestedPermissions, outstandingPermissions, listenerReq) c.Assert(err, IsNil) c.Assert(merged, Equals, false) checkCurrentNotices(c, noticeChan, prompt2.ID, nil) @@ -1120,7 +1120,7 @@ func (s *requestpromptsSuite) TestPromptExpiration(c *C) { // Add prompt again listenerReq = &listener.Request{} - prompt, merged, err = pdb.AddOrMerge(metadata, path, requestedPermissions, remainingPermissions, listenerReq) + prompt, merged, err = pdb.AddOrMerge(metadata, path, requestedPermissions, outstandingPermissions, listenerReq) c.Assert(err, IsNil) c.Assert(merged, Equals, false) checkCurrentNotices(c, noticeChan, prompt.ID, nil) @@ -1160,7 +1160,7 @@ func (s *requestpromptsSuite) TestPromptExpiration(c *C) { // Add prompt again listenerReq = &listener.Request{} - prompt, merged, err = pdb.AddOrMerge(metadata, path, requestedPermissions, remainingPermissions, listenerReq) + prompt, merged, err = pdb.AddOrMerge(metadata, path, requestedPermissions, outstandingPermissions, listenerReq) c.Assert(err, IsNil) c.Assert(merged, Equals, false) checkCurrentNotices(c, noticeChan, prompt.ID, nil) @@ -1220,7 +1220,7 @@ func (s *requestpromptsSuite) TestPromptExpirationRace(c *C) { } path := "/home/test/foo" requestedPermissions := []string{"read", "write", "execute"} - remainingPermissions := []string{"write", "execute"} + outstandingPermissions := []string{"write", "execute"} noticeChan := make(chan noticeInfo, 1) pdb, err := requestprompts.New(func(userID uint32, promptID prompting.IDType, data map[string]string) error { @@ -1236,7 +1236,7 @@ func (s *requestpromptsSuite) TestPromptExpirationRace(c *C) { // Add prompt listenerReq := &listener.Request{} - prompt, merged, err := pdb.AddOrMerge(metadata, path, requestedPermissions, remainingPermissions, listenerReq) + prompt, merged, err := pdb.AddOrMerge(metadata, path, requestedPermissions, outstandingPermissions, listenerReq) c.Assert(err, IsNil) c.Assert(merged, Equals, false) checkCurrentNotices(c, noticeChan, prompt.ID, nil) diff --git a/overlord/ifacestate/apparmorprompting/prompting.go b/overlord/ifacestate/apparmorprompting/prompting.go index 97461bbabd2..0e9813b7dcf 100644 --- a/overlord/ifacestate/apparmorprompting/prompting.go +++ b/overlord/ifacestate/apparmorprompting/prompting.go @@ -212,7 +212,7 @@ func (m *InterfacesRequestsManager) handleListenerReq(req *listener.Request) err return requestReply(req, nil) } - remainingPerms := make([]string, 0, len(permissions)) + outstandingPerms := make([]string, 0, len(permissions)) satisfiedPerms := make([]string, 0, len(permissions)) // we're done with early checks, serious business starts now, and we can @@ -234,7 +234,7 @@ func (m *InterfacesRequestsManager) handleListenerReq(req *listener.Request) err logger.Noticef("error while checking request against existing rules: %v", err) } // No matching rule found - remainingPerms = append(remainingPerms, perm) + outstandingPerms = append(outstandingPerms, perm) } } if matchedDenyRule { @@ -250,7 +250,7 @@ func (m *InterfacesRequestsManager) handleListenerReq(req *listener.Request) err return requestReply(req, allowedPermission) } - if len(remainingPerms) == 0 { + if len(outstandingPerms) == 0 { logger.Debugf("request allowed by existing rule: %+v", req) // We don't want to just send back req.Permission() here, since that @@ -273,7 +273,7 @@ func (m *InterfacesRequestsManager) handleListenerReq(req *listener.Request) err Interface: iface, } - newPrompt, merged, err := m.prompts.AddOrMerge(metadata, path, permissions, remainingPerms, req) + newPrompt, merged, err := m.prompts.AddOrMerge(metadata, path, permissions, outstandingPerms, req) if err != nil { logger.Noticef("error while checking request against prompt DB: %v", err) @@ -390,10 +390,10 @@ func (m *InterfacesRequestsManager) HandleReply(userID uint32, promptID promptin // XXX: do we want to allow only replying to a select subset of permissions, and // auto-deny the rest? - contained := constraints.ContainPermissions(prompt.Constraints.RemainingPermissions()) + contained := constraints.ContainPermissions(prompt.Constraints.OutstandingPermissions()) if !contained { return nil, &prompting_errors.RequestedPermissionsNotMatchedError{ - Requested: prompt.Constraints.RemainingPermissions(), + Requested: prompt.Constraints.OutstandingPermissions(), Replied: replyConstraints.Permissions, // equivalent to keys of constraints.Permissions } } diff --git a/overlord/ifacestate/apparmorprompting/prompting_test.go b/overlord/ifacestate/apparmorprompting/prompting_test.go index 4f4449fe000..1448d23bb03 100644 --- a/overlord/ifacestate/apparmorprompting/prompting_test.go +++ b/overlord/ifacestate/apparmorprompting/prompting_test.go @@ -590,8 +590,8 @@ func (s *apparmorpromptingSuite) TestExistingRulePartiallyAllowsNewPrompt(c *C) } _, prompt := s.simulateRequest(c, reqChan, mgr, partialReq, false) - // Check that prompt was created for remaining "write" permission - c.Check(prompt.Constraints.RemainingPermissions(), DeepEquals, []string{"write"}) + // Check that prompt was created for outstanding "write" permission + c.Check(prompt.Constraints.OutstandingPermissions(), DeepEquals, []string{"write"}) c.Assert(mgr.Stop(), IsNil) } @@ -765,7 +765,7 @@ func (s *apparmorpromptingSuite) TestNewRuleAllowExistingPrompt(c *C) { c.Check(err, NotNil) // Check that rwPrompt only has write permission left - c.Check(rwPrompt.Constraints.RemainingPermissions(), DeepEquals, []string{"write"}) + c.Check(rwPrompt.Constraints.OutstandingPermissions(), DeepEquals, []string{"write"}) // Check that two prompts still exist prompts, err := mgr.Prompts(s.defaultUser, clientActivity) @@ -1097,7 +1097,7 @@ func (s *apparmorpromptingSuite) TestRequestMerged(c *C) { } s.simulateRequest(c, reqChan, mgr, identicalReqAgain, true) - // Now new requests for just write access will have identical remaining + // Now new requests for just write access will have identical outstanding // permissions, but not identical original permissions, so should not merge readReq := &listener.Request{ Permission: notify.AA_MAY_WRITE, From 7b637697fb549ed11bc6d61fb0cd9d91c6a18cac Mon Sep 17 00:00:00 2001 From: Oliver Calder Date: Wed, 16 Oct 2024 19:19:58 -0500 Subject: [PATCH 2/9] i/p/requestrules,o/i/apparmorprompting: move request matching logic into requestrules Signed-off-by: Oliver Calder --- .../prompting/requestrules/export_test.go | 10 ++ .../prompting/requestrules/requestrules.go | 37 ++++- .../requestrules/requestrules_test.go | 146 +++++++++++++++++- .../ifacestate/apparmorprompting/prompting.go | 65 +++----- 4 files changed, 203 insertions(+), 55 deletions(-) diff --git a/interfaces/prompting/requestrules/export_test.go b/interfaces/prompting/requestrules/export_test.go index 1f27b0803ae..b70a0fafdac 100644 --- a/interfaces/prompting/requestrules/export_test.go +++ b/interfaces/prompting/requestrules/export_test.go @@ -21,6 +21,8 @@ package requestrules import ( "time" + + "github.com/snapcore/snapd/testutil" ) var JoinInternalErrors = joinInternalErrors @@ -30,3 +32,11 @@ type RulesDBJSON rulesDBJSON func (rule *Rule) Validate(currTime time.Time) (expired bool, err error) { return rule.validate(currTime) } + +func (rdb *RuleDB) IsPathPermAllowed(user uint32, snap string, iface string, path string, permission string) (bool, error) { + return rdb.isPathPermAllowed(user, snap, iface, path, permission) +} + +func MockIsPathPermAllowed(f func(rdb *RuleDB, user uint32, snap string, iface string, path string, permission string) (bool, error)) func() { + return testutil.Mock(&isPathPermAllowedByRuleDB, f) +} diff --git a/interfaces/prompting/requestrules/requestrules.go b/interfaces/prompting/requestrules/requestrules.go index 3caa1087063..1c617aac5dc 100644 --- a/interfaces/prompting/requestrules/requestrules.go +++ b/interfaces/prompting/requestrules/requestrules.go @@ -735,10 +735,43 @@ func (rdb *RuleDB) makeNewRule(user uint32, snap string, iface string, constrain return &newRule, nil } -// IsPathAllowed checks whether the given path with the given permission is +// IsRequestAllowed checks whether a request with the given parameters is +// allowed or denied by existing rules. +// +// If any of the given permissions are allowed, they are returned as +// allowedPerms. If any permissions are denied, then returns anyDenied as true. +// If any of the given permissions were not matched by an existing rule, then +// they are returned as outstandingPerms. If an error occurred, returns it. +func (rdb *RuleDB) IsRequestAllowed(user uint32, snap string, iface string, path string, permissions []string) (allowedPerms []string, anyDenied bool, outstandingPerms []string, err error) { + allowedPerms = make([]string, 0, len(permissions)) + outstandingPerms = make([]string, 0, len(permissions)) + var errs []error + for _, perm := range permissions { + allowed, err := isPathPermAllowedByRuleDB(rdb, user, snap, iface, path, perm) + switch { + case err == nil: + if allowed { + allowedPerms = append(allowedPerms, perm) + } else { + anyDenied = true + } + case errors.Is(err, prompting_errors.ErrNoMatchingRule): + outstandingPerms = append(outstandingPerms, perm) + default: + errs = append(errs, err) + } + } + return allowedPerms, anyDenied, outstandingPerms, strutil.JoinErrors(errs...) +} + +var isPathPermAllowedByRuleDB = func(rdb *RuleDB, user uint32, snap string, iface string, path string, permission string) (bool, error) { + return rdb.isPathPermAllowed(user, snap, iface, path, permission) +} + +// isPathPermAllowed checks whether the given path with the given permission is // allowed or denied by existing rules for the given user, snap, and interface. // If no rule applies, returns prompting_errors.ErrNoMatchingRule. -func (rdb *RuleDB) IsPathAllowed(user uint32, snap string, iface string, path string, permission string) (bool, error) { +func (rdb *RuleDB) isPathPermAllowed(user uint32, snap string, iface string, path string, permission string) (bool, error) { rdb.mutex.RLock() defer rdb.mutex.RUnlock() permissionMap, ok := rdb.permissionDBForUserSnapInterfacePermission(user, snap, iface, permission) diff --git a/interfaces/prompting/requestrules/requestrules_test.go b/interfaces/prompting/requestrules/requestrules_test.go index 1dd589ce74d..c04e8020607 100644 --- a/interfaces/prompting/requestrules/requestrules_test.go +++ b/interfaces/prompting/requestrules/requestrules_test.go @@ -968,7 +968,141 @@ func (s *requestrulesSuite) TestAddRulePartiallyExpired(c *C) { c.Check(exists, Equals, true) } -func (s *requestrulesSuite) TestIsPathAllowedSimple(c *C) { +type isPathPermAllowedResult struct { + allowed bool + err error +} + +func (s *requestrulesSuite) TestIsRequestAllowed(c *C) { + rdb, err := requestrules.New(nil) + c.Assert(err, IsNil) + c.Assert(rdb, NotNil) + + user := uint32(1234) + snap := "firefox" + iface := "powerful" + path := "/path/to/something" + + for _, testCase := range []struct { + requestedPerms []string + permReturns map[string]isPathPermAllowedResult + allowedPerms []string + anyDenied bool + outstandingPerms []string + errStr string + }{ + { + requestedPerms: []string{"foo", "bar", "baz"}, + permReturns: map[string]isPathPermAllowedResult{ + "foo": {true, nil}, + "bar": {true, nil}, + "baz": {true, nil}, + }, + allowedPerms: []string{"foo", "bar", "baz"}, + anyDenied: false, + outstandingPerms: []string{}, + errStr: "", + }, + { + requestedPerms: []string{"foo", "bar", "baz"}, + permReturns: map[string]isPathPermAllowedResult{ + "foo": {true, prompting_errors.ErrNoMatchingRule}, + "bar": {false, prompting_errors.ErrNoMatchingRule}, + "baz": {true, prompting_errors.ErrNoMatchingRule}, + }, + allowedPerms: []string{}, + anyDenied: false, + outstandingPerms: []string{"foo", "bar", "baz"}, + errStr: "", + }, + { + requestedPerms: []string{"foo", "bar", "baz"}, + permReturns: map[string]isPathPermAllowedResult{ + "foo": {true, prompting_errors.ErrNoMatchingRule}, + "bar": {true, nil}, + "baz": {false, prompting_errors.ErrNoMatchingRule}, + }, + allowedPerms: []string{"bar"}, + anyDenied: false, + outstandingPerms: []string{"foo", "baz"}, + errStr: "", + }, + { + requestedPerms: []string{"foo", "bar", "baz"}, + permReturns: map[string]isPathPermAllowedResult{ + "foo": {false, nil}, + "bar": {true, nil}, + "baz": {false, prompting_errors.ErrNoMatchingRule}, + }, + allowedPerms: []string{"bar"}, + anyDenied: true, + outstandingPerms: []string{"baz"}, + errStr: "", + }, + { + requestedPerms: []string{"foo", "bar"}, + permReturns: map[string]isPathPermAllowedResult{ + "foo": {true, nil}, + "bar": {true, nil}, + "baz": {true, fmt.Errorf("baz")}, + }, + allowedPerms: []string{"foo", "bar"}, + anyDenied: false, + outstandingPerms: []string{}, + errStr: "", + }, + { + requestedPerms: []string{"foo", "bar", "baz"}, + permReturns: map[string]isPathPermAllowedResult{ + "foo": {true, fmt.Errorf("foo")}, + "bar": {true, nil}, + "baz": {true, nil}, + }, + allowedPerms: []string{"bar", "baz"}, + anyDenied: false, + outstandingPerms: []string{}, + errStr: "foo", + }, + { + requestedPerms: []string{"foo", "bar", "baz", "qux", "fizz", "buzz"}, + permReturns: map[string]isPathPermAllowedResult{ + "foo": {true, fmt.Errorf("foo")}, + "bar": {true, nil}, + "baz": {true, prompting_errors.ErrNoMatchingRule}, + "qux": {false, fmt.Errorf("qux")}, + "fizz": {false, nil}, + "buzz": {false, prompting_errors.ErrNoMatchingRule}, + }, + allowedPerms: []string{"bar"}, + anyDenied: true, + outstandingPerms: []string{"baz", "buzz"}, + errStr: "foo\nqux", + }, + } { + restore := requestrules.MockIsPathPermAllowed(func(r *requestrules.RuleDB, u uint32, s string, i string, p string, perm string) (bool, error) { + c.Assert(r, Equals, rdb) + c.Assert(u, Equals, user) + c.Assert(s, Equals, snap) + c.Assert(i, Equals, iface) + c.Assert(p, Equals, path) + result := testCase.permReturns[perm] + return result.allowed, result.err + }) + defer restore() + + allowedPerms, anyDenied, outstandingPerms, err := rdb.IsRequestAllowed(user, snap, iface, path, testCase.requestedPerms) + c.Check(allowedPerms, DeepEquals, testCase.allowedPerms) + c.Check(anyDenied, Equals, testCase.anyDenied) + c.Check(outstandingPerms, DeepEquals, testCase.outstandingPerms) + if testCase.errStr == "" { + c.Check(err, IsNil) + } else { + c.Check(err, ErrorMatches, testCase.errStr) + } + } +} + +func (s *requestrulesSuite) TestIsPathPermAllowedSimple(c *C) { // Target user := s.defaultUser snap := "firefox" @@ -1051,7 +1185,7 @@ func (s *requestrulesSuite) TestIsPathAllowedSimple(c *C) { s.checkNewNoticesSimple(c, nil) } - allowed, err := rdb.IsPathAllowed(user, snap, iface, path, permission) + allowed, err := rdb.IsPathPermAllowed(user, snap, iface, path, permission) c.Check(err, Equals, testCase.err) c.Check(allowed, Equals, testCase.allowed) // Check that no notices were recorded when checking @@ -1065,7 +1199,7 @@ func (s *requestrulesSuite) TestIsPathAllowedSimple(c *C) { } } -func (s *requestrulesSuite) TestIsPathAllowedPrecedence(c *C) { +func (s *requestrulesSuite) TestIsPathPermAllowedPrecedence(c *C) { // Target user := s.defaultUser snap := "firefox" @@ -1122,13 +1256,13 @@ func (s *requestrulesSuite) TestIsPathAllowedPrecedence(c *C) { mostRecentOutcome, err := ruleContents.Outcome.AsBool() c.Check(err, IsNil) - allowed, err := rdb.IsPathAllowed(user, snap, iface, path, permission) + allowed, err := rdb.IsPathPermAllowed(user, snap, iface, path, permission) c.Check(err, IsNil) c.Check(allowed, Equals, mostRecentOutcome, Commentf("most recent: %+v", ruleContents)) } } -func (s *requestrulesSuite) TestIsPathAllowedExpiration(c *C) { +func (s *requestrulesSuite) TestIsPathPermAllowedExpiration(c *C) { // Target user := s.defaultUser snap := "firefox" @@ -1191,7 +1325,7 @@ func (s *requestrulesSuite) TestIsPathAllowedExpiration(c *C) { c.Check(err, IsNil) // Check that the outcome of the most specific unexpired rule has precedence - allowed, err := rdb.IsPathAllowed(user, snap, iface, path, permission) + allowed, err := rdb.IsPathPermAllowed(user, snap, iface, path, permission) c.Check(err, IsNil) c.Check(allowed, Equals, expectedOutcome, Commentf("last unexpired: %+v", rule)) diff --git a/overlord/ifacestate/apparmorprompting/prompting.go b/overlord/ifacestate/apparmorprompting/prompting.go index 0e9813b7dcf..f45a51fc758 100644 --- a/overlord/ifacestate/apparmorprompting/prompting.go +++ b/overlord/ifacestate/apparmorprompting/prompting.go @@ -20,7 +20,6 @@ package apparmorprompting import ( - "errors" "fmt" "sync" @@ -212,54 +211,26 @@ func (m *InterfacesRequestsManager) handleListenerReq(req *listener.Request) err return requestReply(req, nil) } - outstandingPerms := make([]string, 0, len(permissions)) - satisfiedPerms := make([]string, 0, len(permissions)) - // we're done with early checks, serious business starts now, and we can // take the lock m.lock.Lock() defer m.lock.Unlock() - matchedDenyRule := false - for _, perm := range permissions { - // TODO: move this for-loop to a helper in requestrules - if yesNo, err := m.rules.IsPathAllowed(userID, snap, iface, path, perm); err == nil { - if !yesNo { - matchedDenyRule = true - } else { - satisfiedPerms = append(satisfiedPerms, perm) - } - } else { - if !errors.Is(err, prompting_errors.ErrNoMatchingRule) { - logger.Noticef("error while checking request against existing rules: %v", err) - } - // No matching rule found - outstandingPerms = append(outstandingPerms, perm) + allowedPerms, matchedDenyRule, outstandingPerms, err := m.rules.IsRequestAllowed(userID, snap, iface, path, permissions) + if err != nil || matchedDenyRule || len(outstandingPerms) == 0 { + switch { + case err != nil: + logger.Noticef("error while checking request against existing rules: %v", err) + case matchedDenyRule: + logger.Debugf("request denied by existing rule: %+v", req) + case len(outstandingPerms) == 0: + logger.Debugf("request allowed by existing rule: %+v", req) } - } - if matchedDenyRule { - logger.Debugf("request denied by existing rule: %+v", req) - - // Respond with this information by allowing any requested permissions - // which were explicitly allowed by existing rules (there may be no - // such permissions) and let the listener deny all permissions which - // were not explicitly included in the allowed permissions. - allowedPermission, _ := prompting.AbstractPermissionsToAppArmorPermissions(iface, satisfiedPerms) - // Error should not occur, but if it does, allowedPermission is set to - // empty, leaving it to the listener to default deny all permissions. - return requestReply(req, allowedPermission) - } - - if len(outstandingPerms) == 0 { - logger.Debugf("request allowed by existing rule: %+v", req) - - // We don't want to just send back req.Permission() here, since that - // could include unrecognized permissions which were discarded, and - // were not matched by an existing rule. So only respond with the - // permissions which were matched and allowed, the listener will - // deny any permissions which were not explicitly included in the - // allowed permissions. - allowedPermission, _ := prompting.AbstractPermissionsToAppArmorPermissions(iface, satisfiedPerms) + // Allow any requested permissions which were explicitly allowed by + // existing rules (there may be no such permissions) and let the + // listener deny all permissions which were not explicitly included in + // the allowed permissions. + allowedPermission, _ := prompting.AbstractPermissionsToAppArmorPermissions(iface, allowedPerms) // Error should not occur, but if it does, allowedPermission is set to // empty, leaving it to the listener to default deny all permissions. return requestReply(req, allowedPermission) @@ -280,18 +251,18 @@ func (m *InterfacesRequestsManager) handleListenerReq(req *listener.Request) err // We weren't able to create a new prompt, so respond with the best // information we have, which is to allow any permissions which were // allowed by existing rules, and let the listener deny the rest. - allowedPermission, _ := prompting.AbstractPermissionsToAppArmorPermissions(iface, satisfiedPerms) + allowedPermission, _ := prompting.AbstractPermissionsToAppArmorPermissions(iface, allowedPerms) // Error should not occur, but if it does, allowedPermission is set to // empty, leaving it to the listener to default deny all permissions. return requestReply(req, allowedPermission) } + if merged { logger.Debugf("new prompt merged with identical existing prompt: %+v", newPrompt) - return nil + } else { + logger.Debugf("adding prompt to internal storage: %+v", newPrompt) } - logger.Debugf("adding prompt to internal storage: %+v", newPrompt) - return nil } From aee9e5e9066e06fa2956330b9cb784cb95eaa8b5 Mon Sep 17 00:00:00 2001 From: Oliver Calder Date: Sat, 23 Nov 2024 18:30:50 -0600 Subject: [PATCH 3/9] i/prompting: add function to compare lifespan supersedence Signed-off-by: Oliver Calder --- interfaces/prompting/prompting.go | 30 ++++++++++++++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/interfaces/prompting/prompting.go b/interfaces/prompting/prompting.go index 3049a00e5e5..7e55dc2b647 100644 --- a/interfaces/prompting/prompting.go +++ b/interfaces/prompting/prompting.go @@ -211,3 +211,33 @@ func (lifespan LifespanType) ParseDuration(duration string, currTime time.Time) } return expiration, nil } + +var lifespansBySupersedence = map[LifespanType]int{ + LifespanSingle: 1, + LifespanTimespan: 2, + LifespanForever: 3, +} + +// FirstLifespanGreater returns true if the first of the two given +// lifespan/expiration pairs supersedes the second. +// +// LifespanForever supersedes all other lifespans, followed by LifespanTimespan, +// followed by LifepsanSingle. If two lifespans are identical then return true +// if the first expiration timestamp is after the second expiration timestamp. +func FirstLifespanGreater(l1 LifespanType, e1 time.Time, l2 LifespanType, e2 time.Time) bool { + index1 := lifespansBySupersedence[l1] + index2 := lifespansBySupersedence[l2] + switch { + case index1 > index2: + return true + case index1 < index2: + return false + case e1.After(e2): + return true + case e1.Before(e2): + return false + default: + // They're the same + return false + } +} From 716c610054b006bfee2ac8a9c3567444a66e4431 Mon Sep 17 00:00:00 2001 From: Oliver Calder Date: Sat, 23 Nov 2024 18:31:22 -0600 Subject: [PATCH 4/9] i/p/requestrules: merge rules which have identical path patterns Signed-off-by: Oliver Calder --- .../prompting/requestrules/requestrules.go | 275 ++++++++++++++---- .../requestrules/requestrules_test.go | 170 ++++++++--- 2 files changed, 347 insertions(+), 98 deletions(-) diff --git a/interfaces/prompting/requestrules/requestrules.go b/interfaces/prompting/requestrules/requestrules.go index 1c617aac5dc..cbc700bc56b 100644 --- a/interfaces/prompting/requestrules/requestrules.go +++ b/interfaces/prompting/requestrules/requestrules.go @@ -90,6 +90,8 @@ type permissionDB struct { type interfaceDB struct { // interfaceDB contains a map from permission to permissionDB for a particular interface PerPermission map[string]*permissionDB + // PathPatterns maps from path pattern string to rule ID + PathPatterns map[string]prompting.IDType } // snapDB stores a map from interface name to the DB of rules pertaining to that @@ -184,6 +186,9 @@ func (rdb *RuleDB) load() (retErr error) { rdb.perUser = make(map[uint32]*userDB) expiredRules := make(map[prompting.IDType]bool) + // Store map of merged rules, where the original merged (removed) rule ID + // maps to the ID of the rule into which it was merged. + mergedRules := make(map[prompting.IDType]prompting.IDType) f, err := os.Open(rdb.dbPath) if err != nil { @@ -220,12 +225,16 @@ func (rdb *RuleDB) load() (retErr error) { continue } - conflictErr := rdb.addRule(rule) + save := false + mergedRule, merged, conflictErr := rdb.addOrMergeRule(rule, save) if conflictErr != nil { // Duplicate rules on disk or conflicting rule, should not occur errInvalid = fmt.Errorf("cannot add rule: %w", conflictErr) break } + if merged { + mergedRules[rule.ID] = mergedRule.ID + } } if errInvalid != nil { @@ -248,11 +257,16 @@ func (rdb *RuleDB) load() (retErr error) { var data map[string]string if expiredRules[rule.ID] { data = expiredData + } else if newID, exists := mergedRules[rule.ID]; exists { + data = map[string]string{ + "removed": "merged", + "merged-into": newID.String(), + } } rdb.notifyRule(rule.User, rule.ID, data) } - if len(expiredRules) > 0 { + if len(expiredRules) > 0 || len(mergedRules) > 0 { return rdb.save() } @@ -272,6 +286,25 @@ func (rdb *RuleDB) save() error { return osutil.AtomicWriteFile(rdb.dbPath, b, 0o600, 0) } +// lookupRuleByPathPattern checks whether there is an existing rule for the +// given user, snap, and iface, which has an identical path pattern to that in +// the given constraints. If it does exist, returns it, along with a bool +// indicating whether it exists. If an error occurs, returns it. +// +// The caller must ensure that the database lock is held. +func (rdb *RuleDB) lookupRuleByPathPattern(user uint32, snap string, iface string, constraints *prompting.RuleConstraints) (*Rule, bool, error) { + interfaceDB, exists := rdb.interfaceDBForUserSnapInterface(user, snap, iface) + if !exists { + return nil, false, nil + } + ruleID, exists := interfaceDB.PathPatterns[constraints.PathPattern.String()] + if !exists { + return nil, false, nil + } + rule, err := rdb.lookupRuleByID(ruleID) + return rule, true, err +} + // lookupRuleByID returns the rule with the given ID from the rule DB. // // The caller must ensure that the database lock is held. @@ -307,24 +340,155 @@ func (rdb *RuleDB) addRuleToRulesList(rule *Rule) error { return nil } -// addRule adds the given rule to the rule DB. +// addOrMergeRule adds the given rule to the rule DB, or merges it with an +// existing rule if there is an existing rule for the same user, snap, and +// interface with an identical path pattern. +// +// If save is true, saves the DB after the rule has been added or merged. +// +// If the rule's ID is 0 and it cannot be merged with an existing rule, then it +// is assigned a new ID, mutating the rule which was passed in as an argument. // -// If there is a conflicting rule, returns an error, and the rule DB is left -// unchanged. +// If the rule is merged with an existing rule, then both the given rule and the +// existing rule are removed from the DB, and a new rule is constructed with the +// merged contents of the rule to be added and the existing rule, and that +// merged rule is added to the tree, and returned along with merged set to true. +// +// If the rule is not merged with an existing rule, then the given rule is +// returned, and merged is returned as false. +// +// If there is a conflicting rule, or if there is an error while saving the DB, +// returns an error, and the rule DB is left unchanged. // // The caller must ensure that the database lock is held for writing. -func (rdb *RuleDB) addRule(rule *Rule) error { +func (rdb *RuleDB) addOrMergeRule(rule *Rule, save bool) (addedOrMergedRule *Rule, merged bool, err error) { + // Check if rule with identical path pattern exists. + existingRule, exists, err := rdb.lookupRuleByPathPattern(rule.User, rule.Snap, rule.Interface, rule.Constraints) + if err != nil { + // Database was left inconsistent, should not occur + return nil, false, err + } + if !exists { + return rule, false, rdb.addNewRule(rule, save) + } + + currTime := time.Now() + + // Check whether the existing rule has outcomes/lifespans which conflict, + // and compile the new set of permissions. + newPermissions := make(prompting.RulePermissionMap) + var conflicts []prompting_errors.RuleConflict + for perm, entry := range rule.Constraints.Permissions { + existingEntry, exists := existingRule.Constraints.Permissions[perm] + if !exists || existingEntry.Expired(currTime) { + newPermissions[perm] = entry + continue + } + if entry.Outcome != existingEntry.Outcome { + // New entry outcome conflicts with outcome of existing entry + conflicts = append(conflicts, prompting_errors.RuleConflict{ + Permission: perm, + Variant: rule.Constraints.PathPattern.String(), // XXX: we're mis-using the full path pattern in place of the variant + ConflictingID: existingRule.ID.String(), + }) + continue + } + if prompting.FirstLifespanGreater(entry.Lifespan, entry.Expiration, existingEntry.Lifespan, existingEntry.Expiration) { + newPermissions[perm] = entry + } else { + newPermissions[perm] = existingEntry + } + } + // If there were any conflicts with the existing rule with identical path + // pattern, return error. + if len(conflicts) > 0 { + return nil, false, &prompting_errors.RuleConflictError{ + Conflicts: conflicts, + } + } + // Add any non-expired permissions which were left over from the existing rule. + for existingPerm, existingEntry := range existingRule.Constraints.Permissions { + if existingEntry.Expired(currTime) { + continue + } + if _, exists := newPermissions[existingPerm]; exists { + continue + } + newPermissions[existingPerm] = existingEntry + } + + // Create new rule based on the contents of the existing rule + newRuleContents := *existingRule + newRule := &newRuleContents + // Copy constraints as well, since copying the rule just copied the pointer + newConstraints := *(existingRule.Constraints) + newRule.Constraints = &newConstraints + // Now set the permissions of the copied constraints to newPermissions + newRule.Constraints.Permissions = newPermissions + + // Remove the existing rule from the tree. An error should not occur, since + // we just looked up the rule and know it exists. + rdb.removeRuleByID(existingRule.ID) + + if err := rdb.addNewRule(newRule, save); err != nil { + // Error while adding the new merged rule, likely due to a conflict + // caused by the new permissions in the rule to be added. + + // Re-add original the original rule so all is unchanged, which should + // succeed since addNewRule should have rolled back successfully and + // we're now simply re-adding the existing rule which we just removed. + // Don't save, since nothing should have changed after the rollback is + // complete. + if restoreErr := rdb.addNewRule(existingRule, false); restoreErr != nil { + // Error should not occur, but if it does, wrap it in the other error + err = strutil.JoinErrors(err, fmt.Errorf("cannot re-add existing rule: %w", restoreErr)) + } + return nil, false, err + } + + return newRule, true, nil +} + +// addNewRule adds the given rule to the rule DB without checking whether there +// are any existing rules with the same path pattern. If save is true, saves +// the DB after the new rule has been added. +// +// This method should only be called from addOrMergeRule, or when rolling back +// the removal of a removal of a rule by re-adding it after an error occurs. +// +// Returns an error if the rule conflicts with an existing rule. +// +// The caller must ensure that the database lock is held for writing. +func (rdb *RuleDB) addNewRule(rule *Rule, save bool) error { + // If the rule has no ID, assign a new one. + if rule.ID == 0 { + id, _ := rdb.maxIDMmap.NextID() + rule.ID = id + } if err := rdb.addRuleToRulesList(rule); err != nil { return err } conflictErr := rdb.addRuleToTree(rule) - if conflictErr == nil { + if conflictErr != nil { + // remove just-added rule from rules list and IDs + rdb.rules = rdb.rules[:len(rdb.rules)-1] + delete(rdb.indexByID, rule.ID) + return conflictErr + } + + if !save { return nil } - // remove just-added rule from rules list and IDs - rdb.rules = rdb.rules[:len(rdb.rules)-1] - delete(rdb.indexByID, rule.ID) - return conflictErr + + if err := rdb.save(); err != nil { + // Should not occur, but if it does, roll back to the original state. + // The following should succeeded, since we're removing the rule which + // we just successfully added. + rdb.removeRuleByID(rule.ID) + return err + } + + return nil } // removeRuleByIDFromRulesList removes the rule with the given ID from the rules @@ -412,6 +576,11 @@ func (rdb *RuleDB) addRuleToTree(rule *Rule) *prompting_errors.RuleConflictError } } + // Add rule to the interfaceDB's map of path patterns to rule IDs. We know + // the interfaceDB exists, since we just modified a rule there. + interfaceDB, _ := rdb.interfaceDBForUserSnapInterface(rule.User, rule.Snap, rule.Interface) + interfaceDB.PathPatterns[rule.Constraints.PathPattern.String()] = rule.ID + return nil } @@ -542,6 +711,15 @@ func (rdb *RuleDB) removeRuleFromTree(rule *Rule) error { errs = append(errs, err) } } + + // Remove rule from the interfaceDB's map of path patterns to rule IDs. If + // the interfaceDB doesn't exist, then there must have been a previous error, + // and regardless, the path pattern doesn't exist in its map anymore. + interfaceDB, exists := rdb.interfaceDBForUserSnapInterface(rule.User, rule.Snap, rule.Interface) + if exists { + delete(interfaceDB.PathPatterns, rule.Constraints.PathPattern.String()) + } + return joinInternalErrors(errs) } @@ -600,23 +778,35 @@ func joinInternalErrors(errs []error) error { // // The caller must ensure that the database lock is held. func (rdb *RuleDB) permissionDBForUserSnapInterfacePermission(user uint32, snap string, iface string, permission string) (*permissionDB, bool) { - userSnaps := rdb.perUser[user] - if userSnaps == nil { + interfaceRules, exists := rdb.interfaceDBForUserSnapInterface(user, snap, iface) + if !exists { return nil, false } - snapInterfaces := userSnaps.PerSnap[snap] - if snapInterfaces == nil { + permRules := interfaceRules.PerPermission[permission] + if permRules == nil { return nil, false } - interfacePerms := snapInterfaces.PerInterface[iface] - if interfacePerms == nil { + return permRules, true +} + +// interfaceDBForUserSnapInterface returns the interface DB for the given user, +// snap, and interface, if it exists. +// +// The caller must ensure that the database lock is held. +func (rdb *RuleDB) interfaceDBForUserSnapInterface(user uint32, snap string, iface string) (*interfaceDB, bool) { + userRules := rdb.perUser[user] + if userRules == nil { return nil, false } - permVariants := interfacePerms.PerPermission[permission] - if permVariants == nil { + snapRules := userRules.PerSnap[snap] + if snapRules == nil { + return nil, false + } + interfaceRules := snapRules.PerInterface[iface] + if interfaceRules == nil { return nil, false } - return permVariants, true + return interfaceRules, true } // ensurePermissionDBForUserSnapInterfacePermission returns the permission DB @@ -643,6 +833,7 @@ func (rdb *RuleDB) ensurePermissionDBForUserSnapInterfacePermission(user uint32, if interfacePerms == nil { interfacePerms = &interfaceDB{ PerPermission: make(map[string]*permissionDB), + PathPatterns: make(map[string]prompting.IDType), } snapInterfaces.PerInterface[iface] = interfacePerms } @@ -687,27 +878,19 @@ func (rdb *RuleDB) AddRule(user uint32, snap string, iface string, constraints * if err != nil { return nil, err } - if err := rdb.addRule(newRule); err != nil { - // Cannot have expired, since the expiration is based on the lifespan, - // duration, and timestamp, all of which were validated and set in - // makeNewRule. + save := true + newRule, _, err = rdb.addOrMergeRule(newRule, save) + if err != nil { + // If an error occurred, all changes were rolled back. return nil, fmt.Errorf("cannot add rule: %w", err) } - if err := rdb.save(); err != nil { - // Failed to save, so revert the rule addition so no change occurred - // and the rule DB state matches that preserved on disk. - rdb.removeRuleByID(newRule.ID) - // We know that this rule exists, since we just added it, so no error - // can occur. - return nil, err - } - rdb.notifyRule(user, newRule.ID, nil) return newRule, nil } -// makeNewRule creates a new Rule with the given contents. +// makeNewRule creates a new Rule with the given contents. It does not assign +// the rule an ID, in case it can be merged with an existing rule. // // Constructs a new rule with the given parameters as values. The given // constraints are converted to rule constraints, using the timestamp of the @@ -720,11 +903,7 @@ func (rdb *RuleDB) makeNewRule(user uint32, snap string, iface string, constrain return nil, err } - // Don't consume an ID until now, when we know the rule is valid - id, _ := rdb.maxIDMmap.NextID() - newRule := Rule{ - ID: id, Timestamp: currTime, User: user, Snap: snap, @@ -1099,25 +1278,21 @@ func (rdb *RuleDB) PatchRule(user uint32, id prompting.IDType, constraintsPatch // we just looked up the rule and know it exists. rdb.removeRuleByID(origRule.ID) - if addErr := rdb.addRule(newRule); addErr != nil { + save := true + newRule, _, addErr := rdb.addOrMergeRule(newRule, save) + if addErr != nil { err := fmt.Errorf("cannot patch rule: %w", addErr) - // Try to re-add original rule so all is unchanged. - if origErr := rdb.addRule(origRule); origErr != nil { + // Re-add the original rule so all is unchanged, which should + // succeed since we're simply reversing what we just completed. + // Don't save, since nothing should have changed after the rollback + // is complete. + if origErr := rdb.addNewRule(origRule, false); origErr != nil { // Error should not occur, but if it does, wrap it in the other error err = strutil.JoinErrors(err, fmt.Errorf("cannot re-add original rule: %w", origErr)) } return nil, err } - if err := rdb.save(); err != nil { - // Should not occur, but if it does, roll back to the original state. - // All of the following should succeed, since we're reversing what we - // just successfully completed. - rdb.removeRuleByID(newRule.ID) - rdb.addRule(origRule) - return nil, err - } - rdb.notifyRule(newRule.User, newRule.ID, nil) return newRule, nil } diff --git a/interfaces/prompting/requestrules/requestrules_test.go b/interfaces/prompting/requestrules/requestrules_test.go index c04e8020607..17401d41aa6 100644 --- a/interfaces/prompting/requestrules/requestrules_test.go +++ b/interfaces/prompting/requestrules/requestrules_test.go @@ -77,7 +77,7 @@ func (s *requestrulesSuite) SetUpTest(c *C) { s.ruleNotices = make([]*noticeInfo, 0) dirs.SetRootDir(c.MkDir()) s.AddCleanup(func() { dirs.SetRootDir("") }) - c.Assert(os.MkdirAll(dirs.SnapdStateDir(dirs.GlobalRootDir), 0700), IsNil) + c.Assert(os.MkdirAll(dirs.SnapdStateDir(dirs.GlobalRootDir), 0o700), IsNil) } func mustParsePathPattern(c *C, patternStr string) *patterns.PathPattern { @@ -225,6 +225,12 @@ func (s *requestrulesSuite) TestLoadErrorValidate(c *C) { s.testLoadError(c, `internal error: invalid interface: "foo".*`, rules, checkWritten) } +func (s *requestrulesSuite) ruleTemplateWithReadPathPattern(c *C, id prompting.IDType, pattern string) *requestrules.Rule { + rule := s.ruleTemplateWithRead(c, id) + rule.Constraints.PathPattern = mustParsePathPattern(c, pattern) + return rule +} + // ruleTemplateWithRead returns a rule with valid contents, intended to be customized. func (s *requestrulesSuite) ruleTemplateWithRead(c *C, id prompting.IDType) *requestrules.Rule { rule := s.ruleTemplate(c, id) @@ -236,6 +242,13 @@ func (s *requestrulesSuite) ruleTemplateWithRead(c *C, id prompting.IDType) *req return rule } +// ruleTemplateWithPathPattern returns a rule with valid contents, intended to be customized. +func (s *requestrulesSuite) ruleTemplateWithPathPattern(c *C, id prompting.IDType, pattern string) *requestrules.Rule { + rule := s.ruleTemplate(c, id) + rule.Constraints.PathPattern = mustParsePathPattern(c, pattern) + return rule +} + // ruleTemplate returns a rule with valid contents, intended to be customized. func (s *requestrulesSuite) ruleTemplate(c *C, id prompting.IDType) *requestrules.Rule { constraints := prompting.RuleConstraints{ @@ -268,8 +281,7 @@ func (s *requestrulesSuite) TestLoadErrorConflictingID(c *C) { currTime := time.Now() good := s.ruleTemplateWithRead(c, prompting.IDType(1)) // Expired rules should still get a {"removed": "expired"} notice, even if they don't conflict - expired := s.ruleTemplate(c, prompting.IDType(2)) - expired.Constraints.PathPattern = mustParsePathPattern(c, "/home/test/other") + expired := s.ruleTemplateWithPathPattern(c, prompting.IDType(2), "/home/test/other") setPermissionsOutcomeLifespanExpiration(c, expired, []string{"read"}, prompting.OutcomeAllow, prompting.LifespanTimespan, currTime.Add(-10*time.Second)) // Add rule which conflicts with IDs but doesn't otherwise conflict conflicting := s.ruleTemplateWithRead(c, prompting.IDType(1)) @@ -295,13 +307,13 @@ func setPermissionsOutcomeLifespanExpiration(c *C, rule *requestrules.Rule, perm func (s *requestrulesSuite) TestLoadErrorConflictingPattern(c *C) { dbPath := s.prepDBPath(c) currTime := time.Now() - good := s.ruleTemplateWithRead(c, prompting.IDType(1)) + good := s.ruleTemplateWithReadPathPattern(c, prompting.IDType(1), "/home/test/{foo,bar}") // Expired rules should still get a {"removed": "expired"} notice, even if they don't conflict expired := s.ruleTemplateWithRead(c, prompting.IDType(2)) expired.Constraints.PathPattern = mustParsePathPattern(c, "/home/test/other") setPermissionsOutcomeLifespanExpiration(c, expired, []string{"read"}, prompting.OutcomeAllow, prompting.LifespanTimespan, currTime.Add(-10*time.Second)) - // Add rule with conflicting pattern and permissions but not conflicting ID. - conflicting := s.ruleTemplateWithRead(c, prompting.IDType(3)) + // Add rule with conflicting permissions but not conflicting ID. + conflicting := s.ruleTemplateWithReadPathPattern(c, prompting.IDType(3), "/home/test/{bar,foo}") // Even with not all permissions being in conflict, still error var timeZero time.Time setPermissionsOutcomeLifespanExpiration(c, conflicting, []string{"read", "write"}, prompting.OutcomeDeny, prompting.LifespanForever, timeZero) @@ -317,22 +329,20 @@ func (s *requestrulesSuite) TestLoadExpiredRules(c *C) { dbPath := s.prepDBPath(c) currTime := time.Now() - good1 := s.ruleTemplateWithRead(c, prompting.IDType(1)) + good1 := s.ruleTemplateWithReadPathPattern(c, prompting.IDType(1), "/home/test/{foo,bar}") // At the moment, expired rules with conflicts are discarded without error, // but we don't want to test this as part of our contract - expired1 := s.ruleTemplate(c, prompting.IDType(2)) - expired1.Constraints.PathPattern = mustParsePathPattern(c, "/home/test/other") + expired1 := s.ruleTemplateWithPathPattern(c, prompting.IDType(2), "/home/test/other") setPermissionsOutcomeLifespanExpiration(c, expired1, []string{"read"}, prompting.OutcomeAllow, prompting.LifespanTimespan, currTime.Add(-10*time.Second)) - // Rules with same pattern but non-conflicting permissions do not conflict - good2 := s.ruleTemplate(c, prompting.IDType(3)) + // Rules with overlapping pattern but non-conflicting permissions do not conflict + good2 := s.ruleTemplateWithPathPattern(c, prompting.IDType(3), "/home/test/{bar,foo}") var timeZero time.Time setPermissionsOutcomeLifespanExpiration(c, good2, []string{"write"}, prompting.OutcomeDeny, prompting.LifespanForever, timeZero) - expired2 := s.ruleTemplate(c, prompting.IDType(4)) - expired2.Constraints.PathPattern = mustParsePathPattern(c, "/home/test/another") + expired2 := s.ruleTemplateWithPathPattern(c, prompting.IDType(4), "/home/test/another") setPermissionsOutcomeLifespanExpiration(c, expired2, []string{"read"}, prompting.OutcomeAllow, prompting.LifespanTimespan, currTime.Add(-time.Nanosecond)) // Rules with different pattern and conflicting permissions do not conflict @@ -535,15 +545,14 @@ func (s *requestrulesSuite) TestAddRuleHappy(c *C) { var addedRules []*requestrules.Rule - // Add one rule matching template, then a rule with one differing field for - // each conflict-defining field, to check that all rules add without error. + // Add one rule matching template, then rules with at least one differing + // field and check that all rules add without error. for _, ruleContents := range []*addRuleContents{ {}, // use template {User: s.defaultUser + 1}, {Snap: "thunderbird"}, // Can't change interface, as only "home" is supported for now (TODO: update) {PathPattern: "/home/test/**/*.{jpg,png,svg}"}, // no /Pictures/ - {Permissions: []string{"read", "execute"}}, // Differing Outcome, Lifespan or Duration does not prevent conflict {PathPattern: "/home/test/1", Outcome: prompting.OutcomeDeny}, {PathPattern: "/home/test/2", Lifespan: prompting.LifespanTimespan, Duration: "10s"}, @@ -555,6 +564,28 @@ func (s *requestrulesSuite) TestAddRuleHappy(c *C) { s.checkWrittenRuleDB(c, addedRules) s.checkNewNoticesSimple(c, nil, rule) } + + // Add a rule identical to the template but with additional permissions, + // and see that it merges with the existing rule with the same path pattern. + contents := &addRuleContents{Permissions: []string{"read", "execute"}} + rule, err := addRuleFromTemplate(c, rdb, template, contents) + c.Check(err, IsNil) + c.Check(rule.ID, Equals, addedRules[0].ID) + // Prepare the list of current rules, which should contain the new rule and + // not the original rule with that ID. The order is based on the current + // implementation, is not part of the contract and is subject to change, but + // it makes testing easier. + expected := make([]*requestrules.Rule, 0, len(addedRules)) + // Final rule was swapped to the start when the first was removed + expected = append(expected, addedRules[len(addedRules)-1]) + // The rest of the rules between the first and the last were left unchanged + expected = append(expected, addedRules[1:len(addedRules)-1]...) + // The "new" rule with the same ID as the original rule was appended + expected = append(expected, rule) + s.checkWrittenRuleDB(c, expected) + // We get a notice because the original rule was "modified" when the new + // rule was merged into it (though no notice for its intermediate removal). + s.checkNewNoticesSimple(c, nil, rule) } // addRuleFromTemplate takes a template contents and a partial contents and, @@ -723,21 +754,41 @@ func (s *requestrulesSuite) TestAddRuleErrors(c *C) { // Check that the conflicting rule error can be unwrapped as ErrRuleConflict c.Check(errors.Is(finalErr, prompting_errors.ErrRuleConflict), Equals, true) - // Failure to save rule DB should roll-back adding the rule and leave the - // DB unchanged. Set DB parent directory as read-only. + // Failure to save rule DB should roll-back adding the rule when it does not + // merge with an existing rule, and leave the DB unchanged. + // Set DB parent directory as read-only. c.Assert(os.Chmod(prompting.StateDir(), 0o500), IsNil) - result, err := addRuleFromTemplate(c, rdb, template, &addRuleContents{Permissions: []string{"execute"}}) + result, err := addRuleFromTemplate(c, rdb, template, &addRuleContents{PathPattern: "/other", Permissions: []string{"execute"}}) c.Assert(os.Chmod(prompting.StateDir(), 0o700), IsNil) c.Check(err, NotNil) c.Check(result, IsNil) - // Failure should result in no changes to written rules and no notices + // Failure should result in no changes to rules, written or in-memory, and no notices + s.checkWrittenRuleDB(c, []*requestrules.Rule{good}) + s.checkNewNoticesSimple(c, nil) + c.Check(rdb.Rules(s.defaultUser), DeepEquals, []*requestrules.Rule{good}) + + // Failure to save rule DB should roll-back adding the rule when it merges + // with an existing rule, re-add the original rule, and leave the DB + // unchanged. + // Set DB parent directory as read-only. + c.Assert(os.Chmod(prompting.StateDir(), 0o500), IsNil) + result, err = addRuleFromTemplate(c, rdb, template, &addRuleContents{Permissions: []string{"execute"}}) + c.Assert(os.Chmod(prompting.StateDir(), 0o700), IsNil) + c.Check(err, NotNil) + c.Check(result, IsNil) + // Failure should result in no changes to rules, written or in-memory, and no notices s.checkWrittenRuleDB(c, []*requestrules.Rule{good}) s.checkNewNoticesSimple(c, nil) + c.Check(rdb.Rules(s.defaultUser), DeepEquals, []*requestrules.Rule{good}) + // Remove read-only so we can continue // Adding rule after the rule DB has been closed should return an error // immediately. c.Assert(rdb.Close(), IsNil) + // Make sure the save triggered by Close() didn't mess up the written rules + s.checkWrittenRuleDB(c, []*requestrules.Rule{good}) + s.checkNewNoticesSimple(c, nil) result, err = addRuleFromTemplate(c, rdb, template, &addRuleContents{Permissions: []string{"execute"}}) c.Check(err, Equals, prompting_errors.ErrRulesClosed) c.Check(result, IsNil) @@ -768,10 +819,8 @@ func (s *requestrulesSuite) TestAddRuleOverlapping(c *C) { for _, ruleContents := range []*addRuleContents{ {}, // use template {PathPattern: "/home/test/Pictures/**/*.{jpg,png,svg}"}, - {Permissions: []string{"read", "write"}}, {PathPattern: "/home/test/Pictures/**/*.{jp,pn,sv}g"}, {PathPattern: "/home/test/{Documents,Pictures}/**/*.{jpg,png,svg}", Permissions: []string{"read", "write", "execute"}}, - {}, // template again, for good measure } { rule, err := addRuleFromTemplate(c, rdb, template, ruleContents) c.Check(err, IsNil) @@ -781,10 +830,14 @@ func (s *requestrulesSuite) TestAddRuleOverlapping(c *C) { s.checkNewNoticesSimple(c, nil, rule) } - // Lastly, add a conflicting rule, and check that it conflicts with all - // the prior rules + // Add conflicting rule, and check that it conflicts with all the prior rules. + // + // Due to implementation details, its path pattern cannot be identical to + // any of the previous rules, else it will only conflict with that rule, as + // this is checked before checking other rules with different path patterns. rule, err := addRuleFromTemplate(c, rdb, template, &addRuleContents{ - Outcome: prompting.OutcomeDeny, + PathPattern: "/home/test/{Pictures,Videos}/**/*.png", + Outcome: prompting.OutcomeDeny, }) c.Check(err, NotNil) c.Check(rule, IsNil) @@ -828,6 +881,7 @@ func (s *requestrulesSuite) TestAddRuleExpired(c *C) { // Add initial rule which will expire quickly prev, err := addRuleFromTemplate(c, rdb, template, &addRuleContents{ + Outcome: prompting.OutcomeDeny, Lifespan: prompting.LifespanTimespan, Duration: "1ms", }) @@ -842,7 +896,6 @@ func (s *requestrulesSuite) TestAddRuleExpired(c *C) { // Add rules which all conflict but each expire before the next is added, // thus causing the prior one to be removed and not causing a conflict error. for _, ruleContents := range []*addRuleContents{ - {Outcome: prompting.OutcomeDeny}, {Outcome: prompting.OutcomeAllow, PathPattern: "/home/test/{**/secret,**/private}/**"}, {Outcome: prompting.OutcomeDeny, PathPattern: "/home/test/**/{sec,priv}{ret,ate}/**", Permissions: []string{"read", "write"}}, {Outcome: prompting.OutcomeAllow, Permissions: []string{"write", "execute"}}, @@ -1408,9 +1461,9 @@ func (s *requestrulesSuite) prepRuleDBForRulesForSnapInterface(c *C, rdb *reques for _, ruleContents := range []*addRuleContents{ {}, - {Permissions: []string{"write"}}, + {PathPattern: "/home/test/other", Permissions: []string{"write"}}, {Snap: "amberol"}, - {Snap: "amberol", Permissions: []string{"write"}}, // change interface later + {Snap: "amberol", PathPattern: "/home/test/other", Permissions: []string{"write"}}, // change interface later {User: s.defaultUser + 1}, } { rule, err := addRuleFromTemplate(c, rdb, template, ruleContents) @@ -1780,7 +1833,7 @@ func (s *requestrulesSuite) TestPatchRule(c *C) { var rules []*requestrules.Rule for _, ruleContents := range []*addRuleContents{ - {}, + {PathPattern: "/home/test/foo"}, {Permissions: []string{"read"}}, } { rule, err := addRuleFromTemplate(c, rdb, template, ruleContents) @@ -1937,6 +1990,23 @@ func (s *requestrulesSuite) TestPatchRule(c *C) { // After making timestamp the same again, check that the rules are identical patched.Timestamp = origRule.Timestamp c.Check(patched, DeepEquals, &origRule) + + rule = patched + + // Patch rule so it has the same path pattern as an existing rule, and check + // that this results in the rules being merged + constraintsPatch = &prompting.RuleConstraintsPatch{ + PathPattern: mustParsePathPattern(c, "/home/test/foo"), + } + patched, err = rdb.PatchRule(rule.User, rule.ID, constraintsPatch) + c.Assert(err, IsNil) + // Check that rule inherited the ID of the rule with which it was merged + c.Check(patched.ID, Equals, rules[0].ID) + // Check that the ID of the patched rule changed, rather than that of the + // rule into which it was merged. + c.Check(patched.ID, Not(Equals), rule.ID) + s.checkWrittenRuleDB(c, []*requestrules.Rule{patched}) + s.checkNewNoticesSimple(c, nil, patched) } func (s *requestrulesSuite) TestPatchRuleErrors(c *C) { @@ -1957,7 +2027,7 @@ func (s *requestrulesSuite) TestPatchRuleErrors(c *C) { var rules []*requestrules.Rule for _, ruleContents := range []*addRuleContents{ - {}, + {PathPattern: "/home/test/foo"}, {Permissions: []string{"read"}}, } { rule, err := addRuleFromTemplate(c, rdb, template, ruleContents) @@ -2000,8 +2070,9 @@ func (s *requestrulesSuite) TestPatchRuleErrors(c *C) { s.checkWrittenRuleDB(c, rules) s.checkNewNoticesSimple(c, nil) - // Conflicting rule + // Conflicting with other rule conflictingPatch := &prompting.RuleConstraintsPatch{ + PathPattern: mustParsePathPattern(c, "/home/test/{foo,{Downloads,Documents}/**/*.{ical,mail,txt,gpg}}"), Permissions: prompting.PermissionMap{ "read": &prompting.PermissionEntry{ Outcome: prompting.OutcomeDeny, @@ -2051,7 +2122,6 @@ func (s *requestrulesSuite) TestPatchRuleExpired(c *C) { User: s.defaultUser, Snap: "thunderbird", Interface: "home", - PathPattern: "/home/test/{Downloads,Documents}/**/*.{ical,mail,txt,gpg}", Permissions: []string{"write"}, Outcome: prompting.OutcomeAllow, Lifespan: prompting.LifespanForever, @@ -2061,9 +2131,9 @@ func (s *requestrulesSuite) TestPatchRuleExpired(c *C) { var rules []*requestrules.Rule for _, ruleContents := range []*addRuleContents{ - {Lifespan: prompting.LifespanTimespan, Duration: "1ms"}, - {Lifespan: prompting.LifespanTimespan, Duration: "1ms", Permissions: []string{"read"}}, - {Permissions: []string{"execute"}}, + {PathPattern: "/foo", Lifespan: prompting.LifespanTimespan, Duration: "1ms"}, + {PathPattern: "/bar", Lifespan: prompting.LifespanTimespan, Duration: "1ms", Permissions: []string{"read"}}, + {PathPattern: "/baz", Permissions: []string{"execute"}}, } { rule, err := addRuleFromTemplate(c, rdb, template, ruleContents) c.Check(err, IsNil) @@ -2078,6 +2148,7 @@ func (s *requestrulesSuite) TestPatchRuleExpired(c *C) { // Patching doesn't conflict with already-expired rules rule := rules[2] constraintsPatch := &prompting.RuleConstraintsPatch{ + PathPattern: mustParsePathPattern(c, "/{foo,bar}"), Permissions: prompting.PermissionMap{ "read": &prompting.PermissionEntry{ Outcome: prompting.OutcomeDeny, @@ -2116,20 +2187,23 @@ func (s *requestrulesSuite) TestPatchRuleExpired(c *C) { s.checkNewNoticesUnordered(c, expectedNotices) // Check that timestamp has changed c.Check(patched.Timestamp.Equal(rule.Timestamp), Equals, false) - // After making timestamp the same again, check that the rules are identical + // After making timestamp and constraints the same again, check that the rules are identical patched.Timestamp = rule.Timestamp - rule.Constraints.Permissions = prompting.RulePermissionMap{ - "read": &prompting.RulePermissionEntry{ - Outcome: prompting.OutcomeDeny, - Lifespan: prompting.LifespanForever, - }, - "write": &prompting.RulePermissionEntry{ - Outcome: prompting.OutcomeDeny, - Lifespan: prompting.LifespanForever, - }, - "execute": &prompting.RulePermissionEntry{ - Outcome: prompting.OutcomeDeny, - Lifespan: prompting.LifespanForever, + rule.Constraints = &prompting.RuleConstraints{ + PathPattern: mustParsePathPattern(c, "/{foo,bar}"), + Permissions: prompting.RulePermissionMap{ + "read": &prompting.RulePermissionEntry{ + Outcome: prompting.OutcomeDeny, + Lifespan: prompting.LifespanForever, + }, + "write": &prompting.RulePermissionEntry{ + Outcome: prompting.OutcomeDeny, + Lifespan: prompting.LifespanForever, + }, + "execute": &prompting.RulePermissionEntry{ + Outcome: prompting.OutcomeDeny, + Lifespan: prompting.LifespanForever, + }, }, } c.Check(patched, DeepEquals, rule) From 94d50def4305427f3f222725d6e90f4286aa259a Mon Sep 17 00:00:00 2001 From: Oliver Calder Date: Mon, 2 Dec 2024 16:24:23 -0600 Subject: [PATCH 5/9] i/prompting: add unit test for FirstLifespanGreater Signed-off-by: Oliver Calder --- interfaces/prompting/prompting_test.go | 78 ++++++++++++++++++++++++++ 1 file changed, 78 insertions(+) diff --git a/interfaces/prompting/prompting_test.go b/interfaces/prompting/prompting_test.go index bcb27b8d960..78c00095ad5 100644 --- a/interfaces/prompting/prompting_test.go +++ b/interfaces/prompting/prompting_test.go @@ -228,3 +228,81 @@ func (s *promptingSuite) TestParseDuration(c *C) { c.Check(err, IsNil) c.Check(expiration2.Equal(expiration), Equals, true) } + +func (s *promptingSuite) TestFirstLifespanGreater(c *C) { + for _, testCase := range []struct { + l1 prompting.LifespanType + e1 time.Time + l2 prompting.LifespanType + e2 time.Time + result bool + }{ + { + l1: prompting.LifespanForever, + l2: prompting.LifespanForever, + result: false, + }, + { + l1: prompting.LifespanForever, + l2: prompting.LifespanTimespan, + e2: time.Now().Add(time.Second), + result: true, + }, + { + l1: prompting.LifespanForever, + l2: prompting.LifespanSingle, + result: true, + }, + { + l1: prompting.LifespanTimespan, + e1: time.Now().Add(time.Second), + l2: prompting.LifespanForever, + result: false, + }, + { + l1: prompting.LifespanTimespan, + e1: time.Now().Add(2 * time.Second), + l2: prompting.LifespanTimespan, + e2: time.Now().Add(time.Second), + result: true, + }, + { + l1: prompting.LifespanTimespan, + e1: time.Now().Add(time.Second), + l2: prompting.LifespanTimespan, + e2: time.Now().Add(time.Second), + result: false, + }, + { + l1: prompting.LifespanTimespan, + e1: time.Now().Add(time.Second), + l2: prompting.LifespanTimespan, + e2: time.Now().Add(2 * time.Second), + result: false, + }, + { + l1: prompting.LifespanTimespan, + e1: time.Now().Add(time.Second), + l2: prompting.LifespanSingle, + result: true, + }, + { + l1: prompting.LifespanSingle, + l2: prompting.LifespanForever, + result: false, + }, + { + l1: prompting.LifespanSingle, + l2: prompting.LifespanTimespan, + e2: time.Now().Add(time.Second), + result: false, + }, + { + l1: prompting.LifespanSingle, + l2: prompting.LifespanSingle, + result: false, + }, + } { + c.Check(prompting.FirstLifespanGreater(testCase.l1, testCase.e1, testCase.l2, testCase.e2), Equals, testCase.result, Commentf("testCase: %+v", testCase)) + } +} From a189fff01f223658d083c3a6aba6889b67d17afb Mon Sep 17 00:00:00 2001 From: Oliver Calder Date: Mon, 2 Dec 2024 18:10:24 -0600 Subject: [PATCH 6/9] i/p/requestrules: add test case for add rule conflict with identical path pattern Signed-off-by: Oliver Calder --- interfaces/prompting/requestrules/requestrules_test.go | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/interfaces/prompting/requestrules/requestrules_test.go b/interfaces/prompting/requestrules/requestrules_test.go index 17401d41aa6..c17deaa576a 100644 --- a/interfaces/prompting/requestrules/requestrules_test.go +++ b/interfaces/prompting/requestrules/requestrules_test.go @@ -733,7 +733,7 @@ func (s *requestrulesSuite) TestAddRuleErrors(c *C) { &addRuleContents{Lifespan: prompting.LifespanSingle}, prompting_errors.NewRuleLifespanSingleError(prompting.SupportedRuleLifespans).Error(), }, - { // Conflicting rule + { // Conflicting rule with overlapping pattern variants &addRuleContents{ PathPattern: "/home/test/Pictures/**/*.{svg,jpg}", Permissions: []string{"read", "write"}, @@ -741,6 +741,13 @@ func (s *requestrulesSuite) TestAddRuleErrors(c *C) { }, fmt.Sprintf("cannot add rule: %v", prompting_errors.ErrRuleConflict), }, + { // Conflicting rule with identical path pattern + &addRuleContents{ + Permissions: []string{"read", "write"}, + Outcome: prompting.OutcomeDeny, + }, + fmt.Sprintf("cannot add rule: %v", prompting_errors.ErrRuleConflict), + }, } { result, err := addRuleFromTemplate(c, rdb, template, testCase.contents) c.Check(err, ErrorMatches, testCase.errStr) From 1377d0e43b42278e6a7559f7aa6301a99f10c977 Mon Sep 17 00:00:00 2001 From: Oliver Calder Date: Mon, 2 Dec 2024 21:26:24 -0600 Subject: [PATCH 7/9] i/p/requestrules: use new rule timestamp for merged rule and test merging Signed-off-by: Oliver Calder --- .../prompting/requestrules/requestrules.go | 10 +- .../requestrules/requestrules_test.go | 195 ++++++++++++++++++ 2 files changed, 200 insertions(+), 5 deletions(-) diff --git a/interfaces/prompting/requestrules/requestrules.go b/interfaces/prompting/requestrules/requestrules.go index cbc700bc56b..927e5864ea9 100644 --- a/interfaces/prompting/requestrules/requestrules.go +++ b/interfaces/prompting/requestrules/requestrules.go @@ -372,15 +372,13 @@ func (rdb *RuleDB) addOrMergeRule(rule *Rule, save bool) (addedOrMergedRule *Rul return rule, false, rdb.addNewRule(rule, save) } - currTime := time.Now() - // Check whether the existing rule has outcomes/lifespans which conflict, // and compile the new set of permissions. newPermissions := make(prompting.RulePermissionMap) var conflicts []prompting_errors.RuleConflict for perm, entry := range rule.Constraints.Permissions { existingEntry, exists := existingRule.Constraints.Permissions[perm] - if !exists || existingEntry.Expired(currTime) { + if !exists || existingEntry.Expired(rule.Timestamp) { newPermissions[perm] = entry continue } @@ -408,7 +406,7 @@ func (rdb *RuleDB) addOrMergeRule(rule *Rule, save bool) (addedOrMergedRule *Rul } // Add any non-expired permissions which were left over from the existing rule. for existingPerm, existingEntry := range existingRule.Constraints.Permissions { - if existingEntry.Expired(currTime) { + if existingEntry.Expired(rule.Timestamp) { continue } if _, exists := newPermissions[existingPerm]; exists { @@ -417,9 +415,11 @@ func (rdb *RuleDB) addOrMergeRule(rule *Rule, save bool) (addedOrMergedRule *Rul newPermissions[existingPerm] = existingEntry } - // Create new rule based on the contents of the existing rule + // Create new rule based on the contents of the existing rule, but copy the + // timestamp from the new rule. newRuleContents := *existingRule newRule := &newRuleContents + newRule.Timestamp = rule.Timestamp // Copy constraints as well, since copying the rule just copied the pointer newConstraints := *(existingRule.Constraints) newRule.Constraints = &newConstraints diff --git a/interfaces/prompting/requestrules/requestrules_test.go b/interfaces/prompting/requestrules/requestrules_test.go index c17deaa576a..c53ea57fe1a 100644 --- a/interfaces/prompting/requestrules/requestrules_test.go +++ b/interfaces/prompting/requestrules/requestrules_test.go @@ -864,6 +864,201 @@ outer: } } +func (s *requestrulesSuite) TestAddRuleMerges(c *C) { + for _, testCase := range []struct { + input []prompting.PermissionMap + output []prompting.PermissionMap + }{ + { + input: []prompting.PermissionMap{ + { + "read": &prompting.PermissionEntry{ + Outcome: prompting.OutcomeAllow, + Lifespan: prompting.LifespanForever, + }, + }, + { + "write": &prompting.PermissionEntry{ + Outcome: prompting.OutcomeDeny, + Lifespan: prompting.LifespanTimespan, + Duration: "10s", + }, + }, + }, + output: []prompting.PermissionMap{ + { + "read": &prompting.PermissionEntry{ + Outcome: prompting.OutcomeAllow, + Lifespan: prompting.LifespanForever, + }, + "write": &prompting.PermissionEntry{ + Outcome: prompting.OutcomeDeny, + Lifespan: prompting.LifespanTimespan, + Duration: "10s", + }, + }, + }, + }, + { + input: []prompting.PermissionMap{ + { + "read": &prompting.PermissionEntry{ + Outcome: prompting.OutcomeDeny, + Lifespan: prompting.LifespanForever, + }, + "write": &prompting.PermissionEntry{ + Outcome: prompting.OutcomeAllow, + Lifespan: prompting.LifespanTimespan, + Duration: "10s", + }, + "execute": &prompting.PermissionEntry{ + Outcome: prompting.OutcomeAllow, + Lifespan: prompting.LifespanTimespan, + Duration: "1s", + }, + }, + { + "read": &prompting.PermissionEntry{ + Outcome: prompting.OutcomeDeny, + Lifespan: prompting.LifespanTimespan, + Duration: "10s", + }, + "write": &prompting.PermissionEntry{ + Outcome: prompting.OutcomeAllow, + Lifespan: prompting.LifespanForever, + }, + }, + }, + output: []prompting.PermissionMap{ + { + "read": &prompting.PermissionEntry{ + Outcome: prompting.OutcomeDeny, + Lifespan: prompting.LifespanForever, + }, + "write": &prompting.PermissionEntry{ + Outcome: prompting.OutcomeAllow, + Lifespan: prompting.LifespanForever, + }, + "execute": &prompting.PermissionEntry{ + Outcome: prompting.OutcomeAllow, + Lifespan: prompting.LifespanTimespan, + Duration: "1s", + }, + }, + }, + }, + { + input: []prompting.PermissionMap{ + { + "read": &prompting.PermissionEntry{ + Outcome: prompting.OutcomeDeny, + Lifespan: prompting.LifespanTimespan, + Duration: "10s", + }, + "write": &prompting.PermissionEntry{ + Outcome: prompting.OutcomeAllow, + Lifespan: prompting.LifespanTimespan, + Duration: "10s", + }, + "execute": &prompting.PermissionEntry{ + Outcome: prompting.OutcomeAllow, + Lifespan: prompting.LifespanTimespan, + Duration: "1ns", // Will expire and be dropped + }, + }, + { + "read": &prompting.PermissionEntry{ + Outcome: prompting.OutcomeDeny, + Lifespan: prompting.LifespanTimespan, + Duration: "20s", + }, + "write": &prompting.PermissionEntry{ + Outcome: prompting.OutcomeAllow, + Lifespan: prompting.LifespanForever, + }, + }, + }, + output: []prompting.PermissionMap{ + { + "read": &prompting.PermissionEntry{ + Outcome: prompting.OutcomeDeny, + Lifespan: prompting.LifespanTimespan, + Duration: "20s", + }, + "write": &prompting.PermissionEntry{ + Outcome: prompting.OutcomeAllow, + Lifespan: prompting.LifespanForever, + }, + }, + }, + }, + } { + // Set root so rule creation does not interfere between test cases + dirs.SetRootDir(c.MkDir()) + s.AddCleanup(func() { dirs.SetRootDir("") }) + c.Assert(os.MkdirAll(dirs.SnapdStateDir(dirs.GlobalRootDir), 0o700), IsNil) + + rdb, err := requestrules.New(s.defaultNotifyRule) + c.Assert(err, IsNil) + + user := s.defaultUser + snap := "firefox" + iface := "home" + pathPattern := mustParsePathPattern(c, "/path/to/foo/ba{r,z}/**") + + // Add all the rules + for _, perms := range testCase.input { + constraints := &prompting.Constraints{ + PathPattern: pathPattern, + Permissions: perms, + } + _, err = rdb.AddRule(user, snap, iface, constraints) + c.Assert(err, IsNil, Commentf("\ntestCase: %+v\nperms: %+v", testCase, perms)) + } + + rules := rdb.Rules(s.defaultUser) + c.Check(rules, HasLen, len(testCase.output), Commentf("\ntestCase: %+v\nrules: %+v", testCase, rules)) + for i, perms := range testCase.output { + // Build RuleConstraints based on output perms using the timestamp + // of the corresponding rule. + rule := rules[i] + constraints := &prompting.Constraints{ + PathPattern: pathPattern, + Permissions: perms, + } + ruleConstraints, err := constraints.ToRuleConstraints(iface, rule.Timestamp) + c.Assert(err, IsNil) + expectedPerms := ruleConstraints.Permissions + // Check that the permissions match what is expected. + // Other parameters should be trivially identical. + // Need to be careful because timestamps aren't "DeepEqual", so + // first set equivalent timestamps equal to each other. + for perm, entry := range rule.Constraints.Permissions { + expectedEntry, exists := expectedPerms[perm] + c.Assert(exists, Equals, true, Commentf("\ntestCase: %+v\nrules: %+v\npermission not found: %s", testCase, rules, perm)) + c.Check(entry.Lifespan, Equals, expectedEntry.Lifespan, Commentf("\ntestCase: %+v\nrules: %+v\nlifespans not equal: %v != %v", testCase, rules, entry.Lifespan, expectedEntry.Lifespan)) + // Expiration will be duration after the timestamp of one of + // the created rules, but it may not be the final one which was + // merged into the resulting rule, so we don't actually have an + // absolute timestamp with which we can compute an expiration + // using the duration. So subtract the timestamps and check + // that the difference is less than 100ms. We'll always have + // deltas of 1s for time differences we care about. + difference := entry.Expiration.Sub(expectedEntry.Expiration) + // TODO: call Abs() once we're on Go 1.19+ + if difference < 0 { + difference *= -1 + } + c.Check(difference < 100*time.Millisecond, Equals, true, Commentf("\ntestCase: %+v\nrules: %+v\nexpirations not within 100ms: %v != %v", testCase, rules, entry.Expiration, expectedEntry.Expiration)) + expectedEntry.Expiration = entry.Expiration + } + c.Check(rule.Constraints.Permissions, DeepEquals, expectedPerms) + } + + c.Assert(rdb.Close(), IsNil) + } +} + func (s *requestrulesSuite) TestAddRuleExpired(c *C) { rdb, err := requestrules.New(s.defaultNotifyRule) c.Assert(err, IsNil) From 116ba5655e524dac5d24ce94249274398d63295a Mon Sep 17 00:00:00 2001 From: Oliver Calder Date: Tue, 3 Dec 2024 15:32:37 -0600 Subject: [PATCH 8/9] i/p/requestrules: add test for merge behavior when loading rules from disk Signed-off-by: Oliver Calder --- .../requestrules/requestrules_test.go | 209 ++++++++++++++++++ 1 file changed, 209 insertions(+) diff --git a/interfaces/prompting/requestrules/requestrules_test.go b/interfaces/prompting/requestrules/requestrules_test.go index c53ea57fe1a..c094c77317f 100644 --- a/interfaces/prompting/requestrules/requestrules_test.go +++ b/interfaces/prompting/requestrules/requestrules_test.go @@ -393,6 +393,213 @@ func (s *requestrulesSuite) TestLoadExpiredRules(c *C) { s.checkNewNotices(c, expectedNoticeInfo) } +func (s *requestrulesSuite) TestLoadMergedRules(c *C) { + dbPath := s.prepDBPath(c) + + good1 := s.ruleTemplateWithReadPathPattern(c, prompting.IDType(1), "/home/test/{foo,bar}") + identical1 := s.ruleTemplateWithReadPathPattern(c, prompting.IDType(2), "/home/test/{foo,bar}") + expected1 := good1 + expected1.Timestamp = identical1.Timestamp // Timestamp should be the second timestamp + + // Rules with identical pattern but non-overlapping permissions do not conflict + good2 := s.ruleTemplateWithPathPattern(c, prompting.IDType(3), "/home/test/something") + good2.Constraints.Permissions = prompting.RulePermissionMap{ + "read": &prompting.RulePermissionEntry{ + Outcome: prompting.OutcomeAllow, + Lifespan: prompting.LifespanForever, + }, + } + nonOverlap2 := s.ruleTemplateWithPathPattern(c, prompting.IDType(4), "/home/test/something") + nonOverlap2.Constraints.Permissions = prompting.RulePermissionMap{ + "write": &prompting.RulePermissionEntry{ + Outcome: prompting.OutcomeDeny, + Lifespan: prompting.LifespanTimespan, + Expiration: nonOverlap2.Timestamp.Add(10 * time.Second), + }, + } + expected2 := s.ruleTemplateWithPathPattern(c, prompting.IDType(3), "/home/test/something") + expected2.Timestamp = nonOverlap2.Timestamp // Timestamp should be the second timestamp + expected2.Constraints.Permissions = prompting.RulePermissionMap{ + "read": &prompting.RulePermissionEntry{ + Outcome: prompting.OutcomeAllow, + Lifespan: prompting.LifespanForever, + }, + "write": &prompting.RulePermissionEntry{ + Outcome: prompting.OutcomeDeny, + Lifespan: prompting.LifespanTimespan, + // Expiration should be based on nonOverlap2 timestamp + Expiration: nonOverlap2.Timestamp.Add(10 * time.Second), + }, + } + + // Rules which overlap but don't conflict preserve longer lifespan + good3 := s.ruleTemplateWithPathPattern(c, prompting.IDType(5), "/home/test/another") + good3.Constraints.Permissions = prompting.RulePermissionMap{ + "read": &prompting.RulePermissionEntry{ + Outcome: prompting.OutcomeDeny, + Lifespan: prompting.LifespanForever, + }, + "write": &prompting.RulePermissionEntry{ + Outcome: prompting.OutcomeAllow, + Lifespan: prompting.LifespanTimespan, + Expiration: good3.Timestamp.Add(10 * time.Second), + }, + "execute": &prompting.RulePermissionEntry{ + Outcome: prompting.OutcomeAllow, + Lifespan: prompting.LifespanTimespan, + Expiration: good3.Timestamp.Add(time.Second), + }, + } + overlap3 := s.ruleTemplateWithPathPattern(c, prompting.IDType(6), "/home/test/another") + overlap3.Constraints.Permissions = prompting.RulePermissionMap{ + "read": &prompting.RulePermissionEntry{ + Outcome: prompting.OutcomeDeny, + Lifespan: prompting.LifespanTimespan, + Expiration: overlap3.Timestamp.Add(10 * time.Second), + }, + "write": &prompting.RulePermissionEntry{ + Outcome: prompting.OutcomeAllow, + Lifespan: prompting.LifespanForever, + }, + } + expected3 := s.ruleTemplateWithPathPattern(c, prompting.IDType(5), "/home/test/another") + expected3.Timestamp = overlap3.Timestamp // Timestamp should be the second timestamp + expected3.Constraints.Permissions = prompting.RulePermissionMap{ + "read": &prompting.RulePermissionEntry{ + Outcome: prompting.OutcomeDeny, + Lifespan: prompting.LifespanForever, + }, + "write": &prompting.RulePermissionEntry{ + Outcome: prompting.OutcomeAllow, + Lifespan: prompting.LifespanForever, + }, + "execute": &prompting.RulePermissionEntry{ + Outcome: prompting.OutcomeAllow, + Lifespan: prompting.LifespanTimespan, + // Expiration should be based on good3 timestamp + Expiration: good3.Timestamp.Add(time.Second), + }, + } + + // Rules which overlap but don't conflict preserve longer lifespan, and + // will be merged into existing rule even if that rule is completely + // superseded. + good4 := s.ruleTemplateWithPathPattern(c, prompting.IDType(7), "/home/test/one/more") + good4.Constraints.Permissions = prompting.RulePermissionMap{ + "read": &prompting.RulePermissionEntry{ + Outcome: prompting.OutcomeDeny, + Lifespan: prompting.LifespanTimespan, + Expiration: good4.Timestamp.Add(10 * time.Second), + }, + "write": &prompting.RulePermissionEntry{ + Outcome: prompting.OutcomeAllow, + Lifespan: prompting.LifespanTimespan, + Expiration: good4.Timestamp.Add(10 * time.Second), + }, + "execute": &prompting.RulePermissionEntry{ + Outcome: prompting.OutcomeAllow, + Lifespan: prompting.LifespanTimespan, + Expiration: good4.Timestamp.Add(time.Nanosecond), // will expire + }, + } + overlap4 := s.ruleTemplateWithPathPattern(c, prompting.IDType(8), "/home/test/one/more") + overlap4.Constraints.Permissions = prompting.RulePermissionMap{ + "read": &prompting.RulePermissionEntry{ + Outcome: prompting.OutcomeDeny, + Lifespan: prompting.LifespanTimespan, + Expiration: overlap4.Timestamp.Add(20 * time.Second), + }, + "write": &prompting.RulePermissionEntry{ + Outcome: prompting.OutcomeAllow, + Lifespan: prompting.LifespanForever, + }, + } + expected4 := s.ruleTemplateWithPathPattern(c, prompting.IDType(7), "/home/test/one/more") + expected4.Timestamp = overlap4.Timestamp // Timestamp should be the second timestamp + expected4.Constraints.Permissions = prompting.RulePermissionMap{ + "read": &prompting.RulePermissionEntry{ + Outcome: prompting.OutcomeDeny, + Lifespan: prompting.LifespanTimespan, + // Expiration should be based on overlap4 timestamp + Expiration: overlap4.Timestamp.Add(20 * time.Second), + }, + "write": &prompting.RulePermissionEntry{ + Outcome: prompting.OutcomeAllow, + Lifespan: prompting.LifespanForever, + }, + } + + rules := []*requestrules.Rule{good1, identical1, good2, nonOverlap2, good3, overlap3, good4, overlap4} + s.writeRules(c, dbPath, rules) + + logbuf, restore := logger.MockLogger() + defer restore() + rdb, err := requestrules.New(s.defaultNotifyRule) + c.Check(err, IsNil) + c.Check(rdb, NotNil) + // Check that no error was logged + c.Check(logbuf.String(), HasLen, 0) + + expectedWrittenRules := []*requestrules.Rule{expected1, expected2, expected3, expected4} + s.checkWrittenRuleDB(c, expectedWrittenRules) + + expectedNoticeInfo := []*noticeInfo{ + { + userID: good1.User, + ruleID: good1.ID, + data: nil, + }, + { + userID: identical1.User, + ruleID: identical1.ID, + data: map[string]string{ + "removed": "merged", + "merged-into": good1.ID.String(), + }, + }, + { + userID: good2.User, + ruleID: good2.ID, + data: nil, + }, + { + userID: nonOverlap2.User, + ruleID: nonOverlap2.ID, + data: map[string]string{ + "removed": "merged", + "merged-into": good2.ID.String(), + }, + }, + { + userID: good3.User, + ruleID: good3.ID, + data: nil, + }, + { + userID: overlap3.User, + ruleID: overlap3.ID, + data: map[string]string{ + "removed": "merged", + "merged-into": good3.ID.String(), + }, + }, + { + userID: good4.User, + ruleID: good4.ID, + data: nil, + }, + { + userID: overlap4.User, + ruleID: overlap4.ID, + data: map[string]string{ + "removed": "merged", + "merged-into": good4.ID.String(), + }, + }, + } + s.checkNewNotices(c, expectedNoticeInfo) +} + func (s *requestrulesSuite) TestLoadHappy(c *C) { dbPath := s.prepDBPath(c) @@ -948,6 +1155,8 @@ func (s *requestrulesSuite) TestAddRuleMerges(c *C) { }, }, { + // First rule is entirely superseded by the latter, but still use + // the ID of the former in the merged rule. input: []prompting.PermissionMap{ { "read": &prompting.PermissionEntry{ From 76cf4f37bf16f10dafb7e4d1a121048f6f89e1e6 Mon Sep 17 00:00:00 2001 From: Oliver Calder Date: Tue, 3 Dec 2024 16:17:33 -0600 Subject: [PATCH 9/9] i/p/requestrules: add test cases for partially- and fully-expired conflicing merged rules Signed-off-by: Oliver Calder --- .../requestrules/requestrules_test.go | 97 +++++++++++++++++++ 1 file changed, 97 insertions(+) diff --git a/interfaces/prompting/requestrules/requestrules_test.go b/interfaces/prompting/requestrules/requestrules_test.go index c094c77317f..db08ebc409b 100644 --- a/interfaces/prompting/requestrules/requestrules_test.go +++ b/interfaces/prompting/requestrules/requestrules_test.go @@ -1201,6 +1201,103 @@ func (s *requestrulesSuite) TestAddRuleMerges(c *C) { }, }, }, + { + // First rule is fully expired but would otherwise conflict with + // the second rule. + input: []prompting.PermissionMap{ + { + "read": &prompting.PermissionEntry{ + Outcome: prompting.OutcomeDeny, + Lifespan: prompting.LifespanTimespan, + Duration: "1ns", // Will expire and not conflict + }, + "write": &prompting.PermissionEntry{ + Outcome: prompting.OutcomeAllow, + Lifespan: prompting.LifespanTimespan, + Duration: "1ns", // Will expire and not conflict + }, + }, + { + "read": &prompting.PermissionEntry{ + Outcome: prompting.OutcomeAllow, + Lifespan: prompting.LifespanTimespan, + Duration: "10s", + }, + "write": &prompting.PermissionEntry{ + Outcome: prompting.OutcomeDeny, + Lifespan: prompting.LifespanForever, + }, + }, + }, + output: []prompting.PermissionMap{ + { + "read": &prompting.PermissionEntry{ + Outcome: prompting.OutcomeAllow, + Lifespan: prompting.LifespanTimespan, + Duration: "10s", + }, + "write": &prompting.PermissionEntry{ + Outcome: prompting.OutcomeDeny, + Lifespan: prompting.LifespanForever, + }, + }, + }, + }, + { + // First rule is partially expired but would otherwise conflict + // with the second rule. + input: []prompting.PermissionMap{ + { + "read": &prompting.PermissionEntry{ + Outcome: prompting.OutcomeDeny, + Lifespan: prompting.LifespanTimespan, + Duration: "1ns", // Will expire and not conflict + }, + "write": &prompting.PermissionEntry{ + Outcome: prompting.OutcomeDeny, + Lifespan: prompting.LifespanTimespan, + Duration: "10s", + }, + "execute": &prompting.PermissionEntry{ + Outcome: prompting.OutcomeAllow, + Lifespan: prompting.LifespanForever, + }, + }, + { + "read": &prompting.PermissionEntry{ + Outcome: prompting.OutcomeAllow, + Lifespan: prompting.LifespanTimespan, + Duration: "10s", + }, + "write": &prompting.PermissionEntry{ + Outcome: prompting.OutcomeDeny, + Lifespan: prompting.LifespanForever, + }, + "execute": &prompting.PermissionEntry{ + Outcome: prompting.OutcomeAllow, + Lifespan: prompting.LifespanTimespan, + Duration: "10s", + }, + }, + }, + output: []prompting.PermissionMap{ + { + "read": &prompting.PermissionEntry{ + Outcome: prompting.OutcomeAllow, + Lifespan: prompting.LifespanTimespan, + Duration: "10s", + }, + "write": &prompting.PermissionEntry{ + Outcome: prompting.OutcomeDeny, + Lifespan: prompting.LifespanForever, + }, + "execute": &prompting.PermissionEntry{ + Outcome: prompting.OutcomeAllow, + Lifespan: prompting.LifespanForever, + }, + }, + }, + }, } { // Set root so rule creation does not interfere between test cases dirs.SetRootDir(c.MkDir())