interfaces: support permanent security snippets #583

Merged
merged 10 commits into from Mar 4, 2016

Conversation

Projects
None yet
3 participants
Contributor

zyga commented Mar 4, 2016

This branch changes the Interface interface to expose four security-related
methods. The methods are Connected{Slot,Plug}Snippet(), which replace the older
{Slot,Plug}SecuritySnippet(), and a pair of new Permanent{Slot,Plug}Snippet().

This change is driven by the realization that we can simplify security
and number of interfaces by disassociating some permissions from
established connections.

For example, a display server interface can now be just a single
interface rather than two. Consider this example:

There's a Mir snap, with the "mir" slot.
There's a xeyes snap with a "mir" plug.

The mir snap, simply because it has the mir slot gets to have access
to graphics cards and all the required machinery. Mir can start even
without xeyes running yet.

As a connection is made between xeyes and mir, a new set of permissions
are granted: Xeyes can now talk to mir socket.

The same example works with any managed, shared resource that the
managing snap needs to be able to control regardless of who is connected
at a particular moment.

This patch also changes test interface that is use for testing to
support the extra methods.

zyga added some commits Mar 4, 2016

interfaces: support static and connections-specific security snippets
This patch changes the Interface interface to expose four
security-related methods. In addition to existing SlotSecuritySnippet()
and PlugSecuritySnippet() there are now StaticSlotSecuritySnippet() and
StaticPlugSecuritySnippet().

This change is driven by the realization that we can simplify security
and number of interfaces by disassociating some permissions from
established connections.

For example, a display server interface can now be just a single
interface rather than two. Consider this example:

1. There's a Mir snap, with the "mir" slot.
2. There's a xeyes snap with a "mir" plug.

The mir snap, simply because it has the mir *slot* gets to have access
to graphics cards and all the required machinery. Mir can start even
without xeyes running yet.

As a connection is made between xeyes and mir, a new set of permissions
are granted: Xeyes can now talk to mir socket.

The same example works with any managed, shared resource that the
managing snap needs to be able to control regardless of who is connected
at a particular moment.

This patch also changes test interface that is use for testing to
support the extra methods.

Signed-off-by: Zygmunt Krynicki <zygmunt.krynicki@canonical.com>
interfaces: use 'iface' to refer to interface object
Signed-off-by: Zygmunt Krynicki <zygmunt.krynicki@canonical.com>
interfaces: respect static security snippets
This patch expands the logic that collects security snippets for a
particular snap to respect the static, connection agnostic, security
snippets.

Signed-off-by: Zygmunt Krynicki <zygmunt.krynicki@canonical.com>
interfaces: add tests for static security snippets
Signed-off-by: Zygmunt Krynicki <zygmunt.krynicki@canonical.com>
interfaces/builtin: add dummy static snippets to bool-file
Signed-off-by: Zygmunt Krynicki <zygmunt.krynicki@canonical.com>
interfaces/repo_test.go
+ return nil, fmt.Errorf("cannot compute static snippet for consumer")
+ },
+ StaticPlugSecuritySnippetCallback: func(plug *Plug, securitySystem SecuritySystem) ([]byte, error) {
+ return nil, fmt.Errorf("cannot compute static snippet for provider")
@jdstrand

jdstrand Mar 4, 2016

Contributor

The language of 'consumer' and 'provider' here seems at odds with the changes above where you renamed consumer to slot and producer to plug. Should this be more consistent?

@zyga

zyga Mar 4, 2016

Contributor

I've added a TODO to do a quick pass over all the test data to use consistent wording.

I wonder what that wording should be though? Any suggestions

+ c.Assert(err, ErrorMatches, "cannot compute static snippet for provider")
+ c.Check(snippets, IsNil)
+ snippets, err = repo.SecuritySnippetsForSnap(s.slot.Snap, testSecurity)
+ c.Assert(err, ErrorMatches, "cannot compute static snippet for consumer")
@jdstrand

jdstrand Mar 4, 2016

Contributor

Likewise in this section.

Contributor

jdstrand commented Mar 4, 2016

Without doing full code-review, the direction of the branch LGTM and is consistent with my understanding of the meeting yesterday. I do have some small questions inline.

interfaces/builtin/bool_file.go
@@ -101,6 +101,12 @@ func (iface *BoolFileInterface) SlotSecuritySnippet(plug *interfaces.Plug, slot
}
}
+// StaticSlotSecuritySnippet returns the configuration snippet required to provide a bool-file interface.
+func (iface *BoolFileInterface) StaticSlotSecuritySnippet(slot *interfaces.Slot, securitySystem interfaces.SecuritySystem) ([]byte, error) {
@niemeyer

niemeyer Mar 4, 2016

Contributor

Let's please call these "Permanent" rather than "Static".

Let's please have these method names instead:

  • PermanentSlotSnippet
  • PermanentPlugSnippet
  • ConnectedSlotSnippet
  • ConnectedPlugSnippet
@zyga

zyga Mar 4, 2016

Contributor

+1, will do, thanks

interfaces,interfaces/builtin: s/Static/Permanent/g
We wish to talk about *permanent* security snippets, not static, since
they already can vary on either plug or slot. They are permanent because
they are not associated with any connection.

Signed-off-by: Zygmunt Krynicki <zygmunt.krynicki@canonical.com>
interfaces/core.go
@@ -78,23 +78,61 @@ type Interface interface {
// SanitizeSlot checks if a slot is correct, altering if necessary.
SanitizeSlot(slot *Slot) error
- // SlotSecuritySnippet returns the configuration snippet needed by the
- // given security system to allow a snap to offer a slot of this interface.
+ // StaticPlugSecuritySnippet returns static, plug-side security snippet.
@niemeyer

niemeyer Mar 4, 2016

Contributor
// PermanentPlugSnippet returns the snippet of text for the given security system that is used
// during the whole lifetime of affected applications, whether the plug is connected or not.

Others should be changed in an equivalent way.

@niemeyer niemeyer changed the title from interfaces: support static and connections-specific security snippets to interfaces: support permanent security snippets Mar 4, 2016

Contributor

niemeyer commented Mar 4, 2016

LGTM, assuming the indicated changes land with it.

Contributor

niemeyer commented Mar 4, 2016

Please note the description needs to be tweaked before using it as the commit message.

zyga added some commits Mar 4, 2016

interfaces: fix typo
Signed-off-by: Zygmunt Krynicki <zygmunt.krynicki@canonical.com>
interfaces,interfaces/builtin: refer to Snippets (sans Security)
This patch applies this vim regular expression replacement:

:%s/SlotSecuritySnippet/SlotSnippet/g|%s/PlugSecuritySnippet/PlugSnippet/g

This drops the word "Security" from various snippet-related functions.

Signed-off-by: Zygmunt Krynicki <zygmunt.krynicki@canonical.com>
interfaces,interfaces/builtin: refer to *Connected* snippets
This patch applies this vim regular expression replacement:

:%s/\<PlugSnippet\>/ConnectedPlugSnippet/g|%s/\<SlotSnippet\>/ConnectedSlotSnippet/g

Signed-off-by: Zygmunt Krynicki <zygmunt.krynicki@canonical.com>
interfaces: tweak commends on Interface snippet methods
Signed-off-by: Zygmunt Krynicki <zygmunt.krynicki@canonical.com>

zyga added a commit that referenced this pull request Mar 4, 2016

Merge pull request #583 from zyga/static-snippets
interfaces: support permanent security snippets

@zyga zyga merged commit 71778ba into snapcore:master Mar 4, 2016

2 checks passed

Integration tests Success 67 tests run, 0 skipped, 0 failed.
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@zyga zyga deleted the zyga:static-snippets branch Mar 8, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment