Skip to content

Commit

Permalink
fix: bug fix for token auth
Browse files Browse the repository at this point in the history
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 <jasdeepsingh.uppal@gmail.com>
  • Loading branch information
jay-dee7 committed Oct 21, 2021
1 parent 0cc8437 commit 7b142e6
Show file tree
Hide file tree
Showing 8 changed files with 128 additions and 16 deletions.
14 changes: 12 additions & 2 deletions .github/workflows/go.yml → .github/workflows/conformance.yml
Original file line number Diff line number Diff line change
Expand Up @@ -13,14 +13,24 @@ 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
export $(cat env-vars.example| xargs)
./openregistry >> openregistry.log 2>&1 &
sleep 5
curl -XPOST -d ${{ secrets.OPENREGISTRY_SIGNUP_PAYLOAD }} "http://${IP}:5000/auth/signup"
- name: Run OCI Distribution Spec conformance tests
Expand Down
1 change: 1 addition & 0 deletions auth/auth.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ type Authentication interface {
BasicAuth() echo.MiddlewareFunc
Token(ctx echo.Context) error
JWT() echo.MiddlewareFunc
ACL() echo.MiddlewareFunc
}

type auth struct {
Expand Down
64 changes: 63 additions & 1 deletion auth/basic_auth.go
Original file line number Diff line number Diff line change
@@ -1,8 +1,11 @@
package auth

import (
// "fmt"
"encoding/base64"
"fmt"
"net/http"
"strconv"
"strings"

"github.com/containerish/OpenRegistry/registry/v2"
Expand Down Expand Up @@ -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)

Expand Down Expand Up @@ -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
}
}
}
31 changes: 31 additions & 0 deletions auth/jwt_middleware.go
Original file line number Diff line number Diff line change
@@ -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"
Expand Down Expand Up @@ -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)
}
}
}
26 changes: 17 additions & 9 deletions auth/token.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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)
Expand All @@ -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) {
Expand Down Expand Up @@ -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
}
1 change: 1 addition & 0 deletions env-vars.example
Original file line number Diff line number Diff line change
Expand Up @@ -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
3 changes: 1 addition & 2 deletions registry/v2/blobs.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ import (
"fmt"
"io"
"net/http"
"strings"

"github.com/fatih/color"
"github.com/labstack/echo/v4"
Expand Down Expand Up @@ -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 {
Expand Down
4 changes: 2 additions & 2 deletions router/router.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down

0 comments on commit 7b142e6

Please sign in to comment.