Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adding HMAC support to http_endpoint input #20744

Closed
wants to merge 6 commits into from
Closed

Adding HMAC support to http_endpoint input #20744

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
ec02dbc
Adding HMAC support to http_endpoint input
P1llus Aug 23, 2020
4eeca2f
need to add the new values to the apiValidator
P1llus Aug 23, 2020
b5288d5
updating withValidator
P1llus Aug 23, 2020
de1a61d
shorter version of those new if statements
P1llus Aug 23, 2020
955c5e5
fixing a small typo
P1llus Aug 25, 2020
20f8444
rewriting the middleware logic to make it easier to extend
P1llus Aug 25, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions x-pack/filebeat/input/http_endpoint/config.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,9 @@ type config struct {
ContentType string `config:"content_type"`
SecretHeader string `config:"secret.header"`
SecretValue string `config:"secret.value"`
HmacHeader string `config:"hmac.header"`
HmacPrefix string `config:"hmac.prefix"`
HmacToken string `config:"hmac.token"`
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

At some point we should consider to introduce structs for Secret or HMAC :)

nit: HMACX, not HmacX (common go naming convention)

}

func defaultConfig() config {
Expand All @@ -42,6 +45,9 @@ func defaultConfig() config {
ContentType: "application/json",
SecretHeader: "",
SecretValue: "",
HmacHeader: "",
HmacPrefix: "",
HmacToken: "",
}
}

Expand Down
15 changes: 8 additions & 7 deletions x-pack/filebeat/input/http_endpoint/handler.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,17 +36,16 @@ var errUnsupportedType = errors.New("Only JSON objects are accepted")
func (h *httpHandler) apiResponse(w http.ResponseWriter, r *http.Request) {
obj, status, err := httpReadJsonObject(r.Body)
if err != nil {
w.Header().Add("Content-Type", "application/json")
sendErrorResponse(w, status, err)
return
}

h.publishEvent(obj)
w.Header().Add("Content-Type", "application/json")
h.sendResponse(w, h.responseCode, h.responseBody)
}

func (h *httpHandler) sendResponse(w http.ResponseWriter, status int, message string) {
w.Header().Add("Content-Type", "application/json")
w.WriteHeader(status)
io.WriteString(w, message)
}
Expand All @@ -62,13 +61,15 @@ func (h *httpHandler) publishEvent(obj common.MapStr) {
h.publisher.Publish(event)
}

func withValidator(v validator, handler http.HandlerFunc) http.HandlerFunc {
func withValidators(handler http.HandlerFunc, vs ...validator) http.HandlerFunc {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Orignally the validator interface and withValidators was introduced for validating headers only. The idea was to leave body validation to the actual handler. Validation of headers and the body should be separate steps.

Maybe let's rename the function to withHeaderValidators and pass the headers instead of the fulll request to the validators.

The difference between headers and body is that the headers are pre-parsed by the HTTP server and passed as a map[string]string. We can do as many checks as we want. The body on the other hand is not parsed, and we only get access to the stream (io.Reader) via r.Body. In case the body is not explicitely copied, it is consumed while readom from the reader. We also need to check/limit the body size, as we don't want to overload/DoS the Beat.

Alternatively to changing the interface of withValidators we could introduct a composeValidator:

type validatorList []validator

func (l validatorList) Validate(headers http.Headers) error {
  for _, v := range l { ... }
  return nil
}

return func(w http.ResponseWriter, r *http.Request) {
if status, err := v.ValidateHeader(r); status != 0 && err != nil {
sendErrorResponse(w, status, err)
} else {
handler(w, r)
for _, v := range vs {
if status, err := v.Validate(r); status != 0 && err != nil {
sendErrorResponse(w, status, err)
return
}
}
handler(w, r)
}
}

Expand Down
19 changes: 15 additions & 4 deletions x-pack/filebeat/input/http_endpoint/input.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -82,16 +82,25 @@ func (e *httpEndpoint) Test(_ v2.TestContext) error {
func (e *httpEndpoint) Run(ctx v2.Context, publisher stateless.Publisher) error {
log := ctx.Logger.With("address", e.addr)

validator := &apiValidator{
authValidator := &authValidator{
basicAuth: e.config.BasicAuth,
username: e.config.Username,
password: e.config.Password,
method: http.MethodPost,
contentType: e.config.ContentType,
secretHeader: e.config.SecretHeader,
secretValue: e.config.SecretValue,
}

hmacValidator := &hmacValidator{
hmacHeader: e.config.HmacHeader,
hmacPrefix: e.config.HmacPrefix,
hmacToken: e.config.HmacToken,
}

headerValidator := &headerValidator{
method: http.MethodPost,
contentType: e.config.ContentType,
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The validators need to check on every request if they are configured or not. Worse, they even check for missconfiguration at runtime (users can detect beats miss configuration in HTTP client at runtime only). I'd propose to separate configuration time from runtime, and build an array of configured/enabled header validators (e.g. validatorList type shown in another comment). Faulty configurations must be detected when creating the endpoint object, not maybe later after running it.


handler := &httpHandler{
log: log,
publisher: publisher,
Expand All @@ -100,8 +109,10 @@ func (e *httpEndpoint) Run(ctx v2.Context, publisher stateless.Publisher) error
responseBody: e.config.ResponseBody,
}

validateHandler := withValidators(handler.apiResponse, headerValidator, hmacValidator, authValidator)

mux := http.NewServeMux()
mux.HandleFunc(e.config.URL, withValidator(validator, handler.apiResponse))
mux.HandleFunc(e.config.URL, validateHandler)
server := &http.Server{Addr: e.addr, TLSConfig: e.tlsConfig, Handler: mux}
_, cancel := ctxtool.WithFunc(ctxtool.FromCanceller(ctx.Cancelation), func() {
server.Close()
Expand Down
64 changes: 56 additions & 8 deletions x-pack/filebeat/input/http_endpoint/validate.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,43 +5,59 @@
package http_endpoint

import (
"bytes"
"crypto/hmac"
"crypto/sha1"
"encoding/hex"
"errors"
"fmt"
"io/ioutil"
"net/http"
)

type validator interface {
// ValidateHeader checks the HTTP headers for compliance. The body must not
// be touched.
ValidateHeader(*http.Request) (int, error)
Validate(*http.Request) (int, error)
P1llus marked this conversation as resolved.
Show resolved Hide resolved
}

type apiValidator struct {
type authValidator struct {
basicAuth bool
username, password string
method string
contentType string
secretHeader string
secretValue string
}

type headerValidator struct {
method string
contentType string
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

all validators are essentially header validators. Consider to rename this one into method validator or split this one into method and content-type validators.


type hmacValidator struct {
hmacHeader string
hmacToken string
hmacPrefix string
}

var errIncorrectUserOrPass = errors.New("Incorrect username or password")
var errIncorrectHeaderSecret = errors.New("Incorrect header or header secret")
var errIncorrectHmac = errors.New("The HMAC signature of the request body does not match with the configured secret")

func (v *apiValidator) ValidateHeader(r *http.Request) (int, error) {
func (v *authValidator) Validate(r *http.Request) (int, error) {
if v.basicAuth {
username, password, _ := r.BasicAuth()
if v.username != username || v.password != password {
return http.StatusUnauthorized, errIncorrectUserOrPass
}
}

if v.secretHeader != "" && v.secretValue != "" {
if v.secretValue != r.Header.Get(v.secretHeader) {
return http.StatusUnauthorized, errIncorrectHeaderSecret
}
}

return 0, nil
}

func (v *headerValidator) Validate(r *http.Request) (int, error) {
if v.method != "" && v.method != r.Method {
return http.StatusMethodNotAllowed, fmt.Errorf("Only %v requests supported", v.method)
}
Expand All @@ -52,3 +68,35 @@ func (v *apiValidator) ValidateHeader(r *http.Request) (int, error) {

return 0, nil
}

func (v *hmacValidator) Validate(r *http.Request) (int, error) {
if v.hmacHeader != "" && v.hmacToken == "" || v.hmacHeader == "" && v.hmacToken != "" {
return http.StatusMethodNotAllowed, fmt.Errorf("Both hmacToken and hmacHeader has to be set")
}

if v.hmacToken != "" && v.hmacHeader != "" {
if len(r.Header.Get(v.hmacHeader)) == 0 {
return http.StatusInternalServerError, fmt.Errorf("The HMAC signature in the configured request header is empty")
}
s := hmac.New(sha1.New, []byte(v.hmacToken))
b, err := ioutil.ReadAll(r.Body)
P1llus marked this conversation as resolved.
Show resolved Hide resolved
if err != nil {
return http.StatusInternalServerError, fmt.Errorf("Failed to read the request body: %v", err)
}
r.Body.Close()
r.Body = ioutil.NopCloser(bytes.NewBuffer(b))

s.Write(b)
h := r.Header.Get(v.hmacHeader)
// If the header includes a prefix before the SHA-1 key, we need to only grab the signature after the prefix. Can also be 0
hWithPrefix := make([]byte, 20)
hex.Decode(hWithPrefix, []byte(h[len(v.hmacPrefix):]))

if !hmac.Equal(s.Sum(nil), hWithPrefix) {
return http.StatusUnauthorized, errIncorrectHmac
}

}

return 0, nil
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hm, this is doing more than just validating the contents. This looks like an intermediate data processing level. Maybe this should be some middleware layer that wraps the body. The custom reader would stream all writes from body to the hmac 'validator'. If we hit EOF, the reader does the final check and fails if there was a failure. In case the wrapped handler did not consume the body, the middleware handler would read until EOF (or error) in order to enforce the HMAC check.

Using layers we would create a processing chain like: bodyLimit -> hmac -> header validation -> body validation. The bodyLimit would wrap the body and fail in case the body (or contents-length) is bigger then the configured limits.

Are there packages/projects providing authentication support as middleware, we can use?