Skip to content

Commit

Permalink
ssh_config: do not overwrite values from config file
Browse files Browse the repository at this point in the history
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: e523734 ("Add support for ssh_config for connection")

Signed-off-by: Paul Holzinger <[email protected]>
(cherry picked from commit a7120b5)
  • Loading branch information
Luap99 committed Nov 19, 2024
1 parent c49944e commit 28e7b23
Show file tree
Hide file tree
Showing 2 changed files with 53 additions and 23 deletions.
61 changes: 39 additions & 22 deletions pkg/bindings/connection.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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())
Expand Down
15 changes: 14 additions & 1 deletion pkg/machine/e2e/machine_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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 {
Expand Down

0 comments on commit 28e7b23

Please sign in to comment.