Skip to content

Commit

Permalink
many: support mixed outcomes for permissions in prompting constraints
Browse files Browse the repository at this point in the history
Let rule content constraints have heterogeneous outcomes and lifespans
for different permissions in the constraints. As such, convert the list
of permissions to a map from permission to permission entry, where the
entry holds the outcome, lifespan, and duration/expiration for that
particular permission, where previous those were global to the
containing rule, rule contents, or patch contents.

However, the existing concept of replying "allow"/"deny" to a particular
set of requested permisisons is clear and simple. We want to keep
outcome, lifespan, and duration as reply-scoped values, not
permission-specific, when accepting prompt replies. So we need different
types of constraints for prompt replies vs. rule contents.

The motivation behind this is so that we can have only a single rule for
any given path pattern. We may have a situation where the user
previously replied with "allow read `/path/to/foo`" and they're now
prompted for write access, they need to be able to respond with "deny
read `/path/to/foo`". If we only support a single outcome for any given
rule, then we'd need two rules for the same path `/path/to/foo`. Thus,
we need rules to support different outcomes for different permissions.

The same logic applies for lifetimes and expirations, though this adds
additional complexity now that the concept of rule expiration is shifted
to being permission-specific. We care about expired rules in two primary
places: when loading rules from disk, we want to discard any expired
rules, and when adding a new rule, we want to discard any expired
permisison entry for a rule which shares a pattern variant with the new
rule. For cases where that expired permission entry had a conflicting
outcome, we clearly can't have that, and we want to remove the expired
permission entry from its containing rule as well, so as to avoid
confusion for the user without them needing to check expiration
timestamps. Even if the outcome of the expired entry matches that of the
new rule's entry for the same permission, we still want to prune the
expired permission from the old rule to avoid confusion. The complexity
is around when a notice is recorded for a rule for which some
permissions have expired. At the moment, the logic is that a notice is
recorded in these cases:

- when a rule is loaded from disk
    - data may be `"removed": "expired"` if all permissions are expired
- when a rule is added
- when a rule is patched
- when a rule is removed (with data `"removed": "removed"`)
- when a rule is found to be expired when attempting to add a new rule

Notably, a notice is not recorded automatically when a permission entry
expires. Nor is a notice recorded when a permission is found to be
expired, so long as its associated rule still has at least one
non-expired permission. Neither pruning an expired permission entry from
the rule tree nor from the entry's containing rule results in a notice,
even though technically the rule data has changed, since the expired
permission has been erased. The rationale is that the semantics of the
rule have not changed, since the expiration of that permission was part
of the semantics of the rule.

Since durations are used when adding/patching a rule and expirations are
used when retrieving a rule, in addition to the differences for prompt
replies vs. rule contents, we now need several different variants of
constraints:
- `promptConstraints`:
    - path, requested permissions list, available permissions list
    - internal to `requestprompts`, unchanged
- `ReplyConstraints`:
    - path pattern, list of permissions
    - containing `PromptReply` holds outcome/lifespan/expiration
    - unchanged from before, though under a new name
    - converted to a `Constraints` if reply warrants a new rule
- `Constraints`:
    - path pattern, map from permission to outcome, lifespan, duration
    - used when adding rule to the rule DB
    - converted to `RuleConstraints` when the new rule is created
- `RuleConstraints`:
    - path pattern, map from permisison to outcome, lifespan, expiration
    - used when retrieving rules from the rule DB
    - never used when POSTing to the API
- `PatchConstraints`:
    - identical to `Constraints`, but with omitempty fields
    - converted to `RuleConstraints` when the patched rule is created

To support this, we define some new types, including `{,Rule}PermissionMap`
and `{,Rule}PermissionEntry`. The latter of these is used in the leaves
of the rule DB tree in place of the previous set of rule IDs of rules
whose patterns render to a given pattern variant.

Whenever possible, logic surrounding constraints, permissions, and
expiration is pushed down to methods on these new types, thus
simplifiying the logic of their callers.

Signed-off-by: Oliver Calder <[email protected]>
  • Loading branch information
olivercalder committed Oct 8, 2024
1 parent fc2a992 commit 82e0611
Show file tree
Hide file tree
Showing 15 changed files with 1,293 additions and 677 deletions.
20 changes: 7 additions & 13 deletions daemon/api_prompting.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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 {
Expand Down Expand Up @@ -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)
}
Expand Down Expand Up @@ -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)
}
Expand Down
180 changes: 103 additions & 77 deletions daemon/api_prompting_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -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
Expand All @@ -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
}

Expand All @@ -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
}

Expand Down Expand Up @@ -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"},
}
Expand All @@ -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)
Expand Down Expand Up @@ -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(),
},
}

Expand Down Expand Up @@ -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",
Expand All @@ -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)
Expand Down Expand Up @@ -881,35 +884,42 @@ 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),
Timestamp: time.Now(),
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),
Timestamp: time.Now(),
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(),
},
}

Expand Down Expand Up @@ -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)
Expand All @@ -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",
Expand All @@ -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)
Expand All @@ -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",
Expand Down
Loading

0 comments on commit 82e0611

Please sign in to comment.