1. origin
  2. Commit e8a1ebe273a56182dc390c4c5efff60d37e8d24b
Browse code

Correct registry auth for pruning

With #3166, we modified the registry authorization handler to reject
unknown resource types and/or actions. This unfortunately broke layer
and signature pruning, as those operations use both admin/prune and
repo/* (aka DELETE).

Fix authorization check to associate repo/* with admin/prune.

Fixes bug 1235148.

Andy Goldstein authored on 2015/06/26 22:17:52
Showing 3 changed files
images/dockerregistry/Dockerfile
History View file @ e8a1ebe
... ... 
@@ -6,14 +6,14 @@
6 6 
 #
7 7 
 FROM openshift/origin-base
8 8
9 
-ADD config.yml /config.yml
10 
-ADD bin/dockerregistry /dockerregistry
11 
-
12 
-ENV REGISTRY_CONFIGURATION_PATH=/config.yml
13 
-
14 9 
 RUN yum install -y tree findutils epel-release && \
15 10 
     yum clean all
16 11
12 
+ENV REGISTRY_CONFIGURATION_PATH=/config.yml
13 
+
17 14 
 EXPOSE 5000
18 15 
 VOLUME /registry
19 16 
 CMD REGISTRY_URL=${DOCKER_REGISTRY_SERVICE_HOST}:${DOCKER_REGISTRY_SERVICE_PORT} /dockerregistry /config.yml
17 
+
18 
+ADD config.yml /config.yml
19 
+ADD bin/dockerregistry /dockerregistry
pkg/dockerregistry/server/auth.go
History View file @ e8a1ebe
... ... 
@@ -137,6 +137,8 @@ func (ac *AccessController) Authorized(ctx context.Context, accessRecords ...reg
137 137 
 		}
138 138 
 	}
139 139
140 
+	verifiedPrune := false
141 
+
140 142 
 	// Validate all requested accessRecords
141 143 
 	// Only return failure errors from this loop. Success should continue to validate all records
142 144 
 	for _, access := range accessRecords {
... ... 
@@ -155,20 +157,37 @@ func (ac *AccessController) Authorized(ctx context.Context, accessRecords ...reg
155 155 
 				verb = "update"
156 156 
 			case "pull":
157 157 
 				verb = "get"
158 
+			case "*":
159 
+				verb = "prune"
158 160 
 			default:
159 161 
 				return nil, ac.wrapErr(ErrUnsupportedAction)
160 162 
 			}
161 163
162 
-			if err := verifyImageStreamAccess(imageStreamNS, imageStreamName, verb, client); err != nil {
163 
-				return nil, ac.wrapErr(err)
164 
+			switch verb {
165 
+			case "prune":
166 
+				if verifiedPrune {
167 
+					continue
168 
+				}
169 
+				if err := verifyPruneAccess(client); err != nil {
170 
+					return nil, ac.wrapErr(err)
171 
+				}
172 
+				verifiedPrune = true
173 
+			default:
174 
+				if err := verifyImageStreamAccess(imageStreamNS, imageStreamName, verb, client); err != nil {
175 
+					return nil, ac.wrapErr(err)
176 
+				}
164 177 
 			}
165 178
166 179 
 		case "admin":
167 180 
 			switch access.Action {
168 181 
 			case "prune":
182 
+				if verifiedPrune {
183 
+					continue
184 
+				}
169 185 
 				if err := verifyPruneAccess(client); err != nil {
170 186 
 					return nil, ac.wrapErr(err)
171 187 
 				}
188 
+				verifiedPrune = true
172 189 
 			default:
173 190 
 				return nil, ac.wrapErr(ErrUnsupportedAction)
174 191 
 			}
pkg/dockerregistry/server/auth_test.go
History View file @ e8a1ebe
... ... 
@@ -234,6 +234,32 @@ func TestAccessController(t *testing.T) {
234 234 
 			expectedChallenge: false,
235 235 
 			expectedActions:   []string{"POST /oapi/v1/namespaces/foo/subjectaccessreviews"},
236 236 
 		},
237 
+		"pruning": {
238 
+			access: []auth.Access{
239 
+				{
240 
+					Resource: auth.Resource{
241 
+						Type: "admin",
242 
+					},
243 
+					Action: "prune",
244 
+				},
245 
+				{
246 
+					Resource: auth.Resource{
247 
+						Type: "repository",
248 
+						Name: "foo/bar",
249 
+					},
250 
+					Action: "*",
251 
+				},
252 
+			},
253 
+			basicToken: "b3BlbnNoaWZ0OmF3ZXNvbWU=",
254 
+			openshiftResponses: []response{
255 
+				{200, runtime.EncodeOrDie(latest.Codec, &api.SubjectAccessReviewResponse{Allowed: true, Reason: "authorized!"})},
256 
+			},
257 
+			expectedError:     nil,
258 
+			expectedChallenge: false,
259 
+			expectedActions: []string{
260 
+				"POST /oapi/v1/subjectaccessreviews",
261 
+			},
262 
+		},
237 263 
 	}
238 264
239 265 
 	for k, test := range tests {