Obfuscates ACL tokens appearing in /v1/acl/<verb>/<token> APIs. (#3276)

* Obfuscates ACL tokens appearing in /v1/acl APIs.

* Makes test positively identify the desired strings.

* Adds an example and explanation of the regular expression.
This commit is contained in:
James Phillips 2017-07-15 00:07:08 -07:00 committed by GitHub
parent 8b0e14ade4
commit ac7c48c3ea
2 changed files with 65 additions and 15 deletions

View File

@ -6,6 +6,7 @@ import (
"net/http"
"net/http/pprof"
"net/url"
"regexp"
"strconv"
"strings"
"time"
@ -166,6 +167,29 @@ func (s *HTTPServer) handler(enableDebug bool) http.Handler {
return mux
}
// aclEndpointRE is used to find old ACL endpoints that take tokens in the URL
// so that we can redact them. The ACL endpoints that take the token in the URL
// are all of the form /v1/acl/<verb>/<token>, and can optionally include query
// parameters which are indicated by a question mark. We capture the part before
// the token, the token, and any query parameters after, and then reassemble as
// $1<hidden>$3 (the token in $2 isn't used), which will give:
//
// /v1/acl/clone/foo -> /v1/acl/clone/<hidden>
// /v1/acl/clone/foo?token=bar -> /v1/acl/clone/<hidden>?token=<hidden>
//
// The query parameter in the example above is obfuscated like any other, after
// this regular expression is applied, so the regular expression substitution
// results in:
//
// /v1/acl/clone/foo?token=bar -> /v1/acl/clone/<hidden>?token=bar
// ^---- $1 ----^^- $2 -^^-- $3 --^
//
// And then the loop that looks for parameters called "token" does the last
// step to get to the final redacted form.
var (
aclEndpointRE = regexp.MustCompile("^(/v1/acl/[^/]+/)([^?]+)([?]?.*)$")
)
// wrap is used to wrap functions to make them more convenient
func (s *HTTPServer) wrap(handler func(resp http.ResponseWriter, req *http.Request) (interface{}, error)) http.HandlerFunc {
return func(resp http.ResponseWriter, req *http.Request) {
@ -189,6 +213,7 @@ func (s *HTTPServer) wrap(handler func(resp http.ResponseWriter, req *http.Reque
logURL = strings.Replace(logURL, token, "<hidden>", -1)
}
}
logURL = aclEndpointRE.ReplaceAllString(logURL, "$1<hidden>$3")
if s.blacklist.Block(req.URL.Path) {
errMsg := "Endpoint is blocked by agent configuration"
@ -209,15 +234,6 @@ func (s *HTTPServer) wrap(handler func(resp http.ResponseWriter, req *http.Reque
fmt.Fprint(resp, errMsg)
}
// TODO (slackpad) We may want to consider redacting prepared
// query names/IDs here since they are proxies for tokens. But,
// knowing one only gives you read access to service listings
// which is pretty trivial, so it's probably not worth the code
// complexity and overhead of filtering them out. You can't
// recover the token it's a proxy for with just the query info;
// you'd need the actual token (or a management token) to read
// that back.
// Invoke the handler
start := time.Now()
defer func() {

View File

@ -331,16 +331,50 @@ func TestHTTP_wrap_obfuscateLog(t *testing.T) {
a.Start()
defer a.Shutdown()
resp := httptest.NewRecorder()
req, _ := http.NewRequest("GET", "/some/url?token=secret1&token=secret2", nil)
handler := func(resp http.ResponseWriter, req *http.Request) (interface{}, error) {
return nil, nil
}
a.srv.wrap(handler)(resp, req)
// Make sure no tokens from the URL show up in the log
if strings.Contains(buf.String(), "secret") {
t.Fatalf("bad: %s", buf.String())
for _, pair := range [][]string{
{
"/some/url?token=secret1&token=secret2",
"/some/url?token=<hidden>&token=<hidden>",
},
{
"/v1/acl/clone/secret1",
"/v1/acl/clone/<hidden>",
},
{
"/v1/acl/clone/secret1?token=secret2",
"/v1/acl/clone/<hidden>?token=<hidden>",
},
{
"/v1/acl/destroy/secret1",
"/v1/acl/destroy/<hidden>",
},
{
"/v1/acl/destroy/secret1?token=secret2",
"/v1/acl/destroy/<hidden>?token=<hidden>",
},
{
"/v1/acl/info/secret1",
"/v1/acl/info/<hidden>",
},
{
"/v1/acl/info/secret1?token=secret2",
"/v1/acl/info/<hidden>?token=<hidden>",
},
} {
url, want := pair[0], pair[1]
t.Run(url, func(t *testing.T) {
resp := httptest.NewRecorder()
req, _ := http.NewRequest("GET", url, nil)
a.srv.wrap(handler)(resp, req)
if got := buf.String(); !strings.Contains(got, want) {
t.Fatalf("got %s want %s", got, want)
}
})
}
}