diff --git a/README.md b/README.md index da58ef535..a3678de9e 100644 --- a/README.md +++ b/README.md @@ -138,6 +138,7 @@ expect - see [Known Issues](#known-issues). - [Flag `FF_KANIKO_VOLUME_SKIP_MKDIR`](#flag-ff_kaniko_volume_skip_mkdir) - [Flag `FF_KANIKO_PRESERVE_HARDLINKS`](#flag-ff_kaniko_preserve_hardlinks) - [Flag `FF_KANIKO_RELATIVE_LINK_TARGETS`](#flag-ff_kaniko_relative_link_targets) + - [Flag `FF_KANIKO_COPY_SKIP_SPECIAL_FILES`](#flag-ff_kaniko_copy_skip_special_files) - [Flag `FF_KANIKO_SKIP_WRITE_WHITEOUTS`](#flag-ff_kaniko_skip_write_whiteouts) - [Flag `FF_KANIKO_BUILDKIT_ARG_ENV_PRECEDENCE`](#flag-ff_kaniko_buildkit_arg_env_precedence) - [Flag `FF_KANIKO_INFER_CROSS_STAGE_CACHE_KEY`](#flag-ff_kaniko_infer_cross_stage_cache_key) @@ -1280,6 +1281,12 @@ Set this flag to `true` to write hardlink targets relative to the tar root. Defaults to `true`. Will be deprecated in `v1.29.0`. +#### Flag `FF_KANIKO_COPY_SKIP_SPECIAL_FILES` + +`COPY` reads each source file to copy it, which is wrong for anything that is not a regular file. A socket fails the build with `ENXIO`, and a block or character device is read as if it were a file, baking its contents into the image in place of the device node. +Set this flag to `true` to skip both with a warning instead. Fifos are always recreated with `mkfifo` regardless of this flag, since opening one hung the build outright. Defaults to `false`. +Becomes default in `v1.29.0`. + #### Flag `FF_KANIKO_SKIP_WRITE_WHITEOUTS` When kaniko extracts a cached layer it applies the layer's whiteouts by deleting the target files, but it also writes the `.wh.` marker files onto the working filesystem. With `--cache-copy-layers` a later cross-stage `COPY --from=` copies such a marker verbatim and commits it as a real whiteout, so a cache-hit build deletes a file that the cache-miss build kept. diff --git a/integration/dockerfiles/Dockerfile_test_issue_1599 b/integration/dockerfiles/Dockerfile_test_issue_1599 new file mode 100644 index 000000000..02928398b --- /dev/null +++ b/integration/dockerfiles/Dockerfile_test_issue_1599 @@ -0,0 +1,24 @@ +# 1599: COPY of a fifo hangs forever on v1.28.1. Source files are copied by +# opening and reading them, and open(2) on a fifo blocks until a writer shows +# up, so the build never terminates and never times out. Covers the fifo inside +# a copied directory, named directly as the source, and with --chown/--chmod. +ARG IMAGE_REPO +FROM ${IMAGE_REPO}busybox AS builder +RUN mkdir /work \ + && echo "content" > /work/regular.txt \ + && mkfifo -m 640 /work/pipe + +FROM ${IMAGE_REPO}busybox +# fifo inside a copied directory +COPY --from=builder /work/ /work/ +RUN test -p /work/pipe +RUN test "$(stat -c %a /work/pipe)" = "640" +RUN grep -q 'content' /work/regular.txt +# fifo named directly as the copy source +COPY --from=builder /work/pipe /direct/pipe +RUN test -p /direct/pipe +# --chown and --chmod must apply to a fifo like any other file +COPY --chown=1000:1000 --chmod=600 --from=builder /work/pipe /owned/pipe +RUN test -p /owned/pipe +RUN test "$(stat -c %u:%g /owned/pipe)" = "1000:1000" +RUN test "$(stat -c %a /owned/pipe)" = "600" diff --git a/integration/dockerfiles/Dockerfile_test_issue_1599_2 b/integration/dockerfiles/Dockerfile_test_issue_1599_2 new file mode 100644 index 000000000..d9b5ed9d4 --- /dev/null +++ b/integration/dockerfiles/Dockerfile_test_issue_1599_2 @@ -0,0 +1,13 @@ +# 1599: a fifo COPY must respect the kaniko directory guard like any other COPY. +# CreateFifo removes the destination before calling mkfifo, and the fifo branch +# skipped the check that refuses copies into /kaniko. A fifo copied over +# /kaniko/tini replaces the init every RUN execs through, so the build dies on +# the next RUN instead of skipping the copy with a warning. +# Breaks on the fifo COPY path added in PR #948. +ARG IMAGE_REPO +FROM ${IMAGE_REPO}busybox AS builder +RUN mkfifo /pipe + +FROM ${IMAGE_REPO}busybox +COPY --from=builder /pipe /kaniko/tini +RUN echo alive diff --git a/integration/images.go b/integration/images.go index eec008f51..331cfe469 100644 --- a/integration/images.go +++ b/integration/images.go @@ -132,6 +132,7 @@ var KanikoEnv = []string{ "FF_KANIKO_PATH_SCOPED_REGISTRY_AUTH=1", "FF_KANIKO_DEPRECATE_LAYERLESS_CACHE_ENTRIES=1", "FF_KANIKO_PLATFORM_CACHE_KEY=1", + "FF_KANIKO_COPY_SKIP_SPECIAL_FILES=1", "KANIKO_PRINT_PLAN=1", "KANIKO_TELEMETRY_ENDPOINT", "OTEL_EXPORTER_OTLP_HEADERS", @@ -301,6 +302,9 @@ var diffArgsMap = map[string][]string{ // But we discovered a new issue with this. For builtins, buildkit will emit "whiteout" files, // to remember that it was removed, we don't. So we end up with a diff in the resulting image. "TestRun/test_Dockerfile_test_issue_mz511": {"--extra-ignore-files=etc/.wh.nsswitch.conf", "--extra-ignore-layer-length-mismatch"}, + // 1599: docker copies the fifo into /kaniko, we refuse to write there at all, + // so the docker image keeps a /kaniko/tini our image never has. + "TestRun/test_Dockerfile_test_issue_1599_2": {"--extra-ignore-files=kaniko/", "--extra-ignore-layer-length-mismatch"}, // mz793: with FF_KANIKO_VOLUME_SKIP_MKDIR off, VOLUME creates the directory fresh on // each build, so its mtime differs between the two cached builds. That divergence is the // known volume non-determinism the flag fixes, here we only assert the build no longer panics. @@ -415,6 +419,8 @@ var expectedWarnings = map[string]string{ // mz991: the repro needs a MAINTAINER, which warns twice, once from the buildkit // linter and once from kaniko skipping the command. Both lines say "is deprecated". "Dockerfile_test_issue_mz991": "is deprecated", + // 1599: a fifo COPY into /kaniko must warn and be skipped like any other file. + "Dockerfile_test_issue_1599_2": "Skipping copy targeting kaniko directory", } func checkNoWarnings(dockerfile string, out []byte) error { diff --git a/pkg/commands/copy.go b/pkg/commands/copy.go index 59903f198..bb8de7deb 100644 --- a/pkg/commands/copy.go +++ b/pkg/commands/copy.go @@ -127,6 +127,19 @@ func (c *CopyCommand) ExecuteCommand(config *v1.Config, buildArgs *dockerfile.Bu continue } c.snapshotFiles = append(c.snapshotFiles, destPath) + } else if fi.Mode()&os.ModeNamedPipe != 0 { + // Opening a fifo blocks until it has a writer, so recreate it instead. + exclude, err := util.CreateFifo(fullPath, destPath, fi, c.fileContext, uid, gid, chmod, useDefaultChmod) + if err != nil { + return fmt.Errorf("copying fifo: %w", err) + } + if exclude { + continue + } + c.snapshotFiles = append(c.snapshotFiles, destPath) + } else if !fi.Mode().IsRegular() && kConfig.FF.CopySkipSpecialFiles { + logrus.Warnf("Ignoring special file %s, not copying to %s", fullPath, destPath) + continue } else { // ... Else, we want to copy over a file exclude, err := util.CopyFile(fullPath, destPath, c.fileContext, uid, gid, chmod, useDefaultChmod) diff --git a/pkg/config/featureflags.go b/pkg/config/featureflags.go index ebd0b0462..5fe1b9b43 100644 --- a/pkg/config/featureflags.go +++ b/pkg/config/featureflags.go @@ -34,6 +34,7 @@ type FeatureFlags struct { CleanKanikoDir bool CopyAsRoot bool CopyChmodOnImplicitDirs bool + CopySkipSpecialFiles bool CrossRepoMount bool DeprecateInterStageRestore bool DeprecateLayerlessCacheEntries bool @@ -108,6 +109,7 @@ func InitFeatureFlags() { CleanKanikoDir: featureFlag("FF_KANIKO_CLEAN_KANIKO_DIR", true), CopyAsRoot: featureFlag("FF_KANIKO_COPY_AS_ROOT", false), CopyChmodOnImplicitDirs: featureFlag("FF_KANIKO_COPY_CHMOD_ON_IMPLICIT_DIRS", false), + CopySkipSpecialFiles: featureFlag("FF_KANIKO_COPY_SKIP_SPECIAL_FILES", false), CrossRepoMount: featureFlag("FF_KANIKO_CROSS_REPO_MOUNT", false), DeprecateInterStageRestore: featureFlag("FF_KANIKO_DEPRECATE_INTER_STAGE_RESTORE", true), DeprecateLayerlessCacheEntries: featureFlag("FF_KANIKO_DEPRECATE_LAYERLESS_CACHE_ENTRIES", false), diff --git a/pkg/util/fs_util.go b/pkg/util/fs_util.go index 2776857da..af46e7ec8 100644 --- a/pkg/util/fs_util.go +++ b/pkg/util/fs_util.go @@ -37,6 +37,7 @@ import ( "github.com/moby/go-archive" "github.com/moby/patternmatcher" "github.com/moby/patternmatcher/ignorefile" + "github.com/osscontainertools/kaniko/pkg/assert" "github.com/osscontainertools/kaniko/pkg/config" "github.com/osscontainertools/kaniko/pkg/timing" otiai10Cpy "github.com/otiai10/copy" @@ -825,9 +826,12 @@ func CopyDir(src, dest string, context FileContext, uid, gid int64, chmod mode.S } } else if IsSymlink(fi) { // If file is a symlink, we want to create the same relative symlink - if _, err := CopySymlink(fullPath, destPath, context); err != nil { + exclude, err := CopySymlink(fullPath, destPath, context) + if err != nil { return nil, err } + // This loop already skipped matches + assert.Assert("util.copydir.symlink-not-excluded", !exclude, "CopySymlink refused to copy %s to %s", fullPath, destPath) } else if linkDst, ok := checkCopyHardlink(fi, destPath, hardlinksSeen); ok && config.FF.PreserveHardlinks { // #2594: inode already copied — create a hardlink instead of duplicating content. logrus.Tracef("Creating hardlink %s -> %s", destPath, linkDst) @@ -835,11 +839,25 @@ func CopyDir(src, dest string, context FileContext, uid, gid int64, chmod mode.S return nil, err } isHardlink = true + } else if fi.Mode()&os.ModeNamedPipe != 0 { + // Opening a fifo blocks until it has a writer, so recreate it instead. + exclude, err := CreateFifo(fullPath, destPath, fi, context, uid, gid, chmod, useDefaultChmod) + if err != nil { + return nil, err + } + // This loop already skipped matches + assert.Assert("util.copydir.fifo-not-excluded", !exclude, "CreateFifo refused to copy %s to %s", fullPath, destPath) + } else if !fi.Mode().IsRegular() && config.FF.CopySkipSpecialFiles { + logrus.Warnf("Ignoring special file %s, not copying to %s", fullPath, destPath) + continue } else { // ... Else, we want to copy over a file - if _, err := CopyFile(fullPath, destPath, context, uid, gid, chmod, useDefaultChmod); err != nil { + exclude, err := CopyFile(fullPath, destPath, context, uid, gid, chmod, useDefaultChmod) + if err != nil { return nil, err } + // This loop already skipped matches + assert.Assert("util.copydir.file-not-excluded", !exclude, "CopyFile refused to copy %s to %s", fullPath, destPath) } if !IsSymlink(fi) && !isHardlink { updates = append(updates, timestampUpdate{src: fullPath, dest: destPath}) @@ -855,6 +873,47 @@ func CopyDir(src, dest string, context FileContext, uid, gid int64, chmod mode.S return copiedFiles, nil } +// CreateFifo recreates the fifo at src as dest. Opening a fifo blocks until it +// has a writer, so it can never be copied by reading it. +func CreateFifo(src, dest string, fi os.FileInfo, context FileContext, uid, gid int64, chmod mode.Set, useDefaultChmod bool) (bool, error) { + if context.ExcludesFile(src) { + logrus.Debugf("%s found in .dockerignore, ignoring", src) + return true, nil + } + if HasFilepathPrefix(dest, config.KanikoDir, false) { + logrus.Warnf("Skipping copy targeting kaniko directory: %s", dest) + logrus.Info("Writes to the kaniko directory are blocked to prevent overwriting the executor.") + logrus.Info("To copy files there, relocate kaniko with KANIKO_DIR: https://github.com/osscontainertools/kaniko#bootstrapping-kaniko") + return true, nil + } + if src == dest { + // Recreating the fifo in place would drop it and take its readers with it. + return false, nil + } + uid, gid = DetermineTargetFileOwnership(fi, uid, gid) + if err := createParentDirectory(dest, int(uid), int(gid), chmod.Apply(0o755)); err != nil { + return false, err + } + if FilepathExists(dest) { + if err := os.RemoveAll(dest); err != nil { + return false, err + } + } + perm := fi.Mode() + if !useDefaultChmod { + perm = chmod.Apply(perm) + } + logrus.Tracef("Creating fifo %s", dest) + if err := unix.Mkfifo(dest, uint32(perm.Perm())); err != nil { + return false, fmt.Errorf("creating fifo %s: %w", dest, err) + } + // mkfifo applies the umask, chmod to get the mode we were actually asked for + if err := os.Chmod(dest, perm.Perm()); err != nil { + return false, err + } + return false, os.Lchown(dest, int(uid), int(gid)) +} + func checkCopyHardlink(fi os.FileInfo, dest string, seen map[uint64]string) (string, bool) { stat := getSyscallStatT(fi) if stat == nil || stat.Nlink <= 1 { @@ -1554,8 +1613,38 @@ func (NoAtimeFS) Open(name string) (fs.File, error) { return os.OpenFile(name, os.O_RDONLY|unix.O_NOATIME, 0) } +// Without StatFS, fs.Stat opens the file to stat it, which blocks on a fifo. +func (NoAtimeFS) Stat(name string) (fs.FileInfo, error) { + return os.Stat(name) +} + +// Without ReadLinkFS, fs.Lstat degrades to fs.Stat and dereferences the link. +func (NoAtimeFS) Lstat(name string) (fs.FileInfo, error) { + return os.Lstat(name) +} + +// Without ReadLinkFS, fs.ReadLink refuses to read any link at all. +func (NoAtimeFS) ReadLink(name string) (string, error) { + return os.Readlink(name) +} + type OSFS struct{} func (OSFS) Open(name string) (fs.File, error) { return os.Open(name) } + +// Without StatFS, fs.Stat opens the file to stat it, which blocks on a fifo. +func (OSFS) Stat(name string) (fs.FileInfo, error) { + return os.Stat(name) +} + +// Without ReadLinkFS, fs.Lstat degrades to fs.Stat and dereferences the link. +func (OSFS) Lstat(name string) (fs.FileInfo, error) { + return os.Lstat(name) +} + +// Without ReadLinkFS, fs.ReadLink refuses to read any link at all. +func (OSFS) ReadLink(name string) (string, error) { + return os.Readlink(name) +}