Add security system #312
Add security system #312
Conversation
Signed-off-by: Zygmunt Krynicki <zygmunt.krynicki@canonical.com>
This patch adds a small dummy implementation for getting/setting capability attribute. The real implementation is proposed for merging separately. Signed-off-by: Zygmunt Krynicki <zygmunt.krynicki@canonical.com>
This patch adds an interface for security systems. Security systems are an abstraction for configuring security associated with capabilities. Each type will define security systems that make the permission side of the capability work. Various security systems can be used (apparmor, seccomp, dbus, etc.). Signed-off-by: Zygmunt Krynicki <zygmunt.krynicki@canonical.com>
Signed-off-by: Zygmunt Krynicki <zygmunt.krynicki@canonical.com>
Signed-off-by: Zygmunt Krynicki <zygmunt.krynicki@canonical.com>
This patch adds a wrapper to use hw-access as a capability security system. Signed-off-by: Zygmunt Krynicki <zygmunt.krynicki@canonical.com>
Signed-off-by: Zygmunt Krynicki <zygmunt.krynicki@canonical.com>
Signed-off-by: Zygmunt Krynicki <zygmunt.krynicki@canonical.com>
Signed-off-by: Zygmunt Krynicki <zygmunt.krynicki@canonical.com>
| // 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.
Ah, great catch, corrected!
|
I was asked to look at the approach (ie, I haven't done an in depth code review or tested this). The direction this is going looks good from a security capabilities definition/grant/revoke perspective. Thanks! |
Signed-off-by: Zygmunt Krynicki <zygmunt.krynicki@canonical.com>
Signed-off-by: Zygmunt Krynicki <zygmunt.krynicki@canonical.com>
|
@jdstrand thanks for looking at this! |
| 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.
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
|
this seems ok to me. The amount of code that's exactly line-by-line the same between Type.RevokePermissions and Type.GrantPermissions makes me think there's got to be a way of writing the code once, but it can happen later if you'd rather. |
| @@ -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. | |||
There was a problem hiding this comment.
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.
|
@zyga I'm closing this for the time being. Once the points and design we discussed yesterday are addressed, please reopen it or push a new PR. |
This branch sets the stage for pluggable security systems.
Each capability type can now refer to a list of security systems. The type can be asked to grant or revoke permission expressed by those systems. A trivial "legacy" hw-assign security system is provided.
The main intent of this branch is to discuss the interface as I have some code that uses this for apparmor and seccomp coming up.