Skip to content

Commit 0cccbdc

Browse files
committed
Copy ParseIDMap from idtools and extend supporting @ParentID
This topic has been discussed at length at containers#18333, with @giuseppe, @Luap99 and with the feedback from @rhatdan. The requirements were defined there and this aims to be the implementation. Motivation =========== These series of patches aim to make --uidmap and --gidmap easier to use, especially in rootless podman setups. (I will focus here on the --gidmap option, although the same applies for --uidmap.) In rootless podman, the user namespace mapping happens in two steps, through an intermediate mapping. See https://docs.podman.io/en/latest/markdown/podman-run.1.html#uidmap-container-uid-from-uid-amount for further detail, here is a summary: First the user GID is mapped to 0 (root), and all subordinate GIDs (defined at /etc/subgid, and usually >100000) are mapped starting at 1. If we want to change it further, we can use the --gidmap option, to map that intermediate mapping to the final mapping that will be seen by the container. As an example, let's say we have as main GID the group 1000, and we also belong to the additional GID 2000, that we want to make accessible inside the container. We first ask the sysadmin to subordinate the group to us, by adding "$user:2000:1" to /etc/subgid. Then we need to use --gidmap to specify that we want to map GID 2000 into some GID inside the container. And here is the first trouble: Since the --gidmap option operates on the intermediate mapping, we first need to figure out where has podman placed our GID 2000 in that intermediate mapping using: podman unshare cat /proc/self/gid_map Then, we may see that GID 2000 was mapped intermediate GID 5. So our --gidmap option should include: --gidmap 20000:5:1 This intermediate mapping may change in the future if further groups are subordinated to us (or we stop having its subordination), so we are forced to verify the mapping with `podman unshare cat /proc/self/gid_map` every time, and parse it if we want to script it. **The first usability improvement** we agreed on containers#18333 is to be able to use: --gidmap 20000:@2000:1 so podman does this lookup in the parent user namespace for us. But this is only part of the problem. We must specify a full gidmap and not only what we want: --gidmap 0:0:5 --gidmap 5:6:15000 --gidmap 20000:5:1 This is becoming complicated. We had to break the gidmap at 5, because the intermediate 5 had to be mapped to another value (20000), and then we had to keep mapping all other subordinate ids... up to close to the maximum number of subordinate ids that we have (or some reasonable value). This is hard to explain to someone who does not understand how the mappings work internally. **The second usability improvement** is to be able to use: --gidmap "+20000:@2000:1" where the plus sign (`+`) states that we want to start with an identity mapping, and break it where necessary so this mapping gets included. One final improvement related to this is the following: By default, when podman gets a --gidmap argument but not a --uidmap argument, it copies the mapping. With the new syntax this copying does not make sense. Having a GID subordinated to us does not imply that the same UID will be subordinated as well. This means, that when we wanted to use: --gidmap 0:0:5 --gidmap 5:6:15000 --gidmap 20000:5:1 We also had to include: --gidmap 0:0:5 --gidmap 5:6:15000 --gidmap 20000:5:1 --uidmap 0:0:65000 making everything even harder to understand without proper context. In this series of patches, when a "break and insert" gidmap is given (using the described `+` syntax) without a --uidmap, we assume that we want the "identity mapping" as --uidmap (0:0:65000). To preserve backwards compatibility, this different default mapping is only used when the `+` syntax is used, so users who rely on the previous behaviour don't suffer any changes. Signed-off-by: Sergio Oller <[email protected]>
1 parent e7dc507 commit 0cccbdc

File tree

1 file changed

+86
-0
lines changed

1 file changed

+86
-0
lines changed

pkg/util/utils.go

+86
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import (
44
"errors"
55
"fmt"
66
"math"
7+
"math/bits"
78
"os"
89
"os/user"
910
"path/filepath"
@@ -30,6 +31,7 @@ import (
3031
stypes "github.com/containers/storage/types"
3132
securejoin "github.com/cyphar/filepath-securejoin"
3233
"github.com/opencontainers/runtime-spec/specs-go"
34+
ruser "github.com/opencontainers/runc/libcontainer/user"
3335
"github.com/sirupsen/logrus"
3436
"golang.org/x/term"
3537
)
@@ -292,6 +294,90 @@ func GetNoMapMapping() (*stypes.IDMappingOptions, int, int, error) {
292294
return &options, 0, 0, nil
293295
}
294296

297+
298+
func mapIDwithMapping(id uint64, mapping []ruser.IDMap, mapSetting string) (mappedid uint64, err error) {
299+
for _, v := range mapping {
300+
if v.Count == 0 {
301+
continue
302+
}
303+
if id >= uint64(v.ParentID) && id < uint64(v.ParentID + v.Count) {
304+
offset := id - uint64(v.ParentID)
305+
return uint64(v.ID) + offset, nil;
306+
}
307+
}
308+
return uint64(0), fmt.Errorf("Parent ID %s %d is not mapped/delegated.", mapSetting, id)
309+
}
310+
311+
// Extension of idTools.parseTriple that parses idmap triples from string.
312+
// This extension covers the "@" syntax: The "101001:@1001:1" mapping
313+
// means "take the 1001 id from the parent namespace and map it to 101001"
314+
// See https://github.com/containers/podman/issues/18333 for details
315+
func parseTriple(spec []string, parentMapping []ruser.IDMap, mapSetting string) (container, host, size uint32, err error) {
316+
cid, err := strconv.ParseUint(spec[0], 10, 32)
317+
if err != nil {
318+
return 0, 0, 0, fmt.Errorf("parsing id map value %q: %w", spec[0], err)
319+
}
320+
hid := uint64(0)
321+
if spec[1][0] != '@' {
322+
hid, err = strconv.ParseUint(spec[1], 10, 32)
323+
} else {
324+
hparentid, err := strconv.ParseUint(spec[1][1:], 10, 32)
325+
if err != nil {
326+
return 0, 0, 0, fmt.Errorf("parsing id map value %q: %w", spec[1], err)
327+
}
328+
hid, err = mapIDwithMapping(hparentid, parentMapping, mapSetting)
329+
if err != nil {
330+
return 0, 0, 0, err
331+
}
332+
}
333+
if err != nil {
334+
return 0, 0, 0, fmt.Errorf("parsing id map value %q: %w", spec[1], err)
335+
}
336+
sz, err := strconv.ParseUint(spec[2], 10, 32)
337+
if err != nil {
338+
return 0, 0, 0, fmt.Errorf("parsing id map value %q: %w", spec[2], err)
339+
}
340+
return uint32(cid), uint32(hid), uint32(sz), nil
341+
}
342+
343+
// Extension of idTools.ParseIDMap that parses idmap triples from string.
344+
// This extension covers the "@" syntax: The "101001:@1001:1" mapping
345+
// means "take the 1001 id from the parent namespace and map it to 101001"
346+
// See https://github.com/containers/podman/issues/18333 for details
347+
func ParseIDMap(mapSpec []string, mapSetting string, parentMapping []ruser.IDMap) (idmap []idtools.IDMap, err error) {
348+
stdErr := fmt.Errorf("aaa initializing ID mappings: %s setting is malformed expected [\"uint32:[@]uint32:uint32\"] : %q", mapSetting, mapSpec)
349+
for _, idMapSpec := range mapSpec {
350+
if idMapSpec == "" {
351+
continue
352+
}
353+
idSpec := strings.Split(idMapSpec, ":")
354+
if len(idSpec)%3 != 0 {
355+
return nil, stdErr
356+
}
357+
for i := range idSpec {
358+
if i%3 != 0 {
359+
continue
360+
}
361+
cid, hid, size, err := parseTriple(idSpec[i : i+3], parentMapping, mapSetting)
362+
if err != nil {
363+
return nil, err
364+
}
365+
// Avoid possible integer overflow on 32bit builds
366+
if bits.UintSize == 32 && (cid > math.MaxInt32 || hid > math.MaxInt32 || size > math.MaxInt32) {
367+
return nil, stdErr
368+
}
369+
mapping := idtools.IDMap{
370+
ContainerID: int(cid),
371+
HostID: int(hid),
372+
Size: int(size),
373+
}
374+
idmap = append(idmap, mapping)
375+
}
376+
}
377+
return idmap, nil
378+
}
379+
380+
295381
// ParseIDMapping takes idmappings and subuid and subgid maps and returns a storage mapping
296382
func ParseIDMapping(mode namespaces.UsernsMode, uidMapSlice, gidMapSlice []string, subUIDMap, subGIDMap string) (*stypes.IDMappingOptions, error) {
297383
options := stypes.IDMappingOptions{

0 commit comments

Comments
 (0)