Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: metrics endpoint must be secured behind authN #1864

Merged
merged 1 commit into from
Oct 2, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion examples/config-policy.json
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@
}
],
"defaultPolicy": ["read"]
}
}
},
"adminPolicy": {
"users": ["admin"],
Expand Down
20 changes: 10 additions & 10 deletions pkg/api/controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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).
Expand Down Expand Up @@ -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).
Expand Down Expand Up @@ -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).
Expand Down Expand Up @@ -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).
Expand All @@ -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).
Expand Down Expand Up @@ -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).
Expand All @@ -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).
Expand Down Expand Up @@ -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).
Expand Down Expand Up @@ -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).
Expand All @@ -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).
Expand Down
4 changes: 2 additions & 2 deletions pkg/api/errors/errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.",
},

Expand Down
1 change: 1 addition & 0 deletions pkg/extensions/extension_metrics_disabled.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
}
6 changes: 3 additions & 3 deletions pkg/extensions/extensions_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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).
Expand Down Expand Up @@ -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).
Expand All @@ -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).
Expand Down
29 changes: 17 additions & 12 deletions test/blackbox/anonymous_policy.bats
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand All @@ -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",
Expand All @@ -35,7 +40,7 @@ function setup_file() {
"port": "8080",
"auth": {
"htpasswd": {
"path": "${htpasswordFile}"
"path": "${zot_htpasswd_file}"
}
},
"accessControl": {
Expand All @@ -45,7 +50,7 @@ function setup_file() {
"policies": [
{
"users": [
"test"
"${AUTH_USER}"
],
"actions": [
"read",
Expand Down Expand Up @@ -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 ]
}
36 changes: 21 additions & 15 deletions test/blackbox/detect_manifest_collision.bats
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand All @@ -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",
Expand All @@ -35,7 +41,7 @@ function setup_file() {
"port": "8080",
"auth": {
"htpasswd": {
"path": "${htpasswordFile}"
"path": "${zot_htpasswd_file}"
}
},
"accessControl": {
Expand All @@ -50,7 +56,7 @@ function setup_file() {
"policies": [
{
"users": [
"test"
"${AUTH_USER}"
],
"actions": [
"read",
Expand Down Expand Up @@ -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"* ]]
Expand All @@ -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 ]
}
6 changes: 6 additions & 0 deletions test/blackbox/helpers_metrics.bash
Original file line number Diff line number Diff line change
@@ -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} ]
}
2 changes: 2 additions & 0 deletions test/blackbox/helpers_zot.bash
Original file line number Diff line number Diff line change
Expand Up @@ -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}

Expand Down
Loading
Loading