Apply caching limits to authorized requests too

This commit is contained in:
Tim Allclair 2019-01-24 13:37:30 -08:00
parent e23c15a0f3
commit d512173c86
2 changed files with 14 additions and 11 deletions

View File

@ -41,8 +41,8 @@ var (
const ( const (
retryBackoff = 500 * time.Millisecond retryBackoff = 500 * time.Millisecond
// The maximum key length for unauthorized request keys to qualify for caching. // The maximum length of requester-controlled attributes to allow caching.
maxUnauthorizedCachedKeySize = 10000 maxControlledAttrCacheSize = 10000
) )
// Ensure Webhook implements the authorizer.Authorizer interface. // Ensure Webhook implements the authorizer.Authorizer interface.
@ -197,10 +197,10 @@ func (w *WebhookAuthorizer) Authorize(attr authorizer.Attributes) (decision auth
return w.decisionOnError, "", err return w.decisionOnError, "", err
} }
r.Status = result.Status r.Status = result.Status
if r.Status.Allowed { if shouldCache(attr) {
w.responseCache.Add(string(key), r.Status, w.authorizedTTL) if r.Status.Allowed {
} else { w.responseCache.Add(string(key), r.Status, w.authorizedTTL)
if callerControlledAttributeSize(attr) < maxUnauthorizedCachedKeySize { } else {
w.responseCache.Add(string(key), r.Status, w.unauthorizedTTL) w.responseCache.Add(string(key), r.Status, w.unauthorizedTTL)
} }
} }
@ -269,8 +269,10 @@ func (t *subjectAccessReviewClient) Create(subjectAccessReview *authorization.Su
return result, err return result, err
} }
func callerControlledAttributeSize(attr authorizer.Attributes) int64 { // shouldCache determines whether it is safe to cache the given request attributes. If the
return int64(len(attr.GetNamespace())) + // requester-controlled attributes are too large, this may be a DoS attempt, so we skip the cache.
func shouldCache(attr authorizer.Attributes) bool {
controlledAttrSize := int64(len(attr.GetNamespace())) +
int64(len(attr.GetVerb())) + int64(len(attr.GetVerb())) +
int64(len(attr.GetAPIGroup())) + int64(len(attr.GetAPIGroup())) +
int64(len(attr.GetAPIVersion())) + int64(len(attr.GetAPIVersion())) +
@ -278,4 +280,5 @@ func callerControlledAttributeSize(attr authorizer.Attributes) int64 {
int64(len(attr.GetSubresource())) + int64(len(attr.GetSubresource())) +
int64(len(attr.GetName())) + int64(len(attr.GetName())) +
int64(len(attr.GetPath())) int64(len(attr.GetPath()))
return controlledAttrSize < maxControlledAttrCacheSize
} }

View File

@ -616,10 +616,10 @@ func TestWebhookCache(t *testing.T) {
{name: "ridiculous unauthorized request", attr: bobRidiculousAttr, allow: false, statusCode: 200, expectedErr: false, expectedAuthorized: false, expectedCalls: 1}, {name: "ridiculous unauthorized request", attr: bobRidiculousAttr, allow: false, statusCode: 200, expectedErr: false, expectedAuthorized: false, expectedCalls: 1},
// later ridiculous requests within the cache window still hit the backend // later ridiculous requests within the cache window still hit the backend
{name: "ridiculous unauthorized request again", attr: bobRidiculousAttr, allow: false, statusCode: 200, expectedErr: false, expectedAuthorized: false, expectedCalls: 1}, {name: "ridiculous unauthorized request again", attr: bobRidiculousAttr, allow: false, statusCode: 200, expectedErr: false, expectedAuthorized: false, expectedCalls: 1},
// ridiculous authorized requests are still cached. // ridiculous authorized requests are not cached.
{name: "ridiculous authorized request", attr: aliceRidiculousAttr, allow: true, statusCode: 200, expectedErr: false, expectedAuthorized: true, expectedCalls: 1}, {name: "ridiculous authorized request", attr: aliceRidiculousAttr, allow: true, statusCode: 200, expectedErr: false, expectedAuthorized: true, expectedCalls: 1},
// later ridiculous requests within the cache window don't hit the backend // later ridiculous requests within the cache window still hit the backend
{name: "ridiculous authorized request again", attr: aliceRidiculousAttr, allow: false, statusCode: 500, expectedErr: false, expectedAuthorized: true, expectedCalls: 0}, {name: "ridiculous authorized request again", attr: aliceRidiculousAttr, allow: true, statusCode: 200, expectedErr: false, expectedAuthorized: true, expectedCalls: 1},
} }
for i, test := range tests { for i, test := range tests {