diff --git a/authorization/authz.go b/authorization/authz.go index 8a15b2b..f703908 100644 --- a/authorization/authz.go +++ b/authorization/authz.go @@ -54,13 +54,11 @@ type Ctx struct { // 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) { - if r.ContentLength < maxBodySize { - var err error - body, r.Body, err = drainBody(r.Body) - if err != nil { - return err - } + if sendBody(ctx.requestURI, r.Header) && r.ContentLength > 0 && r.ContentLength < maxBodySize { + var err error + body, r.Body, err = drainBody(r.Body) + if err != nil { + return err } } @@ -121,23 +119,23 @@ func (ctx *Ctx) AuthZResponse(rm ResponseModifier, r *http.Request) error { return nil } -// drainBody dump the body, it reads the body data into memory and -// see go sources /go/src/net/http/httputil/dump.go +// drainBody dump the body (if it's length is less than 1MB) without modifying the request state func drainBody(body io.ReadCloser) ([]byte, io.ReadCloser, error) { bufReader := bufio.NewReaderSize(body, maxBodySize) newBody := ioutils.NewReadCloserWrapper(bufReader, func() error { return body.Close() }) data, err := bufReader.Peek(maxBodySize) - if err != io.EOF { - // This means the request is larger than our max - if err == bufio.ErrBufferFull { - return nil, newBody, nil - } - // This means we had an error reading - return nil, nil, err + // Body size exceeds max body size + if err == nil { + logrus.Warnf("Request body is larger than: '%d' skipping body", maxBodySize) + return nil, newBody, nil } - - return data, newBody, nil + // Body size is less than maximum size + if err == io.EOF { + return data, newBody, nil + } + // Unknown error + return nil, newBody, err } // sendBody returns true when request/response body should be sent to AuthZPlugin @@ -148,8 +146,7 @@ func sendBody(url string, header http.Header) bool { } // body is sent only for text or json messages - v := header.Get("Content-Type") - return strings.HasPrefix(v, "text/") || v == "application/json" + return header.Get("Content-Type") == "application/json" } // headers returns flatten version of the http headers excluding authorization diff --git a/authorization/authz_unix_test.go b/authorization/authz_unix_test.go index a2487ef..b79e3f2 100644 --- a/authorization/authz_unix_test.go +++ b/authorization/authz_unix_test.go @@ -17,9 +17,11 @@ import ( "reflect" "testing" + "bytes" "github.com/docker/docker/pkg/plugins" "github.com/docker/go-connections/tlsconfig" "github.com/gorilla/mux" + "strings" ) const pluginAddress = "authzplugin.sock" @@ -135,6 +137,41 @@ func TestResponseModifier(t *testing.T) { } } +func TestDrainBody(t *testing.T) { + + tests := []struct { + length int // length is the message length send to drainBody + expectedBodyLength int // expectedBodyLength is the expected body length after drainBody is called + }{ + {10, 10}, // Small message size + {maxBodySize - 1, maxBodySize - 1}, // Max message size + {maxBodySize * 2, 0}, // Large message size (skip copying body) + + } + + for _, test := range tests { + + msg := strings.Repeat("a", test.length) + body, closer, err := drainBody(ioutil.NopCloser(bytes.NewReader([]byte(msg)))) + if len(body) != test.expectedBodyLength { + t.Fatalf("Body must be copied, actual length: '%d'", len(body)) + } + if closer == nil { + t.Fatalf("Closer must not be nil") + } + if err != nil { + t.Fatalf("Error must not be nil: '%v'", err) + } + modified, err := ioutil.ReadAll(closer) + if err != nil { + t.Fatalf("Error must not be nil: '%v'", err) + } + if len(modified) != len(msg) { + t.Fatalf("Result should not be truncated. Original length: '%d', new length: '%d'", len(msg), len(modified)) + } + } +} + func TestResponseModifierOverride(t *testing.T) { r := httptest.NewRecorder() m := NewResponseModifier(r)