diff --git a/weed/s3api/s3api_object_handlers_postpolicy.go b/weed/s3api/s3api_object_handlers_postpolicy.go index d16810f4e..c17953d5c 100644 --- a/weed/s3api/s3api_object_handlers_postpolicy.go +++ b/weed/s3api/s3api_object_handlers_postpolicy.go @@ -97,8 +97,8 @@ func (s3a *S3ApiServer) PostPolicyBucketHandler(w http.ResponseWriter, r *http.R // Make sure formValues adhere to policy restrictions. if err = policy.CheckPostPolicy(formValues, postPolicyForm); err != nil { - w.Header().Set("Location", r.URL.Path) - w.WriteHeader(http.StatusTemporaryRedirect) + glog.V(3).Infof("PostPolicy check failed for bucket %s: %v", bucket, err) + s3err.WriteErrorResponseWithMessage(w, r, s3err.ErrAccessDenied, err.Error()) return } diff --git a/weed/s3api/s3api_object_handlers_postpolicy_test.go b/weed/s3api/s3api_object_handlers_postpolicy_test.go index 0e181d7a1..fd6c65960 100644 --- a/weed/s3api/s3api_object_handlers_postpolicy_test.go +++ b/weed/s3api/s3api_object_handlers_postpolicy_test.go @@ -2,14 +2,20 @@ package s3api import ( "bytes" + "encoding/base64" + "encoding/hex" + "fmt" "io" "mime/multipart" "net/http" "net/http/httptest" "strings" + "sync" "testing" + "time" "github.com/gorilla/mux" + "github.com/seaweedfs/seaweedfs/weed/pb/iam_pb" "github.com/seaweedfs/seaweedfs/weed/s3api/s3_constants" "github.com/stretchr/testify/assert" ) @@ -383,3 +389,111 @@ func TestPostPolicyBucketHandlerKeyExtraction(t *testing.T) { }) } } + +// TestPostPolicyBucketHandler_PolicyViolationReturns403 drives the handler +// end-to-end with a signed multipart POST whose policy conditions cannot be +// satisfied by the form fields. It verifies the handler responds 403 +// AccessDenied with no Location redirect, rather than the old 307 Temporary +// Redirect that obscured the policy failure. +func TestPostPolicyBucketHandler_PolicyViolationReturns403(t *testing.T) { + const ( + accessKey = "AKIATESTTESTTEST" + secretKey = "secret-key-for-tests" + region = "us-east-1" + service = "s3" + testBucket = "test-bucket" + ) + + iam := &IdentityAccessManagement{ + hashes: make(map[string]*sync.Pool), + hashCounters: make(map[string]*int32), + } + err := iam.loadS3ApiConfiguration(&iam_pb.S3ApiConfiguration{ + Identities: []*iam_pb.Identity{{ + Name: "tester", + Credentials: []*iam_pb.Credential{{AccessKey: accessKey, SecretKey: secretKey}}, + Actions: []string{"Admin", "Read", "Write"}, + }}, + }) + assert.NoError(t, err, "loadS3ApiConfiguration should succeed") + + s3a := &S3ApiServer{ + option: &S3ApiServerOption{BucketsPath: "/buckets"}, + iam: iam, + } + // Pre-populate the bucket registry so validateTableBucketObjectPath sees + // a non-table bucket without needing a live filer connection. + s3a.bucketRegistry = &BucketRegistry{ + metadataCache: map[string]*BucketMetaData{ + testBucket: {Name: testBucket, IsTableBucket: false}, + }, + notFound: make(map[string]struct{}), + s3a: s3a, + } + + now := time.Now().UTC() + amzDate := now.Format(iso8601Format) + yyyymmddStr := now.Format(yyyymmdd) + credential := fmt.Sprintf("%s/%s/%s/%s/aws4_request", accessKey, yyyymmddStr, region, service) + expiration := now.Add(1 * time.Hour).Format("2006-01-02T15:04:05.000Z") + + policyJSON := fmt.Sprintf( + `{"expiration":"%s","conditions":[`+ + `["eq","$bucket","%s"],`+ + `["eq","$key","required.txt"],`+ + `["eq","$x-amz-credential","%s"],`+ + `["eq","$x-amz-algorithm","AWS4-HMAC-SHA256"],`+ + `["eq","$x-amz-date","%s"]`+ + `]}`, + expiration, testBucket, credential, amzDate, + ) + encodedPolicy := base64.StdEncoding.EncodeToString([]byte(policyJSON)) + + signingKey := getSigningKey(secretKey, yyyymmddStr, region, service) + signature := getSignature(signingKey, encodedPolicy) + // Sanity-check: the signature must be valid hex of the expected length so + // that doesPolicySignatureV4Match does not trip on formatting. + _, decodeErr := hex.DecodeString(signature) + assert.NoError(t, decodeErr, "computed signature should be hex-encoded") + + var buf bytes.Buffer + writer := multipart.NewWriter(&buf) + assert.NoError(t, writer.WriteField("bucket", testBucket)) + // Deliberately mismatch the policy's required key to force a violation. + assert.NoError(t, writer.WriteField("key", "wrong.txt")) + assert.NoError(t, writer.WriteField("x-amz-credential", credential)) + assert.NoError(t, writer.WriteField("x-amz-algorithm", "AWS4-HMAC-SHA256")) + assert.NoError(t, writer.WriteField("x-amz-date", amzDate)) + assert.NoError(t, writer.WriteField("policy", encodedPolicy)) + assert.NoError(t, writer.WriteField("x-amz-signature", signature)) + + filePart, err := writer.CreateFormFile("file", "payload.txt") + assert.NoError(t, err) + _, err = filePart.Write([]byte("contents that should never be uploaded")) + assert.NoError(t, err) + assert.NoError(t, writer.Close()) + + req := httptest.NewRequest(http.MethodPost, "/"+testBucket, &buf) + req.Header.Set("Content-Type", writer.FormDataContentType()) + req = mux.SetURLVars(req, map[string]string{"bucket": testBucket}) + rec := httptest.NewRecorder() + + s3a.PostPolicyBucketHandler(rec, req) + + assert.Equal(t, http.StatusForbidden, rec.Code, + "policy violation must return 403, actual body: %s", rec.Body.String()) + assert.NotEqual(t, http.StatusTemporaryRedirect, rec.Code, + "must not return 307 Temporary Redirect on policy violation") + assert.Empty(t, rec.Header().Get("Location"), + "403 response must not set a redirect Location header") + assert.Contains(t, rec.Body.String(), "AccessDenied", + "response body should identify AccessDenied; actual body: %s", rec.Body.String()) + assert.Contains(t, rec.Body.String(), "Policy Condition failed", + "response body should carry the specific policy-failure message; actual body: %s", rec.Body.String()) + // Guard against the signing setup silently failing before the policy + // branch ever runs, which would make the 403 assertion meaningless. + assert.NotContains(t, rec.Body.String(), "SignatureDoesNotMatch", + "signature must match so the handler reaches the policy-check branch") + assert.NotContains(t, rec.Body.String(), "InvalidAccessKeyId", + "access key must be known so the handler reaches the policy-check branch") +} diff --git a/weed/s3api/s3err/error_handler.go b/weed/s3api/s3err/error_handler.go index ec819dfc8..180546e43 100644 --- a/weed/s3api/s3err/error_handler.go +++ b/weed/s3api/s3err/error_handler.go @@ -40,6 +40,15 @@ func WriteEmptyResponse(w http.ResponseWriter, r *http.Request, statusCode int) } func WriteErrorResponse(w http.ResponseWriter, r *http.Request, errorCode ErrorCode) { + WriteErrorResponseWithMessage(w, r, errorCode, "") +} + +// WriteErrorResponseWithMessage writes an S3 error response that uses the +// standard error code mapping (status + Code). When message is non-empty, +// it overrides the default Message field so the caller can surface why the +// request was rejected (e.g. which POST policy condition failed) instead +// of the generic APIError Description. +func WriteErrorResponseWithMessage(w http.ResponseWriter, r *http.Request, errorCode ErrorCode, message string) { r, reqID := request_id.Ensure(r) vars := mux.Vars(r) bucket := vars["bucket"] @@ -50,6 +59,9 @@ func WriteErrorResponse(w http.ResponseWriter, r *http.Request, errorCode ErrorC apiError := GetAPIError(errorCode) errorResponse := getRESTErrorResponse(apiError, r.URL.Path, bucket, object, reqID) + if message != "" { + errorResponse.Message = message + } WriteXMLResponse(w, r, apiError.HTTPStatusCode, errorResponse) PostLog(r, apiError.HTTPStatusCode, errorCode) }