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
Add security system #312
Add security system #312
Changes from 9 commits
6ed9d63
b89199d
067bd5d
16c973e
63da285
f544c3b
8992d4e
ac296da
36e4a0c
42bf961
0b8f54b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -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 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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 | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -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 legacySecurtySystem struct { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Typo: should be legacySecuritySystem There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ah, great catch, corrected! |
||
AttrName string | ||
} | ||
|
||
func (sec *legacySecurtySystem) 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) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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 *legacySecurtySystem) 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 | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -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 | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -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) | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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 hardware | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 'hardware' is probably not the right word here for this comment when considering pure security capabilities (eg, 'firewall-management' as opposed to 'camera') There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Corrected. |
||
// 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] | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. why not |
||
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)) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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 { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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 | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.