From d0a9e9b4755efdaafd8e34a228898322ac14cc5b Mon Sep 17 00:00:00 2001 From: Stephen J Day Date: Thu, 18 Dec 2014 12:30:19 -0800 Subject: [PATCH] Integrate auth.AccessController into registry app This changeset integrates the AccessController into the main registry app. This includes support for configuration and a test implementation, called "silly" auth. Auth is only enabled if the configuration is present but takes measure to ensure that configuration errors don't allow the appserver to start with open access. --- api/v2/descriptors.go | 8 ++ api/v2/errors.go | 3 + app.go | 110 +++++++++++++++++++++++++--- app_test.go | 63 ++++++++++++++++ auth/silly/access.go | 89 ++++++++++++++++++++++ auth/silly/access_test.go | 58 +++++++++++++++ cmd/registry/main.go | 6 +- configuration/configuration.go | 71 +++++++++++++++++- configuration/configuration_test.go | 22 ++++++ 9 files changed, 413 insertions(+), 17 deletions(-) create mode 100644 auth/silly/access.go create mode 100644 auth/silly/access_test.go diff --git a/api/v2/descriptors.go b/api/v2/descriptors.go index 68d18241..97e41b74 100644 --- a/api/v2/descriptors.go +++ b/api/v2/descriptors.go @@ -39,6 +39,14 @@ var ErrorDescriptors = []ErrorDescriptor{ Description: `Generic error returned when the error does not have an API classification.`, }, + { + Code: ErrorCodeUnauthorized, + Value: "UNAUTHORIZED", + Message: "access to the requested resource is not authorized", + Description: `The access controller denied access for the operation on + a resource. Often this will be accompanied by a 401 Unauthorized + response status.`, + }, { Code: ErrorCodeDigestInvalid, Value: "DIGEST_INVALID", diff --git a/api/v2/errors.go b/api/v2/errors.go index 8c85d3a9..94c646fc 100644 --- a/api/v2/errors.go +++ b/api/v2/errors.go @@ -13,6 +13,9 @@ const ( // ErrorCodeUnknown is a catch-all for errors not defined below. ErrorCodeUnknown ErrorCode = iota + // ErrorCodeUnauthorized is returned if a request is not authorized. + ErrorCodeUnauthorized + // ErrorCodeDigestInvalid is returned when uploading a blob if the // provided digest does not match the blob contents. ErrorCodeDigestInvalid diff --git a/app.go b/app.go index 5a770c6c..7d1ee9bb 100644 --- a/app.go +++ b/app.go @@ -5,11 +5,11 @@ import ( "net/http" "github.com/docker/docker-registry/api/v2" - "github.com/docker/docker-registry/storagedriver" - "github.com/docker/docker-registry/storagedriver/factory" - + "github.com/docker/docker-registry/auth" "github.com/docker/docker-registry/configuration" "github.com/docker/docker-registry/storage" + "github.com/docker/docker-registry/storagedriver" + "github.com/docker/docker-registry/storagedriver/factory" log "github.com/Sirupsen/logrus" "github.com/gorilla/mux" @@ -28,6 +28,8 @@ type App struct { // services contains the main services instance for the application. services *storage.Services + + accessController auth.AccessController } // NewApp takes a configuration and returns a configured app, ready to serve @@ -61,6 +63,16 @@ func NewApp(configuration configuration.Configuration) *App { app.driver = driver app.services = storage.NewServices(app.driver) + authType := configuration.Auth.Type() + + if authType != "" { + accessController, err := auth.GetAccessController(configuration.Auth.Type(), configuration.Auth.Parameters()) + if err != nil { + panic(fmt.Sprintf("unable to configure authorization (%s): %v", authType, err)) + } + app.accessController = accessController + } + return app } @@ -111,15 +123,11 @@ func (ssrw *singleStatusResponseWriter) WriteHeader(status int) { // handler, using the dispatch factory function. func (app *App) dispatcher(dispatch dispatchFunc) http.Handler { return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - vars := mux.Vars(r) - context := &Context{ - App: app, - Name: vars["name"], - urlBuilder: v2.NewURLBuilderFromRequest(r), - } + context := app.context(r) - // Store vars for underlying handlers. - context.vars = vars + if err := app.authorized(w, r, context); err != nil { + return + } context.log = log.WithField("name", context.Name) handler := dispatch(context, r) @@ -140,6 +148,86 @@ func (app *App) dispatcher(dispatch dispatchFunc) http.Handler { }) } +// context constructs the context object for the application. This only be +// called once per request. +func (app *App) context(r *http.Request) *Context { + vars := mux.Vars(r) + context := &Context{ + App: app, + Name: vars["name"], + urlBuilder: v2.NewURLBuilderFromRequest(r), + } + + // Store vars for underlying handlers. + context.vars = vars + + return context +} + +// authorized checks if the request can proceed with with request access- +// level. If it cannot, the method will return an error. +func (app *App) authorized(w http.ResponseWriter, r *http.Request, context *Context) error { + if app.accessController == nil { + return nil // access controller is not enabled. + } + + var accessRecords []auth.Access + resource := auth.Resource{ + Type: "repository", + Name: context.Name, + } + + switch r.Method { + case "GET", "HEAD": + accessRecords = append(accessRecords, + auth.Access{ + Resource: resource, + Action: "pull", + }) + case "POST", "PUT", "PATCH": + accessRecords = append(accessRecords, + auth.Access{ + Resource: resource, + Action: "pull", + }, + auth.Access{ + Resource: resource, + Action: "push", + }) + case "DELETE": + // DELETE access requires full admin rights, which is represented + // as "*". This may not be ideal. + accessRecords = append(accessRecords, + auth.Access{ + Resource: resource, + Action: "*", + }) + } + + if err := app.accessController.Authorized(r, accessRecords...); err != nil { + switch err := err.(type) { + case auth.Challenge: + w.Header().Set("Content-Type", "application/json") + err.ServeHTTP(w, r) + + var errs v2.Errors + errs.Push(v2.ErrorCodeUnauthorized, accessRecords) + serveJSON(w, errs) + default: + // This condition is a potential security problem either in + // the configuration or whatever is backing the access + // controller. Just return a bad request with no information + // to avoid exposure. The request should not proceed. + context.log.Errorf("error checking authorization: %v", err) + w.WriteHeader(http.StatusBadRequest) + } + + return err + } + + return nil +} + // apiBase implements a simple yes-man for doing overall checks against the // api. This can support auth roundtrips to support docker login. func apiBase(w http.ResponseWriter, r *http.Request) { diff --git a/app_test.go b/app_test.go index f256c968..3e9d191d 100644 --- a/app_test.go +++ b/app_test.go @@ -1,12 +1,14 @@ package registry import ( + "encoding/json" "net/http" "net/http/httptest" "net/url" "testing" "github.com/docker/docker-registry/api/v2" + _ "github.com/docker/docker-registry/auth/silly" "github.com/docker/docker-registry/configuration" ) @@ -124,3 +126,64 @@ func TestAppDispatcher(t *testing.T) { } } } + +// TestNewApp covers the creation of an application via NewApp with a +// configuration. +func TestNewApp(t *testing.T) { + config := configuration.Configuration{ + Storage: configuration.Storage{ + "inmemory": nil, + }, + Auth: configuration.Auth{ + // For now, we simply test that new auth results in a viable + // application. + "silly": { + "realm": "realm-test", + "service": "service-test", + }, + }, + } + + // Mostly, with this test, given a sane configuration, we are simply + // ensuring that NewApp doesn't panic. We might want to tweak this + // behavior. + app := NewApp(config) + + server := httptest.NewServer(app) + builder, err := v2.NewURLBuilderFromString(server.URL) + if err != nil { + t.Fatalf("error creating urlbuilder: %v", err) + } + + baseURL, err := builder.BuildBaseURL() + if err != nil { + t.Fatalf("error creating baseURL: %v", err) + } + + // TODO(stevvooe): The rest of this test might belong in the API tests. + + // Just hit the app and make sure we get a 401 Unauthorized error. + req, err := http.Get(baseURL) + if err != nil { + t.Fatalf("unexpected error during GET: %v", err) + } + defer req.Body.Close() + + if req.StatusCode != http.StatusUnauthorized { + t.Fatalf("unexpected status code during request: %v", err) + } + + if req.Header.Get("Content-Type") != "application/json" { + t.Fatalf("unexpected content-type: %v != %v", req.Header.Get("Content-Type"), "application/json") + } + + var errs v2.Errors + dec := json.NewDecoder(req.Body) + if err := dec.Decode(&errs); err != nil { + t.Fatalf("error decoding error response: %v", err) + } + + if errs.Errors[0].Code != v2.ErrorCodeUnauthorized { + t.Fatalf("unexpected error code: %v != %v", errs.Errors[0].Code, v2.ErrorCodeUnauthorized) + } +} diff --git a/auth/silly/access.go b/auth/silly/access.go new file mode 100644 index 00000000..a747fb6d --- /dev/null +++ b/auth/silly/access.go @@ -0,0 +1,89 @@ +// Package silly provides a simple authentication scheme that checks for the +// existence of an Authorization header and issues access if is present and +// non-empty. +// +// This package is present as an example implementation of a minimal +// auth.AccessController and for testing. This is not suitable for any kind of +// production security. +package silly + +import ( + "fmt" + "net/http" + "strings" + + "github.com/docker/docker-registry/auth" +) + +// accessController provides a simple implementation of auth.AccessController +// that simply checks for a non-empty Authorization header. It is useful for +// demonstration and testing. +type accessController struct { + realm string + service string +} + +var _ auth.AccessController = &accessController{} + +func newAccessController(options map[string]interface{}) (auth.AccessController, error) { + realm, present := options["realm"] + if _, ok := realm.(string); !present || !ok { + return nil, fmt.Errorf(`"realm" must be set for silly access controller`) + } + + service, present := options["service"] + if _, ok := service.(string); !present || !ok { + return nil, fmt.Errorf(`"service" must be set for silly access controller`) + } + + return &accessController{realm: realm.(string), service: service.(string)}, nil +} + +// Authorized simply checks for the existence of the authorization header, +// responding with a bearer challenge if it doesn't exist. +func (ac *accessController) Authorized(req *http.Request, accessRecords ...auth.Access) error { + if req.Header.Get("Authorization") == "" { + challenge := challenge{ + realm: ac.realm, + service: ac.service, + } + + if len(accessRecords) > 0 { + var scopes []string + for _, access := range accessRecords { + scopes = append(scopes, fmt.Sprintf("%s:%s:%s", access.Type, access.Resource, access.Action)) + } + challenge.scope = strings.Join(scopes, " ") + } + + return &challenge + } + + return nil +} + +type challenge struct { + realm string + service string + scope string +} + +func (ch *challenge) ServeHTTP(w http.ResponseWriter, r *http.Request) { + header := fmt.Sprintf("Bearer realm=%q,service=%q", ch.realm, ch.service) + + if ch.scope != "" { + header = fmt.Sprintf("%s,scope=%q", header, ch.scope) + } + + w.Header().Set("Authorization", header) + w.WriteHeader(http.StatusUnauthorized) +} + +func (ch *challenge) Error() string { + return fmt.Sprintf("silly authentication challenge: %#v", ch) +} + +// init registers the silly auth backend. +func init() { + auth.Register("silly", auth.InitFunc(newAccessController)) +} diff --git a/auth/silly/access_test.go b/auth/silly/access_test.go new file mode 100644 index 00000000..a412c101 --- /dev/null +++ b/auth/silly/access_test.go @@ -0,0 +1,58 @@ +package silly + +import ( + "net/http" + "net/http/httptest" + "testing" + + "github.com/docker/docker-registry/auth" +) + +func TestSillyAccessController(t *testing.T) { + ac := &accessController{ + realm: "test-realm", + service: "test-service", + } + + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if err := ac.Authorized(r); err != nil { + switch err := err.(type) { + case auth.Challenge: + err.ServeHTTP(w, r) + return + default: + t.Fatalf("unexpected error authorizing request: %v", err) + } + } + + w.WriteHeader(http.StatusNoContent) + })) + + resp, err := http.Get(server.URL) + if err != nil { + t.Fatalf("unexpected error during GET: %v", err) + } + defer resp.Body.Close() + + // Request should not be authorized + if resp.StatusCode != http.StatusUnauthorized { + t.Fatalf("unexpected response status: %v != %v", resp.StatusCode, http.StatusUnauthorized) + } + + req, err := http.NewRequest("GET", server.URL, nil) + if err != nil { + t.Fatalf("unexpected error creating new request: %v", err) + } + req.Header.Set("Authorization", "seriously, anything") + + resp, err = http.DefaultClient.Do(req) + if err != nil { + t.Fatalf("unexpected error during GET: %v", err) + } + defer resp.Body.Close() + + // Request should not be authorized + if resp.StatusCode != http.StatusNoContent { + t.Fatalf("unexpected response status: %v != %v", resp.StatusCode, http.StatusNoContent) + } +} diff --git a/cmd/registry/main.go b/cmd/registry/main.go index ba859eec..ea20a916 100644 --- a/cmd/registry/main.go +++ b/cmd/registry/main.go @@ -7,14 +7,14 @@ import ( _ "net/http/pprof" "os" - "github.com/gorilla/handlers" - log "github.com/Sirupsen/logrus" - "github.com/bugsnag/bugsnag-go" + "github.com/gorilla/handlers" "github.com/yvasiyarov/gorelic" "github.com/docker/docker-registry" + _ "github.com/docker/docker-registry/auth/silly" + _ "github.com/docker/docker-registry/auth/token" "github.com/docker/docker-registry/configuration" _ "github.com/docker/docker-registry/storagedriver/filesystem" _ "github.com/docker/docker-registry/storagedriver/inmemory" diff --git a/configuration/configuration.go b/configuration/configuration.go index 8e68190d..6ac64147 100644 --- a/configuration/configuration.go +++ b/configuration/configuration.go @@ -20,6 +20,10 @@ type Configuration struct { // Storage is the configuration for the registry's storage driver Storage Storage `yaml:"storage"` + // Auth allows configuration of various authorization methods that may be + // used to gate requests. + Auth Auth `yaml:"auth"` + // Reporting is the configuration for error reporting Reporting Reporting `yaml:"reporting"` @@ -85,6 +89,9 @@ func (loglevel *Loglevel) UnmarshalYAML(unmarshal func(interface{}) error) error return nil } +// Parameters defines a key-value parameters mapping +type Parameters map[string]interface{} + // Storage defines the configuration for registry object storage type Storage map[string]Parameters @@ -137,13 +144,71 @@ func (storage *Storage) UnmarshalYAML(unmarshal func(interface{}) error) error { // MarshalYAML implements the yaml.Marshaler interface func (storage Storage) MarshalYAML() (interface{}, error) { if storage.Parameters() == nil { - return storage.Type, nil + return storage.Type(), nil } return map[string]Parameters(storage), nil } -// Parameters defines a key-value parameters mapping -type Parameters map[string]interface{} +// Auth defines the configuration for registry authorization. +type Auth map[string]Parameters + +// Type returns the storage driver type, such as filesystem or s3 +func (auth Auth) Type() string { + // Return only key in this map + for k := range auth { + return k + } + return "" +} + +// Parameters returns the Parameters map for an Auth configuration +func (auth Auth) Parameters() Parameters { + return auth[auth.Type()] +} + +// setParameter changes the parameter at the provided key to the new value +func (auth Auth) setParameter(key string, value interface{}) { + auth[auth.Type()][key] = value +} + +// UnmarshalYAML implements the yaml.Unmarshaler interface +// Unmarshals a single item map into a Storage or a string into a Storage type with no parameters +func (auth *Auth) UnmarshalYAML(unmarshal func(interface{}) error) error { + var m map[string]Parameters + err := unmarshal(&m) + if err == nil { + if len(m) > 1 { + types := make([]string, 0, len(m)) + for k := range m { + types = append(types, k) + } + + // TODO(stevvooe): May want to change this slightly for + // authorization to allow multiple challenges. + return fmt.Errorf("must provide exactly one type. Provided: %v", types) + + } + *auth = m + return nil + } + + var authType string + err = unmarshal(&authType) + if err == nil { + *auth = Auth{authType: Parameters{}} + return nil + } + + return err +} + +// MarshalYAML implements the yaml.Marshaler interface +func (auth Auth) MarshalYAML() (interface{}, error) { + if auth.Parameters() == nil { + return auth.Type(), nil + } + return map[string]Parameters(auth), nil +} // Reporting defines error reporting methods. type Reporting struct { diff --git a/configuration/configuration_test.go b/configuration/configuration_test.go index e6fd6295..91169e03 100644 --- a/configuration/configuration_test.go +++ b/configuration/configuration_test.go @@ -29,6 +29,12 @@ var configStruct = Configuration{ "port": 42, }, }, + Auth: Auth{ + "silly": Parameters{ + "realm": "silly", + "service": "silly", + }, + }, Reporting: Reporting{ Bugsnag: BugsnagReporting{ APIKey: "BugsnagApiKey", @@ -51,6 +57,10 @@ storage: secretkey: SUPERSECRET host: ~ port: 42 +auth: + silly: + realm: silly + service: silly reporting: bugsnag: apikey: BugsnagApiKey @@ -62,6 +72,10 @@ var inmemoryConfigYamlV0_1 = ` version: 0.1 loglevel: info storage: inmemory +auth: + silly: + realm: silly + service: silly ` type ConfigSuite struct { @@ -113,10 +127,13 @@ func (suite *ConfigSuite) TestParseIncomplete(c *C) { c.Assert(err, NotNil) suite.expectedConfig.Storage = Storage{"filesystem": Parameters{"rootdirectory": "/tmp/testroot"}} + suite.expectedConfig.Auth = Auth{"silly": Parameters{"realm": "silly"}} suite.expectedConfig.Reporting = Reporting{} os.Setenv("REGISTRY_STORAGE", "filesystem") os.Setenv("REGISTRY_STORAGE_FILESYSTEM_ROOTDIRECTORY", "/tmp/testroot") + os.Setenv("REGISTRY_AUTH", "silly") + os.Setenv("REGISTRY_AUTH_SILLY_REALM", "silly") config, err := Parse(bytes.NewReader([]byte(incompleteConfigYaml))) c.Assert(err, IsNil) @@ -259,5 +276,10 @@ func copyConfig(config Configuration) *Configuration { NewRelic: NewRelicReporting{config.Reporting.NewRelic.LicenseKey, config.Reporting.NewRelic.Name}, } + configCopy.Auth = Auth{config.Auth.Type(): Parameters{}} + for k, v := range config.Auth.Parameters() { + configCopy.Auth.setParameter(k, v) + } + return configCopy }