[FEATURE] Buffer size configuration and additional http error handling (#944)

* implement read buffer size config option
* implement write buffer size config option
* implement fasthttp ErrorHandler so we can log errors to Authelia as well
* add struct/schema validation
* add default value
* add docs
* add config key to validator
* refactoring
* apply suggestions from code review

Co-authored-by: Amir Zarrinkafsh <nightah@me.com>

config.template.yml

@@ -8,6 +8,15 @@ port: 9091
 # tls_key: /var/lib/authelia/ssl/key.pem
 # tls_cert: /var/lib/authelia/ssl/cert.pem
 
+# Configuration options specific to the internal http server
+server:
+  # Buffers usually should be configured to be the same value.
+  # Explanation at https://docs.authelia.com/configuration/server.html
+  # Read buffer size configures the http server's maximum incoming request size in bytes.
+  read_buffer_size: 4096
+  # Write buffer size configures the http server's maximum outgoing response size in bytes.
+  write_buffer_size: 4096
+
 # Level of verbosity for logs: info, debug, trace
 log_level: debug
 ## File path where the logs will be written. If not set logs are written to stdout.

docs/configuration/server.md

@@ -0,0 +1,29 @@
+---
+layout: default
+title: Server
+parent: Configuration
+nav_order: 9
+---
+
+# Server
+
+The server section configures and tunes the http server module Authelia uses.
+
+## Configuration
+
+```yaml
+# Configuration options specific to the internal http server
+server:
+  # Buffers usually should be configured to be the same value.
+  # Explanation at https://docs.authelia.com/configuration/server.html
+  # Read buffer size configures the http server's maximum incoming request size in bytes.
+  read_buffer_size: 4096
+  # Write buffer size configures the http server's maximum outgoing response size in bytes.
+  write_buffer_size: 4096
+```
+
+### Buffer Sizes
+
+The read and write buffer sizes generally should be the same. This is because when Authelia verifies 
+if the user is authorized to visit a URL, it also sends back nearly the same size response 
+(write_buffer_size) as the request (read_buffer_size).

docs/configuration/session.md

@@ -2,7 +2,7 @@
 layout: default
 title: Session
 parent: Configuration
-nav_order: 9
+nav_order: 10
 ---
 
 # Session

internal/configuration/schema/configuration.go

@@ -2,27 +2,24 @@ package schema
 
 // Configuration object extracted from YAML configuration file.
 type Configuration struct {
-	Host    string `mapstructure:"host"`
+	Host                      string `mapstructure:"host"`
-	Port    int    `mapstructure:"port"`
+	Port                      int    `mapstructure:"port"`
-	TLSCert string `mapstructure:"tls_cert"`
+	TLSCert                   string `mapstructure:"tls_cert"`
-	TLSKey  string `mapstructure:"tls_key"`
+	TLSKey                    string `mapstructure:"tls_key"`
-
+	LogLevel                  string `mapstructure:"log_level"`
-	LogLevel    string `mapstructure:"log_level"`
+	LogFilePath               string `mapstructure:"log_file_path"`
-	LogFilePath string `mapstructure:"log_file_path"`
-
-	// This secret is used by the identity validation process to forge JWT tokens
-	// representing the permission to proceed with the operation.
 	JWTSecret                 string `mapstructure:"jwt_secret"`
 	DefaultRedirectionURL     string `mapstructure:"default_redirection_url"`
 	GoogleAnalyticsTrackingID string `mapstructure:"google_analytics"`
 
+	// TODO: Consider refactoring the following pointers as they don't seem to need to be pointers: TOTP, Notifier, Regulation
 	AuthenticationBackend AuthenticationBackendConfiguration `mapstructure:"authentication_backend"`
 	Session               SessionConfiguration               `mapstructure:"session"`
-
+	TOTP                  *TOTPConfiguration                 `mapstructure:"totp"`
-	TOTP          *TOTPConfiguration         `mapstructure:"totp"`
+	DuoAPI                *DuoAPIConfiguration               `mapstructure:"duo_api"`
-	DuoAPI        *DuoAPIConfiguration       `mapstructure:"duo_api"`
+	AccessControl         AccessControlConfiguration         `mapstructure:"access_control"`
-	AccessControl AccessControlConfiguration `mapstructure:"access_control"`
+	Regulation            *RegulationConfiguration           `mapstructure:"regulation"`
-	Regulation    *RegulationConfiguration   `mapstructure:"regulation"`
+	Storage               StorageConfiguration               `mapstructure:"storage"`
-	Storage       StorageConfiguration       `mapstructure:"storage"`
+	Notifier              *NotifierConfiguration             `mapstructure:"notifier"`
-	Notifier      *NotifierConfiguration     `mapstructure:"notifier"`
+	Server                ServerConfiguration                `mapstructure:"server"`
 }

internal/configuration/schema/server.go

@@ -0,0 +1,13 @@
+package schema
+
+// ServerConfiguration represents the configuration of the http server.
+type ServerConfiguration struct {
+	ReadBufferSize  int `mapstructure:"read_buffer_size"`
+	WriteBufferSize int `mapstructure:"write_buffer_size"`
+}
+
+// DefaultServerConfiguration represents the default values of the ServerConfiguration.
+var DefaultServerConfiguration = ServerConfiguration{
+	ReadBufferSize:  4096,
+	WriteBufferSize: 4096,
+}

internal/configuration/validator/configuration.go

@@ -11,6 +11,7 @@ var defaultPort = 8080
 var defaultLogLevel = "info"
 
 // ValidateConfiguration and adapt the configuration read from file.
+//nolint:gocyclo // This function is likely to always have lots of if/else statements, as long as we keep the flow clean it should be understandable
 func ValidateConfiguration(configuration *schema.Configuration, validator *schema.StructValidator) {
 	if configuration.Host == "" {
 		configuration.Host = "0.0.0.0"
@@ -42,7 +43,7 @@ func ValidateConfiguration(configuration *schema.Configuration, validator *schem
 	}
 
 	if configuration.TOTP == nil {
-		configuration.TOTP = &schema.TOTPConfiguration{}
+		configuration.TOTP = &schema.DefaultTOTPConfiguration
 	}
 	ValidateTOTP(configuration.TOTP, validator)
 
@@ -55,10 +56,12 @@ func ValidateConfiguration(configuration *schema.Configuration, validator *schem
 	ValidateSession(&configuration.Session, validator)
 
 	if configuration.Regulation == nil {
-		configuration.Regulation = &schema.RegulationConfiguration{}
+		configuration.Regulation = &schema.DefaultRegulationConfiguration
 	}
 	ValidateRegulation(configuration.Regulation, validator)
 
+	ValidateServer(&configuration.Server, validator)
+
 	ValidateStorage(configuration.Storage, validator)
 
 	if configuration.Notifier == nil {

internal/configuration/validator/const.go

@@ -12,6 +12,10 @@ var validKeys = []string{
 	"tls_cert",
 	"google_analytics",
 
+	// Server Keys.
+	"server.read_buffer_size",
+	"server.write_buffer_size",
+
 	// TOTP Keys
 	"totp.issuer",
 	"totp.period",

internal/configuration/validator/server.go

@@ -0,0 +1,25 @@
+package validator
+
+import (
+	"fmt"
+
+	"github.com/authelia/authelia/internal/configuration/schema"
+)
+
+var defaultReadBufferSize = 4096
+var defaultWriteBufferSize = 4096
+
+// ValidateServer checks a server configuration is correct.
+func ValidateServer(configuration *schema.ServerConfiguration, validator *schema.StructValidator) {
+	if configuration.ReadBufferSize == 0 {
+		configuration.ReadBufferSize = defaultReadBufferSize
+	} else if configuration.ReadBufferSize < 0 {
+		validator.Push(fmt.Errorf("server read buffer size must be above 0"))
+	}
+
+	if configuration.WriteBufferSize == 0 {
+		configuration.WriteBufferSize = defaultWriteBufferSize
+	} else if configuration.WriteBufferSize < 0 {
+		validator.Push(fmt.Errorf("server write buffer size must be above 0"))
+	}
+}

internal/configuration/validator/server_test.go

@@ -0,0 +1,35 @@
+package validator
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+
+	"github.com/authelia/authelia/internal/configuration/schema"
+)
+
+func TestShouldSetDefaultConfig(t *testing.T) {
+	validator := schema.NewStructValidator()
+	config := schema.ServerConfiguration{}
+
+	ValidateServer(&config, validator)
+
+	require.Len(t, validator.Errors(), 0)
+	assert.Equal(t, defaultReadBufferSize, config.ReadBufferSize)
+	assert.Equal(t, defaultWriteBufferSize, config.WriteBufferSize)
+}
+
+func TestShouldRaiseOnNegativeValues(t *testing.T) {
+	validator := schema.NewStructValidator()
+	config := schema.ServerConfiguration{
+		ReadBufferSize:  -1,
+		WriteBufferSize: -1,
+	}
+
+	ValidateServer(&config, validator)
+
+	require.Len(t, validator.Errors(), 2)
+	assert.EqualError(t, validator.Errors()[0], "server read buffer size must be above 0")
+	assert.EqualError(t, validator.Errors()[1], "server write buffer size must be above 0")
+}

internal/server/error_handler.go

@@ -0,0 +1,26 @@
+package server
+
+import (
+	"net"
+
+	"github.com/valyala/fasthttp"
+
+	"github.com/authelia/authelia/internal/logging"
+)
+
+// Replacement for the default error handler in fasthttp.
+func autheliaErrorHandler(ctx *fasthttp.RequestCtx, err error) {
+	if _, ok := err.(*fasthttp.ErrSmallBuffer); ok {
+		// Note: Getting X-Forwarded-For or Request URI is impossible for ths error.
+		logging.Logger().Tracef("Request was too large to handle from client %s. Response Code %d.", ctx.RemoteIP().String(), fasthttp.StatusRequestHeaderFieldsTooLarge)
+		ctx.Error("Request header too large", fasthttp.StatusRequestHeaderFieldsTooLarge)
+	} else if netErr, ok := err.(*net.OpError); ok && netErr.Timeout() {
+		// TODO: Add X-Forwarded-For Check here.
+		logging.Logger().Tracef("Request timeout occurred while handling from client %s: %s. Response Code %d.", ctx.RemoteIP().String(), ctx.RequestURI(), fasthttp.StatusRequestTimeout)
+		ctx.Error("Request timeout", fasthttp.StatusRequestTimeout)
+	} else {
+		// TODO: Add X-Forwarded-For Check here.
+		logging.Logger().Tracef("An unknown error occurred while handling a request from client %s: %s. Response Code %d.", ctx.RemoteIP().String(), ctx.RequestURI(), fasthttp.StatusBadRequest)
+		ctx.Error("Error when parsing request", fasthttp.StatusBadRequest)
+	}
+}

internal/server/server.go

@@ -115,8 +115,12 @@ func StartServer(configuration schema.Configuration, providers middlewares.Provi
 	router.NotFound = ServeIndex(embeddedAssets)
 
 	server := &fasthttp.Server{
-		Handler: middlewares.LogRequestMiddleware(router.Handler),
+		Handler:         middlewares.LogRequestMiddleware(router.Handler),
+		ErrorHandler:    autheliaErrorHandler,
+		ReadBufferSize:  configuration.Server.ReadBufferSize,
+		WriteBufferSize: configuration.Server.WriteBufferSize,
 	}
+
 	addrPattern := fmt.Sprintf("%s:%d", configuration.Host, configuration.Port)
 
 	if configuration.TLSCert != "" && configuration.TLSKey != "" {