From f034804e5d98f70d087c2d577a3fb6f0091e742f Mon Sep 17 00:00:00 2001 From: zeripath Date: Thu, 10 Jun 2021 22:26:32 +0100 Subject: [PATCH] Set self-adjusting deadline for connection writing (#16068) (#16123) In #16055 it appears that the simple 5s deadline doesn't work for large file writes. Now we can't - or at least shouldn't just set no deadline as go will happily let these connections block indefinitely. However, what seems reasonable is to set some minimum rate we expect for writing. This PR suggests the following algorithm: * Every write has a minimum timeout of 5s (adjustable at compile time.) * If there has been a previous write - then consider its previous deadline, add half of the minimum timeout + 2s per kb about to written. * If that new deadline is after the minimum timeout use that. Fix #16055 Signed-off-by: Andrew Thornton Co-authored-by: 6543 <6543@obermui.de> --- custom/conf/app.example.ini | 4 ++ .../doc/advanced/config-cheat-sheet.en-us.md | 6 ++ modules/graceful/server.go | 64 ++++++++++++------- modules/setting/setting.go | 32 ++++++---- modules/ssh/ssh_graceful.go | 3 + 5 files changed, 75 insertions(+), 34 deletions(-) diff --git a/custom/conf/app.example.ini b/custom/conf/app.example.ini index fe4fec7e9..2dc13ce59 100644 --- a/custom/conf/app.example.ini +++ b/custom/conf/app.example.ini @@ -281,6 +281,10 @@ HTTP_PORT = 3000 ; PORT_TO_REDIRECT. REDIRECT_OTHER_PORT = false PORT_TO_REDIRECT = 80 +; Timeout for any write to the connection. (Set to 0 to disable all timeouts.) +PER_WRITE_TIMEOUT = 30s +; Timeout per Kb written to connections. +PER_WRITE_PER_KB_TIMEOUT = 30s ; Permission for unix socket UNIX_SOCKET_PERMISSION = 666 ; Local (DMZ) URL for Gitea workers (such as SSH update) accessing web service. diff --git a/docs/content/doc/advanced/config-cheat-sheet.en-us.md b/docs/content/doc/advanced/config-cheat-sheet.en-us.md index e32112f02..10775515b 100644 --- a/docs/content/doc/advanced/config-cheat-sheet.en-us.md +++ b/docs/content/doc/advanced/config-cheat-sheet.en-us.md @@ -237,6 +237,9 @@ Values containing `#` or `;` must be quoted using `` ` `` or `"""`. most cases you do not need to change the default value. Alter it only if your SSH server node is not the same as HTTP node. Do not set this variable if `PROTOCOL` is set to `unix`. +- `PER_WRITE_TIMEOUT`: **30s**: Timeout for any write to the connection. (Set to 0 to + disable all timeouts.) +- `PER_WRITE_PER_KB_TIMEOUT`: **10s**: Timeout per Kb written to connections. - `DISABLE_SSH`: **false**: Disable SSH feature when it's not available. - `START_SSH_SERVER`: **false**: When enabled, use the built-in SSH server. @@ -260,6 +263,9 @@ Values containing `#` or `;` must be quoted using `` ` `` or `"""`. - `SSH_KEY_TEST_PATH`: **/tmp**: Directory to create temporary files in when testing public keys using ssh-keygen, default is the system temporary directory. - `SSH_KEYGEN_PATH`: **ssh-keygen**: Path to ssh-keygen, default is 'ssh-keygen' which means the shell is responsible for finding out which one to call. - `SSH_EXPOSE_ANONYMOUS`: **false**: Enable exposure of SSH clone URL to anonymous visitors, default is false. +- `SSH_PER_WRITE_TIMEOUT`: **30s**: Timeout for any write to the SSH connections. (Set to + 0 to disable all timeouts.) +- `SSH_PER_WRITE_PER_KB_TIMEOUT`: **10s**: Timeout per Kb written to SSH connections. - `MINIMUM_KEY_SIZE_CHECK`: **true**: Indicate whether to check minimum key size with corresponding type. - `OFFLINE_MODE`: **false**: Disables use of CDN for static files and Gravatar for profile pictures. diff --git a/modules/graceful/server.go b/modules/graceful/server.go index 26195c632..704aa8a2b 100644 --- a/modules/graceful/server.go +++ b/modules/graceful/server.go @@ -17,6 +17,7 @@ import ( "time" "code.gitea.io/gitea/modules/log" + "code.gitea.io/gitea/modules/setting" ) var ( @@ -26,11 +27,12 @@ var ( DefaultWriteTimeOut time.Duration // DefaultMaxHeaderBytes default max header bytes DefaultMaxHeaderBytes int + // PerWriteWriteTimeout timeout for writes + PerWriteWriteTimeout = 30 * time.Second + // PerWriteWriteTimeoutKbTime is a timeout taking account of how much there is to be written + PerWriteWriteTimeoutKbTime = 10 * time.Second ) -// PerWriteWriteTimeout timeout for writes -const PerWriteWriteTimeout = 5 * time.Second - func init() { DefaultMaxHeaderBytes = 0 // use http.DefaultMaxHeaderBytes - which currently is 1 << 20 (1MB) } @@ -40,14 +42,16 @@ type ServeFunction = func(net.Listener) error // Server represents our graceful server type Server struct { - network string - address string - listener net.Listener - wg sync.WaitGroup - state state - lock *sync.RWMutex - BeforeBegin func(network, address string) - OnShutdown func() + network string + address string + listener net.Listener + wg sync.WaitGroup + state state + lock *sync.RWMutex + BeforeBegin func(network, address string) + OnShutdown func() + PerWriteTimeout time.Duration + PerWritePerKbTimeout time.Duration } // NewServer creates a server on network at provided address @@ -58,11 +62,13 @@ func NewServer(network, address, name string) *Server { log.Info("Starting new %s server: %s:%s on PID: %d", name, network, address, os.Getpid()) } srv := &Server{ - wg: sync.WaitGroup{}, - state: stateInit, - lock: &sync.RWMutex{}, - network: network, - address: address, + wg: sync.WaitGroup{}, + state: stateInit, + lock: &sync.RWMutex{}, + network: network, + address: address, + PerWriteTimeout: setting.PerWriteTimeout, + PerWritePerKbTimeout: setting.PerWritePerKbTimeout, } srv.BeforeBegin = func(network, addr string) { @@ -224,9 +230,11 @@ func (wl *wrappedListener) Accept() (net.Conn, error) { closed := int32(0) c = wrappedConn{ - Conn: c, - server: wl.server, - closed: &closed, + Conn: c, + server: wl.server, + closed: &closed, + perWriteTimeout: wl.server.PerWriteTimeout, + perWritePerKbTimeout: wl.server.PerWritePerKbTimeout, } wl.server.wg.Add(1) @@ -249,13 +257,23 @@ func (wl *wrappedListener) File() (*os.File, error) { type wrappedConn struct { net.Conn - server *Server - closed *int32 + server *Server + closed *int32 + deadline time.Time + perWriteTimeout time.Duration + perWritePerKbTimeout time.Duration } func (w wrappedConn) Write(p []byte) (n int, err error) { - if PerWriteWriteTimeout > 0 { - _ = w.Conn.SetWriteDeadline(time.Now().Add(PerWriteWriteTimeout)) + if w.perWriteTimeout > 0 { + minTimeout := time.Duration(len(p)/1024) * w.perWritePerKbTimeout + minDeadline := time.Now().Add(minTimeout).Add(w.perWriteTimeout) + + w.deadline = w.deadline.Add(minTimeout) + if minDeadline.After(w.deadline) { + w.deadline = minDeadline + } + _ = w.Conn.SetWriteDeadline(w.deadline) } return w.Conn.Write(p) } diff --git a/modules/setting/setting.go b/modules/setting/setting.go index 9d27a5d74..b394e7260 100644 --- a/modules/setting/setting.go +++ b/modules/setting/setting.go @@ -117,6 +117,8 @@ var ( GracefulRestartable bool GracefulHammerTime time.Duration StartupTimeout time.Duration + PerWriteTimeout = 30 * time.Second + PerWritePerKbTimeout = 10 * time.Second StaticURLPrefix string AbsoluteAssetURL string @@ -147,18 +149,22 @@ var ( TrustedUserCAKeys []string `ini:"SSH_TRUSTED_USER_CA_KEYS"` TrustedUserCAKeysFile string `ini:"SSH_TRUSTED_USER_CA_KEYS_FILENAME"` TrustedUserCAKeysParsed []gossh.PublicKey `ini:"-"` + PerWriteTimeout time.Duration `ini:"SSH_PER_WRITE_TIMEOUT"` + PerWritePerKbTimeout time.Duration `ini:"SSH_PER_WRITE_PER_KB_TIMEOUT"` }{ - Disabled: false, - StartBuiltinServer: false, - Domain: "", - Port: 22, - ServerCiphers: []string{"aes128-ctr", "aes192-ctr", "aes256-ctr", "aes128-gcm@openssh.com", "arcfour256", "arcfour128"}, - ServerKeyExchanges: []string{"diffie-hellman-group1-sha1", "diffie-hellman-group14-sha1", "ecdh-sha2-nistp256", "ecdh-sha2-nistp384", "ecdh-sha2-nistp521", "curve25519-sha256@libssh.org"}, - ServerMACs: []string{"hmac-sha2-256-etm@openssh.com", "hmac-sha2-256", "hmac-sha1", "hmac-sha1-96"}, - KeygenPath: "ssh-keygen", - MinimumKeySizeCheck: true, - MinimumKeySizes: map[string]int{"ed25519": 256, "ed25519-sk": 256, "ecdsa": 256, "ecdsa-sk": 256, "rsa": 2048}, - ServerHostKeys: []string{"ssh/gitea.rsa", "ssh/gogs.rsa"}, + Disabled: false, + StartBuiltinServer: false, + Domain: "", + Port: 22, + ServerCiphers: []string{"aes128-ctr", "aes192-ctr", "aes256-ctr", "aes128-gcm@openssh.com", "arcfour256", "arcfour128"}, + ServerKeyExchanges: []string{"diffie-hellman-group1-sha1", "diffie-hellman-group14-sha1", "ecdh-sha2-nistp256", "ecdh-sha2-nistp384", "ecdh-sha2-nistp521", "curve25519-sha256@libssh.org"}, + ServerMACs: []string{"hmac-sha2-256-etm@openssh.com", "hmac-sha2-256", "hmac-sha1", "hmac-sha1-96"}, + KeygenPath: "ssh-keygen", + MinimumKeySizeCheck: true, + MinimumKeySizes: map[string]int{"ed25519": 256, "ed25519-sk": 256, "ecdsa": 256, "ecdsa-sk": 256, "rsa": 2048}, + ServerHostKeys: []string{"ssh/gitea.rsa", "ssh/gogs.rsa"}, + PerWriteTimeout: PerWriteTimeout, + PerWritePerKbTimeout: PerWritePerKbTimeout, } // Security settings @@ -607,6 +613,8 @@ func NewContext() { GracefulRestartable = sec.Key("ALLOW_GRACEFUL_RESTARTS").MustBool(true) GracefulHammerTime = sec.Key("GRACEFUL_HAMMER_TIME").MustDuration(60 * time.Second) StartupTimeout = sec.Key("STARTUP_TIMEOUT").MustDuration(0 * time.Second) + PerWriteTimeout = sec.Key("PER_WRITE_TIMEOUT").MustDuration(PerWriteTimeout) + PerWritePerKbTimeout = sec.Key("PER_WRITE_PER_KB_TIMEOUT").MustDuration(PerWritePerKbTimeout) defaultAppURL := string(Protocol) + "://" + Domain if (Protocol == HTTP && HTTPPort != "80") || (Protocol == HTTPS && HTTPPort != "443") { @@ -772,6 +780,8 @@ func NewContext() { } SSH.ExposeAnonymous = sec.Key("SSH_EXPOSE_ANONYMOUS").MustBool(false) + SSH.PerWriteTimeout = sec.Key("SSH_PER_WRITE_TIMEOUT").MustDuration(PerWriteTimeout) + SSH.PerWritePerKbTimeout = sec.Key("SSH_PER_WRITE_PER_KB_TIMEOUT").MustDuration(PerWritePerKbTimeout) if err = Cfg.Section("oauth2").MapTo(&OAuth2); err != nil { log.Fatal("Failed to OAuth2 settings: %v", err) diff --git a/modules/ssh/ssh_graceful.go b/modules/ssh/ssh_graceful.go index c213aa7b8..08a7c8575 100644 --- a/modules/ssh/ssh_graceful.go +++ b/modules/ssh/ssh_graceful.go @@ -7,12 +7,15 @@ package ssh import ( "code.gitea.io/gitea/modules/graceful" "code.gitea.io/gitea/modules/log" + "code.gitea.io/gitea/modules/setting" "github.com/gliderlabs/ssh" ) func listen(server *ssh.Server) { gracefulServer := graceful.NewServer("tcp", server.Addr, "SSH") + gracefulServer.PerWriteTimeout = setting.SSH.PerWriteTimeout + gracefulServer.PerWritePerKbTimeout = setting.SSH.PerWritePerKbTimeout err := gracefulServer.ListenAndServe(server.Serve) if err != nil {