diff --git a/asserts/confdb.go b/asserts/confdb.go index 9a8196723cd..f4fe2e04404 100644 --- a/asserts/confdb.go +++ b/asserts/confdb.go @@ -209,7 +209,7 @@ func parseConfdbControlGroups(rawGroups []interface{}) (map[string]*confdb.Opera return nil, fmt.Errorf(`%s: "views" must be provided`, errPrefix) } - if err := operator.AddControlGroup(views, auth); err != nil { + if err := operator.Delegate(views, auth); err != nil { return nil, fmt.Errorf(`%s: %w`, errPrefix, err) } } diff --git a/asserts/confdb_test.go b/asserts/confdb_test.go index 80925626ad9..4e54fa83c7b 100644 --- a/asserts/confdb_test.go +++ b/asserts/confdb_test.go @@ -335,12 +335,12 @@ func (s *confdbCtrlSuite) TestDecodeInvalid(c *C) { { " - operator-key", " - foo-bar", - "cannot parse group at position 1: cannot add group: invalid authentication method: foo-bar", + "cannot parse group at position 1: cannot delegate: invalid authentication method: foo-bar", }, { "canonical/network/control-interfaces", "canonical", - `cannot parse group at position 2: view "canonical" must be in the format account/confdb/view`, + `cannot parse group at position 2: cannot delegate: view "canonical" must be in the format account/confdb/view`, }, } diff --git a/confdb/confdb_control.go b/confdb/confdb_control.go index 5246777c638..c83d4681ff7 100644 --- a/confdb/confdb_control.go +++ b/confdb/confdb_control.go @@ -83,6 +83,31 @@ type ControlGroup struct { Views []*ViewRef } +// findView binary searches the position of the given view in the control group. +func (g *ControlGroup) findView(view *ViewRef) (int, bool) { + left, right := 0, len(g.Views)-1 + + for left <= right { + mid := (left + right) / 2 + cmp := g.Views[mid].compare(view) + + if cmp == 0 { + return mid, true + } else if cmp < 0 { + left = mid + 1 + } else { + right = mid - 1 + } + } + + return 0, false +} + +// deleteViewAt removes the view at the given index. +func (g *ControlGroup) deleteViewAt(idx int) { + g.Views = append(g.Views[:idx], g.Views[idx+1:]...) +} + // ViewRef holds the reference to account/confdb/view as parsed from the // confdb-control assertion. type ViewRef struct { @@ -91,58 +116,224 @@ type ViewRef struct { View string } +// newViewRef parses account/confdb/view into ViewRef +func newViewRef(view string) (*ViewRef, error) { + viewPath := strings.Split(view, "/") + if len(viewPath) != 3 { + return nil, fmt.Errorf(`view "%s" must be in the format account/confdb/view`, view) + } + + account := viewPath[0] + if !validAccountID.MatchString(account) { + return nil, fmt.Errorf("invalid Account ID %s", account) + } + + confdb := viewPath[1] + if !ValidConfdbName.MatchString(confdb) { + return nil, fmt.Errorf("invalid confdb name %s", confdb) + } + + viewName := viewPath[2] + if !ValidViewName.MatchString(viewName) { + return nil, fmt.Errorf("invalid view name %s", viewName) + } + + return &ViewRef{ + Account: account, + Confdb: confdb, + View: viewName, + }, nil +} + +// compare compares two ViewRefs lexicographically based the Account, Confdb, & View fields. +// It returns: +// - -1 if `v` is less than `b`, +// - 1 if `v` is greater than `b`, +// - 0 if `v` is equal to `b`. +func (v *ViewRef) compare(b *ViewRef) int { + if v.Account != b.Account { + if v.Account < b.Account { + return -1 + } + return 1 + } + + if v.Confdb != b.Confdb { + if v.Confdb < b.Confdb { + return -1 + } + return 1 + } + + if v.View != b.View { + if v.View < b.View { + return -1 + } + return 1 + } + + return 0 +} + +// groupWithView returns the group that holds the given view. +func (op *Operator) groupWithView(view *ViewRef) (*ControlGroup, int) { + for _, group := range op.Groups { + index, ok := group.findView(view) + if ok { + return group, index + } + } + + return nil, 0 +} + +// groupWithAuthentication returns the group with the given auth. +// The provided auth should be sorted. +func (op *Operator) groupWithAuthentication(auth []AuthenticationMethod) *ControlGroup { + for _, group := range op.Groups { + if checkListEqual(group.Authentication, auth) { + return group + } + } + + return nil +} + +// IsDelegated checks if // is delegated to +// the operator under the given auth. +func (op *Operator) IsDelegated(view *ViewRef, auth AuthenticationMethod) bool { + group, _ := op.groupWithView(view) + if group == nil { + return false + } + + return checkListContains(group.Authentication, auth) +} + // AddControlGroup adds the group to an operator under the given authentication. -func (op *Operator) AddControlGroup(views, auth []string) error { - if len(auth) == 0 { - return errors.New(`cannot add group: "auth" must be a non-empty list`) +func (op *Operator) Delegate(views, rawAuth []string) error { + if len(rawAuth) == 0 { + return errors.New(`cannot delegate: "auth" must be a non-empty list`) } - authentication, err := convertToAuthenticationMethods(auth) + auth, err := convertToAuthenticationMethods(rawAuth) if err != nil { - return fmt.Errorf("cannot add group: %w", err) + return fmt.Errorf("cannot delegate: %w", err) } if len(views) == 0 { - return errors.New(`cannot add group: "views" must be a non-empty list`) + return errors.New(`cannot delegate: "views" must be a non-empty list`) } - parsedViews := []*ViewRef{} for _, view := range views { - viewPath := strings.Split(view, "/") - if len(viewPath) != 3 { - return fmt.Errorf(`view "%s" must be in the format account/confdb/view`, view) + parsedView, err := newViewRef(view) + if err != nil { + return fmt.Errorf("cannot delegate: %w", err) } - account := viewPath[0] - if !validAccountID.MatchString(account) { - return fmt.Errorf("invalid Account ID %s", account) - } + op.delegateOne(parsedView, auth) + } + + op.compact() + return nil +} + +// delegateOne grants remote registry control to //. +func (op *Operator) delegateOne(view *ViewRef, auth []AuthenticationMethod) { + newAuth := auth + existingGroup, viewIdx := op.groupWithView(view) + if existingGroup != nil { + newAuth = append(newAuth, existingGroup.Authentication...) + sort.Slice(newAuth, func(i, j int) bool { + return newAuth[i] < newAuth[j] + }) + newAuth = unique(newAuth) + } + + newGroup := op.groupWithAuthentication(newAuth) + if existingGroup == newGroup && existingGroup != nil { + // already delegated, nothing to do + return + } - confdb := viewPath[1] - if !ValidConfdbName.MatchString(confdb) { - return fmt.Errorf("invalid confdb name %s", confdb) + if newGroup == nil { + newGroup = &ControlGroup{Authentication: newAuth, Views: []*ViewRef{view}} + op.Groups = append(op.Groups, newGroup) + } else { + newGroup.Views = append(newGroup.Views, view) + sort.Slice(newGroup.Views, func(i, j int) bool { + return newGroup.Views[i].compare(newGroup.Views[j]) < 0 + }) + } + + if existingGroup != nil { + // remove the view from the old group + existingGroup.deleteViewAt(viewIdx) + } +} + +// Revoke withdraws remote access to the views that have been delegated under +// the authentication methods. +func (op *Operator) Revoke(views []string, rawAuth []string) error { + var err error + var auth []AuthenticationMethod + if len(rawAuth) == 0 { + // if no authentication is provided, revoke all auth methods + auth = []AuthenticationMethod{OperatorKey, Store} + } else { + auth, err = convertToAuthenticationMethods(rawAuth) + if err != nil { + return fmt.Errorf("cannot revoke: %w", err) } + } - viewName := viewPath[2] - if !ValidViewName.MatchString(viewName) { - return fmt.Errorf("invalid view name %s", viewName) + for _, view := range views { + parsedView, err := newViewRef(view) + if err != nil { + return fmt.Errorf("cannot revoke: %w", err) } - parsedView := &ViewRef{ - Account: account, - Confdb: confdb, - View: viewName, + op.revokeOne(parsedView, auth) + } + + op.compact() + return nil +} + +// revokeOne revokes remote registry control over //. +func (op *Operator) revokeOne(view *ViewRef, auth []AuthenticationMethod) { + group, viewIdx := op.groupWithView(view) + if group == nil { + // not delegated, nothing to do + return + } + + remaining := make([]AuthenticationMethod, 0, len(group.Authentication)) + for _, existingAuth := range group.Authentication { + if !checkListContains(auth, existingAuth) { + remaining = append(remaining, existingAuth) } - parsedViews = append(parsedViews, parsedView) } - group := &ControlGroup{ - Authentication: authentication, - Views: parsedViews, + // remove the view from the group + group.deleteViewAt(viewIdx) + + if len(remaining) != 0 { + // delegate the view with the remaining authentication method(s) + op.delegateOne(view, remaining) } - op.Groups = append(op.Groups, group) +} - return nil +// compact removes empty groups. +func (op *Operator) compact() { + groups := make([]*ControlGroup, 0, len(op.Groups)) + for _, group := range op.Groups { + if len(group.Views) != 0 { + groups = append(groups, group) + } + } + + op.Groups = groups } // unique replaces consecutive runs of equal elements with a single copy. @@ -162,3 +353,29 @@ func unique[T comparable](s []T) []T { return s[:j] } + +// checkListContains checks if the slice contains the given value. +func checkListContains[T comparable](s []T, v T) bool { + for _, item := range s { + if item == v { + return true + } + } + + return false +} + +// checkListEqual checks if two slices are equal. +func checkListEqual[T comparable](a, b []T) bool { + if len(a) != len(b) { + return false + } + + for i := range a { + if a[i] != b[i] { + return false + } + } + + return true +} diff --git a/confdb/confdb_control_test.go b/confdb/confdb_control_test.go index a13264541f0..1a8b0f51f47 100644 --- a/confdb/confdb_control_test.go +++ b/confdb/confdb_control_test.go @@ -48,12 +48,142 @@ func (s *confdbCtrlSuite) TestConvertToAuthenticationMethods(c *C) { c.Assert(converted, DeepEquals, expected) } -func (s *confdbCtrlSuite) TestAddGroupOK(c *C) { +func (s *confdbCtrlSuite) TestFindViewInGroup(c *C) { + operator := confdb.Operator{ID: "jane"} + + err := operator.Delegate( + []string{ + "canonical/network/control-device", + "canonical/network/control-interface", + "canonical/network/observe-device", + "canonical/network/observe-interface", + }, + []string{"operator-key"}, + ) + c.Assert(err, IsNil) + + type testcase struct { + view confdb.ViewRef + idx int + found bool + } + tcs := []testcase{ + { + view: confdb.ViewRef{Account: "canonical", Confdb: "network", View: "observe-device"}, + idx: 2, + found: true, + }, + { + view: confdb.ViewRef{Account: "canonical", Confdb: "network", View: "control-device"}, + idx: 0, + found: true, + }, + {view: confdb.ViewRef{Account: "unknown", Confdb: "network", View: "control-device"}}, + {view: confdb.ViewRef{Account: "canonical", Confdb: "unknown", View: "control-device"}}, + {view: confdb.ViewRef{Account: "canonical", Confdb: "network", View: "unknown"}}, + } + + g := operator.Groups[0] + for i, tc := range tcs { + cmt := Commentf("test number %d", i+1) + idx, found := g.FindView(&tc.view) + c.Assert(found, Equals, tc.found, cmt) + c.Assert(idx, Equals, tc.idx, cmt) + } +} + +func (s *confdbCtrlSuite) TestCompareViewRef(c *C) { + observeDevice := confdb.ViewRef{Account: "canonical", Confdb: "device", View: "observe-device"} + controlDevice := confdb.ViewRef{Account: "canonical", Confdb: "device", View: "control-device"} + observeInterface := confdb.ViewRef{Account: "canonical", Confdb: "network", View: "observe-interface"} + controlConfig := confdb.ViewRef{Account: "system", Confdb: "telemetry", View: "control-config"} + + type testcase struct { + a confdb.ViewRef + b confdb.ViewRef + result int + } + tcs := []testcase{ + {a: observeDevice, b: observeDevice, result: 0}, + {a: observeDevice, b: controlDevice, result: 1}, + {a: controlDevice, b: observeDevice, result: -1}, + {a: observeInterface, b: controlDevice, result: 1}, + {a: controlDevice, b: observeInterface, result: -1}, + {a: controlConfig, b: controlDevice, result: 1}, + {a: controlDevice, b: controlConfig, result: -1}, + } + for i, tc := range tcs { + cmt := Commentf("test number %d", i+1) + result := tc.a.Compare(&tc.b) + c.Assert(result, Equals, tc.result, cmt) + } +} + +func (s *confdbCtrlSuite) TestGroupWithView(c *C) { + operator := confdb.Operator{ID: "canonical"} + + err := operator.Delegate( + []string{"canonical/network/control-interface", "canonical/network/observe-interface"}, + []string{"operator-key"}, + ) + c.Assert(err, IsNil) + + err = operator.Delegate( + []string{"canonical/network/control-device", "canonical/network/observe-device"}, + []string{"store"}, + ) + c.Assert(err, IsNil) + + group, idx := operator.GroupWithView(&confdb.ViewRef{ + Account: "canonical", Confdb: "network", View: "control-interface", + }) + c.Assert(group, Equals, operator.Groups[0]) + c.Assert(idx, Equals, 0) + + group, idx = operator.GroupWithView(&confdb.ViewRef{ + Account: "canonical", Confdb: "network", View: "observe-device", + }) + c.Assert(group, Equals, operator.Groups[1]) + c.Assert(idx, Equals, 1) +} + +func (s *confdbCtrlSuite) TestGroupWithAuthentication(c *C) { + operator := confdb.Operator{ID: "canonical"} + + err := operator.Delegate( + []string{"canonical/network/control-interface"}, + []string{"operator-key"}, + ) + c.Assert(err, IsNil) + + err = operator.Delegate( + []string{"canonical/network/observe-device"}, + []string{"store"}, + ) + c.Assert(err, IsNil) + + err = operator.Delegate( + []string{"canonical/network/observe-interface"}, + []string{"store", "operator-key"}, + ) + c.Assert(err, IsNil) + + group := operator.GroupWithAuthentication([]confdb.AuthenticationMethod{confdb.OperatorKey}) + c.Assert(group, Equals, operator.Groups[0]) + + group = operator.GroupWithAuthentication([]confdb.AuthenticationMethod{confdb.Store}) + c.Assert(group, Equals, operator.Groups[1]) + + group = operator.GroupWithAuthentication([]confdb.AuthenticationMethod{confdb.OperatorKey, confdb.Store}) + c.Assert(group, Equals, operator.Groups[2]) +} + +func (s *confdbCtrlSuite) TestDelegateOK(c *C) { operator := confdb.Operator{ID: "canonical"} views := []string{"canonical/network/control-device", "canonical/network/observe-device"} auth := []string{"operator-key", "store"} - err := operator.AddControlGroup(views, auth) + err := operator.Delegate(views, auth) c.Assert(err, IsNil) c.Assert(len(operator.Groups), Equals, 1) @@ -67,7 +197,7 @@ func (s *confdbCtrlSuite) TestAddGroupOK(c *C) { c.Assert(g.Authentication, DeepEquals, expectedAuth) } -func (s *confdbCtrlSuite) TestAddGroupFail(c *C) { +func (s *confdbCtrlSuite) TestDelegateFail(c *C) { operator := confdb.Operator{ID: "canonical"} type testcase struct { @@ -76,44 +206,111 @@ func (s *confdbCtrlSuite) TestAddGroupFail(c *C) { err string } tcs := []testcase{ - {err: `cannot add group: "auth" must be a non-empty list`}, - {auth: []string{"magic"}, err: "cannot add group: invalid authentication method: magic"}, - {auth: []string{"store"}, err: `cannot add group: "views" must be a non-empty list`}, + {err: `cannot delegate: "auth" must be a non-empty list`}, + {auth: []string{"magic"}, err: "cannot delegate: invalid authentication method: magic"}, + {auth: []string{"store"}, err: `cannot delegate: "views" must be a non-empty list`}, { views: []string{"a/b/c/d"}, auth: []string{"store"}, - err: `view "a/b/c/d" must be in the format account/confdb/view`, + err: `cannot delegate: view "a/b/c/d" must be in the format account/confdb/view`, }, { views: []string{"a/b"}, auth: []string{"store"}, - err: `view "a/b" must be in the format account/confdb/view`, + err: `cannot delegate: view "a/b" must be in the format account/confdb/view`, }, { views: []string{"ab/"}, auth: []string{"store"}, - err: `view "ab/" must be in the format account/confdb/view`, + err: `cannot delegate: view "ab/" must be in the format account/confdb/view`, }, { views: []string{"@foo/network/control-device"}, auth: []string{"store"}, - err: "invalid Account ID @foo", + err: "cannot delegate: invalid Account ID @foo", }, { views: []string{"canonical/123/control-device"}, auth: []string{"store"}, - err: "invalid confdb name 123", + err: "cannot delegate: invalid confdb name 123", }, { views: []string{"canonical/network/_view"}, auth: []string{"store"}, - err: "invalid view name _view", + err: "cannot delegate: invalid view name _view", + }, + } + + for i, tc := range tcs { + cmt := Commentf("test number %d", i+1) + err := operator.Delegate(tc.views, tc.auth) + c.Assert(err, NotNil) + c.Assert(err, ErrorMatches, tc.err, cmt) + } +} + +func (s *confdbCtrlSuite) TestRevoke(c *C) { + operator := confdb.Operator{ID: "john"} + controlInterface := confdb.ViewRef{ + Account: "canonical", Confdb: "network", View: "control-interface", + } + observeInterface := confdb.ViewRef{ + Account: "canonical", Confdb: "network", View: "observe-interface", + } + + err := operator.Delegate( + []string{"canonical/network/control-interface", "canonical/network/observe-interface"}, + []string{"operator-key"}, + ) + c.Assert(err, IsNil) + + operator.Revoke( + []string{"canonical/network/control-interface"}, + []string{"operator-key"}, + ) + c.Check(operator.IsDelegated(&controlInterface, confdb.OperatorKey), Equals, false) + + // test idempotency + operator.Revoke( + []string{"canonical/network/control-interface"}, + []string{"operator-key"}, + ) + c.Check(operator.IsDelegated(&controlInterface, confdb.OperatorKey), Equals, false) + + // revoke all auth + err = operator.Delegate( + []string{"canonical/network/observe-interface", "canonical/network/control-vpn"}, + []string{"store", "operator-key"}, + ) + c.Assert(err, IsNil) + + err = operator.Revoke([]string{"canonical/network/observe-interface"}, nil) + c.Assert(err, IsNil) + + c.Check(operator.IsDelegated(&observeInterface, confdb.OperatorKey), Equals, false) + c.Check(operator.IsDelegated(&observeInterface, confdb.Store), Equals, false) +} + +func (s *confdbCtrlSuite) TestRevokeFail(c *C) { + operator := confdb.Operator{ID: "canonical"} + + type testcase struct { + views []string + auth []string + err string + } + tcs := []testcase{ + {auth: []string{"magic"}, err: "cannot revoke: invalid authentication method: magic"}, + { + views: []string{"invalid"}, + auth: []string{"store"}, + err: `cannot revoke: view "invalid" must be in the format account/confdb/view`, }, } for i, tc := range tcs { cmt := Commentf("test number %d", i+1) - err := operator.AddControlGroup(tc.views, tc.auth) + err := operator.Revoke(tc.views, tc.auth) c.Assert(err, NotNil) c.Assert(err, ErrorMatches, tc.err, cmt) } diff --git a/confdb/export_test.go b/confdb/export_test.go index 828159840ab..20d508615ff 100644 --- a/confdb/export_test.go +++ b/confdb/export_test.go @@ -34,3 +34,23 @@ var IsValidAuthenticationMethod = isValidAuthenticationMethod // convertToAuthenticationMethods exposed for tests var ConvertToAuthenticationMethods = convertToAuthenticationMethods + +// groupWithView exposed for tests +func (o *Operator) GroupWithView(view *ViewRef) (*ControlGroup, int) { + return o.groupWithView(view) +} + +// groupWithAuthentication exposed for tests +func (o *Operator) GroupWithAuthentication(auth []AuthenticationMethod) *ControlGroup { + return o.groupWithAuthentication(auth) +} + +// findView exposed for tests +func (g *ControlGroup) FindView(view *ViewRef) (int, bool) { + return g.findView(view) +} + +// compare exposed for tests +func (v *ViewRef) Compare(b *ViewRef) int { + return v.compare(b) +} diff --git a/daemon/api_confdb.go b/daemon/api_confdb.go index 1c4551b3671..861fbc382ee 100644 --- a/daemon/api_confdb.go +++ b/daemon/api_confdb.go @@ -47,7 +47,7 @@ func getView(c *Command, r *http.Request, _ *auth.UserState) Response { st.Lock() defer st.Unlock() - if err := validateConfdbFeatureFlag(st); err != nil { + if err := validateConfdbFeatureFlag(st, features.Confdbs); err != nil { return err } @@ -73,7 +73,7 @@ func setView(c *Command, r *http.Request, _ *auth.UserState) Response { st.Lock() defer st.Unlock() - if err := validateConfdbFeatureFlag(st); err != nil { + if err := validateConfdbFeatureFlag(st, features.Confdbs); err != nil { return err } @@ -122,16 +122,20 @@ func toAPIError(err error) *apiError { } } -func validateConfdbFeatureFlag(st *state.State) *apiError { +func validateConfdbFeatureFlag(st *state.State, feature features.SnapdFeature) *apiError { tr := config.NewTransaction(st) - enabled, err := features.Flag(tr, features.Confdbs) + enabled, err := features.Flag(tr, feature) if err != nil && !config.IsNoOption(err) { - return InternalError(fmt.Sprintf("internal error: cannot check confdbs feature flag: %s", err)) + return InternalError( + fmt.Sprintf("internal error: cannot check %s feature flag: %s", feature.String(), err), + ) } if !enabled { - _, confName := features.Confdbs.ConfigOption() - return BadRequest(fmt.Sprintf(`"confdbs" feature flag is disabled: set '%s' to true`, confName)) + _, confName := feature.ConfigOption() + return BadRequest( + fmt.Sprintf(`"%s" feature flag is disabled: set '%s' to true`, feature.String(), confName), + ) } return nil }