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/COPY: create the destination directory first, chroot to it #3015

Merged
merged 5 commits into from
Mar 3, 2021
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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 23 additions & 3 deletions add.go
Original file line number Diff line number Diff line change
Expand Up @@ -342,13 +342,33 @@ func (b *Builder) Add(destination string, extract bool, options AddAndCopyOption
return errors.Wrapf(err, "error processing excludes list %v", options.Excludes)
}

// Copy each source in turn.
// Make sure that, if it's a symlink, we'll chroot to the target of the link;
// knowing that target requires that we resolve it within the chroot.
evalOptions := copier.EvalOptions{}
evaluated, err := copier.Eval(mountPoint, extractDirectory, evalOptions)
if err != nil {
return errors.Wrapf(err, "error checking on destination %v", extractDirectory)
}
extractDirectory = evaluated

// Set up ID maps.
var srcUIDMap, srcGIDMap []idtools.IDMap
if options.IDMappingOptions != nil {
srcUIDMap, srcGIDMap = convertRuntimeIDMaps(options.IDMappingOptions.UIDMap, options.IDMappingOptions.GIDMap)
}
destUIDMap, destGIDMap := convertRuntimeIDMaps(b.IDMappingOptions.UIDMap, b.IDMappingOptions.GIDMap)

// Create the target directory if it doesn't exist yet.
mkdirOptions := copier.MkdirOptions{
UIDMap: destUIDMap,
GIDMap: destGIDMap,
ChownNew: chownDirs,
}
if err := copier.Mkdir(mountPoint, extractDirectory, mkdirOptions); err != nil {
Copy link
Member

Choose a reason for hiding this comment

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

Would it be possible to move that down into the copier package? podman cp would benefit from it, also when copying from a container to the host.

Copy link
Member Author

Choose a reason for hiding this comment

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

Not without pretty drastic restructuring - in cases where the destination doesn't exist yet, and the path can include components which are symbolic links, we have to chroot to the rootfs first, then evaluate the path for symbolic links, then create the target directory, and then chroot to the target directory, and trying to stuff all of that into Put() broke just about every other unit test and several integration tests. In the end, having Add() do both an Eval() and a Mkdir() explicitly looks much easier to test.

return errors.Wrapf(err, "error ensuring target directory exists")
}

// Copy each source in turn.
for _, src := range sources {
var multiErr *multierror.Error
var getErr, closeErr, renameErr, putErr error
Expand Down Expand Up @@ -381,7 +401,7 @@ func (b *Builder) Add(destination string, extract bool, options AddAndCopyOption
ChmodFiles: nil,
IgnoreDevices: rsystem.RunningInUserNS(),
}
putErr = copier.Put(mountPoint, extractDirectory, putOptions, io.TeeReader(pipeReader, hasher))
putErr = copier.Put(extractDirectory, extractDirectory, putOptions, io.TeeReader(pipeReader, hasher))
}
hashCloser.Close()
pipeReader.Close()
Expand Down Expand Up @@ -516,7 +536,7 @@ func (b *Builder) Add(destination string, extract bool, options AddAndCopyOption
ChmodFiles: nil,
IgnoreDevices: rsystem.RunningInUserNS(),
}
putErr = copier.Put(mountPoint, extractDirectory, putOptions, io.TeeReader(pipeReader, hasher))
putErr = copier.Put(extractDirectory, extractDirectory, putOptions, io.TeeReader(pipeReader, hasher))
}
hashCloser.Close()
pipeReader.Close()
Expand Down
65 changes: 62 additions & 3 deletions copier/copier.go
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,7 @@ func isArchivePath(path string) bool {
type requestType string

const (
requestEval requestType = "EVAL"
requestStat requestType = "STAT"
requestGet requestType = "GET"
requestPut requestType = "PUT"
Expand All @@ -95,6 +96,8 @@ type request struct {

func (req *request) Excludes() []string {
switch req.Request {
case requestEval:
return nil
case requestStat:
return req.StatOptions.Excludes
case requestGet:
Expand All @@ -112,6 +115,8 @@ func (req *request) Excludes() []string {

func (req *request) UIDMap() []idtools.IDMap {
switch req.Request {
case requestEval:
return nil
case requestStat:
return nil
case requestGet:
Expand All @@ -129,6 +134,8 @@ func (req *request) UIDMap() []idtools.IDMap {

func (req *request) GIDMap() []idtools.IDMap {
switch req.Request {
case requestEval:
return nil
case requestStat:
return nil
case requestGet:
Expand All @@ -148,6 +155,7 @@ func (req *request) GIDMap() []idtools.IDMap {
type response struct {
Error string `json:",omitempty"`
Stat statResponse
Eval evalResponse
Get getResponse
Put putResponse
Mkdir mkdirResponse
Expand All @@ -158,6 +166,11 @@ type statResponse struct {
Globs []*StatsForGlob
}

// evalResponse encodes a response for a single Eval request.
type evalResponse struct {
Evaluated string
}

// StatsForGlob encode results for a single glob pattern passed to Stat().
type StatsForGlob struct {
Error string `json:",omitempty"` // error if the Glob pattern was malformed
Expand Down Expand Up @@ -192,6 +205,33 @@ type putResponse struct {
type mkdirResponse struct {
}

// EvalOptions controls parts of Eval()'s behavior.
type EvalOptions struct {
}

// Eval evaluates the directory's path, including any intermediate symbolic
// links.
// If root is specified and the current OS supports it, and the calling process
// has the necessary privileges, evaluation is performed in a chrooted context.
// If the directory is specified as an absolute path, it should either be the
// root directory or a subdirectory of the root directory. Otherwise, the
// directory is treated as a path relative to the root directory.
func Eval(root string, directory string, options EvalOptions) (string, error) {
req := request{
Request: requestEval,
Root: root,
Directory: directory,
}
resp, err := copier(nil, nil, req)
if err != nil {
return "", err
}
if resp.Error != "" {
return "", errors.New(resp.Error)
}
return resp.Eval.Evaluated, nil
}

// StatOptions controls parts of Stat()'s behavior.
type StatOptions struct {
CheckForArchives bool // check for and populate the IsArchive bit in returned values
Expand Down Expand Up @@ -243,6 +283,7 @@ type GetOptions struct {
StripXattrs bool // don't record extended attributes of items being copied. no effect on archives being extracted
KeepDirectoryNames bool // don't strip the top directory's basename from the paths of items in subdirectories
Rename map[string]string // rename items with the specified names, or under the specified names
NoDerefSymlinks bool // don't follow symlinks when globs match them
}

// Get produces an archive containing items that match the specified glob
Expand Down Expand Up @@ -557,6 +598,9 @@ func copierWithSubprocess(bulkReader io.Reader, bulkWriter io.Writer, req reques
return killAndReturn(err, "error encoding request for copier subprocess")
}
if err = decoder.Decode(&resp); err != nil {
if errors.Is(err, io.EOF) && errorBuffer.Len() > 0 {
return killAndReturn(errors.New(errorBuffer.String()), "error in copier subprocess")
}
return killAndReturn(err, "error decoding response from copier subprocess")
}
if err = encoder.Encode(&request{Request: requestQuit}); err != nil {
Expand Down Expand Up @@ -667,7 +711,7 @@ func copierMain() {
var err error
chrooted, err = chroot(req.Root)
if err != nil {
fmt.Fprintf(os.Stderr, "error changing to intended-new-root directory %q: %v", req.Root, err)
fmt.Fprintf(os.Stderr, "%v", err)
os.Exit(1)
}
}
Expand Down Expand Up @@ -762,6 +806,9 @@ func copierHandler(bulkReader io.Reader, bulkWriter io.Writer, req request) (*re
switch req.Request {
default:
return nil, nil, errors.Errorf("not an implemented request type: %q", req.Request)
case requestEval:
resp := copierHandlerEval(req)
return resp, nil, nil
case requestStat:
resp := copierHandlerStat(req, pm)
return resp, nil, nil
Expand Down Expand Up @@ -870,6 +917,17 @@ func resolvePath(root, path string, pm *fileutils.PatternMatcher) (string, error
return workingPath, nil
}

func copierHandlerEval(req request) *response {
errorResponse := func(fmtspec string, args ...interface{}) *response {
return &response{Error: fmt.Sprintf(fmtspec, args...), Eval: evalResponse{}}
}
resolvedTarget, err := resolvePath(req.Root, req.Directory, nil)
if err != nil {
return errorResponse("copier: eval: error resolving %q: %v", req.Directory, err)
}
return &response{Eval: evalResponse{Evaluated: filepath.Join(req.rootPrefix, resolvedTarget)}}
}

func copierHandlerStat(req request, pm *fileutils.PatternMatcher) *response {
errorResponse := func(fmtspec string, args ...interface{}) *response {
return &response{Error: fmt.Sprintf(fmtspec, args...), Stat: statResponse{}}
Expand Down Expand Up @@ -1024,7 +1082,7 @@ func copierHandlerGet(bulkWriter io.Writer, req request, pm *fileutils.PatternMa
// chase links. if we hit a dead end, we should just fail
followedLinks := 0
const maxFollowedLinks = 16
for info.Mode()&os.ModeType == os.ModeSymlink && followedLinks < maxFollowedLinks {
for !req.GetOptions.NoDerefSymlinks && info.Mode()&os.ModeType == os.ModeSymlink && followedLinks < maxFollowedLinks {
path, err := os.Readlink(item)
if err != nil {
continue
Expand Down Expand Up @@ -1139,7 +1197,8 @@ func handleRename(rename map[string]string, name string) string {
return path.Join(mappedPrefix, remainder)
}
if prefix[len(prefix)-1] == '/' {
if mappedPrefix, ok := rename[prefix[:len(prefix)-1]]; ok {
prefix = prefix[:len(prefix)-1]
if mappedPrefix, ok := rename[prefix]; ok {
return path.Join(mappedPrefix, remainder)
}
}
Expand Down
Loading