diff --git a/CHANGELOG.md b/CHANGELOG.md index d7b997c..9f4cd85 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,12 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0. - Nothing should go in this section, please add to the latest unreleased version (and update the corresponding date), or add a new version. +## [8.0.9] - 2023-04-21 + +### Security +- Redact credentials dumped to logs with `--debug` flag + [cyberark/conjur-cli-go#130](https://github.com/cyberark/conjur-cli-go/pull/130) + ## [8.0.8] - 2023-04-19 ### Fixed diff --git a/pkg/utils/http_dump_transport.go b/pkg/utils/http_dump_transport.go index ed5769c..c724ea6 100644 --- a/pkg/utils/http_dump_transport.go +++ b/pkg/utils/http_dump_transport.go @@ -1,21 +1,137 @@ package utils import ( + "bytes" + "io" "net/http" "net/http/httputil" + "regexp" + "strings" ) +const redactedString = "[REDACTED]" + type dumpTransport struct { roundTripper http.RoundTripper logRequest func([]byte) logResponse func([]byte) } -func (d *dumpTransport) RoundTrip(req *http.Request) (*http.Response, error) { +// redactAuthz purges Authorization headers from a given request, +// and returns a function to restore them. +func redactAuthz(req *http.Request) (restore func()) { + restore = func() {} + + origAuthz := req.Header.Get("Authorization") + if origAuthz != "" { + req.Header.Set("Authorization", redactedString) + restore = func() { + req.Header.Set("Authorization", origAuthz) + } + } + + return +} + +// redactBody determines whether a given request or response body should +// be redacted, and returns a copy of the body to reattach to the +// request or response in question. +func redactBody(rc io.ReadCloser, rx *regexp.Regexp) (bool, io.ReadCloser, error) { + if rc == nil || rc == http.NoBody { + return true, http.NoBody, nil + } + + var content bytes.Buffer + if _, err := content.ReadFrom(rc); err != nil { + return false, rc, err + } + if err := rc.Close(); err != nil { + return false, rc, err + } + + return rx.Match(content.Bytes()), io.NopCloser(&content), nil +} + +func redactRequestBody(req *http.Request) (restore func()) { + restore = func() {} + + redactedReader := io.NopCloser(strings.NewReader(redactedString)) + + redact, origBody, _ := redactBody(req.Body, regexp.MustCompile(".*")) + + if redact { + origLength := req.ContentLength + + req.Body = redactedReader + req.ContentLength = int64(len(redactedString)) + + restore = func() { + req.Body = origBody + req.ContentLength = origLength + } + } else { + req.Body = origBody + } + + return +} + +func redactResponseBody(res *http.Response) (restore func()) { + restore = func() {} + + redactedReader := io.NopCloser(strings.NewReader(redactedString)) + + redact, origBody, _ := redactBody(res.Body, regexp.MustCompile("{\"protected\":\".*\",\"payload\":\".*\",\"signature\":\".*\"}")) + + if redact { + origLength := res.ContentLength + + res.Body = redactedReader + res.ContentLength = int64(len(redactedString)) + + restore = func() { + res.Body = origBody + res.ContentLength = origLength + } + } else { + res.Body = origBody + } + + return +} + +// dumpRequest logs the contents of a given HTTP request, but first: +// 1. sanitizes the Authorization header +// 2. sanitizes the request body if the request is for authentication +func (d *dumpTransport) dumpRequest(req *http.Request) []byte { + restoreAuthz := redactAuthz(req) + defer restoreAuthz() + + if strings.Contains(req.URL.Path, "/authn") { + restoreBody := redactRequestBody(req) + defer restoreBody() + } + dump, _ := httputil.DumpRequestOut(req, true) + return dump +} + +// dumpResponse logs the contents of a given HTTP response, but first +// sanitizes the response body if it includes a Conjur token. +func (d *dumpTransport) dumpResponse(res *http.Response) []byte { + restoreBody := redactResponseBody(res) + defer restoreBody() + + dump, _ := httputil.DumpResponse(res, true) + return dump +} + +func (d *dumpTransport) RoundTrip(req *http.Request) (*http.Response, error) { + dump := d.dumpRequest(req) if d.logRequest != nil { d.logRequest(dump) } + res, err := d.roundTripper.RoundTrip(req) if err != nil { if d.logResponse != nil { @@ -23,10 +139,12 @@ func (d *dumpTransport) RoundTrip(req *http.Request) (*http.Response, error) { } return res, err } - dump, _ = httputil.DumpResponse(res, true) + + dump = d.dumpResponse(res) if d.logResponse != nil { d.logResponse(dump) } + return res, err } diff --git a/pkg/utils/http_dump_transport_test.go b/pkg/utils/http_dump_transport_test.go new file mode 100644 index 0000000..9c760c3 --- /dev/null +++ b/pkg/utils/http_dump_transport_test.go @@ -0,0 +1,110 @@ +package utils + +import ( + "bytes" + "fmt" + "io/ioutil" + "net/http" + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestDumpTransport(t *testing.T) { + reqTestCases := []struct { + description string + path string + headers map[string]string + body string + assert func(t *testing.T, req *http.Request, dump string) + }{ + { + description: "Only Authz header is redacted", + headers: map[string]string{ + "Authorization": "some-token", + "Other-Header": "other-value", + }, + assert: func(t *testing.T, req *http.Request, dump string) { + assert.NotContains(t, dump, "some-token") + assert.Contains(t, dump, "other-value") + + assert.Equal(t, req.Header.Get("Authorization"), "some-token") + assert.Equal(t, req.Header.Get("Other-Header"), "other-value") + }, + }, + { + description: "Request body is redacted on authentication requests", + path: "/authn-xyz/account/login", + body: "some-body", + assert: func(t *testing.T, req *http.Request, dump string) { + assert.Contains(t, dump, redactedString) + assert.NotContains(t, dump, "some-body") + + reqBody, err := ioutil.ReadAll(req.Body) + assert.Nil(t, err) + assert.Equal(t, string(reqBody), "some-body") + }, + }, + { + description: "Request body is maintained on other requests", + body: "some-body", + assert: func(t *testing.T, req *http.Request, dump string) { + assert.Contains(t, dump, "some-body") + }, + }, + } + + for _, tc := range reqTestCases { + t.Run(tc.description, func(t *testing.T) { + req, err := http.NewRequest( + "POST", + fmt.Sprintf("http://somehost.com%s", tc.path), + bytes.NewBuffer([]byte(tc.body)), + ) + assert.Nil(t, err) + for k, v := range tc.headers { + req.Header.Add(k, v) + } + + dump := NewDumpTransport(nil, nil).dumpRequest(req) + tc.assert(t, req, string(dump)) + }) + } + + respTestCases := []struct { + description string + body string + assert func(t *testing.T, res *http.Response, dump string) + }{ + { + description: "Body is redacted if it contains a Conjur token", + body: "{\"protected\":\"abcde\",\"payload\":\"fghijk\",\"signature\":\"lmnop\"}", + assert: func(t *testing.T, res *http.Response, dump string) { + assert.Contains(t, dump, redactedString) + assert.NotContains(t, dump, "{\"protected\":\"abcde\",\"payload\":\"fghijk\",\"signature\":\"lmnop\"}") + + reqBody, err := ioutil.ReadAll(res.Body) + assert.Nil(t, err) + assert.Contains(t, string(reqBody), "{\"protected\":\"abcde\",\"payload\":\"fghijk\",\"signature\":\"lmnop\"}") + }, + }, + { + description: "Body is maintained otherwise", + body: "some-body", + assert: func(t *testing.T, res *http.Response, dump string) { + assert.Contains(t, dump, "some-body") + }, + }, + } + + for _, tc := range respTestCases { + t.Run(tc.description, func(t *testing.T) { + resp := http.Response{ + Body: ioutil.NopCloser(bytes.NewBufferString(tc.body)), + } + + dump := NewDumpTransport(nil, nil).dumpResponse(&resp) + tc.assert(t, &resp, string(dump)) + }) + } +}