summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorIgor Drozdov <idrozdov@gitlab.com>2022-04-25 16:30:55 +0400
committerIgor Drozdov <idrozdov@gitlab.com>2022-04-25 16:46:09 +0400
commit9fc056c0bb3fca68f642a48c5b7864456226d6ac (patch)
treec8b403fa10a0d45684e16653776436e37d00b898
parentc0953bdbc1af5b547d1130cb152e46d83e202186 (diff)
downloadgitlab-shell-9fc056c0bb3fca68f642a48c5b7864456226d6ac.tar.gz
Revert "Abort long-running unauthenticated SSH connections"
This reverts commit 3a2c8f2c47774a35d840ec8baf54341beede5d43.
-rw-r--r--config.yml.example2
-rw-r--r--internal/config/config.go6
-rw-r--r--internal/sshd/sshd.go17
-rw-r--r--internal/sshd/sshd_test.go29
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)