Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add --chown flag support for ADD/COPY commands for Windows #35521

Merged
merged 1 commit into from Aug 17, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
30 changes: 15 additions & 15 deletions builder/dockerfile/builder.go
Expand Up @@ -56,21 +56,21 @@ type SessionGetter interface {

// BuildManager is shared across all Builder objects
type BuildManager struct {
idMappings *idtools.IDMappings
backend builder.Backend
pathCache pathCache // TODO: make this persistent
sg SessionGetter
fsCache *fscache.FSCache
idMapping *idtools.IdentityMapping
backend builder.Backend
pathCache pathCache // TODO: make this persistent
sg SessionGetter
fsCache *fscache.FSCache
}

// NewBuildManager creates a BuildManager
func NewBuildManager(b builder.Backend, sg SessionGetter, fsCache *fscache.FSCache, idMappings *idtools.IDMappings) (*BuildManager, error) {
func NewBuildManager(b builder.Backend, sg SessionGetter, fsCache *fscache.FSCache, identityMapping *idtools.IdentityMapping) (*BuildManager, error) {
bm := &BuildManager{
backend: b,
pathCache: &syncmap.Map{},
sg: sg,
idMappings: idMappings,
fsCache: fsCache,
backend: b,
pathCache: &syncmap.Map{},
sg: sg,
idMapping: identityMapping,
fsCache: fsCache,
}
if err := fsCache.RegisterTransport(remotecontext.ClientSessionRemote, NewClientSessionTransport()); err != nil {
return nil, err
Expand Down Expand Up @@ -111,7 +111,7 @@ func (bm *BuildManager) Build(ctx context.Context, config backend.BuildConfig) (
ProgressWriter: config.ProgressWriter,
Backend: bm.backend,
PathCache: bm.pathCache,
IDMappings: bm.idMappings,
IDMapping: bm.idMapping,
}
b, err := newBuilder(ctx, builderOptions)
if err != nil {
Expand Down Expand Up @@ -159,7 +159,7 @@ type builderOptions struct {
Backend builder.Backend
ProgressWriter backend.ProgressWriter
PathCache pathCache
IDMappings *idtools.IDMappings
IDMapping *idtools.IdentityMapping
}

// Builder is a Dockerfile builder
Expand All @@ -175,7 +175,7 @@ type Builder struct {
docker builder.Backend
clientCtx context.Context

idMappings *idtools.IDMappings
idMapping *idtools.IdentityMapping
disableCommit bool
imageSources *imageSources
pathCache pathCache
Expand All @@ -199,7 +199,7 @@ func newBuilder(clientCtx context.Context, options builderOptions) (*Builder, er
Aux: options.ProgressWriter.AuxFormatter,
Output: options.ProgressWriter.Output,
docker: options.Backend,
idMappings: options.IDMappings,
idMapping: options.IDMapping,
imageSources: newImageSources(clientCtx, options),
pathCache: options.PathCache,
imageProber: newImageProber(options.Backend, config.CacheFrom, config.NoCache),
Expand Down
16 changes: 8 additions & 8 deletions builder/dockerfile/copy.go
Expand Up @@ -451,7 +451,7 @@ func downloadSource(output io.Writer, stdout io.Writer, srcURL string) (remote b

type copyFileOptions struct {
decompress bool
chownPair idtools.IDPair
identity idtools.Identity
archiver Archiver
}

Expand Down Expand Up @@ -481,7 +481,7 @@ func performCopyForInfo(dest copyInfo, source copyInfo, options copyFileOptions)
return errors.Wrapf(err, "source path not found")
}
if src.IsDir() {
return copyDirectory(archiver, srcEndpoint, destEndpoint, options.chownPair)
return copyDirectory(archiver, srcEndpoint, destEndpoint, options.identity)
}
if options.decompress && isArchivePath(source.root, srcPath) && !source.noDecompress {
return archiver.UntarPath(srcPath, destPath)
Expand All @@ -499,7 +499,7 @@ func performCopyForInfo(dest copyInfo, source copyInfo, options copyFileOptions)
destPath = dest.root.Join(destPath, source.root.Base(source.path))
destEndpoint = &copyEndpoint{driver: dest.root, path: destPath}
}
return copyFile(archiver, srcEndpoint, destEndpoint, options.chownPair)
return copyFile(archiver, srcEndpoint, destEndpoint, options.identity)
}

func isArchivePath(driver containerfs.ContainerFS, path string) bool {
Expand All @@ -517,7 +517,7 @@ func isArchivePath(driver containerfs.ContainerFS, path string) bool {
return err == nil
}

func copyDirectory(archiver Archiver, source, dest *copyEndpoint, chownPair idtools.IDPair) error {
func copyDirectory(archiver Archiver, source, dest *copyEndpoint, identity idtools.Identity) error {
destExists, err := isExistingDirectory(dest)
if err != nil {
return errors.Wrapf(err, "failed to query destination path")
Expand All @@ -527,17 +527,17 @@ func copyDirectory(archiver Archiver, source, dest *copyEndpoint, chownPair idto
return errors.Wrapf(err, "failed to copy directory")
}
// TODO: @gupta-ak. Investigate how LCOW permission mappings will work.
return fixPermissions(source.path, dest.path, chownPair, !destExists)
return fixPermissions(source.path, dest.path, identity, !destExists)
}

func copyFile(archiver Archiver, source, dest *copyEndpoint, chownPair idtools.IDPair) error {
func copyFile(archiver Archiver, source, dest *copyEndpoint, identity idtools.Identity) error {
if runtime.GOOS == "windows" && dest.driver.OS() == "linux" {
// LCOW
if err := dest.driver.MkdirAll(dest.driver.Dir(dest.path), 0755); err != nil {
return errors.Wrapf(err, "failed to create new directory")
}
} else {
if err := idtools.MkdirAllAndChownNew(filepath.Dir(dest.path), 0755, chownPair); err != nil {
if err := idtools.MkdirAllAndChownNew(filepath.Dir(dest.path), 0755, identity); err != nil {
// Normal containers
return errors.Wrapf(err, "failed to create new directory")
}
Expand All @@ -547,7 +547,7 @@ func copyFile(archiver Archiver, source, dest *copyEndpoint, chownPair idtools.I
return errors.Wrapf(err, "failed to copy file")
}
// TODO: @gupta-ak. Investigate how LCOW permission mappings will work.
return fixPermissions(source.path, dest.path, chownPair, false)
return fixPermissions(source.path, dest.path, identity, false)
}

func endsInSlash(driver containerfs.Driver, path string) bool {
Expand Down
4 changes: 2 additions & 2 deletions builder/dockerfile/copy_unix.go
Expand Up @@ -10,7 +10,7 @@ import (
"github.com/docker/docker/pkg/idtools"
)

func fixPermissions(source, destination string, rootIDs idtools.IDPair, overrideSkip bool) error {
func fixPermissions(source, destination string, identity idtools.Identity, overrideSkip bool) error {
var (
skipChownRoot bool
err error
Expand Down Expand Up @@ -39,7 +39,7 @@ func fixPermissions(source, destination string, rootIDs idtools.IDPair, override
}

fullpath = filepath.Join(destination, cleaned)
return os.Lchown(fullpath, rootIDs.UID, rootIDs.GID)
return os.Lchown(fullpath, identity.UID, identity.GID)
})
}

Expand Down
74 changes: 70 additions & 4 deletions builder/dockerfile/copy_windows.go
@@ -1,21 +1,87 @@
package dockerfile // import "github.com/docker/docker/builder/dockerfile"

import (
"errors"
"fmt"
"os"
"path/filepath"
"strings"

"github.com/Microsoft/go-winio"
"github.com/docker/docker/pkg/idtools"
"github.com/docker/docker/pkg/reexec"
"github.com/docker/docker/pkg/system"
"github.com/pkg/errors"
"golang.org/x/sys/windows"
)

var pathBlacklist = map[string]bool{
"c:\\": true,
"c:\\windows": true,
}

func fixPermissions(source, destination string, rootIDs idtools.IDPair, overrideSkip bool) error {
// chown is not supported on Windows
return nil
func init() {
reexec.Register("windows-fix-permissions", fixPermissionsReexec)
}

func fixPermissions(source, destination string, identity idtools.Identity, _ bool) error {
if identity.SID == "" {
return nil
}

cmd := reexec.Command("windows-fix-permissions", source, destination, identity.SID)
output, err := cmd.CombinedOutput()

return errors.Wrapf(err, "failed to exec windows-fix-permissions: %s", output)
}

func fixPermissionsReexec() {
err := fixPermissionsWindows(os.Args[1], os.Args[2], os.Args[3])
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: a more idiomatic way to write these is to scope the err to the if;

if err := fixPermissionsWindows(os.Args[1], os.Args[2], os.Args[3]); err != nil {
 ....
}

if err != nil {
fmt.Fprint(os.Stderr, err)
os.Exit(1)
}
}

func fixPermissionsWindows(source, destination, SID string) error {

privileges := []string{winio.SeRestorePrivilege, system.SeTakeOwnershipPrivilege}

err := winio.EnableProcessPrivileges(privileges)
if err != nil {
return err
}

defer winio.DisableProcessPrivileges(privileges)

sid, err := windows.StringToSid(SID)
if err != nil {
return err
}

// Owners on *nix have read/write/delete/read control and write DAC.
// Add an ACE that grants this to the user/group specified with the
// chown option. Currently Windows is not honoring the owner change,
// however, they are aware of this and it should be fixed at some
// point.

sddlString := system.SddlAdministratorsLocalSystem
sddlString += "(A;OICI;GRGWGXRCWDSD;;;" + SID + ")"

securityDescriptor, err := winio.SddlToSecurityDescriptor(sddlString)
if err != nil {
return err
}

var daclPresent uint32
var daclDefaulted uint32
var dacl *byte
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

style nit; no need to change; generally we group these, i.e.;

var (
    daclPresent uint32
    daclDefaulted uint32
    dacl *byte
)


err = system.GetSecurityDescriptorDacl(&securityDescriptor[0], &daclPresent, &dacl, &daclDefaulted)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same here (not a blocker)

if err := system.GetSecurityDescriptorDacl(&securityDescriptor[0], &daclPresent, &dacl, &daclDefaulted); err != nil {
    return err
}

if err != nil {
return err
}

return system.SetNamedSecurityInfo(windows.StringToUTF16Ptr(destination), system.SE_FILE_OBJECT, system.OWNER_SECURITY_INFORMATION|system.DACL_SECURITY_INFORMATION, sid, nil, dacl, nil)
}

func validateCopySourcePath(imageSource *imageMount, origPath, platform string) error {
Expand Down
24 changes: 14 additions & 10 deletions builder/dockerfile/internals.go
Expand Up @@ -37,7 +37,7 @@ type Archiver interface {
UntarPath(src, dst string) error
CopyWithTar(src, dst string) error
CopyFileWithTar(src, dst string) error
IDMappings() *idtools.IDMappings
IdentityMapping() *idtools.IdentityMapping
}

// The builder will use the following interfaces if the container fs implements
Expand Down Expand Up @@ -68,11 +68,11 @@ func tarFunc(i interface{}) containerfs.TarFunc {
func (b *Builder) getArchiver(src, dst containerfs.Driver) Archiver {
t, u := tarFunc(src), untarFunc(dst)
return &containerfs.Archiver{
SrcDriver: src,
DstDriver: dst,
Tar: t,
Untar: u,
IDMappingsVar: b.idMappings,
SrcDriver: src,
DstDriver: dst,
Tar: t,
Untar: u,
IDMapping: b.idMapping,
}
}

Expand Down Expand Up @@ -185,22 +185,26 @@ func (b *Builder) performCopy(req dispatchRequest, inst copyInstruction) error {
return err
}

chownPair := b.idMappings.RootPair()
identity := b.idMapping.RootPair()
// if a chown was requested, perform the steps to get the uid, gid
// translated (if necessary because of user namespaces), and replace
// the root pair with the chown pair for copy operations
if inst.chownStr != "" {
chownPair, err = parseChownFlag(inst.chownStr, destInfo.root.Path(), b.idMappings)
identity, err = parseChownFlag(b, state, inst.chownStr, destInfo.root.Path(), b.idMapping)
if err != nil {
return errors.Wrapf(err, "unable to convert uid/gid chown string to host mapping")
if b.options.Platform != "windows" {
return errors.Wrapf(err, "unable to convert uid/gid chown string to host mapping")
}

return errors.Wrapf(err, "unable to map container user account name to SID")
}
}

for _, info := range inst.infos {
opts := copyFileOptions{
decompress: inst.allowLocalDecompression,
archiver: b.getArchiver(info.root, destInfo.root),
chownPair: chownPair,
identity: identity,
}
if err := performCopyForInfo(destInfo, info, opts); err != nil {
return errors.Wrapf(err, "failed to copy files")
Expand Down
16 changes: 8 additions & 8 deletions builder/dockerfile/internals_linux.go
Expand Up @@ -11,11 +11,11 @@ import (
"github.com/pkg/errors"
)

func parseChownFlag(chown, ctrRootPath string, idMappings *idtools.IDMappings) (idtools.IDPair, error) {
func parseChownFlag(builder *Builder, state *dispatchState, chown, ctrRootPath string, identityMapping *idtools.IdentityMapping) (idtools.Identity, error) {
var userStr, grpStr string
parts := strings.Split(chown, ":")
if len(parts) > 2 {
return idtools.IDPair{}, errors.New("invalid chown string format: " + chown)
return idtools.Identity{}, errors.New("invalid chown string format: " + chown)
}
if len(parts) == 1 {
// if no group specified, use the user spec as group as well
Expand All @@ -26,25 +26,25 @@ func parseChownFlag(chown, ctrRootPath string, idMappings *idtools.IDMappings) (

passwdPath, err := symlink.FollowSymlinkInScope(filepath.Join(ctrRootPath, "etc", "passwd"), ctrRootPath)
if err != nil {
return idtools.IDPair{}, errors.Wrapf(err, "can't resolve /etc/passwd path in container rootfs")
return idtools.Identity{}, errors.Wrapf(err, "can't resolve /etc/passwd path in container rootfs")
}
groupPath, err := symlink.FollowSymlinkInScope(filepath.Join(ctrRootPath, "etc", "group"), ctrRootPath)
if err != nil {
return idtools.IDPair{}, errors.Wrapf(err, "can't resolve /etc/group path in container rootfs")
return idtools.Identity{}, errors.Wrapf(err, "can't resolve /etc/group path in container rootfs")
}
uid, err := lookupUser(userStr, passwdPath)
if err != nil {
return idtools.IDPair{}, errors.Wrapf(err, "can't find uid for user "+userStr)
return idtools.Identity{}, errors.Wrapf(err, "can't find uid for user "+userStr)
}
gid, err := lookupGroup(grpStr, groupPath)
if err != nil {
return idtools.IDPair{}, errors.Wrapf(err, "can't find gid for group "+grpStr)
return idtools.Identity{}, errors.Wrapf(err, "can't find gid for group "+grpStr)
}

// convert as necessary because of user namespaces
chownPair, err := idMappings.ToHost(idtools.IDPair{UID: uid, GID: gid})
chownPair, err := identityMapping.ToHost(idtools.Identity{UID: uid, GID: gid})
if err != nil {
return idtools.IDPair{}, errors.Wrapf(err, "unable to convert uid/gid to host mapping")
return idtools.Identity{}, errors.Wrapf(err, "unable to convert uid/gid to host mapping")
}
return chownPair, nil
}
Expand Down