Skip to content

Commit

Permalink
i/p/requestrules: use new rule timestamp for merged rule and test mer…
Browse files Browse the repository at this point in the history 
…ging

Signed-off-by: Oliver Calder <[email protected]>
  • Loading branch information
@olivercalder
olivercalder committed Dec 4, 2024
1 parent 8a0cfd2 commit ec71e26
Show file tree
Hide file tree
Showing 2 changed files with 200 additions and 5 deletions.
10 changes: 5 additions & 5 deletions interfaces/prompting/requestrules/requestrules.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -371,15 +371,13 @@ func (rdb *RuleDB) addOrMergeRule(rule *Rule, save bool) (addedOrMergedRule *Rul
return rule, false, rdb.addNewRule(rule, save)
}

currTime := time.Now()

// Check whether the existing rule has outcomes/lifespans which conflict,
// and compile the new set of permissions.
newPermissions := make(prompting.RulePermissionMap)
var conflicts []prompting_errors.RuleConflict
for perm, entry := range rule.Constraints.Permissions {
existingEntry, exists := existingRule.Constraints.Permissions[perm]
if !exists || existingEntry.Expired(currTime) {
if !exists || existingEntry.Expired(rule.Timestamp) {
newPermissions[perm] = entry
continue
}
Expand Down Expand Up @@ -407,7 +405,7 @@ func (rdb *RuleDB) addOrMergeRule(rule *Rule, save bool) (addedOrMergedRule *Rul
}
// Add any non-expired permissions which were left over from the existing rule.
for existingPerm, existingEntry := range existingRule.Constraints.Permissions {
if existingEntry.Expired(currTime) {
if existingEntry.Expired(rule.Timestamp) {
continue
}
if _, exists := newPermissions[existingPerm]; exists {
Expand All @@ -416,9 +414,11 @@ func (rdb *RuleDB) addOrMergeRule(rule *Rule, save bool) (addedOrMergedRule *Rul
newPermissions[existingPerm] = existingEntry
}

// Create new rule based on the contents of the existing rule
// Create new rule based on the contents of the existing rule, but copy the
// timestamp from the new rule.
newRuleContents := *existingRule
newRule := &newRuleContents
newRule.Timestamp = rule.Timestamp
// Copy constraints as well, since copying the rule just copied the pointer
newConstraints := *(existingRule.Constraints)
newRule.Constraints = &newConstraints
Expand Down
195 changes: 195 additions & 0 deletions interfaces/prompting/requestrules/requestrules_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -864,6 +864,201 @@ outer:
}
}

func (s *requestrulesSuite) TestAddRuleMerges(c *C) {
for _, testCase := range []struct {
input []prompting.PermissionMap
output []prompting.PermissionMap
}{
{
input: []prompting.PermissionMap{
{
"read": &prompting.PermissionEntry{
Outcome: prompting.OutcomeAllow,
Lifespan: prompting.LifespanForever,
},
},
{
"write": &prompting.PermissionEntry{
Outcome: prompting.OutcomeDeny,
Lifespan: prompting.LifespanTimespan,
Duration: "10s",
},
},
},
output: []prompting.PermissionMap{
{
"read": &prompting.PermissionEntry{
Outcome: prompting.OutcomeAllow,
Lifespan: prompting.LifespanForever,
},
"write": &prompting.PermissionEntry{
Outcome: prompting.OutcomeDeny,
Lifespan: prompting.LifespanTimespan,
Duration: "10s",
},
},
},
},
{
input: []prompting.PermissionMap{
{
"read": &prompting.PermissionEntry{
Outcome: prompting.OutcomeDeny,
Lifespan: prompting.LifespanForever,
},
"write": &prompting.PermissionEntry{
Outcome: prompting.OutcomeAllow,
Lifespan: prompting.LifespanTimespan,
Duration: "10s",
},
"execute": &prompting.PermissionEntry{
Outcome: prompting.OutcomeAllow,
Lifespan: prompting.LifespanTimespan,
Duration: "1s",
},
},
{
"read": &prompting.PermissionEntry{
Outcome: prompting.OutcomeDeny,
Lifespan: prompting.LifespanTimespan,
Duration: "10s",
},
"write": &prompting.PermissionEntry{
Outcome: prompting.OutcomeAllow,
Lifespan: prompting.LifespanForever,
},
},
},
output: []prompting.PermissionMap{
{
"read": &prompting.PermissionEntry{
Outcome: prompting.OutcomeDeny,
Lifespan: prompting.LifespanForever,
},
"write": &prompting.PermissionEntry{
Outcome: prompting.OutcomeAllow,
Lifespan: prompting.LifespanForever,
},
"execute": &prompting.PermissionEntry{
Outcome: prompting.OutcomeAllow,
Lifespan: prompting.LifespanTimespan,
Duration: "1s",
},
},
},
},
{
input: []prompting.PermissionMap{
{
"read": &prompting.PermissionEntry{
Outcome: prompting.OutcomeDeny,
Lifespan: prompting.LifespanTimespan,
Duration: "10s",
},
"write": &prompting.PermissionEntry{
Outcome: prompting.OutcomeAllow,
Lifespan: prompting.LifespanTimespan,
Duration: "10s",
},
"execute": &prompting.PermissionEntry{
Outcome: prompting.OutcomeAllow,
Lifespan: prompting.LifespanTimespan,
Duration: "1ns", // Will expire and be dropped
},
},
{
"read": &prompting.PermissionEntry{
Outcome: prompting.OutcomeDeny,
Lifespan: prompting.LifespanTimespan,
Duration: "20s",
},
"write": &prompting.PermissionEntry{
Outcome: prompting.OutcomeAllow,
Lifespan: prompting.LifespanForever,
},
},
},
output: []prompting.PermissionMap{
{
"read": &prompting.PermissionEntry{
Outcome: prompting.OutcomeDeny,
Lifespan: prompting.LifespanTimespan,
Duration: "20s",
},
"write": &prompting.PermissionEntry{
Outcome: prompting.OutcomeAllow,
Lifespan: prompting.LifespanForever,
},
},
},
},
} {
// Set root so rule creation does not interfere between test cases
dirs.SetRootDir(c.MkDir())
s.AddCleanup(func() { dirs.SetRootDir("") })
c.Assert(os.MkdirAll(dirs.SnapdStateDir(dirs.GlobalRootDir), 0o700), IsNil)

rdb, err := requestrules.New(s.defaultNotifyRule)
c.Assert(err, IsNil)

user := s.defaultUser
snap := "firefox"
iface := "home"
pathPattern := mustParsePathPattern(c, "/path/to/foo/ba{r,z}/**")

// Add all the rules
for _, perms := range testCase.input {
constraints := &prompting.Constraints{
PathPattern: pathPattern,
Permissions: perms,
}
_, err = rdb.AddRule(user, snap, iface, constraints)
c.Assert(err, IsNil, Commentf("\ntestCase: %+v\nperms: %+v", testCase, perms))
}

rules := rdb.Rules(s.defaultUser)
c.Check(rules, HasLen, len(testCase.output), Commentf("\ntestCase: %+v\nrules: %+v", testCase, rules))
for i, perms := range testCase.output {
// Build RuleConstraints based on output perms using the timestamp
// of the corresponding rule.
rule := rules[i]
constraints := &prompting.Constraints{
PathPattern: pathPattern,
Permissions: perms,
}
ruleConstraints, err := constraints.ToRuleConstraints(iface, rule.Timestamp)
c.Assert(err, IsNil)
expectedPerms := ruleConstraints.Permissions
// Check that the permissions match what is expected.
// Other parameters should be trivially identical.
// Need to be careful because timestamps aren't "DeepEqual", so
// first set equivalent timestamps equal to each other.
for perm, entry := range rule.Constraints.Permissions {
expectedEntry, exists := expectedPerms[perm]
c.Assert(exists, Equals, true, Commentf("\ntestCase: %+v\nrules: %+v\npermission not found: %s", testCase, rules, perm))
c.Check(entry.Lifespan, Equals, expectedEntry.Lifespan, Commentf("\ntestCase: %+v\nrules: %+v\nlifespans not equal: %v != %v", testCase, rules, entry.Lifespan, expectedEntry.Lifespan))
// Expiration will be duration after the timestamp of one of
// the created rules, but it may not be the final one which was
// merged into the resulting rule, so we don't actually have an
// absolute timestamp with which we can compute an expiration
// using the duration. So subtract the timestamps and check
// that the difference is less than 100ms. We'll always have
// deltas of 1s for time differences we care about.
difference := entry.Expiration.Sub(expectedEntry.Expiration)
// TODO: call Abs() once we're on Go 1.19+
if difference < 0 {
difference *= -1
}
c.Check(difference < 100*time.Millisecond, Equals, true, Commentf("\ntestCase: %+v\nrules: %+v\nexpirations not within 100ms: %v != %v", testCase, rules, entry.Expiration, expectedEntry.Expiration))
expectedEntry.Expiration = entry.Expiration
}
c.Check(rule.Constraints.Permissions, DeepEquals, expectedPerms)
}

c.Assert(rdb.Close(), IsNil)
}
}

func (s *requestrulesSuite) TestAddRuleExpired(c *C) {
rdb, err := requestrules.New(s.defaultNotifyRule)
c.Assert(err, IsNil)
Expand Down

0 comments on commit ec71e26

Please sign in to comment.