Refactor HMAC validation

Validate the HMAC header before progressing to the HMAC calculation.

Avoid copying body contents twice.

x-pack/filebeat/input/http_endpoint/validate.go

@@ -67,34 +67,44 @@ func (v *apiValidator) ValidateHeader(r *http.Request) (int, error) {
 	}
 
 	if v.hmacHeader != "" && v.hmacKey != "" && v.hmacType != "" {
-		// We need access to the request body to validate the signature.
-		buf, _ := ioutil.ReadAll(r.Body)
-		rdr1 := ioutil.NopCloser(bytes.NewBuffer(buf))
-		originalBody := ioutil.NopCloser(bytes.NewBuffer(buf))
+		// Read HMAC signature from HTTP header.
+		hmacHeaderValue := r.Header.Get(v.hmacHeader)
+		if v.hmacHeader == "" {
+			return http.StatusUnauthorized, errMissingHMACHeader
+		}
+		if v.hmacPrefix != "" {
+			hmacHeaderValue = strings.TrimPrefix(hmacHeaderValue, v.hmacPrefix)
+		}
+		signature, err := hex.DecodeString(hmacHeaderValue)
+		if err != nil {
+			return http.StatusUnauthorized, fmt.Errorf("invalid HMAC signature hex: %w", err)
+		}
+
+		// We need access to the request body to validate the signature, but we
+		// must leave the body intact for future processing.
+		buf, err := ioutil.ReadAll(r.Body)
+		if err != nil {
+			return http.StatusInternalServerError, fmt.Errorf("failed to read request body: %w", err)
+		}
+		// Set r.Body back to untouched original value.
+		r.Body = ioutil.NopCloser(bytes.NewBuffer(buf))
 
-		var bodyBytes, _ = ioutil.ReadAll(rdr1)
+		// Compute HMAC of raw body.
 		var mac hash.Hash
-		if v.hmacType == "sha256" {
+		switch v.hmacType {
+		case "sha256":
 			mac = hmac.New(sha256.New, []byte(v.hmacKey))
-		} else {
+		case "sha1":
 			mac = hmac.New(sha1.New, []byte(v.hmacKey))
+		default:
+			// Upstream config validation prevents this from happening.
+			panic(fmt.Errorf("unhandled hmac.type %q", v.hmacType))
 		}
-
-		mac.Write(bodyBytes)
+		mac.Write(buf)
 		actualMAC := mac.Sum(nil)
 
-		hmacHeaderValue := r.Header.Get(v.hmacHeader)
-		if v.hmacPrefix != "" {
-			hmacHeaderValue = strings.Replace(hmacHeaderValue, v.hmacPrefix, "", 1)
-		}
-
-		signature, _ := hex.DecodeString(hmacHeaderValue)
-
-		// Set r.Body back to untouched original value.
-		r.Body = originalBody
-
 		if !hmac.Equal(signature, actualMAC) {
-			return http.StatusUnauthorized, errIncorrectHmacSignature
+			return http.StatusUnauthorized, errIncorrectHMACSignature
 		}
 	}
 

x-pack/filebeat/tests/system/test_http_endpoint.py

@@ -102,7 +102,7 @@ def test_http_endpoint_wrong_content_header(self):
         print("response:", r.status_code, r.text)
 
         assert r.status_code == 415
-        assert r.text == '{"message": "Wrong Content-Type header, expecting application/json"}'
+        assert r.json()['message'] == 'wrong Content-Type header, expecting application/json'
 
     def test_http_endpoint_missing_auth_value(self):
         """
@@ -115,7 +115,7 @@ def test_http_endpoint_missing_auth_value(self):
 """
         self.get_config(options)
         filebeat = self.start_beat()
-        self.wait_until(lambda: self.log_contains("Username and password required when basicauth is enabled"))
+        self.wait_until(lambda: self.log_contains("username and password required when basicauth is enabled"))
         filebeat.kill_and_wait()
 
     def test_http_endpoint_wrong_auth_value(self):
@@ -141,7 +141,7 @@ def test_http_endpoint_wrong_auth_value(self):
         print("response:", r.status_code, r.text)
 
         assert r.status_code == 401
-        assert r.text == '{"message": "Incorrect username or password"}'
+        assert r.json()['message'] == 'incorrect username or password'
 
     def test_http_endpoint_wrong_auth_header(self):
         """
@@ -165,7 +165,7 @@ def test_http_endpoint_wrong_auth_header(self):
         print("response:", r.status_code, r.text)
 
         assert r.status_code == 401
-        assert r.text == '{"message": "Incorrect header or header secret"}'
+        assert r.json()['message'] == 'incorrect header or header secret'
 
     def test_http_endpoint_correct_auth_header(self):
         """
@@ -246,7 +246,7 @@ def test_http_endpoint_invalid_hmac(self):
         print("response:", r.status_code, r.text)
 
         assert r.status_code == 401
-        assert r.text == '{"message": "Invalid HMAC signature"}'
+        self.assertRegex(r.json()['message'], 'invalid HMAC signature')
 
     def test_http_endpoint_empty_body(self):
         """
@@ -264,7 +264,7 @@ def test_http_endpoint_empty_body(self):
         print("response:", r.status_code, r.text)
 
         assert r.status_code == 406
-        assert r.text == '{"message": "Body cannot be empty"}'
+        assert r.json()['message'] == 'body cannot be empty'
 
     def test_http_endpoint_malformed_json(self):
         """
@@ -283,7 +283,7 @@ def test_http_endpoint_malformed_json(self):
         print("response:", r.status_code, r.text)
 
         assert r.status_code == 400
-        assert r.text.startswith('{"message": "Malformed JSON body:')
+        self.assertRegex(r.json()['message'], 'malformed JSON body')
 
     def test_http_endpoint_get_request(self):
         """
@@ -302,4 +302,4 @@ def test_http_endpoint_get_request(self):
         print("response:", r.status_code, r.text)
 
         assert r.status_code == 405
-        assert r.text == '{"message": "Only POST requests supported"}'
+        assert r.json()['message'] == 'only POST requests are allowed'