From 88c4b7690840044ce15489699294ec7c5dadf5dd Mon Sep 17 00:00:00 2001 From: Jameson Hyde Date: Mon, 26 Nov 2018 14:15:22 -0500 Subject: CVE-2024-41110 [PATCH] Authz plugin security fixes for 0-length content and path validation Signed-off-by: Jameson Hyde MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit fix comments [debian description] A security vulnerability has been detected in certain versions of Docker Engine, which could allow an attacker to bypass authorization plugins (AuthZ) under specific circumstances. The base likelihood of this being exploited is low. (cherry picked from commit 9659c3a52bac57e615b5fb49b0652baca448643e) Signed-off-by: Paweł Gronowski (cherry picked from commit 2ac8a479c53d9b8e67c55f1e283da9d85d2b3415) Signed-off-by: Paweł Gronowski origin: https://github.com/moby/moby/commit/88c4b7690840044ce15489699294ec7c5dadf5dd debian-bug: https://bugs.debian.org/1084993 bug: https://github.com/moby/moby/security/advisories/GHSA-v23v-6jw2-98fq --- pkg/authorization/authz.go | 38 ++++++++++++++++++--- pkg/authorization/authz_unix_test.go | 49 ++++++++++++++++++++++++++-- 2 files changed, 80 insertions(+), 7 deletions(-) diff --git a/pkg/authorization/authz.go b/pkg/authorization/authz.go index 590ac8dddd883..da748865dd9e2 100644 --- a/engine/pkg/authorization/authz.go +++ b/engine/pkg/authorization/authz.go @@ -7,6 +7,8 @@ import ( "io" "mime" "net/http" + "net/url" + "regexp" "strings" "github.com/docker/docker/pkg/ioutils" @@ -52,10 +54,23 @@ type Ctx struct { authReq *Request } +func isChunked(r *http.Request) bool { + // RFC 7230 specifies that content length is to be ignored if Transfer-Encoding is chunked + if strings.ToLower(r.Header.Get("Transfer-Encoding")) == "chunked" { + return true + } + for _, v := range r.TransferEncoding { + if 0 == strings.Compare(strings.ToLower(v), "chunked") { + return true + } + } + return false +} + // AuthZRequest authorized the request to the docker daemon using authZ plugins func (ctx *Ctx) AuthZRequest(w http.ResponseWriter, r *http.Request) error { var body []byte - if sendBody(ctx.requestURI, r.Header) && r.ContentLength > 0 && r.ContentLength < maxBodySize { + if sendBody(ctx.requestURI, r.Header) && (r.ContentLength > 0 || isChunked(r)) && r.ContentLength < maxBodySize { var err error body, r.Body, err = drainBody(r.Body) if err != nil { @@ -108,7 +123,6 @@ func (ctx *Ctx) AuthZResponse(rm ResponseModifier, r *http.Request) error { if sendBody(ctx.requestURI, rm.Header()) { ctx.authReq.ResponseBody = rm.RawBody() } - for _, plugin := range ctx.plugins { logrus.Debugf("AuthZ response using plugin %s", plugin.Name()) @@ -146,10 +160,26 @@ func drainBody(body io.ReadCloser) ([]byte, io.ReadCloser, error) { return nil, newBody, err } +func isAuthEndpoint(urlPath string) (bool, error) { + // eg www.test.com/v1.24/auth/optional?optional1=something&optional2=something (version optional) + matched, err := regexp.MatchString(`^[^\/]+\/(v\d[\d\.]*\/)?auth.*`, urlPath) + if err != nil { + return false, err + } + return matched, nil +} + // sendBody returns true when request/response body should be sent to AuthZPlugin -func sendBody(url string, header http.Header) bool { +func sendBody(inURL string, header http.Header) bool { + u, err := url.Parse(inURL) + // Assume no if the URL cannot be parsed - an empty request will still be forwarded to the plugin and should be rejected + if err != nil { + return false + } + // Skip body for auth endpoint - if strings.HasSuffix(url, "/auth") { + isAuth, err := isAuthEndpoint(u.Path) + if isAuth || err != nil { return false } diff --git a/pkg/authorization/authz_unix_test.go b/pkg/authorization/authz_unix_test.go index 835cb703839be..1fce6d03b76a8 100644 --- a/engine/pkg/authorization/authz_unix_test.go +++ b/engine/pkg/authorization/authz_unix_test.go @@ -175,8 +175,8 @@ func TestDrainBody(t *testing.T) { func TestSendBody(t *testing.T) { var ( - url = "nothing.com" testcases = []struct { + url string contentType string expected bool }{ @@ -220,15 +220,58 @@ func TestSendBody(t *testing.T) { contentType: "", expected: false, }, + { + url: "nothing.com/auth", + contentType: "", + expected: false, + }, + { + url: "nothing.com/auth", + contentType: "application/json;charset=UTF8", + expected: false, + }, + { + url: "nothing.com/auth?p1=test", + contentType: "application/json;charset=UTF8", + expected: false, + }, + { + url: "nothing.com/test?p1=/auth", + contentType: "application/json;charset=UTF8", + expected: true, + }, + { + url: "nothing.com/something/auth", + contentType: "application/json;charset=UTF8", + expected: true, + }, + { + url: "nothing.com/auth/test", + contentType: "application/json;charset=UTF8", + expected: false, + }, + { + url: "nothing.com/v1.24/auth/test", + contentType: "application/json;charset=UTF8", + expected: false, + }, + { + url: "nothing.com/v1/auth/test", + contentType: "application/json;charset=UTF8", + expected: false, + }, } ) for _, testcase := range testcases { header := http.Header{} header.Set("Content-Type", testcase.contentType) + if testcase.url == "" { + testcase.url = "nothing.com" + } - if b := sendBody(url, header); b != testcase.expected { - t.Fatalf("Unexpected Content-Type; Expected: %t, Actual: %t", testcase.expected, b) + if b := sendBody(testcase.url, header); b != testcase.expected { + t.Fatalf("sendBody failed: url: %s, content-type: %s; Expected: %t, Actual: %t", testcase.url, testcase.contentType, testcase.expected, b) } } }