Add security system #312

Closed
wants to merge 11 commits into
from
View
@@ -1,7 +1,7 @@
// -*- Mode: Go; indent-tabs-mode: t -*-
/*
- * Copyright (C) 2015 Canonical Ltd
+ * Copyright (C) 2015-2016 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
@@ -19,6 +19,10 @@
package caps
+import (
+ "fmt"
+)
+
// Capability holds information about a capability that a snap may request
// from a snappy system to do its job while running on it.
type Capability struct {
@@ -42,3 +46,22 @@ type Capability struct {
func (c Capability) String() string {
return c.Name
}
+
+// SetAttr sets capability attribute to a given value.
+// TODO: remove temporary function implementation once attrtypes are merged.
@niemeyer

niemeyer Jan 12, 2016

Contributor

Can we please drop these temporary methods altogether for the time being? They're purely a setter and a getter for a public map attribute, with literally zero benefit. That means we're simply adding cruft that needs to be removed later and that is hard to agree or disagree because the full replacement TODOs give them a blank check of existence.

+func (c *Capability) SetAttr(name string, value string) error {
+ if c.Attrs == nil {
+ c.Attrs = make(map[string]string)
+ }
+ c.Attrs[name] = value
+ return nil
+}
+
+// GetAttr gets capability attribute with a given name.
+// TODO: remove temporary function implementation once attrtypes are merged.
+func (c *Capability) GetAttr(name string) (interface{}, error) {
+ if value, ok := c.Attrs[name]; ok {
+ return value, nil
+ }
+ return nil, fmt.Errorf("%s is not set", name)
+}
View
@@ -0,0 +1,29 @@
+// -*- Mode: Go; indent-tabs-mode: t -*-
+
+/*
+ * Copyright (C) 2016 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 caps
+
+// securitySystem abstracts the implementation details of a given security
+// system (syccomp, apparmor, etc) from the point of view of a capability.
+type securitySystem interface {
+ // TODO: change snap to appropriate type after current transition
+ GrantPermissions(snapName string, cap *Capability) error
@niemeyer

niemeyer Jan 12, 2016

Contributor

We agreed to not have this in our call yesterday. Every single capability type in our system will be unique, precisely because its solo purpose of existence is informing the system of a new capacity which comes along with new security behavior. We cannot hand off an arbitrary capability to a security system and expect it will know what to do with it.

+ // TODO: change snap to appropriate type after current transition
+ RevokePermissions(snapName string, cap *Capability) error
+}
View
@@ -0,0 +1,73 @@
+// -*- Mode: Go; indent-tabs-mode: t -*-
+
+/*
+ * Copyright (C) 2016 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 caps
+
+import (
+ "fmt"
+
+ "github.com/ubuntu-core/snappy/snappy"
+)
+
+// legacySecurtySystem aids in the ongoing work to transition capability system
+// from hwaccess API to a more direct approach. It allows particular capability
+// types to define a common interface that doesn't expose hwaccess API anymore.
+type legacySecuritySystem struct {
+ AttrName string
+}
+
+func (sec *legacySecuritySystem) GrantPermissions(snapName string, cap *Capability) error {
+ const errPrefix = "cannot grant permissions"
+ // Find the snap
+ snap := snappy.ActiveSnapByName(snapName)
+ if snap == nil {
+ return fmt.Errorf("%s, no such package", errPrefix)
+ }
+ // Fetch the attribute where the path is stored
+ path, err := cap.GetAttr(sec.AttrName)
+ if err != nil {
+ return fmt.Errorf("%s, cannot get required attribute: %q", errPrefix, err)
@niemeyer

niemeyer Jan 12, 2016

Contributor

Here the mismatch becomes obvious. How can the legacy security system know that an arbitrary capability which it has no knowledge about contains a given attribute?

+ }
+ // Use hw-access layer to grant the permissions to access
+ qualName := snappy.QualifiedName(snap)
+ if err := snappy.AddHWAccess(qualName, path.(string)); err != nil {
+ return fmt.Errorf("%s, hw-access failed: %q", errPrefix, err)
+ }
+ return nil
+}
+
+func (sec *legacySecuritySystem) RevokePermissions(snapName string, cap *Capability) error {
+ const errPrefix = "cannot revoke permissions"
+ // Find the snap
+ snap := snappy.ActiveSnapByName(snapName)
+ if snap == nil {
+ return fmt.Errorf("%s, no such package", errPrefix)
+ }
+ // Fetch the attribute where the path is stored
+ path, err := cap.GetAttr(sec.AttrName)
+ if err != nil {
+ return fmt.Errorf("%s, cannot get required attribute: %q", errPrefix, err)
+ }
+ // Use hw-access layer to grant the permissions to access
+ qualName := snappy.QualifiedName(snap)
+ if err := snappy.RemoveHWAccess(qualName, path.(string)); err != nil {
+ return fmt.Errorf("%s, hw-access failed: %q", errPrefix, err)
+ }
+ return nil
+}
View
@@ -0,0 +1,69 @@
+// -*- Mode: Go; indent-tabs-mode: t -*-
+
+/*
+ * Copyright (C) 2016 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 caps
+
+const (
+ mockSecurityInitial = iota
+ mockSecurityGranted = iota
+ mockSecurityRevoked = iota
+)
+
+type mockSecurity struct {
+ SecurityName string
+ StateMap map[string]int
+ GrantErrorMap map[string]error
+ RevokeErrorMap map[string]error
+}
+
+func (sec *mockSecurity) GrantPermissions(snapName string, cap *Capability) error {
+ if err := sec.GrantErrorMap[snapName]; err != nil {
+ return err
+ }
+ if sec.StateMap == nil {
+ sec.StateMap = make(map[string]int)
+ }
+ sec.StateMap[snapName] = mockSecurityGranted
+ return nil
+}
+
+func (sec *mockSecurity) RevokePermissions(snapName string, cap *Capability) error {
+ if err := sec.RevokeErrorMap[snapName]; err != nil {
+ return err
+ }
+ if sec.StateMap == nil {
+ sec.StateMap = make(map[string]int)
+ }
+ sec.StateMap[snapName] = mockSecurityRevoked
+ return nil
+}
+
+func (sec *mockSecurity) SetGrantPermissionsError(snapName string, err error) {
+ if sec.GrantErrorMap == nil {
+ sec.GrantErrorMap = make(map[string]error)
+ }
+ sec.GrantErrorMap[snapName] = err
+}
+
+func (sec *mockSecurity) SetRevokePermissionsError(snapName string, err error) {
+ if sec.RevokeErrorMap == nil {
+ sec.RevokeErrorMap = make(map[string]error)
+ }
+ sec.RevokeErrorMap[snapName] = err
+}
@@ -0,0 +1,64 @@
+// -*- Mode: Go; indent-tabs-mode: t -*-
+
+/*
+ * Copyright (C) 2015 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 caps
+
+import (
+ "fmt"
+
+ . "gopkg.in/check.v1"
+)
+
+type MockSecuritySuite struct{}
+
+var _ = Suite(&MockSecuritySuite{})
+
+func (s *MockSecuritySuite) TestGrantPermissionSuccess(c *C) {
+ const snapName = "snap"
+ sec := &mockSecurity{}
+ err := sec.GrantPermissions(snapName, testCapability)
+ c.Assert(err, IsNil)
+ c.Assert(sec.StateMap[snapName], Equals, mockSecurityGranted)
+}
+
+func (s *MockSecuritySuite) TestGrantPermissionFailure(c *C) {
+ const snapName = "snap"
+ sec := &mockSecurity{}
+ sec.SetGrantPermissionsError(snapName, fmt.Errorf("boom"))
+ err := sec.GrantPermissions(snapName, testCapability)
+ c.Assert(err, ErrorMatches, "boom")
+ c.Assert(sec.StateMap[snapName], Equals, mockSecurityInitial)
+}
+
+func (s *MockSecuritySuite) TestRevokePermissionSuccess(c *C) {
+ const snapName = "snap"
+ sec := &mockSecurity{}
+ err := sec.RevokePermissions(snapName, testCapability)
+ c.Assert(err, IsNil)
+ c.Assert(sec.StateMap[snapName], Equals, mockSecurityRevoked)
+}
+
+func (s *MockSecuritySuite) TestRevokePermissionFailure(c *C) {
+ const snapName = "snap"
+ sec := &mockSecurity{}
+ sec.SetRevokePermissionsError(snapName, fmt.Errorf("boom"))
+ err := sec.RevokePermissions(snapName, testCapability)
+ c.Assert(err, ErrorMatches, "boom")
+ c.Assert(sec.StateMap[snapName], Equals, mockSecurityInitial)
+}
View
@@ -34,6 +34,9 @@ type Type struct {
// RequiredAttrs contains names of attributes that are required by
// capability of this type.
RequiredAttrs []string
+ // SecuritySystems contains the associated security systems that enable actual
+ // access to system resources needed by this capability.
+ SecuritySystems []securitySystem
}
var (
@@ -83,3 +86,52 @@ func (t *Type) MarshalJSON() ([]byte, error) {
func (t *Type) UnmarshalJSON(data []byte) error {
return json.Unmarshal(data, &t.Name)
}
+
+// GrantPermissions makes it possible for the package `snapName` to use the
+// resource described by the capability `cap`.
+func (t *Type) GrantPermissions(snapName string, cap *Capability) error {
+ // Ensure that the capability is valid
+ if err := t.Validate(cap); err != nil {
+ return err
+ }
+ // Grant all permissions required.
+ for i := range cap.Type.SecuritySystems {
+ sec := cap.Type.SecuritySystems[i]
@chipaca

chipaca Jan 12, 2016

Member

why not i, sec := range ...?

+ if err := sec.GrantPermissions(snapName, cap); err != nil {
+ // If we already granted something, try to revoke that permission instead
+ // NOTE: "i - 1" because grant that fails should fail atomically
+ for j := i - 1; j >= 0; j-- {
+ sec := cap.Type.SecuritySystems[j]
+ if err := sec.RevokePermissions(snapName, cap); err != nil {
+ // XXX: Should we do something other than panic here?
+ panic(fmt.Sprintf("unable to revoke partially granted permissions: %q", err))
@chipaca

chipaca Jan 12, 2016

Member

eventually yes, there's got to be a big red "give up and work out the current state of the system from the top again" button in the Overseer

+ }
+ }
+ return err
+ }
+ }
+ return nil
+}
+
+// RevokePermissions undoes the effects of GrantPermissions.
+func (t *Type) RevokePermissions(snapName string, cap *Capability) error {
@chipaca

chipaca Jan 12, 2016

Member

these two are so similar!

+ // Ensure that the capability is valid
+ if err := t.Validate(cap); err != nil {
+ return err
+ }
+ // Revoke all permissions required
+ for i, sec := range cap.Type.SecuritySystems {
+ if err := sec.RevokePermissions(snapName, cap); err != nil {
+ // If we already revoked something, try to grant that same permission again
+ for j := i - 1; j >= 0; j-- {
+ sec := cap.Type.SecuritySystems[j]
+ if err := sec.GrantPermissions(snapName, cap); err != nil {
+ // XXX: Should we do something other than panic here?
+ panic(fmt.Sprintf("unable to grant partially revoked permissions: %q", err))
+ }
+ }
+ return err
+ }
+ }
+ return nil
+}
Oops, something went wrong.