From aa3fa9a2bb9911cf27a85e0f1f4fe61dcac6a074 Mon Sep 17 00:00:00 2001 From: Christophe Fergeau Date: Thu, 11 Jan 2024 11:51:09 -0500 Subject: [PATCH 1/2] ssh: Introduce 'retry' helper initialConnection retries multiple times to establish the TCP connection which will be used for ssh communication. This commit adds a generic helper to handle the retry which will be useful in the next commits. Signed-off-by: Christophe Fergeau --- pkg/sshclient/ssh_forwarder.go | 29 ++++++++++++++++++++--------- 1 file changed, 20 insertions(+), 9 deletions(-) diff --git a/pkg/sshclient/ssh_forwarder.go b/pkg/sshclient/ssh_forwarder.go index 38075c230..2cd7ab235 100644 --- a/pkg/sshclient/ssh_forwarder.go +++ b/pkg/sshclient/ssh_forwarder.go @@ -2,6 +2,7 @@ package sshclient import ( "context" + "fmt" "io" "net" "net/url" @@ -180,16 +181,19 @@ func setupProxy(ctx context.Context, socketURI *url.URL, dest *url.URL, identity return &SSHForward{listener, &bastion, socketURI}, nil } -func initialConnection(ctx context.Context, connectFunc ConnectCallback) (net.Conn, error) { +const maxRetries = 60 +const initialBackoff = 100 * time.Millisecond + +func retry[T comparable](ctx context.Context, retryFunc func() (T, error), retryMsg string) (T, error) { var ( - conn net.Conn - err error + returnVal T + err error ) - backoff := 100 * time.Millisecond + backoff := initialBackoff loop: - for i := 0; i < 60; i++ { + for i := 0; i < maxRetries; i++ { select { case <-ctx.Done(): break loop @@ -197,15 +201,22 @@ loop: // proceed } - conn, err = connectFunc(ctx, nil) + returnVal, err = retryFunc() if err == nil { - break + return returnVal, nil } - logrus.Debugf("Waiting for sshd: %s", backoff) + logrus.Debugf("%s (%s)", retryMsg, backoff) sleep(ctx, backoff) backoff = backOff(backoff) } - return conn, err + return returnVal, fmt.Errorf("timeout: %w", err) +} + +func initialConnection(ctx context.Context, connectFunc ConnectCallback) (net.Conn, error) { + retryFunc := func() (net.Conn, error) { + return connectFunc(ctx, nil) + } + return retry(ctx, retryFunc, "Waiting for sshd socket") } func acceptConnection(ctx context.Context, listener net.Listener, bastion *Bastion, socketURI *url.URL) error { From 8357aa4852cc749a46715ef6868f2c1d665ccd21 Mon Sep 17 00:00:00 2001 From: Christophe Fergeau Date: Thu, 11 Jan 2024 11:53:47 -0500 Subject: [PATCH 2/2] ssh: Add when to setupProxy When using podman-machine with applehv, sometimes gvproxy would die shortly after being started. The following happens: - podman starts gvproxy with --listen-vfkit - gvproxy starts and waits for vfkit to create a network connection - podman starts vfkit - vfkit creates the VM and connects to gvproxy - gvproxy resumes its execution, and tries to create the ssh forwards podman asked for on the command line - gvproxy fails to create the ssh forward and exits This happens because setupProxy fails in (*Bastion).reconnect with "ssh: handshake failed: EOF". This is related to https://www.man7.org/linux/man-pages/man8/systemd-user-sessions.8.html even if it's possible to create a TCP connection to the ssh port, sshd/pam won't necessarily allow you to connect at the ssh level. This commit fixes this bug by adding a retry to the calls to CreateBastion() to complement the retries already present in initialConnection(). CreateBastion now returns *Bastion instead of Bastion as the latter type is not `comparable` which is required by the generic `retry` function. Signed-off-by: Christophe Fergeau --- pkg/sshclient/bastion.go | 8 ++++---- pkg/sshclient/ssh_forwarder.go | 9 ++++++--- 2 files changed, 10 insertions(+), 7 deletions(-) diff --git a/pkg/sshclient/bastion.go b/pkg/sshclient/bastion.go index c879bcc33..f10bddda0 100644 --- a/pkg/sshclient/bastion.go +++ b/pkg/sshclient/bastion.go @@ -84,13 +84,13 @@ func HostKey(host string) ssh.PublicKey { return nil } -func CreateBastion(_url *url.URL, passPhrase string, identity string, initial net.Conn, connect ConnectCallback) (Bastion, error) { +func CreateBastion(_url *url.URL, passPhrase string, identity string, initial net.Conn, connect ConnectCallback) (*Bastion, error) { var authMethods []ssh.AuthMethod if len(identity) > 0 { s, err := PublicKey(identity, []byte(passPhrase)) if err != nil { - return Bastion{}, errors.Wrapf(err, "failed to parse identity %q", identity) + return nil, errors.Wrapf(err, "failed to parse identity %q", identity) } authMethods = append(authMethods, ssh.PublicKeys(s)) } @@ -100,7 +100,7 @@ func CreateBastion(_url *url.URL, passPhrase string, identity string, initial ne } if len(authMethods) == 0 { - return Bastion{}, errors.New("No available auth methods") + return nil, errors.New("No available auth methods") } port := _url.Port() @@ -149,7 +149,7 @@ func CreateBastion(_url *url.URL, passPhrase string, identity string, initial ne } bastion := Bastion{nil, config, _url.Hostname(), port, _url.Path, connect} - return bastion, bastion.reconnect(context.Background(), initial) + return &bastion, bastion.reconnect(context.Background(), initial) } func (bastion *Bastion) Reconnect(ctx context.Context) error { diff --git a/pkg/sshclient/ssh_forwarder.go b/pkg/sshclient/ssh_forwarder.go index 2cd7ab235..75c43991d 100644 --- a/pkg/sshclient/ssh_forwarder.go +++ b/pkg/sshclient/ssh_forwarder.go @@ -171,14 +171,17 @@ func setupProxy(ctx context.Context, socketURI *url.URL, dest *url.URL, identity return &SSHForward{}, err } - bastion, err := CreateBastion(dest, passphrase, identity, conn, connectFunc) + createBastion := func() (*Bastion, error) { + return CreateBastion(dest, passphrase, identity, conn, connectFunc) + } + bastion, err := retry(ctx, createBastion, "Waiting for sshd") if err != nil { - return &SSHForward{}, err + return &SSHForward{}, fmt.Errorf("setupProxy failed: %w", err) } logrus.Debugf("Socket forward established: %s -> %s\n", socketURI.Path, dest.Path) - return &SSHForward{listener, &bastion, socketURI}, nil + return &SSHForward{listener, bastion, socketURI}, nil } const maxRetries = 60