Handle cross repository mount

During a push with a cross-repo mount, authorize only against target
repository, not against source repository.

In OpenShift we allow to pull any blob stored in the registry as long as
the user is authorized to pull from repository he requested.

Signed-off-by: Michal Minar <[email protected]>

Author: Michal Minar

pkg/dockerregistry/server/auth.go

@@ -5,6 +5,7 @@ import (
 	"errors"
 	"fmt"
 	"net/http"
+	"net/url"
 	"strings"
 
 	log "github.com/Sirupsen/logrus"
@@ -196,7 +197,7 @@ func (ac *AccessController) Authorized(ctx context.Context, accessRecords ...reg
 				}
 				verifiedPrune = true
 			default:
-				if err := verifyImageStreamAccess(ctx, imageStreamNS, imageStreamName, verb, osClient); err != nil {
+				if err := authorizeRepositoryAccess(ctx, imageStreamNS, imageStreamName, verb, osClient); err != nil {
 					return nil, ac.wrapErr(err)
 				}
 			}
@@ -272,6 +273,59 @@ func verifyOpenShiftUser(ctx context.Context, client client.UsersInterface) erro
 	return nil
 }
 
+// authorizeRepositoryAccess returns nil if the given client is granted access to the given
+// <namespace>/<imageRepo> repository. Otherwise the access is denied.
+func authorizeRepositoryAccess(ctx context.Context, namespace, imageRepo, verb string, client client.LocalSubjectAccessReviewsNamespacer) error {
+	if isCrossRepoMountFromSourceRepository(ctx, namespace, imageRepo, verb) {
+		return nil
+	}
+
+	return verifyImageStreamAccess(ctx, namespace, imageRepo, verb, client)
+}
+
+// isCrossRepoMountFromSourceRepository handles a case of accessing source repository requested by a cross
+// repository mount during a blob upload. During a push with the cross-repo mount, there are two authorization
+// checks:
+//
+//   1. repository:<source-namespace>/<source-name>:pull
+//   2. repository:<target-namespace>/<target-name>:push
+//
+// This function always grants access in the first case because OpenShift registry treats all the local blobs
+// as accessible once authorized to access given repository (target one).
+func isCrossRepoMountFromSourceRepository(ctx context.Context, namespace, imageRepo, verb string) bool {
+	if verb != "get" {
+		return false
+	}
+	if method := strings.ToLower(getStringValueFromContext(ctx, "http.request.method", "")); method != "post" {
+		return false
+	}
+
+	uriString := getStringValue(ctx, "http.request.uri")
+	uri, err := url.Parse(uriString)
+	if err != nil {
+		context.GetLogger(ctx).Warnf("failed to parse requested uri %q: %v", uri, err)
+		return false
+	}
+	from, exists := uri.Query()["from"]
+	if !exists {
+		context.GetLogger(ctx).Infof("cross-repo mount check: missing from attribute in query")
+		return false
+	}
+	repoName := fmt.Sprintf("%s/%s", namespace, imageRepo)
+	for _, f := range from {
+		if f == repoName {
+			context.GetLogger(ctx).Infof("authorized a cross-repo mount from=%q to repository %q",
+				repoName, getStringValueFromContext(ctx, "vars.name", "UNKNOWN"))
+			return true
+		}
+	}
+
+	return false
+}
+
+// verifyImageStreamAccess returns nil if the given client is granted access to an image stream identified by
+// <namespace>/<imageRepo>. Otherwise the access is denied. The user embedded in given client must be able to
+// <verb> (get/update) imagestreams/layers resource in the <namespace> to have the access granted.
 func verifyImageStreamAccess(ctx context.Context, namespace, imageRepo, verb string, client client.LocalSubjectAccessReviewsNamespacer) error {
 	sar := authorizationapi.LocalSubjectAccessReview{
 		Action: authorizationapi.AuthorizationAttributes{
@@ -299,6 +353,9 @@ func verifyImageStreamAccess(ctx context.Context, namespace, imageRepo, verb str
 	return nil
 }
 
+// verifyPruneAccess returns nil if the given client is granted access to images resource. Otherwise the
+// access is denied. The user embedded in given client must be able to delete images resource in a cluster to
+// have the access granted.
 func verifyPruneAccess(ctx context.Context, client client.SubjectAccessReviews) error {
 	sar := authorizationapi.SubjectAccessReview{
 		Action: authorizationapi.AuthorizationAttributes{
@@ -321,3 +378,13 @@ func verifyPruneAccess(ctx context.Context, client client.SubjectAccessReviews)
 	}
 	return nil
 }
+
+// getStringValueFromContext returns a value stored in given context under given key if it exists. The
+// defaultValue will be returned otherwise.
+func getStringValueFromContext(ctx context.Context, key, defaultValue string) string {
+	value := ctx.Value(key)
+	if str, ok := value.(string); ok {
+		return str
+	}
+	return defaultValue
+}

pkg/dockerregistry/server/auth_test.go

@@ -91,6 +91,8 @@ func TestAccessController(t *testing.T) {
 
 	tests := map[string]struct {
 		access             []auth.Access
+		uri                string
+		method             string
 		basicToken         string
 		openshiftResponses []response
 		expectedError      error
@@ -263,6 +265,92 @@ func TestAccessController(t *testing.T) {
 				"POST /oapi/v1/subjectaccessreviews",
 			},
 		},
+		"cross-repo mount": {
+			access: []auth.Access{
+				{
+					Resource: auth.Resource{
+						Type: "repository",
+						Name: "crossrepo/source",
+					},
+					Action: "pull",
+				},
+				{
+					Resource: auth.Resource{
+						Type: "repository",
+						Name: "foo/destination",
+					},
+					Action: "push",
+				},
+			},
+			uri:        "/v2/crossrepo/destination/blobs/uploads/?from=crossrepo/source&mount=sha256:da71393503ec9136cf62056c233f5d25b878e372c840170d91d65f8cdf94def2",
+			method:     "POST",
+			basicToken: "b3BlbnNoaWZ0OmF3ZXNvbWU=",
+			openshiftResponses: []response{
+				{200, runtime.EncodeOrDie(kapi.Codecs.LegacyCodec(registered.GroupOrDie(kapi.GroupName).GroupVersions[0]), &api.SubjectAccessReviewResponse{Namespace: "crossrepo", Allowed: true, Reason: "authorized!"})},
+			},
+			expectedError:     nil,
+			expectedChallenge: false,
+			expectedActions:   []string{"POST /oapi/v1/namespaces/foo/localsubjectaccessreviews"},
+		},
+		"cross-repo mount missing from attribute": {
+			access: []auth.Access{
+				{
+					Resource: auth.Resource{
+						Type: "repository",
+						Name: "crossrepo/source",
+					},
+					Action: "pull",
+				},
+				{
+					Resource: auth.Resource{
+						Type: "repository",
+						Name: "foo/destination",
+					},
+					Action: "push",
+				},
+			},
+			uri:        "/v2/foo/destination/blobs/uploads/?mount=sha256:da71393503ec9136cf62056c233f5d25b878e372c840170d91d65f8cdf94def2",
+			method:     "POST",
+			basicToken: "b3BlbnNoaWZ0OmF3ZXNvbWU=",
+			openshiftResponses: []response{
+				{200, runtime.EncodeOrDie(kapi.Codecs.LegacyCodec(registered.GroupOrDie(kapi.GroupName).GroupVersions[0]), &api.SubjectAccessReviewResponse{Namespace: "crossrepo", Allowed: false, Reason: "no!"})},
+				{200, runtime.EncodeOrDie(kapi.Codecs.LegacyCodec(registered.GroupOrDie(kapi.GroupName).GroupVersions[0]), &api.SubjectAccessReviewResponse{Namespace: "foo", Allowed: true, Reason: "authorized!"})},
+			},
+			expectedError:     ErrOpenShiftAccessDenied,
+			expectedChallenge: true,
+			expectedActions:   []string{"POST /oapi/v1/namespaces/crossrepo/localsubjectaccessreviews"},
+		},
+		"cross-repo mount with unexpected method": {
+			access: []auth.Access{
+				{
+					Resource: auth.Resource{
+						Type: "repository",
+						Name: "crossrepo/source",
+					},
+					Action: "pull",
+				},
+				{
+					Resource: auth.Resource{
+						Type: "repository",
+						Name: "foo/destination",
+					},
+					Action: "push",
+				},
+			},
+			uri:        "/v2/crossrepo/destination/blobs/uploads/?from=crossrepo/source&mount=sha256:da71393503ec9136cf62056c233f5d25b878e372c840170d91d65f8cdf94def2",
+			method:     "PUT",
+			basicToken: "b3BlbnNoaWZ0OmF3ZXNvbWU=",
+			openshiftResponses: []response{
+				{200, runtime.EncodeOrDie(kapi.Codecs.LegacyCodec(registered.GroupOrDie(kapi.GroupName).GroupVersions[0]), &api.SubjectAccessReviewResponse{Namespace: "crossrepo", Allowed: true, Reason: "authorized!"})},
+				{200, runtime.EncodeOrDie(kapi.Codecs.LegacyCodec(registered.GroupOrDie(kapi.GroupName).GroupVersions[0]), &api.SubjectAccessReviewResponse{Namespace: "foo", Allowed: false, Reason: "authorized!"})},
+			},
+			expectedError:     ErrOpenShiftAccessDenied,
+			expectedChallenge: true,
+			expectedActions: []string{
+				"POST /oapi/v1/namespaces/crossrepo/localsubjectaccessreviews",
+				"POST /oapi/v1/namespaces/foo/localsubjectaccessreviews",
+			},
+		},
 	}
 
 	for k, test := range tests {
@@ -274,7 +362,11 @@ func TestAccessController(t *testing.T) {
 		if len(test.basicToken) > 0 {
 			req.Header.Set("Authorization", fmt.Sprintf("Basic %s", test.basicToken))
 		}
-		ctx := context.WithValue(context.Background(), "http.request", req)
+		ctx := context.WithValues(context.Background(), map[string]interface{}{
+			"http.request":        req,
+			"http.request.uri":    test.uri,
+			"http.request.method": test.method,
+		})
 
 		server, actions := simulateOpenShiftMaster(test.openshiftResponses)
 		DefaultRegistryClient = NewRegistryClient(&clientcmd.Config{

test/end-to-end/core.sh

@@ -220,15 +220,6 @@ echo "[INFO] Run pod diagnostics"
 # Requires a node to run the origin-deployer pod; expects registry deployed, deployer image pulled
 os::cmd::expect_success_and_text 'oadm diagnostics DiagnosticPod --images='"'""${USE_IMAGES}""'" 'Running diagnostic: PodCheckDns'
 
-# This needed because docker 1.10+ utilizes cross-repo mount feature. Docker client keeps track of source
-# repositories for all the blobs. If it uploads a blob to a repository of registry having an entry in this
-# mapping, it will request a mount from this repository. The authorization check is done for both destination
-# and mounted repository. Without the next statement, the auth check for mounted repository
-# (cache/ruby-22-centos7) will fail.
-# TODO: remove this once the registry can access global blob store without a layer link authorization check
-oc policy -n cache add-role-to-user system:image-puller system:serviceaccount:test:builder
-
-
 echo "[INFO] Applying STI application config"
 os::cmd::expect_success "oc create -f ${STI_CONFIG_FILE}"