From a693e2997c9d437041c31ebd800d1bfaf9eb60cc Mon Sep 17 00:00:00 2001 From: jay-dee7 Date: Thu, 21 Oct 2021 00:19:02 +0530 Subject: [PATCH] fix: bug fix for token auth We recently added JWT auth and it introduced a bug that let's anyone push images under any namespace: This PR fixes the bug introduced in PR #48 **johndoe** want to push **app1** so he creates an image named `openregistry.dev/johndoe/app1`, since he's the owner of the image, he can push the image. **johndoe** wants to push **app1** to **janedoe's** account, so he makes a request as: `openregistry.dev/janedoe/app1` and this worked too (which it should not) With this PR, we now check that only `pull` should be allowed and `push` should be restricted to user's own namespace. Signed-off-by: jay-dee7 --- .github/workflows/{go.yml => conformance.yml} | 13 +++- auth/auth.go | 1 + auth/basic_auth.go | 64 ++++++++++++++++++- auth/jwt_middleware.go | 31 +++++++++ auth/token.go | 26 +++++--- env-vars.example | 1 + registry/v2/blobs.go | 3 +- router/router.go | 4 +- 8 files changed, 127 insertions(+), 16 deletions(-) rename .github/workflows/{go.yml => conformance.yml} (72%) diff --git a/.github/workflows/go.yml b/.github/workflows/conformance.yml similarity index 72% rename from .github/workflows/go.yml rename to .github/workflows/conformance.yml index 24f34249..d6a7916b 100644 --- a/.github/workflows/go.yml +++ b/.github/workflows/conformance.yml @@ -13,14 +13,23 @@ jobs: docker_version: 18.09 docker_channel: stable - uses: actions/checkout@v2 + - uses: actions/setup-go@v2 + with: + go-version: '^1.17.2' # The Go version to download (if necessary) and use. + - run: go version - name: start distribution server run: | IP=`hostname -I | awk '{print $1}'` echo "IP=$IP" >> $GITHUB_ENV echo "OCI_ROOT_URL=http://$IP:5000" >> $GITHUB_ENV DISTRIBUTION_REF="local-distribution:v$(date +%Y%m%d%H%M%S)" - docker build -f ./Dockerfile -t "${DISTRIBUTION_REF}" . - docker run --rm -p 5000:5000 --env-file ./env-vars.example -e REGISTRY_STORAGE_DELETE_ENABLED=true -d "${DISTRIBUTION_REF}" + # docker build -f ./Dockerfile -t "${DISTRIBUTION_REF}" . + # docker run --net=host --rm -p 5000:5000 --env-file ./env-vars.example -e REGISTRY_STORAGE_DELETE_ENABLED=true -d "${DISTRIBUTION_REF}" + make mod-fix + sed -in "s/OPEN_REGISTRY_HOST=0.0.0.0/OPEN_REGISTRY_HOST=$IP/g" env-vars.example + go mod download && CGO_ENABLED=0 go build -o openregistry -ldflags="-w -s" main.go + source env-vars.example + ./openregistry sleep 5 curl -XPOST -d ${{ secrets.OPENREGISTRY_SIGNUP_PAYLOAD }} "http://${IP}:5000/auth/signup" - name: Run OCI Distribution Spec conformance tests diff --git a/auth/auth.go b/auth/auth.go index 80118fb1..e19c6ede 100644 --- a/auth/auth.go +++ b/auth/auth.go @@ -13,6 +13,7 @@ type Authentication interface { BasicAuth() echo.MiddlewareFunc Token(ctx echo.Context) error JWT() echo.MiddlewareFunc + ACL() echo.MiddlewareFunc } type auth struct { diff --git a/auth/basic_auth.go b/auth/basic_auth.go index cd05508d..72d291fd 100644 --- a/auth/basic_auth.go +++ b/auth/basic_auth.go @@ -1,8 +1,11 @@ package auth import ( + // "fmt" + "encoding/base64" "fmt" "net/http" + "strconv" "strings" "github.com/containerish/OpenRegistry/registry/v2" @@ -33,7 +36,7 @@ Strict-Transport-Security: max-age=31536000 // BasicAuth returns a middleware which in turn can be used to perform http basic auth func (a *auth) BasicAuth() echo.MiddlewareFunc { - return middleware.BasicAuthWithConfig(middleware.BasicAuthConfig{ + return BasicAuthWithConfig(middleware.BasicAuthConfig{ Skipper: func(ctx echo.Context) bool { authHeader := ctx.Request().Header.Get(AuthorizationHeaderKey) @@ -96,3 +99,62 @@ func (a *auth) checkJWT(authHeader string) bool { return strings.EqualFold(parts[0], "Bearer") } + +const ( + defaultRealm = "Restricted" + authScheme = "Bearer" +) + +// BasicAuthConfig is a local copy of echo's middleware.BasicAuthWithConfig +func BasicAuthWithConfig(config middleware.BasicAuthConfig) echo.MiddlewareFunc { + // Defaults + if config.Validator == nil { + panic("echo: basic-auth middleware requires a validator function") + } + if config.Skipper == nil { + config.Skipper = middleware.DefaultBasicAuthConfig.Skipper + } + if config.Realm == "" { + config.Realm = defaultRealm + } + + return func(next echo.HandlerFunc) echo.HandlerFunc { + return func(c echo.Context) error { + if config.Skipper(c) { + return next(c) + } + + auth := c.Request().Header.Get(echo.HeaderAuthorization) + l := len(authScheme) + + if len(auth) > l+1 && strings.EqualFold(auth[:l], authScheme) { + b, err := base64.StdEncoding.DecodeString(auth[l+1:]) + if err != nil { + return err + } + cred := string(b) + for i := 0; i < len(cred); i++ { + if cred[i] == ':' { + // Verify credentials + valid, err := config.Validator(cred[:i], cred[i+1:], c) + if err != nil { + return err + } else if valid { + return next(c) + } + break + } + } + } + + realm := defaultRealm + if config.Realm != defaultRealm { + realm = strconv.Quote(config.Realm) + } + + // Need to return `401` for browsers to pop-up login box. + c.Response().Header().Set(echo.HeaderWWWAuthenticate, authScheme+" realm="+realm) + return echo.ErrUnauthorized + } + } +} diff --git a/auth/jwt_middleware.go b/auth/jwt_middleware.go index 6b79ac50..5f897dcd 100644 --- a/auth/jwt_middleware.go +++ b/auth/jwt_middleware.go @@ -1,6 +1,8 @@ package auth import ( + "net/http" + "github.com/golang-jwt/jwt" "github.com/labstack/echo/v4" "github.com/labstack/echo/v4/middleware" @@ -34,3 +36,32 @@ func (a *auth) JWT() echo.MiddlewareFunc { Claims: &Claims{}, }) } + +// ACL implies a basic Access Control List on protected resources +func (a *auth) ACL() echo.MiddlewareFunc { + return func(hf echo.HandlerFunc) echo.HandlerFunc { + return func(ctx echo.Context) error { + m := ctx.Request().Method + if m == http.MethodGet || m == http.MethodHead { + return hf(ctx) + } + + token, ok := ctx.Get("user").(*jwt.Token) + if !ok { + return ctx.NoContent(http.StatusUnauthorized) + } + + claims, ok := token.Claims.(*Claims) + if !ok { + return ctx.NoContent(http.StatusUnauthorized) + } + + username := ctx.Param("username") + if claims.Subject == username { + return hf(ctx) + } + + return ctx.NoContent(http.StatusUnauthorized) + } + } +} diff --git a/auth/token.go b/auth/token.go index 93360819..1f0d533c 100644 --- a/auth/token.go +++ b/auth/token.go @@ -17,7 +17,8 @@ func (a *auth) Token(ctx echo.Context) error { // TODO (jay-dee7) - check for all valid query params here like serive, client_id, offline_token, etc // more at this link - https://docs.docker.com/registry/spec/auth/token/ - if ctx.Request().Header.Get(AuthorizationHeaderKey) != "" { + authHeader := ctx.Request().Header.Get(AuthorizationHeaderKey) + if authHeader != "" { username, password, err := a.getCredsFromHeader(ctx.Request()) if err != nil { return ctx.NoContent(http.StatusUnauthorized) @@ -41,7 +42,9 @@ func (a *auth) Token(ctx echo.Context) error { }) } - if len(scope.Actions) >= 1 && scope.Actions[0] == "pull" { + // issue a free-public token to pull any repository + // TODO (jay-dee7) - this should be restricted to only public repositories in the future + if len(scope.Actions) == 1 && scope.Actions["pull"] { token, err := a.newPublicPullToken() if err != nil { return ctx.NoContent(http.StatusInternalServerError) @@ -52,7 +55,7 @@ func (a *auth) Token(ctx echo.Context) error { }) } - return ctx.JSON(http.StatusOK, scope) + return ctx.NoContent(http.StatusUnauthorized) } func (a *auth) getCredsFromHeader(r *http.Request) (string, string, error) { @@ -80,15 +83,20 @@ func (a *auth) getScopeFromQueryParams(param string) (*Scope, error) { return nil, fmt.Errorf("invalid scope in params") } - return &Scope{ - Type: parts[0], - Name: parts[1], - Actions: strings.Split(parts[2], ","), - }, nil + scope := &Scope{Type: parts[0], Name: parts[1]} + scope.Actions = make(map[string]bool) + + for _, action := range strings.Split(parts[2], ",") { + if action != "" { + scope.Actions[action] = true + } + } + + return scope, nil } type Scope struct { Type string Name string - Actions []string + Actions map[string]bool } diff --git a/env-vars.example b/env-vars.example index af04519a..f60e063d 100644 --- a/env-vars.example +++ b/env-vars.example @@ -5,3 +5,4 @@ OPEN_REGISTRY_PORT=5000 OPEN_REGISTRY_SKYNET_PORTAL_URL=https://siasky.net OPEN_REGISTRY_SIGNING_SECRET="3tYnaKp@^%hbQA%J&x3cX!r2#mK%EBfAbTvPMv5CU2DP7bAoQGnUfT2&dW" OPEN_REGISTRY_SUPPORTED_SERVICES=github,token +OPEN_REGISTRY_ENVIRONMENT=local diff --git a/registry/v2/blobs.go b/registry/v2/blobs.go index 89079c41..8593c285 100644 --- a/registry/v2/blobs.go +++ b/registry/v2/blobs.go @@ -6,7 +6,6 @@ import ( "fmt" "io" "net/http" - "strings" "github.com/fatih/color" "github.com/labstack/echo/v4" @@ -55,7 +54,7 @@ func (b *blobs) HEAD(ctx echo.Context) error { func (b *blobs) UploadBlob(ctx echo.Context) error { namespace := ctx.Param("username") + "/" + ctx.Param("imagename") contentRange := ctx.Request().Header.Get("Content-Range") - uuid := strings.Split(ctx.Request().RequestURI, "/")[6] + uuid := ctx.Param("uuid") if contentRange == "" { if _, ok := b.uploads[uuid]; ok { diff --git a/router/router.go b/router/router.go index cba2d25f..8da11bb8 100644 --- a/router/router.go +++ b/router/router.go @@ -19,20 +19,20 @@ func Register(e *echo.Echo, reg registry.Registry, authSvc auth.Authentication, e.Use(middleware.Recover()) e.Use(middleware.CORS()) // JWT Auth Endpoint - e.Add(http.MethodGet, TokenAuth, authSvc.Token) e.HideBanner = true p := prometheus.NewPrometheus("OpenRegistry", nil) p.Use(e) v2Router := e.Group(V2, authSvc.BasicAuth(), authSvc.JWT()) - nsRouter := v2Router.Group(Namespace) + nsRouter := v2Router.Group(Namespace, authSvc.ACL()) internal := e.Group(Internal) authRouter := e.Group(Auth) betaRouter := e.Group(Beta) v2Router.Add(http.MethodGet, Root, reg.ApiVersion) + e.Add(http.MethodGet, TokenAuth, authSvc.Token) RegisterNSRoutes(nsRouter, reg) RegisterAuthRoutes(authRouter, authSvc)