fix: metrics endpoint must be secured behind authN (#1864)

Signed-off-by: Alexei Dodon <[email protected]>
project-zot · Oct 2, 2023 · 2fd7bfc · 2fd7bfc
1 parent 0eb9844
commit 2fd7bfc
Show file tree

Hide file tree

Showing 14 changed files with 216 additions and 84 deletions.
diff --git a/Makefile b/Makefile
@@ -408,7 +408,8 @@ run-blackbox-ci: check-blackbox-prerequisites binary binary-minimal cli
 	$(BATS) $(BATS_FLAGS) test/blackbox/sync_replica_cluster.bats && \
 	$(BATS) $(BATS_FLAGS) test/blackbox/scrub.bats && \
 	$(BATS) $(BATS_FLAGS) test/blackbox/garbage_collect.bats && \
-	$(BATS) $(BATS_FLAGS) test/blackbox/metrics.bats
+	$(BATS) $(BATS_FLAGS) test/blackbox/metrics.bats && \
+	$(BATS) $(BATS_FLAGS) test/blackbox/metrics_minimal.bats
 
 .PHONY: run-blackbox-cloud-ci
 run-blackbox-cloud-ci: check-blackbox-prerequisites check-awslocal binary $(BATS)

diff --git a/examples/config-policy.json b/examples/config-policy.json
@@ -60,7 +60,7 @@
             }
           ],
           "defaultPolicy": ["read"]
-        }  
+        }
       },
       "adminPolicy": {
         "users": ["admin"],

diff --git a/pkg/api/controller_test.go b/pkg/api/controller_test.go
@@ -2195,7 +2195,7 @@ func TestBearerAuth(t *testing.T) {
 		So(resp, ShouldNotBeNil)
 		So(resp.StatusCode(), ShouldEqual, http.StatusUnauthorized)
 
-		authorizationHeader := authutils.ParseBearerAuthHeader(resp.Header().Get("Www-Authenticate"))
+		authorizationHeader := authutils.ParseBearerAuthHeader(resp.Header().Get("WWW-Authenticate"))
 		resp, err = resty.R().
 			SetQueryParam("service", authorizationHeader.Service).
 			SetQueryParam("scope", authorizationHeader.Scope).
@@ -2225,7 +2225,7 @@ func TestBearerAuth(t *testing.T) {
 		So(resp, ShouldNotBeNil)
 		So(resp.StatusCode(), ShouldEqual, http.StatusUnauthorized)
 
-		authorizationHeader = authutils.ParseBearerAuthHeader(resp.Header().Get("Www-Authenticate"))
+		authorizationHeader = authutils.ParseBearerAuthHeader(resp.Header().Get("WWW-Authenticate"))
 		resp, err = resty.R().
 			SetQueryParam("service", authorizationHeader.Service).
 			SetQueryParam("scope", authorizationHeader.Scope).
@@ -2254,7 +2254,7 @@ func TestBearerAuth(t *testing.T) {
 		So(resp, ShouldNotBeNil)
 		So(resp.StatusCode(), ShouldEqual, http.StatusUnauthorized)
 
-		authorizationHeader = authutils.ParseBearerAuthHeader(resp.Header().Get("Www-Authenticate"))
+		authorizationHeader = authutils.ParseBearerAuthHeader(resp.Header().Get("WWW-Authenticate"))
 		resp, err = resty.R().
 			SetQueryParam("service", authorizationHeader.Service).
 			SetQueryParam("scope", authorizationHeader.Scope).
@@ -2283,7 +2283,7 @@ func TestBearerAuth(t *testing.T) {
 		So(resp, ShouldNotBeNil)
 		So(resp.StatusCode(), ShouldEqual, http.StatusUnauthorized)
 
-		authorizationHeader = authutils.ParseBearerAuthHeader(resp.Header().Get("Www-Authenticate"))
+		authorizationHeader = authutils.ParseBearerAuthHeader(resp.Header().Get("WWW-Authenticate"))
 		resp, err = resty.R().
 			SetQueryParam("service", authorizationHeader.Service).
 			SetQueryParam("scope", authorizationHeader.Scope).
@@ -2307,7 +2307,7 @@ func TestBearerAuth(t *testing.T) {
 		So(resp, ShouldNotBeNil)
 		So(resp.StatusCode(), ShouldEqual, http.StatusUnauthorized)
 
-		authorizationHeader = authutils.ParseBearerAuthHeader(resp.Header().Get("Www-Authenticate"))
+		authorizationHeader = authutils.ParseBearerAuthHeader(resp.Header().Get("WWW-Authenticate"))
 		resp, err = resty.R().
 			SetQueryParam("service", authorizationHeader.Service).
 			SetQueryParam("scope", authorizationHeader.Scope).
@@ -2392,7 +2392,7 @@ func TestBearerAuthWithAllowReadAccess(t *testing.T) {
 		So(resp, ShouldNotBeNil)
 		So(resp.StatusCode(), ShouldEqual, http.StatusUnauthorized)
 
-		authorizationHeader := authutils.ParseBearerAuthHeader(resp.Header().Get("Www-Authenticate"))
+		authorizationHeader := authutils.ParseBearerAuthHeader(resp.Header().Get("WWW-Authenticate"))
 		resp, err = resty.R().
 			SetQueryParam("service", authorizationHeader.Service).
 			SetQueryParam("scope", authorizationHeader.Scope).
@@ -2416,7 +2416,7 @@ func TestBearerAuthWithAllowReadAccess(t *testing.T) {
 		So(resp, ShouldNotBeNil)
 		So(resp.StatusCode(), ShouldEqual, http.StatusUnauthorized)
 
-		authorizationHeader = authutils.ParseBearerAuthHeader(resp.Header().Get("Www-Authenticate"))
+		authorizationHeader = authutils.ParseBearerAuthHeader(resp.Header().Get("WWW-Authenticate"))
 		resp, err = resty.R().
 			SetQueryParam("service", authorizationHeader.Service).
 			SetQueryParam("scope", authorizationHeader.Scope).
@@ -2445,7 +2445,7 @@ func TestBearerAuthWithAllowReadAccess(t *testing.T) {
 		So(resp, ShouldNotBeNil)
 		So(resp.StatusCode(), ShouldEqual, http.StatusUnauthorized)
 
-		authorizationHeader = authutils.ParseBearerAuthHeader(resp.Header().Get("Www-Authenticate"))
+		authorizationHeader = authutils.ParseBearerAuthHeader(resp.Header().Get("WWW-Authenticate"))
 		resp, err = resty.R().
 			SetQueryParam("service", authorizationHeader.Service).
 			SetQueryParam("scope", authorizationHeader.Scope).
@@ -2474,7 +2474,7 @@ func TestBearerAuthWithAllowReadAccess(t *testing.T) {
 		So(resp, ShouldNotBeNil)
 		So(resp.StatusCode(), ShouldEqual, http.StatusUnauthorized)
 
-		authorizationHeader = authutils.ParseBearerAuthHeader(resp.Header().Get("Www-Authenticate"))
+		authorizationHeader = authutils.ParseBearerAuthHeader(resp.Header().Get("WWW-Authenticate"))
 		resp, err = resty.R().
 			SetQueryParam("service", authorizationHeader.Service).
 			SetQueryParam("scope", authorizationHeader.Scope).
@@ -2498,7 +2498,7 @@ func TestBearerAuthWithAllowReadAccess(t *testing.T) {
 		So(resp, ShouldNotBeNil)
 		So(resp.StatusCode(), ShouldEqual, http.StatusUnauthorized)
 
-		authorizationHeader = authutils.ParseBearerAuthHeader(resp.Header().Get("Www-Authenticate"))
+		authorizationHeader = authutils.ParseBearerAuthHeader(resp.Header().Get("WWW-Authenticate"))
 		resp, err = resty.R().
 			SetQueryParam("service", authorizationHeader.Service).
 			SetQueryParam("scope", authorizationHeader.Scope).

diff --git a/pkg/api/errors/errors.go b/pkg/api/errors/errors.go
@@ -120,8 +120,8 @@ func NewError(code ErrorCode) *Error {
 
 		UNAUTHORIZED: {
 			Message: "authentication required",
-			Description: "The access controller was unable to authenticate the client." +
-				"Often this will be accompanied by a Www-Authenticate HTTP response header " +
+			Description: "The access controller was unable to authenticate the client. " +
+				"Often this will be accompanied by a WWW-Authenticate HTTP response header " +
 				"indicating how to authenticate.",
 		},
 

diff --git a/pkg/extensions/extension_metrics_disabled.go b/pkg/extensions/extension_metrics_disabled.go
@@ -29,5 +29,6 @@ func SetupMetricsRoutes(conf *config.Config, router *mux.Router,
 		zcommon.WriteJSON(w, http.StatusOK, m)
 	}
 
+	router.Use(authFunc)
 	router.HandleFunc("/metrics", getMetrics).Methods("GET")
 }
diff --git a/pkg/extensions/extensions_test.go b/pkg/extensions/extensions_test.go
@@ -799,7 +799,7 @@ func TestMgmtWithBearer(t *testing.T) {
 		So(resp, ShouldNotBeNil)
 		So(resp.StatusCode(), ShouldEqual, http.StatusUnauthorized)
 
-		authorizationHeader := authutils.ParseBearerAuthHeader(resp.Header().Get("Www-Authenticate"))
+		authorizationHeader := authutils.ParseBearerAuthHeader(resp.Header().Get("WWW-Authenticate"))
 		resp, err = resty.R().
 			SetQueryParam("service", authorizationHeader.Service).
 			SetQueryParam("scope", authorizationHeader.Scope).
@@ -829,7 +829,7 @@ func TestMgmtWithBearer(t *testing.T) {
 		So(resp, ShouldNotBeNil)
 		So(resp.StatusCode(), ShouldEqual, http.StatusUnauthorized)
 
-		authorizationHeader = authutils.ParseBearerAuthHeader(resp.Header().Get("Www-Authenticate"))
+		authorizationHeader = authutils.ParseBearerAuthHeader(resp.Header().Get("WWW-Authenticate"))
 		resp, err = resty.R().
 			SetQueryParam("service", authorizationHeader.Service).
 			SetQueryParam("scope", authorizationHeader.Scope).
@@ -853,7 +853,7 @@ func TestMgmtWithBearer(t *testing.T) {
 		So(resp, ShouldNotBeNil)
 		So(resp.StatusCode(), ShouldEqual, http.StatusUnauthorized)
 
-		authorizationHeader = authutils.ParseBearerAuthHeader(resp.Header().Get("Www-Authenticate"))
+		authorizationHeader = authutils.ParseBearerAuthHeader(resp.Header().Get("WWW-Authenticate"))
 		resp, err = resty.R().
 			SetQueryParam("service", authorizationHeader.Service).
 			SetQueryParam("scope", authorizationHeader.Scope).

diff --git a/test/blackbox/anonymous_policy.bats b/test/blackbox/anonymous_policy.bats
@@ -5,6 +5,11 @@
 load helpers_zot
 
 function verify_prerequisites {
+    if [ ! $(command -v htpasswd) ]; then
+        echo "you need to install htpasswd as a prerequisite to running the tests" >&3
+        return 1
+    fi
+
     return 0
 }
 
@@ -15,15 +20,15 @@ function setup_file() {
     fi
 
     # Download test data to folder common for the entire suite, not just this file
-    skopeo --insecure-policy copy --format=oci docker://ghcr.io/project-zot/golang:1.20 oci:${TEST_DATA_DIR}/golang:1.20
+    skopeo --insecure-policy copy --format=oci docker://ghcr.io/project-zot/test-images/busybox:1.36 oci:${TEST_DATA_DIR}/busybox:1.36
     # Setup zot server
     local zot_root_dir=${BATS_FILE_TMPDIR}/zot
     local zot_config_file=${BATS_FILE_TMPDIR}/zot_config.json
     local oci_data_dir=${BATS_FILE_TMPDIR}/oci
-    local htpasswordFile=${BATS_FILE_TMPDIR}/htpasswd
+    local zot_htpasswd_file=${BATS_FILE_TMPDIR}/htpasswd
     mkdir -p ${zot_root_dir}
     mkdir -p ${oci_data_dir}
-    echo 'test:$2a$10$EIIoeCnvsIDAJeDL4T1sEOnL2fWOvsq7ACZbs3RT40BBBXg.Ih7V.' >> ${htpasswordFile}
+    htpasswd -Bbn ${AUTH_USER} ${AUTH_PASS} >> ${zot_htpasswd_file}
     cat > ${zot_config_file}<<EOF
 {
     "distSpecVersion": "1.1.0-dev",
@@ -35,7 +40,7 @@ function setup_file() {
         "port": "8080",
         "auth": {
             "htpasswd": {
-                "path": "${htpasswordFile}"
+                "path": "${zot_htpasswd_file}"
             }
         },
         "accessControl": {
@@ -45,7 +50,7 @@ function setup_file() {
                     "policies": [
                         {
                             "users": [
-                                "test"
+                                "${AUTH_USER}"
                             ],
                             "actions": [
                                 "read",
@@ -78,23 +83,23 @@ function teardown_file() {
 }
 
 @test "push image user policy" {
-    run skopeo --insecure-policy copy --dest-creds test:test --dest-tls-verify=false \
-        oci:${TEST_DATA_DIR}/golang:1.20 \
-        docker://127.0.0.1:8080/golang:1.20
+    run skopeo --insecure-policy copy --dest-creds ${AUTH_USER}:${AUTH_PASS} --dest-tls-verify=false \
+        oci:${TEST_DATA_DIR}/busybox:1.36 \
+        docker://127.0.0.1:8080/busybox:1.36
     [ "$status" -eq 0 ]
 }
 
 @test "pull image anonymous policy" {
     local oci_data_dir=${BATS_FILE_TMPDIR}/oci
     run skopeo --insecure-policy copy --src-tls-verify=false \
-        docker://127.0.0.1:8080/golang:1.20 \
-        oci:${oci_data_dir}/golang:1.20
+        docker://127.0.0.1:8080/busybox:1.36 \
+        oci:${oci_data_dir}/busybox:1.36
     [ "$status" -eq 0 ]
 }
 
 @test "push image anonymous policy" {
     run skopeo --insecure-policy copy --dest-tls-verify=false \
-        oci:${TEST_DATA_DIR}/golang:1.20 \
-        docker://127.0.0.1:8080/golang:1.20
+        oci:${TEST_DATA_DIR}/busybox:1.36 \
+        docker://127.0.0.1:8080/busybox:1.36
     [ "$status" -eq 1 ]
 }
diff --git a/test/blackbox/detect_manifest_collision.bats b/test/blackbox/detect_manifest_collision.bats
@@ -5,6 +5,11 @@
 load helpers_zot
 
 function verify_prerequisites {
+    if [ ! $(command -v htpasswd) ]; then
+        echo "you need to install htpasswd as a prerequisite to running the tests" >&3
+        return 1
+    fi
+
     return 0
 }
 
@@ -15,15 +20,16 @@ function setup_file() {
     fi
 
     # Download test data to folder common for the entire suite, not just this file
-    skopeo --insecure-policy copy --format=oci docker://ghcr.io/project-zot/golang:1.20 oci:${TEST_DATA_DIR}/golang:1.20
+    skopeo --insecure-policy copy --format=oci docker://ghcr.io/project-zot/test-images/busybox:1.36 oci:${TEST_DATA_DIR}/busybox:1.36
+
     # Setup zot server
     local zot_root_dir=${BATS_FILE_TMPDIR}/zot
     local zot_config_file=${BATS_FILE_TMPDIR}/zot_config.json
     local oci_data_dir=${BATS_FILE_TMPDIR}/oci
-    local htpasswordFile=${BATS_FILE_TMPDIR}/htpasswd
+    local zot_htpasswd_file=${BATS_FILE_TMPDIR}/htpasswd
     mkdir -p ${zot_root_dir}
     mkdir -p ${oci_data_dir}
-    echo 'test:$2a$10$EIIoeCnvsIDAJeDL4T1sEOnL2fWOvsq7ACZbs3RT40BBBXg.Ih7V.' >> ${htpasswordFile}
+    htpasswd -Bbn ${AUTH_USER} ${AUTH_PASS} >> ${zot_htpasswd_file}
     cat > ${zot_config_file}<<EOF
 {
     "distSpecVersion": "1.1.0-dev",
@@ -35,7 +41,7 @@ function setup_file() {
         "port": "8080",
         "auth": {
             "htpasswd": {
-                "path": "${htpasswordFile}"
+                "path": "${zot_htpasswd_file}"
             }
         },
         "accessControl": {
@@ -50,7 +56,7 @@ function setup_file() {
                     "policies": [
                         {
                             "users": [
-                                "test"
+                                "${AUTH_USER}"
                             ],
                             "actions": [
                                 "read",
@@ -83,21 +89,21 @@ function teardown_file() {
 }
 
 @test "push 2 images with same manifest with user policy" {
-    run skopeo --insecure-policy copy --dest-creds test:test --dest-tls-verify=false \
-        oci:${TEST_DATA_DIR}/golang:1.20 \
-        docker://127.0.0.1:8080/golang:1.20
+    run skopeo --insecure-policy copy --dest-creds ${AUTH_USER}:${AUTH_PASS} --dest-tls-verify=false \
+        oci:${TEST_DATA_DIR}/busybox:1.36 \
+        docker://127.0.0.1:8080/busybox:1.36
     [ "$status" -eq 0 ]
 
-    run skopeo --insecure-policy copy --dest-creds test:test --dest-tls-verify=false \
-        oci:${TEST_DATA_DIR}/golang:1.20 \
-        docker://127.0.0.1:8080/golang:latest
+    run skopeo --insecure-policy copy --dest-creds ${AUTH_USER}:${AUTH_PASS} --dest-tls-verify=false \
+        oci:${TEST_DATA_DIR}/busybox:1.36 \
+        docker://127.0.0.1:8080/busybox:latest
     [ "$status" -eq 0 ]
 }
 
 @test "skopeo delete image with anonymous policy should fail" {
     # skopeo deletes by digest, so it should fail with detectManifestCollision policy
     run skopeo --insecure-policy delete --tls-verify=false \
-        docker://127.0.0.1:8080/golang:1.20
+        docker://127.0.0.1:8080/busybox:1.36
     [ "$status" -eq 1 ]
     # conflict status code
     [[ "$output" == *"manifest invalid"* ]]
@@ -107,15 +113,15 @@ function teardown_file() {
     run regctl registry set localhost:8080 --tls disabled
     [ "$status" -eq 0 ]
 
-    run regctl image delete localhost:8080/golang:1.20 --force-tag-dereference
+    run regctl image delete localhost:8080/busybox:1.36 --force-tag-dereference
     [ "$status" -eq 1 ]
     # conflict status code
     [[ "$output" == *"409"* ]]
 }
 
 @test "delete image with user policy should work" {
     # should work without detectManifestCollision policy
-    run skopeo --insecure-policy delete --creds test:test --tls-verify=false \
-        docker://127.0.0.1:8080/golang:1.20
+    run skopeo --insecure-policy delete --creds ${AUTH_USER}:${AUTH_PASS} --tls-verify=false \
+        docker://127.0.0.1:8080/busybox:1.36
     [ "$status" -eq 0 ]
 }
diff --git a/test/blackbox/helpers_metrics.bash b/test/blackbox/helpers_metrics.bash
@@ -0,0 +1,6 @@
+function metrics_route_check () {
+    local servername="http://127.0.0.1:${1}/metrics"
+    status_code=$(curl --write-out '%{http_code}' ${2}  --silent --output /dev/null ${servername})
+
+    [ "$status_code" -eq ${3} ]
+}
diff --git a/test/blackbox/helpers_zot.bash b/test/blackbox/helpers_zot.bash
@@ -6,6 +6,8 @@ ZOT_PATH=${ROOT_DIR}/bin/zot-${OS}-${ARCH}
 ZLI_PATH=${ROOT_DIR}/bin/zli-${OS}-${ARCH}
 ZOT_MINIMAL_PATH=${ROOT_DIR}/bin/zot-${OS}-${ARCH}-minimal
 ZB_PATH=${ROOT_DIR}/bin/zb-${OS}-${ARCH}
+AUTH_USER=poweruser
+AUTH_PASS=sup*rSecr9T
 
 mkdir -p ${TEST_DATA_DIR}
-Original file line number
+Diff line change
@@ Expand Up / @@ -60,7 +60,7 @@ @@
                 }
               ],
               "defaultPolicy": ["read"]
-            }
+            }
           },
           "adminPolicy": {
             "users": ["admin"],
@@ Expand Down @@