From d19ebd8a502eda06f8f654811c86a9a8d7318fae Mon Sep 17 00:00:00 2001 From: Andrew Kroh Date: Wed, 12 May 2021 18:54:53 -0400 Subject: [PATCH] Refactor HMAC validation Validate the HMAC header before progressing to the HMAC calculation. Avoid copying body contents twice. --- .../filebeat/input/http_endpoint/validate.go | 50 +++++++++++-------- .../tests/system/test_http_endpoint.py | 16 +++--- 2 files changed, 38 insertions(+), 28 deletions(-) diff --git a/x-pack/filebeat/input/http_endpoint/validate.go b/x-pack/filebeat/input/http_endpoint/validate.go index 9c41d0c30ea..5a46d1e6203 100644 --- a/x-pack/filebeat/input/http_endpoint/validate.go +++ b/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 } } diff --git a/x-pack/filebeat/tests/system/test_http_endpoint.py b/x-pack/filebeat/tests/system/test_http_endpoint.py index 5b01c213237..7be145945e6 100644 --- a/x-pack/filebeat/tests/system/test_http_endpoint.py +++ b/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'