From c72bb4f8feb05a63c189fe6ce264ca652b399022 Mon Sep 17 00:00:00 2001 From: Valentin Rothberg Date: Mon, 10 Aug 2020 15:19:02 +0200 Subject: generate systemd: fix error handling Fix a bug in the error handling which returned nil instead of an error and ultimately lead to nil dereferences in the client. To prevent future regressions, add a test and check for the error message. Fixes: #7271 Signed-off-by: Valentin Rothberg --- test/e2e/generate_systemd_test.go | 12 ++++++++++++ 1 file changed, 12 insertions(+) (limited to 'test/e2e') diff --git a/test/e2e/generate_systemd_test.go b/test/e2e/generate_systemd_test.go index 654e99531..151d3f6fc 100644 --- a/test/e2e/generate_systemd_test.go +++ b/test/e2e/generate_systemd_test.go @@ -53,6 +53,18 @@ var _ = Describe("Podman generate systemd", func() { Expect(session).To(ExitWithError()) }) + It("podman generate systemd bad restart-policy value", func() { + session := podmanTest.Podman([]string{"create", "--name", "foobar", "alpine", "top"}) + session.WaitWithDefaultTimeout() + Expect(session.ExitCode()).To(Equal(0)) + + session = podmanTest.Podman([]string{"generate", "systemd", "--restart-policy", "bogus", "foobar"}) + session.WaitWithDefaultTimeout() + Expect(session).To(ExitWithError()) + found, _ := session.ErrorGrepString("Error: bogus is not a valid restart policy") + Expect(found).Should(BeTrue()) + }) + It("podman generate systemd good timeout value", func() { session := podmanTest.Podman([]string{"create", "--name", "foobar", "alpine", "top"}) session.WaitWithDefaultTimeout() -- cgit v1.2.3-54-g00ecf From d9c86fd0f2ea2d2587d470327192bd3d33875b2d Mon Sep 17 00:00:00 2001 From: Paul Holzinger Date: Wed, 5 Aug 2020 14:12:55 +0200 Subject: fix podman logs --tail when log is bigger than pagesize Signed-off-by: Paul Holzinger --- libpod/logs/log.go | 12 ++++++------ libpod/logs/reversereader/reversereader.go | 4 ++-- test/e2e/logs_test.go | 8 ++++---- 3 files changed, 12 insertions(+), 12 deletions(-) (limited to 'test/e2e') diff --git a/libpod/logs/log.go b/libpod/logs/log.go index 03acadb18..f66cbbadd 100644 --- a/libpod/logs/log.go +++ b/libpod/logs/log.go @@ -101,11 +101,14 @@ func getTailLog(path string, tail int) ([]*LogLine, error) { if err != nil { if errors.Cause(err) == io.EOF { inputs <- []string{leftover} - close(inputs) - break + } else { + logrus.Error(err) } - logrus.Error(err) close(inputs) + if err := f.Close(); err != nil { + logrus.Error(err) + } + break } line := strings.Split(s+leftover, "\n") if len(line) > 1 { @@ -136,9 +139,6 @@ func getTailLog(path string, tail int) ([]*LogLine, error) { } // if we have enough loglines, we can hangup if nllCounter >= tail { - if err := f.Close(); err != nil { - logrus.Error(err) - } break } } diff --git a/libpod/logs/reversereader/reversereader.go b/libpod/logs/reversereader/reversereader.go index 72d9ad975..4fa1a3f88 100644 --- a/libpod/logs/reversereader/reversereader.go +++ b/libpod/logs/reversereader/reversereader.go @@ -60,7 +60,7 @@ func (r *ReverseReader) Read() (string, error) { if int64(n) < r.readSize { b = b[0:n] } - // Set to the next page boundary - r.offset = -r.readSize + // Move the offset one pagesize up + r.offset -= r.readSize return string(b), nil } diff --git a/test/e2e/logs_test.go b/test/e2e/logs_test.go index 298174a63..20af3daa3 100644 --- a/test/e2e/logs_test.go +++ b/test/e2e/logs_test.go @@ -72,16 +72,16 @@ var _ = Describe("Podman logs", func() { Expect(len(results.OutputToStringArray())).To(Equal(0)) }) - It("tail 99 lines", func() { - logc := podmanTest.Podman([]string{"run", "-dt", ALPINE, "sh", "-c", "echo podman; echo podman; echo podman"}) + It("tail 800 lines", func() { + logc := podmanTest.Podman([]string{"run", "-dt", ALPINE, "sh", "-c", "i=1; while [ \"$i\" -ne 1000 ]; do echo \"line $i\"; i=$((i + 1)); done"}) logc.WaitWithDefaultTimeout() Expect(logc).To(Exit(0)) cid := logc.OutputToString() - results := podmanTest.Podman([]string{"logs", "--tail", "99", cid}) + results := podmanTest.Podman([]string{"logs", "--tail", "800", cid}) results.WaitWithDefaultTimeout() Expect(results).To(Exit(0)) - Expect(len(results.OutputToStringArray())).To(Equal(3)) + Expect(len(results.OutputToStringArray())).To(Equal(800)) }) It("tail 2 lines with timestamps", func() { -- cgit v1.2.3-54-g00ecf From a53b97beb3dfd3b95b205e35872344c5fadbb7c1 Mon Sep 17 00:00:00 2001 From: Qi Wang Date: Fri, 24 Jul 2020 14:09:12 -0400 Subject: fix bug podman sign storage path - fix the bud podman not using specified --directory as signature storage. - use manifest and image referce to set repo@digest. close #6994 close #6993 Backported-by: Valentin Rothberg Signed-off-by: Qi Wang --- go.mod | 5 +- go.sum | 8 +-- pkg/domain/infra/abi/images.go | 65 ++++++++++------------ pkg/trust/trust.go | 4 +- test/e2e/image_sign_test.go | 62 +++++++++++++++++++++ test/e2e/sign/secret-key.asc | 57 +++++++++++++++++++ .../k8s.io/apimachinery/pkg/apis/meta/v1/types.go | 3 + vendor/k8s.io/apimachinery/pkg/util/net/http.go | 2 +- vendor/modules.txt | 4 +- 9 files changed, 161 insertions(+), 49 deletions(-) create mode 100644 test/e2e/image_sign_test.go create mode 100644 test/e2e/sign/secret-key.asc (limited to 'test/e2e') diff --git a/go.mod b/go.mod index 227979afb..492b46032 100644 --- a/go.mod +++ b/go.mod @@ -63,8 +63,7 @@ require ( golang.org/x/net v0.0.0-20200520004742-59133d7f0dd7 golang.org/x/sync v0.0.0-20200317015054-43a5402ce75a golang.org/x/sys v0.0.0-20200519105757-fe76b779f299 - gopkg.in/yaml.v2 v2.3.0 - k8s.io/api v0.18.4 - k8s.io/apimachinery v0.18.4 + k8s.io/api v0.18.6 + k8s.io/apimachinery v0.18.6 k8s.io/client-go v0.0.0-20190620085101-78d2af792bab ) diff --git a/go.sum b/go.sum index 32601c7a6..3bb51cab8 100644 --- a/go.sum +++ b/go.sum @@ -630,11 +630,11 @@ gotest.tools v2.2.0+incompatible/go.mod h1:DsYFclhRJ6vuDpmuTbkuFWG+y2sxOXAzmJt81 honnef.co/go/tools v0.0.0-20190102054323-c2f93a96b099/go.mod h1:rf3lG4BRIbNafJWhAfAdb/ePZxsR/4RtNHQocxwk9r4= honnef.co/go/tools v0.0.0-20190523083050-ea95bdfd59fc/go.mod h1:rf3lG4BRIbNafJWhAfAdb/ePZxsR/4RtNHQocxwk9r4= k8s.io/api v0.0.0-20190620084959-7cf5895f2711/go.mod h1:TBhBqb1AWbBQbW3XRusr7n7E4v2+5ZY8r8sAMnyFC5A= -k8s.io/api v0.18.4 h1:8x49nBRxuXGUlDlwlWd3RMY1SayZrzFfxea3UZSkFw4= -k8s.io/api v0.18.4/go.mod h1:lOIQAKYgai1+vz9J7YcDZwC26Z0zQewYOGWdyIPUUQ4= +k8s.io/api v0.18.6 h1:osqrAXbOQjkKIWDTjrqxWQ3w0GkKb1KA1XkUGHHYpeE= +k8s.io/api v0.18.6/go.mod h1:eeyxr+cwCjMdLAmr2W3RyDI0VvTawSg/3RFFBEnmZGI= k8s.io/apimachinery v0.0.0-20190612205821-1799e75a0719/go.mod h1:I4A+glKBHiTgiEjQiCCQfCAIcIMFGt291SmsvcrFzJA= -k8s.io/apimachinery v0.18.4 h1:ST2beySjhqwJoIFk6p7Hp5v5O0hYY6Gngq/gUYXTPIA= -k8s.io/apimachinery v0.18.4/go.mod h1:OaXp26zu/5J7p0f92ASynJa1pZo06YlV9fG7BoWbCko= +k8s.io/apimachinery v0.18.6 h1:RtFHnfGNfd1N0LeSrKCUznz5xtUP1elRGvHJbL3Ntag= +k8s.io/apimachinery v0.18.6/go.mod h1:OaXp26zu/5J7p0f92ASynJa1pZo06YlV9fG7BoWbCko= k8s.io/client-go v0.0.0-20190620085101-78d2af792bab h1:E8Fecph0qbNsAbijJJQryKu4Oi9QTp5cVpjTE+nqg6g= k8s.io/client-go v0.0.0-20190620085101-78d2af792bab/go.mod h1:E95RaSlHr79aHaX0aGSwcPNfygDiPKOVXdmivCIZT0k= k8s.io/gengo v0.0.0-20190128074634-0689ccc1d7d6/go.mod h1:ezvh/TsK7cY6rbqRK0oQQ8IAqLxYwwyPxAX1Pzy0ii0= diff --git a/pkg/domain/infra/abi/images.go b/pkg/domain/infra/abi/images.go index 9f594d728..5f19f416a 100644 --- a/pkg/domain/infra/abi/images.go +++ b/pkg/domain/infra/abi/images.go @@ -7,6 +7,7 @@ import ( "io/ioutil" "net/url" "os" + "path" "path/filepath" "strconv" "strings" @@ -564,10 +565,6 @@ func (ir *ImageEngine) Shutdown(_ context.Context) { } func (ir *ImageEngine) Sign(ctx context.Context, names []string, options entities.SignOptions) (*entities.SignReport, error) { - dockerRegistryOptions := image.DockerRegistryOptions{ - DockerCertPath: options.CertDir, - } - mech, err := signature.NewGPGSigningMechanism() if err != nil { return nil, errors.Wrap(err, "error initializing GPG") @@ -586,7 +583,6 @@ func (ir *ImageEngine) Sign(ctx context.Context, names []string, options entitie } for _, signimage := range names { - var sigStoreDir string srcRef, err := alltransports.ParseImageName(signimage) if err != nil { return nil, errors.Wrapf(err, "error parsing image name") @@ -607,40 +603,38 @@ func (ir *ImageEngine) Sign(ctx context.Context, names []string, options entitie if dockerReference == nil { return nil, errors.Errorf("cannot determine canonical Docker reference for destination %s", transports.ImageName(rawSource.Reference())) } - - // create the signstore file - rtc, err := ir.Libpod.GetConfig() - if err != nil { - return nil, err - } - newImage, err := ir.Libpod.ImageRuntime().New(ctx, signimage, rtc.Engine.SignaturePolicyPath, "", os.Stderr, &dockerRegistryOptions, image.SigningOptions{SignBy: options.SignBy}, nil, util.PullImageMissing) - if err != nil { - return nil, errors.Wrapf(err, "error pulling image %s", signimage) + var sigStoreDir string + if options.Directory != "" { + sigStoreDir = options.Directory } if sigStoreDir == "" { if rootless.IsRootless() { sigStoreDir = filepath.Join(filepath.Dir(ir.Libpod.StorageConfig().GraphRoot), "sigstore") } else { + var sigStoreURI string registryInfo := trust.HaveMatchRegistry(rawSource.Reference().DockerReference().String(), registryConfigs) if registryInfo != nil { - if sigStoreDir = registryInfo.SigStoreStaging; sigStoreDir == "" { - sigStoreDir = registryInfo.SigStore - + if sigStoreURI = registryInfo.SigStoreStaging; sigStoreURI == "" { + sigStoreURI = registryInfo.SigStore } } + if sigStoreURI == "" { + return nil, errors.Errorf("no signature storage configuration found for %s", rawSource.Reference().DockerReference().String()) + + } + sigStoreDir, err = localPathFromURI(sigStoreURI) + if err != nil { + return nil, errors.Wrapf(err, "invalid signature storage %s", sigStoreURI) + } } } - sigStoreDir, err = isValidSigStoreDir(sigStoreDir) + manifestDigest, err := manifest.Digest(getManifest) if err != nil { - return nil, errors.Wrapf(err, "invalid signature storage %s", sigStoreDir) - } - repos, err := newImage.RepoDigests() - if err != nil { - return nil, errors.Wrapf(err, "error calculating repo digests for %s", signimage) + return nil, err } - if len(repos) == 0 { - logrus.Errorf("no repodigests associated with the image %s", signimage) - continue + repo := reference.Path(dockerReference) + if path.Clean(repo) != repo { // Coverage: This should not be reachable because /./ and /../ components are not valid in docker references + return nil, errors.Errorf("Unexpected path elements in Docker reference %s for signature storage", dockerReference.String()) } // create signature @@ -648,22 +642,21 @@ func (ir *ImageEngine) Sign(ctx context.Context, names []string, options entitie if err != nil { return nil, errors.Wrapf(err, "error creating new signature") } - - trimmedDigest := strings.TrimPrefix(repos[0], strings.Split(repos[0], "/")[0]) - sigStoreDir = filepath.Join(sigStoreDir, strings.Replace(trimmedDigest, ":", "=", 1)) - if err := os.MkdirAll(sigStoreDir, 0751); err != nil { + // create the signstore file + signatureDir := fmt.Sprintf("%s@%s=%s", filepath.Join(sigStoreDir, repo), manifestDigest.Algorithm(), manifestDigest.Hex()) + if err := os.MkdirAll(signatureDir, 0751); err != nil { // The directory is allowed to exist if !os.IsExist(err) { - logrus.Errorf("error creating directory %s: %s", sigStoreDir, err) + logrus.Errorf("error creating directory %s: %s", signatureDir, err) continue } } - sigFilename, err := getSigFilename(sigStoreDir) + sigFilename, err := getSigFilename(signatureDir) if err != nil { logrus.Errorf("error creating sigstore file: %v", err) continue } - err = ioutil.WriteFile(filepath.Join(sigStoreDir, sigFilename), newSig, 0644) + err = ioutil.WriteFile(filepath.Join(signatureDir, sigFilename), newSig, 0644) if err != nil { logrus.Errorf("error storing signature for %s", rawSource.Reference().DockerReference().String()) continue @@ -691,14 +684,12 @@ func getSigFilename(sigStoreDirPath string) (string, error) { } } -func isValidSigStoreDir(sigStoreDir string) (string, error) { - writeURIs := map[string]bool{"file": true} +func localPathFromURI(sigStoreDir string) (string, error) { url, err := url.Parse(sigStoreDir) if err != nil { return sigStoreDir, errors.Wrapf(err, "invalid directory %s", sigStoreDir) } - _, exists := writeURIs[url.Scheme] - if !exists { + if url.Scheme != "file" { return sigStoreDir, errors.Errorf("writing to %s is not supported. Use a supported scheme", sigStoreDir) } sigStoreDir = url.Path diff --git a/pkg/trust/trust.go b/pkg/trust/trust.go index 60de099fa..2348bc410 100644 --- a/pkg/trust/trust.go +++ b/pkg/trust/trust.go @@ -12,9 +12,9 @@ import ( "strings" "github.com/containers/image/v5/types" + "github.com/ghodss/yaml" "github.com/pkg/errors" "github.com/sirupsen/logrus" - "gopkg.in/yaml.v2" ) // PolicyContent struct for policy.json file @@ -157,7 +157,7 @@ func HaveMatchRegistry(key string, registryConfigs *RegistryConfiguration) *Regi searchKey = searchKey[:strings.LastIndex(searchKey, "/")] } } - return nil + return registryConfigs.DefaultDocker } // CreateTmpFile creates a temp file under dir and writes the content into it diff --git a/test/e2e/image_sign_test.go b/test/e2e/image_sign_test.go new file mode 100644 index 000000000..d5d460b38 --- /dev/null +++ b/test/e2e/image_sign_test.go @@ -0,0 +1,62 @@ +// +build !remote + +package integration + +import ( + "os" + "os/exec" + "path/filepath" + + . "github.com/containers/libpod/v2/test/utils" + . "github.com/onsi/ginkgo" + . "github.com/onsi/gomega" +) + +var _ = Describe("Podman image sign", func() { + var ( + origGNUPGHOME string + tempdir string + err error + podmanTest *PodmanTestIntegration + ) + + BeforeEach(func() { + tempdir, err = CreateTempDirInTempDir() + if err != nil { + os.Exit(1) + } + podmanTest = PodmanTestCreate(tempdir) + podmanTest.Setup() + podmanTest.SeedImages() + + tempGNUPGHOME := filepath.Join(podmanTest.TempDir, "tmpGPG") + err := os.Mkdir(tempGNUPGHOME, os.ModePerm) + Expect(err).To(BeNil()) + + origGNUPGHOME = os.Getenv("GNUPGHOME") + err = os.Setenv("GNUPGHOME", tempGNUPGHOME) + Expect(err).To(BeNil()) + + }) + + AfterEach(func() { + podmanTest.Cleanup() + f := CurrentGinkgoTestDescription() + processTestResult(f) + os.Setenv("GNUPGHOME", origGNUPGHOME) + }) + + It("podman sign image", func() { + cmd := exec.Command("gpg", "--import", "sign/secret-key.asc") + err := cmd.Run() + Expect(err).To(BeNil()) + sigDir := filepath.Join(podmanTest.TempDir, "test-sign") + err = os.MkdirAll(sigDir, os.ModePerm) + Expect(err).To(BeNil()) + session := podmanTest.Podman([]string{"image", "sign", "--directory", sigDir, "--sign-by", "foo@bar.com", "docker://library/alpine"}) + session.WaitWithDefaultTimeout() + Expect(session.ExitCode()).To(Equal(0)) + _, err = os.Stat(filepath.Join(sigDir, "library")) + Expect(err).To(BeNil()) + }) +}) diff --git a/test/e2e/sign/secret-key.asc b/test/e2e/sign/secret-key.asc new file mode 100644 index 000000000..23c0d05c3 --- /dev/null +++ b/test/e2e/sign/secret-key.asc @@ -0,0 +1,57 @@ +-----BEGIN PGP PRIVATE KEY BLOCK----- + +lQOYBF8kNqwBCAC0x3Kog+WlDNwcR6rWIP8Gj2T6LrQ2/3knSyAWzTgC/OBB6Oh0 +KAokXLjy8J3diG3EaSltE7erGG/bZCz8jYvMiwDJScON4zzidotqjoY80E+NeRDg +CC0gqvqmh0ftJIjYNBHzSxqrGRQwzwZU+u6ezlE8+0dvsHcHY+MRnxXJQrdM07EP +Prp85kKckChDlJ1tyGUB/YHieFQmOW5+TERA7ZqQOAQ12Vviv6V4kNfEJJq3MS2c +csZpO323tcHt3oebqsZCIElhX7uVw6GAeCw1tm4NZXs4g1yIC21Of/hzPeC18F72 +splCgKaAOiE9w/nMGLNEYy2NzgEclZLs2Y7jABEBAAEAB/9VOcwHvvrWN3xThsP2 +5CJmxNZtjfQfE4zZ5fRwW3pjCjVtTTC9hhzV7LKysZYzGPzqwksp5chKjKA7VXxR +6ic0nHmX68MaEr2i5BExAJUveWNvxloazC/+PS0ishdKKNWs28t0n/0oGZAnvIn3 +KT+ytYCeF7ajZJWQ8dncdlvuf86I8GdmqP2Og9A67LUpJfH2rtpBjzH25nLSZ3Qz +QbHoUIv318Wwb1sIidkmPsZufZG3pMsYjtFtJjkWt0lRsJQnSE9AQOpQTkLsVsh2 +FYQZ2ODhH8+NE86UNAAr2JiMZHoTrEUL2SLwpXEthFIR78N009dOS4nw8CLB61BL +pr6lBADH6yoF95rI0LR3jphfr7e8K3BViTPK97wke6EqwTlVo0TCdR785sa8T8O8 +HvlYx4a+h3e6D4D0vjDPzxtevjdTjYmxqI3cwE2N3NFALgGBwHs01qRRIw4qxZlC +1L1byJ8rUVi5z3YMO7X4RcXSAtg3fM2fx2x+sdpyH30jafuKPwQA533PVRo/Rlgt +pOak9Fs+3KOIb+oO8ypy7RRQcnsTKajts0sUE9scBhT4tSDeSa/HaDvzLiE8KKCK +3rhn8ZKLTW1fvKNZBj6oGlIRyfOFjtN/jMRjo0WVHSUDJ59Zr8C0khpP5J73yhTr +fDhcuTPWiCjlDYeSFHV/a4Z45GG2Kl0EAL1I31kxnSQR9bN5ZmvV+aOhTRKOuHDm +6nISF/XnVwuGHCvMbFRKsTxGkGrPO5VQZflFOqVab9umIQkOIcrzeKj+slYlm5VA +zKfCQ1vZ2f74QYCNP8oeRa1r3D46fszcElZJQxtZZewYRKX63bvU4F+hql8dJTqe +e3wVq8QD657yRwC0FGZvb2JhciA8Zm9vQGJhci5jb20+iQFUBBMBCAA+FiEERyT4 +ac7LLibByeabqaoHAy6P2bIFAl8kNqwCGwMFCQPCZwAFCwkIBwIGFQoJCAsCBBYC +AwECHgECF4AACgkQqaoHAy6P2bKtuggAgv54/F8wgi+uMrtFr8rqNtZMDyXRxfXa +XUy5uGNfqHD83yqxweEqxiA8lmFkRHixPWtgZ2MniFXMVc9kVmg8GNIIuzewXrPq +tXztvuURQo9phK68v8fXEqqT6K25wtq8TiQZ0J3mQIJPPTMe3pCCOyR6+W3iMtQp +2AmitxKbzLP3J3GG2i0rG5S147A2rPnzTeMYhds819+JE7jNMD7FkV+TcQlOVl4w +yOQhNEJcjb6rA6EUe5+s85pIFTBSyPMJpJ03Y0dLdcSGpKdncGTK2X9+hS96G1+F +P/t8hRIDblqUHtBRXe3Ozz6zSqpqu1DbAQSMbIrLYxXfnZEN+ro0dJ0DmARfJDas +AQgAncvLLZUHZkJWDPka3ocysJ7+/lmrXyAjT3D4r7UM4oaLBOMKjvaKSDw1uW5q +YmTxnnsqFDI0O5+XJxD1/0qEf6l2oUpnILdxVruf28FuvymbsyhDgs+MBoHz0jLW +WPHUW2oWLIqcvaF0BePQ1GS6UoZlmZejsLwwcSpbaAHJng7An/iLuqOBr5EdUA5X +MXqmdMFDrjh0uZezImJ2Eacu/hshBdu3IY49J5XP18GWrSdUnP27cv3tOii9j5Lf +l8QAvCN89vkALIU3eZtnMlWZqLgl5o6COVFmzpyx+iHOoCznQBt0aGoSNmE/dAqW +IQS/xCSFqMHI6kNd9N0oR0rEHwARAQABAAf+L3mAhBLJ2qDLrfSGenv3qr7zXggR +cLnFFeIZ2BdjLIYpLku2wgN34DrJOSR4umi/bxyEMPZX07Z0rgrC0E+VpKkSKX2u +oF/AqEUj1+SPEtGMaC8NfL4/1Tdk6ZFk/vanGufEixsbBEyekSUVD8nMawbHa5n9 +ZC+CbZG+VYDwLW6u0Pb26CIhqpFNQL3E88uLeVNhnE+nNJfgB2Nyo8gUQszovUxk +hv64UlXYA3wt49mpc9ORs9qKMZkuKdJfYmJkmvqLE35YpRRz6i1+hg71doj7Sjel +naBV3qIrIbcN6I2/9ZUwVwCzttpeHDfKOxQk5szWFop6H79TZGRsrV6boQQAwnFO +v5pjsZLhqHIZPty3zXz7Tv3LmaatwA260n6NcLBuQFiuEoh2QsMfVtLU8bBzyuC8 +Znx3kPlGCCognSjkEis+aEjsZgvCzR3aP+FWejkhnZnFiSJDvgEftODLF3gSPVp3 +dhc6q5GLysc0iN/gkBZN8Qm1lL/kEyeri4mbWT8EAM/AczrXL0tPEV3YzyeyM972 +HP9OnIYoyIkCa4M0PA0qhUPJ+vBHl/1+p5WZD/eokXqJ2M8IqNSlinuou3azbg+r +N3xTaB0a+Vx6O/RRI73+4UK2fyN9gYRH437eliNBRTkZeZCQ6Dd5eYcABaL2DbSs +1dyGXzRWfzdvGVu/r/0hBACER5u/uac+y9sXr79imoLVya25XkjvswGrDxmrlmNg +cfn/bix9Z93TXScYPiyxzLwlDDd7ovlpv1+mbHNgj6krSGG+R+uLQ2+nm+9glMmz +KupEYF59lzOgEYScJaHQWBULPRGUy/7HmZGpsDmz8zpj8lHaFNLlqDzrxw3MNKxO +F0NFiQE8BBgBCAAmFiEERyT4ac7LLibByeabqaoHAy6P2bIFAl8kNqwCGwwFCQPC +ZwAACgkQqaoHAy6P2bJfjQgAje6YR+p1QaNlTN9l4t2kGzy9RhkfYMrTgI2fEqbS +9bFJUy3Y3mH+vj/r2gN/kaN8LHH4K1d7fAohBsFqSI0flzHHIx2rfti9zAlbXcAE +rbnG+f0fk0AaqU7KelU35vjPfNe6Vn7ky6G9CC6jW04NkLZDNFA2GusdYf1aM0LW +ew5t4WZaquLVFhL36q9eHaogO/fcPR/quvQefHokk+b541ytwMN9l/g43rTbCvAj +rUDHwipbGbw91Wg2XjbecRiCXDKWds2M149BpxUzY5xHFtD5t5WSEE/SkkryGTMm +TxS3tuQZ9PdtCPGrNDO6Ts/amORF04Tf+YMJgfv3IWxMeQ== +=6kcB +-----END PGP PRIVATE KEY BLOCK----- diff --git a/vendor/k8s.io/apimachinery/pkg/apis/meta/v1/types.go b/vendor/k8s.io/apimachinery/pkg/apis/meta/v1/types.go index bf125b62a..e7aaead8c 100644 --- a/vendor/k8s.io/apimachinery/pkg/apis/meta/v1/types.go +++ b/vendor/k8s.io/apimachinery/pkg/apis/meta/v1/types.go @@ -873,6 +873,9 @@ const ( // FieldManagerConflict is used to report when another client claims to manage this field, // It should only be returned for a request using server-side apply. CauseTypeFieldManagerConflict CauseType = "FieldManagerConflict" + // CauseTypeResourceVersionTooLarge is used to report that the requested resource version + // is newer than the data observed by the API server, so the request cannot be served. + CauseTypeResourceVersionTooLarge CauseType = "ResourceVersionTooLarge" ) // +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object diff --git a/vendor/k8s.io/apimachinery/pkg/util/net/http.go b/vendor/k8s.io/apimachinery/pkg/util/net/http.go index 7449cbb0a..7b64e6815 100644 --- a/vendor/k8s.io/apimachinery/pkg/util/net/http.go +++ b/vendor/k8s.io/apimachinery/pkg/util/net/http.go @@ -446,7 +446,7 @@ redirectLoop: // Only follow redirects to the same host. Otherwise, propagate the redirect response back. if requireSameHostRedirects && location.Hostname() != originalLocation.Hostname() { - break redirectLoop + return nil, nil, fmt.Errorf("hostname mismatch: expected %s, found %s", originalLocation.Hostname(), location.Hostname()) } // Reset the connection. diff --git a/vendor/modules.txt b/vendor/modules.txt index bf2a14d4c..4d1121553 100644 --- a/vendor/modules.txt +++ b/vendor/modules.txt @@ -689,10 +689,10 @@ gopkg.in/tomb.v1 gopkg.in/yaml.v2 # gopkg.in/yaml.v3 v3.0.0-20200313102051-9f266ea9e77c gopkg.in/yaml.v3 -# k8s.io/api v0.18.4 +# k8s.io/api v0.18.6 k8s.io/api/apps/v1 k8s.io/api/core/v1 -# k8s.io/apimachinery v0.18.4 +# k8s.io/apimachinery v0.18.6 k8s.io/apimachinery/pkg/api/errors k8s.io/apimachinery/pkg/api/resource k8s.io/apimachinery/pkg/apis/meta/v1 -- cgit v1.2.3-54-g00ecf From d6442f5f571112d66fd62309a2e8e15c163ff4f3 Mon Sep 17 00:00:00 2001 From: Matthew Heon Date: Mon, 3 Aug 2020 13:33:08 -0400 Subject: Do not set host IP on ports when 0.0.0.0 requested Docker and CNI have very different ideas of what 0.0.0.0 means. Docker takes it to be 0.0.0.0/0 - that is, bind to every IPv4 address on the host. CNI (and, thus, root Podman) take it to mean the literal IP 0.0.0.0. Instead, CNI interprets the empty string ("") as "bind to all IPs". We could ask CNI to change, but given this is established behavior, that's unlikely. Instead, let's just catch 0.0.0.0 and turn it into "" when we parse ports. Fixes #7014 Signed-off-by: Matthew Heon --- cmd/podman/common/util.go | 13 ++++++++----- docs/source/markdown/podman-create.1.md | 3 ++- docs/source/markdown/podman-run.1.md | 2 ++ test/e2e/run_networking_test.go | 12 ++++++++++++ 4 files changed, 24 insertions(+), 6 deletions(-) (limited to 'test/e2e') diff --git a/cmd/podman/common/util.go b/cmd/podman/common/util.go index e21e349d9..52b637a78 100644 --- a/cmd/podman/common/util.go +++ b/cmd/podman/common/util.go @@ -175,12 +175,15 @@ func parseSplitPort(hostIP, hostPort *string, ctrPort string, protocol *string) if hostIP != nil { if *hostIP == "" { return newPort, errors.Errorf("must provide a non-empty container host IP to publish") + } else if *hostIP != "0.0.0.0" { + // If hostIP is 0.0.0.0, leave it unset - CNI treats + // 0.0.0.0 and empty differently, Docker does not. + testIP := net.ParseIP(*hostIP) + if testIP == nil { + return newPort, errors.Errorf("cannot parse %q as an IP address", *hostIP) + } + newPort.HostIP = testIP.String() } - testIP := net.ParseIP(*hostIP) - if testIP == nil { - return newPort, errors.Errorf("cannot parse %q as an IP address", *hostIP) - } - newPort.HostIP = testIP.String() } if hostPort != nil { if *hostPort == "" { diff --git a/docs/source/markdown/podman-create.1.md b/docs/source/markdown/podman-create.1.md index 2b383f51b..715d6c1c9 100644 --- a/docs/source/markdown/podman-create.1.md +++ b/docs/source/markdown/podman-create.1.md @@ -628,7 +628,8 @@ Both hostPort and containerPort can be specified as a range of ports. When specifying ranges for both, the number of container ports in the range must match the number of host ports in the range. (e.g., `podman run -p 1234-1236:1222-1224 --name thisWorks -t busybox` but not `podman run -p 1230-1236:1230-1240 --name RangeContainerPortsBiggerThanRangeHostPorts -t busybox`) -With ip: `podman run -p 127.0.0.1:$HOSTPORT:$CONTAINERPORT --name CONTAINER -t someimage` +With host IP: `podman run -p 127.0.0.1:$HOSTPORT:$CONTAINERPORT --name CONTAINER -t someimage` +If host IP is set to 0.0.0.0 or not set at all, the port will be bound on all IPs on the host. Host port does not have to be specified (e.g. `podman run -p 127.0.0.1::80`). If it is not, the container port will be randomly assigned a port on the host. Use `podman port` to see the actual mapping: `podman port CONTAINER $CONTAINERPORT` diff --git a/docs/source/markdown/podman-run.1.md b/docs/source/markdown/podman-run.1.md index d20b69507..fab648593 100644 --- a/docs/source/markdown/podman-run.1.md +++ b/docs/source/markdown/podman-run.1.md @@ -638,6 +638,8 @@ Both hostPort and containerPort can be specified as a range of ports. When specifying ranges for both, the number of container ports in the range must match the number of host ports in the range. +If host IP is set to 0.0.0.0 or not set at all, the port will be bound on all IPs on the host. + Host port does not have to be specified (e.g. `podman run -p 127.0.0.1::80`). If it is not, the container port will be randomly assigned a port on the host. diff --git a/test/e2e/run_networking_test.go b/test/e2e/run_networking_test.go index 40cc9e1e6..88e289f1f 100644 --- a/test/e2e/run_networking_test.go +++ b/test/e2e/run_networking_test.go @@ -238,6 +238,18 @@ var _ = Describe("Podman run networking", func() { Expect((hp1 == "4000" && hp2 == "8000") || (hp1 == "8000" && hp2 == "4000")).To(BeTrue()) }) + It("podman run -p 0.0.0.0:8080:80", func() { + name := "testctr" + session := podmanTest.Podman([]string{"create", "-t", "-p", "0.0.0.0:8080:80", "--name", name, ALPINE, "/bin/sh"}) + session.WaitWithDefaultTimeout() + inspectOut := podmanTest.InspectContainer(name) + Expect(len(inspectOut)).To(Equal(1)) + Expect(len(inspectOut[0].NetworkSettings.Ports)).To(Equal(1)) + Expect(len(inspectOut[0].NetworkSettings.Ports["80/tcp"])).To(Equal(1)) + Expect(inspectOut[0].NetworkSettings.Ports["80/tcp"][0].HostPort).To(Equal("8080")) + Expect(inspectOut[0].NetworkSettings.Ports["80/tcp"][0].HostIP).To(Equal("")) + }) + It("podman run network expose host port 80 to container port 8000", func() { SkipIfRootless() session := podmanTest.Podman([]string{"run", "-dt", "-p", "80:8000", ALPINE, "/bin/sh"}) -- cgit v1.2.3-54-g00ecf From 85d7cb5fd42642b26b0fe3c98f165c081b41c45e Mon Sep 17 00:00:00 2001 From: zhangguanzhang Date: Sat, 1 Aug 2020 23:37:41 +0800 Subject: implement the exitcode when start a container with attach Signed-off-by: zhangguanzhang --- cmd/podman/containers/start.go | 11 ++++++++--- test/e2e/start_test.go | 12 ++++++++++++ 2 files changed, 20 insertions(+), 3 deletions(-) (limited to 'test/e2e') diff --git a/cmd/podman/containers/start.go b/cmd/podman/containers/start.go index 8f9984421..84a7aa4e2 100644 --- a/cmd/podman/containers/start.go +++ b/cmd/podman/containers/start.go @@ -99,12 +99,17 @@ func start(cmd *cobra.Command, args []string) error { } for _, r := range responses { - if r.Err == nil && !startOptions.Attach { - fmt.Println(r.RawInput) + if r.Err == nil { + if startOptions.Attach { + // Implement the exitcode when the only one container is enabled attach + registry.SetExitCode(r.ExitCode) + } else { + fmt.Println(r.RawInput) + } } else { errs = append(errs, r.Err) } } - // TODO need to understand an implement exitcodes + return errs.PrintErrors() } diff --git a/test/e2e/start_test.go b/test/e2e/start_test.go index 761b1c4ca..3cc1668e7 100644 --- a/test/e2e/start_test.go +++ b/test/e2e/start_test.go @@ -86,6 +86,18 @@ var _ = Describe("Podman start", func() { Expect(session.OutputToString()).To(Equal(name)) }) + It("podman start single container with attach and test the signal", func() { + SkipIfRemote() + session := podmanTest.Podman([]string{"create", "--entrypoint", "sh", ALPINE, "-c", "exit 1"}) + session.WaitWithDefaultTimeout() + Expect(session.ExitCode()).To(Equal(0)) + cid := session.OutputToString() + session = podmanTest.Podman([]string{"start", "--attach", cid}) + session.WaitWithDefaultTimeout() + // It should forward the signal + Expect(session.ExitCode()).To(Equal(1)) + }) + It("podman start multiple containers", func() { session := podmanTest.Podman([]string{"create", "-d", "--name", "foobar99", ALPINE, "ls"}) session.WaitWithDefaultTimeout() -- cgit v1.2.3-54-g00ecf From 895e0d0e2ea74cbba4a4a351e865c55f313bdf99 Mon Sep 17 00:00:00 2001 From: Paul Holzinger Date: Wed, 29 Jul 2020 21:58:46 +0200 Subject: fix pod creation with "new:" syntax When you execute podman create/run with the --pod new: syntax the pod was created but the namespaces where not shared and therefore containers could not communicate over localhost. Add the default namespaces and pass the network options to the pod create options. Signed-off-by: Paul Holzinger --- cmd/podman/containers/create.go | 13 ++++++------- cmd/podman/containers/run.go | 2 +- pkg/specgen/generate/namespaces.go | 4 ++++ test/e2e/run_test.go | 6 +++++- 4 files changed, 16 insertions(+), 9 deletions(-) (limited to 'test/e2e') diff --git a/cmd/podman/containers/create.go b/cmd/podman/containers/create.go index 41e63da76..0cd56f540 100644 --- a/cmd/podman/containers/create.go +++ b/cmd/podman/containers/create.go @@ -124,7 +124,7 @@ func create(cmd *cobra.Command, args []string) error { return err } - if _, err := createPodIfNecessary(s); err != nil { + if _, err := createPodIfNecessary(s, cliVals.Net); err != nil { return err } @@ -279,7 +279,7 @@ func openCidFile(cidfile string) (*os.File, error) { // createPodIfNecessary automatically creates a pod when requested. if the pod name // has the form new:ID, the pod ID is created and the name in the spec generator is replaced // with ID. -func createPodIfNecessary(s *specgen.SpecGenerator) (*entities.PodCreateReport, error) { +func createPodIfNecessary(s *specgen.SpecGenerator, netOpts *entities.NetOptions) (*entities.PodCreateReport, error) { if !strings.HasPrefix(s.Pod, "new:") { return nil, nil } @@ -288,11 +288,10 @@ func createPodIfNecessary(s *specgen.SpecGenerator) (*entities.PodCreateReport, return nil, errors.Errorf("new pod name must be at least one character") } createOptions := entities.PodCreateOptions{ - Name: podName, - Infra: true, - Net: &entities.NetOptions{ - PublishPorts: s.PortMappings, - }, + Name: podName, + Infra: true, + Net: netOpts, + CreateCommand: os.Args, } s.Pod = podName return registry.ContainerEngine().PodCreate(context.Background(), createOptions) diff --git a/cmd/podman/containers/run.go b/cmd/podman/containers/run.go index 638b1c96e..d8ffa5725 100644 --- a/cmd/podman/containers/run.go +++ b/cmd/podman/containers/run.go @@ -173,7 +173,7 @@ func run(cmd *cobra.Command, args []string) error { } runOpts.Spec = s - if _, err := createPodIfNecessary(s); err != nil { + if _, err := createPodIfNecessary(s, cliVals.Net); err != nil { return err } diff --git a/pkg/specgen/generate/namespaces.go b/pkg/specgen/generate/namespaces.go index 566830cd8..39a45398d 100644 --- a/pkg/specgen/generate/namespaces.go +++ b/pkg/specgen/generate/namespaces.go @@ -452,6 +452,10 @@ func specConfigureNamespaces(s *specgen.SpecGenerator, g *generate.Generator, rt func GetNamespaceOptions(ns []string) ([]libpod.PodCreateOption, error) { var options []libpod.PodCreateOption var erroredOptions []libpod.PodCreateOption + if ns == nil { + //set the default namespaces + ns = strings.Split(specgen.DefaultKernelNamespaces, ",") + } for _, toShare := range ns { switch toShare { case "cgroup": diff --git a/test/e2e/run_test.go b/test/e2e/run_test.go index d2950ed43..95eecd042 100644 --- a/test/e2e/run_test.go +++ b/test/e2e/run_test.go @@ -797,7 +797,11 @@ USER mail` }) It("podman run --pod automatically", func() { - session := podmanTest.Podman([]string{"run", "--pod", "new:foobar", ALPINE, "ls"}) + session := podmanTest.Podman([]string{"run", "-d", "--pod", "new:foobar", ALPINE, "nc", "-l", "-p", "8080"}) + session.WaitWithDefaultTimeout() + Expect(session.ExitCode()).To(Equal(0)) + + session = podmanTest.Podman([]string{"run", "--pod", "foobar", ALPINE, "/bin/sh", "-c", "echo test | nc -w 1 127.0.0.1 8080"}) session.WaitWithDefaultTimeout() Expect(session.ExitCode()).To(Equal(0)) -- cgit v1.2.3-54-g00ecf From 3dfd8630a51a37734ad8c51162c4d004b8ffffb2 Mon Sep 17 00:00:00 2001 From: Daniel J Walsh Date: Tue, 30 Jun 2020 15:44:14 -0400 Subject: Add username to /etc/passwd inside of container if --userns keep-id If I enter a continer with --userns keep-id, my UID will be present inside of the container, but most likely my user will not be defined. This patch will take information about the user and stick it into the container. Signed-off-by: Daniel J Walsh --- cmd/podman/common/specgen.go | 2 ++ libpod/container.go | 8 ++++- libpod/container_internal_linux.go | 58 ++++++++++++++++++++++++++++----- libpod/container_internal_linux_test.go | 42 ++++++++++++++++++++++++ libpod/options.go | 14 ++++++++ pkg/specgen/generate/namespaces.go | 4 ++- test/e2e/run_userns_test.go | 10 ++++++ 7 files changed, 128 insertions(+), 10 deletions(-) create mode 100644 libpod/container_internal_linux_test.go (limited to 'test/e2e') diff --git a/cmd/podman/common/specgen.go b/cmd/podman/common/specgen.go index 7716fc150..2333f2f7e 100644 --- a/cmd/podman/common/specgen.go +++ b/cmd/podman/common/specgen.go @@ -260,6 +260,8 @@ func FillOutSpecGen(s *specgen.SpecGenerator, c *ContainerCLIOpts, args []string // If some mappings are specified, assume a private user namespace if userNS.IsDefaultValue() && (!s.IDMappings.HostUIDMapping || !s.IDMappings.HostGIDMapping) { s.UserNS.NSMode = specgen.Private + } else { + s.UserNS.NSMode = specgen.NamespaceMode(userNS) } s.Terminal = c.TTY diff --git a/libpod/container.go b/libpod/container.go index 6a87ad3a9..9ad938a5c 100644 --- a/libpod/container.go +++ b/libpod/container.go @@ -277,6 +277,9 @@ type ContainerConfig struct { User string `json:"user,omitempty"` // Additional groups to add Groups []string `json:"groups,omitempty"` + // AddCurrentUserPasswdEntry indicates that the current user passwd entry + // should be added to the /etc/passwd within the container + AddCurrentUserPasswdEntry bool `json:"addCurrentUserPasswdEntry,omitempty"` // Namespace Config // IDs of container to share namespaces with @@ -774,7 +777,10 @@ func (c *Container) Hostname() string { // WorkingDir returns the containers working dir func (c *Container) WorkingDir() string { - return c.config.Spec.Process.Cwd + if c.config.Spec.Process != nil { + return c.config.Spec.Process.Cwd + } + return "/" } // State Accessors diff --git a/libpod/container_internal_linux.go b/libpod/container_internal_linux.go index cfcf9b823..f86903fd1 100644 --- a/libpod/container_internal_linux.go +++ b/libpod/container_internal_linux.go @@ -9,6 +9,7 @@ import ( "io/ioutil" "net" "os" + "os/user" "path" "path/filepath" "strconv" @@ -33,7 +34,7 @@ import ( "github.com/containers/libpod/v2/pkg/util" "github.com/containers/storage/pkg/archive" securejoin "github.com/cyphar/filepath-securejoin" - "github.com/opencontainers/runc/libcontainer/user" + User "github.com/opencontainers/runc/libcontainer/user" spec "github.com/opencontainers/runtime-spec/specs-go" "github.com/opencontainers/runtime-tools/generate" "github.com/opencontainers/selinux/go-selinux/label" @@ -1420,9 +1421,23 @@ func (c *Container) getHosts() string { return hosts } -// generatePasswd generates a container specific passwd file, -// iff g.config.User is a number -func (c *Container) generatePasswd() (string, error) { +// generateCurrentUserPasswdEntry generates an /etc/passwd entry for the user +// running the container engine +func (c *Container) generateCurrentUserPasswdEntry() (string, error) { + uid := rootless.GetRootlessUID() + if uid == 0 { + return "", nil + } + u, err := user.LookupId(strconv.Itoa(rootless.GetRootlessUID())) + if err != nil { + return "", errors.Wrapf(err, "failed to get current user") + } + return fmt.Sprintf("%s:x:%s:%s:%s:%s:/bin/sh\n", u.Username, u.Uid, u.Gid, u.Username, c.WorkingDir()), nil +} + +// generateUserPasswdEntry generates an /etc/passwd entry for the container user +// to run in the container. +func (c *Container) generateUserPasswdEntry() (string, error) { var ( groupspec string gid int @@ -1440,14 +1455,16 @@ func (c *Container) generatePasswd() (string, error) { if err != nil { return "", nil } + // Lookup the user to see if it exists in the container image _, err = lookup.GetUser(c.state.Mountpoint, userspec) - if err != nil && err != user.ErrNoPasswdEntries { + if err != nil && err != User.ErrNoPasswdEntries { return "", err } if err == nil { return "", nil } + if groupspec != "" { ugid, err := strconv.ParseUint(groupspec, 10, 32) if err == nil { @@ -1460,14 +1477,39 @@ func (c *Container) generatePasswd() (string, error) { gid = group.Gid } } + return fmt.Sprintf("%d:x:%d:%d:container user:%s:/bin/sh\n", uid, uid, gid, c.WorkingDir()), nil +} + +// generatePasswd generates a container specific passwd file, +// iff g.config.User is a number +func (c *Container) generatePasswd() (string, error) { + if !c.config.AddCurrentUserPasswdEntry && c.config.User == "" { + return "", nil + } + pwd := "" + if c.config.User != "" { + entry, err := c.generateUserPasswdEntry() + if err != nil { + return "", err + } + pwd += entry + } + if c.config.AddCurrentUserPasswdEntry { + entry, err := c.generateCurrentUserPasswdEntry() + if err != nil { + return "", err + } + pwd += entry + } + if pwd == "" { + return "", nil + } originPasswdFile := filepath.Join(c.state.Mountpoint, "/etc/passwd") orig, err := ioutil.ReadFile(originPasswdFile) if err != nil && !os.IsNotExist(err) { return "", errors.Wrapf(err, "unable to read passwd file %s", originPasswdFile) } - - pwd := fmt.Sprintf("%s%d:x:%d:%d:container user:%s:/bin/sh\n", orig, uid, uid, gid, c.WorkingDir()) - passwdFile, err := c.writeStringToRundir("passwd", pwd) + passwdFile, err := c.writeStringToRundir("passwd", string(orig)+pwd) if err != nil { return "", errors.Wrapf(err, "failed to create temporary passwd file") } diff --git a/libpod/container_internal_linux_test.go b/libpod/container_internal_linux_test.go new file mode 100644 index 000000000..078cc53a7 --- /dev/null +++ b/libpod/container_internal_linux_test.go @@ -0,0 +1,42 @@ +// +build linux + +package libpod + +import ( + "io/ioutil" + "os" + "testing" + + spec "github.com/opencontainers/runtime-spec/specs-go" + "github.com/stretchr/testify/assert" +) + +func TestGenerateUserPasswdEntry(t *testing.T) { + dir, err := ioutil.TempDir("", "libpod_test_") + if err != nil { + t.Fatal(err) + } + defer os.RemoveAll(dir) + + c := Container{ + config: &ContainerConfig{ + User: "123:456", + Spec: &spec.Spec{}, + }, + state: &ContainerState{ + Mountpoint: "/does/not/exist/tmp/", + }, + } + user, err := c.generateUserPasswdEntry() + if err != nil { + t.Fatal(err) + } + assert.Equal(t, user, "123:x:123:456:container user:/:/bin/sh\n") + + c.config.User = "567" + user, err = c.generateUserPasswdEntry() + if err != nil { + t.Fatal(err) + } + assert.Equal(t, user, "567:x:567:0:container user:/:/bin/sh\n") +} diff --git a/libpod/options.go b/libpod/options.go index bff3f3c18..560b406e2 100644 --- a/libpod/options.go +++ b/libpod/options.go @@ -844,6 +844,20 @@ func WithPIDNSFrom(nsCtr *Container) CtrCreateOption { } } +// WithAddCurrentUserPasswdEntry indicates that container should add current +// user entry to /etc/passwd, since the UID will be mapped into the container, +// via user namespace +func WithAddCurrentUserPasswdEntry() CtrCreateOption { + return func(ctr *Container) error { + if ctr.valid { + return define.ErrCtrFinalized + } + + ctr.config.AddCurrentUserPasswdEntry = true + return nil + } +} + // WithUserNSFrom indicates the the container should join the user namespace of // the given container. // If the container has joined a pod, it can only join the namespaces of diff --git a/pkg/specgen/generate/namespaces.go b/pkg/specgen/generate/namespaces.go index 39a45398d..22670ca61 100644 --- a/pkg/specgen/generate/namespaces.go +++ b/pkg/specgen/generate/namespaces.go @@ -153,7 +153,9 @@ func namespaceOptions(ctx context.Context, s *specgen.SpecGenerator, rt *libpod. // User switch s.UserNS.NSMode { case specgen.KeepID: - if !rootless.IsRootless() { + if rootless.IsRootless() { + toReturn = append(toReturn, libpod.WithAddCurrentUserPasswdEntry()) + } else { // keep-id as root doesn't need a user namespace s.UserNS.NSMode = specgen.Host } diff --git a/test/e2e/run_userns_test.go b/test/e2e/run_userns_test.go index c0d98f7b1..24d3e42eb 100644 --- a/test/e2e/run_userns_test.go +++ b/test/e2e/run_userns_test.go @@ -89,6 +89,16 @@ var _ = Describe("Podman UserNS support", func() { Expect(ok).To(BeTrue()) }) + It("podman --userns=keep-id check passwd", func() { + session := podmanTest.Podman([]string{"run", "--userns=keep-id", "alpine", "id", "-un"}) + session.WaitWithDefaultTimeout() + Expect(session.ExitCode()).To(Equal(0)) + u, err := user.Current() + Expect(err).To(BeNil()) + ok, _ := session.GrepString(u.Name) + Expect(ok).To(BeTrue()) + }) + It("podman --userns=keep-id root owns /usr", func() { session := podmanTest.Podman([]string{"run", "--userns=keep-id", "alpine", "stat", "-c%u", "/usr"}) session.WaitWithDefaultTimeout() -- cgit v1.2.3-54-g00ecf From ca00601b14f2253e5b9f89280d39c5d498efc9f3 Mon Sep 17 00:00:00 2001 From: Matthew Heon Date: Wed, 15 Jul 2020 15:25:12 -0400 Subject: Make changes to /etc/passwd on disk for non-read only Bind-mounting /etc/passwd into the container is problematic becuase of how system utilities like `useradd` work. They want to make a copy and then rename to try to prevent breakage; this is, unfortunately, impossible when the file they want to rename is a bind mount. The current behavior is fine for read-only containers, though, because we expect useradd to fail in those cases. Instead of bind-mounting, we can edit /etc/passwd in the container's rootfs. This is kind of gross, because the change will show up in `podman diff` and similar tools, and will be included in images made by `podman commit`. However, it's a lot better than breaking important system tools. Fixes #6953 Signed-off-by: Matthew Heon --- libpod/container_internal_linux.go | 48 ++++++++++++++++++++++++++++++++++---- test/e2e/run_passwd_test.go | 8 +++---- test/e2e/run_userns_test.go | 25 ++++++++++++++++++++ 3 files changed, 72 insertions(+), 9 deletions(-) (limited to 'test/e2e') diff --git a/libpod/container_internal_linux.go b/libpod/container_internal_linux.go index f86903fd1..d72e3ad47 100644 --- a/libpod/container_internal_linux.go +++ b/libpod/container_internal_linux.go @@ -1428,11 +1428,26 @@ func (c *Container) generateCurrentUserPasswdEntry() (string, error) { if uid == 0 { return "", nil } + u, err := user.LookupId(strconv.Itoa(rootless.GetRootlessUID())) if err != nil { return "", errors.Wrapf(err, "failed to get current user") } - return fmt.Sprintf("%s:x:%s:%s:%s:%s:/bin/sh\n", u.Username, u.Uid, u.Gid, u.Username, c.WorkingDir()), nil + + // Lookup the user to see if it exists in the container image. + _, err = lookup.GetUser(c.state.Mountpoint, u.Username) + if err != User.ErrNoPasswdEntries { + return "", err + } + + // If the user's actual home directory exists, or was mounted in - use + // that. + homeDir := c.WorkingDir() + if MountExists(c.config.Spec.Mounts, u.HomeDir) { + homeDir = u.HomeDir + } + + return fmt.Sprintf("%s:x:%s:%s:%s:%s:/bin/sh\n", u.Username, u.Uid, u.Gid, u.Username, homeDir), nil } // generateUserPasswdEntry generates an /etc/passwd entry for the container user @@ -1458,12 +1473,9 @@ func (c *Container) generateUserPasswdEntry() (string, error) { // Lookup the user to see if it exists in the container image _, err = lookup.GetUser(c.state.Mountpoint, userspec) - if err != nil && err != User.ErrNoPasswdEntries { + if err != User.ErrNoPasswdEntries { return "", err } - if err == nil { - return "", nil - } if groupspec != "" { ugid, err := strconv.ParseUint(groupspec, 10, 32) @@ -1504,6 +1516,32 @@ func (c *Container) generatePasswd() (string, error) { if pwd == "" { return "", nil } + + // If we are *not* read-only - edit /etc/passwd in the container. + // This is *gross* (shows up in changes to the container, will be + // committed to images based on the container) but it actually allows us + // to add users to the container (a bind mount breaks useradd). + // We should never get here twice, because generateUserPasswdEntry will + // not return anything if the user already exists in /etc/passwd. + if !c.IsReadOnly() { + containerPasswd, err := securejoin.SecureJoin(c.state.Mountpoint, "/etc/passwd") + if err != nil { + return "", errors.Wrapf(err, "error looking up location of container %s /etc/passwd", c.ID()) + } + + f, err := os.OpenFile(containerPasswd, os.O_APPEND|os.O_WRONLY, 0600) + if err != nil { + return "", errors.Wrapf(err, "error opening container %s /etc/passwd", c.ID()) + } + defer f.Close() + + if _, err := f.WriteString(pwd); err != nil { + return "", errors.Wrapf(err, "unable to append to container %s /etc/passwd", c.ID()) + } + + return "", nil + } + originPasswdFile := filepath.Join(c.state.Mountpoint, "/etc/passwd") orig, err := ioutil.ReadFile(originPasswdFile) if err != nil && !os.IsNotExist(err) { diff --git a/test/e2e/run_passwd_test.go b/test/e2e/run_passwd_test.go index d7f3233ef..6caf8a094 100644 --- a/test/e2e/run_passwd_test.go +++ b/test/e2e/run_passwd_test.go @@ -35,27 +35,27 @@ var _ = Describe("Podman run passwd", func() { }) It("podman run no user specified ", func() { - session := podmanTest.Podman([]string{"run", BB, "mount"}) + session := podmanTest.Podman([]string{"run", "--read-only", BB, "mount"}) session.WaitWithDefaultTimeout() Expect(session.ExitCode()).To(Equal(0)) Expect(session.LineInOutputContains("passwd")).To(BeFalse()) }) It("podman run user specified in container", func() { - session := podmanTest.Podman([]string{"run", "-u", "bin", BB, "mount"}) + session := podmanTest.Podman([]string{"run", "--read-only", "-u", "bin", BB, "mount"}) session.WaitWithDefaultTimeout() Expect(session.ExitCode()).To(Equal(0)) Expect(session.LineInOutputContains("passwd")).To(BeFalse()) }) It("podman run UID specified in container", func() { - session := podmanTest.Podman([]string{"run", "-u", "2:1", BB, "mount"}) + session := podmanTest.Podman([]string{"run", "--read-only", "-u", "2:1", BB, "mount"}) session.WaitWithDefaultTimeout() Expect(session.ExitCode()).To(Equal(0)) Expect(session.LineInOutputContains("passwd")).To(BeFalse()) }) It("podman run UID not specified in container", func() { - session := podmanTest.Podman([]string{"run", "-u", "20001:1", BB, "mount"}) + session := podmanTest.Podman([]string{"run", "--read-only", "-u", "20001:1", BB, "mount"}) session.WaitWithDefaultTimeout() Expect(session.ExitCode()).To(Equal(0)) Expect(session.LineInOutputContains("passwd")).To(BeTrue()) diff --git a/test/e2e/run_userns_test.go b/test/e2e/run_userns_test.go index 24d3e42eb..fef125e60 100644 --- a/test/e2e/run_userns_test.go +++ b/test/e2e/run_userns_test.go @@ -113,6 +113,31 @@ var _ = Describe("Podman UserNS support", func() { Expect(session.OutputToString()).To(Equal("0")) }) + It("podman run --userns=keep-id can add users", func() { + if os.Geteuid() == 0 { + Skip("Test only runs without root") + } + + userName := os.Getenv("USER") + if userName == "" { + Skip("Can't complete test if no username available") + } + + ctrName := "ctr-name" + session := podmanTest.Podman([]string{"run", "--userns=keep-id", "--user", "root:root", "-d", "--stop-signal", "9", "--name", ctrName, fedoraMinimal, "sleep", "600"}) + session.WaitWithDefaultTimeout() + Expect(session.ExitCode()).To(Equal(0)) + + exec1 := podmanTest.Podman([]string{"exec", "-t", "-i", ctrName, "cat", "/etc/passwd"}) + exec1.WaitWithDefaultTimeout() + Expect(exec1.ExitCode()).To(Equal(0)) + Expect(exec1.OutputToString()).To(ContainSubstring(userName)) + + exec2 := podmanTest.Podman([]string{"exec", "-t", "-i", ctrName, "useradd", "testuser"}) + exec2.WaitWithDefaultTimeout() + Expect(exec2.ExitCode()).To(Equal(0)) + }) + It("podman --userns=auto", func() { u, err := user.Current() Expect(err).To(BeNil()) -- cgit v1.2.3-54-g00ecf