Add --group-add keep-groups: suplimentary groups into container

Currently we have rootless users who want to leak their groups access
into containers, but this group access is only able to be pushed in by
a hard to find OCI Runtime annotation.  This PR makes this option a lot
more visable and hides the complexity within the podman client.

This option is only really needed for local rootless users. It makes
no sense for remote clients, and probably makes little sense for
rootfull containers.

Signed-off-by: Daniel J Walsh <dwalsh@redhat.com>

7 files changed, 133 insertions, 25 deletions

diff --git a/cmd/podman/common/create.go b/cmd/podman/common/create.go
index da391d30d..d496ae308 100644
--- a/cmd/podman/common/create.go
+++ b/cmd/podman/common/create.go
@@ -277,7 +277,7 @@ func DefineCreateFlags(cmd *cobra.Command, cf *ContainerCLIOpts) {
 	createFlags.StringSliceVar(
 		&cf.GroupAdd,
 		groupAddFlagName, []string{},
-		"Add additional groups to join",
+		"Add additional groups to the primary container process. 'keep-groups' allows container processes to use suplementary groups.",
 	)
 	_ = cmd.RegisterFlagCompletionFunc(groupAddFlagName, completion.AutocompleteNone)
 
diff --git a/cmd/podman/containers/create.go b/cmd/podman/containers/create.go
index 507e9c221..3f495e19b 100644
--- a/cmd/podman/containers/create.go
+++ b/cmd/podman/containers/create.go
@@ -193,6 +193,25 @@ func createInit(c *cobra.Command) error {
 		val := c.Flag("entrypoint").Value.String()
 		cliVals.Entrypoint = &val
 	}
+
+	if c.Flags().Changed("group-add") {
+		groups := []string{}
+		for _, g := range cliVals.GroupAdd {
+			if g == "keep-groups" {
+				if len(cliVals.GroupAdd) > 1 {
+					return errors.New("the '--group-add keep-groups' option is not allowed with any other --group-add options")
+				}
+				if registry.IsRemote() {
+					return errors.New("the '--group-add keep-groups' option is not supported in remote mode")
+				}
+				cliVals.Annotation = append(cliVals.Annotation, "run.oci.keep_original_groups=1")
+			} else {
+				groups = append(groups, g)
+			}
+		}
+		cliVals.GroupAdd = groups
+	}
+
 	if c.Flags().Changed("pids-limit") {
 		val := c.Flag("pids-limit").Value.String()
 		pidsLimit, err := strconv.ParseInt(val, 10, 32)
diff --git a/docs/source/markdown/podman-build.1.md b/docs/source/markdown/podman-build.1.md
index 876bfe412..791e2d907 100644
--- a/docs/source/markdown/podman-build.1.md
+++ b/docs/source/markdown/podman-build.1.md
@@ -688,7 +688,7 @@ Set the architecture variant of the image to be pulled.
    bind mounts `/HOST-DIR` in the host to `/CONTAINER-DIR` in the Podman
    container. (This option is not available with the remote Podman client)
 
-   The `OPTIONS` are a comma delimited list and can be: <sup>[[1]](#Footnote1)</sup>
+   The `OPTIONS` are a comma-separated list and can be: <sup>[[1]](#Footnote1)</sup>
 
    * [rw|ro]
    * [z|Z|O]
diff --git a/docs/source/markdown/podman-create.1.md b/docs/source/markdown/podman-create.1.md
index 059a09ae1..1ea9d1ea6 100644
--- a/docs/source/markdown/podman-create.1.md
+++ b/docs/source/markdown/podman-create.1.md
@@ -261,8 +261,8 @@ Note: if _host_device_ is a symbolic link then it will be resolved first.
 The container will only store the major and minor numbers of the host device.
 
 Note: if the user only has access rights via a group, accessing the device
-from inside a rootless container will fail. The **crun**(1) runtime offers a
-workaround for this by adding the option **\-\-annotation run.oci.keep_original_groups=1**.
+from inside a rootless container will fail. Use the `--group-add keep-groups`
+flag to pass the user's supplementary group access into the container.
 
 Podman may load kernel modules required for using the specified
 device. The devices that podman will load modules when necessary are:
@@ -361,9 +361,17 @@ GID map for the user namespace. Using this flag will run the container with user
 
 The following example maps uids 0-2000 in the container to the uids 30000-31999 on the host and gids 0-2000 in the container to the gids 30000-31999 on the host. `--gidmap=0:30000:2000`
 
-#### **\-\-group-add**=*group*
+#### **\-\-group-add**=*group|keep-groups*
 
-Add additional groups to run as
+Add additional groups to assign to primary user running within the container process.
+
+- `keep-groups` is a special flag that tells Podman to keep the supplementary group access.
+
+Allows container to use the user's supplementary group access. If file systems or
+devices are only accessible by the rootless user's group, this flag tells the OCI
+runtime to pass the group access into the container. Currently only available
+with the `crun` OCI runtime. Note: `keep-groups` is exclusive, you cannot add any other groups
+with this flag. (Not available for remote commands)
 
 #### **\-\-health-cmd**=*"command"* | *'["command", "arg1", ...]'*
 
@@ -634,7 +642,7 @@ Valid _mode_ values are:
 - **none**: no networking;
 - **container:**_id_: reuse another container's network stack;
 - **host**: use the Podman host network stack. Note: the host mode gives the container full access to local system services such as D-bus and is therefore considered insecure;
-- _network-id_: connect to a user-defined network, multiple networks should be comma separated;
+- _network-id_: connect to a user-defined network, multiple networks should be comma-separated;
 - **ns:**_path_: path to a network namespace to join;
 - **private**: create a new namespace for the container (default)
 - **slirp4netns[:OPTIONS,...]**: use **slirp4netns**(1) to create a user network stack. This is the default for rootless containers. It is possible to specify these additional options:
@@ -861,6 +869,8 @@ Security Options
 - `label=filetype:TYPE` : Set the label file type for the container files
 - `label=disable`       : Turn off label separation for the container
 
+Note: Labeling can be disabled for all containers by setting label=false in the **containers.conf** (`/etc/containers/containers.conf` or `$HOME/.config/containers/containers.conf`) file.
+
 - `mask=/path/1:/path/2` : The paths to mask separated by a colon. A masked path
   cannot be accessed inside the container.
 
@@ -869,13 +879,13 @@ Security Options
 - `seccomp=unconfined` : Turn off seccomp confinement for the container
 - `seccomp=profile.json` :  White listed syscalls seccomp Json file to be used as a seccomp filter
 
+- `proc-opts=OPTIONS` : Comma-separated list of options to use for the /proc mount. More details for the
+  possible mount options are specified in the **proc(5)** man page.
+
 - `unmask=ALL or /path/1:/path/2` : Paths to unmask separated by a colon. If set to **ALL**, it will
   unmask all the paths that are masked or made read only by default.
   The default masked paths are **/proc/acpi, /proc/kcore, /proc/keys, /proc/latency_stats, /proc/sched_debug, /proc/scsi, /proc/timer_list, /proc/timer_stats, /sys/firmware, and /sys/fs/selinux.**  The default paths that are read only are **/proc/asound, /proc/bus, /proc/fs, /proc/irq, /proc/sys, /proc/sysrq-trigger, /sys/fs/cgroup**.
 
-- `proc-opts=OPTIONS` : Comma separated list of options to use for the /proc mount. More details for the
-  possible mount options are specified at **proc(5)** man page.
-
 Note: Labeling can be disabled for all containers by setting label=false in the **containers.conf** (`/etc/containers/containers.conf` or `$HOME/.config/containers/containers.conf`) file.
 
 #### **\-\-shm-size**=*size*
@@ -1093,9 +1103,9 @@ Create a bind mount. If you specify, ` -v /HOST-DIR:/CONTAINER-DIR`, Podman
 bind mounts `/HOST-DIR` in the host to `/CONTAINER-DIR` in the Podman
 container. Similarly, `-v SOURCE-VOLUME:/CONTAINER-DIR` will mount the volume
 in the host to the container. If no such named volume exists, Podman will
-create one. The `OPTIONS` are a comma delimited list and can be: <sup>[[1]](#Footnote1)</sup>  (Note when using the remote client, the volumes will be mounted from the remote server, not necessarly the client machine.)
+create one. The `OPTIONS` are a comma-separated list and can be: <sup>[[1]](#Footnote1)</sup>  (Note when using the remote client, the volumes will be mounted from the remote server, not necessarly the client machine.)
 
-The _options_ is a comma delimited list and can be:
+The _options_ is a comma-separated list and can be:
 
 * **rw**|**ro**
 * **z**|**Z**
@@ -1185,7 +1195,7 @@ host into the container to allow speeding up builds.
 Content mounted into the container is labeled with the private label.
        On SELinux systems, labels in the source directory must be readable
 by the container label. Usually containers can read/execute `container_share_t`
-and can read/write `container_file_t`. If you can not change the labels on a
+and can read/write `container_file_t`. If you cannot change the labels on a
 source volume, SELinux container separation must be disabled for the container
 to work.
      - The source directory mounted into the container with an overlay mount
@@ -1245,10 +1255,14 @@ will convert /foo into a `shared` mount point. Alternatively one can directly
 change propagation properties of source mount. Say `/` is source mount for
 `/foo`, then use `mount --make-shared /` to convert `/` into a `shared` mount.
 
+Note: if the user only has access rights via a group, accessing the volume
+from inside a rootless container will fail. Use the `--group-add keep-groups`
+flag to pass the user's supplementary group access into the container.
+
 #### **\-\-volumes-from**[=*CONTAINER*[:*OPTIONS*]]
 
 Mount volumes from the specified container(s). Used to share volumes between
-containers. The *options* is a comma delimited list with the following available elements:
+containers. The *options* is a comma-separated list with the following available elements:
 
 * **rw**|**ro**
 * **z**
@@ -1351,6 +1365,11 @@ $ podman create --name container1 -t -i fedora bash
 $ podman create --name container2 -t -i fedora bash
 $ podman create --name container3 --requires container1,container2 -t -i fedora bash
 $ podman start --attach container3
+
+### Configure keep supplemental groups for access to volume
+
+```
+$ podman create -v /var/lib/design:/var/lib/design --group-add keep-groups ubi8
 ```
 
 ### Rootless Containers
diff --git a/docs/source/markdown/podman-pod-create.1.md b/docs/source/markdown/podman-pod-create.1.md
index 9ecde1ca3..6f3d7f1ca 100644
--- a/docs/source/markdown/podman-pod-create.1.md
+++ b/docs/source/markdown/podman-pod-create.1.md
@@ -125,7 +125,7 @@ If another pod with the same name already exists, replace and remove it.  The de
 
 #### **\-\-share**=*namespace*
 
-A comma delimited list of kernel namespaces to share. If none or "" is specified, no namespaces will be shared. The namespaces to choose from are ipc, net, pid, uts.
+A comma-separated list of kernel namespaces to share. If none or "" is specified, no namespaces will be shared. The namespaces to choose from are ipc, net, pid, uts.
 
 The operator can identify a pod in three ways:
 UUID long identifier (“f78375b1c487e03c9438c729345e54db9d20cfa2ac1fc3494b6eb60872e74778”)
diff --git a/docs/source/markdown/podman-run.1.md b/docs/source/markdown/podman-run.1.md
index b607a65ff..3a2651f98 100644
--- a/docs/source/markdown/podman-run.1.md
+++ b/docs/source/markdown/podman-run.1.md
@@ -299,8 +299,8 @@ Note: if _host_device_ is a symbolic link then it will be resolved first.
 The container will only store the major and minor numbers of the host device.
 
 Note: if the user only has access rights via a group, accessing the device
-from inside a rootless container will fail. The **crun**(1) runtime offers a
-workaround for this by adding the option **\-\-annotation run.oci.keep_original_groups=1**.
+from inside a rootless container will fail. Use the `--group-add keep-groups`
+flag to pass the user's supplementary group access into the container.
 
 Podman may load kernel modules required for using the specified
 device. The devices that Podman will load modules when necessary are:
@@ -405,9 +405,17 @@ Meaning **groupname** is initially mapped to gid **100000** which is referenced
 above: The group **groupname** is mapped to group **100000** of the initial namespace then the
 **30000**st id of this namespace (which is gid 130000 in this namespace) is mapped to container namespace group id **0**. (groupname -> 100000 / 30000 -> 0)
 
-#### **\-\-group-add**=*group*
+#### **\-\-group-add**=*group|keep-groups*
 
-Add additional groups to run as
+Add additional groups to assign to primary user running within the container process.
+
+- `keep-groups` is a special flag that tells Podman to keep the supplementary group access.
+
+Allows container to use the user's supplementary group access. If file systems or
+devices are only accessible by the rootless user's group, this flag tells the OCI
+runtime to pass the group access into the container. Currently only available
+with the `crun` OCI runtime. Note: `keep-groups` is exclusive, you cannot add any other groups
+with this flag. (Not available for remote commands)
 
 #### **\-\-health-cmd**=*"command"* | *'["command", "arg1", ...]'*
 
@@ -670,7 +678,7 @@ Valid _mode_ values are:
 - **none**: no networking;
 - **container:**_id_: reuse another container's network stack;
 - **host**: use the Podman host network stack. Note: the host mode gives the container full access to local system services such as D-bus and is therefore considered insecure;
-- _network-id_: connect to a user-defined network, multiple networks should be comma separated;
+- _network-id_: connect to a user-defined network, multiple networks should be comma-separated;
 - **ns:**_path_: path to a network namespace to join;
 - **private**: create a new namespace for the container (default)
 - **slirp4netns[:OPTIONS,...]**: use **slirp4netns**(1) to create a user network stack. This is the default for rootless containers. It is possible to specify these additional options:
@@ -905,19 +913,27 @@ Security Options
 
 - **apparmor=unconfined** : Turn off apparmor confinement for the container
 - **apparmor**=_your-profile_ : Set the apparmor confinement profile for the container
+
 - **label=user:**_USER_: Set the label user for the container processes
 - **label=role:**_ROLE_: Set the label role for the container processes
 - **label=type:**_TYPE_: Set the label process type for the container processes
 - **label=level:**_LEVEL_: Set the label level for the container processes
 - **label=filetype:**TYPE_: Set the label file type for the container files
 - **label=disable**: Turn off label separation for the container
+
+Note: Labeling can be disabled for all containers by setting label=false in the **containers.conf** (`/etc/containers/containers.conf` or `$HOME/.config/containers/containers.conf`) file.
+
 - **mask**=_/path/1:/path/2_: The paths to mask separated by a colon. A masked path
   cannot be accessed inside the container.
+
 - **no-new-privileges**: Disable container processes from gaining additional privileges
+
 - **seccomp=unconfined**: Turn off seccomp confinement for the container
 - **seccomp**=_profile.json_: Allowed syscall list seccomp JSON file to be used as a seccomp filter
-- **proc-opts**=_OPTIONS_ : Comma separated list of options to use for the /proc mount. More details
-  for the possible mount options are specified at **proc(5)** man page.
+
+- **proc-opts**=_OPTIONS_ : Comma-separated list of options to use for the /proc mount. More details
+  for the possible mount options are specified in the **proc(5)** man page.
+
 - **unmask**=_ALL_ or _/path/1:/path/2_: Paths to unmask separated by a colon. If set to **ALL**, it will
   unmask all the paths that are masked or made read only by default.
   The default masked paths are **/proc/acpi, /proc/kcore, /proc/keys, /proc/latency_stats, /proc/sched_debug, /proc/scsi, /proc/timer_list, /proc/timer_stats, /sys/firmware, and /sys/fs/selinux.**.  The default paths that are read only are **/proc/asound**, **/proc/bus**, **/proc/fs**, **/proc/irq**, **/proc/sys**, **/proc/sysrq-trigger**, **/sys/fs/cgroup**.
@@ -1164,7 +1180,7 @@ container. Similarly, _SOURCE-VOLUME_:_/CONTAINER-DIR_ will mount the volume
 in the host to the container. If no such named volume exists, Podman will
 create one. (Note when using the remote client, the volumes will be mounted from the remote server, not necessarly the client machine.)
 
-The _options_ is a comma delimited list and can be: <sup>[[1]](#Footnote1)</sup>
+The _options_ is a comma-separated list and can be: <sup>[[1]](#Footnote1)</sup>
 
 * **rw**|**ro**
 * **z**|**Z**
@@ -1252,7 +1268,7 @@ host into the container to allow speeding up builds.
 Content mounted into the container is labeled with the private label.
        On SELinux systems, labels in the source directory must be readable
 by the container label. Usually containers can read/execute `container_share_t`
-and can read/write `container_file_t`. If you can not change the labels on a
+and can read/write `container_file_t`. If you cannot change the labels on a
 source volume, SELinux container separation must be disabled for the container
 to work.
      - The source directory mounted into the container with an overlay mount
@@ -1314,10 +1330,14 @@ will convert /foo into a shared mount point. Alternatively, one can directly
 change propagation properties of source mount. Say, if _/_ is source mount for
 _/foo_, then use **mount --make-shared /** to convert _/_ into a shared mount.
 
+Note: if the user only has access rights via a group, accessing the volume
+from inside a rootless container will fail. Use the `--group-add keep-groups`
+flag to pass the user's supplementary group access into the container.
+
 #### **\-\-volumes-from**[=*CONTAINER*[:*OPTIONS*]]
 
 Mount volumes from the specified container(s). Used to share volumes between
-containers. The *options* is a comma delimited list with the following available elements:
+containers. The *options* is a comma-separated list with the following available elements:
 
 * **rw**|**ro**
 * **z**
@@ -1699,6 +1719,11 @@ Multiple containers can be required.
 $ podman create --name container1 -t -i fedora bash
 $ podman create --name container2 -t -i fedora bash
 $ podman run --name container3 --requires container1,container2 -t -i fedora bash
+
+### Configure keep supplemental groups for access to volume
+
+```
+$ podman run -v /var/lib/design:/var/lib/design --group-add keep-groups ubi8
 ```
 
 ### Rootless Containers
diff --git a/test/system/170-run-userns.bats b/test/system/170-run-userns.bats
new file mode 100644
index 000000000..2dc5b078f
--- /dev/null
+++ b/test/system/170-run-userns.bats
@@ -0,0 +1,45 @@
+#!/usr/bin/env bats   -*- bats -*-
+# shellcheck disable=SC2096
+#
+# Tests for podman build
+#
+
+load helpers
+
+@test "podman --group-add keep-groups while in a userns" {
+    skip_if_rootless "choot is not allowed in rootless mode"
+    skip_if_remote "--group-add keep-groups not supported in remote mode"
+    run chroot --groups 1234 / ${PODMAN} run --uidmap 0:200000:5000 --group-add keep-groups $IMAGE id
+    is "$output" ".*65534(nobody)" "Check group leaked into user namespace"
+}
+
+@test "podman --group-add keep-groups while not in a userns" {
+    skip_if_rootless "choot is not allowed in rootless mode"
+    skip_if_remote "--group-add keep-groups not supported in remote mode"
+    run chroot --groups 1234,5678 / ${PODMAN} run --group-add keep-groups $IMAGE id
+    is "$output" ".*1234" "Check group leaked into container"
+}
+
+@test "podman --group-add without keep-groups while in a userns" {
+    skip_if_rootless "choot is not allowed in rootless mode"
+    skip_if_remote "--group-add keep-groups not supported in remote mode"
+    run chroot --groups 1234,5678 / ${PODMAN} run --uidmap 0:200000:5000 --group-add 457 $IMAGE id
+    is "$output" ".*457" "Check group leaked into container"
+}
+
+@test "podman --remote --group-add keep-groups " {
+    if is_remote; then
+        run_podman 125 run --group-add keep-groups $IMAGE id
+        is "$output" ".*not supported in remote mode" "Remote check --group-add keep-groups"
+    fi
+}
+
+@test "podman --group-add without keep-groups " {
+    run_podman run --group-add 457 $IMAGE id
+    is "$output" ".*457" "Check group leaked into container"
+}
+
+@test "podman --group-add keep-groups plus added groups " {
+    run_podman 125 run --group-add keep-groups --group-add 457 $IMAGE id
+    is "$output" ".*the '--group-add keep-groups' option is not allowed with any other --group-add options" "Check group leaked into container"
+}