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
This commit is contained in:
@@ -87,10 +87,16 @@ pub async fn list_objects(
|
||||
|
||||
pub async fn get_object(
|
||||
Path((bucket, key)): Path<(String, String)>,
|
||||
State(_state): State<crate::server::AppState>,
|
||||
State(state): State<crate::server::AppState>,
|
||||
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<crate::server::AppState>,
|
||||
State(state): State<crate::server::AppState>,
|
||||
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<crate::server::AppState>) -> im
|
||||
|
||||
pub async fn delete_object(
|
||||
Path((bucket, key)): Path<(String, String)>,
|
||||
State(_state): State<crate::server::AppState>,
|
||||
State(state): State<crate::server::AppState>,
|
||||
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<Arc<RwLock<HashMap<String, Multi
|
||||
|
||||
pub async fn initiate_multipart_upload(
|
||||
Path((bucket, key)): Path<(String, String)>,
|
||||
State(_state): State<crate::server::AppState>,
|
||||
State(state): State<crate::server::AppState>,
|
||||
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<crate::server::AppState>,
|
||||
State(state): State<crate::server::AppState>,
|
||||
query: axum::extract::Query<UploadPartQuery>,
|
||||
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<crate::server::AppState>,
|
||||
State(state): State<crate::server::AppState>,
|
||||
query: axum::extract::Query<CompleteMultipartQuery>,
|
||||
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<crate::server::AppState>,
|
||||
State(state): State<crate::server::AppState>,
|
||||
query: axum::extract::Query<AbortMultipartQuery>,
|
||||
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<String> {
|
||||
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<S3Key> = 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
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user