From 260332c72661e1f6769722f2aaf5dcdd6684e87c Mon Sep 17 00:00:00 2001 From: blotus Date: Tue, 9 Feb 2021 19:10:14 +0100 Subject: [PATCH] Add use_forwarded_for_headers configuration option for LAPI (#610) * Add use_forwarded_for_headers configuration option for LAPI * update documentation --- docs/v1.X/docs/references/crowdsec-config.md | 9 ++ pkg/apiserver/apiserver.go | 11 ++- pkg/apiserver/apiserver_test.go | 62 ++++++++++++++ pkg/apiserver/machines_test.go | 90 ++++++++++++++++++++ pkg/csconfig/api.go | 17 ++-- pkg/csconfig/config.go | 3 +- 6 files changed, 179 insertions(+), 13 deletions(-) diff --git a/docs/v1.X/docs/references/crowdsec-config.md b/docs/v1.X/docs/references/crowdsec-config.md index 95a752954..0a5518872 100644 --- a/docs/v1.X/docs/references/crowdsec-config.md +++ b/docs/v1.X/docs/references/crowdsec-config.md @@ -50,6 +50,7 @@ api: log_level: info listen_uri: 127.0.0.1:8080 profiles_path: /etc/crowdsec/profiles.yaml + use_forwarded_for_headers: false online_client: # Crowdsec API credentials_path: /etc/crowdsec/online_api_credentials.yaml # tls: @@ -132,6 +133,7 @@ api: log_level: (error|info|debug|trace>) listen_uri: # host:port profiles_path: + use_forwarded_for_headers: online_client: credentials_path: tls: @@ -304,6 +306,7 @@ api: log_level: (error|info|debug|trace>) listen_uri: # host:port profiles_path: + use_forwarded_for_headers: (true|false) online_client: credentials_path: tls: @@ -340,6 +343,7 @@ server: log_level: (error|info|debug|trace) listen_uri: # host:port profiles_path: + use_forwarded_for_headers: (true|false) online_client: credentials_path: tls: @@ -357,6 +361,11 @@ Address and port listen configuration, the form `host:port`. The path to the {{v1X.profiles.htmlname}} configuration. +#### `use_forwarded_for_headers` +> string + +Allow the usage of `X-Forwarded-For` or `X-Real-IP` to get the client IP address. Do not enable if you are not running the LAPI behind a trusted reverse-proxy or LB. + #### `online_client` Configuration to push signals and receive bad IPs from Crowdsec API. diff --git a/pkg/apiserver/apiserver.go b/pkg/apiserver/apiserver.go index 8a73f61c4..de452c2e6 100644 --- a/pkg/apiserver/apiserver.go +++ b/pkg/apiserver/apiserver.go @@ -61,10 +61,13 @@ func NewServer(config *csconfig.LocalApiServerCfg) (*APIServer, error) { } log.Debugf("starting router, logging to %s", logFile) router := gin.New() - /*related to https://github.com/gin-gonic/gin/pull/2474 - Gin team doesn't seem to be willing to have a opt-in/opt-out on the trusted proxies. - For now, let's not trust that. */ - router.ForwardedByClientIP = false + /* See https://github.com/gin-gonic/gin/pull/2474: + Gin does not handle safely X-Forwarded-For or X-Real-IP. + We do not trust them by default, but the user can opt-in + if they host LAPI behind a trusted proxy which sanitize + X-Forwarded-For and X-Real-IP. + */ + router.ForwardedByClientIP = config.UseForwardedForHeaders /*The logger that will be used by handlers*/ clog := log.New() diff --git a/pkg/apiserver/apiserver_test.go b/pkg/apiserver/apiserver_test.go index b35e7f907..1fac71783 100644 --- a/pkg/apiserver/apiserver_test.go +++ b/pkg/apiserver/apiserver_test.go @@ -57,6 +57,33 @@ func LoadTestConfig() csconfig.GlobalConfig { return config } +func LoadTestConfigForwardedFor() csconfig.GlobalConfig { + config := csconfig.GlobalConfig{} + maxAge := "1h" + flushConfig := csconfig.FlushDBCfg{ + MaxAge: &maxAge, + } + dbconfig := csconfig.DatabaseCfg{ + Type: "sqlite", + DbPath: "./ent", + Flush: &flushConfig, + } + apiServerConfig := csconfig.LocalApiServerCfg{ + ListenURI: "http://127.0.0.1:8080", + DbConfig: &dbconfig, + ProfilesPath: "./tests/profiles.yaml", + UseForwardedForHeaders: true, + } + apiConfig := csconfig.APICfg{ + Server: &apiServerConfig, + } + config.API = &apiConfig + if err := config.API.Server.LoadProfiles(); err != nil { + log.Fatalf("failed to load profiles: %s", err) + } + return config +} + func NewAPITest() (*gin.Engine, error) { config := LoadTestConfig() @@ -74,6 +101,23 @@ func NewAPITest() (*gin.Engine, error) { return router, nil } +func NewAPITestForwardedFor() (*gin.Engine, error) { + config := LoadTestConfigForwardedFor() + + os.Remove("./ent") + apiServer, err := NewServer(config.API.Server) + if err != nil { + return nil, fmt.Errorf("unable to run local API: %s", err) + } + log.Printf("Creating new API server") + gin.SetMode(gin.TestMode) + router, err := apiServer.Router() + if err != nil { + return nil, fmt.Errorf("unable to run local API: %s", err) + } + return router, nil +} + func ValidateMachine(machineID string) error { config := LoadTestConfig() dbClient, err := database.NewClient(config.API.Server.DbConfig) @@ -86,6 +130,24 @@ func ValidateMachine(machineID string) error { return nil } +func GetMachineIP(machineID string) (string, error) { + config := LoadTestConfig() + dbClient, err := database.NewClient(config.API.Server.DbConfig) + if err != nil { + return "", fmt.Errorf("unable to create new database client: %s", err) + } + machines, err := dbClient.ListMachines() + if err != nil { + return "", fmt.Errorf("Unable to list machines: %s", err) + } + for _, machine := range machines { + if machine.MachineId == machineID { + return machine.IpAddress, nil + } + } + return "", nil +} + func CreateTestMachine(router *gin.Engine) (string, error) { b, err := json.Marshal(MachineTest) if err != nil { diff --git a/pkg/apiserver/machines_test.go b/pkg/apiserver/machines_test.go index 18c12b3e0..1fbe5c13a 100644 --- a/pkg/apiserver/machines_test.go +++ b/pkg/apiserver/machines_test.go @@ -52,6 +52,96 @@ func TestCreateMachine(t *testing.T) { } +func TestCreateMachineWithForwardedFor(t *testing.T) { + router, err := NewAPITestForwardedFor() + if err != nil { + log.Fatalf("unable to run local API: %s", err) + } + + // Create machine + b, err := json.Marshal(MachineTest) + if err != nil { + log.Fatalf("unable to marshal MachineTest") + } + body := string(b) + + w := httptest.NewRecorder() + req, _ := http.NewRequest("POST", "/v1/watchers", strings.NewReader(body)) + req.Header.Add("User-Agent", UserAgent) + req.Header.Add("X-Real-IP", "1.1.1.1") + router.ServeHTTP(w, req) + + assert.Equal(t, 201, w.Code) + assert.Equal(t, "", w.Body.String()) + + ip, err := GetMachineIP(*MachineTest.MachineID) + if err != nil { + log.Fatalf("Could not get machine IP : %s", err) + } + assert.Equal(t, "1.1.1.1", ip) +} + +func TestCreateMachineWithForwardedForNoConfig(t *testing.T) { + router, err := NewAPITest() + if err != nil { + log.Fatalf("unable to run local API: %s", err) + } + + // Create machine + b, err := json.Marshal(MachineTest) + if err != nil { + log.Fatalf("unable to marshal MachineTest") + } + body := string(b) + + w := httptest.NewRecorder() + req, _ := http.NewRequest("POST", "/v1/watchers", strings.NewReader(body)) + req.Header.Add("User-Agent", UserAgent) + req.Header.Add("X-Real-IP", "1.1.1.1") + router.ServeHTTP(w, req) + + assert.Equal(t, 201, w.Code) + assert.Equal(t, "", w.Body.String()) + + ip, err := GetMachineIP(*MachineTest.MachineID) + if err != nil { + log.Fatalf("Could not get machine IP : %s", err) + } + //For some reason, the IP is empty when running tests + //if no forwarded-for headers are present + assert.Equal(t, "", ip) +} + +func TestCreateMachineWithoutForwardedFor(t *testing.T) { + router, err := NewAPITestForwardedFor() + if err != nil { + log.Fatalf("unable to run local API: %s", err) + } + + // Create machine + b, err := json.Marshal(MachineTest) + if err != nil { + log.Fatalf("unable to marshal MachineTest") + } + body := string(b) + + w := httptest.NewRecorder() + req, _ := http.NewRequest("POST", "/v1/watchers", strings.NewReader(body)) + req.Header.Add("User-Agent", UserAgent) + router.ServeHTTP(w, req) + + assert.Equal(t, 201, w.Code) + assert.Equal(t, "", w.Body.String()) + + ip, err := GetMachineIP(*MachineTest.MachineID) + if err != nil { + log.Fatalf("Could not get machine IP : %s", err) + } + //For some reason, the IP is empty when running tests + //if no forwarded-for headers are present + assert.Equal(t, "", ip) +} + func TestCreateMachineAlreadyExist(t *testing.T) { router, err := NewAPITest() if err != nil { diff --git a/pkg/csconfig/api.go b/pkg/csconfig/api.go index 40180090e..126e3526c 100644 --- a/pkg/csconfig/api.go +++ b/pkg/csconfig/api.go @@ -28,14 +28,15 @@ type LocalApiClientCfg struct { /*local api service configuration*/ type LocalApiServerCfg struct { - ListenURI string `yaml:"listen_uri,omitempty"` //127.0.0.1:8080 - TLS *TLSCfg `yaml:"tls"` - DbConfig *DatabaseCfg `yaml:"-"` - LogDir string `yaml:"-"` - OnlineClient *OnlineApiClientCfg `yaml:"online_client"` - ProfilesPath string `yaml:"profiles_path,omitempty"` - Profiles []*ProfileCfg `yaml:"-"` - LogLevel *log.Level `yaml:"log_level"` + ListenURI string `yaml:"listen_uri,omitempty"` //127.0.0.1:8080 + TLS *TLSCfg `yaml:"tls"` + DbConfig *DatabaseCfg `yaml:"-"` + LogDir string `yaml:"-"` + OnlineClient *OnlineApiClientCfg `yaml:"online_client"` + ProfilesPath string `yaml:"profiles_path,omitempty"` + Profiles []*ProfileCfg `yaml:"-"` + LogLevel *log.Level `yaml:"log_level"` + UseForwardedForHeaders bool `yaml:"use_forwarded_for_headers,omitempty"` } type TLSCfg struct { diff --git a/pkg/csconfig/config.go b/pkg/csconfig/config.go index beda02442..f12f8e7f2 100644 --- a/pkg/csconfig/config.go +++ b/pkg/csconfig/config.go @@ -232,7 +232,8 @@ func NewDefaultConfig() *GlobalConfig { CredentialsFilePath: "/etc/crowdsec/config/lapi-secrets.yaml", }, Server: &LocalApiServerCfg{ - ListenURI: "127.0.0.1:8080", + ListenURI: "127.0.0.1:8080", + UseForwardedForHeaders: false, OnlineClient: &OnlineApiClientCfg{ CredentialsFilePath: "/etc/crowdsec/config/online-api-secrets.yaml", },