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 <rothberg@redhat.com>
  • Loading branch information
vrothberg authored and mheon committed Nov 12, 2021
1 parent c8c5d08 commit 1348285
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 @@ -479,9 +479,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")
}

c.config.IDMappings.UIDMap = containerInfo.UIDMap
Expand Down
9 changes: 0 additions & 9 deletions libpod/runtime_ctr.go
Original file line number Diff line number Diff line change
Expand Up @@ -322,15 +322,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 1348285

Please sign in to comment.