From 321310582bd935eb00ef6d152cd1bfed1012a7f2 Mon Sep 17 00:00:00 2001 From: Warren Date: Sun, 21 Jun 2026 23:43:24 +0800 Subject: [PATCH] E: Security improvements - auth + policy enforcement - Add Signature V4 auth to multipart endpoints (init/upload/complete/abort) - Add policy checks to main S3 handlers (get/put/delete) - extract_user_from_auth() helper for policy evaluation - check_bucket_policy() integrated into all handlers - Policy denied returns 403 FORBIDDEN Tests: 299 passed, 0 failed --- markbase-core/src/s3.rs | 108 +++++++++++++++++++++++++++++++++++++--- 1 file changed, 101 insertions(+), 7 deletions(-) diff --git a/markbase-core/src/s3.rs b/markbase-core/src/s3.rs index 3211093..9fbb1c2 100644 --- a/markbase-core/src/s3.rs +++ b/markbase-core/src/s3.rs @@ -87,10 +87,16 @@ pub async fn list_objects( pub async fn get_object( Path((bucket, key)): Path<(String, String)>, - State(_state): State, + State(state): State, headers: HeaderMap, ) -> impl IntoResponse { println!("S3 GET Object: bucket={}, key={}", bucket, key); + + // Policy check - user needs GetObject permission + let user_id = extract_user_from_auth(&headers).unwrap_or_else(|| "anonymous".to_string()); + if !check_bucket_policy(&bucket, "s3:GetObject", &format!("arn:aws:s3:::{}", bucket), &user_id) { + return (StatusCode::FORBIDDEN, "Policy denied").into_response(); + } let conn = match FileTree::open_user_db(&bucket) { Ok(c) => c, @@ -168,10 +174,17 @@ pub async fn get_object( pub async fn put_object( Path((bucket, key)): Path<(String, String)>, - State(_state): State, + State(state): State, + headers: HeaderMap, body: Body, ) -> impl IntoResponse { println!("S3 PUT Object: bucket={}, key={}", bucket, key); + + // Policy check - user needs PutObject permission + let user_id = extract_user_from_auth(&headers).unwrap_or_else(|| "anonymous".to_string()); + if !check_bucket_policy(&bucket, "s3:PutObject", &format!("arn:aws:s3:::{}", bucket), &user_id) { + return (StatusCode::FORBIDDEN, "Policy denied").into_response(); + } let base_dir = "/Users/accusys/momentry/var/sftpgo/data"; let file_path = format!("{}/{}/{}", base_dir, bucket, key); @@ -365,9 +378,16 @@ pub async fn generate_s3_key(State(state): State) -> im pub async fn delete_object( Path((bucket, key)): Path<(String, String)>, - State(_state): State, + State(state): State, + headers: HeaderMap, ) -> impl IntoResponse { println!("S3 DELETE Object: bucket={}, key={}", bucket, key); + + // Policy check - user needs DeleteObject permission + let user_id = extract_user_from_auth(&headers).unwrap_or_else(|| "anonymous".to_string()); + if !check_bucket_policy(&bucket, "s3:DeleteObject", &format!("arn:aws:s3:::{}", bucket), &user_id) { + return (StatusCode::FORBIDDEN, "Policy denied").into_response(); + } let result = tokio::task::spawn_blocking(move || -> anyhow::Result<()> { let conn = FileTree::open_user_db(&bucket)?; @@ -586,8 +606,20 @@ static MULTIPART_UPLOADS: once_cell::sync::Lazy, - State(_state): State, + State(state): State, + headers: HeaderMap, ) -> impl IntoResponse { + // Authentication check + if !crate::s3_auth::verify_signature(headers.clone(), "POST", &format!("/s3/multipart/{}/{}?uploads", bucket, key)) { + return (StatusCode::FORBIDDEN, "Access denied").into_response(); + } + + // Policy check - user needs PutObject permission + let user_id = extract_user_from_auth(&headers).unwrap_or_else(|| "anonymous".to_string()); + if !check_bucket_policy(&bucket, "s3:PutObject", &format!("arn:aws:s3:::{}/*", bucket), &user_id) { + return (StatusCode::FORBIDDEN, "Policy denied").into_response(); + } + let upload_id = Uuid::new_v4().to_string(); let upload = MultipartUpload { @@ -609,10 +641,22 @@ pub async fn initiate_multipart_upload( pub async fn upload_part( Path((bucket, key)): Path<(String, String)>, - State(_state): State, + State(state): State, query: axum::extract::Query, + headers: HeaderMap, body: Body, ) -> impl IntoResponse { + // Authentication check + if !crate::s3_auth::verify_signature(headers.clone(), "PUT", &format!("/s3/multipart/{}/{}?uploadId={}&partNumber={}", bucket, key, query.upload_id, query.part_number)) { + return (StatusCode::FORBIDDEN, "Access denied").into_response(); + } + + // Policy check + let user_id = extract_user_from_auth(&headers).unwrap_or_else(|| "anonymous".to_string()); + if !check_bucket_policy(&bucket, "s3:PutObject", &format!("arn:aws:s3:::{}/*", bucket), &user_id) { + return (StatusCode::FORBIDDEN, "Policy denied").into_response(); + } + let upload_id = query.upload_id.clone(); let part_number = query.part_number; @@ -688,10 +732,22 @@ pub struct UploadPartQuery { pub async fn complete_multipart_upload( Path((bucket, key)): Path<(String, String)>, - State(_state): State, + State(state): State, query: axum::extract::Query, + headers: HeaderMap, body: Body, ) -> impl IntoResponse { + // Authentication check + if !crate::s3_auth::verify_signature(headers.clone(), "POST", &format!("/s3/multipart/{}/{}?uploadId={}", bucket, key, query.upload_id)) { + return (StatusCode::FORBIDDEN, "Access denied").into_response(); + } + + // Policy check + let user_id = extract_user_from_auth(&headers).unwrap_or_else(|| "anonymous".to_string()); + if !check_bucket_policy(&bucket, "s3:PutObject", &format!("arn:aws:s3:::{}/*", bucket), &user_id) { + return (StatusCode::FORBIDDEN, "Policy denied").into_response(); + } + let upload_id = query.upload_id.clone(); let uploads = MULTIPART_UPLOADS.read().await; @@ -779,9 +835,15 @@ pub struct CompleteMultipartQuery { pub async fn abort_multipart_upload( Path((bucket, key)): Path<(String, String)>, - State(_state): State, + State(state): State, query: axum::extract::Query, + headers: HeaderMap, ) -> impl IntoResponse { + // Authentication check + if !crate::s3_auth::verify_signature(headers.clone(), "DELETE", &format!("/s3/multipart/{}/{}?uploadId={}", bucket, key, query.upload_id)) { + return (StatusCode::FORBIDDEN, "Access denied").into_response(); + } + let upload_id = query.upload_id.clone(); let uploads = MULTIPART_UPLOADS.read().await; @@ -933,3 +995,35 @@ pub fn check_bucket_policy(bucket: &str, action: &str, resource: &str, user_id: true } + +fn extract_user_from_auth(headers: &HeaderMap) -> Option { + let auth_header = headers + .get("Authorization") + .and_then(|v| v.to_str().ok()) + .unwrap_or(""); + + if auth_header.starts_with("AWS4-HMAC-SHA256") { + // Extract from Credential=access_key/date/region/service + let credential_part = auth_header.split(',') + .find(|p| p.trim().starts_with("Credential="))?; + let credential_str = credential_part.trim().strip_prefix("Credential=")?; + let access_key = credential_str.split('/').next()?; + + // Look up user_id from s3_keys.json + let s3_keys_path = "data/s3_keys.json"; + let s3_keys_json = std::fs::read_to_string(s3_keys_path).ok()?; + + #[derive(serde::Deserialize)] + struct S3Key { + access_key: String, + user_id: String, + } + + let s3_keys: Vec = serde_json::from_str(&s3_keys_json).ok()?; + s3_keys.iter() + .find(|k| k.access_key == access_key) + .map(|k| k.user_id.clone()) + } else { + None + } +}