From 1944be9db35e894bebb2a95758762fb72ba233a5 Mon Sep 17 00:00:00 2001 From: Stephen J Day Date: Wed, 1 Apr 2015 18:57:59 -0700 Subject: [PATCH] Stronger validation for uuid field in urls This change adds strong validation for the uuid variable for v2 routes. This is a minor specification change but is okay since the uuid field is controlled by the server. The character set is restricted to avoid path traversal, allowing for alphanumeric values and urlsafe base64 encoding. This change has no effect on client implementations. Signed-off-by: Stephen J Day --- doc/spec/api.md | 8 ++++---- registry/api/v2/descriptors.go | 4 ++-- registry/api/v2/routes_test.go | 16 ++++++++++++++++ 3 files changed, 22 insertions(+), 6 deletions(-) diff --git a/doc/spec/api.md b/doc/spec/api.md index 9aa6794b..d42b5674 100644 --- a/doc/spec/api.md +++ b/doc/spec/api.md @@ -2035,7 +2035,7 @@ The following parameters should be specified on the request: |`Host`|header|Standard HTTP Host Header. Should be set to the registry host.| |`Authorization`|header|An RFC7235 compliant authorization header.| |`name`|path|Name of the target repository.| -|`uuid`|path|A uuid identifying the upload. This field can accept almost anything.| +|`uuid`|path|A uuid identifying the upload. This field can accept characters that match `[a-zA-Z0-9-_.=]+`.| @@ -2193,7 +2193,7 @@ The following parameters should be specified on the request: |`Content-Range`|header|Range of bytes identifying the desired block of content represented by the body. Start must the end offset retrieved via status check plus one. Note that this is a non-standard use of the `Content-Range` header.| |`Content-Length`|header|Length of the chunk being uploaded, corresponding the length of the request body.| |`name`|path|Name of the target repository.| -|`uuid`|path|A uuid identifying the upload. This field can accept almost anything.| +|`uuid`|path|A uuid identifying the upload. This field can accept characters that match `[a-zA-Z0-9-_.=]+`.| @@ -2363,7 +2363,7 @@ The following parameters should be specified on the request: |`Content-Range`|header|Range of bytes identifying the block of content represented by the body. Start must the end offset retrieved via status check plus one. Note that this is a non-standard use of the `Content-Range` header. May be omitted if no data is provided.| |`Content-Length`|header|Length of the chunk being uploaded, corresponding to the length of the request body. May be zero if no data is provided.| |`name`|path|Name of the target repository.| -|`uuid`|path|A uuid identifying the upload. This field can accept almost anything.| +|`uuid`|path|A uuid identifying the upload. This field can accept characters that match `[a-zA-Z0-9-_.=]+`.| |`digest`|query|Digest of uploaded blob.| @@ -2538,7 +2538,7 @@ The following parameters should be specified on the request: |`Authorization`|header|An RFC7235 compliant authorization header.| |`Content-Length`|header|The `Content-Length` header must be zero and the body must be empty.| |`name`|path|Name of the target repository.| -|`uuid`|path|A uuid identifying the upload. This field can accept almost anything.| +|`uuid`|path|A uuid identifying the upload. This field can accept characters that match `[a-zA-Z0-9-_.=]+`.| diff --git a/registry/api/v2/descriptors.go b/registry/api/v2/descriptors.go index 5f091bbc..73f8b463 100644 --- a/registry/api/v2/descriptors.go +++ b/registry/api/v2/descriptors.go @@ -28,7 +28,7 @@ var ( Name: "uuid", Type: "opaque", Required: true, - Description: `A uuid identifying the upload. This field can accept almost anything.`, + Description: "A uuid identifying the upload. This field can accept characters that match `[a-zA-Z0-9-_.=]+`.", } digestPathParameter = ParameterDescriptor{ @@ -985,7 +985,7 @@ var routeDescriptors = []RouteDescriptor{ { Name: RouteNameBlobUploadChunk, - Path: "/v2/{name:" + RepositoryNameRegexp.String() + "}/blobs/uploads/{uuid}", + Path: "/v2/{name:" + RepositoryNameRegexp.String() + "}/blobs/uploads/{uuid:[a-zA-Z0-9-_.=]+}", Entity: "Blob Upload", Description: "Interact with blob uploads. Clients should never assemble URLs for this endpoint and should only take it through the `Location` header on related API requests. The `Location` header and its parameters should be preserved by clients, using the latest value returned via upload related API calls.", Methods: []MethodDescriptor{ diff --git a/registry/api/v2/routes_test.go b/registry/api/v2/routes_test.go index afab71fc..fb268336 100644 --- a/registry/api/v2/routes_test.go +++ b/registry/api/v2/routes_test.go @@ -98,6 +98,7 @@ func TestRouter(t *testing.T) { }, }, { + // support uuid proper RouteName: RouteNameBlobUploadChunk, RequestURI: "/v2/foo/bar/blobs/uploads/D95306FA-FAD3-4E36-8D41-CF1C93EF8286", Vars: map[string]string{ @@ -113,6 +114,21 @@ func TestRouter(t *testing.T) { "uuid": "RDk1MzA2RkEtRkFEMy00RTM2LThENDEtQ0YxQzkzRUY4Mjg2IA==", }, }, + { + // supports urlsafe base64 + RouteName: RouteNameBlobUploadChunk, + RequestURI: "/v2/foo/bar/blobs/uploads/RDk1MzA2RkEtRkFEMy00RTM2LThENDEtQ0YxQzkzRUY4Mjg2IA_-==", + Vars: map[string]string{ + "name": "foo/bar", + "uuid": "RDk1MzA2RkEtRkFEMy00RTM2LThENDEtQ0YxQzkzRUY4Mjg2IA_-==", + }, + }, + { + // does not match + RouteName: RouteNameBlobUploadChunk, + RequestURI: "/v2/foo/bar/blobs/uploads/totalandcompletejunk++$$-==", + StatusCode: http.StatusNotFound, + }, { // Check ambiguity: ensure we can distinguish between tags for // "foo/bar/image/image" and image for "foo/bar/image" with tag