From dcedf5f21129c26e5fd558ee66ae168905c88b38 Mon Sep 17 00:00:00 2001 From: Paul Holzinger Date: Tue, 19 Nov 2024 11:25:30 +0100 Subject: [PATCH 1/7] cirrus: set proper DEST_BRANCH for 5.3 Signed-off-by: Paul Holzinger --- .cirrus.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.cirrus.yml b/.cirrus.yml index 107008e70b..b905ef49d2 100644 --- a/.cirrus.yml +++ b/.cirrus.yml @@ -6,7 +6,7 @@ env: #### Global variables used for all tasks #### # Name of the ultimate destination branch for this CI run, PR or post-merge. - DEST_BRANCH: "main" + DEST_BRANCH: "v5.3" # Sane (default) value for GOPROXY and GOSUMDB. GOPROXY: "https://proxy.golang.org,direct" GOSUMDB: "sum.golang.org" From f7877bf9db55f0fd31578ea61c5161e0a0a0a216 Mon Sep 17 00:00:00 2001 From: Giuseppe Scrivano Date: Tue, 12 Nov 2024 16:44:33 +0100 Subject: [PATCH 2/7] spec: clamp rlimits in a userns commit 5ebba75dbd4462da47283b3f018804b7361d52bf implemented this behaviour for rootless users, but the same limitation exists for any user in a user namespace. Change the check to use the clamp to the current values anytime podman runs in a user namespace. Closes: https://github.com/containers/podman/issues/24508 Signed-off-by: Giuseppe Scrivano (cherry picked from commit 0a69aefa41d55d2aa30333d6a4ce76b178d1ed5b) Signed-off-by: Paul Holzinger --- libpod/container_internal_common.go | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/libpod/container_internal_common.go b/libpod/container_internal_common.go index 13701b52b9..4f94c22665 100644 --- a/libpod/container_internal_common.go +++ b/libpod/container_internal_common.go @@ -662,7 +662,6 @@ func (c *Container) generateSpec(ctx context.Context) (s *spec.Spec, cleanupFunc // setup rlimits nofileSet := false nprocSet := false - isRootless := rootless.IsRootless() isRunningInUserNs := unshare.IsRootless() if isRunningInUserNs && g.Config.Process != nil && g.Config.Process.OOMScoreAdj != nil { var err error @@ -682,7 +681,7 @@ func (c *Container) generateSpec(ctx context.Context) (s *spec.Spec, cleanupFunc if !nofileSet { max := rlimT(define.RLimitDefaultValue) current := rlimT(define.RLimitDefaultValue) - if isRootless { + if isRunningInUserNs { var rlimit unix.Rlimit if err := unix.Getrlimit(unix.RLIMIT_NOFILE, &rlimit); err != nil { logrus.Warnf("Failed to return RLIMIT_NOFILE ulimit %q", err) @@ -699,7 +698,7 @@ func (c *Container) generateSpec(ctx context.Context) (s *spec.Spec, cleanupFunc if !nprocSet { max := rlimT(define.RLimitDefaultValue) current := rlimT(define.RLimitDefaultValue) - if isRootless { + if isRunningInUserNs { var rlimit unix.Rlimit if err := unix.Getrlimit(unix.RLIMIT_NPROC, &rlimit); err != nil { logrus.Warnf("Failed to return RLIMIT_NPROC ulimit %q", err) From c49944ea02767c12f1832752043e3760e46c668f Mon Sep 17 00:00:00 2001 From: Paul Holzinger Date: Thu, 14 Nov 2024 16:17:50 +0100 Subject: [PATCH 3/7] connection: ignore errors when parsing ssh_config The new ssh_Config feature doesn't work on my system because the lib fails to parse configs using Match[1]. However Fedora and RHEL based distros seem to ship /etc/ssh/ssh_config.d/50-redhat.conf which contains a Match line thus it always fails to parse and never uses the proper values from my home dir config. [1] https://github.com/kevinburke/ssh_config/issues/6 Signed-off-by: Paul Holzinger (cherry picked from commit 8a5ec2c505392588827d9dad847fd797578d0ffb) --- pkg/bindings/connection.go | 1 + 1 file changed, 1 insertion(+) diff --git a/pkg/bindings/connection.go b/pkg/bindings/connection.go index 0fce6c9a27..aa29062779 100644 --- a/pkg/bindings/connection.go +++ b/pkg/bindings/connection.go @@ -170,6 +170,7 @@ func sshClient(_url *url.URL, uri string, identity string, machine bool) (Connec // ssh_config alias := _url.Hostname() cfg := ssh_config.DefaultUserSettings + cfg.IgnoreErrors = true found := false if val := cfg.Get(alias, "User"); val != "" { userinfo = url.User(val) From 28e7b239ba6fa4a44ab9b24be1a7277fe820aa94 Mon Sep 17 00:00:00 2001 From: Paul Holzinger Date: Thu, 14 Nov 2024 16:44:54 +0100 Subject: [PATCH 4/7] ssh_config: do not overwrite values from config file When we alreadty get a full URL with user, port and identity then we should not read the config file just to overwrite them with wrong values. This is a bad regression for user using * wildcard in their ssh_config as it makes podman machine unusable. Fixes: #24567 Fixes: e523734ab6 ("Add support for ssh_config for connection") Signed-off-by: Paul Holzinger (cherry picked from commit a7120b50b1a57edc5b96abda7a47ed23ec94b5ad) --- pkg/bindings/connection.go | 61 +++++++++++++++++++++------------ pkg/machine/e2e/machine_test.go | 15 +++++++- 2 files changed, 53 insertions(+), 23 deletions(-) diff --git a/pkg/bindings/connection.go b/pkg/bindings/connection.go index aa29062779..9a12cbebe3 100644 --- a/pkg/bindings/connection.go +++ b/pkg/bindings/connection.go @@ -147,20 +147,14 @@ func NewConnectionWithIdentity(ctx context.Context, uri string, identity string, func sshClient(_url *url.URL, uri string, identity string, machine bool) (Connection, error) { var ( - err error + err error + port int ) connection := Connection{ URI: _url, } userinfo := _url.User - if _url.User == nil { - u, err := user.Current() - if err != nil { - return connection, fmt.Errorf("current user could not be determined: %w", err) - } - userinfo = url.User(u.Username) - } - port := 22 + if _url.Port() != "" { port, err = strconv.Atoi(_url.Port()) if err != nil { @@ -172,29 +166,52 @@ func sshClient(_url *url.URL, uri string, identity string, machine bool) (Connec cfg := ssh_config.DefaultUserSettings cfg.IgnoreErrors = true found := false - if val := cfg.Get(alias, "User"); val != "" { - userinfo = url.User(val) - found = true + + if userinfo == nil { + if val := cfg.Get(alias, "User"); val != "" { + userinfo = url.User(val) + found = true + } } + // not in url or ssh_config so default to current user + if userinfo == nil { + u, err := user.Current() + if err != nil { + return connection, fmt.Errorf("current user could not be determined: %w", err) + } + userinfo = url.User(u.Username) + } + if val := cfg.Get(alias, "Hostname"); val != "" { uri = val found = true } - if val := cfg.Get(alias, "Port"); val != "" { - if val != ssh_config.Default("Port") { - port, err = strconv.Atoi(val) - if err != nil { - return connection, fmt.Errorf("port is not an int: %s: %w", val, err) + + if port == 0 { + if val := cfg.Get(alias, "Port"); val != "" { + if val != ssh_config.Default("Port") { + port, err = strconv.Atoi(val) + if err != nil { + return connection, fmt.Errorf("port is not an int: %s: %w", val, err) + } + found = true } - found = true } } - if val := cfg.Get(alias, "IdentityFile"); val != "" { - if val != ssh_config.Default("IdentityFile") { - identity = strings.Trim(val, "\"") - found = true + // not in ssh config or url so use default 22 port + if port == 0 { + port = 22 + } + + if identity == "" { + if val := cfg.Get(alias, "IdentityFile"); val != "" { + if val != ssh_config.Default("IdentityFile") { + identity = strings.Trim(val, "\"") + found = true + } } } + if found { logrus.Debugf("ssh_config alias found: %s", alias) logrus.Debugf(" User: %s", userinfo.Username()) diff --git a/pkg/machine/e2e/machine_test.go b/pkg/machine/e2e/machine_test.go index ee1d57c3d1..7ef0c60549 100644 --- a/pkg/machine/e2e/machine_test.go +++ b/pkg/machine/e2e/machine_test.go @@ -98,6 +98,19 @@ var _ = SynchronizedAfterSuite(func() {}, func() { } }) +// The config does not matter to much for our testing, however we +// would like to be sure podman machine is not effected by certain +// settings as we should be using full URLs anywhere. +// https://github.com/containers/podman/issues/24567 +const sshConfigContent = ` +Host * + User NOT_REAL + Port 9999 +Host 127.0.0.1 + User blah + IdentityFile ~/.ssh/id_ed25519 +` + func setup() (string, *machineTestBuilder) { // Set TMPDIR if this needs a new directory if value, ok := os.LookupEnv("TMPDIR"); ok { @@ -118,7 +131,7 @@ func setup() (string, *machineTestBuilder) { if err != nil { Fail(fmt.Sprintf("failed to create ssh config: %q", err)) } - if _, err := sshConfig.WriteString("IdentitiesOnly=yes"); err != nil { + if _, err := sshConfig.WriteString(sshConfigContent); err != nil { Fail(fmt.Sprintf("failed to write ssh config: %q", err)) } if err := sshConfig.Close(); err != nil { From 93562b4955aaab1100060b2b7b875a188ae13ad0 Mon Sep 17 00:00:00 2001 From: Paul Holzinger Date: Thu, 14 Nov 2024 16:52:27 +0100 Subject: [PATCH 5/7] ssh_config: allow IdentityFile file with tilde The ssh_config can contain a path with ~/ to refer to the home dir like done on shells. Handle that special case and resolve the path correctly so it can be used. Signed-off-by: Paul Holzinger (cherry picked from commit cbb2820a7e1c0cb997b8809c6d5cd43f76e4fd7d) --- pkg/bindings/connection.go | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/pkg/bindings/connection.go b/pkg/bindings/connection.go index 9a12cbebe3..762e77d810 100644 --- a/pkg/bindings/connection.go +++ b/pkg/bindings/connection.go @@ -11,6 +11,7 @@ import ( "net/url" "os" "os/user" + "path/filepath" "strconv" "strings" "time" @@ -205,10 +206,15 @@ func sshClient(_url *url.URL, uri string, identity string, machine bool) (Connec if identity == "" { if val := cfg.Get(alias, "IdentityFile"); val != "" { - if val != ssh_config.Default("IdentityFile") { - identity = strings.Trim(val, "\"") - found = true + identity = strings.Trim(val, "\"") + if strings.HasPrefix(identity, "~/") { + homedir, err := os.UserHomeDir() + if err != nil { + return connection, fmt.Errorf("failed to find home dir: %w", err) + } + identity = filepath.Join(homedir, identity[2:]) } + found = true } } From 4886a0ba645ebad6b7c61d1aa4867b38b4c18f7e Mon Sep 17 00:00:00 2001 From: Paul Holzinger Date: Thu, 14 Nov 2024 16:56:24 +0100 Subject: [PATCH 6/7] only read ssh_config for non machine connections For machine we know we have all the info we need so there is no reason to read and parse another file. Signed-off-by: Paul Holzinger (cherry picked from commit 71f1f52894dd0b549cccf8f95af38bd45b61c914) --- pkg/bindings/connection.go | 107 +++++++++++++++++++------------------ 1 file changed, 56 insertions(+), 51 deletions(-) diff --git a/pkg/bindings/connection.go b/pkg/bindings/connection.go index 762e77d810..abf7ab496e 100644 --- a/pkg/bindings/connection.go +++ b/pkg/bindings/connection.go @@ -162,69 +162,74 @@ func sshClient(_url *url.URL, uri string, identity string, machine bool) (Connec return connection, err } } - // ssh_config - alias := _url.Hostname() - cfg := ssh_config.DefaultUserSettings - cfg.IgnoreErrors = true - found := false - if userinfo == nil { - if val := cfg.Get(alias, "User"); val != "" { - userinfo = url.User(val) + // only parse ssh_config when we are not connecting to a machine + // For machine connections we always have the full URL in the + // system connection so reading the file is just unnecessary. + if !machine { + alias := _url.Hostname() + cfg := ssh_config.DefaultUserSettings + cfg.IgnoreErrors = true + found := false + + if userinfo == nil { + if val := cfg.Get(alias, "User"); val != "" { + userinfo = url.User(val) + found = true + } + } + // not in url or ssh_config so default to current user + if userinfo == nil { + u, err := user.Current() + if err != nil { + return connection, fmt.Errorf("current user could not be determined: %w", err) + } + userinfo = url.User(u.Username) + } + + if val := cfg.Get(alias, "Hostname"); val != "" { + uri = val found = true } - } - // not in url or ssh_config so default to current user - if userinfo == nil { - u, err := user.Current() - if err != nil { - return connection, fmt.Errorf("current user could not be determined: %w", err) + + if port == 0 { + if val := cfg.Get(alias, "Port"); val != "" { + if val != ssh_config.Default("Port") { + port, err = strconv.Atoi(val) + if err != nil { + return connection, fmt.Errorf("port is not an int: %s: %w", val, err) + } + found = true + } + } + } + // not in ssh config or url so use default 22 port + if port == 0 { + port = 22 } - userinfo = url.User(u.Username) - } - if val := cfg.Get(alias, "Hostname"); val != "" { - uri = val - found = true - } - - if port == 0 { - if val := cfg.Get(alias, "Port"); val != "" { - if val != ssh_config.Default("Port") { - port, err = strconv.Atoi(val) - if err != nil { - return connection, fmt.Errorf("port is not an int: %s: %w", val, err) + if identity == "" { + if val := cfg.Get(alias, "IdentityFile"); val != "" { + identity = strings.Trim(val, "\"") + if strings.HasPrefix(identity, "~/") { + homedir, err := os.UserHomeDir() + if err != nil { + return connection, fmt.Errorf("failed to find home dir: %w", err) + } + identity = filepath.Join(homedir, identity[2:]) } found = true } } - } - // not in ssh config or url so use default 22 port - if port == 0 { - port = 22 - } - if identity == "" { - if val := cfg.Get(alias, "IdentityFile"); val != "" { - identity = strings.Trim(val, "\"") - if strings.HasPrefix(identity, "~/") { - homedir, err := os.UserHomeDir() - if err != nil { - return connection, fmt.Errorf("failed to find home dir: %w", err) - } - identity = filepath.Join(homedir, identity[2:]) - } - found = true + if found { + logrus.Debugf("ssh_config alias found: %s", alias) + logrus.Debugf(" User: %s", userinfo.Username()) + logrus.Debugf(" Hostname: %s", uri) + logrus.Debugf(" Port: %d", port) + logrus.Debugf(" IdentityFile: %q", identity) } } - - if found { - logrus.Debugf("ssh_config alias found: %s", alias) - logrus.Debugf(" User: %s", userinfo.Username()) - logrus.Debugf(" Hostname: %s", uri) - logrus.Debugf(" Port: %d", port) - logrus.Debugf(" IdentityFile: %q", identity) - } conn, err := ssh.Dial(&ssh.ConnectionDialOptions{ Host: uri, Identity: identity, From 98353f27ed1798fe6a2b58ae57e57f25260a8b90 Mon Sep 17 00:00:00 2001 From: Paul Holzinger Date: Tue, 19 Nov 2024 11:31:49 +0100 Subject: [PATCH 7/7] docs: add 5.3 as Reference version Signed-off-by: Paul Holzinger (cherry picked from commit e6e9d2c21c4769a9ce07fca234f6ab7419c11580) --- docs/source/Reference.rst | 2 ++ 1 file changed, 2 insertions(+) diff --git a/docs/source/Reference.rst b/docs/source/Reference.rst index 297ec6dfc4..61dcc64dc1 100644 --- a/docs/source/Reference.rst +++ b/docs/source/Reference.rst @@ -7,6 +7,8 @@ Show the API documentation for version: * `latest (main branch) <_static/api.html>`_ +* `version 5.3 <_static/api.html?version=v5.3>`_ + * `version 5.2 <_static/api.html?version=v5.2>`_ * `version 5.1 <_static/api.html?version=v5.1>`_