docker: validate that containers do not run as ContainerAdmin on Windows #23443
Conversation
pkazmierczak
added
backport/ent/1.8.x+ent
There was a problem hiding this comment.
The overall approach here is on the right track, @pkazmierczak
drivers/docker/driver_windows.go
Outdated
| return nil | ||
| } | ||
|
|
||
| if (user == "ContainerAdmin" || taskUser == "ContainerAdmin") && !driverConfig.Privileged { |
There was a problem hiding this comment.
The task.user will override the user from the image manifest. Ex. if you have task.user = "ContainerUser" (safe) it'll override "ContainerAdmin" (unsafe) from the image and the container will run as "ContainerUser". It looks like we have it right in the docs, but I don't think we want this to be and "or" condition.
There was a problem hiding this comment.
I fixed in f32473e, have a look if I got this correctly.
drivers/docker/driver.go
Outdated
| // validate the image user (windows only) | ||
| if err := d.validateImageUser(imageUser, task.User, driverConfig); err != nil { | ||
| return "", err | ||
| } |
There was a problem hiding this comment.
Suppose we pull the image and find that it's running as "ContainerAdmin". If I try to run the task again on the same node, the image may not have been GC'd. In that case, we'll hit the case above:
// We're going to check whether the image is already downloaded.
And then skip this validation entirely! I think we want to either copy this method into the case above, or move it into the caller so that any image will get checked.
There was a problem hiding this comment.
good catch. We validate in the caller now: 492a750
Co-authored-by: Tim Gross <[email protected]>
pkazmierczak
added
the
backport/website
Co-authored-by: Tim Gross <[email protected]>
pkazmierczak
changed the title
pkazmierczak
changed the title
pkazmierczak
added
the
backport/ent/1.7.x+ent
…ows (#23443) This enables checks for ContainerAdmin user on docker images on Windows. It's only checked if users run docker with process isolation and not hyper-v, because hyper-v provides its own, proper sandboxing. --------- Co-authored-by: Tim Gross <[email protected]>
pkazmierczak
added
the
backport/ent/1.6.x+ent
…ows (#23443) (#23450) This enables checks for ContainerAdmin user on docker images on Windows. It's only checked if users run docker with process isolation and not hyper-v, because hyper-v provides its own, proper sandboxing. --------- Co-authored-by: Piotr Kazmierczak <[email protected]> Co-authored-by: Tim Gross <[email protected]>
Following #23443, we no longer need to sandbox template rendering on Windows.
This enables checks for
ContainerAdminuser on docker images on Windows. It's only checked if users run docker with process isolation and not hyper-v, because hyper-v provides its own, proper sandboxing.Relates to: #20585
Relates to: #20034
Internal ref: https://hashicorp.atlassian.net/browse/NET-9311
This is part of a solution outlined in RFC NMD-195.