Repository: authelia/authelia (go)

Issue:
HTTP 431 error returned for very long request URIs
This is a bit of a weird one, but I have an application that has _very_ long request URIs. When I secure this application with authelia (via ingress-nginx), authelia will return an HTTP 431.

Here is an example (truncated) request URI:
```/accounts/reconcile/9/overview/2020-01-15/2020-02-14?startBalance=-6133.89&endBalance=-2752.65&startDate=2020-01-15&startEnd=2020-02-14&journals%5B%5D=126&journals%5B%5D=125&journals%5B%5D=124&journals%5B%5D=123&journals%5B%5D=122&journals%5B%5D=121&journals%5B%5D=120&journals%5B%5D=117&journals%5B%5D=119&journals%5B%5D=118&journals%5B%5D=116&journals%5B%5D=115&journals%5B%5D=113&journals%5B%5D=112&journals%5B%5D=114&journals%5B%5D=110&journals%5B%5D=111&journals%5B%5D=127&journals%5B%5D=109&journals%5B%5D=108&journals%5B%5D=106&journals%5B%5D=107&journals%5B%5D=105&journals%5B%5D=104&journals%5B%5D=103&journals%5B%5D=102&journals%5B%5D=101&journals%5B%5D=98&journals%5B%5D=100&journals%5B%5D=99&journals%5B%5D=97&journals%5B%5D=96&journals%5B%5D=95&journals%5B%5D=94&journals%5B%5D=93&journals%5B%5D=91&journals%5B%5D=89&journals%5B%5D=92&journals%5B%5D=90&journals%5B%5D=88&journals%5B%5D=87&journals%5B%5D=86&journals%5B%5D=85&journals%5B%5D=84&journals%5B%5D=83&journals%5B%5D=82&journals%5B%5D=81&journals%5B%5D=80&cleared%5B%5D=42```

I confirmed that removing authelia auth allows this app to "work as expected".

I'm currently running `4.15.1` with with `log_level: trace` but not seeing anything in the authelia logs that would explain this. However, via chrome dev tools, ingress-nginx logs and a tcpdump from my authelia container, I can see the 431s from authelia.

A few questions:
* Is it possible to configure the `fasthttp` `ReadBufferSize`? It's a bit of a guess, but I'm thinking this might be limiting my max header size.
* Are there any additional options to increase logging? 
* Any additional data I can provide to further troubleshoot this?

Pull request:
[FEATURE] Buffer size configuration and additional http error handling
Fixes #939 and #518

Patch:
diff --git a/config.template.yml b/config.template.yml
index 881c6d6a0..28fc12739 100644
--- a/config.template.yml
+++ b/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.
diff --git a/docs/configuration/server.md b/docs/configuration/server.md
new file mode 100644
index 000000000..83dd38b3f
--- /dev/null
+++ b/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).
diff --git a/docs/configuration/session.md b/docs/configuration/session.md
index 41790b7a2..ac73b6751 100644
--- a/docs/configuration/session.md
+++ b/docs/configuration/session.md
@@ -2,7 +2,7 @@
 layout: default
 title: Session
 parent: Configuration
-nav_order: 9
+nav_order: 10
 ---
 
 # Session
diff --git a/internal/configuration/schema/configuration.go b/internal/configuration/schema/configuration.go
index f4178ca7e..da6fc047d 100644
--- a/internal/configuration/schema/configuration.go
+++ b/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"`
-	Port    int    `mapstructure:"port"`
-	TLSCert string `mapstructure:"tls_cert"`
-	TLSKey  string `mapstructure:"tls_key"`
-
-	LogLevel    string `mapstructure:"log_level"`
-	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.
+	Host                      string `mapstructure:"host"`
+	Port                      int    `mapstructure:"port"`
+	TLSCert                   string `mapstructure:"tls_cert"`
+	TLSKey                    string `mapstructure:"tls_key"`
+	LogLevel                  string `mapstructure:"log_level"`
+	LogFilePath               string `mapstructure:"log_file_path"`
 	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"`
-	DuoAPI        *DuoAPIConfiguration       `mapstructure:"duo_api"`
-	AccessControl AccessControlConfiguration `mapstructure:"access_control"`
-	Regulation    *RegulationConfiguration   `mapstructure:"regulation"`
-	Storage       StorageConfiguration       `mapstructure:"storage"`
-	Notifier      *NotifierConfiguration     `mapstructure:"notifier"`
+	TOTP                  *TOTPConfiguration                 `mapstructure:"totp"`
+	DuoAPI                *DuoAPIConfiguration               `mapstructure:"duo_api"`
+	AccessControl         AccessControlConfiguration         `mapstructure:"access_control"`
+	Regulation            *RegulationConfiguration           `mapstructure:"regulation"`
+	Storage               StorageConfiguration               `mapstructure:"storage"`
+	Notifier              *NotifierConfiguration             `mapstructure:"notifier"`
+	Server                ServerConfiguration                `mapstructure:"server"`
 }
diff --git a/internal/configuration/schema/server.go b/internal/configuration/schema/server.go
new file mode 100644
index 000000000..2dfe29fcd
--- /dev/null
+++ b/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,
+}
diff --git a/internal/configuration/validator/configuration.go b/internal/configuration/validator/configuration.go
index e43777526..8e0bc09df 100644
--- a/internal/configuration/validator/configuration.go
+++ b/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 {
diff --git a/internal/configuration/validator/const.go b/internal/configuration/validator/const.go
index 70f6d3412..019892b3a 100644
--- a/internal/configuration/validator/const.go
+++ b/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",
diff --git a/internal/configuration/validator/server.go b/internal/configuration/validator/server.go
new file mode 100644
index 000000000..5c6f670ca
--- /dev/null
+++ b/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"))
+	}
+}
diff --git a/internal/server/error_handler.go b/internal/server/error_handler.go
new file mode 100644
index 000000000..bc1254f30
--- /dev/null
+++ b/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)
+	}
+}
diff --git a/internal/server/server.go b/internal/server/server.go
index 6b2658a9c..fee0fdda3 100644
--- a/internal/server/server.go
+++ b/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 != "" {


Test patch:
diff --git a/internal/configuration/validator/server_test.go b/internal/configuration/validator/server_test.go
new file mode 100644
index 000000000..2bd10577c
--- /dev/null
+++ b/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")
+}