fix: information disclosure bug in preconditions GET (minio#19810)

precondition check was being honored before, validating if anonymous access is allowed on the metadata of an object, leading to metadata disclosure of the following headers. ``` Last-Modified Etag x-amz-version-id Expires: Cache-Control: ``` although the information presented is minimal in nature, and of opaque nature. It still simply discloses that an object by a specific name exists or not without even having enough permissions.
telekom · May 27, 2024 · e0fe7cc · e0fe7cc
1 parent 9d20dec
commit e0fe7cc
Show file tree

Hide file tree

Showing 19 changed files with 91 additions and 92 deletions.
diff --git a/.github/workflows/multipart/migrate.sh b/.github/workflows/multipart/migrate.sh
@@ -24,8 +24,6 @@ if [ ! -f ./mc ]; then
 		chmod +x mc
 fi
 
-go install -v github.com/minio/minio/docs/debugging/s3-check-md5@latest
-
 export RELEASE=RELEASE.2023-08-29T23-07-35Z
 
 docker-compose -f docker-compose-site1.yaml up -d
@@ -45,10 +43,10 @@ sleep 30s
 
 sleep 5
 
-s3-check-md5 -h
+./s3-check-md5 -h
 
-failed_count_site1=$(s3-check-md5 -versions -access-key minioadmin -secret-key minioadmin -endpoint http://site1-nginx:9001 -bucket testbucket 2>&1 | grep FAILED | wc -l)
-failed_count_site2=$(s3-check-md5 -versions -access-key minioadmin -secret-key minioadmin -endpoint http://site2-nginx:9002 -bucket testbucket 2>&1 | grep FAILED | wc -l)
+failed_count_site1=$(./s3-check-md5 -versions -access-key minioadmin -secret-key minioadmin -endpoint http://site1-nginx:9001 -bucket testbucket 2>&1 | grep FAILED | wc -l)
+failed_count_site2=$(./s3-check-md5 -versions -access-key minioadmin -secret-key minioadmin -endpoint http://site2-nginx:9002 -bucket testbucket 2>&1 | grep FAILED | wc -l)
 
 if [ $failed_count_site1 -ne 0 ]; then
 	echo "failed with multipart on site1 uploads"
@@ -64,8 +62,8 @@ fi
 
 sleep 5
 
-failed_count_site1=$(s3-check-md5 -versions -access-key minioadmin -secret-key minioadmin -endpoint http://site1-nginx:9001 -bucket testbucket 2>&1 | grep FAILED | wc -l)
-failed_count_site2=$(s3-check-md5 -versions -access-key minioadmin -secret-key minioadmin -endpoint http://site2-nginx:9002 -bucket testbucket 2>&1 | grep FAILED | wc -l)
+failed_count_site1=$(./s3-check-md5 -versions -access-key minioadmin -secret-key minioadmin -endpoint http://site1-nginx:9001 -bucket testbucket 2>&1 | grep FAILED | wc -l)
+failed_count_site2=$(./s3-check-md5 -versions -access-key minioadmin -secret-key minioadmin -endpoint http://site2-nginx:9002 -bucket testbucket 2>&1 | grep FAILED | wc -l)
 
 ## we do not need to fail here, since we are going to test
 ## upgrading to master, healing and being able to recover
@@ -93,8 +91,8 @@ for i in $(seq 1 10); do
 	./mc admin heal -r --remove --json site2/ 2>&1 >/dev/null
 done
 
-failed_count_site1=$(s3-check-md5 -versions -access-key minioadmin -secret-key minioadmin -endpoint http://site1-nginx:9001 -bucket testbucket 2>&1 | grep FAILED | wc -l)
-failed_count_site2=$(s3-check-md5 -versions -access-key minioadmin -secret-key minioadmin -endpoint http://site2-nginx:9002 -bucket testbucket 2>&1 | grep FAILED | wc -l)
+failed_count_site1=$(./s3-check-md5 -versions -access-key minioadmin -secret-key minioadmin -endpoint http://site1-nginx:9001 -bucket testbucket 2>&1 | grep FAILED | wc -l)
+failed_count_site2=$(./s3-check-md5 -versions -access-key minioadmin -secret-key minioadmin -endpoint http://site2-nginx:9002 -bucket testbucket 2>&1 | grep FAILED | wc -l)
 
 if [ $failed_count_site1 -ne 0 ]; then
 	echo "failed with multipart on site1 uploads"

diff --git a/.gitignore b/.gitignore
@@ -43,4 +43,13 @@ docs/debugging/inspect/inspect
 docs/debugging/pprofgoparser/pprofgoparser
 docs/debugging/reorder-disks/reorder-disks
 docs/debugging/populate-hard-links/populate-hardlinks
-docs/debugging/xattr/xattr
+docs/debugging/xattr/xattr
+hash-set
+healing-bin
+inspect
+pprofgoparser
+reorder-disks
+s3-check-md5
+s3-verify
+xattr
+xl-meta
diff --git a/Makefile b/Makefile
@@ -45,7 +45,7 @@ lint-fix: getdeps ## runs golangci-lint suite of linters with automatic fixes
 	@$(GOLANGCI) run --build-tags kqueue --timeout=10m --config ./.golangci.yml --fix
 
 check: test
-test: verifiers build build-debugging ## builds minio, runs linters, tests
+test: verifiers build ## builds minio, runs linters, tests
 	@echo "Running unit tests"
 	@MINIO_API_REQUESTS_MAX=10000 CGO_ENABLED=0 go test -v -tags kqueue ./...
 
@@ -127,37 +127,32 @@ test-site-replication-minio: install-race ## verify automatic site replication
 	@echo "Running tests for automatic site replication of SSE-C objects with compression enabled for site"
 	@(env bash $(PWD)/docs/site-replication/run-ssec-object-replication-with-compression.sh)
 
-verify: ## verify minio various setups
+verify: install-race ## verify minio various setups
 	@echo "Verifying build with race"
-	@GORACE=history_size=7 CGO_ENABLED=1 go build -race -tags kqueue -trimpath --ldflags "$(LDFLAGS)" -o $(PWD)/minio 1>/dev/null
 	@(env bash $(PWD)/buildscripts/verify-build.sh)
 
-verify-healing: ## verify healing and replacing disks with minio binary
+verify-healing: install-race ## verify healing and replacing disks with minio binary
 	@echo "Verify healing build with race"
-	@GORACE=history_size=7 CGO_ENABLED=1 go build -race -tags kqueue -trimpath --ldflags "$(LDFLAGS)" -o $(PWD)/minio 1>/dev/null
 	@(env bash $(PWD)/buildscripts/verify-healing.sh)
 	@(env bash $(PWD)/buildscripts/verify-healing-empty-erasure-set.sh)
 	@(env bash $(PWD)/buildscripts/heal-inconsistent-versions.sh)
 
-verify-healing-with-root-disks: ## verify healing root disks
+verify-healing-with-root-disks: install-race ## verify healing root disks
 	@echo "Verify healing with root drives"
-	@GORACE=history_size=7 CGO_ENABLED=1 go build -race -tags kqueue -trimpath --ldflags "$(LDFLAGS)" -o $(PWD)/minio 1>/dev/null
 	@(env bash $(PWD)/buildscripts/verify-healing-with-root-disks.sh)
 
-verify-healing-with-rewrite: ## verify healing to rewrite old xl.meta -> new xl.meta
+verify-healing-with-rewrite: install-race ## verify healing to rewrite old xl.meta -> new xl.meta
 	@echo "Verify healing with rewrite"
-	@GORACE=history_size=7 CGO_ENABLED=1 go build -race -tags kqueue -trimpath --ldflags "$(LDFLAGS)" -o $(PWD)/minio 1>/dev/null
 	@(env bash $(PWD)/buildscripts/rewrite-old-new.sh)
 
-verify-healing-inconsistent-versions: ## verify resolving inconsistent versions
+verify-healing-inconsistent-versions: install-race ## verify resolving inconsistent versions
 	@echo "Verify resolving inconsistent versions build with race"
-	@GORACE=history_size=7 CGO_ENABLED=1 go build -race -tags kqueue -trimpath --ldflags "$(LDFLAGS)" -o $(PWD)/minio 1>/dev/null
 	@(env bash $(PWD)/buildscripts/resolve-right-versions.sh)
 
 build-debugging:
 	@(env bash $(PWD)/docs/debugging/build.sh)
 
-build: checks ## builds minio to $(PWD)
+build: checks build-debugging ## builds minio to $(PWD)
 	@echo "Building minio binary to './minio'"
 	@CGO_ENABLED=0 go build -tags kqueue -trimpath --ldflags "$(LDFLAGS)" -o $(PWD)/minio 1>/dev/null
 
@@ -196,15 +191,15 @@ docker: build ## builds minio docker container
 	@echo "Building minio docker image '$(TAG)'"
 	@docker build -q --no-cache -t $(TAG) . -f Dockerfile
 
-install-race: checks ## builds minio to $(PWD)
+install-race: checks build-debugging ## builds minio to $(PWD)
 	@echo "Building minio binary with -race to './minio'"
 	@GORACE=history_size=7 CGO_ENABLED=1 go build -tags kqueue -race -trimpath --ldflags "$(LDFLAGS)" -o $(PWD)/minio 1>/dev/null
 	@echo "Installing minio binary with -race to '$(GOPATH)/bin/minio'"
-	@mkdir -p $(GOPATH)/bin && cp -f $(PWD)/minio $(GOPATH)/bin/minio
+	@mkdir -p $(GOPATH)/bin && cp -af $(PWD)/minio $(GOPATH)/bin/minio
 
 install: build ## builds minio and installs it to $GOPATH/bin.
 	@echo "Installing minio binary to '$(GOPATH)/bin/minio'"
-	@mkdir -p $(GOPATH)/bin && cp -f $(PWD)/minio $(GOPATH)/bin/minio
+	@mkdir -p $(GOPATH)/bin && cp -af $(PWD)/minio $(GOPATH)/bin/minio
 	@echo "Installation successful. To learn more, try \"minio --help\"."
 
 clean: ## cleanup all generated assets

diff --git a/buildscripts/rewrite-old-new.sh b/buildscripts/rewrite-old-new.sh
@@ -45,7 +45,8 @@ function verify_rewrite() {
 	"${MINIO_OLD[@]}" --address ":$start_port" "${WORK_DIR}/xl{1...16}" >"${WORK_DIR}/server1.log" 2>&1 &
 	pid=$!
 	disown $pid
-	sleep 10
+
+	"${WORK_DIR}/mc" ready minio/
 
 	if ! ps -p ${pid} 1>&2 >/dev/null; then
 		echo "server1 log:"
@@ -77,7 +78,8 @@ function verify_rewrite() {
 	"${MINIO[@]}" --address ":$start_port" "${WORK_DIR}/xl{1...16}" >"${WORK_DIR}/server1.log" 2>&1 &
 	pid=$!
 	disown $pid
-	sleep 10
+
+	"${WORK_DIR}/mc" ready minio/
 
 	if ! ps -p ${pid} 1>&2 >/dev/null; then
 		echo "server1 log:"
@@ -87,14 +89,12 @@ function verify_rewrite() {
 		exit 1
 	fi
 
-	go install -v github.com/minio/minio/docs/debugging/s3-check-md5@latest
-
-	if ! s3-check-md5 \
+	if ! ./s3-check-md5 \
 		-debug \
 		-versions \
 		-access-key minio \
 		-secret-key minio123 \
-		-endpoint http://127.0.0.1:${start_port}/ 2>&1 | grep INTACT; then
+		-endpoint "http://127.0.0.1:${start_port}/" 2>&1 | grep INTACT; then
 		echo "server1 log:"
 		cat "${WORK_DIR}/server1.log"
 		echo "FAILED"
@@ -114,7 +114,7 @@ function verify_rewrite() {
 	go run ./buildscripts/heal-manual.go "127.0.0.1:${start_port}" "minio" "minio123"
 	sleep 1
 
-	if ! s3-check-md5 \
+	if ! ./s3-check-md5 \
 		-debug \
 		-versions \
 		-access-key minio \

diff --git a/buildscripts/verify-healing-empty-erasure-set.sh b/buildscripts/verify-healing-empty-erasure-set.sh
@@ -19,7 +19,7 @@ function start_minio_3_node() {
 	export MINIO_ERASURE_SET_DRIVE_COUNT=6
 	export MINIO_CI_CD=1
 
-	start_port=$2
+	start_port=$1
 	args=""
 	for i in $(seq 1 3); do
 		args="$args http://127.0.0.1:$((start_port + i))${WORK_DIR}/$i/1/ http://127.0.0.1:$((start_port + i))${WORK_DIR}/$i/2/ http://127.0.0.1:$((start_port + i))${WORK_DIR}/$i/3/ http://127.0.0.1:$((start_port + i))${WORK_DIR}/$i/4/ http://127.0.0.1:$((start_port + i))${WORK_DIR}/$i/5/ http://127.0.0.1:$((start_port + i))${WORK_DIR}/$i/6/"
@@ -37,7 +37,8 @@ function start_minio_3_node() {
 	pid3=$!
 	disown $pid3
 
-	sleep "$1"
+	export MC_HOST_myminio="http://minio:[email protected]:$((start_port + 1))"
+	/tmp/mc ready myminio
 
 	if ! ps -p $pid1 1>&2 >/dev/null; then
 		echo "server1 log:"
@@ -99,18 +100,23 @@ function __init__() {
 
 	## version is purposefully set to '3' for minio to migrate configuration file
 	echo '{"version": "3", "credential": {"accessKey": "minio", "secretKey": "minio123"}, "region": "us-east-1"}' >"$MINIO_CONFIG_DIR/config.json"
+
+	if [ ! -f /tmp/mc ]; then
+		wget --quiet -O /tmp/mc https://dl.minio.io/client/mc/release/linux-amd64/mc &&
+			chmod +x /tmp/mc
+	fi
 }
 
 function perform_test() {
-	start_minio_3_node 120 $2
+	start_minio_3_node $2
 
 	echo "Testing Distributed Erasure setup healing of drives"
 	echo "Remove the contents of the disks belonging to '${1}' erasure set"
 
 	rm -rf ${WORK_DIR}/${1}/*/
 
 	set -x
-	start_minio_3_node 120 $2
+	start_minio_3_node $2
 
 	rv=$(check_online)
 	if [ "$rv" == "1" ]; then

diff --git a/buildscripts/verify-healing.sh b/buildscripts/verify-healing.sh
@@ -15,14 +15,18 @@ MINIO=("$PWD/minio" --config-dir "$MINIO_CONFIG_DIR" server)
 GOPATH=/tmp/gopath
 
 function start_minio_3_node() {
+	for i in $(seq 1 3); do
+		rm "${WORK_DIR}/dist-minio-server$i.log"
+	done
+
 	export MINIO_ROOT_USER=minio
 	export MINIO_ROOT_PASSWORD=minio123
 	export MINIO_ERASURE_SET_DRIVE_COUNT=6
 	export MINIO_CI_CD=1
 
 	first_time=$(find ${WORK_DIR}/ | grep format.json | wc -l)
 
-	start_port=$2
+	start_port=$1
 	args=""
 	for d in $(seq 1 3 5); do
 		args="$args http://127.0.0.1:$((start_port + 1))${WORK_DIR}/1/${d}/ http://127.0.0.1:$((start_port + 2))${WORK_DIR}/2/${d}/ http://127.0.0.1:$((start_port + 3))${WORK_DIR}/3/${d}/ "
@@ -42,9 +46,11 @@ function start_minio_3_node() {
 	pid3=$!
 	disown $pid3
 
-	sleep "$1"
+	export MC_HOST_myminio="http://minio:[email protected]:$((start_port + 1))"
+	/tmp/mc ready myminio
 
-	[ ${first_time} -eq 0 ] && upload_objects $start_port
+	[ ${first_time} -eq 0 ] && upload_objects
+	[ ${first_time} -ne 0 ] && sleep 120
 
 	if ! ps -p $pid1 1>&2 >/dev/null; then
 		echo "server1 log:"
@@ -127,10 +133,6 @@ function __init__() {
 }
 
 function upload_objects() {
-	start_port=$1
-
-	/tmp/mc alias set myminio http://127.0.0.1:$((start_port + 1)) minio minio123 --api=s3v4
-	/tmp/mc ready myminio
 	/tmp/mc mb myminio/testbucket/
 	for ((i = 0; i < 20; i++)); do
 		echo "my content" | /tmp/mc pipe myminio/testbucket/file-$i
@@ -140,15 +142,15 @@ function upload_objects() {
 function perform_test() {
 	start_port=$2
 
-	start_minio_3_node 120 $start_port
+	start_minio_3_node $start_port
 
 	echo "Testing Distributed Erasure setup healing of drives"
 	echo "Remove the contents of the disks belonging to '${1}' node"
 
 	rm -rf ${WORK_DIR}/${1}/*/
 
 	set -x
-	start_minio_3_node 120 $start_port
+	start_minio_3_node $start_port
 
 	check_heal ${1}
 	rv=$?

diff --git a/cmd/object-handlers.go b/cmd/object-handlers.go
@@ -476,6 +476,15 @@ func (api objectAPIHandlers) getObjectHandler(ctx context.Context, objectAPI Obj
 			return true
 		}
 
+		if oi.UserTags != "" {
+			r.Header.Set(xhttp.AmzObjectTagging, oi.UserTags)
+		}
+
+		if s3Error := authorizeRequest(ctx, r, policy.GetObjectAction); s3Error != ErrNone {
+			writeErrorResponse(ctx, w, errorCodes.ToAPIErr(s3Error), r.URL)
+			return true
+		}
+
 		return checkPreconditions(ctx, w, r, oi, opts)
 	}
 
@@ -547,15 +556,6 @@ func (api objectAPIHandlers) getObjectHandler(ctx context.Context, objectAPI Obj
 
 	objInfo := gr.ObjInfo
 
-	if objInfo.UserTags != "" {
-		r.Header.Set(xhttp.AmzObjectTagging, objInfo.UserTags)
-	}
-
-	if s3Error := authorizeRequest(ctx, r, policy.GetObjectAction); s3Error != ErrNone {
-		writeErrorResponse(ctx, w, errorCodes.ToAPIErr(s3Error), r.URL)
-		return
-	}
-
 	if !proxy.Proxy { // apply lifecycle rules only for local requests
 		// Automatically remove the object/version if an expiry lifecycle rule can be applied
 		if lc, err := globalLifecycleSys.Get(bucket); err == nil {

diff --git a/docs/bucket/replication/setup_3site_replication.sh b/docs/bucket/replication/setup_3site_replication.sh
@@ -43,8 +43,6 @@ unset MINIO_KMS_KES_KEY_FILE
 unset MINIO_KMS_KES_ENDPOINT
 unset MINIO_KMS_KES_KEY_NAME
 
-go install -v github.com/minio/minio/docs/debugging/s3-check-md5@latest
-
 wget -q -O mc https://dl.minio.io/client/mc/release/linux-amd64/mc &&
 	chmod +x mc
 
@@ -202,19 +200,19 @@ head -c 221227088 </dev/urandom >200M
 sleep 10
 
 echo "Verifying ETag for all objects"
-s3-check-md5 -versions -access-key minio -secret-key minio123 -endpoint http://127.0.0.1:9001/ -bucket bucket
-s3-check-md5 -versions -access-key minio -secret-key minio123 -endpoint http://127.0.0.1:9002/ -bucket bucket
-s3-check-md5 -versions -access-key minio -secret-key minio123 -endpoint http://127.0.0.1:9003/ -bucket bucket
-s3-check-md5 -versions -access-key minio -secret-key minio123 -endpoint http://127.0.0.1:9004/ -bucket bucket
-s3-check-md5 -versions -access-key minio -secret-key minio123 -endpoint http://127.0.0.1:9005/ -bucket bucket
-s3-check-md5 -versions -access-key minio -secret-key minio123 -endpoint http://127.0.0.1:9006/ -bucket bucket
-
-s3-check-md5 -versions -access-key minio -secret-key minio123 -endpoint http://127.0.0.1:9001/ -bucket olockbucket
-s3-check-md5 -versions -access-key minio -secret-key minio123 -endpoint http://127.0.0.1:9002/ -bucket olockbucket
-s3-check-md5 -versions -access-key minio -secret-key minio123 -endpoint http://127.0.0.1:9003/ -bucket olockbucket
-s3-check-md5 -versions -access-key minio -secret-key minio123 -endpoint http://127.0.0.1:9004/ -bucket olockbucket
-s3-check-md5 -versions -access-key minio -secret-key minio123 -endpoint http://127.0.0.1:9005/ -bucket olockbucket
-s3-check-md5 -versions -access-key minio -secret-key minio123 -endpoint http://127.0.0.1:9006/ -bucket olockbucket
+./s3-check-md5 -versions -access-key minio -secret-key minio123 -endpoint http://127.0.0.1:9001/ -bucket bucket
+./s3-check-md5 -versions -access-key minio -secret-key minio123 -endpoint http://127.0.0.1:9002/ -bucket bucket
+./s3-check-md5 -versions -access-key minio -secret-key minio123 -endpoint http://127.0.0.1:9003/ -bucket bucket
+./s3-check-md5 -versions -access-key minio -secret-key minio123 -endpoint http://127.0.0.1:9004/ -bucket bucket
+./s3-check-md5 -versions -access-key minio -secret-key minio123 -endpoint http://127.0.0.1:9005/ -bucket bucket
+./s3-check-md5 -versions -access-key minio -secret-key minio123 -endpoint http://127.0.0.1:9006/ -bucket bucket
+
+./s3-check-md5 -versions -access-key minio -secret-key minio123 -endpoint http://127.0.0.1:9001/ -bucket olockbucket
+./s3-check-md5 -versions -access-key minio -secret-key minio123 -endpoint http://127.0.0.1:9002/ -bucket olockbucket
+./s3-check-md5 -versions -access-key minio -secret-key minio123 -endpoint http://127.0.0.1:9003/ -bucket olockbucket
+./s3-check-md5 -versions -access-key minio -secret-key minio123 -endpoint http://127.0.0.1:9004/ -bucket olockbucket
+./s3-check-md5 -versions -access-key minio -secret-key minio123 -endpoint http://127.0.0.1:9005/ -bucket olockbucket
+./s3-check-md5 -versions -access-key minio -secret-key minio123 -endpoint http://127.0.0.1:9006/ -bucket olockbucket
 
 # additional tests for encryption object alignment
 go install -v github.com/minio/multipart-debug@latest

diff --git a/docs/debugging/build.sh b/docs/debugging/build.sh
@@ -2,5 +2,6 @@
 
 export CGO_ENABLED=0
 for dir in docs/debugging/*/; do
-	go build -C ${dir} -v
+	bin=$(basename ${dir})
+	go build -C ${dir} -o ${PWD}/${bin}
 done
diff --git a/docs/debugging/inspect/go.mod b/docs/debugging/inspect/go.mod
@@ -1,6 +1,6 @@
 module github.com/minio/minio/docs/debugging/inspect
 
-go 1.19
+go 1.21
 
 require (
 	github.com/klauspost/compress v1.17.4

diff --git a/docs/debugging/pprofgoparser/go.mod b/docs/debugging/pprofgoparser/go.mod
@@ -1,3 +1,3 @@
 module github.com/minio/minio/docs/debugging/pprofgoparser
 
-go 1.19
+go 1.21
diff --git a/docs/debugging/reorder-disks/go.mod b/docs/debugging/reorder-disks/go.mod
@@ -1,5 +1,5 @@
 module github.com/minio/minio/docs/debugging/reorder-disks
 
-go 1.19
+go 1.21
 
-require github.com/minio/pkg/v2 v2.0.6
+require github.com/minio/pkg/v3 v3.0.1
diff --git a/docs/debugging/reorder-disks/go.sum b/docs/debugging/reorder-disks/go.sum
@@ -1,2 +1,2 @@
-github.com/minio/pkg/v2 v2.0.6 h1:n+PpbSMaJK1FfQkP55l1y0wj5Hi9R5w2DtGhxiGdP9I=
-github.com/minio/pkg/v2 v2.0.6/go.mod h1:Z9Z/LzhTIxZ6zhPeW658vmLRilRek3zBOqNB9j+lxSY=
+github.com/minio/pkg/v3 v3.0.1 h1:qts6g9rYjAdeomRdwjnMc1IaQ6KbaJs3dwqBntXziaw=
+github.com/minio/pkg/v3 v3.0.1/go.mod h1:53gkSUVHcfYoskOs5YAJ3D99nsd2SKru90rdE9whlXU=