Browse code

Authz plugin security fixes for 0-length content and path validation Signed-off-by: Jameson Hyde <jameson.hyde@docker.com>

fix comments

(cherry picked from commit 9659c3a52bac57e615b5fb49b0652baca448643e)
Signed-off-by: Paweł Gronowski <pawel.gronowski@docker.com>

Jameson Hyde authored on 2018/11/27 04:15:22
Showing 2 changed files
... ...
@@ -8,6 +8,8 @@ import (
8 8
 	"io"
9 9
 	"mime"
10 10
 	"net/http"
11
+	"net/url"
12
+	"regexp"
11 13
 	"strings"
12 14
 
13 15
 	"github.com/containerd/log"
... ...
@@ -53,10 +55,23 @@ type Ctx struct {
53 53
 	authReq *Request
54 54
 }
55 55
 
56
+func isChunked(r *http.Request) bool {
57
+	// RFC 7230 specifies that content length is to be ignored if Transfer-Encoding is chunked
58
+	if strings.ToLower(r.Header.Get("Transfer-Encoding")) == "chunked" {
59
+		return true
60
+	}
61
+	for _, v := range r.TransferEncoding {
62
+		if 0 == strings.Compare(strings.ToLower(v), "chunked") {
63
+			return true
64
+		}
65
+	}
66
+	return false
67
+}
68
+
56 69
 // AuthZRequest authorized the request to the docker daemon using authZ plugins
57 70
 func (ctx *Ctx) AuthZRequest(w http.ResponseWriter, r *http.Request) error {
58 71
 	var body []byte
59
-	if sendBody(ctx.requestURI, r.Header) && r.ContentLength > 0 && r.ContentLength < maxBodySize {
72
+	if sendBody(ctx.requestURI, r.Header) && (r.ContentLength > 0 || isChunked(r)) && r.ContentLength < maxBodySize {
60 73
 		var err error
61 74
 		body, r.Body, err = drainBody(r.Body)
62 75
 		if err != nil {
... ...
@@ -109,7 +124,6 @@ func (ctx *Ctx) AuthZResponse(rm ResponseModifier, r *http.Request) error {
109 109
 	if sendBody(ctx.requestURI, rm.Header()) {
110 110
 		ctx.authReq.ResponseBody = rm.RawBody()
111 111
 	}
112
-
113 112
 	for _, plugin := range ctx.plugins {
114 113
 		log.G(context.TODO()).Debugf("AuthZ response using plugin %s", plugin.Name())
115 114
 
... ...
@@ -147,10 +161,26 @@ func drainBody(body io.ReadCloser) ([]byte, io.ReadCloser, error) {
147 147
 	return nil, newBody, err
148 148
 }
149 149
 
150
+func isAuthEndpoint(urlPath string) (bool, error) {
151
+	// eg www.test.com/v1.24/auth/optional?optional1=something&optional2=something (version optional)
152
+	matched, err := regexp.MatchString(`^[^\/]+\/(v\d[\d\.]*\/)?auth.*`, urlPath)
153
+	if err != nil {
154
+		return false, err
155
+	}
156
+	return matched, nil
157
+}
158
+
150 159
 // sendBody returns true when request/response body should be sent to AuthZPlugin
151
-func sendBody(url string, header http.Header) bool {
160
+func sendBody(inURL string, header http.Header) bool {
161
+	u, err := url.Parse(inURL)
162
+	// Assume no if the URL cannot be parsed - an empty request will still be forwarded to the plugin and should be rejected
163
+	if err != nil {
164
+		return false
165
+	}
166
+
152 167
 	// Skip body for auth endpoint
153
-	if strings.HasSuffix(url, "/auth") {
168
+	isAuth, err := isAuthEndpoint(u.Path)
169
+	if isAuth || err != nil {
154 170
 		return false
155 171
 	}
156 172
 
... ...
@@ -174,8 +174,8 @@ func TestDrainBody(t *testing.T) {
174 174
 
175 175
 func TestSendBody(t *testing.T) {
176 176
 	var (
177
-		url       = "nothing.com"
178 177
 		testcases = []struct {
178
+			url         string
179 179
 			contentType string
180 180
 			expected    bool
181 181
 		}{
... ...
@@ -219,15 +219,58 @@ func TestSendBody(t *testing.T) {
219 219
 				contentType: "",
220 220
 				expected:    false,
221 221
 			},
222
+			{
223
+				url:         "nothing.com/auth",
224
+				contentType: "",
225
+				expected:    false,
226
+			},
227
+			{
228
+				url:         "nothing.com/auth",
229
+				contentType: "application/json;charset=UTF8",
230
+				expected:    false,
231
+			},
232
+			{
233
+				url:         "nothing.com/auth?p1=test",
234
+				contentType: "application/json;charset=UTF8",
235
+				expected:    false,
236
+			},
237
+			{
238
+				url:         "nothing.com/test?p1=/auth",
239
+				contentType: "application/json;charset=UTF8",
240
+				expected:    true,
241
+			},
242
+			{
243
+				url:         "nothing.com/something/auth",
244
+				contentType: "application/json;charset=UTF8",
245
+				expected:    true,
246
+			},
247
+			{
248
+				url:         "nothing.com/auth/test",
249
+				contentType: "application/json;charset=UTF8",
250
+				expected:    false,
251
+			},
252
+			{
253
+				url:         "nothing.com/v1.24/auth/test",
254
+				contentType: "application/json;charset=UTF8",
255
+				expected:    false,
256
+			},
257
+			{
258
+				url:         "nothing.com/v1/auth/test",
259
+				contentType: "application/json;charset=UTF8",
260
+				expected:    false,
261
+			},
222 262
 		}
223 263
 	)
224 264
 
225 265
 	for _, testcase := range testcases {
226 266
 		header := http.Header{}
227 267
 		header.Set("Content-Type", testcase.contentType)
268
+		if testcase.url == "" {
269
+			testcase.url = "nothing.com"
270
+		}
228 271
 
229
-		if b := sendBody(url, header); b != testcase.expected {
230
-			t.Fatalf("Unexpected Content-Type; Expected: %t, Actual: %t", testcase.expected, b)
272
+		if b := sendBody(testcase.url, header); b != testcase.expected {
273
+			t.Fatalf("sendBody failed: url: %s, content-type: %s; Expected: %t, Actual: %t", testcase.url, testcase.contentType, testcase.expected, b)
231 274
 		}
232 275
 	}
233 276
 }