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

Allow types to hand out security snippets #326

Merged
merged 13 commits into from Jan 15, 2016
Merged

Conversation

zyga
Copy link
Collaborator

@zyga zyga commented Jan 14, 2016

This patch commences the work on the security side of capabilities. Each
capability type now has a way to hand out "snippets" of security
information applicable to a given security system. The information
describes alterations of the security system needed to consume a given
capability.

The patch defines three security system names:

  • apparmor
  • seccomp
  • dbus

This patch commences the work on the security side of capabilities. Each
capability type now has a way to hand out "snippets" of security
information applicable to a given security system. The information
describes alterations of the security system needed to consume a given
capability.

The patch defines three security system names:
 - apparmor
 - seccomp
 - dbus

Signed-off-by: Zygmunt Krynicki <zygmunt.krynicki@canonical.com>

package caps

// NOTE: all the security constants are used by Type.SecuritySnippet()
Copy link
Contributor

Choose a reason for hiding this comment

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

The note doesn't seem necessary, and will likely be wrong soon. The constants are used wherever they are used.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Removed.

Signed-off-by: Zygmunt Krynicki <zygmunt.krynicki@canonical.com>
Signed-off-by: Zygmunt Krynicki <zygmunt.krynicki@canonical.com>
@@ -31,6 +31,9 @@ type Type interface {
Name() string
// Sanitize a capability (altering if necessary).
Sanitize(c *Capability) error
// Obtain the security snippet for the given security system.
// If no security snippet is needed, hand out empty string.
Copy link
Contributor

Choose a reason for hiding this comment

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

// SecuritySnippet returns the configuration snippet that should be used by
// the given security system to enable this capability to be consumed.
// An empty snippet is returned when the capability doesn't require anything
// from the security system to work, in addition to the default configuration.
// ErrUnknownSecurity is returned when the capability cannot deal with the
// requested security system.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Copied, thanks :-)

Signed-off-by: Zygmunt Krynicki <zygmunt.krynicki@canonical.com>
@@ -31,6 +31,9 @@ type Type interface {
Name() string
// Sanitize a capability (altering if necessary).
Sanitize(c *Capability) error
// Obtain the security snippet for the given security system.
// If no security snippet is needed, hand out empty string.
SecuritySnippet(c *Capability, securitySystem string) (string, error)
Copy link
Contributor

Choose a reason for hiding this comment

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

Considering the data nature, I think the most appropriate result type here is actually a []byte.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Ah, good point, thanks.

Signed-off-by: Zygmunt Krynicki <zygmunt.krynicki@canonical.com>
Signed-off-by: Zygmunt Krynicki <zygmunt.krynicki@canonical.com>
@@ -60,6 +63,24 @@ func (t *BoolFileType) Sanitize(c *Capability) error {
return nil
}

// SecuritySnippet for bool-file capability type.
Copy link
Contributor

Choose a reason for hiding this comment

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

The standard convention for Go comments should be used here too.

@niemeyer
Copy link
Contributor

The structure looks good. Please ping again when the details above are addressed.

There are two changes, based on feedback from code review.

First the return type is now using []byte rather than string since it
will likely fit the intended problem better (it's not clear that all
security systems will consume plain strings).

The second change add a well-known error type for reporting unknown
security systems.

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>
This error will likely be checked for in a handful of places in the
whole codebase so we don't have to have a unique type for it. At the
same time we'll know exactly what the security system name was at that
point so carrying this around is useless.

Signed-off-by: Zygmunt Krynicki <zygmunt.krynicki@canonical.com>
// TODO: switch to the real path later
path := c.Attrs["path"]
// Allow read, write and lock on the file designated by the path.
return ([]byte)(fmt.Sprintf("%s rwl,\n", path)), nil
Copy link
Contributor

Choose a reason for hiding this comment

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

[]byte("foo") works.. no need for the extra parenthesis.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Fixed, C habits die hard.

@niemeyer
Copy link
Contributor

LGTM.. just a trivial above.

Signed-off-by: Zygmunt Krynicki <zygmunt.krynicki@canonical.com>
@mvo5
Copy link
Contributor

mvo5 commented Jan 15, 2016

Looks good.

mvo5 added a commit that referenced this pull request Jan 15, 2016
Allow types to hand out security snippets
@mvo5 mvo5 merged commit 981d961 into snapcore:master Jan 15, 2016
@zyga zyga deleted the type-security branch February 1, 2016 11:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants