Add a field to disable following redirects on http checks

.changelog/12685.txt

@@ -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.
+```

agent/agent.go

@@ -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 {

agent/checks/check.go

@@ -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
@@ -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
 		}

agent/checks/check_test.go

@@ -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

agent/config/builder.go

@@ -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),

agent/config/config.go

@@ -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"`

agent/config/runtime.go

@@ -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

agent/config/runtime_test.go

@@ -5743,6 +5743,7 @@ func TestLoad_FullConfig(t *testing.T) {
 				},
 				Method:                         "aldrIQ4l",
 				Body:                           "wSjTy7dg",
+				DisableRedirects:               true,
 				TCP:                            "RJQND605",
 				H2PING:                         "9N1cSb5B",
 				H2PingUseTLS:                   false,
@@ -5770,6 +5771,7 @@ func TestLoad_FullConfig(t *testing.T) {
 				},
 				Method:                         "gLrztrNw",
 				Body:                           "0jkKgGUC",
+				DisableRedirects:               false,
 				OutputMaxSize:                  checks.DefaultBufSize,
 				TCP:                            "4jG5casb",
 				H2PING:                         "HCHU7gEb",
@@ -5797,6 +5799,7 @@ func TestLoad_FullConfig(t *testing.T) {
 				},
 				Method:                         "Dou0nGT5",
 				Body:                           "5PBQd2OT",
+				DisableRedirects:               true,
 				OutputMaxSize:                  checks.DefaultBufSize,
 				TCP:                            "JY6fTTcw",
 				H2PING:                         "rQ8eyCSF",
@@ -6008,6 +6011,7 @@ func TestLoad_FullConfig(t *testing.T) {
 						},
 						Method:                         "X5DrovFc",
 						Body:                           "WeikigLh",
+						DisableRedirects:               true,
 						OutputMaxSize:                  checks.DefaultBufSize,
 						TCP:                            "ICbxkpSF",
 						H2PING:                         "7s7BbMyb",
@@ -6204,6 +6208,7 @@ func TestLoad_FullConfig(t *testing.T) {
 						},
 						Method:                         "T66MFBfR",
 						Body:                           "OwGjTFQi",
+						DisableRedirects:               true,
 						OutputMaxSize:                  checks.DefaultBufSize,
 						TCP:                            "bNnNfx2A",
 						H2PING:                         "qC1pidiW",
@@ -6229,6 +6234,7 @@ func TestLoad_FullConfig(t *testing.T) {
 						},
 						Method:                         "ciYHWors",
 						Body:                           "lUVLGYU7",
+						DisableRedirects:               false,
 						OutputMaxSize:                  checks.DefaultBufSize,
 						TCP:                            "FfvCwlqH",
 						H2PING:                         "spI3muI3",
@@ -6254,6 +6260,7 @@ func TestLoad_FullConfig(t *testing.T) {
 						},
 						Method:                         "9afLm3Mj",
 						Body:                           "wVVL2V6f",
+						DisableRedirects:               true,
 						OutputMaxSize:                  checks.DefaultBufSize,
 						TCP:                            "fjiLFqVd",
 						H2PING:                         "5NbNWhan",

agent/config/testdata/TestRuntimeConfig_Sanitize.golden

@@ -90,6 +90,7 @@
             "AliasService": "",
             "Body": "",
             "DeregisterCriticalServiceAfter": "0s",
+            "DisableRedirects": false,
             "DockerContainerID": "",
             "EnterpriseMeta": {},
             "FailuresBeforeCritical": 0,
@@ -297,6 +298,7 @@
                 "Body": "",
                 "CheckID": "",
                 "DeregisterCriticalServiceAfter": "0s",
+                "DisableRedirects": false,
                 "DockerContainerID": "",
                 "FailuresBeforeCritical": 0,
                 "FailuresBeforeWarning": 0,
@@ -456,4 +458,4 @@
     "Version": "",
     "VersionPrerelease": "",
     "Watches": []
-}
+}

agent/config/testdata/full-config.hcl

@@ -110,6 +110,7 @@ check = {
     }
     method = "Dou0nGT5"
     body = "5PBQd2OT"
+    disable_redirects = true
     tcp = "JY6fTTcw"
     h2ping = "rQ8eyCSF"
     h2ping_use_tls = false
@@ -138,6 +139,7 @@ checks = [
         }
         method = "aldrIQ4l"
         body = "wSjTy7dg"
+        disable_redirects = true
         tcp = "RJQND605"
         h2ping = "9N1cSb5B"
         h2ping_use_tls = false
@@ -165,6 +167,7 @@ checks = [
         }
         method = "gLrztrNw"
         body = "0jkKgGUC"
+        disable_redirects = false
         tcp = "4jG5casb"
         h2ping = "HCHU7gEb"
         h2ping_use_tls = false
@@ -389,6 +392,7 @@ service = {
         }
         method = "9afLm3Mj"
         body = "wVVL2V6f"
+        disable_redirects = true
         tcp = "fjiLFqVd"
         h2ping = "5NbNWhan"
         h2ping_use_tls = false
@@ -414,6 +418,7 @@ service = {
             }
             method = "T66MFBfR"
             body = "OwGjTFQi"
+            disable_redirects = true
             tcp = "bNnNfx2A"
             h2ping = "qC1pidiW"
             h2ping_use_tls = false
@@ -439,6 +444,7 @@ service = {
             }
             method = "ciYHWors"
             body = "lUVLGYU7"
+            disable_redirects = false
             tcp = "FfvCwlqH"
             h2ping = "spI3muI3"
             h2ping_use_tls = false
@@ -478,6 +484,7 @@ services = [
             }
             method = "X5DrovFc"
             body = "WeikigLh"
+            disable_redirects = true
             tcp = "ICbxkpSF"
             h2ping = "7s7BbMyb"
             h2ping_use_tls = false
@@ -520,6 +527,7 @@ services = [
                 }
                 method = "5wkAxCUE"
                 body = "7CRjCJyz"
+                disable_redirects = false
                 tcp = "MN3oA9D2"
                 h2ping = "OV6Q2XEg"
                 h2ping_use_tls = false

agent/config/testdata/full-config.json

@@ -111,6 +111,7 @@
     },
     "method": "Dou0nGT5",
     "body": "5PBQd2OT",
+    "disable_redirects": true,
     "output_max_size": 4096,
     "tcp": "JY6fTTcw",
     "h2ping": "rQ8eyCSF",
@@ -139,6 +140,7 @@
       },
       "method": "aldrIQ4l",
       "body": "wSjTy7dg",
+      "disable_redirects": true,
       "tcp": "RJQND605",
       "h2ping": "9N1cSb5B",
       "h2ping_use_tls": false,
@@ -166,6 +168,7 @@
       },
       "method": "gLrztrNw",
       "body": "0jkKgGUC",
+      "disable_redirects": false,
       "tcp": "4jG5casb",
       "h2ping": "HCHU7gEb",
       "h2ping_use_tls": false,
@@ -385,6 +388,7 @@
       },
       "method": "9afLm3Mj",
       "body": "wVVL2V6f",
+      "disable_redirects": true,
       "tcp": "fjiLFqVd",
       "h2ping": "5NbNWhan",
       "h2ping_use_tls": false,
@@ -411,6 +415,7 @@
         },
         "method": "T66MFBfR",
         "body": "OwGjTFQi",
+        "disable_redirects": true,
         "tcp": "bNnNfx2A",
         "h2ping": "qC1pidiW",
         "h2ping_use_tls": false,
@@ -436,6 +441,7 @@
         },
         "method": "ciYHWors",
         "body": "lUVLGYU7",
+        "disable_redirects": false,
         "tcp": "FfvCwlqH",
         "h2ping": "spI3muI3",
         "h2ping_use_tls": false,
@@ -475,6 +481,7 @@
         },
         "method": "X5DrovFc",
         "body": "WeikigLh",
+        "disable_redirects": true,
         "tcp": "ICbxkpSF",
         "h2ping": "7s7BbMyb",
         "h2ping_use_tls": false,
@@ -517,6 +524,7 @@
           },
           "method": "5wkAxCUE",
           "body": "7CRjCJyz",
+          "disable_redirects": false,
           "tcp": "MN3oA9D2",
           "h2ping": "OV6Q2XEg",
           "h2ping_use_tls": false,

agent/structs/check_definition.go

@@ -29,6 +29,7 @@ type CheckDefinition struct {
 	Header                         map[string][]string
 	Method                         string
 	Body                           string
+	DisableRedirects               bool
 	TCP                            string
 	Interval                       time.Duration
 	DockerContainerID              string
@@ -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
 	}{
@@ -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
@@ -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,

agent/structs/check_type.go

@@ -37,6 +37,7 @@ type CheckType struct {
 	Header                 map[string][]string
 	Method                 string
 	Body                   string
+	DisableRedirects       bool
 	TCP                    string
 	Interval               time.Duration
 	AliasNode              string