Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

i/prompting: add package for prompting common types/functions #13849

Merged
Merged
Show file tree
Hide file tree
Changes from 7 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
139 changes: 139 additions & 0 deletions interfaces/prompting/prompting.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,139 @@
// -*- 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 <http://www.gnu.org/licenses/>.
*
*/

package prompting

import (
"encoding/base32"
"encoding/binary"
"fmt"
"time"
)

type OutcomeType string
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this needs a doc string


const (
OutcomeUnset OutcomeType = ""
OutcomeAllow OutcomeType = "allow"
OutcomeDeny OutcomeType = "deny"
)

// AsBool returns the outcome as a boolean, or an error if it cannot be parsed.
func (outcome OutcomeType) AsBool() (bool, error) {
olivercalder marked this conversation as resolved.
Show resolved Hide resolved
switch outcome {
case OutcomeAllow:
return true, nil
case OutcomeDeny:
return false, nil
default:
return false, fmt.Errorf(`invalid outcome: must be %q or %q: %q`, OutcomeAllow, OutcomeDeny, outcome)
}
}

// ValidateOutcome returns nil if the given outcome is valid, otherwise an error.
func ValidateOutcome(outcome OutcomeType) error {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is a bit strange, shouldn't it be a parse function taking a string and returning the OutcomeType or an error?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

or maybe what you need is MarshalJSON UnmarshalJSON methods?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since OutcomeType is an alias for string, the json {,un}marshaller works on OutcomeType directly, so potentially malformed outcomes are already OutcomeType variables which need to be validated, rather than strings. Theoretically, this could be built into custom {M,Unm}arshalJSON functions, but then there's no way to validate values when explicitly defined manually (i.e. myOutcome := OutcomeType("allow") instead of OutcomeAllow). There should probably never be a case where custom strings are cast to OutcomeTypes, so perhaps this negative isn't really relavant and a custom {,un}marshaller for OutcomeType would be ergonomic.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hm if we don't care about defining new values of OutcomeType manually, then I think a trivial UnmarshalJSON would be preferred. You do not need MarshalJSON in such case.

switch outcome {
case OutcomeAllow, OutcomeDeny:
return nil
default:
return fmt.Errorf(`invalid outcome: must be %q or %q: %q`, OutcomeAllow, OutcomeDeny, outcome)
}
}

type LifespanType string
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same


const (
LifespanUnset LifespanType = ""
LifespanForever LifespanType = "forever"
LifespanSession LifespanType = "session"
olivercalder marked this conversation as resolved.
Show resolved Hide resolved
LifespanSingle LifespanType = "single"
LifespanTimespan LifespanType = "timespan"
bboozzoo marked this conversation as resolved.
Show resolved Hide resolved
)

// ValidateLifespanExpiration checks that the given lifespan is valid and that
// the given expiration is valid for that lifespan.
//
// If the lifespan is LifespanTimespan LifespanTimespan, then expiration must
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
// If the lifespan is LifespanTimespan LifespanTimespan, then expiration must
// If the lifespan is LifespanTimespan , then expiration must

// be non-nil. Otherwise, it must be nil. Returns an error if any of the above
// are invalid.
func ValidateLifespanExpiration(lifespan LifespanType, expiration *time.Time, currTime time.Time) error {
olivercalder marked this conversation as resolved.
Show resolved Hide resolved
switch lifespan {
case LifespanForever, LifespanSession, LifespanSingle:
if expiration != nil {
return fmt.Errorf(`invalid expiration: expiration must be nil when lifespan is %q, but received non-nil expiration: %q`, lifespan, *expiration)
olivercalder marked this conversation as resolved.
Show resolved Hide resolved
}
case LifespanTimespan:
if expiration == nil {
return fmt.Errorf(`invalid expiration: expiration must be non-nil when lifespan is %q, but received nil expiration`, lifespan)
}
if currTime.After(*expiration) {
return fmt.Errorf("invalid expiration: expiration time has already passed: %q", *expiration)
}
default:
return fmt.Errorf(`invalid lifespan: %q`, lifespan)
}
return nil
}

// ValidateLifespanParseDuration checks that the given lifespan is valid and
// that the given duration is valid for that lifespan.
//
// If the lifespan is LifespanTimespan, then duration must be a string parsable
// by time.ParseDuration(), representing the duration of time for which the rule
// should be valid. Otherwise, it must be empty. Returns an error if any of the
// above are invalid, otherwise computes the expiration time of the rule based
// on the current time and the given duration and returns it.
func ValidateLifespanParseDuration(lifespan LifespanType, duration string) (*time.Time, error) {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

me: should probably take currTime as well, for consistency

var expiration *time.Time
switch lifespan {
case LifespanForever, LifespanSession, LifespanSingle:
if duration != "" {
return nil, fmt.Errorf(`invalid duration: duration must be empty when lifespan is %q, but received non-empty duration: %q`, lifespan, duration)
olivercalder marked this conversation as resolved.
Show resolved Hide resolved
}
case LifespanTimespan:
if duration == "" {
return nil, fmt.Errorf(`invalid duration: duration must be non-empty when lifespan is %q, but received empty expiration`, lifespan)
}
parsedDuration, err := time.ParseDuration(duration)
if err != nil {
return nil, fmt.Errorf(`invalid duration: error parsing duration string: %q`, duration)
}
if parsedDuration <= 0 {
return nil, fmt.Errorf(`invalid duration: duration must be greater than zero: %q`, duration)
}
expirationValue := time.Now().Add(parsedDuration)
expiration = &expirationValue
default:
return nil, fmt.Errorf(`invalid lifespan: %q`, lifespan)
}
return expiration, nil
}

// NewIDAndTimestamp returns a new unique ID and corresponding timestamp.
//
// The ID is the current unix time in nanoseconds encoded as a string in base32.
// The timestamp is the same time, encoded as a string in time.RFC3339Nano.
func NewIDAndTimestamp() (id string, timestamp time.Time) {
now := time.Now()
nowUnix := uint64(now.UnixNano())
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What happens when the time reports the same unixnano?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just use a counter. Store with rules storage? And use with request prompts as well.

Or combine boot ID with monotonic boot time. (look for helper, reads from /proc)

CLOCK_MONOTONIC
A nonsettable system-wide clock that represents monotonic time since—as described by POSIX—"some unspecified point in the past". On Linux, that point corresponds to the number of seconds that the system has been running since it was booted.
The CLOCK_MONOTONIC clock is not affected by discontinuous jumps in the system time (e.g., if the system administrator manually changes the clock), but is affected by frequency adjustments.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

According to the time package documentation (https://pkg.go.dev/time#hdr-Monotonic_Clocks), time.Now() automatically includes both monotonic and wall time:

If Times t and u both contain monotonic clock readings, the operations t.After(u), t.Before(u), t.Equal(u), t.Compare(u), and t.Sub(u) are carried out using the monotonic clock readings alone, ignoring the wall clock readings. If either t or u contains no monotonic clock reading, these operations fall back to using the wall clock readings.

Because the monotonic clock reading has no meaning outside the current process, the serialized forms generated by t.GobEncode, t.MarshalBinary, t.MarshalJSON, and t.MarshalText omit the monotonic clock reading, and t.Format provides no format for it. Similarly, the constructors time.Date, time.Parse, time.ParseInLocation, and time.Unix, as well as the unmarshalers t.GobDecode, t.UnmarshalBinary. t.UnmarshalJSON, and t.UnmarshalText always create times with no monotonic clock reading.

Thus, the monotonic time is only stripped when rules/prompts are read from disk. This occurs when snapd is restarted, including on system start. Unfortunately, this means time.Now() is not guaranteed to be unique between snapd restarts, so we likely need to include both boot ID and system monotonic time when creating rule/prompt IDs.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

where and what are this IDs used for ? in general snapd prefers to use sequential increasing IDs? at least for norification from the kernel don't we get them in a single goroutine? in that context using sequential IDs would be cheap, no?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AFAIU the IDs are stored within the prompting namespace in notices and we need them to be unique across reboots too (@olivercalder please correct me if I'm wrong), so to keep them sequential we'd either need to store some state.

Copy link
Collaborator

@pedronis pedronis Apr 25, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

well, we could restart counting from the max of the pending ones when we restart

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, that's a good point

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could start from the max of the pending ones, though that would allow for the potential that IDs are reused for different rules, if a rule is removed, then snapd restarted. This isn't necessarily a problem, as we don't guarantee that IDs are never reused, but I'd prefer to keep them unique if possible.

I think storing a count as part of the rule storage would solve this problem, and it can be atomically incremented whenever a new rule is added. For prompt requests, which are not written to disk (because of listener/notify socket behavior, but that can be discussed in a later PR), we can just keep a count in memory and atomically increment it.

So, my opinion is that we should remove this function for now and add a NewID function when necessary in the requestrules and requestprompts packages.

nowBytes := make([]byte, 8)
binary.BigEndian.PutUint64(nowBytes, nowUnix)
id = base32.StdEncoding.EncodeToString(nowBytes)
return id, now
}
157 changes: 157 additions & 0 deletions interfaces/prompting/prompting_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,157 @@
// -*- 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 <http://www.gnu.org/licenses/>.
*
*/

package prompting_test

import (
"encoding/base32"
"encoding/binary"
"testing"
"time"

. "gopkg.in/check.v1"

"github.com/snapcore/snapd/dirs"
"github.com/snapcore/snapd/interfaces/prompting"
)

func Test(t *testing.T) { TestingT(t) }

type promptingSuite struct {
tmpdir string
}

var _ = Suite(&promptingSuite{})

func (s *promptingSuite) SetUpTest(c *C) {
s.tmpdir = c.MkDir()
dirs.SetRootDir(s.tmpdir)
}

func (s *promptingSuite) TestOutcomeAsBool(c *C) {
result, err := prompting.OutcomeAllow.AsBool()
c.Check(err, IsNil)
c.Check(result, Equals, true)
result, err = prompting.OutcomeDeny.AsBool()
c.Check(err, IsNil)
c.Check(result, Equals, false)
_, err = prompting.OutcomeUnset.AsBool()
c.Check(err, ErrorMatches, `invalid outcome.*`)
_, err = prompting.OutcomeType("foo").AsBool()
c.Check(err, ErrorMatches, `invalid outcome.*`)
}

func (s *promptingSuite) TestValidateOutcome(c *C) {
c.Assert(prompting.ValidateOutcome(prompting.OutcomeAllow), Equals, nil)
c.Assert(prompting.ValidateOutcome(prompting.OutcomeDeny), Equals, nil)
c.Assert(prompting.ValidateOutcome(prompting.OutcomeUnset), ErrorMatches, `invalid outcome.*`)
c.Assert(prompting.ValidateOutcome(prompting.OutcomeType("foo")), ErrorMatches, `invalid outcome.*`)
}

func (s *promptingSuite) TestValidateLifespanExpiration(c *C) {
var unsetExpiration *time.Time
currTime := time.Now()
negativeExpirationValue := currTime.Add(-5 * time.Second)
negativeExpiration := &negativeExpirationValue
validExpirationValue := currTime.Add(10 * time.Minute)
validExpiration := &validExpirationValue

for _, lifespan := range []prompting.LifespanType{
prompting.LifespanForever,
prompting.LifespanSession,
prompting.LifespanSingle,
} {
err := prompting.ValidateLifespanExpiration(lifespan, unsetExpiration, currTime)
c.Check(err, IsNil)
for _, exp := range []*time.Time{negativeExpiration, validExpiration} {
err = prompting.ValidateLifespanExpiration(lifespan, exp, currTime)
c.Check(err, ErrorMatches, `invalid expiration: expiration must be nil.*`)
}
}

err := prompting.ValidateLifespanExpiration(prompting.LifespanTimespan, unsetExpiration, currTime)
c.Check(err, ErrorMatches, `invalid expiration: expiration must be non-nil.*`)

err = prompting.ValidateLifespanExpiration(prompting.LifespanTimespan, negativeExpiration, currTime)
c.Check(err, ErrorMatches, `invalid expiration: expiration time has already passed.*`)

err = prompting.ValidateLifespanExpiration(prompting.LifespanTimespan, validExpiration, currTime)
c.Check(err, IsNil)
}

func (s *promptingSuite) TestValidateLifespanParseDuration(c *C) {
unsetDuration := ""
invalidDuration := "foo"
negativeDuration := "-5s"
validDuration := "10m"
parsedValidDuration, err := time.ParseDuration(validDuration)
c.Assert(err, IsNil)

for _, lifespan := range []prompting.LifespanType{
prompting.LifespanForever,
prompting.LifespanSession,
prompting.LifespanSingle,
} {
expiration, err := prompting.ValidateLifespanParseDuration(lifespan, unsetDuration)
c.Check(expiration, IsNil)
c.Check(err, IsNil)
for _, dur := range []string{invalidDuration, negativeDuration, validDuration} {
expiration, err = prompting.ValidateLifespanParseDuration(lifespan, dur)
c.Check(expiration, IsNil)
c.Check(err, ErrorMatches, `invalid duration: duration must be empty.*`)
}
}

expiration, err := prompting.ValidateLifespanParseDuration(prompting.LifespanTimespan, unsetDuration)
c.Check(expiration, IsNil)
c.Check(err, ErrorMatches, `invalid duration: duration must be non-empty.*`)

expiration, err = prompting.ValidateLifespanParseDuration(prompting.LifespanTimespan, invalidDuration)
c.Check(expiration, IsNil)
c.Check(err, ErrorMatches, `invalid duration: error parsing duration string.*`)

expiration, err = prompting.ValidateLifespanParseDuration(prompting.LifespanTimespan, negativeDuration)
c.Check(expiration, IsNil)
c.Check(err, ErrorMatches, `invalid duration: duration must be greater than zero.*`)

expiration, err = prompting.ValidateLifespanParseDuration(prompting.LifespanTimespan, validDuration)
c.Check(err, IsNil)
c.Check(expiration.After(time.Now()), Equals, true)
c.Check(expiration.Before(time.Now().Add(parsedValidDuration)), Equals, true)
}

func (s *promptingSuite) TestNewIDAndTimestamp(c *C) {
before := time.Now()
id, _ := prompting.NewIDAndTimestamp()
idPaired, timestampPaired := prompting.NewIDAndTimestamp()
after := time.Now()
data1, err := base32.StdEncoding.DecodeString(id)
c.Assert(err, IsNil)
data2, err := base32.StdEncoding.DecodeString(idPaired)
c.Assert(err, IsNil)
parsedNs := int64(binary.BigEndian.Uint64(data1))
parsedNsPaired := int64(binary.BigEndian.Uint64(data2))
parsedTime := time.Unix(parsedNs/1000000000, parsedNs%1000000000)
parsedTimePaired := time.Unix(parsedNsPaired/1000000000, parsedNsPaired%1000000000)
c.Assert(parsedTime.After(before), Equals, true)
c.Assert(parsedTime.Before(after), Equals, true)
c.Assert(parsedTimePaired.After(before), Equals, true)
c.Assert(parsedTimePaired.Before(after), Equals, true)
c.Assert(parsedTimePaired.Equal(timestampPaired), Equals, true)
}
13 changes: 12 additions & 1 deletion snap/info.go
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
// -*- Mode: Go; indent-tabs-mode: t -*-

/*
* Copyright (C) 2014-2022 Canonical Ltd
* Copyright (C) 2014-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
Expand Down Expand Up @@ -208,6 +208,17 @@ func ScopedSecurityTag(snapName, scopeName, suffix string) string {
return fmt.Sprintf("snap.%s.%s.%s", snapName, scopeName, suffix)
}

// SplitSecurityTag will extract the snap name from a string of the form
// `snap.<snap>[.<app>]`. If the tag is not of that form, returns an error.
olivercalder marked this conversation as resolved.
Show resolved Hide resolved
func SplitSecurityTag(tag string) (string, error) {
olivercalder marked this conversation as resolved.
Show resolved Hide resolved
olivercalder marked this conversation as resolved.
Show resolved Hide resolved
errMsg := fmt.Errorf("invalid security tag: must be of the form 'snap.<snap>' or 'snap.<snap>.<app>': %q", tag)
l := strings.SplitN(tag, ".", 3)
if len(l) < 2 || l[0] != "snap" || len(l[1]) == 0 || (len(l) == 3 && len(l[2]) == 0) {
return "", errMsg
}
return l[1], nil
}

// SecurityTag returns the snap-specific security tag.
func SecurityTag(snapName string) string {
return fmt.Sprintf("snap.%s", snapName)
Expand Down
49 changes: 49 additions & 0 deletions snap/info_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -246,6 +246,55 @@ func (s *infoSuite) TestWebsiteFromLinks(c *C) {
c.Check(info.Website(), Equals, "http://website1")
}

func (s *infoSuite) TestSplitSecurityTagHappy(c *C) {
cases := []struct {
tag string
snap string
}{
{
tag: "snap.foo",
snap: "foo",
},
{
tag: "snap.foo.foo",
snap: "foo",
},
{
tag: "snap.foo.bar",
snap: "foo",
},
{
tag: "snap.foo_bar",
snap: "foo_bar",
},
{
tag: "snap.foo_bar.baz",
snap: "foo_bar",
},
}
for _, testCase := range cases {
snap, err := snap.SplitSecurityTag(testCase.tag)
c.Check(err, IsNil)
c.Check(snap, Equals, testCase.snap)
}
}

func (s *infoSuite) TestSplitSecurityTagUnhappy(c *C) {
cases := []string{
"snap",
"snap_foo",
"snap.",
olivercalder marked this conversation as resolved.
Show resolved Hide resolved
"SNAP.foo",
"SNAP.FOO",
"snap.foo.",
}
for _, testCase := range cases {
snap, err := snap.SplitSecurityTag(testCase)
c.Check(err, Not(IsNil))
c.Check(snap, Equals, "")
}
}

func (s *infoSuite) TestAppInfoSecurityTag(c *C) {
appInfo := &snap.AppInfo{Snap: &snap.Info{SuggestedName: "http"}, Name: "GET"}
c.Check(appInfo.SecurityTag(), Equals, "snap.http.GET")
Expand Down