Merge commit from fork

[19.03] AuthZ plugin security fixes

pkg/archive/archive_unix.go

@@ -81,11 +81,6 @@ func getFileUIDGID(stat interface{}) (idtools.Identity, error) {
 // handleTarTypeBlockCharFifo is an OS-specific helper function used by
 // createTarFile to handle the following types of header: Block; Char; Fifo
 func handleTarTypeBlockCharFifo(hdr *tar.Header, path string) error {
-	if rsystem.RunningInUserNS() {
-		// cannot create a device if running in user namespace
-		return nil
-	}
-
 	mode := uint32(hdr.Mode & 07777)
 	switch hdr.Typeflag {
 	case tar.TypeBlock:
@@ -96,7 +91,12 @@ func handleTarTypeBlockCharFifo(hdr *tar.Header, path string) error {
 		mode |= unix.S_IFIFO
 	}
 
-	return system.Mknod(path, mode, int(system.Mkdev(hdr.Devmajor, hdr.Devminor)))
+	err := system.Mknod(path, mode, int(system.Mkdev(hdr.Devmajor, hdr.Devminor)))
+	if errors.Is(err, syscall.EPERM) && rsystem.RunningInUserNS() {
+		// In most cases, cannot create a device if running in user namespace
+		err = nil
+	}
+	return err
 }
 
 func handleLChmod(hdr *tar.Header, path string, hdrInfo os.FileInfo) error {

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.EqualFold(r.Header.Get("Transfer-Encoding"), "chunked") {
+		return true
+	}
+	for _, v := range r.TransferEncoding {
+		if strings.EqualFold(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
 	}
 

pkg/authorization/authz_unix_test.go

@@ -174,8 +174,8 @@ func TestDrainBody(t *testing.T) {
 
 func TestSendBody(t *testing.T) {
 	var (
-		url       = "nothing.com"
 		testcases = []struct {
+			url         string
 			contentType string
 			expected    bool
 		}{
@@ -219,15 +219,93 @@ 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,
+			},
+			{
+				url:         "www.nothing.com/v1.24/auth/test",
+				contentType: "application/json;charset=UTF8",
+				expected:    false,
+			},
+			{
+				url:         "https://www.nothing.com/v1.24/auth/test",
+				contentType: "application/json;charset=UTF8",
+				expected:    false,
+			},
+			{
+				url:         "http://nothing.com/v1.24/auth/test",
+				contentType: "application/json;charset=UTF8",
+				expected:    false,
+			},
+			{
+				url:         "www.nothing.com/test?p1=/auth",
+				contentType: "application/json;charset=UTF8",
+				expected:    true,
+			},
+			{
+				url:         "http://www.nothing.com/test?p1=/auth",
+				contentType: "application/json;charset=UTF8",
+				expected:    true,
+			},
+			{
+				url:         "www.nothing.com/something/auth",
+				contentType: "application/json;charset=UTF8",
+				expected:    true,
+			},
+			{
+				url:         "https://www.nothing.com/something/auth",
+				contentType: "application/json;charset=UTF8",
+				expected:    true,
+			},
 		}
 	)
 
 	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)
 		}
 	}
 }