Unverified Commit e38c5314 authored by John DiSanti's avatar John DiSanti Committed by GitHub
Browse files

Fix S3 canary and canary permissions (#994)

parent a536c15b
Loading
Loading
Loading
Loading
+23 −4
Original line number Diff line number Diff line
@@ -8,6 +8,8 @@ use anyhow::Context;
use aws_sdk_s3 as s3;
use uuid::Uuid;

const METADATA_TEST_VALUE: &str = "some   value";

pub async fn s3_canary(client: s3::Client, s3_bucket_name: String) -> anyhow::Result<()> {
    use s3::{error::GetObjectError, error::GetObjectErrorKind, SdkError};
    let test_key = Uuid::new_v4().as_u128().to_string();
@@ -46,7 +48,7 @@ pub async fn s3_canary(client: s3::Client, s3_bucket_name: String) -> anyhow::Re
        .bucket(&s3_bucket_name)
        .key(&test_key)
        .body(s3::ByteStream::from_static(b"test"))
        .metadata("something", "テスト テスト!")
        .metadata("something", METADATA_TEST_VALUE)
        .send()
        .await
        .context("s3::PutObject")?;
@@ -65,10 +67,10 @@ pub async fn s3_canary(client: s3::Client, s3_bucket_name: String) -> anyhow::Re
        Some(map) => {
            // Option::as_deref doesn't work here since the deref of &String is String
            let value = map.get("something").map(|s| s.as_str()).unwrap_or("");
            if value != "テスト テスト!" {
            if value != METADATA_TEST_VALUE {
                result = Err(CanaryError(format!(
                    "S3 metadata was incorrect. Expected `テスト テスト!` but got `{}`.",
                    value
                    "S3 metadata was incorrect. Expected `{}` but got `{}`.",
                    METADATA_TEST_VALUE, value
                ))
                .into());
            }
@@ -99,3 +101,20 @@ pub async fn s3_canary(client: s3::Client, s3_bucket_name: String) -> anyhow::Re

    result
}

// This test runs against an actual AWS account. Comment out the `ignore` to run it.
// Be sure to set the `TEST_S3_BUCKET` environment variable to the S3 bucket to use,
// and also make sure the credential profile sets the region (or set `AWS_DEFAULT_PROFILE`).
#[ignore]
#[cfg(test)]
#[tokio::test]
async fn test_s3_canary() {
    let config = aws_config::load_from_env().await;
    let client = s3::Client::new(&config);
    s3_canary(
        client,
        std::env::var("TEST_S3_BUCKET").expect("TEST_S3_BUCKET must be set"),
    )
    .await
    .expect("success");
}
+19 −1
Original line number Diff line number Diff line
@@ -44,6 +44,7 @@ export class CanaryStack extends Stack {
                    "lambda:CreateFunction",
                    "lambda:DeleteFunction",
                    "lambda:InvokeFunction",
                    "lambda:GetFunctionConfiguration",
                ],
                effect: Effect.ALLOW,
                // Only allow this for functions starting with prefix `canary-`
@@ -66,7 +67,8 @@ export class CanaryStack extends Stack {
            removalPolicy: RemovalPolicy.DESTROY,
        });

        // Allow the OIDC role to PutObject to the code bucket
        // Allow the OIDC role to GetObject and PutObject to the code bucket
        this.canaryCodeBucket.grantRead(this.awsSdkRustOidcRole.oidcRole);
        this.canaryCodeBucket.grantWrite(this.awsSdkRustOidcRole.oidcRole);

        // Create S3 bucket for the canaries to talk to
@@ -110,5 +112,21 @@ export class CanaryStack extends Stack {
                resources: ["*"],
            }),
        );

        // Allow the OIDC role to pass the Lambda execution role to Lambda
        this.awsSdkRustOidcRole.oidcRole.addToPolicy(
            new PolicyStatement({
                actions: ["iam:PassRole"],
                effect: Effect.ALLOW,
                // Security: only allow the Lambda execution role to be passed
                resources: [this.lambdaExecutionRole.roleArn],
                // Security: only allow the role to be passed to Lambda
                conditions: {
                    StringEquals: {
                        "iam:PassedToService": "lambda.amazonaws.com",
                    },
                },
            }),
        );
    }
}