interfaces: add a serial-port interface #1301

Merged
merged 4 commits into from Jun 20, 2016

Conversation

Projects
None yet
4 participants
Contributor

jocave commented Jun 9, 2016

A new builtin interface that allows basic access to serial ports. Uses just AppArmor snippets to provider access to /dev/tty[A-Z]{1,3}[0-9]{1,3}$

Contributor

snappy-m-o commented Jun 9, 2016

Can one of the admins verify this patch?

Contributor

zyga commented Jun 9, 2016

whitelist this please

+ if !ok || path == "" {
+ return fmt.Errorf("serial-port slot must have a path attribute")
+ }
+
@zyga

zyga Jun 9, 2016

Contributor

We should perhaps run filepath.Clean on the path before checking it (https://golang.org/pkg/path/filepath/#Clean)

interfaces/builtin/serial_port.go
+func (iface *SerialPortInterface) PermanentSlotSnippet(slot *interfaces.Slot, securitySystem interfaces.SecuritySystem) ([]byte, error) {
+ switch securitySystem {
+ case interfaces.SecurityAppArmor:
+ if iface.isSerialDevice(slot) {
@zyga

zyga Jun 9, 2016

Contributor

I don't think we should do it, you should just have a path() method that returns the path and panics if it is not vaild. This is only called when the slot is sanitized anyway.

interfaces/builtin/serial_port.go
+ case interfaces.SecurityAppArmor:
+ // Allow write and lock on the file designated by the path.
+ // Dereference symbolic links to file path handed out to apparmor
+ path, err := iface.path(slot)
@zyga

zyga Jun 9, 2016

Contributor

Feel free to panic in iface.path(), this is only called after sanitiazation.

interfaces/builtin/serial_port.go
+}
+
+// isSerialDevice checks if a given serial-port slot has sensible path
+func (iface *SerialPortInterface) isSerialDevice(slot *interfaces.Slot) bool {
@zyga

zyga Jun 9, 2016

Contributor

I think this can be dropped unless you plan to call it somewhere other than Sanitize()

@@ -0,0 +1,246 @@
+// -*- Mode: Go; indent-tabs-mode: t -*-
+
@zyga

zyga Jun 9, 2016

Contributor

I'll skip this until the points above are addressed. Thanks

Contributor

zyga commented Jun 9, 2016

A few minor comments but this is close to landing IMHO.

Please add an entry in integration-tests that explain how this can be tested (with the snap you've made).

@zyga zyga added the Reviewed label Jun 9, 2016

Contributor

snappy-m-o commented Jun 14, 2016

Can one of the admins verify this patch?

Collaborator

mvo5 commented Jun 16, 2016

whitelist this branch

jocave added some commits Jun 2, 2016

interfaces/builtin/serial_port.go
+}
+
+func (iface *SerialPortInterface) path(slot *interfaces.Slot) (string, error) {
+ if path, ok := slot.Attrs["path"].(string); ok {
@zyga

zyga Jun 20, 2016

Contributor

Please change path() to return just the string, you don't return any useful errors so that it either "just works" or a panics.

Remove error return from path() signature
An error was never returned, func panics instead
Contributor

zyga commented Jun 20, 2016

retest this please

Contributor

zyga commented Jun 20, 2016

retest this please

Contributor

zyga commented Jun 20, 2016

LGTM

@zyga zyga merged commit c828337 into snapcore:master Jun 20, 2016

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment