diff --git a/daemon/api_prompting.go b/daemon/api_prompting.go index cde77f6a20b..cfba4bb2619 100644 --- a/daemon/api_prompting.go +++ b/daemon/api_prompting.go @@ -283,19 +283,16 @@ var getInterfaceManager = func(c *Command) interfaceManager { } type postPromptBody struct { - Outcome prompting.OutcomeType `json:"action"` - Lifespan prompting.LifespanType `json:"lifespan"` - Duration string `json:"duration,omitempty"` - Constraints *prompting.Constraints `json:"constraints"` + Outcome prompting.OutcomeType `json:"action"` + Lifespan prompting.LifespanType `json:"lifespan"` + Duration string `json:"duration,omitempty"` + Constraints *prompting.ReplyConstraints `json:"constraints"` } type addRuleContents struct { Snap string `json:"snap"` Interface string `json:"interface"` Constraints *prompting.Constraints `json:"constraints"` - Outcome prompting.OutcomeType `json:"outcome"` - Lifespan prompting.LifespanType `json:"lifespan"` - Duration string `json:"duration,omitempty"` } type removeRulesSelector struct { @@ -304,10 +301,7 @@ type removeRulesSelector struct { } type patchRuleContents struct { - Constraints *prompting.Constraints `json:"constraints,omitempty"` - Outcome prompting.OutcomeType `json:"outcome,omitempty"` - Lifespan prompting.LifespanType `json:"lifespan,omitempty"` - Duration string `json:"duration,omitempty"` + Constraints *prompting.PatchConstraints `json:"constraints,omitempty"` } type postRulesRequestBody struct { @@ -452,7 +446,7 @@ func postRules(c *Command, r *http.Request, user *auth.UserState) Response { if postBody.AddRule == nil { return BadRequest(`must include "rule" field in request body when action is "add"`) } - newRule, err := getInterfaceManager(c).InterfacesRequestsManager().AddRule(userID, postBody.AddRule.Snap, postBody.AddRule.Interface, postBody.AddRule.Constraints, postBody.AddRule.Outcome, postBody.AddRule.Lifespan, postBody.AddRule.Duration) + newRule, err := getInterfaceManager(c).InterfacesRequestsManager().AddRule(userID, postBody.AddRule.Snap, postBody.AddRule.Interface, postBody.AddRule.Constraints) if err != nil { return promptingError(err) } @@ -529,7 +523,7 @@ func postRule(c *Command, r *http.Request, user *auth.UserState) Response { if postBody.PatchRule == nil { return BadRequest(`must include "rule" field in request body when action is "patch"`) } - patchedRule, err := getInterfaceManager(c).InterfacesRequestsManager().PatchRule(userID, ruleID, postBody.PatchRule.Constraints, postBody.PatchRule.Outcome, postBody.PatchRule.Lifespan, postBody.PatchRule.Duration) + patchedRule, err := getInterfaceManager(c).InterfacesRequestsManager().PatchRule(userID, ruleID, postBody.PatchRule.Constraints) if err != nil { return promptingError(err) } diff --git a/daemon/api_prompting_test.go b/daemon/api_prompting_test.go index 4e7a5d634cc..fbe751d8cb4 100644 --- a/daemon/api_prompting_test.go +++ b/daemon/api_prompting_test.go @@ -52,14 +52,16 @@ type fakeInterfacesRequestsManager struct { err error // Store most recent received values - userID uint32 - snap string - iface string - id prompting.IDType // used for prompt ID or rule ID - constraints *prompting.Constraints - outcome prompting.OutcomeType - lifespan prompting.LifespanType - duration string + userID uint32 + snap string + iface string + id prompting.IDType // used for prompt ID or rule ID + ruleConstraints *prompting.Constraints + patchConstraints *prompting.PatchConstraints + replyConstraints *prompting.ReplyConstraints + outcome prompting.OutcomeType + lifespan prompting.LifespanType + duration string } func (m *fakeInterfacesRequestsManager) Prompts(userID uint32) ([]*requestprompts.Prompt, error) { @@ -73,10 +75,10 @@ func (m *fakeInterfacesRequestsManager) PromptWithID(userID uint32, promptID pro return m.prompt, m.err } -func (m *fakeInterfacesRequestsManager) HandleReply(userID uint32, promptID prompting.IDType, constraints *prompting.Constraints, outcome prompting.OutcomeType, lifespan prompting.LifespanType, duration string) ([]prompting.IDType, error) { +func (m *fakeInterfacesRequestsManager) HandleReply(userID uint32, promptID prompting.IDType, constraints *prompting.ReplyConstraints, outcome prompting.OutcomeType, lifespan prompting.LifespanType, duration string) ([]prompting.IDType, error) { m.userID = userID m.id = promptID - m.constraints = constraints + m.replyConstraints = constraints m.outcome = outcome m.lifespan = lifespan m.duration = duration @@ -90,14 +92,11 @@ func (m *fakeInterfacesRequestsManager) Rules(userID uint32, snap string, iface return m.rules, m.err } -func (m *fakeInterfacesRequestsManager) AddRule(userID uint32, snap string, iface string, constraints *prompting.Constraints, outcome prompting.OutcomeType, lifespan prompting.LifespanType, duration string) (*requestrules.Rule, error) { +func (m *fakeInterfacesRequestsManager) AddRule(userID uint32, snap string, iface string, constraints *prompting.Constraints) (*requestrules.Rule, error) { m.userID = userID m.snap = snap m.iface = iface - m.constraints = constraints - m.outcome = outcome - m.lifespan = lifespan - m.duration = duration + m.ruleConstraints = constraints return m.rule, m.err } @@ -114,13 +113,10 @@ func (m *fakeInterfacesRequestsManager) RuleWithID(userID uint32, ruleID prompti return m.rule, m.err } -func (m *fakeInterfacesRequestsManager) PatchRule(userID uint32, ruleID prompting.IDType, constraints *prompting.Constraints, outcome prompting.OutcomeType, lifespan prompting.LifespanType, duration string) (*requestrules.Rule, error) { +func (m *fakeInterfacesRequestsManager) PatchRule(userID uint32, ruleID prompting.IDType, constraints *prompting.PatchConstraints) (*requestrules.Rule, error) { m.userID = userID m.id = ruleID - m.constraints = constraints - m.outcome = outcome - m.lifespan = lifespan - m.duration = duration + m.patchConstraints = constraints return m.rule, m.err } @@ -700,7 +696,7 @@ func (s *promptingSuite) TestPostPromptHappy(c *C) { prompting.IDType(0xF00BA4), } - constraints := &prompting.Constraints{ + constraints := &prompting.ReplyConstraints{ PathPattern: mustParsePathPattern(c, "/home/test/Pictures/**/*.{png,svg}"), Permissions: []string{"read", "execute"}, } @@ -718,7 +714,7 @@ func (s *promptingSuite) TestPostPromptHappy(c *C) { // Check parameters c.Check(s.manager.userID, Equals, uint32(1000)) c.Check(s.manager.id, Equals, prompting.IDType(0x0123456789abcdef)) - c.Check(s.manager.constraints, DeepEquals, contents.Constraints) + c.Check(s.manager.replyConstraints, DeepEquals, contents.Constraints) c.Check(s.manager.outcome, Equals, contents.Outcome) c.Check(s.manager.lifespan, Equals, contents.Lifespan) c.Check(s.manager.duration, Equals, contents.Duration) @@ -775,13 +771,15 @@ func (s *promptingSuite) TestGetRulesHappy(c *C) { User: 1234, Snap: "firefox", Interface: "home", - Constraints: &prompting.Constraints{ + Constraints: &prompting.RuleConstraints{ PathPattern: mustParsePathPattern(c, "/foo/bar"), - Permissions: []string{"write"}, + Permissions: prompting.RulePermissionMap{ + "write": &prompting.RulePermissionEntry{ + Outcome: prompting.OutcomeDeny, + Lifespan: prompting.LifespanForever, + }, + }, }, - Outcome: prompting.OutcomeDeny, - Lifespan: prompting.LifespanForever, - Expiration: time.Now(), }, } @@ -810,26 +808,34 @@ func (s *promptingSuite) TestPostRulesAddHappy(c *C) { User: 11235, Snap: "firefox", Interface: "home", - Constraints: &prompting.Constraints{ + Constraints: &prompting.RuleConstraints{ PathPattern: mustParsePathPattern(c, "/foo/bar/baz"), - Permissions: []string{"write"}, + Permissions: prompting.RulePermissionMap{ + "write": &prompting.RulePermissionEntry{ + Outcome: prompting.OutcomeDeny, + Lifespan: prompting.LifespanForever, + }, + }, }, - Outcome: prompting.OutcomeDeny, - Lifespan: prompting.LifespanForever, - Expiration: time.Now(), } constraints := &prompting.Constraints{ PathPattern: mustParsePathPattern(c, "/home/test/{foo,bar,baz}/**/*.{png,svg}"), - Permissions: []string{"read", "write"}, + Permissions: prompting.PermissionMap{ + "read": &prompting.PermissionEntry{ + Outcome: prompting.OutcomeAllow, + Lifespan: prompting.LifespanForever, + }, + "write": &prompting.PermissionEntry{ + Outcome: prompting.OutcomeAllow, + Lifespan: prompting.LifespanForever, + }, + }, } contents := &daemon.AddRuleContents{ Snap: "thunderbird", Interface: "home", Constraints: constraints, - Outcome: prompting.OutcomeAllow, - Lifespan: prompting.LifespanForever, - Duration: "", } postBody := &daemon.PostRulesRequestBody{ Action: "add", @@ -844,10 +850,7 @@ func (s *promptingSuite) TestPostRulesAddHappy(c *C) { c.Check(s.manager.userID, Equals, uint32(11235)) c.Check(s.manager.snap, Equals, contents.Snap) c.Check(s.manager.iface, Equals, contents.Interface) - c.Check(s.manager.constraints, DeepEquals, contents.Constraints) - c.Check(s.manager.outcome, Equals, contents.Outcome) - c.Check(s.manager.lifespan, Equals, contents.Lifespan) - c.Check(s.manager.duration, Equals, contents.Duration) + c.Check(s.manager.ruleConstraints, DeepEquals, contents.Constraints) // Check return value rule, ok := rsp.Result.(*requestrules.Rule) @@ -881,7 +884,6 @@ func (s *promptingSuite) TestPostRulesRemoveHappy(c *C) { s.manager = &fakeInterfacesRequestsManager{} // Set the rules to return - var timeZero time.Time s.manager.rules = []*requestrules.Rule{ { ID: prompting.IDType(1234), @@ -889,13 +891,15 @@ func (s *promptingSuite) TestPostRulesRemoveHappy(c *C) { User: 1001, Snap: "thunderird", Interface: "home", - Constraints: &prompting.Constraints{ + Constraints: &prompting.RuleConstraints{ PathPattern: mustParsePathPattern(c, "/foo/bar/baz/qux"), - Permissions: []string{"write"}, + Permissions: prompting.RulePermissionMap{ + "write": &prompting.RulePermissionEntry{ + Outcome: prompting.OutcomeDeny, + Lifespan: prompting.LifespanForever, + }, + }, }, - Outcome: prompting.OutcomeDeny, - Lifespan: prompting.LifespanForever, - Expiration: timeZero, }, { ID: prompting.IDType(5678), @@ -903,13 +907,19 @@ func (s *promptingSuite) TestPostRulesRemoveHappy(c *C) { User: 1001, Snap: "thunderbird", Interface: "home", - Constraints: &prompting.Constraints{ + Constraints: &prompting.RuleConstraints{ PathPattern: mustParsePathPattern(c, "/fizz/buzz"), - Permissions: []string{"read", "execute"}, + Permissions: prompting.RulePermissionMap{ + "read": &prompting.RulePermissionEntry{ + Outcome: prompting.OutcomeAllow, + Lifespan: prompting.LifespanTimespan, + }, + "execute": &prompting.RulePermissionEntry{ + Outcome: prompting.OutcomeAllow, + Lifespan: prompting.LifespanTimespan, + }, + }, }, - Outcome: prompting.OutcomeAllow, - Lifespan: prompting.LifespanTimespan, - Expiration: time.Now(), }, } @@ -948,13 +958,16 @@ func (s *promptingSuite) TestGetRuleHappy(c *C) { User: 1005, Snap: "thunderbird", Interface: "home", - Constraints: &prompting.Constraints{ + Constraints: &prompting.RuleConstraints{ PathPattern: mustParsePathPattern(c, "/home/test/Videos/**/*.{mkv,mp4,mov}"), - Permissions: []string{"read"}, + Permissions: prompting.RulePermissionMap{ + "read": &prompting.RulePermissionEntry{ + Outcome: prompting.OutcomeAllow, + Lifespan: prompting.LifespanTimespan, + Expiration: time.Now().Add(-24 * time.Hour), + }, + }, }, - Outcome: prompting.OutcomeAllow, - Lifespan: prompting.LifespanTimespan, - Expiration: time.Now().Add(-24 * time.Hour), } rsp := s.makeSyncReq(c, "GET", "/v2/interfaces/requests/rules/000000000000012B", 1005, nil) @@ -974,31 +987,42 @@ func (s *promptingSuite) TestPostRulePatchHappy(c *C) { s.daemon(c) - var timeZero time.Time s.manager.rule = &requestrules.Rule{ ID: prompting.IDType(0x01123581321), Timestamp: time.Now(), User: 999, Snap: "gimp", Interface: "home", - Constraints: &prompting.Constraints{ + Constraints: &prompting.RuleConstraints{ PathPattern: mustParsePathPattern(c, "/home/test/Pictures/**/*.{png,jpg}"), - Permissions: []string{"read", "write"}, + Permissions: prompting.RulePermissionMap{ + "read": &prompting.RulePermissionEntry{ + Outcome: prompting.OutcomeAllow, + Lifespan: prompting.LifespanForever, + }, + "write": &prompting.RulePermissionEntry{ + Outcome: prompting.OutcomeAllow, + Lifespan: prompting.LifespanForever, + }, + }, }, - Outcome: prompting.OutcomeAllow, - Lifespan: prompting.LifespanForever, - Expiration: timeZero, } - constraints := &prompting.Constraints{ + constraints := &prompting.PatchConstraints{ PathPattern: mustParsePathPattern(c, "/home/test/Pictures/**/*.{png,jpg}"), - Permissions: []string{"read", "write"}, + Permissions: prompting.PermissionMap{ + "read": &prompting.PermissionEntry{ + Outcome: prompting.OutcomeAllow, + Lifespan: prompting.LifespanForever, + }, + "write": &prompting.PermissionEntry{ + Outcome: prompting.OutcomeAllow, + Lifespan: prompting.LifespanForever, + }, + }, } contents := &daemon.PatchRuleContents{ Constraints: constraints, - Outcome: prompting.OutcomeAllow, - Lifespan: prompting.LifespanForever, - Duration: "", } postBody := &daemon.PostRuleRequestBody{ Action: "patch", @@ -1011,10 +1035,7 @@ func (s *promptingSuite) TestPostRulePatchHappy(c *C) { // Check parameters c.Check(s.manager.userID, Equals, uint32(999)) - c.Check(s.manager.constraints, DeepEquals, contents.Constraints) - c.Check(s.manager.outcome, Equals, contents.Outcome) - c.Check(s.manager.lifespan, Equals, contents.Lifespan) - c.Check(s.manager.duration, Equals, contents.Duration) + c.Check(s.manager.patchConstraints, DeepEquals, contents.Constraints) // Check return value rule, ok := rsp.Result.(*requestrules.Rule) @@ -1027,20 +1048,25 @@ func (s *promptingSuite) TestPostRuleRemoveHappy(c *C) { s.daemon(c) - var timeZero time.Time s.manager.rule = &requestrules.Rule{ ID: prompting.IDType(0x01123581321), Timestamp: time.Now(), User: 100, Snap: "gimp", Interface: "home", - Constraints: &prompting.Constraints{ + Constraints: &prompting.RuleConstraints{ PathPattern: mustParsePathPattern(c, "/home/test/Pictures/**/*.{png,jpg}"), - Permissions: []string{"read", "write"}, + Permissions: prompting.RulePermissionMap{ + "read": &prompting.RulePermissionEntry{ + Outcome: prompting.OutcomeAllow, + Lifespan: prompting.LifespanForever, + }, + "write": &prompting.RulePermissionEntry{ + Outcome: prompting.OutcomeAllow, + Lifespan: prompting.LifespanForever, + }, + }, }, - Outcome: prompting.OutcomeAllow, - Lifespan: prompting.LifespanForever, - Expiration: timeZero, } postBody := &daemon.PostRuleRequestBody{ Action: "remove", diff --git a/interfaces/prompting/constraints.go b/interfaces/prompting/constraints.go index b07c04a039d..d90c0954fb1 100644 --- a/interfaces/prompting/constraints.go +++ b/interfaces/prompting/constraints.go @@ -22,6 +22,7 @@ package prompting import ( "fmt" "sort" + "time" prompting_errors "github.com/snapcore/snapd/interfaces/prompting/errors" "github.com/snapcore/snapd/interfaces/prompting/patterns" @@ -30,62 +31,128 @@ import ( "github.com/snapcore/snapd/strutil" ) -// Constraints hold information about the applicability of a rule to particular -// paths or permissions. A request matches the constraints if the requested path -// is matched by the path pattern (according to bash's globstar matching) and -// the requested permissions are contained in the constraints' permissions. +// Constraints hold information about the applicability of a new rule to +// particular paths or permissions. A request will be matched by the constraints +// if the requested path is matched by the path pattern (according to bash's +// globstar matching) and one or more requested permissions are denied in the +// permission map, or all of the requested permissions are allowed in the map. +// When snapd creates a new rule, it converts Constraints to RuleConstraints. type Constraints struct { - PathPattern *patterns.PathPattern `json:"path-pattern,omitempty"` - Permissions []string `json:"permissions,omitempty"` + PathPattern *patterns.PathPattern `json:"path-pattern"` + Permissions PermissionMap `json:"permissions"` } -// ValidateForInterface returns nil if the constraints are valid for the given -// interface, otherwise returns an error. -func (c *Constraints) ValidateForInterface(iface string) error { +// ToRuleConstraints validates the receiving Constraints and converts it to +// RuleConstraints. If the constraints are not valid with respect to the given +// interface, returns an error. +func (c *Constraints) ToRuleConstraints(iface string, currTime time.Time) (*RuleConstraints, error) { + if c.PathPattern == nil { + return nil, prompting_errors.NewInvalidPathPatternError("", "no path pattern") + } + rulePermissions, err := c.Permissions.toRulePermissionMap(iface, currTime) + if err != nil { + return nil, err + } + ruleConstraints := &RuleConstraints{ + PathPattern: c.PathPattern, + Permissions: rulePermissions, + } + return ruleConstraints, nil +} + +// RuleConstraints hold information about the applicability of an existing rule +// to particular paths or permissions. A request will be matched by the rule +// constraints if the requested path is matched by the path pattern (according +// to bash's globstar matching) and one or more requested permissions are denied +// in the permission map, or all of the requested permissions are allowed in the +// map. +type RuleConstraints struct { + PathPattern *patterns.PathPattern `json:"path-pattern"` + Permissions RulePermissionMap `json:"permissions"` +} + +// ValidateForInterface checks that the rule constraints are valid for the given +// interface. Any permissions which have expired relative to the given current +// time are pruned. If all permissions have expired, then returns true. If the +// rule is invalid, returns an error. +func (c *RuleConstraints) ValidateForInterface(iface string, currTime time.Time) (expired bool, err error) { + if c.PathPattern == nil { + return false, prompting_errors.NewInvalidPathPatternError("", "no path pattern") + } + return c.Permissions.validateForInterface(iface, currTime) +} + +// Match returns true if the constraints match the given path, otherwise false. +// +// If the constraints or path are invalid, returns an error. +func (c *RuleConstraints) Match(path string) (bool, error) { if c.PathPattern == nil { - return prompting_errors.NewInvalidPathPatternError("", "no path pattern") + return false, prompting_errors.NewInvalidPathPatternError("", "no path pattern") } - return c.validatePermissions(iface) + match, err := c.PathPattern.Match(path) + if err != nil { + // Error should not occur, since it was parsed internally + return false, prompting_errors.NewInvalidPathPatternError(c.PathPattern.String(), err.Error()) + } + return match, nil } -// validatePermissions checks that the permissions for the given constraints -// are valid for the given interface. If not, returns an error, otherwise -// ensures that the permissions are in the order in which they occur in the -// list of available permissions for that interface. -func (c *Constraints) validatePermissions(iface string) error { +// ReplyConstraints hold information about the applicability of a reply to +// particular paths or permissions. Upon receiving the reply, snapd converts +// ReplyConstraints to Constraints. +type ReplyConstraints struct { + PathPattern *patterns.PathPattern `json:"path-pattern"` + Permissions []string `json:"permissions"` +} + +// ToConstraints validates the receiving ReplyConstraints with respect to the +// given interface, along with the given outcome, lifespan, and duration, and +// constructs an equivalent Constraints from the ReplyConstraints. +func (c *ReplyConstraints) ToConstraints(iface string, outcome OutcomeType, lifespan LifespanType, duration string) (*Constraints, error) { + if _, err := outcome.AsBool(); err != nil { + // Should not occur, as outcome is validated when unmarshalled + return nil, err + } + if _, err := lifespan.ParseDuration(duration, time.Now()); err != nil { + return nil, err + } + if c.PathPattern == nil { + return nil, prompting_errors.NewInvalidPathPatternError("", "no path pattern") + } availablePerms, ok := interfacePermissionsAvailable[iface] if !ok { - return prompting_errors.NewInvalidInterfaceError(iface, availableInterfaces()) + return nil, prompting_errors.NewInvalidInterfaceError(iface, availableInterfaces()) + } + if len(c.Permissions) == 0 { + return nil, prompting_errors.NewPermissionsListEmptyError(iface, availablePerms) } - permsSet := make(map[string]bool, len(c.Permissions)) var invalidPerms []string + permissionMap := make(PermissionMap, len(c.Permissions)) for _, perm := range c.Permissions { if !strutil.ListContains(availablePerms, perm) { invalidPerms = append(invalidPerms, perm) continue } - permsSet[perm] = true + permissionMap[perm] = &PermissionEntry{ + Outcome: outcome, + Lifespan: lifespan, + Duration: duration, + } } if len(invalidPerms) > 0 { - return prompting_errors.NewInvalidPermissionsError(iface, invalidPerms, availablePerms) - } - if len(permsSet) == 0 { - return prompting_errors.NewPermissionsListEmptyError(iface, availablePerms) + return nil, prompting_errors.NewInvalidPermissionsError(iface, invalidPerms, availablePerms) } - newPermissions := make([]string, 0, len(permsSet)) - for _, perm := range availablePerms { - if exists := permsSet[perm]; exists { - newPermissions = append(newPermissions, perm) - } + constraints := &Constraints{ + PathPattern: c.PathPattern, + Permissions: permissionMap, } - c.Permissions = newPermissions - return nil + return constraints, nil } // Match returns true if the constraints match the given path, otherwise false. // // If the constraints or path are invalid, returns an error. -func (c *Constraints) Match(path string) (bool, error) { +func (c *ReplyConstraints) Match(path string) (bool, error) { if c.PathPattern == nil { return false, prompting_errors.NewInvalidPathPatternError("", "no path pattern") } @@ -97,9 +164,9 @@ func (c *Constraints) Match(path string) (bool, error) { return match, nil } -// ContainPermissions returns true if the constraints include every one of the -// given permissions. -func (c *Constraints) ContainPermissions(permissions []string) bool { +// ContainPermissions returns true if the permission list in the constraints +// includes every one of the given permissions. +func (c *ReplyConstraints) ContainPermissions(permissions []string) bool { for _, perm := range permissions { if !strutil.ListContains(c.Permissions, perm) { return false @@ -108,6 +175,277 @@ func (c *Constraints) ContainPermissions(permissions []string) bool { return true } +// PatchConstraints hold information about the applicability of the modified +// rule to particular paths or permissions. A request will be matched by the +// constraints if the requested path is matched by the path pattern (according +// to bash's globstar matching) and one or more requested permissions are +// denied in the permission map, or all of the requested permissions are +// allowed in the map. When snapd modifies the rule, it converts +// PatchConstraints to RuleConstraints, using the rule's existing constraints +// wherever a field is omitted. +// +// Any permissions which are omitted from the new permission map are left +// unchanged from the existing rule. To remove an existing permission from the +// rule, the permission should map to null. +type PatchConstraints struct { + PathPattern *patterns.PathPattern `json:"path-pattern,omitempty"` + Permissions PermissionMap `json:"permissions,omitempty"` +} + +// PatchRuleConstraints validates the receiving PatchConstraints and uses the +// existing rule constraints to construct a new RuleConstraints. +// +// If the path pattern or permissions fields are omitted, they are left +// unchanged from the existing rule. If the permissions field is present in +// the patch constraints, then any permissions which are omitted from the +// patch constrants' permission map are left unchanged from the existing rule. +// To remove an an existing permission from the rule, the permission should map +// to null in the permission map of the patch constraints. +// +// The existing rule constraints should never be modified. +func (c *PatchConstraints) PatchRuleConstraints(existing *RuleConstraints, iface string, currTime time.Time) (*RuleConstraints, error) { + ruleConstraints := &RuleConstraints{ + PathPattern: c.PathPattern, + } + if c.PathPattern == nil { + ruleConstraints.PathPattern = existing.PathPattern + } + if c.Permissions == nil { + ruleConstraints.Permissions = existing.Permissions + return ruleConstraints, nil + } + // Permissions are specified in the patch constraints, need to merge them + newPermissions := make(RulePermissionMap, len(c.Permissions)+len(existing.Permissions)) + for perm, entry := range existing.Permissions { + if !entry.Expired(currTime) { + newPermissions[perm] = entry + } + } + availablePerms, ok := interfacePermissionsAvailable[iface] + if !ok { + // Should not occur, as we should use the interface from the existing rule + return nil, prompting_errors.NewInvalidInterfaceError(iface, availableInterfaces()) + } + var errs []error + var invalidPerms []string + for perm, entry := range c.Permissions { + if entry == nil { + delete(newPermissions, perm) + continue + } + if !strutil.ListContains(availablePerms, perm) { + invalidPerms = append(invalidPerms, perm) + continue + } + ruleEntry, err := entry.toRulePermissionEntry(currTime) + if err != nil { + errs = append(errs, err) + continue + } + newPermissions[perm] = ruleEntry + } + if len(invalidPerms) > 0 { + errs = append(errs, prompting_errors.NewInvalidPermissionsError(iface, invalidPerms, availablePerms)) + } + if len(errs) > 0 { + return nil, prompting_errors.Join(errs...) + } + ruleConstraints.Permissions = newPermissions + return ruleConstraints, nil +} + +// PermissionMap is a map from permissions to their corresponding entries, +// which contain information about the outcome and lifespan for those +// permissions. +type PermissionMap map[string]*PermissionEntry + +// toRulePermissionMap validates the receiving PermissionMap and converts it +// to a RulePermissionMap, using the given current time to convert any included +// durations to expirations. If the permission map is not valid with respect to +// the given interface, returns an error. +func (pm PermissionMap) toRulePermissionMap(iface string, currTime time.Time) (RulePermissionMap, error) { + availablePerms, ok := interfacePermissionsAvailable[iface] + if !ok { + return nil, prompting_errors.NewInvalidInterfaceError(iface, availableInterfaces()) + } + if len(pm) == 0 { + return nil, prompting_errors.NewPermissionsListEmptyError(iface, availablePerms) + } + var errs []error + var invalidPerms []string + rulePermissionMap := make(RulePermissionMap, len(pm)) + for perm, entry := range pm { + if !strutil.ListContains(availablePerms, perm) { + invalidPerms = append(invalidPerms, perm) + continue + } + rulePermissionEntry, err := entry.toRulePermissionEntry(currTime) + if err != nil { + errs = append(errs, err) + continue + } + rulePermissionMap[perm] = rulePermissionEntry + } + if len(invalidPerms) > 0 { + errs = append(errs, prompting_errors.NewInvalidPermissionsError(iface, invalidPerms, availablePerms)) + } + if len(errs) > 0 { + return nil, prompting_errors.Join(errs...) + } + return rulePermissionMap, nil +} + +// RulePermissionMap is a map from permissions to their corresponding entries, +// which contain information about the outcome and lifespan for those +// permissions. +type RulePermissionMap map[string]*RulePermissionEntry + +// validateForInterface checks that the rule permission map is valid for the +// given interface. Any permissions which have expired relative to the given +// current time are pruned. If all permissions have expired, then returns true. +// If the permission map is invalid, returns an error. +func (pm RulePermissionMap) validateForInterface(iface string, currTime time.Time) (expired bool, err error) { + availablePerms, ok := interfacePermissionsAvailable[iface] + if !ok { + return false, prompting_errors.NewInvalidInterfaceError(iface, availableInterfaces()) + } + if len(pm) == 0 { + return false, prompting_errors.NewPermissionsListEmptyError(iface, availablePerms) + } + var errs []error + var invalidPerms []string + var expiredPerms []string + for perm, entry := range pm { + if !strutil.ListContains(availablePerms, perm) { + invalidPerms = append(invalidPerms, perm) + continue + } + if entry.Expired(currTime) { + expiredPerms = append(expiredPerms, perm) + continue + } + if err := entry.validate(); err != nil { + errs = append(errs, err) + } + } + if len(invalidPerms) > 0 { + errs = append(errs, prompting_errors.NewInvalidPermissionsError(iface, invalidPerms, availablePerms)) + } + if len(errs) > 0 { + return false, prompting_errors.Join(errs...) + } + for _, perm := range expiredPerms { + delete(pm, perm) + } + if len(pm) == 0 { + // All permissions expired + return true, nil + } + return false, nil +} + +// Expired returns true if all permissions in the map have expired. +func (pm RulePermissionMap) Expired(currTime time.Time) bool { + for _, entry := range pm { + if !entry.Expired(currTime) { + return false + } + } + return true +} + +// PermissionEntry holds the outcome associated with a particular permission +// and the lifespan for which that outcome is applicable. +// +// PermissionEntry is used when replying to a prompt, creating a new rule, or +// modifying an existing rule. +type PermissionEntry struct { + Outcome OutcomeType `json:"outcome"` + Lifespan LifespanType `json:"lifespan"` + Duration string `json:"duration,omitempty"` +} + +// toRulePermissionEntry validates the receiving PermissionEntry and converts +// it to a RulePermissionEntry. +// +// Checks that the entry has a valid outcome, and that its lifespan is valid +// for a rule (i.e. not LifespanSingle), and that it has an appropriate +// duration for that lifespan. The duration, combined with the given current +// time, is used to compute an expiration time, and that is returned as part +// of the corresponding RulePermissionEntry. +func (e *PermissionEntry) toRulePermissionEntry(currTime time.Time) (*RulePermissionEntry, error) { + if _, err := e.Outcome.AsBool(); err != nil { + return nil, err + } + if e.Lifespan == LifespanSingle { + // We don't allow rules with lifespan "single" + return nil, prompting_errors.NewRuleLifespanSingleError(SupportedRuleLifespans) + } + expiration, err := e.Lifespan.ParseDuration(e.Duration, currTime) + if err != nil { + return nil, err + } + rulePermissionEntry := &RulePermissionEntry{ + Outcome: e.Outcome, + Lifespan: e.Lifespan, + Expiration: expiration, + } + return rulePermissionEntry, nil +} + +// RulePermissionEntry holds the outcome associated with a particular permission +// and the lifespan for which that outcome is applicable. +// +// RulePermissionEntry is derived from a PermissionEntry after it has been used +// along with the rule's timestamp to define the expiration timeouts for any +// permissions which have a lifespan of "timespan". RulePermissionEntry is what +// is returned when retrieving rule contents, but PermissionEntry is used when +// replying to prompts, creating new rules, or modifying existing rules. +type RulePermissionEntry struct { + Outcome OutcomeType `json:"outcome"` + Lifespan LifespanType `json:"lifespan"` + Expiration time.Time `json:"expiration,omitempty"` +} + +// Expired returns true if the receiving permission entry has expired and +// should no longer be considered when matching requests. +// +// This is the case if the permission has a lifespan of timespan and the +// current time is after its expiration time. +func (e *RulePermissionEntry) Expired(currTime time.Time) bool { + switch e.Lifespan { + case LifespanTimespan: + if !currTime.Before(e.Expiration) { + return true + } + // TODO: add lifespan session + //case LifespanSession: + // TODO: return true if the user session has changed + } + return false +} + +// validate checks that the entry has a valid outcome, and that its lifespan +// is valid for a rule (i.e. not LifespanSingle), and has an appropriate +// expiration for that lifespan. +func (e *RulePermissionEntry) validate() error { + if _, err := e.Outcome.AsBool(); err != nil { + return err + } + if e.Lifespan == LifespanSingle { + // We don't allow rules with lifespan "single" + return prompting_errors.NewRuleLifespanSingleError(SupportedRuleLifespans) + } + if err := e.Lifespan.ValidateExpiration(e.Expiration); err != nil { + // Should never error due to an API request, since rules are always + // added via the API using duration, rather than expiration. + // Error may occur when validating a rule loaded from disk. + // We don't check expiration as part of validation. + return err + } + return nil +} + var ( // List of permissions available for each interface. This also defines the // order in which the permissions should be presented. diff --git a/interfaces/prompting/constraints_test.go b/interfaces/prompting/constraints_test.go index f2f80ca6d82..784ed9e33b8 100644 --- a/interfaces/prompting/constraints_test.go +++ b/interfaces/prompting/constraints_test.go @@ -20,9 +20,13 @@ package prompting_test import ( + "fmt" + "time" + . "gopkg.in/check.v1" "github.com/snapcore/snapd/interfaces/prompting" + prompting_errors "github.com/snapcore/snapd/interfaces/prompting/errors" "github.com/snapcore/snapd/interfaces/prompting/patterns" "github.com/snapcore/snapd/logger" "github.com/snapcore/snapd/sandbox/apparmor/notify" @@ -33,122 +37,360 @@ type constraintsSuite struct{} var _ = Suite(&constraintsSuite{}) -func (s *constraintsSuite) TestConstraintsValidateForInterface(c *C) { - validPathPattern, err := patterns.ParsePathPattern("/path/to/foo") +func mustParsePathPattern(c *C, patternStr string) *patterns.PathPattern { + pattern, err := patterns.ParsePathPattern(patternStr) c.Assert(err, IsNil) + return pattern +} + +func (s *constraintsSuite) TestConstraintsToRuleConstraintsHappy(c *C) { + currTime := time.Now() + iface := "home" + pathPattern := mustParsePathPattern(c, "/path/to/{foo,*or*,bar}{,/}**") - // Happy constraints := &prompting.Constraints{ - PathPattern: validPathPattern, - Permissions: []string{"read"}, + PathPattern: pathPattern, + Permissions: prompting.PermissionMap{ + "read": &prompting.PermissionEntry{ + Outcome: prompting.OutcomeAllow, + Lifespan: prompting.LifespanForever, + }, + "write": &prompting.PermissionEntry{ + Outcome: prompting.OutcomeDeny, + Lifespan: prompting.LifespanTimespan, + Duration: "10s", + }, + "execute": &prompting.PermissionEntry{ + Outcome: prompting.OutcomeAllow, + Lifespan: prompting.LifespanTimespan, + Duration: "1ns", + }, + }, } - err = constraints.ValidateForInterface("home") - c.Check(err, IsNil) - // Bad interface or permissions - cases := []struct { - iface string - perms []string + expectedRuleConstraints := &prompting.RuleConstraints{ + PathPattern: pathPattern, + Permissions: prompting.RulePermissionMap{ + "read": &prompting.RulePermissionEntry{ + Outcome: prompting.OutcomeAllow, + Lifespan: prompting.LifespanForever, + }, + "write": &prompting.RulePermissionEntry{ + Outcome: prompting.OutcomeDeny, + Lifespan: prompting.LifespanTimespan, + Expiration: currTime.Add(10 * time.Second), + }, + "execute": &prompting.RulePermissionEntry{ + Outcome: prompting.OutcomeAllow, + Lifespan: prompting.LifespanTimespan, + Expiration: currTime.Add(time.Nanosecond), + }, + }, + } + + result, err := constraints.ToRuleConstraints(iface, currTime) + c.Assert(err, IsNil) + c.Assert(result, DeepEquals, expectedRuleConstraints) +} + +func (s *constraintsSuite) TestConstraintsToRuleConstraintsUnhappy(c *C) { + badConstraints := &prompting.Constraints{} + result, err := badConstraints.ToRuleConstraints("home", time.Now()) + c.Check(result, IsNil) + c.Check(err, ErrorMatches, `invalid path pattern: no path pattern.*`) + + constraints := &prompting.Constraints{ + PathPattern: mustParsePathPattern(c, "/path/to/foo"), + Permissions: prompting.PermissionMap{ + "read": &prompting.PermissionEntry{ + Outcome: prompting.OutcomeAllow, + Lifespan: prompting.LifespanForever, + }, + }, + } + result, err = constraints.ToRuleConstraints("foo", time.Now()) + c.Check(result, IsNil) + c.Check(err, ErrorMatches, `invalid interface: "foo"`) + + for _, testCase := range []struct { + perms prompting.PermissionMap errStr string }{ { - "foo", - []string{"read"}, - `invalid interface: "foo"`, + perms: nil, + errStr: `invalid permissions for home interface: permissions list empty`, }, { - "home", - []string{}, - "invalid permissions for home interface: permissions list empty", + perms: prompting.PermissionMap{ + "create": &prompting.PermissionEntry{ + Outcome: prompting.OutcomeAllow, + Lifespan: prompting.LifespanForever, + }, + }, + errStr: `invalid permissions for home interface: "create"`, }, { - "home", - []string{"access"}, - "invalid permissions for home interface.*", + perms: prompting.PermissionMap{ + "read": &prompting.PermissionEntry{ + Outcome: prompting.OutcomeAllow, + Lifespan: prompting.LifespanTimespan, + }, + }, + errStr: `invalid duration: cannot have unspecified duration when lifespan is "timespan".*`, }, - } - for _, testCase := range cases { + { + perms: prompting.PermissionMap{ + "read": &prompting.PermissionEntry{ + Outcome: prompting.OutcomeAllow, + Lifespan: prompting.LifespanTimespan, + }, + "create": &prompting.PermissionEntry{ + Outcome: prompting.OutcomeAllow, + Lifespan: prompting.LifespanForever, + }, + }, + errStr: joinErrorsUnordered(`invalid duration: cannot have unspecified duration when lifespan is "timespan": ""`, `invalid permissions for home interface: "create"`), + }, + } { constraints := &prompting.Constraints{ - PathPattern: validPathPattern, + PathPattern: mustParsePathPattern(c, "/path/to/foo"), Permissions: testCase.perms, } - err = constraints.ValidateForInterface(testCase.iface) - c.Check(err, ErrorMatches, testCase.errStr) + result, err = constraints.ToRuleConstraints("home", time.Now()) + c.Check(result, IsNil, Commentf("testCase: %+v", testCase)) + c.Check(err, ErrorMatches, testCase.errStr, Commentf("testCase: %+v", testCase)) } +} - // Check missing path pattern - constraints = &prompting.Constraints{ - Permissions: []string{"read"}, - } - err = constraints.ValidateForInterface("home") - c.Check(err, ErrorMatches, `invalid path pattern: no path pattern: ""`) +func joinErrorsUnordered(err1, err2 string) string { + return fmt.Sprintf("(%s\n%s|%s\n%s)", err1, err2, err2, err1) } -func (s *constraintsSuite) TestValidatePermissionsHappy(c *C) { +func (s *constraintsSuite) TestRuleConstraintsValidateForInterface(c *C) { + validPathPattern := mustParsePathPattern(c, "/path/to/foo") + currTime := time.Now() + + // Happy + constraints := &prompting.RuleConstraints{ + PathPattern: validPathPattern, + Permissions: prompting.RulePermissionMap{ + "read": &prompting.RulePermissionEntry{ + Outcome: prompting.OutcomeAllow, + Lifespan: prompting.LifespanForever, + }, + "write": &prompting.RulePermissionEntry{ + Outcome: prompting.OutcomeDeny, + Lifespan: prompting.LifespanTimespan, + Expiration: currTime.Add(time.Second), + }, + }, + } + expired, err := constraints.ValidateForInterface("home", currTime) + c.Check(err, IsNil) + c.Check(expired, Equals, false) + + // Bad interface or permissions cases := []struct { - iface string - initial []string - final []string + iface string + perms prompting.RulePermissionMap + errStr string }{ + { + "foo", + prompting.RulePermissionMap{ + "read": &prompting.RulePermissionEntry{ + Outcome: prompting.OutcomeAllow, + Lifespan: prompting.LifespanForever, + }, + }, + prompting_errors.NewInvalidInterfaceError("foo", nil).Error(), + }, { "home", - []string{"write", "read", "execute"}, - []string{"read", "write", "execute"}, + prompting.RulePermissionMap{}, + prompting_errors.NewPermissionsListEmptyError("home", nil).Error(), }, { "home", - []string{"execute", "write", "read"}, - []string{"read", "write", "execute"}, + prompting.RulePermissionMap{ + "access": &prompting.RulePermissionEntry{ + Outcome: prompting.OutcomeAllow, + Lifespan: prompting.LifespanForever, + }, + }, + prompting_errors.NewInvalidPermissionsError("home", []string{"access"}, []string{"read", "write", "execute"}).Error(), }, { "home", - []string{"write", "write", "write"}, - []string{"write"}, + prompting.RulePermissionMap{ + "read": &prompting.RulePermissionEntry{ + Outcome: prompting.OutcomeAllow, + Lifespan: prompting.LifespanForever, + Expiration: time.Now().Add(time.Second), + }, + }, + "invalid expiration: cannot have specified expiration.*", + }, + { + "home", + prompting.RulePermissionMap{ + "read": &prompting.RulePermissionEntry{ + Outcome: prompting.OutcomeAllow, + Lifespan: prompting.LifespanSingle, + }, + }, + `cannot create rule with lifespan "single"`, }, } for _, testCase := range cases { - constraints := prompting.Constraints{ - Permissions: testCase.initial, + constraints := &prompting.RuleConstraints{ + PathPattern: validPathPattern, + Permissions: testCase.perms, } - err := constraints.ValidatePermissions(testCase.iface) - c.Check(err, IsNil, Commentf("testCase: %+v", testCase)) - c.Check(constraints.Permissions, DeepEquals, testCase.final, Commentf("testCase: %+v", testCase)) + expired, err = constraints.ValidateForInterface(testCase.iface, time.Now()) + c.Check(err, ErrorMatches, testCase.errStr) + c.Check(expired, Equals, false) + } + + // Check missing path pattern + constraints = &prompting.RuleConstraints{ + Permissions: prompting.RulePermissionMap{ + "read": &prompting.RulePermissionEntry{ + Outcome: prompting.OutcomeAllow, + Lifespan: prompting.LifespanForever, + }, + }, } + _, err = constraints.ValidateForInterface("home", time.Now()) + c.Check(err, ErrorMatches, `invalid path pattern: no path pattern: ""`) } -func (s *constraintsSuite) TestValidatePermissionsUnhappy(c *C) { - cases := []struct { - iface string - perms []string - errStr string +func (s *constraintsSuite) TestRuleConstraintsValidateForInterfaceExpiration(c *C) { + pathPattern := mustParsePathPattern(c, "/path/to/foo") + currTime := time.Now() + + for _, testCase := range []struct { + perms prompting.RulePermissionMap + expired bool + expected prompting.RulePermissionMap }{ { - "foo", - []string{"read"}, - `invalid interface: "foo"`, + prompting.RulePermissionMap{ + "read": &prompting.RulePermissionEntry{ + Outcome: prompting.OutcomeAllow, + Lifespan: prompting.LifespanForever, + }, + }, + false, + prompting.RulePermissionMap{ + "read": &prompting.RulePermissionEntry{ + Outcome: prompting.OutcomeAllow, + Lifespan: prompting.LifespanForever, + }, + }, }, { - "home", - []string{"access"}, - `invalid permissions for home interface: "access"`, + prompting.RulePermissionMap{ + "read": &prompting.RulePermissionEntry{ + Outcome: prompting.OutcomeAllow, + Lifespan: prompting.LifespanTimespan, + Expiration: currTime, + }, + }, + true, + prompting.RulePermissionMap{}, }, { - "home", - []string{"read", "write", "access"}, - `invalid permissions for home interface: "access"`, + prompting.RulePermissionMap{ + "read": &prompting.RulePermissionEntry{ + Outcome: prompting.OutcomeAllow, + Lifespan: prompting.LifespanTimespan, + Expiration: currTime.Add(-time.Minute), + }, + "write": &prompting.RulePermissionEntry{ + Outcome: prompting.OutcomeDeny, + Lifespan: prompting.LifespanTimespan, + Expiration: currTime.Add(time.Minute), + }, + "execute": &prompting.RulePermissionEntry{ + Outcome: prompting.OutcomeDeny, + Lifespan: prompting.LifespanTimespan, + Expiration: currTime, + }, + }, + false, + prompting.RulePermissionMap{ + "write": &prompting.RulePermissionEntry{ + Outcome: prompting.OutcomeDeny, + Lifespan: prompting.LifespanTimespan, + Expiration: currTime.Add(time.Minute), + }, + }, }, { - "home", - []string{}, - "invalid permissions for home interface: permissions list empty", + prompting.RulePermissionMap{ + "read": &prompting.RulePermissionEntry{ + Outcome: prompting.OutcomeAllow, + Lifespan: prompting.LifespanTimespan, + Expiration: currTime.Add(-time.Minute), + }, + "write": &prompting.RulePermissionEntry{ + Outcome: prompting.OutcomeDeny, + Lifespan: prompting.LifespanTimespan, + Expiration: currTime, + }, + "execute": &prompting.RulePermissionEntry{ + Outcome: prompting.OutcomeAllow, + Lifespan: prompting.LifespanTimespan, + Expiration: currTime, + }, + }, + true, + prompting.RulePermissionMap{}, }, - } - for _, testCase := range cases { - constraints := prompting.Constraints{ - Permissions: testCase.perms, + { + prompting.RulePermissionMap{ + "read": &prompting.RulePermissionEntry{ + Outcome: prompting.OutcomeAllow, + Lifespan: prompting.LifespanTimespan, + Expiration: currTime.Add(-time.Minute), + }, + "write": &prompting.RulePermissionEntry{ + Outcome: prompting.OutcomeAllow, + Lifespan: prompting.LifespanTimespan, + Expiration: currTime.Add(time.Minute), + }, + "execute": &prompting.RulePermissionEntry{ + Outcome: prompting.OutcomeDeny, + Lifespan: prompting.LifespanForever, + }, + }, + false, + prompting.RulePermissionMap{ + "write": &prompting.RulePermissionEntry{ + Outcome: prompting.OutcomeAllow, + Lifespan: prompting.LifespanTimespan, + Expiration: currTime.Add(time.Minute), + }, + "execute": &prompting.RulePermissionEntry{ + Outcome: prompting.OutcomeDeny, + Lifespan: prompting.LifespanForever, + }, + }, + }, + } { + copiedPerms := make(prompting.RulePermissionMap, len(testCase.perms)) + for perm, entry := range testCase.perms { + copiedPerms[perm] = entry } - err := constraints.ValidatePermissions(testCase.iface) - c.Check(err, ErrorMatches, testCase.errStr, Commentf("testCase: %+v", testCase)) + constraints := &prompting.RuleConstraints{ + PathPattern: pathPattern, + Permissions: copiedPerms, + } + expired, err := constraints.ValidateForInterface("home", currTime) + c.Check(err, IsNil) + c.Check(expired, Equals, testCase.expired, Commentf("testCase: %+v\nremaining perms: %+v", testCase, constraints.Permissions)) + c.Check(constraints.Permissions, DeepEquals, testCase.expected, Commentf("testCase: %+v\nremaining perms: %+v", testCase, constraints.Permissions)) } } @@ -170,13 +412,19 @@ func (*constraintsSuite) TestConstraintsMatch(c *C) { }, } for _, testCase := range cases { - pattern, err := patterns.ParsePathPattern(testCase.pattern) - c.Check(err, IsNil) - constraints := &prompting.Constraints{ + pattern := mustParsePathPattern(c, testCase.pattern) + + ruleConstraints := &prompting.RuleConstraints{ PathPattern: pattern, - Permissions: []string{"read"}, } - result, err := constraints.Match(testCase.path) + result, err := ruleConstraints.Match(testCase.path) + c.Check(err, IsNil, Commentf("test case: %+v", testCase)) + c.Check(result, Equals, testCase.matches, Commentf("test case: %+v", testCase)) + + replyConstraints := &prompting.ReplyConstraints{ + PathPattern: pattern, + } + result, err = replyConstraints.Match(testCase.path) c.Check(err, IsNil, Commentf("test case: %+v", testCase)) c.Check(result, Equals, testCase.matches, Commentf("test case: %+v", testCase)) } @@ -184,15 +432,162 @@ func (*constraintsSuite) TestConstraintsMatch(c *C) { func (s *constraintsSuite) TestConstraintsMatchUnhappy(c *C) { badPath := `bad\path\` - badConstraints := &prompting.Constraints{ - Permissions: []string{"read"}, + + badRuleConstraints := &prompting.RuleConstraints{ + PathPattern: nil, + } + matches, err := badRuleConstraints.Match(badPath) + c.Check(err, ErrorMatches, `invalid path pattern: no path pattern: ""`) + c.Check(matches, Equals, false) + + badReplyConstraints := &prompting.ReplyConstraints{ + PathPattern: nil, } - matches, err := badConstraints.Match(badPath) + matches, err = badReplyConstraints.Match(badPath) c.Check(err, ErrorMatches, `invalid path pattern: no path pattern: ""`) c.Check(matches, Equals, false) } -func (s *constraintsSuite) TestConstraintsContainPermissions(c *C) { +func (s *constraintsSuite) TestReplyConstraintsToConstraintsHappy(c *C) { + iface := "home" + pathPattern := mustParsePathPattern(c, "/path/to/dir/{foo*,ba?/**}") + + for _, testCase := range []struct { + pathPattern *patterns.PathPattern + permissions []string + outcome prompting.OutcomeType + lifespan prompting.LifespanType + duration string + expected *prompting.Constraints + }{ + { + permissions: []string{"read", "write", "execute"}, + outcome: prompting.OutcomeAllow, + lifespan: prompting.LifespanForever, + expected: &prompting.Constraints{ + PathPattern: pathPattern, + Permissions: prompting.PermissionMap{ + "read": &prompting.PermissionEntry{ + Outcome: prompting.OutcomeAllow, + Lifespan: prompting.LifespanForever, + }, + "write": &prompting.PermissionEntry{ + Outcome: prompting.OutcomeAllow, + Lifespan: prompting.LifespanForever, + }, + "execute": &prompting.PermissionEntry{ + Outcome: prompting.OutcomeAllow, + Lifespan: prompting.LifespanForever, + }, + }, + }, + }, + { + permissions: []string{"write", "read"}, + outcome: prompting.OutcomeDeny, + lifespan: prompting.LifespanTimespan, + duration: "10m", + expected: &prompting.Constraints{ + PathPattern: pathPattern, + Permissions: prompting.PermissionMap{ + "read": &prompting.PermissionEntry{ + Outcome: prompting.OutcomeDeny, + Lifespan: prompting.LifespanTimespan, + Duration: "10m", + }, + "write": &prompting.PermissionEntry{ + Outcome: prompting.OutcomeDeny, + Lifespan: prompting.LifespanTimespan, + Duration: "10m", + }, + }, + }, + }, + } { + replyConstraints := &prompting.ReplyConstraints{ + PathPattern: pathPattern, + Permissions: testCase.permissions, + } + constraints, err := replyConstraints.ToConstraints(iface, testCase.outcome, testCase.lifespan, testCase.duration) + c.Check(err, IsNil) + c.Check(constraints, DeepEquals, testCase.expected) + } +} + +func (s *constraintsSuite) TestReplyConstraintsToConstraintsUnhappy(c *C) { + for _, testCase := range []struct { + nilPattern bool + permissions []string + iface string + outcome prompting.OutcomeType + lifespan prompting.LifespanType + duration string + errStr string + }{ + { + outcome: prompting.OutcomeType("foo"), + errStr: `invalid outcome: "foo"`, + }, + { + lifespan: prompting.LifespanTimespan, + duration: "", + errStr: `invalid duration: cannot have unspecified duration when lifespan is "timespan":.*`, + }, + { + lifespan: prompting.LifespanForever, + duration: "10s", + errStr: `invalid duration: cannot have specified duration when lifespan is "forever":.*`, + }, + { + nilPattern: true, + errStr: `invalid path pattern: no path pattern: ""`, + }, + { + iface: "foo", + errStr: `invalid interface: "foo"`, + }, + { + permissions: make([]string, 0), + errStr: `invalid permissions for home interface: permissions list empty`, + }, + { + permissions: []string{"read", "append", "write", "create", "execute"}, + errStr: `invalid permissions for home interface: "append", "create"`, + }, + } { + replyConstraints := &prompting.ReplyConstraints{ + PathPattern: mustParsePathPattern(c, "/path/to/foo"), + Permissions: []string{"read", "write", "execute"}, + } + if testCase.nilPattern { + replyConstraints.PathPattern = nil + } + if testCase.permissions != nil { + replyConstraints.Permissions = testCase.permissions + } + iface := "home" + if testCase.iface != "" { + iface = testCase.iface + } + outcome := prompting.OutcomeAllow + if testCase.outcome != prompting.OutcomeUnset { + outcome = testCase.outcome + } + lifespan := prompting.LifespanForever + if testCase.lifespan != prompting.LifespanUnset { + lifespan = testCase.lifespan + } + duration := "" + if testCase.duration != "" { + duration = testCase.duration + } + result, err := replyConstraints.ToConstraints(iface, outcome, lifespan, duration) + c.Check(result, IsNil, Commentf("testCase: %+v", testCase)) + c.Check(err, ErrorMatches, testCase.errStr, Commentf("testCase: %+v", testCase)) + } +} + +func (s *constraintsSuite) TestReplyConstraintsContainPermissions(c *C) { cases := []struct { constPerms []string queryPerms []string @@ -240,9 +635,8 @@ func (s *constraintsSuite) TestConstraintsContainPermissions(c *C) { }, } for _, testCase := range cases { - pathPattern, err := patterns.ParsePathPattern("/arbitrary") - c.Check(err, IsNil) - constraints := &prompting.Constraints{ + pathPattern := mustParsePathPattern(c, "/arbitrary") + constraints := &prompting.ReplyConstraints{ PathPattern: pathPattern, Permissions: testCase.constPerms, } @@ -251,6 +645,269 @@ func (s *constraintsSuite) TestConstraintsContainPermissions(c *C) { } } +func (s *constraintsSuite) TestPatchRuleConstraintsHappy(c *C) { + origTime := time.Now() + patchTime := origTime.Add(time.Second) + iface := "home" + + pathPattern := mustParsePathPattern(c, "/path/to/foo/ba?/**") + + for i, testCase := range []struct { + initial *prompting.RuleConstraints + patch *prompting.PatchConstraints + final *prompting.RuleConstraints + }{ + { + initial: &prompting.RuleConstraints{ + PathPattern: pathPattern, + Permissions: prompting.RulePermissionMap{ + "read": &prompting.RulePermissionEntry{ + Outcome: prompting.OutcomeAllow, + Lifespan: prompting.LifespanTimespan, + Expiration: origTime, + }, + "write": &prompting.RulePermissionEntry{ + Outcome: prompting.OutcomeDeny, + Lifespan: prompting.LifespanTimespan, + Expiration: origTime.Add(-time.Second), + }, + }, + }, + patch: &prompting.PatchConstraints{}, + final: &prompting.RuleConstraints{ + PathPattern: pathPattern, + Permissions: prompting.RulePermissionMap{ + "read": &prompting.RulePermissionEntry{ + Outcome: prompting.OutcomeAllow, + Lifespan: prompting.LifespanTimespan, + Expiration: origTime, + }, + "write": &prompting.RulePermissionEntry{ + Outcome: prompting.OutcomeDeny, + Lifespan: prompting.LifespanTimespan, + Expiration: origTime.Add(-time.Second), // expired perms are not pruned if patch perms are nil + }, + }, + }, + }, + { + initial: &prompting.RuleConstraints{ + PathPattern: pathPattern, + Permissions: prompting.RulePermissionMap{ + "read": &prompting.RulePermissionEntry{ + Outcome: prompting.OutcomeAllow, + Lifespan: prompting.LifespanTimespan, + Expiration: patchTime.Add(time.Second), + }, + "write": &prompting.RulePermissionEntry{ + Outcome: prompting.OutcomeDeny, + Lifespan: prompting.LifespanTimespan, + Expiration: origTime, + }, + }, + }, + patch: &prompting.PatchConstraints{ + Permissions: prompting.PermissionMap{ + "write": nil, + "execute": &prompting.PermissionEntry{ + Outcome: prompting.OutcomeDeny, + Lifespan: prompting.LifespanTimespan, + Duration: "1m", + }, + }, + }, + final: &prompting.RuleConstraints{ + PathPattern: pathPattern, + Permissions: prompting.RulePermissionMap{ + "read": &prompting.RulePermissionEntry{ + Outcome: prompting.OutcomeAllow, + Lifespan: prompting.LifespanTimespan, + Expiration: patchTime.Add(time.Second), + }, + "execute": &prompting.RulePermissionEntry{ + Outcome: prompting.OutcomeDeny, + Lifespan: prompting.LifespanTimespan, + Expiration: patchTime.Add(time.Minute), + }, + }, + }, + }, + { + initial: &prompting.RuleConstraints{ + PathPattern: pathPattern, + Permissions: prompting.RulePermissionMap{ + "read": &prompting.RulePermissionEntry{ + Outcome: prompting.OutcomeAllow, + Lifespan: prompting.LifespanTimespan, + Expiration: patchTime.Add(time.Second), + }, + "write": &prompting.RulePermissionEntry{ + Outcome: prompting.OutcomeDeny, + Lifespan: prompting.LifespanTimespan, + Expiration: origTime, + }, + "execute": &prompting.RulePermissionEntry{ + Outcome: prompting.OutcomeAllow, + Lifespan: prompting.LifespanForever, + }, + }, + }, + patch: &prompting.PatchConstraints{ + Permissions: prompting.PermissionMap{ + "execute": &prompting.PermissionEntry{ + Outcome: prompting.OutcomeAllow, + Lifespan: prompting.LifespanForever, + }, + }, + }, + final: &prompting.RuleConstraints{ + PathPattern: pathPattern, + Permissions: prompting.RulePermissionMap{ + "read": &prompting.RulePermissionEntry{ + Outcome: prompting.OutcomeAllow, + Lifespan: prompting.LifespanTimespan, + Expiration: patchTime.Add(time.Second), + }, + "execute": &prompting.RulePermissionEntry{ + Outcome: prompting.OutcomeAllow, + Lifespan: prompting.LifespanForever, + }, + }, + }, + }, + } { + patched, err := testCase.patch.PatchRuleConstraints(testCase.initial, iface, patchTime) + c.Check(err, IsNil, Commentf("testCase %d", i)) + c.Check(patched, DeepEquals, testCase.final, Commentf("testCase %d", i)) + } +} + +func (s *constraintsSuite) TestPatchRuleConstraintsUnhappy(c *C) { + origTime := time.Now() + patchTime := origTime.Add(time.Second) + iface := "home" + + pathPattern := mustParsePathPattern(c, "/path/to/foo/ba{r,z{,/**/}}") + + goodRule := &prompting.RuleConstraints{ + PathPattern: pathPattern, + Permissions: prompting.RulePermissionMap{ + "read": &prompting.RulePermissionEntry{ + Outcome: prompting.OutcomeAllow, + Lifespan: prompting.LifespanTimespan, + Expiration: patchTime.Add(time.Second), + }, + "write": &prompting.RulePermissionEntry{ + Outcome: prompting.OutcomeDeny, + Lifespan: prompting.LifespanTimespan, + Expiration: origTime, + }, + }, + } + goodPatch := &prompting.PatchConstraints{ + Permissions: prompting.PermissionMap{ + "write": nil, + "execute": &prompting.PermissionEntry{ + Outcome: prompting.OutcomeDeny, + Lifespan: prompting.LifespanForever, + }, + }, + } + + badIface := "foo" + result, err := goodPatch.PatchRuleConstraints(goodRule, badIface, patchTime) + c.Check(err, ErrorMatches, `invalid interface: "foo"`) + c.Check(result, IsNil) + + badPatch := &prompting.PatchConstraints{ + Permissions: prompting.PermissionMap{ + "read": &prompting.PermissionEntry{ + Outcome: prompting.OutcomeAllow, + Lifespan: prompting.LifespanSingle, + }, + "create": &prompting.PermissionEntry{ + Outcome: prompting.OutcomeAllow, + Lifespan: prompting.LifespanForever, + }, + "lock": &prompting.PermissionEntry{ + Outcome: prompting.OutcomeAllow, + Lifespan: prompting.LifespanForever, + }, + "execute": &prompting.PermissionEntry{ + Outcome: prompting.OutcomeAllow, + Lifespan: prompting.LifespanTimespan, + }, + }, + } + expected := joinErrorsUnordered(`invalid duration: cannot have unspecified duration when lifespan is "timespan": ""`, `cannot create rule with lifespan "single"`) + "\n" + `invalid permissions for home interface: ("create", "lock"|"lock", "create")` + + result, err = badPatch.PatchRuleConstraints(goodRule, iface, patchTime) + c.Check(result, IsNil) + c.Check(err, ErrorMatches, expected) +} + +func (s *constraintsSuite) TestRulePermissionMapExpired(c *C) { + currTime := time.Now() + for _, pm := range []prompting.RulePermissionMap{ + {}, + { + "read": &prompting.RulePermissionEntry{ + Outcome: prompting.OutcomeAllow, + Lifespan: prompting.LifespanTimespan, + Expiration: currTime, + }, + }, + { + "read": &prompting.RulePermissionEntry{ + Outcome: prompting.OutcomeAllow, + Lifespan: prompting.LifespanTimespan, + Expiration: currTime.Add(-time.Second), + }, + "write": &prompting.RulePermissionEntry{ + Outcome: prompting.OutcomeDeny, + Lifespan: prompting.LifespanTimespan, + Expiration: currTime, + }, + }, + } { + c.Check(pm.Expired(currTime), Equals, true, Commentf("%+v", pm)) + } + + for _, pm := range []prompting.RulePermissionMap{ + { + "read": &prompting.RulePermissionEntry{ + Outcome: prompting.OutcomeAllow, + Lifespan: prompting.LifespanTimespan, + Expiration: currTime.Add(-time.Second), + }, + "write": &prompting.RulePermissionEntry{ + Outcome: prompting.OutcomeDeny, + Lifespan: prompting.LifespanForever, + }, + }, + { + "read": &prompting.RulePermissionEntry{ + Outcome: prompting.OutcomeAllow, + Lifespan: prompting.LifespanTimespan, + Expiration: currTime.Add(-time.Second), + }, + "write": &prompting.RulePermissionEntry{ + Outcome: prompting.OutcomeDeny, + Lifespan: prompting.LifespanForever, + }, + }, + { + "read": &prompting.RulePermissionEntry{ + Outcome: prompting.OutcomeAllow, + Lifespan: prompting.LifespanTimespan, + Expiration: currTime.Add(time.Second), + }, + }, + } { + c.Check(pm.Expired(currTime), Equals, false, Commentf("%+v", pm)) + } +} + func constructPermissionsMaps() []map[string]map[string]any { var permissionsMaps []map[string]map[string]any // interfaceFilePermissionsMaps diff --git a/interfaces/prompting/errors/errors.go b/interfaces/prompting/errors/errors.go index e2d80015258..2a999c893b7 100644 --- a/interfaces/prompting/errors/errors.go +++ b/interfaces/prompting/errors/errors.go @@ -51,9 +51,8 @@ var ( ErrRuleDBInconsistent = errors.New("internal error: interfaces requests rule database left inconsistent") // Errors which are used internally and should never be returned over the API - ErrNoMatchingRule = errors.New("no rule matches the given path") - ErrInvalidID = errors.New("invalid ID: format must be parsable as uint64") - ErrRuleExpirationInThePast = errors.New("cannot have expiration time in the past") + ErrNoMatchingRule = errors.New("no rule matches the given path") + ErrInvalidID = errors.New("invalid ID: format must be parsable as uint64") ) // Marker for UnsupportedValueError, should never be returned as an actual @@ -125,6 +124,9 @@ func NewInvalidPermissionsError(iface string, unsupported []string, supported [] } func NewPermissionsListEmptyError(iface string, supported []string) *UnsupportedValueError { + // TODO: change language to "permissions empty" rather than "permissions list empty", + // since permissions now come as a list in prompt replies but as a map when creating + // or modifying rules directly. return &UnsupportedValueError{ Field: "permissions", Msg: fmt.Sprintf("invalid permissions for %s interface: permissions list empty", iface), @@ -229,3 +231,28 @@ func (e *RuleConflictError) Error() string { func (e *RuleConflictError) Unwrap() error { return ErrRuleConflict } + +// Join returns an error that wraps the given errors. +// Any nil error values are discarded. +// Join returns nil if every value in errs is nil. +// +// TODO: replace with errors.Join() once we're on golang v1.20+ +// +// This is a lossy implementation of errors.Join, where only the first non-nil +// error is preserved in a state which can be unwrapped. +func Join(errs ...error) error { + var nonNilErrs []error + for _, e := range errs { + if e != nil { + nonNilErrs = append(nonNilErrs, e) + } + } + if len(nonNilErrs) == 0 { + return nil + } + err := nonNilErrs[0] + for _, e := range nonNilErrs[1:] { + err = fmt.Errorf("%w\n%v", err, e) + } + return err +} diff --git a/interfaces/prompting/errors/errors_test.go b/interfaces/prompting/errors/errors_test.go new file mode 100644 index 00000000000..28ee2bae165 --- /dev/null +++ b/interfaces/prompting/errors/errors_test.go @@ -0,0 +1,72 @@ +// -*- Mode: Go; indent-tabs-mode: t -*- + +/* + * Copyright (C) 2024 Canonical Ltd + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 3 as + * published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see . + * + */ + +package errors_test + +import ( + "errors" + "testing" + + . "gopkg.in/check.v1" + + prompting_errors "github.com/snapcore/snapd/interfaces/prompting/errors" +) + +func Test(t *testing.T) { TestingT(t) } + +type errorsSuite struct{} + +var _ = Suite(&errorsSuite{}) + +func (s *errorsSuite) TestJoin(c *C) { + errs := []error{ + errors.New("foo"), + errors.New("bar"), + errors.New("baz"), + } + for _, testCase := range []struct { + errors []error + wrapped error + errStr string + }{ + { + errs, + errs[0], + "foo\nbar\nbaz", + }, + { + []error{nil, errs[2], nil, errs[1], nil}, + errs[2], + "baz\nbar", + }, + { + []error{nil, nil, nil}, + nil, + "", + }, + } { + joined := prompting_errors.Join(testCase.errors...) + c.Check(errors.Is(joined, testCase.wrapped), Equals, true, Commentf("testCase: %+v", testCase)) + if testCase.errStr != "" { + c.Check(joined, ErrorMatches, testCase.errStr) + } else { + c.Check(joined, IsNil) + } + } +} diff --git a/interfaces/prompting/export_test.go b/interfaces/prompting/export_test.go index 943d068930a..6a6ffa98f12 100644 --- a/interfaces/prompting/export_test.go +++ b/interfaces/prompting/export_test.go @@ -23,7 +23,3 @@ var ( InterfacePermissionsAvailable = interfacePermissionsAvailable InterfaceFilePermissionsMaps = interfaceFilePermissionsMaps ) - -func (c *Constraints) ValidatePermissions(iface string) error { - return c.validatePermissions(iface) -} diff --git a/interfaces/prompting/prompting.go b/interfaces/prompting/prompting.go index eafabbdecc5..3049a00e5e5 100644 --- a/interfaces/prompting/prompting.go +++ b/interfaces/prompting/prompting.go @@ -159,10 +159,9 @@ func (lifespan *LifespanType) UnmarshalJSON(data []byte) error { // ValidateExpiration checks that the given expiration is valid for the // receiver lifespan. // -// If the lifespan is LifespanTimespan, then expiration must be non-zero and be -// after the given currTime. Otherwise, it must be zero. Returns an error if -// any of the above are invalid. -func (lifespan LifespanType) ValidateExpiration(expiration time.Time, currTime time.Time) error { +// If the lifespan is LifespanTimespan, then expiration must be non-zero. +// Otherwise, it must be zero. Returns an error if any of the above are invalid. +func (lifespan LifespanType) ValidateExpiration(expiration time.Time) error { switch lifespan { case LifespanForever, LifespanSingle: if !expiration.IsZero() { @@ -172,9 +171,6 @@ func (lifespan LifespanType) ValidateExpiration(expiration time.Time, currTime t if expiration.IsZero() { return prompting_errors.NewInvalidExpirationError(expiration, fmt.Sprintf("cannot have unspecified expiration when lifespan is %q", lifespan)) } - if currTime.After(expiration) { - return fmt.Errorf("%w: %q", prompting_errors.ErrRuleExpirationInThePast, expiration) - } default: // Should not occur, since lifespan is validated when unmarshalled return prompting_errors.NewInvalidLifespanError(string(lifespan), supportedLifespans) diff --git a/interfaces/prompting/prompting_test.go b/interfaces/prompting/prompting_test.go index edcf8b68ddf..bcb27b8d960 100644 --- a/interfaces/prompting/prompting_test.go +++ b/interfaces/prompting/prompting_test.go @@ -169,21 +169,18 @@ func (s *promptingSuite) TestValidateExpiration(c *C) { prompting.LifespanForever, prompting.LifespanSingle, } { - err := lifespan.ValidateExpiration(unsetExpiration, currTime) + err := lifespan.ValidateExpiration(unsetExpiration) c.Check(err, IsNil) for _, exp := range []time.Time{negativeExpiration, validExpiration} { - err = lifespan.ValidateExpiration(exp, currTime) + err = lifespan.ValidateExpiration(exp) c.Check(err, ErrorMatches, `invalid expiration: cannot have specified expiration when lifespan is.*`) } } - err := prompting.LifespanTimespan.ValidateExpiration(unsetExpiration, currTime) + err := prompting.LifespanTimespan.ValidateExpiration(unsetExpiration) c.Check(err, ErrorMatches, `invalid expiration: cannot have unspecified expiration when lifespan is.*`) - err = prompting.LifespanTimespan.ValidateExpiration(negativeExpiration, currTime) - c.Check(err, ErrorMatches, `cannot have expiration time in the past.*`) - - err = prompting.LifespanTimespan.ValidateExpiration(validExpiration, currTime) + err = prompting.LifespanTimespan.ValidateExpiration(validExpiration) c.Check(err, IsNil) } diff --git a/interfaces/prompting/requestprompts/requestprompts.go b/interfaces/prompting/requestprompts/requestprompts.go index 5d8a9a050af..2b655d3ac97 100644 --- a/interfaces/prompting/requestprompts/requestprompts.go +++ b/interfaces/prompting/requestprompts/requestprompts.go @@ -126,20 +126,65 @@ func (pc *promptConstraints) equals(other *promptConstraints) bool { return true } -// subtractPermissions removes all of the given permissions from the list of -// permissions in the constraints. -func (pc *promptConstraints) subtractPermissions(permissions []string) (modified bool) { - newPermissions := make([]string, 0, len(pc.remainingPermissions)) +// applyRuleConstraints modifies the prompt constraints, removing any remaining +// permissions which are matched by the given rule constraints. +// +// Returns whether the prompt constraints were modified, whether the prompt +// requires a response (either because all permissions were allowed or at least +// one permission was denied), and the list of any permissions which were +// denied. If an error occurs, it is returned, and the other parameters can be +// ignored. +// +// If the path pattern does not match the prompt path, or the rule constraints +// do not include any of the remaining prompt permissions, then modified is +// false, and no changes are made to the prompt constraints. +func (pc *promptConstraints) applyRuleConstraints(constraints *prompting.RuleConstraints) (modified, respond bool, denied []string, err error) { + pathMatched, err := constraints.Match(pc.path) + if err != nil { + // Should not occur, only error is if path pattern is malformed, + // which would have thrown an error while parsing, not now. + return false, false, nil, err + } + if !pathMatched { + return false, false, nil, nil + } + + // Path pattern matched, now check if any permissions match + + newRemainingPermissions := make([]string, 0, len(pc.remainingPermissions)) for _, perm := range pc.remainingPermissions { - if !strutil.ListContains(permissions, perm) { - newPermissions = append(newPermissions, perm) + 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) + continue + } + modified = true + allow, err := entry.Outcome.AsBool() + if err != nil { + // This should not occur, as rule constraints are built internally + return false, false, nil, err + } + if !allow { + denied = append(denied, perm) } } - if len(newPermissions) != len(pc.remainingPermissions) { - pc.remainingPermissions = newPermissions - return true + if !modified { + // No permissions matched, so nothing changes, no need to record a + // notice or send a response. + return false, false, nil, nil } - return false + + pc.remainingPermissions = newRemainingPermissions + + if len(pc.remainingPermissions) == 0 || len(denied) > 0 { + // All permissions allowed or at least one permission denied, so tell + // the caller to send a response back to the kernel. + respond = true + } + + return modified, respond, denied, nil } // Path returns the path associated with the request to which the receiving @@ -310,7 +355,8 @@ 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) - allowedPermission := responseForInterfaceConstraintsOutcome(metadata.Interface, constraints, prompting.OutcomeDeny) + // Deny all permissions which are not already allowed by existing rules + allowedPermission := buildResponse(metadata.Interface, constraints, constraints.remainingPermissions) sendReply(listenerReq, allowedPermission) return nil, false, prompting_errors.ErrTooManyPrompts } @@ -330,21 +376,18 @@ func (pdb *PromptDB) AddOrMerge(metadata *prompting.Metadata, path string, reque return prompt, false, nil } -func responseForInterfaceConstraintsOutcome(iface string, constraints *promptConstraints, outcome prompting.OutcomeType) any { - allow, err := outcome.AsBool() - if err != nil { - // This should not occur, but if so, default to deny - allow = false - logger.Debugf("%v", err) - } +// buildResponse creates a listener response based on the given interface, +// prompt constraints, and list of denied permissions. +// +// The response is the AppArmor permission which should be allowed. This +// corresponds to the originally requested permissions from the prompt +// constraints, except with all denied permissions removed. +func buildResponse(iface string, constraints *promptConstraints, denied []string) any { allowedPerms := constraints.originalPermissions - if !allow { - // Remaining permissions were denied, so allow permissions which were - // previously allowed by prompt rules - allowedPerms = make([]string, 0, len(constraints.originalPermissions)-len(constraints.remainingPermissions)) + if len(denied) > 0 { + allowedPerms = make([]string, 0, len(constraints.originalPermissions)-len(denied)) for _, perm := range constraints.originalPermissions { - // Exclude any permissions which were remaining at time of denial - if !strutil.ListContains(constraints.remainingPermissions, perm) { + if !strutil.ListContains(denied, perm) { allowedPerms = append(allowedPerms, perm) } } @@ -408,13 +451,22 @@ func (pdb *PromptDB) promptWithID(user uint32, id prompting.IDType) (*userPrompt // // Records a notice for the prompt, and returns the prompt's former contents. func (pdb *PromptDB) Reply(user uint32, id prompting.IDType, outcome prompting.OutcomeType) (*Prompt, error) { + allow, err := outcome.AsBool() + if err != nil { + // This should not occur + return nil, err + } pdb.mutex.Lock() defer pdb.mutex.Unlock() userEntry, prompt, err := pdb.promptWithID(user, id) if err != nil { return nil, err } - allowedPermission := responseForInterfaceConstraintsOutcome(prompt.Interface, prompt.Constraints, outcome) + var denied []string + if !allow { + denied = prompt.Constraints.remainingPermissions + } + allowedPermission := buildResponse(prompt.Interface, prompt.Constraints, denied) for _, listenerReq := range prompt.listenerReqs { if err := sendReply(listenerReq, allowedPermission); err != nil { // Error should only occur if reply is malformed, and since these @@ -438,10 +490,10 @@ var sendReply = func(listenerReq *listener.Request, allowedPermission any) error // 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 if either the -// outcome is "allow" and all of the prompt's permissions are matched by those -// of the rule contents, or if the outcome is "deny" and any of the permissions -// match. +// 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". // // 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 @@ -450,12 +502,11 @@ var sendReply = func(listenerReq *listener.Request, allowedPermission any) error // // Returns the IDs of any prompts which were fully satisfied by the given rule // contents. -func (pdb *PromptDB) HandleNewRule(metadata *prompting.Metadata, constraints *prompting.Constraints, outcome prompting.OutcomeType) ([]prompting.IDType, error) { - // Validate outcome before locking - allow, err := outcome.AsBool() - if err != nil { - return nil, err - } +// +// Since rule is new, we don't check the expiration timestamps for any +// permissions, since any permissions with lifespan timespan were validated to +// have a non-zero duration, and we handle this rule as it was at its creation. +func (pdb *PromptDB) HandleNewRule(metadata *prompting.Metadata, constraints *prompting.RuleConstraints) ([]prompting.IDType, error) { pdb.mutex.Lock() defer pdb.mutex.Unlock() @@ -472,39 +523,52 @@ func (pdb *PromptDB) HandleNewRule(metadata *prompting.Metadata, constraints *pr if !(prompt.Snap == metadata.Snap && prompt.Interface == metadata.Interface) { continue } - matched, err := constraints.Match(prompt.Constraints.path) + + modified, respond, denied, err := prompt.Constraints.applyRuleConstraints(constraints) if err != nil { + // Should not occur, only error is if path pattern is malformed, + // which would have thrown an error while parsing, not now. return nil, err } - if !matched { - continue - } - - // Record all allowed permissions at the time of match, in case a - // permission is denied and we need to send back a response. - allowedPermission := responseForInterfaceConstraintsOutcome(metadata.Interface, prompt.Constraints, outcome) - - // See if the permission matches any of the prompt's remaining permissions - modified := prompt.Constraints.subtractPermissions(constraints.Permissions) if !modified { - // No permission was matched continue } - id := prompt.ID - if len(prompt.Constraints.remainingPermissions) > 0 && allow == true { - pdb.notifyPrompt(metadata.User, id, nil) + if !respond { + // No response necessary, though the prompt constraints were + // modified, so just record a notice for the prompt. + pdb.notifyPrompt(metadata.User, prompt.ID, nil) continue } - // All permissions of prompt satisfied, or any permission denied + + // A response is necessary, so either all permissions were allowed or + // at least one permission was denied. Construct a response and send it + // back to the kernel, and record a notice that the prompt was satisfied. + if len(denied) > 0 { + // At least one permission was denied by new rule, and we want to + // send a response immediately, so include any remaining + // 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. + denied = append(denied, prompt.Constraints.remainingPermissions...) + } + // Build and send a response with any permissions which were allowed, + // either by this new rule or by previous rules. + allowedPermission := buildResponse(metadata.Interface, prompt.Constraints, denied) for _, listenerReq := range prompt.listenerReqs { - // Reply with any permissions which were allowed, either by this - // new rule or by previous rules. sendReply(listenerReq, allowedPermission) } - userEntry.remove(id) - satisfiedPromptIDs = append(satisfiedPromptIDs, id) + // Now that a response has been sent, remove the rule from the rule DB + // and record a notice indicating that it has been satisfied. + userEntry.remove(prompt.ID) + satisfiedPromptIDs = append(satisfiedPromptIDs, prompt.ID) data := map[string]string{"resolved": "satisfied"} - pdb.notifyPrompt(metadata.User, id, data) + pdb.notifyPrompt(metadata.User, prompt.ID, data) } return satisfiedPromptIDs, nil } diff --git a/interfaces/prompting/requestprompts/requestprompts_test.go b/interfaces/prompting/requestprompts/requestprompts_test.go index 84718555a93..53a21e7b662 100644 --- a/interfaces/prompting/requestprompts/requestprompts_test.go +++ b/interfaces/prompting/requestprompts/requestprompts_test.go @@ -20,6 +20,7 @@ package requestprompts_test import ( + "bytes" "encoding/json" "fmt" "os" @@ -347,7 +348,19 @@ func applyNotices(expectedPromptIDs []prompting.IDType, expectedData map[string] } func (s *requestpromptsSuite) checkNewNotices(c *C, expectedNotices []*noticeInfo) { - c.Check(s.promptNotices, DeepEquals, expectedNotices) + c.Check(s.promptNotices, DeepEquals, expectedNotices, Commentf("%s", func() string { + var buf bytes.Buffer + buf.WriteString("\nobtained: [\n") + for _, n := range s.promptNotices { + buf.WriteString(fmt.Sprintf(" %+v\n", n)) + } + buf.WriteString("]\nexpected: [\n") + for _, n := range expectedNotices { + buf.WriteString(fmt.Sprintf(" %+v\n", n)) + } + buf.WriteString("]\n") + return buf.String() + }())) s.promptNotices = s.promptNotices[:0] } @@ -606,7 +619,7 @@ func (s *requestpromptsSuite) TestReplyErrors(c *C) { s.checkNewNoticesSimple(c, []prompting.IDType{}, nil) } -func (s *requestpromptsSuite) TestHandleNewRuleAllowPermissions(c *C) { +func (s *requestpromptsSuite) TestHandleNewRule(c *C) { listenerReqChan := make(chan *listener.Request, 2) replyChan := make(chan any, 2) restore := requestprompts.MockSendReply(func(listenerReq *listener.Request, allowedPermission any) error { @@ -659,23 +672,26 @@ func (s *requestpromptsSuite) TestHandleNewRuleAllowPermissions(c *C) { pathPattern, err := patterns.ParsePathPattern("/home/test/Documents/**") c.Assert(err, IsNil) - permissions := []string{"read", "write", "append"} - constraints := &prompting.Constraints{ + constraints := &prompting.RuleConstraints{ PathPattern: pathPattern, - Permissions: permissions, + Permissions: prompting.RulePermissionMap{ + "read": &prompting.RulePermissionEntry{Outcome: prompting.OutcomeAllow}, + "execute": &prompting.RulePermissionEntry{Outcome: prompting.OutcomeDeny}, + "append": &prompting.RulePermissionEntry{Outcome: prompting.OutcomeAllow}, + }, } - outcome := prompting.OutcomeAllow - satisfied, err := pdb.HandleNewRule(metadata, constraints, outcome) + satisfied, err := pdb.HandleNewRule(metadata, constraints) c.Assert(err, IsNil) c.Check(satisfied, HasLen, 2) - c.Check(promptIDListContains(satisfied, prompt2.ID), Equals, true) + c.Check(promptIDListContains(satisfied, prompt1.ID), Equals, true) c.Check(promptIDListContains(satisfied, prompt3.ID), Equals, true) - // Read and write permissions of prompt1 satisfied, so notice re-issued, - // but it has one remaining permission. prompt2 and prompt3 fully satisfied. - e1 := ¬iceInfo{promptID: prompt1.ID, data: nil} - e2 := ¬iceInfo{promptID: prompt2.ID, data: map[string]string{"resolved": "satisfied"}} + // Read permissions of prompt2 satisfied, but it has one remaining + // 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"}} + e2 := ¬iceInfo{promptID: prompt2.ID, data: nil} e3 := ¬iceInfo{promptID: prompt3.ID, data: map[string]string{"resolved": "satisfied"}} expectedNotices := []*noticeInfo{e1, e2, e3} s.checkNewNoticesUnordered(c, expectedNotices) @@ -683,15 +699,18 @@ func (s *requestpromptsSuite) TestHandleNewRuleAllowPermissions(c *C) { for i := 0; i < 2; i++ { satisfiedReq, allowedPermission, err := s.waitForListenerReqAndReply(c, listenerReqChan, replyChan) c.Check(err, IsNil) - var perms []string switch satisfiedReq { - case listenerReq2: - perms = permissions2 - case listenerReq3: - perms = permissions3 + case listenerReq1, listenerReq3: + break default: c.Errorf("unexpected request satisfied by new rule") } + // Only "read" permission was allowed for either prompt. + // prompt1 had requested "write" and "execute" as well, but because + // "execute" was denied and there was no rule pertaining to "write", + // the latter were both denied, leaving "read" as the only permission + // allowed. + perms := []string{"read"} expectedPerm, err := prompting.AbstractPermissionsToAppArmorPermissions(metadata.Interface, perms) c.Check(err, IsNil) c.Check(allowedPermission, DeepEquals, expectedPerm) @@ -701,25 +720,27 @@ func (s *requestpromptsSuite) TestHandleNewRuleAllowPermissions(c *C) { c.Assert(err, IsNil) c.Assert(stored, HasLen, 2) - // Check that allowing the final missing permission allows the prompt. - permissions = []string{"execute"} - constraints = &prompting.Constraints{ + // Check that allowing the final missing permission of prompt2 satisfies it + // with an allow response. + constraints = &prompting.RuleConstraints{ PathPattern: pathPattern, - Permissions: permissions, + Permissions: prompting.RulePermissionMap{ + "write": &prompting.RulePermissionEntry{Outcome: prompting.OutcomeAllow}, + }, } - satisfied, err = pdb.HandleNewRule(metadata, constraints, outcome) + satisfied, err = pdb.HandleNewRule(metadata, constraints) c.Assert(err, IsNil) c.Check(satisfied, HasLen, 1) - c.Check(satisfied[0], Equals, prompt1.ID) + c.Check(satisfied[0], Equals, prompt2.ID) expectedData := map[string]string{"resolved": "satisfied"} - s.checkNewNoticesSimple(c, []prompting.IDType{prompt1.ID}, expectedData) + s.checkNewNoticesSimple(c, []prompting.IDType{prompt2.ID}, expectedData) satisfiedReq, allowedPermission, err := s.waitForListenerReqAndReply(c, listenerReqChan, replyChan) c.Check(err, IsNil) - c.Check(satisfiedReq, Equals, listenerReq1) - expectedPerm, err := prompting.AbstractPermissionsToAppArmorPermissions(metadata.Interface, permissions1) + c.Check(satisfiedReq, Equals, listenerReq2) + expectedPerm, err := prompting.AbstractPermissionsToAppArmorPermissions(metadata.Interface, permissions2) c.Check(err, IsNil) c.Check(allowedPermission, DeepEquals, expectedPerm) } @@ -733,94 +754,6 @@ func promptIDListContains(haystack []prompting.IDType, needle prompting.IDType) return false } -func (s *requestpromptsSuite) TestHandleNewRuleDenyPermissions(c *C) { - listenerReqChan := make(chan *listener.Request, 3) - replyChan := make(chan any, 3) - restore := requestprompts.MockSendReply(func(listenerReq *listener.Request, allowedPermission any) error { - listenerReqChan <- listenerReq - replyChan <- allowedPermission - return nil - }) - defer restore() - - pdb, err := requestprompts.New(s.defaultNotifyPrompt) - c.Assert(err, IsNil) - defer pdb.Close() - - metadata := &prompting.Metadata{ - User: s.defaultUser, - Snap: "nextcloud", - Interface: "home", - } - path := "/home/test/Documents/foo.txt" - - permissions1 := []string{"read", "write", "execute"} - listenerReq1 := &listener.Request{} - prompt1, merged, err := pdb.AddOrMerge(metadata, path, permissions1, permissions1, listenerReq1) - c.Assert(err, IsNil) - c.Check(merged, Equals, false) - - permissions2 := []string{"read", "write"} - listenerReq2 := &listener.Request{} - prompt2, merged, err := pdb.AddOrMerge(metadata, path, permissions2, permissions2, listenerReq2) - c.Assert(err, IsNil) - c.Check(merged, Equals, false) - - permissions3 := []string{"read"} - listenerReq3 := &listener.Request{} - prompt3, merged, err := pdb.AddOrMerge(metadata, path, permissions3, permissions3, listenerReq3) - c.Assert(err, IsNil) - c.Check(merged, Equals, false) - - permissions4 := []string{"open"} - listenerReq4 := &listener.Request{} - prompt4, merged, err := pdb.AddOrMerge(metadata, path, permissions4, permissions4, listenerReq4) - c.Assert(err, IsNil) - c.Check(merged, Equals, false) - - s.checkNewNoticesSimple(c, []prompting.IDType{prompt1.ID, prompt2.ID, prompt3.ID, prompt4.ID}, nil) - - stored, err := pdb.Prompts(metadata.User) - c.Assert(err, IsNil) - c.Assert(stored, HasLen, 4) - - pathPattern, err := patterns.ParsePathPattern("/home/test/Documents/**") - c.Assert(err, IsNil) - permissions := []string{"read"} - constraints := &prompting.Constraints{ - PathPattern: pathPattern, - Permissions: permissions, - } - outcome := prompting.OutcomeDeny - - // If one or more permissions denied each for prompts 1-3, so each is denied - satisfied, err := pdb.HandleNewRule(metadata, constraints, outcome) - c.Assert(err, IsNil) - c.Check(satisfied, HasLen, 3) - c.Check(promptIDListContains(satisfied, prompt1.ID), Equals, true) - c.Check(promptIDListContains(satisfied, prompt2.ID), Equals, true) - c.Check(promptIDListContains(satisfied, prompt3.ID), Equals, true) - - expectedData := map[string]string{"resolved": "satisfied"} - s.checkNewNoticesUnorderedSimple(c, []prompting.IDType{prompt1.ID, prompt2.ID, prompt3.ID}, expectedData) - - for i := 0; i < 3; i++ { - satisfiedReq, allowedPermission, err := s.waitForListenerReqAndReply(c, listenerReqChan, replyChan) - c.Check(err, IsNil) - switch satisfiedReq { - case listenerReq1, listenerReq2, listenerReq3: - break - default: - c.Errorf("unexpected request satisfied by new rule") - } - c.Check(allowedPermission, DeepEquals, notify.FilePermission(0)) - } - - stored, err = pdb.Prompts(metadata.User) - c.Check(err, IsNil) - c.Check(stored, HasLen, 1) -} - func (s *requestpromptsSuite) TestHandleNewRuleNonMatches(c *C) { listenerReqChan := make(chan *listener.Request, 1) replyChan := make(chan any, 1) @@ -854,29 +787,38 @@ func (s *requestpromptsSuite) TestHandleNewRuleNonMatches(c *C) { pathPattern, err := patterns.ParsePathPattern("/home/test/Documents/**") c.Assert(err, IsNil) - constraints := &prompting.Constraints{ + constraints := &prompting.RuleConstraints{ PathPattern: pathPattern, - Permissions: permissions, + Permissions: prompting.RulePermissionMap{ + "read": &prompting.RulePermissionEntry{Outcome: prompting.OutcomeAllow}, + }, + } + + badOutcomeConstraints := &prompting.RuleConstraints{ + PathPattern: pathPattern, + Permissions: prompting.RulePermissionMap{ + "read": &prompting.RulePermissionEntry{Outcome: prompting.OutcomeType("foo")}, + }, } - outcome := prompting.OutcomeAllow otherUser := user + 1 otherSnap := "ldx" otherInterface := "system-files" otherPattern, err := patterns.ParsePathPattern("/home/test/Pictures/**.png") c.Assert(err, IsNil) - otherConstraints := &prompting.Constraints{ + otherConstraints := &prompting.RuleConstraints{ PathPattern: otherPattern, - Permissions: permissions, + Permissions: prompting.RulePermissionMap{ + "read": &prompting.RulePermissionEntry{Outcome: prompting.OutcomeAllow}, + }, } - badOutcome := prompting.OutcomeType("foo") stored, err := pdb.Prompts(metadata.User) c.Assert(err, IsNil) c.Assert(stored, HasLen, 1) c.Assert(stored[0], Equals, prompt) - satisfied, err := pdb.HandleNewRule(metadata, constraints, badOutcome) + satisfied, err := pdb.HandleNewRule(metadata, badOutcomeConstraints) c.Check(err, ErrorMatches, `invalid outcome: "foo"`) c.Check(satisfied, IsNil) @@ -887,7 +829,7 @@ func (s *requestpromptsSuite) TestHandleNewRuleNonMatches(c *C) { Snap: snap, Interface: iface, } - satisfied, err = pdb.HandleNewRule(otherUserMetadata, constraints, outcome) + satisfied, err = pdb.HandleNewRule(otherUserMetadata, constraints) c.Check(err, IsNil) c.Check(satisfied, IsNil) @@ -898,7 +840,7 @@ func (s *requestpromptsSuite) TestHandleNewRuleNonMatches(c *C) { Snap: otherSnap, Interface: iface, } - satisfied, err = pdb.HandleNewRule(otherSnapMetadata, constraints, outcome) + satisfied, err = pdb.HandleNewRule(otherSnapMetadata, constraints) c.Check(err, IsNil) c.Check(satisfied, IsNil) @@ -909,19 +851,19 @@ func (s *requestpromptsSuite) TestHandleNewRuleNonMatches(c *C) { Snap: snap, Interface: otherInterface, } - satisfied, err = pdb.HandleNewRule(otherInterfaceMetadata, constraints, outcome) + satisfied, err = pdb.HandleNewRule(otherInterfaceMetadata, constraints) c.Check(err, IsNil) c.Check(satisfied, IsNil) s.checkNewNoticesSimple(c, []prompting.IDType{}, nil) - satisfied, err = pdb.HandleNewRule(metadata, otherConstraints, outcome) + satisfied, err = pdb.HandleNewRule(metadata, otherConstraints) c.Check(err, IsNil) c.Check(satisfied, IsNil) s.checkNewNoticesSimple(c, []prompting.IDType{}, nil) - satisfied, err = pdb.HandleNewRule(metadata, constraints, outcome) + satisfied, err = pdb.HandleNewRule(metadata, constraints) c.Check(err, IsNil) c.Assert(satisfied, HasLen, 1) @@ -1026,7 +968,7 @@ func (s *requestpromptsSuite) TestCloseThenOperate(c *C) { c.Check(err, Equals, prompting_errors.ErrPromptsClosed) c.Check(result, IsNil) - promptIDs, err := pdb.HandleNewRule(nil, nil, prompting.OutcomeDeny) + promptIDs, err := pdb.HandleNewRule(nil, nil) c.Check(err, Equals, prompting_errors.ErrPromptsClosed) c.Check(promptIDs, IsNil) diff --git a/interfaces/prompting/requestrules/export_test.go b/interfaces/prompting/requestrules/export_test.go index 04523754cdd..1f27b0803ae 100644 --- a/interfaces/prompting/requestrules/export_test.go +++ b/interfaces/prompting/requestrules/export_test.go @@ -27,6 +27,6 @@ var JoinInternalErrors = joinInternalErrors type RulesDBJSON rulesDBJSON -func (rule *Rule) Validate(currTime time.Time) error { +func (rule *Rule) Validate(currTime time.Time) (expired bool, err error) { return rule.validate(currTime) } diff --git a/interfaces/prompting/requestrules/requestrules.go b/interfaces/prompting/requestrules/requestrules.go index 2504198b07d..9ae8204f103 100644 --- a/interfaces/prompting/requestrules/requestrules.go +++ b/interfaces/prompting/requestrules/requestrules.go @@ -41,64 +41,39 @@ import ( // Rule stores the contents of a request rule. type Rule struct { - ID prompting.IDType `json:"id"` - Timestamp time.Time `json:"timestamp"` - User uint32 `json:"user"` - Snap string `json:"snap"` - Interface string `json:"interface"` - Constraints *prompting.Constraints `json:"constraints"` - Outcome prompting.OutcomeType `json:"outcome"` - Lifespan prompting.LifespanType `json:"lifespan"` - Expiration time.Time `json:"expiration,omitempty"` + ID prompting.IDType `json:"id"` + Timestamp time.Time `json:"timestamp"` + User uint32 `json:"user"` + Snap string `json:"snap"` + Interface string `json:"interface"` + Constraints *prompting.RuleConstraints `json:"constraints"` } -// Validate verifies internal correctness of the rule -func (rule *Rule) validate(currTime time.Time) error { - if err := rule.Constraints.ValidateForInterface(rule.Interface); err != nil { - return err - } - if _, err := rule.Outcome.AsBool(); err != nil { - return err - } - if rule.Lifespan == prompting.LifespanSingle { - // We don't allow rules with lifespan "single" - return prompting_errors.NewRuleLifespanSingleError(prompting.SupportedRuleLifespans) - } - if err := rule.Lifespan.ValidateExpiration(rule.Expiration, currTime); err != nil { - // Should never error due to an API request, since rules are always - // added via the API using duration, rather than expiration. - // Error may occur when validating a rule loaded from disk. - return err - } - return nil +// Validate verifies internal correctness of the rule's constraints and +// permissions and prunes any expired permissions. If all permissions are +// expired, then returns true. If the rule is invalid, returns an error. +func (rule *Rule) validate(currTime time.Time) (expired bool, err error) { + return rule.Constraints.ValidateForInterface(rule.Interface, currTime) } -// Expired returns true if the receiving rule has a lifespan of timespan and -// the current time is after the rule's expiration time. -func (rule *Rule) Expired(currentTime time.Time) bool { - switch rule.Lifespan { - case prompting.LifespanTimespan: - if currentTime.After(rule.Expiration) { - return true - } - // TODO: add lifespan session - //case prompting.LifespanSession: - // TODO: return true if the user session has changed - } - return false +// expired returns true if all permissions for the receiving rule have expired. +func (rule *Rule) expired(currTime time.Time) bool { + return rule.Constraints.Permissions.Expired(currTime) } // variantEntry stores the actual pattern variant struct which can be used to -// match paths, and the set of rule IDs whose path patterns render to this -// variant. All rules in a particular entry must have the same outcome, and -// that outcome is stored directly in the variant entry itself. +// match paths, and a map from rule IDs whose path patterns render to this +// variant to the relevant permission entry from that rule. All non-expired +// permission entry values in the map must have the same outcome (as long as +// the entry has not expired), and that outcome is also stored directly in the +// variant entry itself. // // Use the rendered string as the key for this entry, since pattern variants // cannot otherwise be easily checked for equality. type variantEntry struct { - Variant patterns.PatternVariant - Outcome prompting.OutcomeType - RuleIDs map[prompting.IDType]bool + Variant patterns.PatternVariant + Outcome prompting.OutcomeType + RuleEntries map[prompting.IDType]*prompting.RulePermissionEntry } // permissionDB stores a map from path pattern variant to the ID of the rule @@ -140,10 +115,9 @@ type RuleDB struct { indexByID map[prompting.IDType]int rules []*Rule - // the incoming request queries are made in the context of a user, snap, - // snap interface, path, so this is essential a secondary compound index - // made of all those properties for being able to identify a rule - // matching given query + // Rules are stored in a tree according to user, snap, interface, and + // permission to simplify the process of checking whether a given request + // is matched by existing rules, and which of those rules has precedence. perUser map[uint32]*userDB dbPath string @@ -208,7 +182,7 @@ func (rdb *RuleDB) load() (retErr error) { rdb.rules = make([]*Rule, 0) rdb.perUser = make(map[uint32]*userDB) - expiredRules := make(map[*Rule]bool) + expiredRules := make(map[prompting.IDType]bool) f, err := os.Open(rdb.dbPath) if err != nil { @@ -227,27 +201,26 @@ func (rdb *RuleDB) load() (retErr error) { loadErr := fmt.Errorf("cannot read stored request rules: %w", err) // Save the empty rule DB to disk to overwrite the previous one which // could not be decoded. - return errorsJoin(loadErr, rdb.save()) + return prompting_errors.Join(loadErr, rdb.save()) } currTime := time.Now() var errInvalid error for _, rule := range wrapped.Rules { - if rule.Expired(currTime) { - expiredRules[rule] = true - continue - } - // If an expired rule happens to be invalid, it's fine, since we remove - // it anyway. - - if err := rule.validate(currTime); err != nil { + expired, err := rule.validate(currTime) + if err != nil { // we're loading previously saved rules, so this should not happen errInvalid = fmt.Errorf("internal error: %w", err) break } + if expired { + expiredRules[rule.ID] = true + continue + } - if conflictErr := rdb.addRule(rule); conflictErr != nil { + conflictErr := rdb.addRule(rule) + if conflictErr != nil { // Duplicate rules on disk or conflicting rule, should not occur errInvalid = fmt.Errorf("cannot add rule: %w", conflictErr) break @@ -266,13 +239,13 @@ func (rdb *RuleDB) load() (retErr error) { // Save the empty rule DB to disk to overwrite the previous one which // was invalid. - return errorsJoin(errInvalid, rdb.save()) + return prompting_errors.Join(errInvalid, rdb.save()) } expiredData := map[string]string{"removed": "expired"} for _, rule := range wrapped.Rules { var data map[string]string - if expiredRules[rule] { + if expiredRules[rule.ID] { data = expiredData } rdb.notifyRule(rule.User, rule.ID, data) @@ -306,6 +279,8 @@ func (rdb *RuleDB) lookupRuleByID(id prompting.IDType) (*Rule, error) { if !exists { return nil, prompting_errors.ErrRuleNotFound } + // XXX: should we check whether a rule is expired and throw ErrRuleNotFound + // if so? if index >= len(rdb.rules) { // Internal inconsistency between rules list and IDs map, should not occur return nil, prompting_errors.ErrRuleDBInconsistent @@ -331,22 +306,24 @@ func (rdb *RuleDB) addRuleToRulesList(rule *Rule) error { return nil } -// addRule adds the given rule to the rule DB. If there is a conflicting rule, -// returns an error, and the rule DB is left unchanged. +// addRule adds the given rule to the rule DB. +// +// If there is a conflicting rule, 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 { if err := rdb.addRuleToRulesList(rule); err != nil { return err } - err := rdb.addRuleToTree(rule) - if err == nil { + conflictErr := rdb.addRuleToTree(rule) + if conflictErr == nil { 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 err + return conflictErr } // removeRuleByIDFromRulesList removes the rule with the given ID from the rules @@ -407,13 +384,16 @@ func (rdb *RuleDB) removeRuleByID(id prompting.IDType) (*Rule, error) { // If there are other rules which have a conflicting path pattern and // permission, returns an error with information about the conflicting rules. // +// Assumes that the rule has already been internally validated. No additional +// validation is done in this function, nor is the expiration of the permissions +// checked. +// // The caller must ensure that the database lock is held for writing. func (rdb *RuleDB) addRuleToTree(rule *Rule) *prompting_errors.RuleConflictError { addedPermissions := make([]string, 0, len(rule.Constraints.Permissions)) - var conflicts []prompting_errors.RuleConflict - for _, permission := range rule.Constraints.Permissions { - permConflicts := rdb.addRulePermissionToTree(rule, permission) + for permission, entry := range rule.Constraints.Permissions { + permConflicts := rdb.addRulePermissionToTree(rule, permission, entry) if len(permConflicts) > 0 { conflicts = append(conflicts, permConflicts...) continue @@ -448,18 +428,21 @@ func (rdb *RuleDB) addRuleToTree(rule *Rule) *prompting_errors.RuleConflictError // call are removed from the variant map, leaving it unchanged, and the list of // conflicts is returned. If there are no conflicts, returns nil. // -// Expired rules, whether their outcome conflicts with the new rule or not, are -// ignored and never treated as conflicts. If there are no conflicts with non- -// expired rules, then all expired rules are removed. If there is a conflict -// with a non-expired rule, then nothing about the rule DB state is changed, -// including expired rules. +// Rules which are expired according to the timestamp of the rule being added, +// whether their outcome conflicts with the new rule or not, are ignored and +// never treated as conflicts. If there are no conflicts with non-expired +// rules, then all expired rules are removed from the tree entry (though not +// removed from the rule DB as a whole, nor is a notice recorded). If there is +// a conflict with a non-expired rule, then nothing about the rule DB state is +// changed, including expired rules. // -// The caller must ensure that the database lock is held for writing. -func (rdb *RuleDB) addRulePermissionToTree(rule *Rule, permission string) []prompting_errors.RuleConflict { +// The caller must ensure that the database lock is held for writing, and that +// the given entry is not expired. +func (rdb *RuleDB) addRulePermissionToTree(rule *Rule, permission string, permissionEntry *prompting.RulePermissionEntry) []prompting_errors.RuleConflict { permVariants := rdb.ensurePermissionDBForUserSnapInterfacePermission(rule.User, rule.Snap, rule.Interface, permission) newVariantEntries := make(map[string]variantEntry, rule.Constraints.PathPattern.NumVariants()) - expiredRules := make(map[prompting.IDType]bool) + partiallyExpiredRules := make(map[prompting.IDType]bool) var conflicts []prompting_errors.RuleConflict addVariant := func(index int, variant patterns.PatternVariant) { @@ -467,28 +450,28 @@ func (rdb *RuleDB) addRulePermissionToTree(rule *Rule, permission string) []prom existingEntry, exists := permVariants.VariantEntries[variantStr] if !exists { newVariantEntries[variantStr] = variantEntry{ - Variant: variant, - Outcome: rule.Outcome, - RuleIDs: map[prompting.IDType]bool{rule.ID: true}, + Variant: variant, + Outcome: permissionEntry.Outcome, + RuleEntries: map[prompting.IDType]*prompting.RulePermissionEntry{rule.ID: permissionEntry}, } return } - newEntry := variantEntry{ - Variant: variant, - Outcome: rule.Outcome, - RuleIDs: make(map[prompting.IDType]bool, len(existingEntry.RuleIDs)+1), + newVariantEntry := variantEntry{ + Variant: variant, + Outcome: permissionEntry.Outcome, + RuleEntries: make(map[prompting.IDType]*prompting.RulePermissionEntry, len(existingEntry.RuleEntries)+1), } - newEntry.RuleIDs[rule.ID] = true - newVariantEntries[variantStr] = newEntry - for id := range existingEntry.RuleIDs { - if rdb.isRuleWithIDExpired(id, rule.Timestamp) { + newVariantEntry.RuleEntries[rule.ID] = permissionEntry + newVariantEntries[variantStr] = newVariantEntry + for id, entry := range existingEntry.RuleEntries { + if entry.Expired(rule.Timestamp) { // Don't preserve expired rules, and don't care if they conflict - expiredRules[id] = true + partiallyExpiredRules[id] = true continue } - if existingEntry.Outcome == rule.Outcome { + if existingEntry.Outcome == permissionEntry.Outcome { // Preserve non-expired rule which doesn't conflict - newEntry.RuleIDs[id] = true + newVariantEntry.RuleEntries[id] = entry continue } // Conflicting non-expired rule @@ -507,37 +490,36 @@ func (rdb *RuleDB) addRulePermissionToTree(rule *Rule, permission string) []prom return conflicts } - for ruleID := range expiredRules { - removedRule, err := rdb.removeRuleByID(ruleID) + expiredData := map[string]string{"removed": "expired"} + for ruleID := range partiallyExpiredRules { + maybeExpired, err := rdb.lookupRuleByIDForUser(rule.User, ruleID) + if err != nil { + // Error shouldn't occur. If it does, the rule was already removed + continue + } + // Already removed the rule's permission from the tree, let's remove + // it from the rule as well + delete(maybeExpired.Constraints.Permissions, permission) + if !maybeExpired.expired(rule.Timestamp) { + continue + } + _, err = rdb.removeRuleByID(ruleID) // Error shouldn't occur. If it does, the rule was already removed. if err == nil { - rdb.notifyRule(removedRule.User, removedRule.ID, - map[string]string{"removed": "expired"}) + rdb.notifyRule(maybeExpired.User, maybeExpired.ID, expiredData) } } - for variantStr, entry := range newVariantEntries { + for variantStr, variantEntry := range newVariantEntries { // Replace the old variant entries with the new ones. // This removes any expired rules from the entries, since these were // not preserved in the new variant entries. - permVariants.VariantEntries[variantStr] = entry + permVariants.VariantEntries[variantStr] = variantEntry } return nil } -// isRuleWithIDExpired returns true if the rule with given ID is expired with respect -// to the provided timestamp, or if it otherwise no longer exists. -// -// The caller must ensure that the database lock is held for writing. -func (rdb *RuleDB) isRuleWithIDExpired(id prompting.IDType, currTime time.Time) bool { - rule, err := rdb.lookupRuleByID(id) - if err != nil { - return true - } - return rule.Expired(currTime) -} - // removeRuleFromTree fully removes the given rule from the rules tree, even if // an error occurs. Whenever possible, it is preferred to use `removeRuleByID` // directly instead, since it ensures consistency between the rules list and the @@ -546,11 +528,11 @@ func (rdb *RuleDB) isRuleWithIDExpired(id prompting.IDType, currTime time.Time) // The caller must ensure that the database lock is held for writing. func (rdb *RuleDB) removeRuleFromTree(rule *Rule) error { var errs []error - for _, permission := range rule.Constraints.Permissions { - if es := rdb.removeRulePermissionFromTree(rule, permission); len(es) > 0 { + for permission := range rule.Constraints.Permissions { + if err := rdb.removeRulePermissionFromTree(rule, permission); err != nil { // Database was left inconsistent, should not occur. // Store the errors, but keep removing. - errs = append(errs, es...) + errs = append(errs, err) } } return joinInternalErrors(errs) @@ -565,14 +547,13 @@ func (rdb *RuleDB) removeRuleFromTree(rule *Rule) error { // errors which occurred. // // The caller must ensure that the database lock is held for writing. -func (rdb *RuleDB) removeRulePermissionFromTree(rule *Rule, permission string) []error { +func (rdb *RuleDB) removeRulePermissionFromTree(rule *Rule, permission string) error { permVariants, ok := rdb.permissionDBForUserSnapInterfacePermission(rule.User, rule.Snap, rule.Interface, permission) if !ok || permVariants == nil { err := fmt.Errorf("internal error: no rules in the rule tree for user %d, snap %q, interface %q, permission %q", rule.User, rule.Snap, rule.Interface, permission) - return []error{err} + return err } seenVariants := make(map[string]bool, rule.Constraints.PathPattern.NumVariants()) - var errs []error removeVariant := func(index int, variant patterns.PatternVariant) { variantStr := variant.String() if seenVariants[variantStr] { @@ -581,23 +562,25 @@ func (rdb *RuleDB) removeRulePermissionFromTree(rule *Rule, permission string) [ seenVariants[variantStr] = true variantEntry, exists := permVariants.VariantEntries[variantStr] if !exists { - // Database was left inconsistent, should not occur - errs = append(errs, fmt.Errorf(`internal error: path pattern variant not found in the rule tree: %q`, variant)) + // If doesn't exist, could have been removed due to another rule's + // variant being removed and, finding all other rules' permissions + // for this variant expired, removing the variant entry. + return } - delete(variantEntry.RuleIDs, rule.ID) - if len(variantEntry.RuleIDs) == 0 { + delete(variantEntry.RuleEntries, rule.ID) + if len(variantEntry.RuleEntries) == 0 { delete(permVariants.VariantEntries, variantStr) } } rule.Constraints.PathPattern.RenderAllVariants(removeVariant) - return errs + return nil } // joinInternalErrors wraps a prompting_errors.ErrRuleDBInconsistent with the given errors. // // If there are no non-nil errors in the given errs list, return nil. func joinInternalErrors(errs []error) error { - joinedErr := errorsJoin(errs...) + joinedErr := prompting_errors.Join(errs...) if joinedErr == nil { return nil } @@ -605,28 +588,6 @@ func joinInternalErrors(errs []error) error { return fmt.Errorf("%w\n%v", prompting_errors.ErrRuleDBInconsistent, joinedErr) } -// errorsJoin returns an error that wraps the given errors. -// Any nil error values are discarded. -// errorsJoin returns nil if every value in errs is nil. -// -// TODO: replace with errors.Join() once we're on golang v1.20+ -func errorsJoin(errs ...error) error { - var nonNilErrs []error - for _, e := range errs { - if e != nil { - nonNilErrs = append(nonNilErrs, e) - } - } - if len(nonNilErrs) == 0 { - return nil - } - err := nonNilErrs[0] - for _, e := range nonNilErrs[1:] { - err = fmt.Errorf("%w\n%v", err, e) - } - return err -} - // permissionDBForUserSnapInterfacePermission returns the permission DB for the // given user, snap, interface, and permission, if it exists. // @@ -707,7 +668,7 @@ func (rdb *RuleDB) Close() error { // Creates a rule with the given information and adds it to the rule database. // If any of the given parameters are invalid, returns an error. Otherwise, // returns the newly-added rule, and saves the database to disk. -func (rdb *RuleDB) AddRule(user uint32, snap string, iface string, constraints *prompting.Constraints, outcome prompting.OutcomeType, lifespan prompting.LifespanType, duration string) (*Rule, error) { +func (rdb *RuleDB) AddRule(user uint32, snap string, iface string, constraints *prompting.Constraints) (*Rule, error) { rdb.mutex.Lock() defer rdb.mutex.Unlock() @@ -715,11 +676,14 @@ func (rdb *RuleDB) AddRule(user uint32, snap string, iface string, constraints * return nil, prompting_errors.ErrRulesClosed } - newRule, err := rdb.makeNewRule(user, snap, iface, constraints, outcome, lifespan, duration) + newRule, err := rdb.makeNewRule(user, snap, iface, 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. return nil, fmt.Errorf("cannot add rule: %w", err) } @@ -738,40 +702,29 @@ func (rdb *RuleDB) AddRule(user uint32, snap string, iface string, constraints * // makeNewRule creates a new Rule with the given contents. // -// Users of requestrules should probably autofill rules from JSON and never call -// this function directly. -// -// Constructs a new rule with the given parameters as values, with the -// exception of duration. Uses the given duration, in addition to the current -// time, to compute the expiration time for the rule, and stores that as part -// of the rule which is returned. If any of the given parameters are invalid, -// returns a corresponding error. -func (rdb *RuleDB) makeNewRule(user uint32, snap string, iface string, constraints *prompting.Constraints, outcome prompting.OutcomeType, lifespan prompting.LifespanType, duration string) (*Rule, error) { +// Constructs a new rule with the given parameters as values. The given +// constraints are converted to rule constraints, using the timestamp of the +// new rule as the baseline with which to compute an expiration from any given +// duration. If any of the given parameters are invalid, returns an error. +func (rdb *RuleDB) makeNewRule(user uint32, snap string, iface string, constraints *prompting.Constraints) (*Rule, error) { currTime := time.Now() - expiration, err := lifespan.ParseDuration(duration, currTime) + ruleConstraints, err := constraints.ToRuleConstraints(iface, currTime) if err != nil { 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, Interface: iface, - Constraints: constraints, - Outcome: outcome, - Lifespan: lifespan, - Expiration: expiration, + Constraints: ruleConstraints, } - if err := newRule.validate(currTime); err != nil { - return nil, err - } - - // Don't consume an ID until now, when we know the rule is valid - id, _ := rdb.maxIDMmap.NextID() - newRule.ID = id - return &newRule, nil } @@ -792,8 +745,8 @@ func (rdb *RuleDB) IsPathAllowed(user uint32, snap string, iface string, path st currTime := time.Now() for variantStr, variantEntry := range variantMap { nonExpired := false - for id := range variantEntry.RuleIDs { - if !rdb.isRuleWithIDExpired(id, currTime) { + for _, rulePermissionEntry := range variantEntry.RuleEntries { + if !rulePermissionEntry.Expired(currTime) { nonExpired = true break } @@ -857,7 +810,11 @@ func (rdb *RuleDB) rulesInternal(ruleFilter func(rule *Rule) bool) []*Rule { rules := make([]*Rule, 0) currTime := time.Now() for _, rule := range rdb.rules { - if rule.Expired(currTime) { + if rule.expired(currTime) { + // XXX: it would be nice if we pruned expired permissions from a + // rule before including it in the rules list, if it's not expired. + // Since we don't hold the write lock, we don't want to + // automatically prune expired permissions here. Should this change? continue } @@ -1038,16 +995,28 @@ func (rdb *RuleDB) RemoveRulesForSnapInterface(user uint32, snap string, iface s return rules, nil } -// PatchRule modifies the rule with the given ID by updating the rule's fields -// corresponding to any of the given parameters which are set/non-empty. +// PatchRule modifies the rule with the given ID by updating the rule's +// constraints for any constraint field or permission which is set/non-empty. +// +// If the path pattern is nil, it is left unchanged from the existing rule. +// Any permissions which are omitted from the new permissions map are left +// unchanged from the existing rule. To remove an existing permission from +// the rule, the permission should map to an empty permission entry. // -// Any of the parameters which are equal to the default/unset value for their -// types are left unchanged from the existing rule. Even if the given new rule -// contents exactly match the existing rule contents, the timestamp of the rule -// is updated to the current time. If there is any error while modifying the -// rule, the rule is rolled back to its previous unmodified state, leaving the -// database unchanged. If the database is changed, it is saved to disk. -func (rdb *RuleDB) PatchRule(user uint32, id prompting.IDType, constraints *prompting.Constraints, outcome prompting.OutcomeType, lifespan prompting.LifespanType, duration string) (r *Rule, err error) { +// Permission entries must be provided as complete units, containing both +// outcome and lifespan (and duration, if lifespan is timespan). +// XXX: does API unmarshalling ensures this, or do we need explicit checks? +// +// Even if the given new rule contents exactly match the existing rule contents, +// the timestamp of the rule is updated to the current time. If there is any +// error while modifying the rule, the rule is rolled back to its previous +// unmodified state, leaving the database unchanged. If the database is changed, +// it is saved to disk. +// +// XXX: should we just remove this method entirely? +// Clients can always delete a rule and re-add it later, which is basically what +// this method already does. +func (rdb *RuleDB) PatchRule(user uint32, id prompting.IDType, patchConstraints *prompting.PatchConstraints) (r *Rule, err error) { rdb.mutex.Lock() defer rdb.mutex.Unlock() @@ -1059,21 +1028,31 @@ func (rdb *RuleDB) PatchRule(user uint32, id prompting.IDType, constraints *prom if err != nil { return nil, err } - if constraints == nil { - constraints = origRule.Constraints - } - if outcome == prompting.OutcomeUnset { - outcome = origRule.Outcome - } - if lifespan == prompting.LifespanUnset { - lifespan = origRule.Lifespan - } - newRule, err := rdb.makeNewRule(user, origRule.Snap, origRule.Interface, constraints, outcome, lifespan, duration) + // XXX: we don't currently check whether the rule is expired or not. + // Do we want to support patching a rule for which all the permissions + // have already expired? Or say if a rule has already expired, we don't + // support patching it? Currently, we don't include fully expired rules + // in the output of Rules(), should the same be done here? + + currTime := time.Now() + + if patchConstraints == nil { + patchConstraints = &prompting.PatchConstraints{} + } + ruleConstraints, err := patchConstraints.PatchRuleConstraints(origRule.Constraints, origRule.Interface, currTime) if err != nil { return nil, err } - newRule.ID = origRule.ID + + newRule := &Rule{ + ID: origRule.ID, + Timestamp: currTime, + User: origRule.User, + Snap: origRule.Snap, + Interface: origRule.Interface, + Constraints: ruleConstraints, + } // Remove the existing rule from the tree. An error should not occur, since // we just looked up the rule and know it exists. @@ -1084,7 +1063,7 @@ func (rdb *RuleDB) PatchRule(user uint32, id prompting.IDType, constraints *prom // Try to re-add original rule so all is unchanged. if origErr := rdb.addRule(origRule); origErr != nil { // Error should not occur, but if it does, wrap it in the other error - err = errorsJoin(err, fmt.Errorf("cannot re-add original rule: %w", origErr)) + err = prompting_errors.Join(err, fmt.Errorf("cannot re-add original rule: %w", origErr)) } return nil, err } diff --git a/interfaces/prompting/requestrules/requestrules_test.go b/interfaces/prompting/requestrules/requestrules_test.go index eb3d1f477f5..2d91cb136b8 100644 --- a/interfaces/prompting/requestrules/requestrules_test.go +++ b/interfaces/prompting/requestrules/requestrules_test.go @@ -25,6 +25,7 @@ import ( "fmt" "os" "path/filepath" + "sort" "strings" "testing" "time" @@ -79,73 +80,12 @@ func (s *requestrulesSuite) SetUpTest(c *C) { c.Assert(os.MkdirAll(dirs.SnapdStateDir(dirs.GlobalRootDir), 0700), IsNil) } -func (s *requestrulesSuite) TestRuleValidate(c *C) { - iface := "home" - pathPattern := mustParsePathPattern(c, "/home/test/**") - - validConstraints := &prompting.Constraints{ - PathPattern: pathPattern, - Permissions: []string{"read"}, - } - invalidConstraints := &prompting.Constraints{ - PathPattern: pathPattern, - Permissions: []string{"foo"}, - } - - validOutcome := prompting.OutcomeAllow - invalidOutcome := prompting.OutcomeUnset - - validLifespan := prompting.LifespanTimespan - invalidLifespan := prompting.LifespanSingle - - currTime := time.Now() - - validExpiration := currTime.Add(time.Millisecond) - invalidExpiration := currTime.Add(-time.Millisecond) - - rule := requestrules.Rule{ - // ID is not validated - // Timestamp is not validated - // User is not validated - // Snap is not validated - Interface: iface, - Constraints: validConstraints, - Outcome: validOutcome, - Lifespan: validLifespan, - Expiration: validExpiration, - } - c.Check(rule.Validate(currTime), IsNil) - - rule.Expiration = invalidExpiration - c.Check(rule.Validate(currTime), ErrorMatches, fmt.Sprintf("%v:.*", prompting_errors.ErrRuleExpirationInThePast)) - - rule.Lifespan = invalidLifespan - c.Check(rule.Validate(currTime), ErrorMatches, prompting_errors.NewRuleLifespanSingleError(prompting.SupportedRuleLifespans).Error()) - - rule.Outcome = invalidOutcome - c.Check(rule.Validate(currTime), ErrorMatches, `invalid outcome: ""`) - - rule.Constraints = invalidConstraints - c.Check(rule.Validate(currTime), ErrorMatches, "invalid permissions for home interface:.*") -} - func mustParsePathPattern(c *C, patternStr string) *patterns.PathPattern { pattern, err := patterns.ParsePathPattern(patternStr) c.Assert(err, IsNil) return pattern } -func (s *requestrulesSuite) TestRuleExpired(c *C) { - currTime := time.Now() - rule := requestrules.Rule{ - // Other fields are not relevant - Lifespan: prompting.LifespanTimespan, - Expiration: currTime, - } - c.Check(rule.Expired(currTime), Equals, false) - c.Check(rule.Expired(currTime.Add(time.Millisecond)), Equals, true) -} - func (s *requestrulesSuite) TestNew(c *C) { rdb, err := requestrules.New(s.defaultNotifyRule) c.Assert(err, IsNil) @@ -237,6 +177,19 @@ func (s *requestrulesSuite) checkNewNotices(c *C, expectedNotices []*noticeInfo) s.ruleNotices = s.ruleNotices[:0] } +func (s *requestrulesSuite) checkNewNoticesUnordered(c *C, expectedNotices []*noticeInfo) { + sort.Slice(sortSliceParams(s.ruleNotices)) + sort.Slice(sortSliceParams(expectedNotices)) + s.checkNewNotices(c, expectedNotices) +} + +func sortSliceParams(list []*noticeInfo) ([]*noticeInfo, func(i, j int) bool) { + less := func(i, j int) bool { + return list[i].ruleID < list[j].ruleID + } + return list, less +} + func (s *requestrulesSuite) TestLoadErrorOpen(c *C) { dbPath := s.prepDBPath(c) // Create unreadable DB file to cause open failure @@ -257,10 +210,11 @@ func (s *requestrulesSuite) TestLoadErrorUnmarshal(c *C) { func (s *requestrulesSuite) TestLoadErrorValidate(c *C) { dbPath := s.prepDBPath(c) - good1 := s.ruleTemplate(c, prompting.IDType(1)) - bad := s.ruleTemplate(c, prompting.IDType(2)) + good1 := s.ruleTemplateWithRead(c, prompting.IDType(1)) + bad := s.ruleTemplateWithRead(c, prompting.IDType(2)) bad.Interface = "foo" // will cause validate() to fail with invalid constraints - good2 := s.ruleTemplate(c, prompting.IDType(3)) + good2 := s.ruleTemplateWithRead(c, prompting.IDType(3)) + good2.Constraints.Permissions["read"].Outcome = prompting.OutcomeDeny // Doesn't matter that rules have conflicting patterns/permissions, // validate() should catch invalid rule and exit before attempting to add. @@ -271,11 +225,22 @@ func (s *requestrulesSuite) TestLoadErrorValidate(c *C) { s.testLoadError(c, `internal error: invalid interface: "foo".*`, rules, checkWritten) } +// 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) + rule.Constraints.Permissions["read"] = &prompting.RulePermissionEntry{ + Outcome: prompting.OutcomeAllow, + Lifespan: prompting.LifespanForever, + // No expiration for lifespan forever + } + 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.Constraints{ + constraints := prompting.RuleConstraints{ PathPattern: mustParsePathPattern(c, "/home/test/foo"), - Permissions: []string{"read"}, + Permissions: make(prompting.RulePermissionMap), } rule := requestrules.Rule{ ID: id, @@ -284,9 +249,6 @@ func (s *requestrulesSuite) ruleTemplate(c *C, id prompting.IDType) *requestrule Snap: "firefox", Interface: "home", Constraints: &constraints, - Outcome: prompting.OutcomeAllow, - Lifespan: prompting.LifespanForever, - // Skip Expiration } return &rule } @@ -304,12 +266,13 @@ func (s *requestrulesSuite) writeRules(c *C, dbPath string, rules []*requestrule func (s *requestrulesSuite) TestLoadErrorConflictingID(c *C) { dbPath := s.prepDBPath(c) currTime := time.Now() - good := s.ruleTemplate(c, prompting.IDType(1)) - // Expired rules should still get a {"removed": "dropped"} notice, even if they don't conflict + 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)) - setPathPatternAndExpiration(c, expired, "/home/test/other", currTime.Add(-10*time.Second)) + expired.Constraints.PathPattern = mustParsePathPattern(c, "/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.ruleTemplate(c, prompting.IDType(1)) + conflicting := s.ruleTemplateWithRead(c, prompting.IDType(1)) conflicting.Constraints.PathPattern = mustParsePathPattern(c, "/home/test/another") rules := []*requestrules.Rule{good, expired, conflicting} @@ -319,24 +282,29 @@ func (s *requestrulesSuite) TestLoadErrorConflictingID(c *C) { s.testLoadError(c, fmt.Sprintf("cannot add rule: %v.*", prompting_errors.ErrRuleIDConflict), rules, checkWritten) } -func setPathPatternAndExpiration(c *C, rule *requestrules.Rule, pathPattern string, expiration time.Time) { - rule.Constraints.PathPattern = mustParsePathPattern(c, pathPattern) - rule.Lifespan = prompting.LifespanTimespan - rule.Expiration = expiration +func setPermissionsOutcomeLifespanExpiration(c *C, rule *requestrules.Rule, permissions []string, outcome prompting.OutcomeType, lifespan prompting.LifespanType, expiration time.Time) { + for _, perm := range permissions { + rule.Constraints.Permissions[perm] = &prompting.RulePermissionEntry{ + Outcome: outcome, + Lifespan: lifespan, + Expiration: expiration, + } + } } func (s *requestrulesSuite) TestLoadErrorConflictingPattern(c *C) { dbPath := s.prepDBPath(c) currTime := time.Now() - good := s.ruleTemplate(c, prompting.IDType(1)) - // Expired rules should still get a {"removed": "dropped"} notice, even if they don't conflict - expired := s.ruleTemplate(c, prompting.IDType(2)) - setPathPatternAndExpiration(c, expired, "/home/test/other", currTime.Add(-10*time.Second)) + good := s.ruleTemplateWithRead(c, prompting.IDType(1)) + // 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.ruleTemplate(c, prompting.IDType(3)) + conflicting := s.ruleTemplateWithRead(c, prompting.IDType(3)) // Even with not all permissions being in conflict, still error - conflicting.Constraints.Permissions = []string{"read", "write"} - conflicting.Outcome = prompting.OutcomeDeny + var timeZero time.Time + setPermissionsOutcomeLifespanExpiration(c, conflicting, []string{"read", "write"}, prompting.OutcomeDeny, prompting.LifespanForever, timeZero) rules := []*requestrules.Rule{good, expired, conflicting} s.writeRules(c, dbPath, rules) @@ -349,23 +317,26 @@ func (s *requestrulesSuite) TestLoadExpiredRules(c *C) { dbPath := s.prepDBPath(c) currTime := time.Now() - good1 := s.ruleTemplate(c, prompting.IDType(1)) + good1 := s.ruleTemplateWithRead(c, prompting.IDType(1)) // 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)) - setPathPatternAndExpiration(c, expired1, "/home/test/other", currTime.Add(-10*time.Second)) + expired1.Constraints.PathPattern = mustParsePathPattern(c, "/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)) - good2.Constraints.Permissions = []string{"write"} + var timeZero time.Time + setPermissionsOutcomeLifespanExpiration(c, good2, []string{"write"}, prompting.OutcomeDeny, prompting.LifespanForever, timeZero) expired2 := s.ruleTemplate(c, prompting.IDType(4)) - setPathPatternAndExpiration(c, expired2, "/home/test/another", currTime.Add(-time.Nanosecond)) + expired2.Constraints.PathPattern = mustParsePathPattern(c, "/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 - good3 := s.ruleTemplate(c, prompting.IDType(5)) + good3 := s.ruleTemplateWithRead(c, prompting.IDType(5)) good3.Constraints.PathPattern = mustParsePathPattern(c, "/home/test/no-conflict") rules := []*requestrules.Rule{good1, expired1, good2, expired2, good3} @@ -415,14 +386,14 @@ func (s *requestrulesSuite) TestLoadExpiredRules(c *C) { func (s *requestrulesSuite) TestLoadHappy(c *C) { dbPath := s.prepDBPath(c) - good1 := s.ruleTemplate(c, prompting.IDType(1)) + good1 := s.ruleTemplateWithRead(c, prompting.IDType(1)) // Rules with different users don't conflict - good2 := s.ruleTemplate(c, prompting.IDType(2)) + good2 := s.ruleTemplateWithRead(c, prompting.IDType(2)) good2.User = s.defaultUser + 1 // Rules with different snaps don't conflict - good3 := s.ruleTemplate(c, prompting.IDType(3)) + good3 := s.ruleTemplateWithRead(c, prompting.IDType(3)) good3.Snap = "thunderbird" rules := []*requestrules.Rule{good1, good2, good3} @@ -490,16 +461,21 @@ func (s *requestrulesSuite) TestCloseSaves(c *C) { // disk when DB is closed. constraints := &prompting.Constraints{ PathPattern: mustParsePathPattern(c, "/home/test/foo"), - Permissions: []string{"read"}, + Permissions: prompting.PermissionMap{ + "read": &prompting.PermissionEntry{ + Outcome: prompting.OutcomeAllow, + Lifespan: prompting.LifespanForever, + }, + }, } - rule, err := rdb.AddRule(s.defaultUser, "firefox", "home", constraints, prompting.OutcomeAllow, prompting.LifespanForever, "") + rule, err := rdb.AddRule(s.defaultUser, "firefox", "home", constraints) c.Assert(err, IsNil) // Check that new rule is on disk s.checkWrittenRuleDB(c, []*requestrules.Rule{rule}) // Mutate rule in memory - rule.Outcome = prompting.OutcomeDeny + rule.Constraints.Permissions["read"].Outcome = prompting.OutcomeDeny // Close DB c.Check(rdb.Close(), IsNil) @@ -611,11 +587,15 @@ func addRuleFromTemplate(c *C, rdb *requestrules.RuleDB, template *addRuleConten partial.Lifespan = template.Lifespan } // Duration default is empty string, so just use partial.Duration - constraints := &prompting.Constraints{ + replyConstraints := &prompting.ReplyConstraints{ PathPattern: mustParsePathPattern(c, partial.PathPattern), Permissions: partial.Permissions, } - return rdb.AddRule(partial.User, partial.Snap, partial.Interface, constraints, partial.Outcome, partial.Lifespan, partial.Duration) + constraints, err := replyConstraints.ToConstraints(partial.Interface, partial.Outcome, partial.Lifespan, partial.Duration) + if err != nil { + return nil, err + } + return rdb.AddRule(partial.User, partial.Snap, partial.Interface, constraints) } func (s *requestrulesSuite) TestAddRuleRemoveRuleDuplicateVariants(c *C) { @@ -844,6 +824,8 @@ func (s *requestrulesSuite) TestAddRuleExpired(c *C) { c.Assert(err, IsNil) c.Assert(good, NotNil) + // TODO: ADD test which tests behavior of rules which partially expire + // Add initial rule which will expire quickly prev, err := addRuleFromTemplate(c, rdb, template, &addRuleContents{ Lifespan: prompting.LifespanTimespan, @@ -1130,7 +1112,7 @@ func (s *requestrulesSuite) TestIsPathAllowedExpiration(c *C) { for i := len(addedRules) - 1; i >= 0; i-- { rule := addedRules[i] - expectedOutcome, err := rule.Outcome.AsBool() + expectedOutcome, err := rule.Constraints.Permissions["read"].Outcome.AsBool() c.Check(err, IsNil) // Check that the outcome of the most specific unexpired rule has precedence @@ -1142,24 +1124,24 @@ func (s *requestrulesSuite) TestIsPathAllowedExpiration(c *C) { s.checkNewNoticesSimple(c, nil) // Expire the highest precedence rule - rule.Expiration = time.Now() + rule.Constraints.Permissions["read"].Expiration = time.Now() } } func (s *requestrulesSuite) TestRuleWithID(c *C) { rdb, _ := requestrules.New(s.defaultNotifyRule) - snap := "lxd" - iface := "home" - constraints := &prompting.Constraints{ - PathPattern: mustParsePathPattern(c, "/home/test/Documents/**"), + template := &addRuleContents{ + User: s.defaultUser, + Snap: "lxd", + Interface: "home", + PathPattern: "/home/test/Documents/**", Permissions: []string{"read", "write", "execute"}, + Outcome: prompting.OutcomeAllow, + Lifespan: prompting.LifespanForever, } - outcome := prompting.OutcomeAllow - lifespan := prompting.LifespanForever - duration := "" - rule, err := rdb.AddRule(s.defaultUser, snap, iface, constraints, outcome, lifespan, duration) + rule, err := addRuleFromTemplate(c, rdb, template, template) c.Assert(err, IsNil) c.Assert(rule, NotNil) @@ -1246,12 +1228,13 @@ func (s *requestrulesSuite) TestRulesExpired(c *C) { rules := s.prepRuleDBForRulesForSnapInterface(c, rdb) // Set some rules to be expired - rules[0].Lifespan = prompting.LifespanTimespan - rules[0].Expiration = time.Now() - rules[2].Lifespan = prompting.LifespanTimespan - rules[2].Expiration = time.Now() - rules[4].Lifespan = prompting.LifespanTimespan - rules[4].Expiration = time.Now() + // This is brittle, relies on details of the rules added by prepRuleDBForRulesForSnapInterface + rules[0].Constraints.Permissions["read"].Lifespan = prompting.LifespanTimespan + rules[0].Constraints.Permissions["read"].Expiration = time.Now() + rules[2].Constraints.Permissions["read"].Lifespan = prompting.LifespanTimespan + rules[2].Constraints.Permissions["read"].Expiration = time.Now() + rules[4].Constraints.Permissions["read"].Lifespan = prompting.LifespanTimespan + rules[4].Constraints.Permissions["read"].Expiration = time.Now() // Expired rules are excluded from the Rules*() functions c.Check(rdb.Rules(s.defaultUser), DeepEquals, []*requestrules.Rule{rules[1], rules[3]}) @@ -1604,7 +1587,7 @@ func (s *requestrulesSuite) TestPatchRule(c *C) { origRule := *rule // Check that patching with no changes works fine, and updates timestamp - patched, err := rdb.PatchRule(rule.User, rule.ID, nil, prompting.OutcomeUnset, prompting.LifespanUnset, "") + patched, err := rdb.PatchRule(rule.User, rule.ID, nil) c.Assert(err, IsNil) s.checkWrittenRuleDB(c, append(rules[:len(rules)-1], patched)) s.checkNewNoticesSimple(c, nil, rule) @@ -1617,7 +1600,16 @@ func (s *requestrulesSuite) TestPatchRule(c *C) { rule = patched // Check that patching with identical content works fine, and updates timestamp - patched, err = rdb.PatchRule(rule.User, rule.ID, rule.Constraints, rule.Outcome, rule.Lifespan, "") + newConstraints := &prompting.PatchConstraints{ + PathPattern: rule.Constraints.PathPattern, + Permissions: prompting.PermissionMap{ + "read": &prompting.PermissionEntry{ + Outcome: rule.Constraints.Permissions["read"].Outcome, + Lifespan: rule.Constraints.Permissions["read"].Lifespan, + }, + }, + } + patched, err = rdb.PatchRule(rule.User, rule.ID, newConstraints) c.Assert(err, IsNil) s.checkWrittenRuleDB(c, append(rules[:len(rules)-1], patched)) s.checkNewNoticesSimple(c, nil, rule) @@ -1629,11 +1621,15 @@ func (s *requestrulesSuite) TestPatchRule(c *C) { rule = patched - newConstraints := &prompting.Constraints{ - PathPattern: rule.Constraints.PathPattern, - Permissions: []string{"read", "execute"}, + newConstraints = &prompting.PatchConstraints{ + Permissions: prompting.PermissionMap{ + "execute": &prompting.PermissionEntry{ + Outcome: rule.Constraints.Permissions["read"].Outcome, + Lifespan: rule.Constraints.Permissions["read"].Lifespan, + }, + }, } - patched, err = rdb.PatchRule(rule.User, rule.ID, newConstraints, prompting.OutcomeUnset, prompting.LifespanUnset, "") + patched, err = rdb.PatchRule(rule.User, rule.ID, newConstraints) c.Assert(err, IsNil) s.checkWrittenRuleDB(c, append(rules[:len(rules)-1], patched)) s.checkNewNoticesSimple(c, nil, rule) @@ -1641,12 +1637,36 @@ func (s *requestrulesSuite) TestPatchRule(c *C) { c.Check(patched.Timestamp.Equal(rule.Timestamp), Equals, false) // After making timestamp the same again, check that the rules are identical patched.Timestamp = rule.Timestamp - rule.Constraints = newConstraints + rule.Constraints = &prompting.RuleConstraints{ + PathPattern: patched.Constraints.PathPattern, + Permissions: prompting.RulePermissionMap{ + "read": &prompting.RulePermissionEntry{ + Outcome: patched.Constraints.Permissions["read"].Outcome, + Lifespan: patched.Constraints.Permissions["read"].Lifespan, + }, + "execute": &prompting.RulePermissionEntry{ + Outcome: patched.Constraints.Permissions["read"].Outcome, + Lifespan: patched.Constraints.Permissions["read"].Lifespan, + }, + }, + } c.Check(patched, DeepEquals, rule) rule = patched - patched, err = rdb.PatchRule(rule.User, rule.ID, nil, prompting.OutcomeDeny, prompting.LifespanUnset, "") + newConstraints = &prompting.PatchConstraints{ + Permissions: prompting.PermissionMap{ + "read": &prompting.PermissionEntry{ + Outcome: prompting.OutcomeDeny, + Lifespan: prompting.LifespanForever, + }, + "execute": &prompting.PermissionEntry{ + Outcome: prompting.OutcomeDeny, + Lifespan: prompting.LifespanForever, + }, + }, + } + patched, err = rdb.PatchRule(rule.User, rule.ID, newConstraints) c.Assert(err, IsNil) s.checkWrittenRuleDB(c, append(rules[:len(rules)-1], patched)) s.checkNewNoticesSimple(c, nil, rule) @@ -1654,12 +1674,27 @@ func (s *requestrulesSuite) TestPatchRule(c *C) { c.Check(patched.Timestamp.Equal(rule.Timestamp), Equals, false) // After making timestamp the same again, check that the rules are identical patched.Timestamp = rule.Timestamp - rule.Outcome = prompting.OutcomeDeny + rule.Constraints.Permissions["read"].Outcome = prompting.OutcomeDeny + rule.Constraints.Permissions["execute"].Outcome = prompting.OutcomeDeny c.Check(patched, DeepEquals, rule) rule = patched - patched, err = rdb.PatchRule(rule.User, rule.ID, nil, prompting.OutcomeUnset, prompting.LifespanTimespan, "10s") + newConstraints = &prompting.PatchConstraints{ + Permissions: prompting.PermissionMap{ + "read": &prompting.PermissionEntry{ + Outcome: prompting.OutcomeDeny, + Lifespan: prompting.LifespanTimespan, + Duration: "10s", + }, + "execute": &prompting.PermissionEntry{ + Outcome: prompting.OutcomeDeny, + Lifespan: prompting.LifespanTimespan, + Duration: "10s", + }, + }, + } + patched, err = rdb.PatchRule(rule.User, rule.ID, newConstraints) c.Assert(err, IsNil) s.checkWrittenRuleDB(c, append(rules[:len(rules)-1], patched)) s.checkNewNoticesSimple(c, nil, rule) @@ -1667,13 +1702,24 @@ func (s *requestrulesSuite) TestPatchRule(c *C) { c.Check(patched.Timestamp.Equal(rule.Timestamp), Equals, false) // After making timestamp the same again, check that the rules are identical patched.Timestamp = rule.Timestamp - rule.Lifespan = prompting.LifespanTimespan - rule.Expiration = patched.Expiration + rule.Constraints.Permissions["read"].Lifespan = prompting.LifespanTimespan + rule.Constraints.Permissions["execute"].Lifespan = prompting.LifespanTimespan + rule.Constraints.Permissions["read"].Expiration = patched.Constraints.Permissions["read"].Expiration + rule.Constraints.Permissions["execute"].Expiration = patched.Constraints.Permissions["execute"].Expiration c.Check(patched, DeepEquals, rule) rule = patched - patched, err = rdb.PatchRule(rule.User, rule.ID, origRule.Constraints, origRule.Outcome, origRule.Lifespan, "") + newConstraints = &prompting.PatchConstraints{ + Permissions: prompting.PermissionMap{ + "read": &prompting.PermissionEntry{ + Outcome: origRule.Constraints.Permissions["read"].Outcome, + Lifespan: origRule.Constraints.Permissions["read"].Lifespan, + }, + "execute": nil, + }, + } + patched, err = rdb.PatchRule(rule.User, rule.ID, newConstraints) c.Assert(err, IsNil) s.checkWrittenRuleDB(c, append(rules[:len(rules)-1], patched)) s.checkNewNoticesSimple(c, nil, rule) @@ -1717,33 +1763,52 @@ func (s *requestrulesSuite) TestPatchRuleErrors(c *C) { rule := rules[len(rules)-1] // Wrong user - result, err := rdb.PatchRule(rule.User+1, rule.ID, nil, prompting.OutcomeUnset, prompting.LifespanUnset, "") + result, err := rdb.PatchRule(rule.User+1, rule.ID, nil) c.Check(err, Equals, prompting_errors.ErrRuleNotAllowed) c.Check(result, IsNil) s.checkWrittenRuleDB(c, rules) s.checkNewNoticesSimple(c, nil) // Wrong ID - result, err = rdb.PatchRule(rule.User, prompting.IDType(1234), nil, prompting.OutcomeUnset, prompting.LifespanUnset, "") + result, err = rdb.PatchRule(rule.User, prompting.IDType(1234), nil) c.Check(err, Equals, prompting_errors.ErrRuleNotFound) c.Check(result, IsNil) s.checkWrittenRuleDB(c, rules) s.checkNewNoticesSimple(c, nil) // Invalid lifespan - result, err = rdb.PatchRule(rule.User, rule.ID, nil, prompting.OutcomeUnset, prompting.LifespanSingle, "") + badConstraints := &prompting.PatchConstraints{ + Permissions: prompting.PermissionMap{ + "read": &prompting.PermissionEntry{ + Outcome: prompting.OutcomeAllow, + Lifespan: prompting.LifespanSingle, + }, + }, + } + result, err = rdb.PatchRule(rule.User, rule.ID, badConstraints) c.Check(err, ErrorMatches, prompting_errors.NewRuleLifespanSingleError(prompting.SupportedRuleLifespans).Error()) c.Check(result, IsNil) s.checkWrittenRuleDB(c, rules) s.checkNewNoticesSimple(c, nil) // Conflicting rule - conflictingOutcome := prompting.OutcomeDeny - conflictingConstraints := &prompting.Constraints{ - PathPattern: mustParsePathPattern(c, template.PathPattern), - Permissions: []string{"read", "write", "execute"}, + conflictingConstraints := &prompting.PatchConstraints{ + Permissions: prompting.PermissionMap{ + "read": &prompting.PermissionEntry{ + Outcome: prompting.OutcomeDeny, + Lifespan: prompting.LifespanForever, + }, + "write": &prompting.PermissionEntry{ + Outcome: prompting.OutcomeDeny, + Lifespan: prompting.LifespanForever, + }, + "execute": &prompting.PermissionEntry{ + Outcome: prompting.OutcomeDeny, + Lifespan: prompting.LifespanForever, + }, + }, } - result, err = rdb.PatchRule(rule.User, rule.ID, conflictingConstraints, conflictingOutcome, prompting.LifespanUnset, "") + result, err = rdb.PatchRule(rule.User, rule.ID, conflictingConstraints) c.Check(err, ErrorMatches, fmt.Sprintf("cannot patch rule: %v", prompting_errors.ErrRuleConflict)) c.Check(result, IsNil) s.checkWrittenRuleDB(c, rules) @@ -1753,7 +1818,7 @@ func (s *requestrulesSuite) TestPatchRuleErrors(c *C) { func() { c.Assert(os.Chmod(prompting.StateDir(), 0o500), IsNil) defer os.Chmod(prompting.StateDir(), 0o700) - result, err = rdb.PatchRule(rule.User, rule.ID, nil, prompting.OutcomeUnset, prompting.LifespanUnset, "") + result, err = rdb.PatchRule(rule.User, rule.ID, nil) c.Check(err, NotNil) c.Check(result, IsNil) s.checkWrittenRuleDB(c, rules) @@ -1762,7 +1827,7 @@ func (s *requestrulesSuite) TestPatchRuleErrors(c *C) { // DB Closed c.Assert(rdb.Close(), IsNil) - result, err = rdb.PatchRule(rule.User, rule.ID, nil, prompting.OutcomeUnset, prompting.LifespanUnset, "") + result, err = rdb.PatchRule(rule.User, rule.ID, nil) c.Check(err, Equals, prompting_errors.ErrRulesClosed) c.Check(result, IsNil) s.checkWrittenRuleDB(c, rules) @@ -1803,11 +1868,23 @@ func (s *requestrulesSuite) TestPatchRuleExpired(c *C) { // Patching doesn't conflict with already-expired rules rule := rules[2] - newConstraints := &prompting.Constraints{ - PathPattern: mustParsePathPattern(c, template.PathPattern), - Permissions: []string{"read", "write", "execute"}, + newConstraints := &prompting.PatchConstraints{ + Permissions: prompting.PermissionMap{ + "read": &prompting.PermissionEntry{ + Outcome: prompting.OutcomeDeny, + Lifespan: prompting.LifespanForever, + }, + "write": &prompting.PermissionEntry{ + Outcome: prompting.OutcomeDeny, + Lifespan: prompting.LifespanForever, + }, + "execute": &prompting.PermissionEntry{ + Outcome: prompting.OutcomeDeny, + Lifespan: prompting.LifespanForever, + }, + }, } - patched, err := rdb.PatchRule(rule.User, rule.ID, newConstraints, prompting.OutcomeUnset, prompting.LifespanUnset, "") + patched, err := rdb.PatchRule(rule.User, rule.ID, newConstraints) c.Assert(err, IsNil) s.checkWrittenRuleDB(c, []*requestrules.Rule{patched}) expectedNotices := []*noticeInfo{ @@ -1827,11 +1904,24 @@ func (s *requestrulesSuite) TestPatchRuleExpired(c *C) { data: nil, }, } - s.checkNewNotices(c, expectedNotices) + 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 patched.Timestamp = rule.Timestamp - rule.Constraints = newConstraints + 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, + }, + } c.Check(patched, DeepEquals, rule) } diff --git a/overlord/ifacestate/apparmorprompting/prompting.go b/overlord/ifacestate/apparmorprompting/prompting.go index bef6cb609c9..2f7efb906d6 100644 --- a/overlord/ifacestate/apparmorprompting/prompting.go +++ b/overlord/ifacestate/apparmorprompting/prompting.go @@ -51,12 +51,12 @@ var ( type Manager interface { Prompts(userID uint32) ([]*requestprompts.Prompt, error) PromptWithID(userID uint32, promptID prompting.IDType) (*requestprompts.Prompt, error) - HandleReply(userID uint32, promptID prompting.IDType, constraints *prompting.Constraints, outcome prompting.OutcomeType, lifespan prompting.LifespanType, duration string) ([]prompting.IDType, error) + HandleReply(userID uint32, promptID prompting.IDType, replyConstraints *prompting.ReplyConstraints, outcome prompting.OutcomeType, lifespan prompting.LifespanType, duration string) ([]prompting.IDType, error) Rules(userID uint32, snap string, iface string) ([]*requestrules.Rule, error) - AddRule(userID uint32, snap string, iface string, constraints *prompting.Constraints, outcome prompting.OutcomeType, lifespan prompting.LifespanType, duration string) (*requestrules.Rule, error) + AddRule(userID uint32, snap string, iface string, constraints *prompting.Constraints) (*requestrules.Rule, error) RemoveRules(userID uint32, snap string, iface string) ([]*requestrules.Rule, error) RuleWithID(userID uint32, ruleID prompting.IDType) (*requestrules.Rule, error) - PatchRule(userID uint32, ruleID prompting.IDType, constraints *prompting.Constraints, outcome prompting.OutcomeType, lifespan prompting.LifespanType, duration string) (*requestrules.Rule, error) + PatchRule(userID uint32, ruleID prompting.IDType, constraints *prompting.PatchConstraints) (*requestrules.Rule, error) RemoveRule(userID uint32, ruleID prompting.IDType) (*requestrules.Rule, error) } @@ -221,6 +221,7 @@ func (m *InterfacesRequestsManager) handleListenerReq(req *listener.Request) err 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 @@ -311,29 +312,7 @@ func (m *InterfacesRequestsManager) disconnect() error { m.rules = nil } - return errorsJoin(errs...) -} - -// errorsJoin returns an error that wraps the given errors. -// Any nil error values are discarded. -// errorsJoin returns nil if every value in errs is nil. -// -// TODO: replace with errors.Join() once we're on golang v1.20+ -func errorsJoin(errs ...error) error { - var nonNilErrs []error - for _, e := range errs { - if e != nil { - nonNilErrs = append(nonNilErrs, e) - } - } - if len(nonNilErrs) == 0 { - return nil - } - err := nonNilErrs[0] - for _, e := range nonNilErrs[1:] { - err = fmt.Errorf("%w\n%v", err, e) - } - return err + return prompting_errors.Join(errs...) } // Stop closes the listener, prompt DB, and rule DB. Stop is idempotent, and @@ -363,7 +342,7 @@ func (m *InterfacesRequestsManager) PromptWithID(userID uint32, promptID prompti // is not "single"). If all of these are true, sends a reply for the prompt with // the given ID, and both creates a new rule and checks any outstanding prompts // against it, if the lifespan is not "single". -func (m *InterfacesRequestsManager) HandleReply(userID uint32, promptID prompting.IDType, constraints *prompting.Constraints, outcome prompting.OutcomeType, lifespan prompting.LifespanType, duration string) (satisfiedPromptIDs []prompting.IDType, retErr error) { +func (m *InterfacesRequestsManager) HandleReply(userID uint32, promptID prompting.IDType, replyConstraints *prompting.ReplyConstraints, outcome prompting.OutcomeType, lifespan prompting.LifespanType, duration string) (satisfiedPromptIDs []prompting.IDType, retErr error) { m.lock.Lock() defer m.lock.Unlock() @@ -372,10 +351,12 @@ func (m *InterfacesRequestsManager) HandleReply(userID uint32, promptID promptin return nil, err } - // Outcome and lifesnap are validated while unmarshalling, and duration is - // validated when the rule is being added. So only need to validate - // constraints. - if err := constraints.ValidateForInterface(prompt.Interface); err != nil { + // Validate reply constraints and convert them to Constraints, which have + // dedicated PermissionEntry values for each permission in the reply. + // Outcome and lifespan are validated while unmarshalling, and duration is + // validated against the given lifespan when constructing the Constraints. + constraints, err := replyConstraints.ToConstraints(prompt.Interface, outcome, lifespan, duration) + if err != nil { return nil, err } @@ -386,24 +367,24 @@ func (m *InterfacesRequestsManager) HandleReply(userID uint32, promptID promptin // constraints, such as check that the path pattern does not match // any paths not granted by the interface. // TODO: Should this be reconsidered? - matches, err := constraints.Match(prompt.Constraints.Path()) + matches, err := replyConstraints.Match(prompt.Constraints.Path()) if err != nil { return nil, err } if !matches { return nil, &prompting_errors.RequestedPathNotMatchedError{ Requested: prompt.Constraints.Path(), - Replied: constraints.PathPattern.String(), + Replied: replyConstraints.PathPattern.String(), } } // 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 := replyConstraints.ContainPermissions(prompt.Constraints.RemainingPermissions()) if !contained { return nil, &prompting_errors.RequestedPermissionsNotMatchedError{ Requested: prompt.Constraints.RemainingPermissions(), - Replied: constraints.Permissions, + Replied: replyConstraints.Permissions, } } @@ -413,7 +394,7 @@ func (m *InterfacesRequestsManager) HandleReply(userID uint32, promptID promptin var newRule *requestrules.Rule if lifespan != prompting.LifespanSingle { // Check that adding the rule doesn't conflict with other rules - newRule, err = m.rules.AddRule(userID, prompt.Snap, prompt.Interface, constraints, outcome, lifespan, duration) + newRule, err = m.rules.AddRule(userID, prompt.Snap, prompt.Interface, constraints) if err != nil { // Rule conflicts with existing rule (at least one identical pattern // variant and permission). This should be considered a bad reply, @@ -451,7 +432,7 @@ func (m *InterfacesRequestsManager) applyRuleToOutstandingPrompts(rule *requestr Snap: rule.Snap, Interface: rule.Interface, } - satisfiedPromptIDs, err := m.prompts.HandleNewRule(metadata, rule.Constraints, rule.Outcome) + satisfiedPromptIDs, err := m.prompts.HandleNewRule(metadata, rule.Constraints) if err != nil { // The rule's constraints and outcome were already validated, so an // error should not occur here unless the prompt DB was already closed. @@ -484,11 +465,11 @@ func (m *InterfacesRequestsManager) Rules(userID uint32, snap string, iface stri // AddRule creates a new rule with the given contents and then checks it against // outstanding prompts, resolving any prompts which it satisfies. -func (m *InterfacesRequestsManager) AddRule(userID uint32, snap string, iface string, constraints *prompting.Constraints, outcome prompting.OutcomeType, lifespan prompting.LifespanType, duration string) (*requestrules.Rule, error) { +func (m *InterfacesRequestsManager) AddRule(userID uint32, snap string, iface string, constraints *prompting.Constraints) (*requestrules.Rule, error) { m.lock.Lock() defer m.lock.Unlock() - newRule, err := m.rules.AddRule(userID, snap, iface, constraints, outcome, lifespan, duration) + newRule, err := m.rules.AddRule(userID, snap, iface, constraints) if err != nil { return nil, err } @@ -531,11 +512,11 @@ func (m *InterfacesRequestsManager) RuleWithID(userID uint32, ruleID prompting.I // PatchRule updates the rule with the given ID using the provided contents. // Any of the given fields which are empty/nil are not updated in the rule. -func (m *InterfacesRequestsManager) PatchRule(userID uint32, ruleID prompting.IDType, constraints *prompting.Constraints, outcome prompting.OutcomeType, lifespan prompting.LifespanType, duration string) (*requestrules.Rule, error) { +func (m *InterfacesRequestsManager) PatchRule(userID uint32, ruleID prompting.IDType, constraints *prompting.PatchConstraints) (*requestrules.Rule, error) { m.lock.Lock() defer m.lock.Unlock() - patchedRule, err := m.rules.PatchRule(userID, ruleID, constraints, outcome, lifespan, duration) + patchedRule, err := m.rules.PatchRule(userID, ruleID, constraints) if err != nil { return nil, err } diff --git a/overlord/ifacestate/apparmorprompting/prompting_test.go b/overlord/ifacestate/apparmorprompting/prompting_test.go index d67b0d7e027..e5716a45be5 100644 --- a/overlord/ifacestate/apparmorprompting/prompting_test.go +++ b/overlord/ifacestate/apparmorprompting/prompting_test.go @@ -270,7 +270,7 @@ func (s *apparmorpromptingSuite) TestHandleReplySimple(c *C) { req, prompt := s.simulateRequest(c, reqChan, mgr, &listener.Request{}, false) // Reply to the request - constraints := prompting.Constraints{ + constraints := prompting.ReplyConstraints{ PathPattern: mustParsePathPattern(c, "/home/test/**"), Permissions: []string{"read"}, } @@ -418,7 +418,7 @@ func (s *apparmorpromptingSuite) TestHandleReplyErrors(c *C) { c.Check(result, IsNil) // Invalid constraints - invalidConstraints := prompting.Constraints{ + invalidConstraints := prompting.ReplyConstraints{ PathPattern: mustParsePathPattern(c, "/home/test/**"), Permissions: []string{"foo"}, } @@ -427,7 +427,7 @@ func (s *apparmorpromptingSuite) TestHandleReplyErrors(c *C) { c.Check(result, IsNil) // Path not matched - badPatternConstraints := prompting.Constraints{ + badPatternConstraints := prompting.ReplyConstraints{ PathPattern: mustParsePathPattern(c, "/home/test/other"), Permissions: []string{"read"}, } @@ -436,7 +436,7 @@ func (s *apparmorpromptingSuite) TestHandleReplyErrors(c *C) { c.Check(result, IsNil) // Permissions not matched - badPermissionConstraints := prompting.Constraints{ + badPermissionConstraints := prompting.ReplyConstraints{ PathPattern: mustParsePathPattern(c, "/home/test/foo"), Permissions: []string{"write"}, } @@ -447,11 +447,21 @@ func (s *apparmorpromptingSuite) TestHandleReplyErrors(c *C) { // Conflicting rule // For this, need to add another rule to the DB first, then try to reply // with a rule which conflicts with it. Reuse badPatternConstraints. - newRule, err := mgr.AddRule(s.defaultUser, "firefox", "home", &badPatternConstraints, prompting.OutcomeAllow, prompting.LifespanTimespan, "10s") + anotherConstraints := prompting.Constraints{ + PathPattern: mustParsePathPattern(c, "/home/test/other"), + Permissions: prompting.PermissionMap{ + "read": &prompting.PermissionEntry{ + Outcome: prompting.OutcomeAllow, + Lifespan: prompting.LifespanTimespan, + Duration: "10s", + }, + }, + } + newRule, err := mgr.AddRule(s.defaultUser, "firefox", "home", &anotherConstraints) c.Assert(err, IsNil) c.Assert(newRule, NotNil) conflictingOutcome := prompting.OutcomeDeny - conflictingConstraints := prompting.Constraints{ + conflictingConstraints := prompting.ReplyConstraints{ PathPattern: mustParsePathPattern(c, "/home/test/{foo,other}"), Permissions: []string{"read"}, } @@ -476,17 +486,27 @@ func (s *apparmorpromptingSuite) TestExistingRuleAllowsNewPrompt(c *C) { // Add allow rule to match read permission constraints := &prompting.Constraints{ PathPattern: mustParsePathPattern(c, "/home/test/**"), - Permissions: []string{"read"}, - } - _, err = mgr.AddRule(s.defaultUser, "firefox", "home", constraints, prompting.OutcomeAllow, prompting.LifespanForever, "") + Permissions: prompting.PermissionMap{ + "read": &prompting.PermissionEntry{ + Outcome: prompting.OutcomeAllow, + Lifespan: prompting.LifespanForever, + }, + }, + } + _, err = mgr.AddRule(s.defaultUser, "firefox", "home", constraints) c.Assert(err, IsNil) // Add allow rule to match write permission constraints = &prompting.Constraints{ PathPattern: mustParsePathPattern(c, "/home/test/**"), - Permissions: []string{"write"}, - } - _, err = mgr.AddRule(s.defaultUser, "firefox", "home", constraints, prompting.OutcomeAllow, prompting.LifespanForever, "") + Permissions: prompting.PermissionMap{ + "write": &prompting.PermissionEntry{ + Outcome: prompting.OutcomeAllow, + Lifespan: prompting.LifespanForever, + }, + }, + } + _, err = mgr.AddRule(s.defaultUser, "firefox", "home", constraints) c.Assert(err, IsNil) // Create request for read and write @@ -524,7 +544,7 @@ func (s *apparmorpromptingSuite) checkRecordedPromptNotices(c *C, since time.Tim After: since, }) s.st.Unlock() - c.Check(n, HasLen, count) + c.Check(n, HasLen, count, Commentf("%+v", n)) } func (s *apparmorpromptingSuite) checkRecordedRuleUpdateNotices(c *C, since time.Time, count int) { @@ -534,7 +554,7 @@ func (s *apparmorpromptingSuite) checkRecordedRuleUpdateNotices(c *C, since time After: since, }) s.st.Unlock() - c.Check(n, HasLen, count) + c.Check(n, HasLen, count, Commentf("%+v", n)) } func (s *apparmorpromptingSuite) TestExistingRulePartiallyAllowsNewPrompt(c *C) { @@ -547,9 +567,14 @@ func (s *apparmorpromptingSuite) TestExistingRulePartiallyAllowsNewPrompt(c *C) // Add rule to match read permission constraints := &prompting.Constraints{ PathPattern: mustParsePathPattern(c, "/home/test/**"), - Permissions: []string{"read"}, - } - _, err = mgr.AddRule(s.defaultUser, "firefox", "home", constraints, prompting.OutcomeAllow, prompting.LifespanForever, "") + Permissions: prompting.PermissionMap{ + "read": &prompting.PermissionEntry{ + Outcome: prompting.OutcomeAllow, + Lifespan: prompting.LifespanForever, + }, + }, + } + _, err = mgr.AddRule(s.defaultUser, "firefox", "home", constraints) c.Assert(err, IsNil) // Do NOT add rule to match write permission @@ -576,9 +601,14 @@ func (s *apparmorpromptingSuite) TestExistingRulePartiallyDeniesNewPrompt(c *C) // Add deny rule to match read permission constraints := &prompting.Constraints{ PathPattern: mustParsePathPattern(c, "/home/test/**"), - Permissions: []string{"read"}, - } - _, err = mgr.AddRule(s.defaultUser, "firefox", "home", constraints, prompting.OutcomeDeny, prompting.LifespanForever, "") + Permissions: prompting.PermissionMap{ + "read": &prompting.PermissionEntry{ + Outcome: prompting.OutcomeDeny, + Lifespan: prompting.LifespanForever, + }, + }, + } + _, err = mgr.AddRule(s.defaultUser, "firefox", "home", constraints) c.Assert(err, IsNil) // Add no rule for write permissions @@ -619,17 +649,27 @@ func (s *apparmorpromptingSuite) TestExistingRulesMixedMatchNewPromptDenies(c *C // Add deny rule to match read permission constraints := &prompting.Constraints{ PathPattern: mustParsePathPattern(c, "/home/test/**"), - Permissions: []string{"read"}, - } - _, err = mgr.AddRule(s.defaultUser, "firefox", "home", constraints, prompting.OutcomeDeny, prompting.LifespanForever, "") + Permissions: prompting.PermissionMap{ + "read": &prompting.PermissionEntry{ + Outcome: prompting.OutcomeDeny, + Lifespan: prompting.LifespanForever, + }, + }, + } + _, err = mgr.AddRule(s.defaultUser, "firefox", "home", constraints) c.Assert(err, IsNil) // Add allow rule for write permissions constraints = &prompting.Constraints{ PathPattern: mustParsePathPattern(c, "/home/test/**"), - Permissions: []string{"write"}, - } - _, err = mgr.AddRule(s.defaultUser, "firefox", "home", constraints, prompting.OutcomeAllow, prompting.LifespanForever, "") + Permissions: prompting.PermissionMap{ + "write": &prompting.PermissionEntry{ + Outcome: prompting.OutcomeAllow, + Lifespan: prompting.LifespanForever, + }, + }, + } + _, err = mgr.AddRule(s.defaultUser, "firefox", "home", constraints) c.Assert(err, IsNil) // Create request for read and write @@ -694,9 +734,14 @@ func (s *apparmorpromptingSuite) TestNewRuleAllowExistingPrompt(c *C) { whenSent := time.Now() constraints := &prompting.Constraints{ PathPattern: mustParsePathPattern(c, "/home/test/**"), - Permissions: []string{"read"}, - } - rule, err := mgr.AddRule(s.defaultUser, "firefox", "home", constraints, prompting.OutcomeAllow, prompting.LifespanForever, "") + Permissions: prompting.PermissionMap{ + "read": &prompting.PermissionEntry{ + Outcome: prompting.OutcomeAllow, + Lifespan: prompting.LifespanForever, + }, + }, + } + rule, err := mgr.AddRule(s.defaultUser, "firefox", "home", constraints) c.Assert(err, IsNil) // Check that kernel received a reply @@ -767,9 +812,14 @@ func (s *apparmorpromptingSuite) TestNewRuleDenyExistingPrompt(c *C) { whenSent := time.Now() constraints := &prompting.Constraints{ PathPattern: mustParsePathPattern(c, "/home/test/**"), - Permissions: []string{"read"}, - } - rule, err := mgr.AddRule(s.defaultUser, "firefox", "home", constraints, prompting.OutcomeDeny, prompting.LifespanForever, "") + Permissions: prompting.PermissionMap{ + "read": &prompting.PermissionEntry{ + Outcome: prompting.OutcomeDeny, + Lifespan: prompting.LifespanForever, + }, + }, + } + rule, err := mgr.AddRule(s.defaultUser, "firefox", "home", constraints) c.Assert(err, IsNil) // Check that kernel received two replies @@ -834,7 +884,7 @@ func (s *apparmorpromptingSuite) TestReplyNewRuleHandlesExistingPrompt(c *C) { // Reply to read prompt with denial whenSent := time.Now() - constraints := &prompting.Constraints{ + constraints := &prompting.ReplyConstraints{ PathPattern: mustParsePathPattern(c, "/home/test/**"), Permissions: []string{"read"}, } @@ -915,7 +965,7 @@ func (s *apparmorpromptingSuite) testReplyRuleHandlesFuturePrompts(c *C, outcome // Reply to read prompt with denial whenSent := time.Now() - constraints := &prompting.Constraints{ + constraints := &prompting.ReplyConstraints{ PathPattern: mustParsePathPattern(c, "/home/test/**"), Permissions: []string{"read", "write"}, } @@ -1017,9 +1067,14 @@ func (s *apparmorpromptingSuite) TestRequestMerged(c *C) { // Add rule to satisfy the read permission constraints := &prompting.Constraints{ PathPattern: mustParsePathPattern(c, "/home/test/**"), - Permissions: []string{"read"}, - } - _, err = mgr.AddRule(s.defaultUser, prompt.Snap, prompt.Interface, constraints, prompting.OutcomeAllow, prompting.LifespanForever, "") + Permissions: prompting.PermissionMap{ + "read": &prompting.PermissionEntry{ + Outcome: prompting.OutcomeAllow, + Lifespan: prompting.LifespanForever, + }, + }, + } + _, err = mgr.AddRule(s.defaultUser, prompt.Snap, prompt.Interface, constraints) c.Assert(err, IsNil) // Create identical request again, it should merge even though some @@ -1077,27 +1132,42 @@ func (s *apparmorpromptingSuite) prepManagerWithRules(c *C) (mgr *apparmorprompt // Add rule for firefox and home constraints := &prompting.Constraints{ PathPattern: mustParsePathPattern(c, "/home/test/1"), - Permissions: []string{"read"}, - } - rule1, err := mgr.AddRule(s.defaultUser, "firefox", "home", constraints, prompting.OutcomeAllow, prompting.LifespanForever, "") + Permissions: prompting.PermissionMap{ + "read": &prompting.PermissionEntry{ + Outcome: prompting.OutcomeAllow, + Lifespan: prompting.LifespanForever, + }, + }, + } + rule1, err := mgr.AddRule(s.defaultUser, "firefox", "home", constraints) c.Assert(err, IsNil) rules = append(rules, rule1) // Add rule for thunderbird and home constraints = &prompting.Constraints{ PathPattern: mustParsePathPattern(c, "/home/test/2"), - Permissions: []string{"read"}, - } - rule2, err := mgr.AddRule(s.defaultUser, "thunderbird", "home", constraints, prompting.OutcomeAllow, prompting.LifespanForever, "") + Permissions: prompting.PermissionMap{ + "read": &prompting.PermissionEntry{ + Outcome: prompting.OutcomeAllow, + Lifespan: prompting.LifespanForever, + }, + }, + } + rule2, err := mgr.AddRule(s.defaultUser, "thunderbird", "home", constraints) c.Assert(err, IsNil) rules = append(rules, rule2) // Add rule for firefox and camera constraints = &prompting.Constraints{ PathPattern: mustParsePathPattern(c, "/home/test/3"), - Permissions: []string{"read"}, - } - rule3, err := mgr.AddRule(s.defaultUser, "firefox", "home", constraints, prompting.OutcomeAllow, prompting.LifespanForever, "") + Permissions: prompting.PermissionMap{ + "read": &prompting.PermissionEntry{ + Outcome: prompting.OutcomeAllow, + Lifespan: prompting.LifespanForever, + }, + }, + } + rule3, err := mgr.AddRule(s.defaultUser, "firefox", "home", constraints) c.Assert(err, IsNil) // Since camera interface isn't supported yet, must adjust the interface // after the rule has been created. This abuses implementation details of @@ -1108,9 +1178,14 @@ func (s *apparmorpromptingSuite) prepManagerWithRules(c *C) (mgr *apparmorprompt // Add rule for firefox and home, but for a different user constraints = &prompting.Constraints{ PathPattern: mustParsePathPattern(c, "/home/test/4"), - Permissions: []string{"read"}, - } - rule4, err := mgr.AddRule(s.defaultUser+1, "firefox", "home", constraints, prompting.OutcomeAllow, prompting.LifespanForever, "") + Permissions: prompting.PermissionMap{ + "read": &prompting.PermissionEntry{ + Outcome: prompting.OutcomeAllow, + Lifespan: prompting.LifespanForever, + }, + }, + } + rule4, err := mgr.AddRule(s.defaultUser+1, "firefox", "home", constraints) c.Assert(err, IsNil) rules = append(rules, rule4) @@ -1206,31 +1281,48 @@ func (s *apparmorpromptingSuite) TestAddRuleWithIDPatchRemove(c *C) { whenAdded := time.Now() constraints := &prompting.Constraints{ PathPattern: mustParsePathPattern(c, "/home/test/**"), - Permissions: []string{"write"}, - } - rule, err := mgr.AddRule(s.defaultUser, "firefox", "home", constraints, prompting.OutcomeAllow, prompting.LifespanForever, "") + Permissions: prompting.PermissionMap{ + "write": &prompting.PermissionEntry{ + Outcome: prompting.OutcomeAllow, + Lifespan: prompting.LifespanForever, + }, + }, + } + rule, err := mgr.AddRule(s.defaultUser, "firefox", "home", constraints) c.Assert(err, IsNil) s.checkRecordedRuleUpdateNotices(c, whenAdded, 1) + s.checkRecordedPromptNotices(c, whenAdded, 0) // Test RuleWithID + whenAccessed := time.Now() retrieved, err := mgr.RuleWithID(rule.User, rule.ID) c.Assert(err, IsNil) c.Assert(retrieved, Equals, rule) + s.checkRecordedRuleUpdateNotices(c, whenAccessed, 0) // Check prompt still exists and no prompt notices recorded since before // the rule was added retrievedPrompt, err := mgr.PromptWithID(s.defaultUser, prompt.ID) c.Assert(err, IsNil) c.Assert(retrievedPrompt, Equals, prompt) - s.checkRecordedPromptNotices(c, whenAdded, 0) + s.checkRecordedPromptNotices(c, whenAccessed, 0) // Patch rule to now cover the outstanding prompt whenPatched := time.Now() - newConstraints := &prompting.Constraints{ + newConstraints := &prompting.PatchConstraints{ PathPattern: mustParsePathPattern(c, "/home/test/{foo,bar,baz}"), - Permissions: []string{"read", "write"}, - } - patched, err := mgr.PatchRule(s.defaultUser, rule.ID, newConstraints, prompting.OutcomeAllow, prompting.LifespanForever, "") + Permissions: prompting.PermissionMap{ + "read": &prompting.PermissionEntry{ + Outcome: prompting.OutcomeAllow, + Lifespan: prompting.LifespanForever, + }, + "write": &prompting.PermissionEntry{ + Outcome: prompting.OutcomeAllow, + Lifespan: prompting.LifespanForever, + }, + }, + } + patched, err := mgr.PatchRule(s.defaultUser, rule.ID, newConstraints) c.Assert(err, IsNil) s.checkRecordedRuleUpdateNotices(c, whenPatched, 1)