gin-gonic / gin

Gin is a HTTP web framework written in Go (Golang). It features a Martini-like API with much better performance -- up to 40 times faster. If you need smashing performance, get yourself some Gin.
https://gin-gonic.com/
MIT License
78.57k stars 8.01k forks source link

Engine.Handler() should initialize HTTP2 server Read/WriteTimeout to adequate Values #3410

Open xaviercrochet opened 1 year ago

xaviercrochet commented 1 year ago

Description

Engine.Handler() initialises http2.Server without setting ReadTimeout and WriteTimeout to adequate values, which leaves them to their default off values.

This expose the http server to slowloris DDoS attacks when dealing with untrusted clients.

You would be leaking connections and run out of descriptors..

How to reproduce

You start a new engine with any of the Run methods.

(...)
gin.SetMode(gin.ReleaseMode)
router = gin.New()
router.RunTLS("some ost url", "some ssl certificate",  "some ssl key")
(...)

Expectations

The method Engine.Handler() should initialise the http server with proper timeout values so it doesn't expose it to the issue explained above.

i.e.

server := &http2.Server{
  (...),
  ReadTimeout:  5 * time.Second,
  WriteTimeout: 10 * time.Second,
  (...),
}

Actual result

func (engine *Engine) Handler() http.Handler {
    if !engine.UseH2C {
        return engine
    }

    h2s := &http2.Server{}
    return h2c.NewHandler(engine, h2s)
}

Environment

Cookiery commented 1 year ago

I think you can create a timeout middleware. Reference: https://gist.github.com/montanaflynn/ef9e7b9cd21b355cfe8332b4f20163c1

araujo88 commented 1 year ago

http2.Server doesn't have a ReadTimeout or WriteTimeout fields, but it does have a IdleTimeout field. Should this field be set to a default value when calling Engine.Handler()?

func (engine *Engine) Handler() http.Handler {
    if !engine.UseH2C {
        return engine
    }

    h2s := &http2.Server{
        IdleTimeout: 10 * time.Second, // sets IdleTimeout default value to 10 seconds
    }
    return h2c.NewHandler(engine, h2s)
}
FirePing32 commented 6 months ago

@appleboy shouldn't we provide a way to call the handler with user-defined params for such cases?