diff options
author | Patrick Bajao <ebajao@gitlab.com> | 2022-04-26 03:24:35 +0000 |
---|---|---|
committer | Patrick Bajao <ebajao@gitlab.com> | 2022-04-26 03:24:35 +0000 |
commit | b3b2f77eb2deed2bdd16eb6ec7dce6c8c4dcf241 (patch) | |
tree | c8b403fa10a0d45684e16653776436e37d00b898 | |
parent | c0953bdbc1af5b547d1130cb152e46d83e202186 (diff) | |
parent | 9fc056c0bb3fca68f642a48c5b7864456226d6ac (diff) | |
download | gitlab-shell-b3b2f77eb2deed2bdd16eb6ec7dce6c8c4dcf241.tar.gz |
Merge branch 'id-revert-ssh-connection-timeouts' into 'main'
Revert "Abort long-running unauthenticated SSH connections"
See merge request gitlab-org/gitlab-shell!605
-rw-r--r-- | config.yml.example | 2 | ||||
-rw-r--r-- | internal/config/config.go | 6 | ||||
-rw-r--r-- | internal/sshd/sshd.go | 17 | ||||
-rw-r--r-- | internal/sshd/sshd_test.go | 29 |
4 files changed, 1 insertions, 53 deletions
diff --git a/config.yml.example b/config.yml.example index f3545ba..3714d73 100644 --- a/config.yml.example +++ b/config.yml.example @@ -84,8 +84,6 @@ sshd: readiness_probe: "/start" # The endpoint that returns 200 OK if the server is alive. Defaults to "/health". liveness_probe: "/health" - # The server disconnects after this time (in seconds) if the user has not successfully logged in. Defaults to 60. - login_grace_time: 60 # SSH host key files. host_key_files: - /run/secrets/ssh-hostkeys/ssh_host_rsa_key diff --git a/internal/config/config.go b/internal/config/config.go index 2d5c29d..c4dc073 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -26,7 +26,6 @@ type ServerConfig struct { ProxyProtocol bool `yaml:"proxy_protocol,omitempty"` WebListen string `yaml:"web_listen,omitempty"` ConcurrentSessionsLimit int64 `yaml:"concurrent_sessions_limit,omitempty"` - LoginGraceTimeSeconds uint64 `yaml:"login_grace_time"` GracePeriodSeconds uint64 `yaml:"grace_period"` ReadinessProbe string `yaml:"readiness_probe"` LivenessProbe string `yaml:"liveness_probe"` @@ -79,7 +78,6 @@ var ( Listen: "[::]:22", WebListen: "localhost:9122", ConcurrentSessionsLimit: 10, - LoginGraceTimeSeconds: 60, GracePeriodSeconds: 10, ReadinessProbe: "/start", LivenessProbe: "/health", @@ -91,10 +89,6 @@ var ( } ) -func (sc *ServerConfig) LoginGraceTime() time.Duration { - return time.Duration(sc.LoginGraceTimeSeconds) * time.Second -} - func (sc *ServerConfig) GracePeriod() time.Duration { return time.Duration(sc.GracePeriodSeconds) * time.Second } diff --git a/internal/sshd/sshd.go b/internal/sshd/sshd.go index 8097e9b..24e4970 100644 --- a/internal/sshd/sshd.go +++ b/internal/sshd/sshd.go @@ -179,12 +179,11 @@ func (s *Server) handleConn(ctx context.Context, nconn net.Conn) { ctxlog.Info("server: handleConn: start") - sconn, chans, reqs, err := s.initSSHConnection(ctx, nconn) + sconn, chans, reqs, err := ssh.NewServerConn(nconn, s.serverConfig.get(ctx)) if err != nil { ctxlog.WithError(err).Error("server: handleConn: failed to initialize SSH connection") return } - go ssh.DiscardRequests(reqs) var establishSessionDuration float64 @@ -211,20 +210,6 @@ func (s *Server) handleConn(ctx context.Context, nconn net.Conn) { }).Info("server: handleConn: done") } -func (s *Server) initSSHConnection(ctx context.Context, nconn net.Conn) (sconn *ssh.ServerConn, chans <-chan ssh.NewChannel, reqs <-chan *ssh.Request, err error) { - timer := time.AfterFunc(s.Config.Server.LoginGraceTime(), func() { - nconn.Close() - }) - defer func() { - // If time.Stop() equals false, that means that AfterFunc has been executed - if !timer.Stop() { - err = fmt.Errorf("initSSHConnection: ssh handshake timeout of %v", s.Config.Server.LoginGraceTime()) - } - }() - - return ssh.NewServerConn(nconn, s.serverConfig.get(ctx)) -} - func unconditionalRequirePolicy(_ net.Addr) (proxyproto.Policy, error) { return proxyproto.REQUIRE, nil } diff --git a/internal/sshd/sshd_test.go b/internal/sshd/sshd_test.go index ce395cf..0c6a8ec 100644 --- a/internal/sshd/sshd_test.go +++ b/internal/sshd/sshd_test.go @@ -3,7 +3,6 @@ package sshd import ( "context" "fmt" - "net" "net/http" "net/http/httptest" "os" @@ -135,33 +134,6 @@ func TestInvalidServerConfig(t *testing.T) { require.Nil(t, s.Shutdown()) } -func TestLoginGraceTime(t *testing.T) { - s := setupServer(t) - - unauthenticatedRequestStatus := make(chan string) - completed := make(chan bool) - - clientCfg := clientConfig(t) - clientCfg.HostKeyCallback = func(_ string, _ net.Addr, _ ssh.PublicKey) error { - unauthenticatedRequestStatus <- "authentication-started" - <-completed // Wait infinitely - - return nil - } - - go func() { - // Start an SSH connection that never ends - ssh.Dial("tcp", serverUrl, clientCfg) - }() - - require.Equal(t, "authentication-started", <-unauthenticatedRequestStatus) - - // Verify that the server shutdowns instantly without waiting for any connection to execute - // That means that the server doesn't have any connections to wait for - require.NoError(t, s.Shutdown()) - verifyStatus(t, s, StatusClosed) -} - func setupServer(t *testing.T) *Server { t.Helper() @@ -211,7 +183,6 @@ func setupServerWithConfig(t *testing.T, cfg *config.Config) *Server { cfg.User = user cfg.Server.Listen = serverUrl cfg.Server.ConcurrentSessionsLimit = 1 - cfg.Server.LoginGraceTimeSeconds = 1 cfg.Server.HostKeyFiles = []string{path.Join(testhelper.TestRoot, "certs/valid/server.key")} s, err := NewServer(cfg) |