From dc12b9be6955169bfe5c53b4d2af56b4c50e6583 Mon Sep 17 00:00:00 2001 From: Avi Deitcher Date: Sun, 21 Apr 2024 11:56:56 +0300 Subject: [PATCH] prevent using same file for input tar and output tar Signed-off-by: Avi Deitcher --- src/cmd/linuxkit/build.go | 9 ++++- src/cmd/linuxkit/moby/build.go | 35 ++++++----------- .../060_input_tar/{ => 000_build}/README.md | 0 .../060_input_tar/{ => 000_build}/test.sh | 2 +- .../060_input_tar/{ => 000_build}/test1.yml | 0 .../060_input_tar/{ => 000_build}/test2.yml | 0 .../060_input_tar/010_same_filename/test.sh | 38 +++++++++++++++++++ .../060_input_tar/010_same_filename/test.yml | 37 ++++++++++++++++++ 8 files changed, 95 insertions(+), 26 deletions(-) rename test/cases/000_build/060_input_tar/{ => 000_build}/README.md (100%) rename test/cases/000_build/060_input_tar/{ => 000_build}/test.sh (98%) rename test/cases/000_build/060_input_tar/{ => 000_build}/test1.yml (100%) rename test/cases/000_build/060_input_tar/{ => 000_build}/test2.yml (100%) create mode 100644 test/cases/000_build/060_input_tar/010_same_filename/test.sh create mode 100644 test/cases/000_build/060_input_tar/010_same_filename/test.yml diff --git a/src/cmd/linuxkit/build.go b/src/cmd/linuxkit/build.go index 9e41b2493..6217e8370 100644 --- a/src/cmd/linuxkit/build.go +++ b/src/cmd/linuxkit/build.go @@ -205,8 +205,10 @@ The generated image can be in one of multiple formats which can be run on variou return nil } - var tf *os.File - var w io.Writer + var ( + tf *os.File + w io.Writer + ) if outfile != nil { w = outfile } else { @@ -216,6 +218,9 @@ The generated image can be in one of multiple formats which can be run on variou defer os.Remove(tf.Name()) w = tf } + if inputTar != "" && inputTar == outputFile { + return fmt.Errorf("input-tar and output file cannot be the same") + } // this is a weird interface, but currently only streamable types can have additional files // need to split up the base tarball outputs from the secondary stages diff --git a/src/cmd/linuxkit/moby/build.go b/src/cmd/linuxkit/moby/build.go index 5b19c38e3..47c9904f9 100644 --- a/src/cmd/linuxkit/moby/build.go +++ b/src/cmd/linuxkit/moby/build.go @@ -139,34 +139,23 @@ func Build(m Moby, w io.Writer, opts BuildOpts) error { } var ( oldConfig *Moby - tmpfile *os.File + in *os.File err error ) if metadataLocation != "" && opts.InputTar != "" { // copy the file over, in case it ends up being the same output - tmpfile, err = os.CreateTemp("", "linuxkit-input.tar") - if err != nil { - return fmt.Errorf("failed to create temporary file: %w", err) - } - defer tmpfile.Close() - in, err := os.Open(opts.InputTar) + in, err = os.Open(opts.InputTar) if err != nil { return fmt.Errorf("failed to open input tar: %w", err) } - if _, err := io.Copy(tmpfile, in); err != nil { - return fmt.Errorf("failed to copy input tar: %w", err) - } - if err := in.Close(); err != nil { - return fmt.Errorf("failed to close input file: %w", err) - } - if _, err := tmpfile.Seek(0, 0); err != nil { + defer in.Close() + if _, err := in.Seek(0, 0); err != nil { return fmt.Errorf("failed to seek to beginning of tmpfile: %w", err) } - // for efficiency, get the trimmed metadata path in advance - tmpTar := tar.NewReader(tmpfile) // read the tar until we find the metadata file + inputTarReader := tar.NewReader(in) for { - hdr, err := tmpTar.Next() + hdr, err := inputTarReader.Next() if err == io.EOF { break } @@ -175,7 +164,7 @@ func Build(m Moby, w io.Writer, opts BuildOpts) error { } if strings.TrimPrefix(hdr.Name, "/") == metadataLocation { buf := new(bytes.Buffer) - if _, err := buf.ReadFrom(tmpTar); err != nil { + if _, err := buf.ReadFrom(inputTarReader); err != nil { return fmt.Errorf("failed to read metadata file from input tar: %w", err) } config, err := NewConfig(buf.Bytes(), nil) @@ -217,7 +206,7 @@ func Build(m Moby, w io.Writer, opts BuildOpts) error { // first check if the existing one had it //if config != nil && len(oldConfig.initRefs) > index+1 && oldConfig.initRefs[index].String() == image { if oldConfig != nil && oldConfig.Kernel.ref != nil && oldConfig.Kernel.ref.String() == m.Kernel.ref.String() { - if err := extractPackageFilesFromTar(tmpfile, iw, m.Kernel.ref.String(), "kernel"); err != nil { + if err := extractPackageFilesFromTar(in, iw, m.Kernel.ref.String(), "kernel"); err != nil { return err } } else { @@ -242,7 +231,7 @@ func Build(m Moby, w io.Writer, opts BuildOpts) error { apkTar := newAPKTarWriter(iw, "init") for i, ii := range m.initRefs { if oldConfig != nil && len(oldConfig.initRefs) > i && oldConfig.initRefs[i].String() == ii.String() { - if err := extractPackageFilesFromTar(tmpfile, apkTar, ii.String(), fmt.Sprintf("init[%d]", i)); err != nil { + if err := extractPackageFilesFromTar(in, apkTar, ii.String(), fmt.Sprintf("init[%d]", i)); err != nil { return err } } else { @@ -262,7 +251,7 @@ func Build(m Moby, w io.Writer, opts BuildOpts) error { } for i, image := range m.Onboot { if oldConfig != nil && len(oldConfig.Onboot) > i && oldConfig.Onboot[i].Equal(image) { - if err := extractPackageFilesFromTar(tmpfile, iw, image.Image, fmt.Sprintf("onboot[%d]", i)); err != nil { + if err := extractPackageFilesFromTar(in, iw, image.Image, fmt.Sprintf("onboot[%d]", i)); err != nil { return err } } else { @@ -278,7 +267,7 @@ func Build(m Moby, w io.Writer, opts BuildOpts) error { } for i, image := range m.Onshutdown { if oldConfig != nil && len(oldConfig.Onshutdown) > i && oldConfig.Onshutdown[i].Equal(image) { - if err := extractPackageFilesFromTar(tmpfile, iw, image.Image, fmt.Sprintf("onshutdown[%d]", i)); err != nil { + if err := extractPackageFilesFromTar(in, iw, image.Image, fmt.Sprintf("onshutdown[%d]", i)); err != nil { return err } } else { @@ -294,7 +283,7 @@ func Build(m Moby, w io.Writer, opts BuildOpts) error { } for i, image := range m.Services { if oldConfig != nil && len(oldConfig.Services) > i && oldConfig.Services[i].Equal(image) { - if err := extractPackageFilesFromTar(tmpfile, iw, image.Image, fmt.Sprintf("services[%d]", i)); err != nil { + if err := extractPackageFilesFromTar(in, iw, image.Image, fmt.Sprintf("services[%d]", i)); err != nil { return err } } else { diff --git a/test/cases/000_build/060_input_tar/README.md b/test/cases/000_build/060_input_tar/000_build/README.md similarity index 100% rename from test/cases/000_build/060_input_tar/README.md rename to test/cases/000_build/060_input_tar/000_build/README.md diff --git a/test/cases/000_build/060_input_tar/test.sh b/test/cases/000_build/060_input_tar/000_build/test.sh similarity index 98% rename from test/cases/000_build/060_input_tar/test.sh rename to test/cases/000_build/060_input_tar/000_build/test.sh index e2f35feed..c5c22ec35 100644 --- a/test/cases/000_build/060_input_tar/test.sh +++ b/test/cases/000_build/060_input_tar/000_build/test.sh @@ -8,7 +8,7 @@ set -e #. "${RT_LIB}" . "${RT_PROJECT_ROOT}/_lib/lib.sh" -NAME=check +NAME=check_input_tar clean_up() { rm -f ${NAME}-*.tar diff --git a/test/cases/000_build/060_input_tar/test1.yml b/test/cases/000_build/060_input_tar/000_build/test1.yml similarity index 100% rename from test/cases/000_build/060_input_tar/test1.yml rename to test/cases/000_build/060_input_tar/000_build/test1.yml diff --git a/test/cases/000_build/060_input_tar/test2.yml b/test/cases/000_build/060_input_tar/000_build/test2.yml similarity index 100% rename from test/cases/000_build/060_input_tar/test2.yml rename to test/cases/000_build/060_input_tar/000_build/test2.yml diff --git a/test/cases/000_build/060_input_tar/010_same_filename/test.sh b/test/cases/000_build/060_input_tar/010_same_filename/test.sh new file mode 100644 index 000000000..a8e368399 --- /dev/null +++ b/test/cases/000_build/060_input_tar/010_same_filename/test.sh @@ -0,0 +1,38 @@ +#!/bin/sh +# SUMMARY: Check that tar output format build is reproducible after leveraging input tar +# LABELS: + +set -e + +# Source libraries. Uncomment if needed/defined +#. "${RT_LIB}" +. "${RT_PROJECT_ROOT}/_lib/lib.sh" + +NAME=check_input_tar_conflict_filename + +clean_up() { + rm -f ${NAME}-*.tar +} + +trap clean_up EXIT + +logfile=$(mktemp) + +# do not include the sbom, because the SBoM unique IDs per file/package are *not* deterministic, +# (currently based upon syft), and thus will make the file non-reproducible + +# the first one should build normally without a problem +linuxkit build --no-sbom --format tar --o "${NAME}-1.tar" ./test.yml + +# second one should fail because the input tar has the same filename as the output tar +set +e +linuxkit build -v --no-sbom --format tar --input-tar "${NAME}-1.tar" --o "${NAME}-1.tar" ./test.yml 2>&1 +ret="$?" +set -e + +if [ "$ret" -eq 0 ]; then + echo "Expected the build to fail, but it succeeded" + exit 1 +fi + +exit 0 diff --git a/test/cases/000_build/060_input_tar/010_same_filename/test.yml b/test/cases/000_build/060_input_tar/010_same_filename/test.yml new file mode 100644 index 000000000..cd7ecc5aa --- /dev/null +++ b/test/cases/000_build/060_input_tar/010_same_filename/test.yml @@ -0,0 +1,37 @@ +kernel: + image: linuxkit/kernel:6.6.13 + cmdline: "console=tty0 console=ttyS0 console=ttyAMA0" +init: + - linuxkit/init:45a1ad5919f0b6acf0f0cf730e9434abfae11fe6 + - linuxkit/runc:6062483d748609d505f2bcde4e52ee64a3329f5f + - linuxkit/containerd:e7a92d9f3282039eac5fb1b07cac2b8664cbf0ad +onboot: + - name: sysctl + image: linuxkit/sysctl:5a374e4bf3e5a7deeacff6571d0f30f7ea8f56db + - name: dhcpcd + image: linuxkit/dhcpcd:e9e3580f2de00e73e7b316a007186d22fea056ee + command: ["/sbin/dhcpcd", "--nobackground", "-f", "/dhcpcd.conf", "-1"] +onshutdown: + - name: shutdown + image: busybox:latest + command: ["/bin/echo", "so long and thanks for all the fish"] +services: + - name: getty + image: linuxkit/getty:5d86a2ce2d890c14ab66b13638dcadf74f29218b + env: + - INSECURE=true + - name: rngd + image: linuxkit/rngd:cdb919e4aee49fed0bf6075f0a104037cba83c39 + - name: nginx + image: nginx:1.19.5-alpine + capabilities: + - CAP_NET_BIND_SERVICE + - CAP_CHOWN + - CAP_SETUID + - CAP_SETGID + - CAP_DAC_OVERRIDE + binds: + - /etc/resolv.conf:/etc/resolv.conf +files: + - path: etc/linuxkit-config + metadata: yaml