Merge lp:~mvo/snappy/snappy-add-apparmor-override into lp:~snappy-dev/snappy/snappy-moved-to-github

[Michael Vogt]

Add $PartID.json.additional apparmor files on OEM hardware assign installs.

This branch adds a $PartID.json.additional file for each OEM hardware
assign package. This way the default apparmor policy can be deny-all
for /dev and we only open that for apps that need /dev - in which case
the launcher will use the device cgroup to grant the specific access.

[Sergio Schvezov (sergiusens)]

Hey there, this looks good, I added a couple of comments but they may be irrelevant as I haven't been looking at apparmor related tasks.

[Jamie Strandboge (jdstrand)]

How does this work with the 'snappy hw-assign'? Is the idea that the OEM hardware assign adds /dev/** since that implements all the udev tagging, but that hw-assign will still only add just what the user said (along with the udev rules from the other branch)?

review: Needs Information

[Michael Vogt (mvo)]

Yes, hwassign adds specific rules, oem all

On 22 April 2015 15:09:43 CEST, Jamie Strandboge <email address hidden> wrote:
>Review: Needs Information
>
>How does this work with the 'snappy hw-assign'? Is the idea that the
>OEM hardware assign adds /dev/** since that implements all the udev
>tagging, but that hw-assign will still only add just what the user said
>(along with the udev rules from the other branch)?
>--
>https://code.launchpad.net/~mvo/snappy/snappy-add-apparmor-override/+merge/257076
>You are the owner of lp:~mvo/snappy/snappy-add-apparmor-override.

--
Sent from my Android phone with K-9 Mail. Please excuse my brevity.

[Michael Vogt (mvo)]

Thanks for the review! I addressed the issues, please let me know if there is anything else I can do or if I missed anything.

[Michael Vogt (mvo)]

Branch updated and conflicts resolved.

[Sergio Schvezov (sergiusens)]

thanks!

review: Approve

[Jamie Strandboge (jdstrand)]

review: Approve

=== modified file 'snappy/oem.go'
--- snappy/oem.go	2015-04-22 08:27:20 +0000
+++ snappy/oem.go	2015-04-22 14:48:35 +0000
@@ -24,6 +24,7 @@
 	"errors"
 	"fmt"
 	"io/ioutil"
+	"log"
 	"os"
 	"os/exec"
 	"path/filepath"
@@ -145,6 +146,15 @@
 		os.Remove(f)
 	}
 
+	// cleanup the additional files
+	for _, h := range m.OEM.Hardware.Assign {
+		jsonAdditionalPath := filepath.Join(snapAppArmorDir, fmt.Sprintf("%s.json.additional", h.PartID))
+		err = os.Remove(jsonAdditionalPath)
+		if err != nil && !os.IsNotExist(err) {
+			log.Printf("Failed to remove %v: %v", jsonAdditionalPath, err)
+		}
+	}
+
 	return nil
 }
 
@@ -188,11 +198,43 @@
 	return runUdevAdm("udevadm", "trigger")
 }
 
+const apparmorAdditionalContent = `{
+ "write_path": [
+   "/dev/**"
+ ]
+}`
+
+// writeApparmorAdditionalFile generate a $partID.json.additional file.
+//
+// This file grants additional access on top of the existing apparmor json
+// rules. This is required for the OEM hardware assign code because by
+// default apparmor will not allow access to /dev. We grant access here
+// and the ubuntu-core-launcher is then used to generate a confinement
+// based on the devices cgroup.
+func writeApparmorAdditionalFile(m *packageYaml) error {
+	if err := helpers.EnsureDir(snapAppArmorDir, 0755); err != nil {
+		return err
+	}
+
+	for _, h := range m.OEM.Hardware.Assign {
+		jsonAdditionalPath := filepath.Join(snapAppArmorDir, fmt.Sprintf("%s.json.additional", h.PartID))
+		if err := ioutil.WriteFile(jsonAdditionalPath, []byte(apparmorAdditionalContent), 0644); err != nil {
+			return err
+		}
+	}
+
+	return nil
+}
+
 func installOemHardwareUdevRules(m *packageYaml) error {
 	if err := writeOemHardwareUdevRules(m); err != nil {
 		return err
 	}
 
+	if err := writeApparmorAdditionalFile(m); err != nil {
+		return err
+	}
+
 	if err := activateOemHardwareUdevRules(); err != nil {
 		return err
 	}

=== modified file 'snappy/oem_test.go'
--- snappy/oem_test.go	2015-04-20 21:04:42 +0000
+++ snappy/oem_test.go	2015-04-22 14:48:35 +0000
@@ -21,6 +21,11 @@
 package snappy
 
 import (
+	"io/ioutil"
+	"path/filepath"
+
+	"launchpad.net/snappy/helpers"
+
 	. "launchpad.net/gocheck"
 )
 
@@ -57,3 +62,30 @@
 func (s *OemSuite) TestStoreID(c *C) {
 	c.Assert(StoreID(), Equals, "ninjablocks")
 }
+
+func (s *OemSuite) TestWriteApparmorAdditionalFile(c *C) {
+	m, err := parsePackageYamlData(hardwareYaml)
+	c.Assert(err, IsNil)
+
+	err = writeApparmorAdditionalFile(m)
+	c.Assert(err, IsNil)
+
+	content, err := ioutil.ReadFile(filepath.Join(snapAppArmorDir, "device-hive-iot-hal.json.additional"))
+	c.Assert(err, IsNil)
+	c.Assert(string(content), Equals, apparmorAdditionalContent)
+}
+
+func (s *OemSuite) TestCleanupOemHardwareRules(c *C) {
+	m, err := parsePackageYamlData(hardwareYaml)
+	c.Assert(err, IsNil)
+
+	err = writeApparmorAdditionalFile(m)
+	c.Assert(err, IsNil)
+
+	additionalFile := filepath.Join(snapAppArmorDir, "device-hive-iot-hal.json.additional")
+	c.Assert(helpers.FileExists(additionalFile), Equals, true)
+
+	err = cleanupOemHardwareUdevRules(m)
+	c.Assert(err, IsNil)
+	c.Assert(helpers.FileExists(additionalFile), Equals, false)
+}