snap-confine: interfaces: Address review comments

- Cleanup and remove TLS-padding permit from AppArmor rules
- Only allow binder access on Halium/libhybris systems
- Die in case the Halium environment cannot be mounted into the target
- Restructure binder device mount operation to iterate through a list of
  allowed device paths

Author: fredldotme

cmd/snap-confine/mount-support-hybris.c

@@ -100,15 +100,17 @@ static void sc_hybris_mount_android_rootfs(const char *rootfs_dir)
 	sc_identity old = sc_set_effective_identity(sc_root_group_identity());
 	int res = mkdir(android_rootfs_dir, 0755);
 	if (res != 0 && errno != EEXIST) {
-		die("cannot create tmpfs target %s", android_rootfs_dir);
+		die("cannot create bind-mount target %s", android_rootfs_dir);
 	}
 	if (res == 0 && (chown(android_rootfs_dir, 0, 0) < 0)) {
 		// Adjust the ownership only if we created the directory.
 		die("cannot change ownership of %s", android_rootfs_dir);
 	}
 	(void)sc_set_effective_identity(old);
 
-	(void)mount(SC_HYBRIS_ROOTFS, android_rootfs_dir, NULL, MS_BIND | MS_REC | MS_RDONLY, NULL);
+	if (mount(SC_HYBRIS_ROOTFS, android_rootfs_dir, NULL, MS_BIND | MS_REC | MS_RDONLY, NULL)) {
+		die("Cannot mount Halium environment into target");
+	}
 
 	sc_must_snprintf(path_buf, sizeof(path_buf), "%s%s", rootfs_dir, SC_HYBRIS_SYSTEM_SYMLINK);
 	const char *android_system_symlink = path_buf;

cmd/snap-confine/udev-support.c

@@ -124,23 +124,24 @@ static void sc_udev_allow_nvidia(sc_device_cgroup *cgroup)
  **/
 static void sc_udev_allow_hybris(sc_device_cgroup *cgroup)
 {
-	struct stat sbuf;
-
-	if (stat("/dev/binderfs/binder", &sbuf) == 0) {
-		sc_device_cgroup_allow(cgroup, S_IFCHR, major(sbuf.st_rdev),
-				       minor(sbuf.st_rdev));
-	}
-	if (stat("/dev/binderfs/hwbinder", &sbuf) == 0) {
-		sc_device_cgroup_allow(cgroup, S_IFCHR, major(sbuf.st_rdev),
-				       minor(sbuf.st_rdev));
-	}
-	if (stat("/dev/binder", &sbuf) == 0) {
-		sc_device_cgroup_allow(cgroup, S_IFCHR, major(sbuf.st_rdev),
-				       minor(sbuf.st_rdev));
+	/* Only go on here if this has been identified as a Halium/libhybris system
+	 *
+	 * In case the host happens to have binder available, but isn't identified as
+	 * a system requiring it to drive host-residing Android drivers, then return early,
+	 * otherwise we would open a hole between confined apps and unconfined Anbox or other
+	 * which causes them to communicate over a potentially unmediated IPC interface.
+	 */
+	struct stat propbuf;
+	if (stat("/system/build.prop", &propbuf) != 0) {
+		return;
 	}
-	if (stat("/dev/hwbinder", &sbuf) == 0) {
-		sc_device_cgroup_allow(cgroup, S_IFCHR, major(sbuf.st_rdev),
-				       minor(sbuf.st_rdev));
+
+	const char *paths[] = {"/dev/binderfs/binder", "/dev/binderfs/hwbinder", "/dev/binder", "/dev/hwbinder"};
+	for (int i = 0; i < sizeof(paths)/sizeof(paths[0]); i++) {
+		struct stat sbuf;
+		if (stat(paths[i], &sbuf) == 0) {
+			sc_device_cgroup_allow(cgroup, S_IFCHR, major(sbuf.st_rdev), minor(sbuf.st_rdev));
+		}
 	}
 }
 

interfaces/builtin/opengl.go

@@ -189,9 +189,6 @@ unix (send, receive) type=dgram peer=(addr="@var/run/nvidia-xdriver-*"),
 
 # Hybris support
 /{,var/}run/shm/hybris_shm_data rw, # FIXME: LP: #1226569 (make app-specific)
-# This is a LD_PRELOADed TLS padding library allowing use of both
-# Android 9 and glibc TLS slots at the same time. Only used on Halium 9.
-/var/lib/snapd/hostfs/usr/lib/@{multiarch}/libtls-padding.so  mr,
 /android{,/**} r,
 /{,android/}system/build.prop r,
 /{,android/}vendor/lib{,64}/**           r,