From f0b1f1c2f67877ddb2eceac5eb7c9c4ea22b4b6b Mon Sep 17 00:00:00 2001 From: xuzhonghu Date: Wed, 20 Jun 2018 11:15:09 +0800 Subject: [PATCH] limit User-Agent max length 1024 and add ...TRUNCATED suffix --- .../src/k8s.io/apiserver/pkg/audit/request.go | 20 ++++++++++++++++--- .../apiserver/pkg/audit/request_test.go | 18 +++++++++++++++++ 2 files changed, 35 insertions(+), 3 deletions(-) diff --git a/staging/src/k8s.io/apiserver/pkg/audit/request.go b/staging/src/k8s.io/apiserver/pkg/audit/request.go index f62b0359f53..9593b6c8ab4 100644 --- a/staging/src/k8s.io/apiserver/pkg/audit/request.go +++ b/staging/src/k8s.io/apiserver/pkg/audit/request.go @@ -37,16 +37,20 @@ import ( "k8s.io/apiserver/pkg/authorization/authorizer" ) +const ( + maxUserAgentLength = 1024 + userAgentTruncateSuffix = "...TRUNCATED" +) + func NewEventFromRequest(req *http.Request, level auditinternal.Level, attribs authorizer.Attributes) (*auditinternal.Event, error) { ev := &auditinternal.Event{ RequestReceivedTimestamp: metav1.NewMicroTime(time.Now()), Verb: attribs.GetVerb(), RequestURI: req.URL.RequestURI(), - UserAgent: req.UserAgent(), + UserAgent: maybeTruncateUserAgent(req), + Level: level, } - ev.Level = level - // prefer the id from the headers. If not available, create a new one. // TODO(audit): do we want to forbid the header for non-front-proxy users? ids := req.Header.Get(auditinternal.HeaderAuditID) @@ -234,3 +238,13 @@ func LogAnnotations(ae *auditinternal.Event, annotations map[string]string) { LogAnnotation(ae, key, value) } } + +// truncate User-Agent if too long, otherwise return it directly. +func maybeTruncateUserAgent(req *http.Request) string { + ua := req.UserAgent() + if len(ua) > maxUserAgentLength { + ua = ua[:maxUserAgentLength] + userAgentTruncateSuffix + } + + return ua +} diff --git a/staging/src/k8s.io/apiserver/pkg/audit/request_test.go b/staging/src/k8s.io/apiserver/pkg/audit/request_test.go index b8bfacbdacd..12c36ccd669 100644 --- a/staging/src/k8s.io/apiserver/pkg/audit/request_test.go +++ b/staging/src/k8s.io/apiserver/pkg/audit/request_test.go @@ -17,9 +17,11 @@ limitations under the License. package audit import ( + "net/http" "testing" "github.com/stretchr/testify/assert" + auditinternal "k8s.io/apiserver/pkg/apis/audit" ) @@ -36,3 +38,19 @@ func TestLogAnnotation(t *testing.T) { LogAnnotation(ev, "qux", "baz") assert.Equal(t, "", ev.Annotations["qux"], "audit annotation should not be overwritten.") } + +func TestMaybeTruncateUserAgent(t *testing.T) { + req := &http.Request{} + req.Header = http.Header{} + + ua := "short-agent" + req.Header.Set("User-Agent", ua) + assert.Equal(t, ua, maybeTruncateUserAgent(req)) + + ua = "" + for i := 0; i < maxUserAgentLength*2; i++ { + ua = ua + "a" + } + req.Header.Set("User-Agent", ua) + assert.NotEqual(t, ua, maybeTruncateUserAgent(req)) +}