diff --git a/test/s3/error_compat/Makefile b/test/s3/error_compat/Makefile new file mode 100644 index 000000000..ef1914bda --- /dev/null +++ b/test/s3/error_compat/Makefile @@ -0,0 +1,101 @@ +# S3/IAM error response compatibility test harness + +.PHONY: all test help build-weed check-deps start-server stop-server test-errors test-with-server clean logs + +WEED_BINARY := ../../../weed/weed_binary +S3_PORT := 8334 +TEST_TIMEOUT := 10m +SERVER_DIR := ./test-volume-data +S3_CONFIG := ./test_s3.json + +.DEFAULT_GOAL := help + +all: test + +test: test-with-server + +help: + @echo "S3/IAM error response compatibility test harness" + @echo "" + @echo "Available targets:" + @echo " build-weed - Build the SeaweedFS binary" + @echo " start-server - Start SeaweedFS mini with S3 and embedded IAM enabled" + @echo " stop-server - Stop the test server" + @echo " test-errors - Run the error compatibility checks" + @echo " test-with-server - Start server, run checks, stop server" + @echo " logs - Show server logs" + @echo " clean - Remove test artifacts" + +build-weed: + @echo "Building SeaweedFS binary..." + @cd ../../../weed && go build -o weed_binary . + @chmod +x $(WEED_BINARY) + +check-deps: build-weed + @command -v go >/dev/null 2>&1 || (echo "Go is required but not installed" && exit 1) + @command -v python3 >/dev/null 2>&1 || (echo "python3 is required but not installed" && exit 1) + @python3 -c "import botocore" 2>/dev/null || (echo "botocore is required: pip install botocore" && exit 1) + @test -f $(WEED_BINARY) || (echo "SeaweedFS binary not found at $(WEED_BINARY)" && exit 1) + @test -f $(S3_CONFIG) || (echo "S3 config not found at $(S3_CONFIG)" && exit 1) + +start-server: check-deps + @echo "Starting SeaweedFS mini for error compatibility tests..." + @rm -f weed-server.pid + @mkdir -p $(SERVER_DIR) + @if curl -sS -o /dev/null http://127.0.0.1:$(S3_PORT)/ >/dev/null 2>&1; then \ + echo "port $(S3_PORT) is already served by another process; refusing to test against it"; \ + exit 1; \ + fi + @$(WEED_BINARY) mini \ + -dir=$(SERVER_DIR) \ + -s3 \ + -s3.port=$(S3_PORT) \ + -s3.config=$(S3_CONFIG) \ + -s3.iam.readOnly=false \ + -master.peers=none \ + > weed-test.log 2>&1 & echo $$! > weed-server.pid + @for i in $$(seq 1 60); do \ + kill -0 $$(cat weed-server.pid) 2>/dev/null || { \ + echo "SeaweedFS server exited during startup"; \ + echo "=== Server logs ==="; \ + test -f weed-test.log && cat weed-test.log || true; \ + exit 1; \ + }; \ + if curl -sS -o /dev/null http://127.0.0.1:$(S3_PORT)/ >/dev/null 2>&1; then \ + echo "SeaweedFS S3 server is ready on port $(S3_PORT)"; \ + sleep 3; \ + exit 0; \ + fi; \ + sleep 1; \ + done; \ + echo "SeaweedFS S3 server failed to start"; \ + echo "=== Server logs ==="; \ + test -f weed-test.log && cat weed-test.log || true; \ + $(MAKE) --no-print-directory stop-server; \ + exit 1 + +stop-server: + @if [ -f weed-server.pid ]; then \ + echo "Stopping SeaweedFS server..."; \ + kill $$(cat weed-server.pid) 2>/dev/null || true; \ + sleep 2; \ + rm -f weed-server.pid; \ + fi + +test-errors: + @echo "Running S3/IAM error compatibility checks..." + python3 s3_error_compat_test.py \ + --endpoint http://127.0.0.1:$(S3_PORT) \ + --access-key admin \ + --secret-key admin + +test-with-server: start-server + @$(MAKE) test-errors; TEST_RESULT=$$?; \ + $(MAKE) stop-server; exit $$TEST_RESULT + +logs: + @test -f weed-test.log && tail -100 weed-test.log || echo "No log file found" + +clean: stop-server + @rm -rf $(SERVER_DIR) weed-test.log weed-server.pid + @echo "Cleaned up test artifacts" diff --git a/test/s3/error_compat/s3_error_compat_test.py b/test/s3/error_compat/s3_error_compat_test.py new file mode 100644 index 000000000..ccda33374 --- /dev/null +++ b/test/s3/error_compat/s3_error_compat_test.py @@ -0,0 +1,216 @@ +#!/usr/bin/env python3 +"""Verify S3/IAM error responses match AWS for common client mistakes. + +Needs only botocore (pip install botocore). Sends raw SigV4 requests so the exact +status and response body are visible, compares each with what AWS returns, then +cleans up everything it created (one bucket, one IAM user). + +Usage: + python3 s3_error_compat_test.py --endpoint http://127.0.0.1:8333 \ + --access-key --secret-key + +The key needs Admin rights, and IAM writes must be enabled (-s3.iam.readOnly=false). +""" +import argparse, atexit, base64, datetime, hashlib, http.client, re, sys, time, urllib.parse, uuid + +from botocore.auth import S3SigV4Auth, SigV4Auth +from botocore.awsrequest import AWSRequest +from botocore.credentials import Credentials + +ap = argparse.ArgumentParser() +ap.add_argument('--endpoint', default='http://127.0.0.1:8333') +ap.add_argument('--access-key', required=True) +ap.add_argument('--secret-key', required=True) +ap.add_argument('--region', default='us-east-1') +args = ap.parse_args() + +EP = args.endpoint.rstrip('/') +U = urllib.parse.urlsplit(EP) +ADMIN = Credentials(args.access_key, args.secret_key) +BAD_KEY = Credentials('AKIAREPRONOSUCHKEY00', 'x' * 40) +BAD_SECRET = Credentials(args.access_key, 'y' * 40) +B = 'errorcompat-' + uuid.uuid4().hex[:8] +S3DOC = 'https://docs.aws.amazon.com/AmazonS3/latest/API/' +IAMDOC = 'https://docs.aws.amazon.com/IAM/latest/APIReference/' +rows = [] + + +def send(method, path, body=b'', headers=None, creds=ADMIN, service='s3', mutate=None, sign=True, + keep_sha256_header=False): + headers = dict(headers or {}) + url = EP + path + if sign: + req = AWSRequest(method=method, url=url, data=body, headers=headers) + if service == 's3': + if 'X-Amz-Content-SHA256' not in req.headers: + req.headers['X-Amz-Content-SHA256'] = hashlib.sha256(body).hexdigest() + # S3SigV4Auth always recomputes X-Amz-Content-SHA256 from the body; the + # generic SigV4Auth signs a caller-supplied value instead, which is how + # a mismatched declared hash can be produced on the wire. + signer = SigV4Auth if keep_sha256_header else S3SigV4Auth + signer(creds, 's3', args.region).add_auth(req) + else: + SigV4Auth(creds, service, args.region).add_auth(req) + headers = dict(req.headers.items()) + if mutate: + headers = mutate(headers) + conn = http.client.HTTPConnection(U.hostname, U.port, timeout=30) + conn.request(method, path, body=body, headers=headers) + r = conn.getresponse() + data = r.read().decode('utf-8', 'replace') + conn.close() + return r.status, data + + +def shape(data): + """Root element and of an XML error body.""" + body = re.sub(r'<\?xml[^>]*\?>', '', data).strip() + root = re.match(r'<(\w+)', body) + code = re.search(r'([^<]*)', data) + return (root.group(1) if root else ('-' if not body else 'non-xml')), (code.group(1) if code else '') + + +def iam(params, creds=ADMIN): + p = dict(params, Version='2010-05-08') + return dict(method='POST', path='/', body=urllib.parse.urlencode(p).encode(), creds=creds, service='iam', + headers={'Content-Type': 'application/x-www-form-urlencoded; charset=utf-8'}) + + +def case(group, name, expect, ref, req, check=None): + """expect = (status, root element or None, code or None); check() adds a side-effect note.""" + st, data = send(**req) + root, code = shape(data) + note = check() if check else '' + ok = st == expect[0] and expect[1] in (None, root) and expect[2] in (None, code) and not note + rows.append((group, name, '%s %s %s' % (st, root, code), '%s %s %s' % (expect[0], expect[1] or '', expect[2] or ''), note, ok, ref)) + + +def exists(key): + return lambda: 'object %r now exists' % key if send('HEAD', '/%s/%s' % (B, key))[0] == 200 else '' + + +def S3(method, path, **kw): + return dict(method=method, path=path, **kw) + + +# ---------------- cleanup (atexit so failed runs still clean up) ---------------- +user = USER = upload_id = None + + +def cleanup(): + try: + if user and USER: + send(**iam({'Action': 'DeleteAccessKey', 'UserName': user, 'AccessKeyId': USER.access_key})) + if user: + send(**iam({'Action': 'DeleteUser', 'UserName': user})) + if upload_id: + send('DELETE', '/%s/mp?uploadId=%s' % (B, upload_id)) + st, d = send('GET', '/%s?list-type=2' % B) + for k in re.findall(r'([^<]+)', d): + send('DELETE', '/%s/%s' % (B, urllib.parse.quote(k))) + send('DELETE', '/' + B) + except Exception as e: + print('cleanup failed: %s' % e) + + +atexit.register(cleanup) + +# ---------------- setup ---------------- +st, d = send('PUT', '/' + B) +if st != 200: + sys.exit('cannot create bucket %s: %s %s' % (B, st, d[:200])) +send('PUT', '/%s/obj' % B, body=b'hello') +st, d = send('POST', '/%s/mp?uploads' % B) +upload_id = re.search(r'([^<]+)', d).group(1) +user = 'errorcompat-' + uuid.uuid4().hex[:6] +send(**iam({'Action': 'CreateUser', 'UserName': user})) +st, d = send(**iam({'Action': 'CreateAccessKey', 'UserName': user})) +USER = Credentials(re.search(r'([^<]+)', d).group(1), re.search(r'([^<]+)', d).group(1)) +for _ in range(15): # wait until the new key is accepted + if 'InvalidAccessKeyId' not in send('GET', '/', creds=USER)[1]: + break + time.sleep(1) + +# ---------------- 1. requests that run a different operation ---------------- +for sub in ('logging', 'notification', 'accelerate', 'website', 'replication', + 'analytics&id=a', 'inventory&id=a', 'metrics&id=a', 'intelligent-tiering&id=a'): + case(1, 'PUT ?%s' % sub, (501, 'Error', 'NotImplemented'), S3DOC + 'API_Error.html#:~:text=Code%3A%20NotImplemented', + S3('PUT', '/%s?%s' % (B, sub), body=b'')) +case(1, 'DELETE ?logging', (501, 'Error', 'NotImplemented'), S3DOC + 'API_Error.html#:~:text=Code%3A%20NotImplemented', + S3('DELETE', '/%s?logging' % B), + lambda: '' if send('HEAD', '/' + B)[0] == 200 else 'BUCKET DELETED') +case(1, 'CopyObject, x-amz-copy-source without "/"', (400, 'Error', 'InvalidArgument'), + S3DOC + 'API_CopyObject.html#AmazonS3-CopyObject-request-header-CopySource', + S3('PUT', '/%s/copy-dst' % B, headers={'x-amz-copy-source': 'nobucketonly'}), exists('copy-dst')) +case(1, 'UploadPart partNumber=abc', (400, 'Error', 'InvalidArgument'), + S3DOC + 'API_UploadPart.html#AmazonS3-UploadPart-request-uri-querystring-PartNumber', + S3('PUT', '/%s/mp?partNumber=abc&uploadId=%s' % (B, upload_id), body=b'part-body'), exists('mp')) +case(1, 'PutObject, x-amz-content-sha256 != body', (400, 'Error', 'XAmzContentSHA256Mismatch'), + 'undocumented code; ' + S3DOC + 'API_UploadPart.html#API_UploadPart_RequestSyntax', + S3('PUT', '/%s/sha-mismatch' % B, body=b'abc', + headers={'X-Amz-Content-SHA256': hashlib.sha256(b'zzz').hexdigest()}, keep_sha256_header=True), + exists('sha-mismatch')) +case(1, 'PutObject, x-amz-content-sha256 not hex', (400, 'Error', None), 'undocumented', + S3('PUT', '/%s/sha-nothex' % B, body=b'abc', + headers={'X-Amz-Content-SHA256': 'nothex'}, keep_sha256_header=True), exists('sha-nothex')) + +# ---------------- 2. IAM failures in the S3 envelope ---------------- +case(2, 'IAM ListUsers, unknown access key', (403, 'ErrorResponse', 'InvalidClientTokenId'), + 'checked against iam.amazonaws.com (docs list UnrecognizedClientException); ' + IAMDOC + 'CommonErrors.html#CommonErrors-UnrecognizedClientException', iam({'Action': 'ListUsers'}, creds=BAD_KEY)) +case(2, 'IAM ListUsers, wrong secret', (403, 'ErrorResponse', None), 'undocumented for IAM', + iam({'Action': 'ListUsers'}, creds=BAD_SECRET)) +case(2, 'IAM ListUsers, non-admin user', (403, 'ErrorResponse', 'AccessDenied'), + IAMDOC + 'CommonErrors.html#CommonErrors-AccessDeniedException', iam({'Action': 'ListUsers'}, creds=USER)) +case(2, 'IAM CreateUser, non-admin user', (403, 'ErrorResponse', 'AccessDenied'), + IAMDOC + 'CommonErrors.html#CommonErrors-AccessDeniedException', iam({'Action': 'CreateUser', 'UserName': user + 'x'}, creds=USER)) + +# ---------------- 3. wrong status or code ---------------- +tags = [('Action', 'TagUser'), ('UserName', user)] + [('Tags.member.%d.%s' % (i, k), 'k%d' % i if k == 'Key' else 'v') + for i in range(1, 52) for k in ('Key', 'Value')] +case(3, 'IAM TagUser with 51 tags', (409, 'ErrorResponse', 'LimitExceeded'), IAMDOC + 'API_TagUser.html#API_TagUser_Errors', + iam(dict(tags))) +case(3, 'IAM unknown Action', (404, 'ErrorResponse', 'InvalidAction'), 'checked against iam.amazonaws.com', + iam({'Action': 'NoSuchActionZZ'})) + + +def no_date(h): + return {k: v for k, v in h.items() if k.lower() != 'x-amz-date'} + + +def drop(field): + return lambda h: dict(h, Authorization=re.sub(r',? *%s=[^,]*' % field, '', h['Authorization'])) + + +case(3, 'S3 request without x-amz-date', (403, 'Error', 'AccessDenied'), 'checked against s3.amazonaws.com; ' + S3DOC + 'API_Error.html#:~:text=Code%3A%20AccessDenied', + S3('GET', '/' + B, mutate=no_date)) +case(3, 'Authorization without Signature=', (400, 'Error', 'AuthorizationHeaderMalformed'), + 'checked against s3.amazonaws.com; ' + S3DOC + 'API_Error.html#:~:text=Code%3A%20AuthorizationHeaderMalformed', S3('GET', '/' + B, mutate=drop('Signature'))) +case(3, 'Authorization without Credential=', (400, 'Error', 'InvalidArgument'), + 'checked against s3.amazonaws.com ("Unsupported Authorization Type")', S3('GET', '/' + B, mutate=drop('Credential'))) +for m, exp, chk in (('GET', (400, 'Error', 'InvalidArgument'), 'checked against s3.amazonaws.com ("Invalid version id specified"); '), + ('HEAD', (400, None, None), 'checked against s3.amazonaws.com; '), + ('DELETE', (400, None, None), 'unverified (needs write access); ')): + case(3, '%s ?versionId=bogus, unversioned bucket' % m, exp, + chk + S3DOC + 'API_GetObject.html#AmazonS3-GetObject-request-uri-querystring-VersionId', + S3(m, '/%s/obj?versionId=bogus' % B)) +for pn in ('0', '10001'): + case(3, 'UploadPart partNumber=%s' % pn, (400, 'Error', 'InvalidArgument'), + S3DOC + 'API_UploadPart.html#AmazonS3-UploadPart-request-uri-querystring-PartNumber', + S3('PUT', '/%s/mp?partNumber=%s&uploadId=%s' % (B, pn, upload_id), body=b'x')) +case(3, 'DeleteObjects with 1001 keys', (400, 'Error', 'MalformedXML'), S3DOC + 'API_DeleteObjects.html#API_DeleteObjects_RequestBody', + S3('POST', '/%s?delete' % B, body=('' + ''.join('k%d' % i for i in range(1001)) + '').encode())) +# last: this one deletes the user when it should be refused +case(3, 'IAM DeleteUser while it has an access key', (409, 'ErrorResponse', 'DeleteConflict'), + IAMDOC + 'API_DeleteUser.html#API_DeleteUser_Errors', iam({'Action': 'DeleteUser', 'UserName': user})) + +# ---------------- report ---------------- +w = max(len(r[1]) for r in rows) +print('%-4s %-*s %-46s %-40s %s' % ('', w, 'case', 'got (status root code)', 'expected (AWS)', 'side effect')) +for g, name, got, exp, note, ok, ref in rows: + print('%-4s %-*s %-46s %-40s %s' % ('ok' if ok else 'DIFF', w, name, got, exp, note)) +print('\nreferences:') +for g, name, got, exp, note, ok, ref in rows: + print(' %-*s %s' % (w, name, ref)) +diff = sum(not r[5] for r in rows) +print('\n%d cases, %d differ from AWS' % (len(rows), diff)) +sys.exit(1 if diff else 0) diff --git a/test/s3/error_compat/test_s3.json b/test/s3/error_compat/test_s3.json new file mode 100644 index 000000000..1f9b46000 --- /dev/null +++ b/test/s3/error_compat/test_s3.json @@ -0,0 +1,20 @@ +{ + "identities": [ + { + "name": "admin", + "credentials": [ + { + "accessKey": "admin", + "secretKey": "admin" + } + ], + "actions": [ + "Admin", + "Read", + "List", + "Tagging", + "Write" + ] + } + ] +} diff --git a/weed/iamapi/iamapi_handlers.go b/weed/iamapi/iamapi_handlers.go index 695ce293d..54c15ef2a 100644 --- a/weed/iamapi/iamapi_handlers.go +++ b/weed/iamapi/iamapi_handlers.go @@ -31,6 +31,7 @@ func writeIamErrorResponse(w http.ResponseWriter, r *http.Request, reqID string, errorResp := newErrorResponse(errCode, errMsg, reqID) internalErrorResponse := newErrorResponse(iam.ErrCodeServiceFailureException, "Internal server error", reqID) + internalErrorResponse.Error.Type = "Receiver" switch errCode { case iam.ErrCodeNoSuchEntityException: diff --git a/weed/iamapi/iamapi_management_handlers.go b/weed/iamapi/iamapi_management_handlers.go index f81a8d76c..e6250d9fb 100644 --- a/weed/iamapi/iamapi_management_handlers.go +++ b/weed/iamapi/iamapi_management_handlers.go @@ -228,6 +228,19 @@ func (iama *IamApiServer) DeleteUser(s3cfg *iam_pb.S3ApiConfiguration, userName resp = &DeleteUserResponse{} for i, ident := range s3cfg.Identities { if userName == ident.Name { + // AWS IAM behavior: prevent deletion if user has dependent resources. + if len(ident.ServiceAccountIds) > 0 { + return resp, &IamError{ + Code: iam.ErrCodeDeleteConflictException, + Error: fmt.Errorf("cannot delete user %s: user has %d service account(s). Delete service accounts first", userName, len(ident.ServiceAccountIds)), + } + } + if len(ident.Credentials) > 0 { + return resp, &IamError{ + Code: iam.ErrCodeDeleteConflictException, + Error: fmt.Errorf("cannot delete user %s: user has %d access key(s). Delete access keys first", userName, len(ident.Credentials)), + } + } // Clean up any inline policies stored for this user policies := Policies{} if pErr := iama.s3ApiConfig.GetPolicies(&policies); pErr != nil && !errors.Is(pErr, filer_pb.ErrNotFound) { @@ -1618,9 +1631,8 @@ func (iama *IamApiServer) DoActions(w http.ResponseWriter, r *http.Request) { } changed = false default: - errNotImplemented := s3err.GetAPIError(s3err.ErrNotImplemented) - errorResponse := newErrorResponse(errNotImplemented.Code, errNotImplemented.Description, reqID) - s3err.WriteXMLResponse(w, r, errNotImplemented.HTTPStatusCode, errorResponse) + errorResponse := newErrorResponse("InvalidAction", fmt.Sprintf("The action %s is not valid for this endpoint", values.Get("Action")), reqID) + s3err.WriteXMLResponse(w, r, http.StatusNotFound, errorResponse) return } if changed { diff --git a/weed/iamapi/iamapi_test.go b/weed/iamapi/iamapi_test.go index 6268e448b..e44c0832d 100644 --- a/weed/iamapi/iamapi_test.go +++ b/weed/iamapi/iamapi_test.go @@ -488,6 +488,29 @@ func TestDeleteUser(t *testing.T) { out := DeleteUserResponse{} response, err := executeRequest(req.HTTPRequest, out) assert.Equal(t, nil, err) + assert.Equal(t, http.StatusConflict, response.Code) + + listReq, _ := iam.New(session.New()).ListAccessKeysRequest(&iam.ListAccessKeysInput{UserName: userName}) + _ = listReq.Build() + listOut := ListAccessKeysResponse{} + listResp, err := executeRequest(listReq.HTTPRequest, listOut) + assert.Equal(t, nil, err) + var listResult ListAccessKeysResponse + assert.Equal(t, nil, xml.Unmarshal(listResp.Body.Bytes(), &listResult)) + for _, key := range listResult.ListAccessKeysResult.AccessKeyMetadata { + delKeyReq, _ := iam.New(session.New()).DeleteAccessKeyRequest(&iam.DeleteAccessKeyInput{ + UserName: userName, + AccessKeyId: key.AccessKeyId, + }) + _ = delKeyReq.Build() + _, err := executeRequest(delKeyReq.HTTPRequest, out) + assert.Equal(t, nil, err) + } + + req, _ = iam.New(session.New()).DeleteUserRequest(params) + _ = req.Build() + response, err = executeRequest(req.HTTPRequest, out) + assert.Equal(t, nil, err) assert.Equal(t, http.StatusOK, response.Code) } diff --git a/weed/s3api/auth_signature_v4.go b/weed/s3api/auth_signature_v4.go index b2067d5a0..7dc76ada1 100644 --- a/weed/s3api/auth_signature_v4.go +++ b/weed/s3api/auth_signature_v4.go @@ -126,6 +126,27 @@ func normalizePayloadHash(payloadHashValue string) string { return payloadHashValue } +// requestSigningService extracts the service name from the SigV4 credential +// scope (…/region/service/aws4_request) so a failed request can be answered in +// that service's error envelope. +func requestSigningService(r *http.Request) string { + credential := r.URL.Query().Get("X-Amz-Credential") + if credential == "" { + v4Auth := strings.Replace(r.Header.Get("Authorization"), " ", "", -1) + v4Auth = strings.TrimPrefix(v4Auth, signV4Algorithm) + for _, field := range strings.Split(v4Auth, ",") { + if cred, ok := strings.CutPrefix(field, "Credential="); ok { + credential = cred + break + } + } + } + if parts := strings.Split(credential, "/"); len(parts) == 5 { + return parts[3] + } + return "" +} + // signValues data type represents structured form of AWS Signature V4 header. type signValues struct { Credential credentialHeader @@ -151,8 +172,20 @@ func parseSignV4(v4Auth string) (sv signValues, aec s3err.ErrorCode) { // Strip off the Algorithm prefix. v4Auth = strings.TrimPrefix(v4Auth, signV4Algorithm) authFields := strings.Split(strings.TrimSpace(v4Auth), ",") - if len(authFields) != 3 { - return sv, s3err.ErrMissingFields + malformed := len(authFields) != 3 + var hasCredential, hasSignature bool + for _, field := range authFields { + field = strings.TrimSpace(field) + malformed = malformed || field == "" + hasCredential = hasCredential || strings.HasPrefix(field, "Credential=") + hasSignature = hasSignature || strings.HasPrefix(field, "Signature=") + } + if malformed { + // AWS distinguishes which required field is absent. + if !hasCredential { + return sv, s3err.ErrInvalidArgument + } + return sv, s3err.ErrAuthorizationHeaderMalformed } // Initialize signature version '4' structured header. @@ -744,13 +777,13 @@ func parseCredentialHeader(credElement string) (ch credentialHeader, aec s3err.E func parseSignature(signElement string) (string, s3err.ErrorCode) { signFields := strings.Split(strings.TrimSpace(signElement), "=") if len(signFields) != 2 { - return "", s3err.ErrMissingFields + return "", s3err.ErrAuthorizationHeaderMalformed } if signFields[0] != "Signature" { return "", s3err.ErrMissingSignTag } if signFields[1] == "" { - return "", s3err.ErrMissingFields + return "", s3err.ErrAuthorizationHeaderMalformed } signature := signFields[1] return signature, s3err.ErrNone diff --git a/weed/s3api/auth_signature_v4_unsigned_headers_test.go b/weed/s3api/auth_signature_v4_unsigned_headers_test.go index a9ccdb72f..49bbe6564 100644 --- a/weed/s3api/auth_signature_v4_unsigned_headers_test.go +++ b/weed/s3api/auth_signature_v4_unsigned_headers_test.go @@ -360,3 +360,29 @@ func TestPresignedPutRejectsUnsignedMetadataHeaders(t *testing.T) { }) } } + +// TestParseSignV4MissingFields exercises how a truncated Authorization header +// maps to the AWS error code for the absent field. +func TestParseSignV4MissingFields(t *testing.T) { + valid := "AWS4-HMAC-SHA256 Credential=AKIA/20260101/us-east-1/s3/aws4_request, SignedHeaders=host;x-amz-date, Signature=0123456789abcdef" + + tests := []struct { + name string + auth string + want s3err.ErrorCode + }{ + {"valid", valid, s3err.ErrNone}, + {"no signature field", "AWS4-HMAC-SHA256 Credential=AKIA/20260101/us-east-1/s3/aws4_request, SignedHeaders=host;x-amz-date", s3err.ErrAuthorizationHeaderMalformed}, + {"empty signature field", "AWS4-HMAC-SHA256 Credential=AKIA/20260101/us-east-1/s3/aws4_request, SignedHeaders=host;x-amz-date, ", s3err.ErrAuthorizationHeaderMalformed}, + {"no credential field", "AWS4-HMAC-SHA256 SignedHeaders=host;x-amz-date, Signature=0123456789abcdef", s3err.ErrInvalidArgument}, + {"leading comma after dropping credential", "AWS4-HMAC-SHA256 , SignedHeaders=host;x-amz-date, Signature=0123456789abcdef", s3err.ErrInvalidArgument}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + _, got := parseSignV4(tt.auth) + if got != tt.want { + t.Fatalf("parseSignV4() = %v, want %v", got, tt.want) + } + }) + } +} diff --git a/weed/s3api/s3api_bucket_config.go b/weed/s3api/s3api_bucket_config.go index 9a3a3b02d..dc13958ec 100644 --- a/weed/s3api/s3api_bucket_config.go +++ b/weed/s3api/s3api_bucket_config.go @@ -646,6 +646,24 @@ func (s3a *S3ApiServer) isVersioningConfigured(bucket string) (bool, error) { return config.Versioning != "" || config.ObjectLockConfig != nil, nil } +// checkVersionIdConfigured reports ErrInvalidArgument when versionId names a +// specific version on a bucket that has never had versioning configured. The +// implicit "null" version of pre-versioning objects stays addressable. +func (s3a *S3ApiServer) checkVersionIdConfigured(bucket, versionId string) s3err.ErrorCode { + if versionId == "" || versionId == "null" { + return s3err.ErrNone + } + configured, err := s3a.isVersioningConfigured(bucket) + if err != nil { + // Missing buckets and lookup failures are answered by the handler path. + return s3err.ErrNone + } + if !configured { + return s3err.ErrInvalidArgument + } + return s3err.ErrNone +} + // isObjectLockEnabled checks if Object Lock is enabled for a bucket (with caching) func (s3a *S3ApiServer) isObjectLockEnabled(bucket string) (bool, error) { config, errCode := s3a.getBucketConfig(bucket) diff --git a/weed/s3api/s3api_bucket_handlers.go b/weed/s3api/s3api_bucket_handlers.go index f1197c60e..a2ec2dfdb 100644 --- a/weed/s3api/s3api_bucket_handlers.go +++ b/weed/s3api/s3api_bucket_handlers.go @@ -224,6 +224,15 @@ func (s3a *S3ApiServer) PutBucketHandler(w http.ResponseWriter, r *http.Request) // collect parameters bucket, _ := s3_constants.GetBucketAndObject(r) + // Only bucket-level PUTs with a registered sub-resource route land in a + // dedicated handler; anything else that reaches here is an operation we do + // not implement, not a bare CreateBucket. + if subresource, found := unroutedSubresource(r, nil); found { + glog.V(1).Infof("unimplemented bucket PUT subresource ?%s", subresource) + s3err.WriteErrorResponse(w, r, s3err.ErrNotImplemented) + return + } + // validate the bucket name err := s3bucket.VerifyS3BucketName(bucket) if err != nil { @@ -424,6 +433,14 @@ func (s3a *S3ApiServer) DeleteBucketHandler(w http.ResponseWriter, r *http.Reque bucket, _ := s3_constants.GetBucketAndObject(r) glog.V(3).Infof("DeleteBucketHandler %s", bucket) + + // Same as PutBucketHandler: a query key no DELETE route claimed is an + // unimplemented subresource, not a bare DeleteBucket. + if subresource, found := unroutedSubresource(r, nil); found { + glog.V(1).Infof("unimplemented bucket DELETE subresource ?%s", subresource) + s3err.WriteErrorResponse(w, r, s3err.ErrNotImplemented) + return + } // The teardown below retries, and a failover walk repeats it once per // filer, so the backoff comes out of one allowance held here. r = r.WithContext(withFilerRetryBudget(r.Context(), filerRetryRequestBudget)) diff --git a/weed/s3api/s3api_bucket_handlers_misc.go b/weed/s3api/s3api_bucket_handlers_misc.go index e60852764..798257a8e 100644 --- a/weed/s3api/s3api_bucket_handlers_misc.go +++ b/weed/s3api/s3api_bucket_handlers_misc.go @@ -185,11 +185,20 @@ var listObjectsQueryParams = map[string]bool{ // catch-all, so anything left is a subresource it does not implement - and answering // it with a bucket listing is worse than saying so. func unroutedBucketSubresource(r *http.Request) (string, bool) { + return unroutedSubresource(r, listObjectsQueryParams) +} + +// unroutedSubresource names a query key that no route claimed, outside the +// request's own parameters, SigV2 presign keys, and SigV4 presign keys +// (all "X-Amz-" prefixed). +func unroutedSubresource(r *http.Request, allowed map[string]bool) (string, bool) { for key := range r.URL.Query() { - if listObjectsQueryParams[key] || strings.HasPrefix(key, "X-Amz-") { - continue + switch { + case allowed[key], strings.HasPrefix(key, "X-Amz-"): + case key == "x-id", key == "AWSAccessKeyId", key == "Signature", key == "Expires": + default: + return key, true } - return key, true } return "", false } diff --git a/weed/s3api/s3api_embedded_iam.go b/weed/s3api/s3api_embedded_iam.go index 055754384..685d575bc 100644 --- a/weed/s3api/s3api_embedded_iam.go +++ b/weed/s3api/s3api_embedded_iam.go @@ -196,10 +196,32 @@ func newIamErrorResponse(errCode string, errMsg string, requestID string) iamErr return errorResp } -func (e *EmbeddedIamApi) writeIamErrorResponse(w http.ResponseWriter, r *http.Request, reqID string, iamErr *iamError) { +// iamAuthErrorCode maps an S3-side signature failure onto the IAM +// query-protocol code IAM clients expect for the same request. +func iamAuthErrorCode(errCode s3err.ErrorCode) string { + switch errCode { + case s3err.ErrInvalidAccessKeyID: + return "InvalidClientTokenId" + case s3err.ErrSignatureDoesNotMatch: + return "SignatureDoesNotMatch" + case s3err.ErrAccessDenied: + return "AccessDenied" + case s3err.ErrExpiredToken: + return "ExpiredToken" + case s3err.ErrRequestTimeTooSkewed: + return "RequestTimeTooSkewed" + case s3err.ErrInternalError: + return iam.ErrCodeServiceFailureException + default: + return "IncompleteSignature" + } +} + +func writeIamErrorResponse(w http.ResponseWriter, r *http.Request, reqID string, iamErr *iamError) { if iamErr == nil { glog.Errorf("writeIamErrorResponse called with nil error") internalResp := newIamErrorResponse(iam.ErrCodeServiceFailureException, "Internal server error", reqID) + internalResp.Error.Type = "Receiver" s3err.WriteXMLResponse(w, r, http.StatusInternalServerError, internalResp) return } @@ -210,20 +232,25 @@ func (e *EmbeddedIamApi) writeIamErrorResponse(w http.ResponseWriter, r *http.Re errorResp := newIamErrorResponse(errCode, errMsg, reqID) internalErrorResponse := newIamErrorResponse(iam.ErrCodeServiceFailureException, "Internal server error", reqID) + internalErrorResponse.Error.Type = "Receiver" switch errCode { case iam.ErrCodeNoSuchEntityException: s3err.WriteXMLResponse(w, r, http.StatusNotFound, errorResp) case iam.ErrCodeEntityAlreadyExistsException: s3err.WriteXMLResponse(w, r, http.StatusConflict, errorResp) - case iam.ErrCodeMalformedPolicyDocumentException, iam.ErrCodeInvalidInputException, "ValidationError": + case iam.ErrCodeMalformedPolicyDocumentException, iam.ErrCodeInvalidInputException, "ValidationError", "IncompleteSignature", "ExpiredToken": s3err.WriteXMLResponse(w, r, http.StatusBadRequest, errorResp) - case "AccessDenied", iam.ErrCodeLimitExceededException: + case "AccessDenied", "InvalidClientTokenId", "SignatureDoesNotMatch", "RequestTimeTooSkewed": s3err.WriteXMLResponse(w, r, http.StatusForbidden, errorResp) + case iam.ErrCodeLimitExceededException: + s3err.WriteXMLResponse(w, r, http.StatusConflict, errorResp) case iam.ErrCodeServiceFailureException: s3err.WriteXMLResponse(w, r, http.StatusInternalServerError, internalErrorResponse) case "NotImplemented": s3err.WriteXMLResponse(w, r, http.StatusNotImplemented, errorResp) + case "InvalidAction": + s3err.WriteXMLResponse(w, r, http.StatusNotFound, errorResp) case iam.ErrCodeDeleteConflictException: s3err.WriteXMLResponse(w, r, http.StatusConflict, errorResp) case iam.ErrCodeUnmodifiableEntityException: @@ -328,8 +355,8 @@ func (e *EmbeddedIamApi) DeleteUser(s3cfg *iam_pb.S3ApiConfiguration, userName s resp := &iamDeleteUserResponse{} for i, ident := range s3cfg.Identities { if userName == ident.Name { - // AWS IAM behavior: prevent deletion if user has service accounts - // This ensures explicit cleanup and prevents orphaned resources + // AWS IAM behavior: prevent deletion if user has dependent resources. + // This ensures explicit cleanup and prevents orphaned resources. if len(ident.ServiceAccountIds) > 0 { return resp, &iamError{ Code: iam.ErrCodeDeleteConflictException, @@ -337,6 +364,13 @@ func (e *EmbeddedIamApi) DeleteUser(s3cfg *iam_pb.S3ApiConfiguration, userName s userName, len(ident.ServiceAccountIds)), } } + if len(ident.Credentials) > 0 { + return resp, &iamError{ + Code: iam.ErrCodeDeleteConflictException, + Error: fmt.Errorf("cannot delete user %s: user has %d access key(s). Delete access keys first", + userName, len(ident.Credentials)), + } + } s3cfg.Identities = append(s3cfg.Identities[:i], s3cfg.Identities[i+1:]...) // Remove user from all groups for _, g := range s3cfg.Groups { @@ -2562,15 +2596,19 @@ func (iam *IdentityAccessManagement) AuthIamManagement(f http.HandlerFunc) http. // needs to hash the body for IAM requests (service != "s3"). // The streamHashRequestBody function in auth_signature_v4.go preserves the body // after reading it, so ParseForm() will work correctly after authentication. + r, reqID := request_id.Ensure(r) identity, errCode := iam.AuthSignatureOnly(r) if errCode != s3err.ErrNone { - s3err.WriteErrorResponse(w, r, errCode) + writeIamErrorResponse(w, r, reqID, &iamError{ + Code: iamAuthErrorCode(errCode), + Error: errors.New(s3err.GetAPIError(errCode).Description), + }) return } // Now parse form to get Action and UserName (body was preserved by auth) if err := r.ParseForm(); err != nil { - s3err.WriteErrorResponse(w, r, s3err.ErrInvalidRequest) + writeIamErrorResponse(w, r, reqID, &iamError{Code: "InvalidInput", Error: err}) return } @@ -2578,7 +2616,10 @@ func (iam *IdentityAccessManagement) AuthIamManagement(f http.HandlerFunc) http. // from, so the authorized target and the acted-on target cannot differ. action := r.Form.Get("Action") if errCode := iam.AuthorizeIamAction(r, identity, action, iamTargetUserName(action, r)); errCode != s3err.ErrNone { - s3err.WriteErrorResponse(w, r, errCode) + writeIamErrorResponse(w, r, reqID, &iamError{ + Code: iamAuthErrorCode(errCode), + Error: errors.New(s3err.GetAPIError(errCode).Description), + }) return } @@ -2974,7 +3015,7 @@ func (e *EmbeddedIamApi) ExecuteAction(ctx context.Context, values url.Values, s } changed = false default: - return nil, &iamError{Code: s3err.GetAPIError(s3err.ErrNotImplemented).Code, Error: errors.New(s3err.GetAPIError(s3err.ErrNotImplemented).Description)} + return nil, &iamError{Code: "InvalidAction", Error: fmt.Errorf("The action %s is not valid for this endpoint", values.Get("Action"))} } if changed { if !skipPersist { @@ -3003,7 +3044,7 @@ func (e *EmbeddedIamApi) ExecuteAction(ctx context.Context, values url.Values, s func (e *EmbeddedIamApi) DoActions(w http.ResponseWriter, r *http.Request) { r, reqID := request_id.Ensure(r) if err := r.ParseForm(); err != nil { - s3err.WriteErrorResponse(w, r, s3err.ErrInvalidRequest) + writeIamErrorResponse(w, r, reqID, &iamError{Code: iam.ErrCodeInvalidInputException, Error: err}) return } values := r.PostForm @@ -3019,7 +3060,7 @@ func (e *EmbeddedIamApi) DoActions(w http.ResponseWriter, r *http.Request) { response, iamErr := e.ExecuteAction(r.Context(), values, false, reqID) if iamErr != nil { - e.writeIamErrorResponse(w, r, reqID, iamErr) + writeIamErrorResponse(w, r, reqID, iamErr) return } diff --git a/weed/s3api/s3api_embedded_iam_test.go b/weed/s3api/s3api_embedded_iam_test.go index 4e82baaa3..5500a0e0b 100644 --- a/weed/s3api/s3api_embedded_iam_test.go +++ b/weed/s3api/s3api_embedded_iam_test.go @@ -1043,7 +1043,7 @@ func TestEmbeddedIamAttachPolicyLimitExceeded(t *testing.T) { response, err := executeEmbeddedIamRequest(api, req.HTTPRequest, nil) assert.NoError(t, err) - assert.Equal(t, http.StatusForbidden, response.Code) + assert.Equal(t, http.StatusConflict, response.Code) code, _ := extractEmbeddedIamErrorCodeAndMessage(response) assert.Equal(t, iam.ErrCodeLimitExceededException, code) assert.Len(t, api.mockConfig.Identities[0].PolicyNames, MaxManagedPoliciesPerUser) @@ -1692,7 +1692,34 @@ func TestEmbeddedIamFullWorkflow(t *testing.T) { assert.Equal(t, http.StatusOK, response.Code) }) - // 5. Delete user + var accessKeyId string + for _, ident := range api.mockConfig.Identities { + if ident.Name == "WorkflowUser" && len(ident.Credentials) > 0 { + accessKeyId = ident.Credentials[0].AccessKey + } + } + assert.NotEmpty(t, accessKeyId) + + // 5. Deleting a user that still has an access key is refused + t.Run("DeleteUserConflict", func(t *testing.T) { + params := &iam.DeleteUserInput{UserName: aws.String("WorkflowUser")} + req, _ := iam.New(session.New()).DeleteUserRequest(params) + _ = req.Build() + response, err := executeEmbeddedIamRequest(api, req.HTTPRequest, nil) + assert.NoError(t, err) + assert.Equal(t, http.StatusConflict, response.Code) + }) + + // 6. Delete the access key, then the user + t.Run("DeleteAccessKey", func(t *testing.T) { + params := &iam.DeleteAccessKeyInput{UserName: aws.String("WorkflowUser"), AccessKeyId: aws.String(accessKeyId)} + req, _ := iam.New(session.New()).DeleteAccessKeyRequest(params) + _ = req.Build() + response, err := executeEmbeddedIamRequest(api, req.HTTPRequest, nil) + assert.NoError(t, err) + assert.Equal(t, http.StatusOK, response.Code) + }) + t.Run("DeleteUser", func(t *testing.T) { params := &iam.DeleteUserInput{UserName: aws.String("WorkflowUser")} req, _ := iam.New(session.New()).DeleteUserRequest(params) @@ -2039,7 +2066,8 @@ func TestEmbeddedIamNotImplementedAction(t *testing.T) { apiRouter.Path("/").Methods(http.MethodPost).HandlerFunc(api.DoActions) apiRouter.ServeHTTP(rr, req) - assert.Equal(t, http.StatusNotImplemented, rr.Code) + assert.Equal(t, http.StatusNotFound, rr.Code) + assert.Contains(t, rr.Body.String(), "InvalidAction") assert.Contains(t, rr.Body.String(), "") assert.NotContains(t, rr.Body.String(), "") assert.Equal(t, rr.Header().Get(request_id.AmzRequestIDHeader), extractEmbeddedIamRequestID(rr)) @@ -2831,3 +2859,43 @@ func TestEmbeddedIamReadOnly(t *testing.T) { assert.NoError(t, err) assert.Equal(t, http.StatusOK, responseList.Code) } + +func TestWriteIamErrorResponseMapping(t *testing.T) { + tests := []struct { + name string + code string + wantStatus int + }{ + {"NoSuchEntity", iam.ErrCodeNoSuchEntityException, http.StatusNotFound}, + {"EntityAlreadyExists", iam.ErrCodeEntityAlreadyExistsException, http.StatusConflict}, + {"InvalidInput", iam.ErrCodeInvalidInputException, http.StatusBadRequest}, + {"LimitExceeded", iam.ErrCodeLimitExceededException, http.StatusConflict}, + {"DeleteConflict", iam.ErrCodeDeleteConflictException, http.StatusConflict}, + {"InvalidAction", "InvalidAction", http.StatusNotFound}, + {"AccessDenied", "AccessDenied", http.StatusForbidden}, + {"InvalidClientTokenId", "InvalidClientTokenId", http.StatusForbidden}, + {"SignatureDoesNotMatch", "SignatureDoesNotMatch", http.StatusForbidden}, + {"IncompleteSignature", "IncompleteSignature", http.StatusBadRequest}, + {"ServiceFailure", iam.ErrCodeServiceFailureException, http.StatusInternalServerError}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + rec := httptest.NewRecorder() + r := httptest.NewRequest(http.MethodPost, "/", nil) + writeIamErrorResponse(rec, r, "req-1", &iamError{Code: tt.code, Error: fmt.Errorf("boom")}) + assert.Equal(t, tt.wantStatus, rec.Code) + body := rec.Body.String() + assert.True(t, strings.HasPrefix(strings.TrimSpace(body), ""+tt.code+"") + }) + } +} + +func TestIamAuthErrorCode(t *testing.T) { + assert.Equal(t, "InvalidClientTokenId", iamAuthErrorCode(s3err.ErrInvalidAccessKeyID)) + assert.Equal(t, "SignatureDoesNotMatch", iamAuthErrorCode(s3err.ErrSignatureDoesNotMatch)) + assert.Equal(t, "AccessDenied", iamAuthErrorCode(s3err.ErrAccessDenied)) + assert.Equal(t, "RequestTimeTooSkewed", iamAuthErrorCode(s3err.ErrRequestTimeTooSkewed)) + assert.Equal(t, "IncompleteSignature", iamAuthErrorCode(s3err.ErrMissingFields)) +} diff --git a/weed/s3api/s3api_embedded_iam_user_tags_test.go b/weed/s3api/s3api_embedded_iam_user_tags_test.go index 07c2b4e25..ba9a5f389 100644 --- a/weed/s3api/s3api_embedded_iam_user_tags_test.go +++ b/weed/s3api/s3api_embedded_iam_user_tags_test.go @@ -200,7 +200,7 @@ func TestEmbeddedIamTagUserLimitExceeded(t *testing.T) { form.Set("Tags.member.1.Value", "v") rr := postTagAction(t, api, form) - assert.Equal(t, http.StatusForbidden, rr.Code) + assert.Equal(t, http.StatusConflict, rr.Code) code, _ := extractEmbeddedIamErrorCodeAndMessage(rr) assert.Equal(t, iam.ErrCodeLimitExceededException, code) diff --git a/weed/s3api/s3api_iam_oidc_test.go b/weed/s3api/s3api_iam_oidc_test.go index 70807ea80..0889885ce 100644 --- a/weed/s3api/s3api_iam_oidc_test.go +++ b/weed/s3api/s3api_iam_oidc_test.go @@ -402,9 +402,8 @@ func TestUpdateThumbprintAndTags(t *testing.T) { // A refusal to change a config-file provider reaches the client as AWS sends // it (400 UnmodifiableEntity), not as an internal error clients retry. func TestUnmodifiableEntityIsAClientError(t *testing.T) { - api := NewEmbeddedIamApiForTest() rec := httptest.NewRecorder() - api.writeIamErrorResponse(rec, httptest.NewRequest(http.MethodPost, "/", nil), "req-1", + writeIamErrorResponse(rec, httptest.NewRequest(http.MethodPost, "/", nil), "req-1", oidcMutationError(fmt.Errorf("%w: arn:aws:iam:::oidc-provider/static.example", integration.ErrOIDCProviderStatic))) if rec.Code != http.StatusBadRequest { t.Fatalf("status = %d, want 400", rec.Code) diff --git a/weed/s3api/s3api_object_handlers.go b/weed/s3api/s3api_object_handlers.go index 5ea729fbc..7b45d186c 100644 --- a/weed/s3api/s3api_object_handlers.go +++ b/weed/s3api/s3api_object_handlers.go @@ -680,6 +680,15 @@ func (s3a *S3ApiServer) GetObjectHandler(w http.ResponseWriter, r *http.Request) bucket, object, totalTime, conditionalHeadersTime, versioningCheckTime, entryFetchTime, streamTime) }() + // A non-null versionId only has meaning once versioning has been + // configured; reject it before directory or conditional handling can + // answer the request as a plain object. + versionId := r.URL.Query().Get("versionId") + if errCode := s3a.checkVersionIdConfigured(bucket, versionId); errCode != s3err.ErrNone { + s3err.WriteErrorResponse(w, r, errCode) + return + } + // Handle directory objects with shared logic if s3a.handleDirectoryObjectRequest(w, r, bucket, object, "GetObjectHandler") { return // Directory object request was handled @@ -693,9 +702,6 @@ func (s3a *S3ApiServer) GetObjectHandler(w http.ResponseWriter, r *http.Request) return } - // Check for specific version ID in query parameters - versionId := r.URL.Query().Get("versionId") - var ( entry *filer_pb.Entry // Declare entry at function scope for SSE processing targetVersionId string @@ -2359,6 +2365,14 @@ func (s3a *S3ApiServer) HeadObjectHandler(w http.ResponseWriter, r *http.Request return // SOSAPI request was handled } + // Reject named versions before directory or conditional handling can + // answer the request as a plain object. + versionId := r.URL.Query().Get("versionId") + if errCode := s3a.checkVersionIdConfigured(bucket, versionId); errCode != s3err.ErrNone { + s3err.WriteErrorResponse(w, r, errCode) + return + } + // Handle directory objects with shared logic if s3a.handleDirectoryObjectRequest(w, r, bucket, object, "HeadObjectHandler") { return // Directory object request was handled @@ -2370,9 +2384,6 @@ func (s3a *S3ApiServer) HeadObjectHandler(w http.ResponseWriter, r *http.Request return } - // Check for specific version ID in query parameters - versionId := r.URL.Query().Get("versionId") - var ( entry *filer_pb.Entry // Declare entry at function scope for SSE processing targetVersionId string diff --git a/weed/s3api/s3api_object_handlers_delete.go b/weed/s3api/s3api_object_handlers_delete.go index a21a73fe2..dec8e3ee4 100644 --- a/weed/s3api/s3api_object_handlers_delete.go +++ b/weed/s3api/s3api_object_handlers_delete.go @@ -232,6 +232,11 @@ func (s3a *S3ApiServer) DeleteObjectHandler(w http.ResponseWriter, r *http.Reque versioningConfigured := (versioningState != "") + if errCode := s3a.checkVersionIdConfigured(bucket, versionId); errCode != s3err.ErrNone { + s3err.WriteErrorResponse(w, r, errCode) + return + } + var auditLog *s3err.AccessLog if s3err.Logger != nil { auditLog = s3err.GetAccessLog(r, http.StatusNoContent, s3err.ErrNone) @@ -421,7 +426,7 @@ func (s3a *S3ApiServer) DeleteMultipleObjectsHandler(w http.ResponseWriter, r *h } if len(deleteObjects.Objects) > deleteMultipleObjectsLimit { - s3err.WriteErrorResponse(w, r, s3err.ErrInvalidMaxDeleteObjects) + s3err.WriteErrorResponse(w, r, s3err.ErrMalformedXML) return } diff --git a/weed/s3api/s3api_object_handlers_multipart.go b/weed/s3api/s3api_object_handlers_multipart.go index 65b3cc90f..4ed8c2f5c 100644 --- a/weed/s3api/s3api_object_handlers_multipart.go +++ b/weed/s3api/s3api_object_handlers_multipart.go @@ -328,6 +328,12 @@ func (s3a *S3ApiServer) ListObjectPartsHandler(w http.ResponseWriter, r *http.Re // PutObjectPartHandler - Put an object part in a multipart upload. func (s3a *S3ApiServer) PutObjectPartHandler(w http.ResponseWriter, r *http.Request) { bucket, object := s3_constants.GetBucketAndObject(r) + if src := r.Header.Get("X-Amz-Copy-Source"); src != "" { + // Well-formed copy sources are claimed by the CopyObjectPart route; what + // is left here has no "bucket/key" form and is not a plain PutObjectPart. + s3err.WriteErrorResponse(w, r, s3err.ErrInvalidArgument) + return + } _, err := validateContentMd5(r.Header) if err != nil { s3err.WriteErrorResponse(w, r, s3err.ErrInvalidDigest) @@ -350,16 +356,8 @@ func (s3a *S3ApiServer) PutObjectPartHandler(w http.ResponseWriter, r *http.Requ partIDString := r.URL.Query().Get("partNumber") partID, err := strconv.Atoi(partIDString) - if err != nil { - s3err.WriteErrorResponse(w, r, s3err.ErrInvalidPart) - return - } - if partID > s3_constants.MaxS3MultipartParts { - s3err.WriteErrorResponse(w, r, s3err.ErrInvalidPart) - return - } - if partID < 1 { - s3err.WriteErrorResponse(w, r, s3err.ErrInvalidPart) + if err != nil || partID < 1 || partID > s3_constants.MaxS3MultipartParts { + s3err.WriteErrorResponse(w, r, s3err.ErrInvalidArgument) return } diff --git a/weed/s3api/s3api_object_handlers_put.go b/weed/s3api/s3api_object_handlers_put.go index deb968074..c2ca1a8e1 100644 --- a/weed/s3api/s3api_object_handlers_put.go +++ b/weed/s3api/s3api_object_handlers_put.go @@ -86,6 +86,20 @@ func (s3a *S3ApiServer) PutObjectHandler(w http.ResponseWriter, r *http.Request) // http://docs.aws.amazon.com/AmazonS3/latest/dev/UploadingObjects.html bucket, object := s3_constants.GetBucketAndObject(r) + + // A copy-source header that did not satisfy the CopyObject route is + // malformed (it needs a '/' separating source bucket from key), and a + // partNumber/uploadId pair that did not satisfy the UploadPart route is + // malformed too; neither should be silently written as a plain object. + if r.Header.Get("X-Amz-Copy-Source") != "" { + s3err.WriteErrorResponse(w, r, s3err.ErrInvalidCopySource) + return + } + if r.URL.Query().Has("partNumber") || r.URL.Query().Has("uploadId") { + s3err.WriteErrorResponse(w, r, s3err.ErrInvalidArgument) + return + } + _, err := validateContentMd5(r.Header) if err != nil { s3err.WriteErrorResponse(w, r, s3err.ErrInvalidDigest) @@ -188,7 +202,11 @@ func (s3a *S3ApiServer) PutObjectHandler(w http.ResponseWriter, r *http.Request) dirContent, readErr = io.ReadAll(dataReader) if readErr != nil { glog.Errorf("PutObjectHandler: failed to read directory marker content %s/%s: %v", bucket, object, readErr) - s3err.WriteErrorResponse(w, r, s3err.ErrInternalError) + if strings.Contains(readErr.Error(), s3err.ErrMsgContentSha256Mismatch) { + s3err.WriteErrorResponse(w, r, s3err.ErrContentSHA256Mismatch) + } else { + s3err.WriteErrorResponse(w, r, s3err.ErrInternalError) + } return } } @@ -1522,6 +1540,8 @@ func mapChunkedUploadErrorToS3Error(reqCtx context.Context, err error) s3err.Err switch { case errors.Is(err, weed_server.ErrReadOnly): return s3err.ErrAccessDenied + case strings.Contains(err.Error(), s3err.ErrMsgContentSha256Mismatch): + return s3err.ErrContentSHA256Mismatch case strings.Contains(err.Error(), s3err.ErrMsgPayloadChecksumMismatch): return s3err.ErrInvalidDigest case errors.Is(err, operation.ErrTruncatedBody): diff --git a/weed/s3api/s3api_put_object_helper.go b/weed/s3api/s3api_put_object_helper.go index d020772e9..12a861ddf 100644 --- a/weed/s3api/s3api_put_object_helper.go +++ b/weed/s3api/s3api_put_object_helper.go @@ -1,6 +1,12 @@ package s3api import ( + "bytes" + "crypto/sha256" + "encoding/base64" + "encoding/hex" + "errors" + "hash" "io" "net/http" @@ -30,6 +36,76 @@ func getRequestDataReader(s3a *S3ApiServer, r *http.Request) (io.ReadCloser, s3e dataReader, s3ErrCode = s3a.iam.newChunkedReader(r) } } + if s3ErrCode != s3err.ErrNone { + return nil, s3ErrCode + } - return dataReader, s3ErrCode + expected, valid := expectedContentSha256(r) + if !valid { + return nil, s3err.ErrInvalidArgument + } + if expected != nil && dataReader != nil { + if r.ContentLength == 0 { + // Handlers may never read an empty body, so check it now. + emptySum := sha256.Sum256(nil) + if !bytes.Equal(expected, emptySum[:]) { + return nil, s3err.ErrContentSHA256Mismatch + } + return dataReader, s3err.ErrNone + } + dataReader = &contentSha256Verifier{reader: dataReader, hasher: sha256.New(), expected: expected} + } + return dataReader, s3err.ErrNone +} + +// expectedContentSha256 decodes the x-amz-content-sha256 header when it declares +// a payload hash — hex or base64 per SigV4 — so the body can be checked as it +// streams. Sentinel values (streaming, unsigned) carry no hash and are skipped; +// anything that decodes to neither returns valid=false. +func expectedContentSha256(r *http.Request) (expected []byte, valid bool) { + v := r.Header.Get("X-Amz-Content-Sha256") + if v == "" || v == unsignedPayload || v == streamingContentSHA256 || + v == streamingContentSHA256Trailer || v == streamingUnsignedPayload { + return nil, true + } + if decoded, err := hex.DecodeString(v); err == nil && len(decoded) == sha256.Size { + return decoded, true + } + if decoded, err := base64.StdEncoding.DecodeString(v); err == nil && len(decoded) == sha256.Size { + return decoded, true + } + return nil, false +} + +// contentSha256Verifier checks the decoded x-amz-content-sha256 once the stream +// is exhausted; handlers that never consume the body are unaffected. +type contentSha256Verifier struct { + reader io.ReadCloser + hasher hash.Hash + expected []byte + done bool + pending error +} + +func (v *contentSha256Verifier) Read(p []byte) (int, error) { + if v.pending != nil { + return 0, v.pending + } + n, err := v.reader.Read(p) + v.hasher.Write(p[:n]) + if err == io.EOF && !v.done { + v.done = true + if !bytes.Equal(v.hasher.Sum(nil), v.expected) { + v.pending = errors.New(s3err.ErrMsgContentSha256Mismatch) + err = nil + } + } + if n == 0 && v.pending != nil { + return 0, v.pending + } + return n, err +} + +func (v *contentSha256Verifier) Close() error { + return v.reader.Close() } diff --git a/weed/s3api/s3api_put_object_helper_test.go b/weed/s3api/s3api_put_object_helper_test.go index 8e6565783..9328bb3db 100644 --- a/weed/s3api/s3api_put_object_helper_test.go +++ b/weed/s3api/s3api_put_object_helper_test.go @@ -1,7 +1,13 @@ package s3api import ( + "bytes" + "crypto/sha256" + "encoding/base64" + "encoding/hex" + "io" "net/http" + "net/http/httptest" "strings" "testing" @@ -183,3 +189,126 @@ func TestAuthTypeDetection(t *testing.T) { }) } } + +func sha256Hex(t *testing.T, b []byte) string { + t.Helper() + sum := sha256.Sum256(b) + return hex.EncodeToString(sum[:]) +} + +func TestExpectedContentSha256(t *testing.T) { + good := sha256Hex(t, []byte("payload")) + + tests := []struct { + name string + header string + wantValid bool + wantNil bool + }{ + {"absent", "", true, true}, + {"unsigned", unsignedPayload, true, true}, + {"streaming signed", streamingContentSHA256, true, true}, + {"streaming trailer", streamingContentSHA256Trailer, true, true}, + {"streaming unsigned", streamingUnsignedPayload, true, true}, + {"hex", good, true, false}, + {"not hex", "nothex", false, true}, + {"short hex", "deadbeef", false, true}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + r := httptest.NewRequest("PUT", "/b/o", nil) + if tt.header != "" { + r.Header.Set("X-Amz-Content-Sha256", tt.header) + } + got, valid := expectedContentSha256(r) + if valid != tt.wantValid { + t.Fatalf("valid=%v want %v", valid, tt.wantValid) + } + if (got == nil) != tt.wantNil { + t.Fatalf("expected nil=%v, got %x", tt.wantNil, got) + } + }) + } + + t.Run("base64 decodes to sha256", func(t *testing.T) { + sum := sha256.Sum256([]byte("payload")) + r := httptest.NewRequest("PUT", "/b/o", nil) + r.Header.Set("X-Amz-Content-Sha256", base64.StdEncoding.EncodeToString(sum[:])) + got, valid := expectedContentSha256(r) + if !valid || !bytes.Equal(got, sum[:]) { + t.Fatalf("valid=%v got=%x", valid, got) + } + }) +} + +func newVerifier(body string, expectedHex string) *contentSha256Verifier { + expected, _ := hex.DecodeString(expectedHex) + return &contentSha256Verifier{ + reader: io.NopCloser(strings.NewReader(body)), + hasher: sha256.New(), + expected: expected, + } +} + +func TestContentSha256VerifierMatch(t *testing.T) { + v := newVerifier("hello", sha256Hex(t, []byte("hello"))) + got, err := io.ReadAll(v) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if string(got) != "hello" { + t.Fatalf("got %q", got) + } +} + +func TestContentSha256VerifierMismatch(t *testing.T) { + v := newVerifier("hello", sha256Hex(t, []byte("other"))) + _, err := io.ReadAll(v) + if err == nil || !strings.Contains(err.Error(), "does not match") { + t.Fatalf("expected mismatch error, got %v", err) + } + if _, err := v.Read(make([]byte, 8)); err == nil { + t.Fatal("expected error to persist") + } +} + +// mimeDetect performs a single Read and ignores its error; the verifier must +// defer the mismatch so a dropped (n>0, err) result still surfaces later. +func TestContentSha256VerifierSurvivesDroppedError(t *testing.T) { + v := newVerifier("hello", sha256Hex(t, []byte("other"))) + buf := make([]byte, 512) + n, _ := v.Read(buf) + rest := io.MultiReader(bytes.NewReader(buf[:n]), v) + _, err := io.ReadAll(rest) + if err == nil || !strings.Contains(err.Error(), "does not match") { + t.Fatalf("expected deferred mismatch error, got %v", err) + } +} + +// A zero-length body never reaches the verifier, so the declared hash is +// compared against the empty-payload digest up front. +func TestGetRequestDataReaderEmptyBodyHash(t *testing.T) { + s3a := &S3ApiServer{ + iam: NewIdentityAccessManagementWithStore(&S3ApiServerOption{}, nil, string(credential.StoreTypeMemory)), + } + s3a.iam.isAuthEnabled = false + + mismatch := httptest.NewRequest("PUT", "/b/dir/", http.NoBody) + mismatch.Header.Set("X-Amz-Content-Sha256", sha256Hex(t, []byte("other"))) + if _, code := getRequestDataReader(s3a, mismatch); code != s3err.ErrContentSHA256Mismatch { + t.Fatalf("empty body with wrong hash: code=%v", code) + } + + malformed := httptest.NewRequest("PUT", "/b/o", strings.NewReader("x")) + malformed.Header.Set("X-Amz-Content-Sha256", "nothex") + if _, code := getRequestDataReader(s3a, malformed); code != s3err.ErrInvalidArgument { + t.Fatalf("malformed sha256 header: code=%v", code) + } + + emptySum := sha256.Sum256(nil) + match := httptest.NewRequest("PUT", "/b/dir/", http.NoBody) + match.Header.Set("X-Amz-Content-Sha256", hex.EncodeToString(emptySum[:])) + if _, code := getRequestDataReader(s3a, match); code != s3err.ErrNone { + t.Fatalf("empty body with empty hash: code=%v", code) + } +} diff --git a/weed/s3api/s3api_server.go b/weed/s3api/s3api_server.go index b5c9305bc..920d8fb47 100644 --- a/weed/s3api/s3api_server.go +++ b/weed/s3api/s3api_server.go @@ -4,6 +4,7 @@ import ( "bytes" "context" "encoding/json" + "errors" "fmt" "io" "math" @@ -730,10 +731,21 @@ func (s3a *S3ApiServer) handleCORSOriginValidation(w http.ResponseWriter, r *htt // UnifiedPostHandler handles authenticated POST requests to the root path // It inspects the Action parameter to dispatch to either STS or IAM handlers func (s3a *S3ApiServer) UnifiedPostHandler(w http.ResponseWriter, r *http.Request) { + // Failures on this route answer in the IAM/STS query-protocol envelope + // unless the request was actually signed for s3. + r, reqID := request_id.Ensure(r) + writeQueryError := func(iamCode string, s3Code s3err.ErrorCode) { + if requestSigningService(r) == "s3" { + s3err.WriteErrorResponse(w, r, s3Code) + return + } + writeIamErrorResponse(w, r, reqID, &iamError{Code: iamCode, Error: errors.New(s3err.GetAPIError(s3Code).Description)}) + } + // 1. Authenticate (preserves body) identity, errCode := s3a.iam.AuthSignatureOnly(r) if errCode != s3err.ErrNone { - s3err.WriteErrorResponse(w, r, errCode) + writeQueryError(iamAuthErrorCode(errCode), errCode) return } @@ -745,7 +757,7 @@ func (s3a *S3ApiServer) UnifiedPostHandler(w http.ResponseWriter, r *http.Reques bodyBytes, err = readRequestBody(r, iamRequestBodyLimit) if err != nil { glog.Errorf("failed to read request body: %v", err) - s3err.WriteErrorResponse(w, r, s3err.ErrInvalidRequest) + writeQueryError("InvalidInput", s3err.ErrInvalidRequest) return } r.Body.Close() @@ -754,7 +766,7 @@ func (s3a *S3ApiServer) UnifiedPostHandler(w http.ResponseWriter, r *http.Reques } if err := r.ParseForm(); err != nil { - s3err.WriteErrorResponse(w, r, s3err.ErrInvalidRequest) + writeQueryError("InvalidInput", s3err.ErrInvalidRequest) return } @@ -781,7 +793,7 @@ func (s3a *S3ApiServer) UnifiedPostHandler(w http.ResponseWriter, r *http.Reques // IAM // IAM API requests must be authenticated - reject nil identity if identity == nil { - s3err.WriteErrorResponse(w, r, s3err.ErrAccessDenied) + writeQueryError("AccessDenied", s3err.ErrAccessDenied) return } @@ -792,7 +804,7 @@ func (s3a *S3ApiServer) UnifiedPostHandler(w http.ResponseWriter, r *http.Reques // UserName comes from the body only, the same place DoActions reads it // from, so the authorized target and the acted-on target cannot differ. if s3a.iam.AuthorizeIamAction(r, identity, action, iamTargetUserName(action, r)) != s3err.ErrNone { - s3err.WriteErrorResponse(w, r, s3err.ErrAccessDenied) + writeQueryError("AccessDenied", s3err.ErrAccessDenied) return } diff --git a/weed/s3api/s3err/s3api_errors.go b/weed/s3api/s3err/s3api_errors.go index a040a59b0..4e3d4886f 100644 --- a/weed/s3api/s3err/s3api_errors.go +++ b/weed/s3api/s3err/s3api_errors.go @@ -99,6 +99,7 @@ const ( ErrPolicyTooLarge ErrMissingSignHeadersTag ErrMissingSignTag + ErrAuthorizationHeaderMalformed ErrUnsignedHeaders ErrInvalidQueryParams ErrInvalidQuerySignatureAlgo @@ -114,6 +115,7 @@ const ( ErrRequestTimeTooSkewed ErrMissingDateHeader ErrInvalidRequest + ErrInvalidArgument ErrAuthNotSetup ErrNotImplemented ErrPreconditionFailed @@ -178,6 +180,7 @@ const ( // Error message constants for checksum validation const ( + ErrMsgContentSha256Mismatch = "the provided 'x-amz-content-sha256' header does not match what was computed" ErrMsgPayloadChecksumMismatch = "payload checksum does not match" ErrMsgChunkSignatureMismatch = "chunk signature does not match" ErrMsgChecksumAlgorithmMismatch = "checksum algorithm mismatch" @@ -483,10 +486,15 @@ var errorCodeResponse = map[ErrorCode]APIError{ HTTPStatusCode: http.StatusBadRequest, }, ErrMissingSignTag: { - Code: "AccessDenied", + Code: "AuthorizationHeaderMalformed", Description: "Signature header missing Signature field.", HTTPStatusCode: http.StatusBadRequest, }, + ErrAuthorizationHeaderMalformed: { + Code: "AuthorizationHeaderMalformed", + Description: "The authorization header is malformed.", + HTTPStatusCode: http.StatusBadRequest, + }, ErrUnsignedHeaders: { Code: "AccessDenied", @@ -561,13 +569,18 @@ var errorCodeResponse = map[ErrorCode]APIError{ ErrMissingDateHeader: { Code: "AccessDenied", Description: "AWS authentication requires a valid Date or x-amz-date header", - HTTPStatusCode: http.StatusBadRequest, + HTTPStatusCode: http.StatusForbidden, }, ErrInvalidRequest: { Code: "InvalidRequest", Description: "Invalid Request", HTTPStatusCode: http.StatusBadRequest, }, + ErrInvalidArgument: { + Code: "InvalidArgument", + Description: "Invalid Argument", + HTTPStatusCode: http.StatusBadRequest, + }, ErrAccessControlListNotSupported: { Code: "AccessControlListNotSupported", Description: "The bucket does not allow ACLs", @@ -614,12 +627,12 @@ var errorCodeResponse = map[ErrorCode]APIError{ HTTPStatusCode: http.StatusConflict, }, ErrTooManyRequest: { - Code: "ErrTooManyRequest", + Code: "SlowDown", Description: "Too many simultaneous request count", HTTPStatusCode: http.StatusServiceUnavailable, }, ErrRequestBytesExceed: { - Code: "ErrRequestBytesExceed", + Code: "SlowDown", Description: "Simultaneous request bytes exceed limitations", HTTPStatusCode: http.StatusServiceUnavailable, },