Skip to content

Commit

Permalink
pod/container create: resolve conflicts of generated names
Browse files Browse the repository at this point in the history
Address the TOCTOU when generating random names by having at most 10
attempts to assign a random name when creating a pod or container.

[NO TESTS NEEDED] since I do not know a way to force a conflict with
randomly generated names in a reasonable time frame.

Fixes: containers#11735
Signed-off-by: Valentin Rothberg <[email protected]>
  • Loading branch information
vrothberg committed Nov 8, 2021
1 parent 865653b commit 6444f24
Show file tree
Hide file tree
Showing 3 changed files with 47 additions and 26 deletions.
24 changes: 21 additions & 3 deletions libpod/container_internal.go
Original file line number Diff line number Diff line change
Expand Up @@ -496,9 +496,27 @@ func (c *Container) setupStorage(ctx context.Context) error {

c.setupStorageMapping(&options.IDMappingOptions, &c.config.IDMappings)

containerInfo, err := c.runtime.storageService.CreateContainerStorage(ctx, c.runtime.imageContext, c.config.RootfsImageName, c.config.RootfsImageID, c.config.Name, c.config.ID, options)
if err != nil {
return errors.Wrapf(err, "error creating container storage")
// Unless the user has specified a name, use a randomly generated one.
// Note that name conflicts may occur (see #11735), so we need to loop.
generateName := c.config.Name == ""
var containerInfo ContainerInfo
var containerInfoErr error
for {
if generateName {
name, err := c.runtime.generateName()
if err != nil {
return err
}
c.config.Name = name
}
containerInfo, containerInfoErr = c.runtime.storageService.CreateContainerStorage(ctx, c.runtime.imageContext, c.config.RootfsImageName, c.config.RootfsImageID, c.config.Name, c.config.ID, options)

if !generateName || errors.Cause(containerInfoErr) != storage.ErrDuplicateName {
break
}
}
if containerInfoErr != nil {
return errors.Wrapf(containerInfoErr, "error creating container storage")
}

// only reconfig IDMappings if layer was mounted from storage
Expand Down
9 changes: 0 additions & 9 deletions libpod/runtime_ctr.go
Original file line number Diff line number Diff line change
Expand Up @@ -326,15 +326,6 @@ func (r *Runtime) setupContainer(ctx context.Context, ctr *Container) (_ *Contai
}
}

if ctr.config.Name == "" {
name, err := r.generateName()
if err != nil {
return nil, err
}

ctr.config.Name = name
}

// Check CGroup parent sanity, and set it if it was not set.
// Only if we're actually configuring CGroups.
if !ctr.config.NoCgroups {
Expand Down
40 changes: 26 additions & 14 deletions libpod/runtime_pod_linux.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,18 +43,6 @@ func (r *Runtime) NewPod(ctx context.Context, p specgen.PodSpecGenerator, option
}
}

if pod.config.Name == "" {
name, err := r.generateName()
if err != nil {
return nil, err
}
pod.config.Name = name
}

if p.InfraContainerSpec != nil && p.InfraContainerSpec.Hostname == "" {
p.InfraContainerSpec.Hostname = pod.config.Name
}

// Allocate a lock for the pod
lock, err := r.lockManager.AllocateLock()
if err != nil {
Expand Down Expand Up @@ -131,9 +119,33 @@ func (r *Runtime) NewPod(ctx context.Context, p specgen.PodSpecGenerator, option
logrus.Infof("Pod has an infra container, but shares no namespaces")
}

if err := r.state.AddPod(pod); err != nil {
return nil, errors.Wrapf(err, "error adding pod to state")
// Unless the user has specified a name, use a randomly generated one.
// Note that name conflicts may occur (see #11735), so we need to loop.
generateName := pod.config.Name == ""
var addPodErr error
for {
if generateName {
name, err := r.generateName()
if err != nil {
return nil, err
}
pod.config.Name = name
}

if p.InfraContainerSpec != nil && p.InfraContainerSpec.Hostname == "" {
p.InfraContainerSpec.Hostname = pod.config.Name
}
if addPodErr = r.state.AddPod(pod); addPodErr == nil {
return pod, nil
}
if !generateName || (errors.Cause(addPodErr) != define.ErrPodExists && errors.Cause(addPodErr) != define.ErrCtrExists) {
break
}
}
if addPodErr != nil {
return nil, errors.Wrapf(addPodErr, "error adding pod to state")
}

return pod, nil
}

Expand Down

0 comments on commit 6444f24

Please sign in to comment.