layer: always set bundle to 0700 on creation

CHANGELOG.md

@@ -35,6 +35,16 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
   switch to the trivially different (but now more consistent) format.
   openSUSE/umoci#167
 
+### Security
+- `umoci unpack` used to create the bundle and rootfs with world
+  read-and-execute permissions by default. This could potentially result in an
+  unsafe rootfs (containing dangerous setuid binaries for instance) being
+  accessible by an unprivileged user. This has been fixed by always setting the
+  mode of the bundle to `0700`, which requires a user to explicitly work around
+  this basic protection. This scenario was documented in our security
+  documentation previously, but has now been fixed. openSUSE/umoci#181
+  openSUSE/umoci#182
+
 [cii]: https://bestpractices.coreinfrastructure.org/projects/1084
 [gomtree-v0.4.0]: https://github.com/vbatts/go-mtree/releases/tag/v0.4.0
 [user_namespaces]: http://man7.org/linux/man-pages/man7/user_namespaces.7.html

doc/security.md

@@ -67,7 +67,7 @@ demographic is not Plan9 so this tid-bit is not of much use to us.
 
 [securejoin]: https://github.com/cyphar/filepath-securejoin
 
-### Arbitrary Inode Creation ###
+### Arbitrary Inode and Mode Creation ###
 
 Note that this attack is only applicable if `umoci` is being executed as a
 **privileged user** (on GNU/Linux this means having `CAP_SYS_ADMIN` and a host
@@ -84,18 +84,14 @@ world-writeable inode that corresponds to the host's hard-drive (or
 `/dev/kmem`). There are a variety of other possible attacks that can occur.
 Note that the default `umoci` runtime configuration defends against containers
 from being able to mess with such files, but this doesn't help against
-host-side attackers.
+host-side attackers. This attack also could be used to provide an unprivileged
+user (in the host) access unsafe set-uid binaries, allowing for possible
+privilege escalation.
 
-**`umoci` currently has no specific defences against this attack.** We are
-working on it, but there is a legitimate concern that we are not truly spec
-compliant if we start rewriting layer entries (not to mention that in some
-cases this will cause the manifests to produce confusing results -- which isn't
-a problem with `--rootless` but will be a problem if we do this in general).
-
-However, a very trivial workaround is to make the `bundle` directory `chmod
+`umoci`'s defence against this attack is to make the `bundle` directory `chmod
 go-rwx` which will ensure that unprivileged users won't be able to resolve the
-dangerous inode (note that bind-mounts can circumvent this, but a user cannot
-create a bind-mount without being able to resolve the path).
+dangerous inode or setuid binary (note that bind-mounts can circumvent this,
+but a user cannot create a bind-mount without being able to resolve the path).
 
 ### Compression Bomb Attacks ###
 

oci/layer/unpack.go

@@ -96,6 +96,13 @@ func UnpackManifest(ctx context.Context, engine cas.Engine, bundle string, manif
 	if err := os.MkdirAll(bundle, 0755); err != nil {
 		return errors.Wrap(err, "mkdir bundle")
 	}
+	// We change the mode of the bundle directory to 0700. A user can easily
+	// change this after-the-fact, but we do this explicitly to avoid cases
+	// where an unprivileged user could recurse into an otherwise unsafe image
+	// (giving them potential root access through setuid binaries for example).
+	if err := os.Chmod(bundle, 0700); err != nil {
+		return errors.Wrap(err, "chmod bundle 0700")
+	}
 
 	configPath := filepath.Join(bundle, "config.json")
 	rootfsPath := filepath.Join(bundle, RootfsName)