Skip to content

Commit

Permalink
Add a field to disable following redirects on http checks
Browse files Browse the repository at this point in the history
  • Loading branch information
@kyhavlov
kyhavlov committed Apr 1, 2022
1 parent 973d2d0 commit c1cf838
Show file tree
Hide file tree
Showing 19 changed files with 276 additions and 152 deletions.
3 changes: 3 additions & 0 deletions .changelog/12685.txt
View file
@@ -0,0 +1,3 @@
```release-note:security
agent: Added a new check field, `disable_redirects`, that allows for disabling the following of redirects for HTTP checks. The intention is to default this to true in a future release so that redirects must explicitly be enabled.
```
25 changes: 13 additions & 12 deletions agent/agent.go
View file
Expand Up @@ -2683,18 +2683,19 @@ func (a *Agent) addCheck(check *structs.HealthCheck, chkType *structs.CheckType,
tlsClientConfig := a.tlsConfigurator.OutgoingTLSConfigForCheck(chkType.TLSSkipVerify, chkType.TLSServerName)

http := &checks.CheckHTTP{
CheckID: cid,
ServiceID: sid,
HTTP: chkType.HTTP,
Header: chkType.Header,
Method: chkType.Method,
Body: chkType.Body,
Interval: chkType.Interval,
Timeout: chkType.Timeout,
Logger: a.logger,
OutputMaxSize: maxOutputSize,
TLSClientConfig: tlsClientConfig,
StatusHandler: statusHandler,
CheckID: cid,
ServiceID: sid,
HTTP: chkType.HTTP,
Header: chkType.Header,
Method: chkType.Method,
Body: chkType.Body,
DisableRedirects: chkType.DisableRedirects,
Interval: chkType.Interval,
Timeout: chkType.Timeout,
Logger: a.logger,
OutputMaxSize: maxOutputSize,
TLSClientConfig: tlsClientConfig,
StatusHandler: statusHandler,
}

if proxy != nil && proxy.Proxy.Expose.Checks {
Expand Down
30 changes: 18 additions & 12 deletions agent/checks/check.go
View file
Expand Up @@ -334,18 +334,19 @@ func (c *CheckTTL) SetStatus(status, output string) string {
// or if the request returns an error
// Supports failures_before_critical and success_before_passing.
type CheckHTTP struct {
CheckID structs.CheckID
ServiceID structs.ServiceID
HTTP string
Header map[string][]string
Method string
Body string
Interval time.Duration
Timeout time.Duration
Logger hclog.Logger
TLSClientConfig *tls.Config
OutputMaxSize int
StatusHandler *StatusHandler
CheckID structs.CheckID
ServiceID structs.ServiceID
HTTP string
Header map[string][]string
Method string
Body string
Interval time.Duration
Timeout time.Duration
Logger hclog.Logger
TLSClientConfig *tls.Config
OutputMaxSize int
StatusHandler *StatusHandler
DisableRedirects bool

httpClient *http.Client
stop bool
Expand Down Expand Up @@ -392,6 +393,11 @@ func (c *CheckHTTP) Start() {
Timeout: 10 * time.Second,
Transport: trans,
}
if c.DisableRedirects {
c.httpClient.CheckRedirect = func(req *http.Request, via []*http.Request) error {
return http.ErrUseLastResponse
}
}
if c.Timeout > 0 {
c.httpClient.Timeout = c.Timeout
}
Expand Down
40 changes: 40 additions & 0 deletions agent/checks/check_test.go
View file
Expand Up @@ -459,6 +459,46 @@ func TestCheckHTTP_NotProxied(t *testing.T) {
})
}

func TestCheckHTTP_DisableRedirects(t *testing.T) {
t.Parallel()

server1 := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
fmt.Fprintln(w, "server1")
}))
defer server1.Close()

server2 := httptest.NewServer(http.RedirectHandler(server1.URL, 301))
defer server2.Close()

notif := mock.NewNotify()
logger := testutil.Logger(t)
statusHandler := NewStatusHandler(notif, logger, 0, 0, 0)
cid := structs.NewCheckID("foo", nil)

check := &CheckHTTP{
CheckID: cid,
HTTP: server2.URL,
Method: "GET",
OutputMaxSize: DefaultBufSize,
Interval: 10 * time.Millisecond,
DisableRedirects: true,
Logger: logger,
StatusHandler: statusHandler,
}
check.Start()
defer check.Stop()

retry.Run(t, func(r *retry.R) {
output := notif.Output(cid)
if !strings.Contains(output, "Moved Permanently") {
r.Fatalf("should have returned 301 body instead of redirecting")
}
if strings.Contains(output, "server1") {
r.Fatalf("followed redirect")
}
})
}

func TestCheckHTTPTCP_BigTimeout(t *testing.T) {
testCases := []struct {
timeoutIn, intervalIn, timeoutWant time.Duration
Expand Down
1 change: 1 addition & 0 deletions agent/config/builder.go
View file
Expand Up @@ -1546,6 +1546,7 @@ func (b *builder) checkVal(v *CheckDefinition) *structs.CheckDefinition {
Header: v.Header,
Method: stringVal(v.Method),
Body: stringVal(v.Body),
DisableRedirects: boolVal(v.DisableRedirects),
TCP: stringVal(v.TCP),
Interval: b.durationVal(fmt.Sprintf("check[%s].interval", id), v.Interval),
DockerContainerID: stringVal(v.DockerContainerID),
Expand Down
1 change: 1 addition & 0 deletions agent/config/config.go
View file
Expand Up @@ -399,6 +399,7 @@ type CheckDefinition struct {
Header map[string][]string `mapstructure:"header"`
Method *string `mapstructure:"method"`
Body *string `mapstructure:"body"`
DisableRedirects *bool `mapstructure:"disable_redirects"`
OutputMaxSize *int `mapstructure:"output_max_size"`
TCP *string `mapstructure:"tcp"`
Interval *string `mapstructure:"interval"`
Expand Down
1 change: 1 addition & 0 deletions agent/config/runtime.go
View file
Expand Up @@ -424,6 +424,7 @@ type RuntimeConfig struct {
// http = string
// header = map[string][]string
// method = string
// disable_redirects = (true|false)
// tcp = string
// h2ping = string
// interval = string
Expand Down
7 changes: 7 additions & 0 deletions agent/config/runtime_test.go
View file
Expand Up @@ -5743,6 +5743,7 @@ func TestLoad_FullConfig(t *testing.T) {
},
Method: "aldrIQ4l",
Body: "wSjTy7dg",
DisableRedirects: true,
TCP: "RJQND605",
H2PING: "9N1cSb5B",
H2PingUseTLS: false,
Expand Down Expand Up @@ -5770,6 +5771,7 @@ func TestLoad_FullConfig(t *testing.T) {
},
Method: "gLrztrNw",
Body: "0jkKgGUC",
DisableRedirects: false,
OutputMaxSize: checks.DefaultBufSize,
TCP: "4jG5casb",
H2PING: "HCHU7gEb",
Expand Down Expand Up @@ -5797,6 +5799,7 @@ func TestLoad_FullConfig(t *testing.T) {
},
Method: "Dou0nGT5",
Body: "5PBQd2OT",
DisableRedirects: true,
OutputMaxSize: checks.DefaultBufSize,
TCP: "JY6fTTcw",
H2PING: "rQ8eyCSF",
Expand Down Expand Up @@ -6008,6 +6011,7 @@ func TestLoad_FullConfig(t *testing.T) {
},
Method: "X5DrovFc",
Body: "WeikigLh",
DisableRedirects: true,
OutputMaxSize: checks.DefaultBufSize,
TCP: "ICbxkpSF",
H2PING: "7s7BbMyb",
Expand Down Expand Up @@ -6204,6 +6208,7 @@ func TestLoad_FullConfig(t *testing.T) {
},
Method: "T66MFBfR",
Body: "OwGjTFQi",
DisableRedirects: true,
OutputMaxSize: checks.DefaultBufSize,
TCP: "bNnNfx2A",
H2PING: "qC1pidiW",
Expand All @@ -6229,6 +6234,7 @@ func TestLoad_FullConfig(t *testing.T) {
},
Method: "ciYHWors",
Body: "lUVLGYU7",
DisableRedirects: false,
OutputMaxSize: checks.DefaultBufSize,
TCP: "FfvCwlqH",
H2PING: "spI3muI3",
Expand All @@ -6254,6 +6260,7 @@ func TestLoad_FullConfig(t *testing.T) {
},
Method: "9afLm3Mj",
Body: "wVVL2V6f",
DisableRedirects: true,
OutputMaxSize: checks.DefaultBufSize,
TCP: "fjiLFqVd",
H2PING: "5NbNWhan",
Expand Down
4 changes: 3 additions & 1 deletion agent/config/testdata/TestRuntimeConfig_Sanitize.golden
View file
Expand Up @@ -90,6 +90,7 @@
"AliasService": "",
"Body": "",
"DeregisterCriticalServiceAfter": "0s",
"DisableRedirects": false,
"DockerContainerID": "",
"EnterpriseMeta": {},
"FailuresBeforeCritical": 0,
Expand Down Expand Up @@ -297,6 +298,7 @@
"Body": "",
"CheckID": "",
"DeregisterCriticalServiceAfter": "0s",
"DisableRedirects": false,
"DockerContainerID": "",
"FailuresBeforeCritical": 0,
"FailuresBeforeWarning": 0,
Expand Down Expand Up @@ -456,4 +458,4 @@
"Version": "",
"VersionPrerelease": "",
"Watches": []
}
}
8 changes: 8 additions & 0 deletions agent/config/testdata/full-config.hcl
View file
Expand Up @@ -110,6 +110,7 @@ check = {
}
method = "Dou0nGT5"
body = "5PBQd2OT"
disable_redirects = true
tcp = "JY6fTTcw"
h2ping = "rQ8eyCSF"
h2ping_use_tls = false
Expand Down Expand Up @@ -138,6 +139,7 @@ checks = [
}
method = "aldrIQ4l"
body = "wSjTy7dg"
disable_redirects = true
tcp = "RJQND605"
h2ping = "9N1cSb5B"
h2ping_use_tls = false
Expand Down Expand Up @@ -165,6 +167,7 @@ checks = [
}
method = "gLrztrNw"
body = "0jkKgGUC"
disable_redirects = false
tcp = "4jG5casb"
h2ping = "HCHU7gEb"
h2ping_use_tls = false
Expand Down Expand Up @@ -389,6 +392,7 @@ service = {
}
method = "9afLm3Mj"
body = "wVVL2V6f"
disable_redirects = true
tcp = "fjiLFqVd"
h2ping = "5NbNWhan"
h2ping_use_tls = false
Expand All @@ -414,6 +418,7 @@ service = {
}
method = "T66MFBfR"
body = "OwGjTFQi"
disable_redirects = true
tcp = "bNnNfx2A"
h2ping = "qC1pidiW"
h2ping_use_tls = false
Expand All @@ -439,6 +444,7 @@ service = {
}
method = "ciYHWors"
body = "lUVLGYU7"
disable_redirects = false
tcp = "FfvCwlqH"
h2ping = "spI3muI3"
h2ping_use_tls = false
Expand Down Expand Up @@ -478,6 +484,7 @@ services = [
}
method = "X5DrovFc"
body = "WeikigLh"
disable_redirects = true
tcp = "ICbxkpSF"
h2ping = "7s7BbMyb"
h2ping_use_tls = false
Expand Down Expand Up @@ -520,6 +527,7 @@ services = [
}
method = "5wkAxCUE"
body = "7CRjCJyz"
disable_redirects = false
tcp = "MN3oA9D2"
h2ping = "OV6Q2XEg"
h2ping_use_tls = false
Expand Down
8 changes: 8 additions & 0 deletions agent/config/testdata/full-config.json
View file
Expand Up @@ -111,6 +111,7 @@
},
"method": "Dou0nGT5",
"body": "5PBQd2OT",
"disable_redirects": true,
"output_max_size": 4096,
"tcp": "JY6fTTcw",
"h2ping": "rQ8eyCSF",
Expand Down Expand Up @@ -139,6 +140,7 @@
},
"method": "aldrIQ4l",
"body": "wSjTy7dg",
"disable_redirects": true,
"tcp": "RJQND605",
"h2ping": "9N1cSb5B",
"h2ping_use_tls": false,
Expand Down Expand Up @@ -166,6 +168,7 @@
},
"method": "gLrztrNw",
"body": "0jkKgGUC",
"disable_redirects": false,
"tcp": "4jG5casb",
"h2ping": "HCHU7gEb",
"h2ping_use_tls": false,
Expand Down Expand Up @@ -385,6 +388,7 @@
},
"method": "9afLm3Mj",
"body": "wVVL2V6f",
"disable_redirects": true,
"tcp": "fjiLFqVd",
"h2ping": "5NbNWhan",
"h2ping_use_tls": false,
Expand All @@ -411,6 +415,7 @@
},
"method": "T66MFBfR",
"body": "OwGjTFQi",
"disable_redirects": true,
"tcp": "bNnNfx2A",
"h2ping": "qC1pidiW",
"h2ping_use_tls": false,
Expand All @@ -436,6 +441,7 @@
},
"method": "ciYHWors",
"body": "lUVLGYU7",
"disable_redirects": false,
"tcp": "FfvCwlqH",
"h2ping": "spI3muI3",
"h2ping_use_tls": false,
Expand Down Expand Up @@ -475,6 +481,7 @@
},
"method": "X5DrovFc",
"body": "WeikigLh",
"disable_redirects": true,
"tcp": "ICbxkpSF",
"h2ping": "7s7BbMyb",
"h2ping_use_tls": false,
Expand Down Expand Up @@ -517,6 +524,7 @@
},
"method": "5wkAxCUE",
"body": "7CRjCJyz",
"disable_redirects": false,
"tcp": "MN3oA9D2",
"h2ping": "OV6Q2XEg",
"h2ping_use_tls": false,
Expand Down
6 changes: 6 additions & 0 deletions agent/structs/check_definition.go
View file
Expand Up @@ -29,6 +29,7 @@ type CheckDefinition struct {
Header map[string][]string
Method string
Body string
DisableRedirects bool
TCP string
Interval time.Duration
DockerContainerID string
Expand Down Expand Up @@ -71,6 +72,7 @@ func (t *CheckDefinition) UnmarshalJSON(data []byte) (err error) {
GRPCUseTLSSnake bool `json:"grpc_use_tls"`
ServiceIDSnake string `json:"service_id"`
H2PingUseTLSSnake bool `json:"h2ping_use_tls"`
DisableRedirectsSnake bool `json:"disable_redirects"`

*Alias
}{
Expand Down Expand Up @@ -116,6 +118,9 @@ func (t *CheckDefinition) UnmarshalJSON(data []byte) (err error) {
if t.ServiceID == "" {
t.ServiceID = aux.ServiceIDSnake
}
if aux.DisableRedirectsSnake {
t.DisableRedirects = aux.DisableRedirectsSnake
}

if (aux.H2PING != "" && !aux.H2PingUseTLSSnake) || (aux.H2PING == "" && aux.H2PingUseTLSSnake) {
t.H2PingUseTLS = aux.H2PingUseTLSSnake
Expand Down Expand Up @@ -205,6 +210,7 @@ func (c *CheckDefinition) CheckType() *CheckType {
Header: c.Header,
Method: c.Method,
Body: c.Body,
DisableRedirects: c.DisableRedirects,
OutputMaxSize: c.OutputMaxSize,
TCP: c.TCP,
Interval: c.Interval,
Expand Down
1 change: 1 addition & 0 deletions agent/structs/check_type.go
View file
Expand Up @@ -37,6 +37,7 @@ type CheckType struct {
Header map[string][]string
Method string
Body string
DisableRedirects bool
TCP string
Interval time.Duration
AliasNode string
Expand Down

0 comments on commit c1cf838

Please sign in to comment.