Unverified Commit 1d7dbd6b authored by Zelda Hessler's avatar Zelda Hessler Committed by GitHub
Browse files

fix: handling of S3 part-level checksums (#2512) 

* fix: handling of S3 part-level checksums

* add: CHANGELOG.next.toml entry

* add: missing integration test dep to codegen

* fix: use - instead of _
parent 92316f75

CHANGELOG.next.toml

+10 −0
Original line number Diff line number Diff line
@@ -52,3 +52,13 @@ message = "The `enableNewCrateOrganizationScheme` codegen flag has been removed. 
references = ["smithy-rs#2507"]

meta = { "breaking" = false, "tada" = false, "bug" = false, "target" = "client" }

author = "jdisanti"



[[aws-sdk-rust]]

message = """

S3's `GetObject` will no longer panic when checksum validation is enabled and the target object was uploaded as a multi-part upload.

However, these objects cannot be checksum validated by the SDK due to the way checksums are calculated for multipart uploads.

For more information, see [this page](https://docs.aws.amazon.com/AmazonS3/latest/userguide/checking-object-integrity.html#large-object-checksums).

"""

references = ["aws-sdk-rust#764"]

meta = { "breaking" = false, "tada" = false, "bug" = true }

author = "Velfi"

aws/rust-runtime/aws-inlineable/Cargo.toml

+1 −1
Original line number Diff line number Diff line
@@ -25,7 +25,7 @@ aws-types = { path = "../aws-types" } 
bytes = "1"

bytes-utils = "0.1.1"

hex = "0.4.3"

http = "0.2.4"

http = "0.2.9"

http-body = "0.4.5"

md-5 = "0.10.1"

ring = "0.16"

aws/rust-runtime/aws-inlineable/src/http_body_checksum.rs

+74 −6
Original line number Diff line number Diff line
@@ -6,7 +6,6 @@ 
//! Functions for modifying requests and responses for the purposes of checksum validation



use aws_smithy_http::operation::error::BuildError;

use http::header::HeaderName;



/// Errors related to constructing checksum-validated HTTP requests

#[derive(Debug)]
@@ -185,15 +184,32 @@ pub(crate) fn check_headers_for_precalculated_checksum( 
        let checksum_algorithm: aws_smithy_checksums::ChecksumAlgorithm = checksum_algorithm.parse().expect(

            "CHECKSUM_ALGORITHMS_IN_PRIORITY_ORDER only contains valid checksum algorithm names",

        );

        if let Some(precalculated_checksum) = headers.get(HeaderName::from(checksum_algorithm)) {

        if let Some(precalculated_checksum) =

            headers.get(http::HeaderName::from(checksum_algorithm))

        {

            let base64_encoded_precalculated_checksum = precalculated_checksum

                .to_str()

                .expect("base64 uses ASCII characters");



            let precalculated_checksum: bytes::Bytes =

                aws_smithy_types::base64::decode(base64_encoded_precalculated_checksum)

                    .expect("services will always base64 encode the checksum value per the spec")

                    .into();

            // S3 needs special handling for checksums of objects uploaded with `MultiPartUpload`.

            if is_part_level_checksum(base64_encoded_precalculated_checksum) {

                tracing::warn!(

                    more_info = "See https://docs.aws.amazon.com/AmazonS3/latest/userguide/checking-object-integrity.html#large-object-checksums for more information.",

                    "This checksum is a part-level checksum which can't be validated by the Rust SDK. Disable checksum validation for this request to fix this warning.",

                );



                return None;

            }



            let precalculated_checksum = match aws_smithy_types::base64::decode(

                base64_encoded_precalculated_checksum,

            ) {

                Ok(decoded_checksum) => decoded_checksum.into(),

                Err(_) => {

                    tracing::error!("Checksum received from server could not be base64 decoded. No checksum validation will be performed.");

                    return None;

                }

            };



            return Some((checksum_algorithm, precalculated_checksum));

        }
@@ -202,9 +218,38 @@ pub(crate) fn check_headers_for_precalculated_checksum( 
    None

}



fn is_part_level_checksum(checksum: &str) -> bool {

    let mut found_number = false;

    let mut found_dash = false;



    for ch in checksum.chars().rev() {

        // this could be bad

        if ch.is_ascii_digit() {

            found_number = true;

            continue;

        }



        // Yup, it's a part-level checksum

        if ch == '-' {

            if found_dash {

                // Found a second dash?? This isn't a part-level checksum.

                return false;

            }



            found_dash = true;

            continue;

        }



        break;

    }



    found_number && found_dash

}



#[cfg(test)]

mod tests {

    use super::wrap_body_with_checksum_validator;

    use crate::http_body_checksum::is_part_level_checksum;

    use aws_smithy_checksums::ChecksumAlgorithm;

    use aws_smithy_http::body::SdkBody;

    use aws_smithy_http::byte_stream::ByteStream;
@@ -347,4 +392,27 @@ mod tests { 


        assert_eq!(input_text, body);

    }



    #[test]

    fn test_is_multipart_object_checksum() {

        // These ARE NOT part-level checksums

        assert!(!is_part_level_checksum("abcd"));

        assert!(!is_part_level_checksum("abcd="));

        assert!(!is_part_level_checksum("abcd=="));

        assert!(!is_part_level_checksum("1234"));

        assert!(!is_part_level_checksum("1234="));

        assert!(!is_part_level_checksum("1234=="));

        // These ARE part-level checksums

        assert!(is_part_level_checksum("abcd-1"));

        assert!(is_part_level_checksum("abcd=-12"));

        assert!(is_part_level_checksum("abcd12-134"));

        assert!(is_part_level_checksum("abcd==-10000"));

        // These are gibberish and shouldn't be regarded as a part-level checksum

        assert!(!is_part_level_checksum(""));

        assert!(!is_part_level_checksum("Spaces? In my header values?"));

        assert!(!is_part_level_checksum("abcd==-134!#{!#"));

        assert!(!is_part_level_checksum("abcd==-"));

        assert!(!is_part_level_checksum("abcd==--11"));

        assert!(!is_part_level_checksum("abcd==-AA"));

    }

}

aws/sdk-codegen/src/main/kotlin/software/amazon/smithy/rustsdk/HttpResponseChecksumDecorator.kt

+5 −4
Original line number Diff line number Diff line
@@ -8,6 +8,7 @@ package software.amazon.smithy.rustsdk 
import software.amazon.smithy.aws.traits.HttpChecksumTrait

import software.amazon.smithy.model.shapes.MemberShape

import software.amazon.smithy.model.shapes.OperationShape

import software.amazon.smithy.model.shapes.ShapeId

import software.amazon.smithy.rust.codegen.client.smithy.ClientCodegenContext

import software.amazon.smithy.rust.codegen.client.smithy.customize.ClientCodegenDecorator

import software.amazon.smithy.rust.codegen.core.rustlang.Writable
@@ -20,7 +21,7 @@ import software.amazon.smithy.rust.codegen.core.util.inputShape 
import software.amazon.smithy.rust.codegen.core.util.letIf

import software.amazon.smithy.rust.codegen.core.util.orNull



private fun HttpChecksumTrait.requestValidationModeMember(

fun HttpChecksumTrait.requestValidationModeMember(

    codegenContext: ClientCodegenContext,

    operationShape: OperationShape,

): MemberShape? {
@@ -33,14 +34,14 @@ class HttpResponseChecksumDecorator : ClientCodegenDecorator { 
    override val order: Byte = 0



    // TODO(enableNewSmithyRuntime): Implement checksumming via interceptor and delete this decorator

    private fun applies(codegenContext: ClientCodegenContext): Boolean =

        !codegenContext.settings.codegenConfig.enableNewSmithyRuntime

    private fun applies(codegenContext: ClientCodegenContext, operationShape: OperationShape): Boolean =

        !codegenContext.settings.codegenConfig.enableNewSmithyRuntime && operationShape.outputShape != ShapeId.from("com.amazonaws.s3#GetObjectOutput")



    override fun operationCustomizations(

        codegenContext: ClientCodegenContext,

        operation: OperationShape,

        baseCustomizations: List<OperationCustomization>,

    ): List<OperationCustomization> = baseCustomizations.letIf(applies(codegenContext)) {

    ): List<OperationCustomization> = baseCustomizations.letIf(applies(codegenContext, operation)) {

        it + HttpResponseChecksumCustomization(codegenContext, operation)

    }

}

aws/sdk-codegen/src/main/kotlin/software/amazon/smithy/rustsdk/IntegrationTestDependencies.kt

+2 −0
Original line number Diff line number Diff line
@@ -24,6 +24,7 @@ import software.amazon.smithy.rust.codegen.core.rustlang.CargoDependency.Compani 
import software.amazon.smithy.rust.codegen.core.rustlang.CargoDependency.Companion.Tracing

import software.amazon.smithy.rust.codegen.core.rustlang.CargoDependency.Companion.TracingAppender

import software.amazon.smithy.rust.codegen.core.rustlang.CargoDependency.Companion.TracingSubscriber

import software.amazon.smithy.rust.codegen.core.rustlang.CargoDependency.Companion.TracingTest

import software.amazon.smithy.rust.codegen.core.rustlang.DependencyScope

import software.amazon.smithy.rust.codegen.core.rustlang.Writable

import software.amazon.smithy.rust.codegen.core.rustlang.writable
@@ -122,5 +123,6 @@ class S3TestDependencies : LibRsCustomization() { 
            addDependency(Smol)

            addDependency(TempFile)

            addDependency(TracingAppender)

            addDependency(TracingTest)

        }

}