Unverified Commit fa25936d authored by Copilot's avatar Copilot Committed by GitHub
Browse files

Add STS test for s3s-fs and fix signature validation for STS requests (#418)



* Initial plan

* Add STS test infrastructure and AssumeRole route

Co-authored-by: default avatarNugine <30099658+Nugine@users.noreply.github.com>

* Fix STS signature validation to support payload hash computation

Co-authored-by: default avatarNugine <30099658+Nugine@users.noreply.github.com>

* Fix formatting and clean up code

Co-authored-by: default avatarNugine <30099658+Nugine@users.noreply.github.com>

* Address code review feedback: improve error handling and constants

Co-authored-by: default avatarNugine <30099658+Nugine@users.noreply.github.com>

* Apply PR review feedback: fix line numbers, use CONTENT_TYPE constant, remove duplicate dependency

Co-authored-by: default avatarNugine <30099658+Nugine@users.noreply.github.com>

* Add unit tests for STS signature validation logic

Co-authored-by: default avatarNugine <30099658+Nugine@users.noreply.github.com>

---------

Co-authored-by: default avatarcopilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: default avatarNugine <30099658+Nugine@users.noreply.github.com>
Co-authored-by: default avatarNugine <nugine@foxmail.com>
parent 4c438779
Loading
Loading
Loading
Loading
+2 −0
Original line number Diff line number Diff line
@@ -2946,6 +2946,7 @@ dependencies = [
 "aws-config",
 "aws-credential-types",
 "aws-sdk-s3",
 "aws-sdk-sts",
 "base64-simd",
 "bytes",
 "chrono",
@@ -2954,6 +2955,7 @@ dependencies = [
 "futures",
 "futures-util",
 "hex-simd",
 "hyper 1.8.1",
 "hyper-util",
 "mime",
 "numeric_cast",
+2 −0
Original line number Diff line number Diff line
@@ -63,7 +63,9 @@ anyhow = { version = "1.0.100", features = ["backtrace"] }
aws-config = { version = "1.8.7", default-features = false }
aws-credential-types = { version = "1.2.6", features = ["test-util"] }
aws-sdk-s3 = { version = "1.107.0", features = ["behavior-version-latest"] }
aws-sdk-sts = { version = "1.87.0", features = ["behavior-version-latest"] }
futures-util = "0.3.31"
hyper = { version = "1.8.1", features = ["http1", "http2"] }
hyper-util = { version = "0.1.18", features = ["server-auto", "server-graceful", "http1", "http2", "tokio"] }
once_cell = "1.21.3"
opendal = { version = "0.55.0", features = ["services-s3"] }
+59 −0
Original line number Diff line number Diff line
use s3s::auth::SimpleAuth;
use s3s::header::CONTENT_TYPE;
use s3s::host::SingleDomain;
use s3s::service::S3ServiceBuilder;
use s3s::validation::NameValidation;
@@ -21,6 +22,7 @@ use aws_sdk_s3::types::CompletedPart;
use aws_sdk_s3::types::CreateBucketConfiguration;

use anyhow::Result;
use hyper::Method;
use tokio::sync::Mutex;
use tokio::sync::MutexGuard;
use tracing::{debug, error};
@@ -30,6 +32,28 @@ const FS_ROOT: &str = concat!(env!("CARGO_TARGET_TMPDIR"), "/s3s-fs-tests-aws");
const DOMAIN_NAME: &str = "localhost:8014";
const REGION: &str = "us-west-2";

// STS AssumeRole route that returns NotImplemented
struct AssumeRoleRoute;

#[async_trait::async_trait]
impl s3s::route::S3Route for AssumeRoleRoute {
    fn is_match(&self, method: &Method, uri: &hyper::Uri, headers: &hyper::HeaderMap, _: &mut hyper::http::Extensions) -> bool {
        if method == Method::POST && uri.path() == "/" {
            if let Some(val) = headers.get(CONTENT_TYPE) {
                if val.as_bytes() == b"application/x-www-form-urlencoded" {
                    return true;
                }
            }
        }
        false
    }

    async fn call(&self, _req: s3s::S3Request<s3s::Body>) -> s3s::S3Result<s3s::S3Response<s3s::Body>> {
        debug!("AssumeRole called - returning NotImplemented");
        Err(s3s::s3_error!(NotImplemented, "STS operations are not supported by s3s-fs"))
    }
}

fn setup_tracing() {
    use tracing_subscriber::EnvFilter;

@@ -62,6 +86,7 @@ fn config() -> &'static SdkConfig {
            let mut b = S3ServiceBuilder::new(fs);
            b.set_auth(SimpleAuth::from_single(cred.access_key_id(), cred.secret_access_key()));
            b.set_host(SingleDomain::new(DOMAIN_NAME).unwrap());
            b.set_route(AssumeRoleRoute);
            b.build()
        };

@@ -767,3 +792,37 @@ async fn test_default_bucket_validation() -> Result<()> {

    Ok(())
}

#[tokio::test]
#[tracing::instrument]
async fn test_sts_assume_role_not_implemented() -> Result<()> {
    let _guard = serial().await;

    // Create STS client using the same config as S3
    let sdk_config = config();
    let sts_client = aws_sdk_sts::Client::new(sdk_config);

    // Attempt to call AssumeRole - should fail with NotImplemented
    let result = sts_client
        .assume_role()
        .role_arn("arn:aws:iam::123456789012:role/test-role")
        .role_session_name("test-session")
        .send()
        .await;

    // Verify the operation returned an error
    assert!(result.is_err(), "Expected AssumeRole to fail with NotImplemented error");

    // Check that the error is NotImplemented
    let error = result.unwrap_err();
    let error_str = format!("{error:?}");
    debug!("AssumeRole error (expected): {error_str}");

    // The error should contain "NotImplemented" or similar indication
    assert!(
        error_str.contains("NotImplemented") || error_str.contains("not implemented"),
        "Expected NotImplemented error, got: {error_str}"
    );

    Ok(())
}
+93 −3
Original line number Diff line number Diff line
@@ -13,6 +13,7 @@ use crate::sig_v4::AmzDate;
use crate::sig_v4::UploadStream;
use crate::sig_v4::{AuthorizationV4, CredentialV4, PostSignatureV4, PresignedUrlV4};
use crate::stream::ByteStream as _;
use crate::utils::crypto::hex_sha256;
use crate::utils::is_base64_encoded;

use std::mem;
@@ -24,6 +25,9 @@ use hyper::Uri;
use mime::Mime;
use tracing::debug;

/// Maximum allowed size for STS request body (8KB should be enough for operations like `AssumeRole`)
const MAX_STS_BODY_SIZE: usize = 8192;

fn extract_amz_content_sha256<'a>(hs: &'_ OrderedHeaders<'a>) -> S3Result<Option<AmzContentSha256<'a>>> {
    let Some(val) = hs.get_unique(crate::header::X_AMZ_CONTENT_SHA256) else { return Ok(None) };
    match AmzContentSha256::parse(val) {
@@ -385,10 +389,34 @@ impl SignatureContext<'_> {
                    return Err(s3_error!(NotImplemented, "AWS4-ECDSA-P256-SHA256 signing method is not implemented yet"));
                }
                None => {
                    // For STS requests, x-amz-content-sha256 header is not required
                    // For S3 requests, this case should have been caught earlier (see lines 325-327)
                    if service == "sts" {
                        // STS requests require computing the payload hash from the body
                        // Read the body (it's small for STS requests like AssumeRole)
                        let body_bytes = self
                            .req_body
                            .store_all_limited(MAX_STS_BODY_SIZE)
                            .await
                            .map_err(|e| invalid_request!("failed to read STS request body: {}", e))?;

                        // Compute SHA256 hash and convert to hex
                        let hash = hex_sha256(&body_bytes, str::to_owned);

                        // Create canonical request with the computed hash
                        sig_v4::create_canonical_request(
                            method,
                            uri_path,
                            query_strings,
                            &headers,
                            sig_v4::Payload::SingleChunk(&hash),
                        )
                    } else {
                        // According to AWS S3 protocol, x-amz-content-sha256 header is required for
                    // all requests authenticated with Signature V4. Reject if missing.
                        // all S3 requests authenticated with Signature V4. Reject if missing.
                        return Err(invalid_request!("missing header: x-amz-content-sha256"));
                    }
                }
            };
            let string_to_sign = sig_v4::create_string_to_sign(&canonical_request, &amz_date, region, service);
            sig_v4::calculate_signature(&string_to_sign, &secret_key, &amz_date, region, service)
@@ -619,4 +647,66 @@ mod tests {
        let err = result.unwrap_err();
        assert!(err.message().unwrap().contains("x-amz-content-sha256"));
    }

    #[tokio::test]
    async fn test_sts_body_hash_computation() {
        // Test that STS request body hash is computed correctly
        use crate::utils::crypto::hex_sha256;

        // Typical STS AssumeRole request body
        let body_content = b"Action=AssumeRole&RoleArn=arn:aws:iam::123456789012:role/test-role&RoleSessionName=test-session";

        // Compute hash
        let hash = hex_sha256(body_content, str::to_owned);

        // Verify hash is a valid hex string of correct length (64 chars for SHA256)
        assert_eq!(hash.len(), 64);
        assert!(hash.chars().all(|c| c.is_ascii_hexdigit()));

        // Verify hash is deterministic
        let hash2 = hex_sha256(body_content, str::to_owned);
        assert_eq!(hash, hash2);
    }

    #[tokio::test]
    async fn test_sts_body_size_limit_enforced() {
        // Test that body size limit is enforced for STS requests
        use bytes::Bytes;

        // Create a body that exceeds MAX_STS_BODY_SIZE
        let large_body = vec![b'x'; MAX_STS_BODY_SIZE + 1];
        let mut body = Body::from(Bytes::from(large_body));

        // Try to read with limit
        let result = body.store_all_limited(MAX_STS_BODY_SIZE).await;

        // Should fail due to size limit
        assert!(result.is_err());
    }

    #[tokio::test]
    async fn test_sts_body_within_limit() {
        // Test that body reading succeeds when within limit
        use bytes::Bytes;

        // Create a body within the limit
        let small_body = b"Action=AssumeRole&RoleArn=test&RoleSessionName=session";
        let mut body = Body::from(Bytes::from(&small_body[..]));

        // Try to read with limit
        let result = body.store_all_limited(MAX_STS_BODY_SIZE).await;

        // Should succeed
        assert!(result.is_ok());
        let bytes = result.unwrap();
        assert_eq!(&bytes[..], &small_body[..]);
    }

    #[test]
    fn test_sts_max_body_size_constant() {
        // Verify the constant is set to a reasonable value
        assert_eq!(MAX_STS_BODY_SIZE, 8192);
        // STS requests are typically small (under 2KB for AssumeRole)
        // 8KB provides a good safety margin
    }
}