Unverified Commit 39c6476f authored by Zelda Hessler's avatar Zelda Hessler Committed by GitHub
Browse files

Use `Identity` instead of `Credentials` in signing code (#2913) 

This PR replaces the access_key, secret_key, and session token fields of
signing params with the Orchestrator's `Identity` type.

## Checklist
<!--- If a checkbox below is not applicable, then please DELETE it
rather than leaving it unchecked -->
- [x] I have updated `CHANGELOG.next.toml` if I made changes to the
smithy-rs codegen or runtime crates
- [x] I have updated `CHANGELOG.next.toml` if I made changes to the AWS
SDK, generated SDK code, or SDK runtime crates

----

_By submitting this pull request, I confirm that you can use, modify,
copy, and redistribute this contribution, under the terms of your
choice._
parent 87d601b4

CHANGELOG.next.toml

+9 −0
Original line number Diff line number Diff line
@@ -51,3 +51,12 @@ message = "In sigV4-related code, rename 'signing service' to 'signing name'. Th 
references = ["smithy-rs#2911"]

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

author = "Velfi"



[[aws-sdk-rust]]

message = """

All versions of SigningParams have been updated to contain an [`Identity`](https://docs.rs/aws-smithy-runtime-api/latest/aws_smithy_runtime_api/client/identity/struct.Identity.html)

as opposed to AWS credentials in `&str` form. [Read more](https://github.com/awslabs/aws-sdk-rust/discussions/868).

"""

references = ["smithy-rs#2913"]

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

author = "Velfi"

aws/rust-runtime/aws-credential-types/Cargo.toml

+1 −3
Original line number Diff line number Diff line
@@ -14,6 +14,7 @@ test-util = [] 
[dependencies]

aws-smithy-async = { path = "../../../rust-runtime/aws-smithy-async" }

aws-smithy-types = { path = "../../../rust-runtime/aws-smithy-types" }

aws-smithy-runtime-api = { path = "../../../rust-runtime/aws-smithy-runtime-api", features = ["client"] }

fastrand = "2.0.0"

tokio = { version = "1.23.1", features = ["sync"] }

tracing = "0.1"
@@ -28,9 +29,6 @@ env_logger = "0.9.0" 


tokio = { version = "1.23.1", features = ["full", "test-util", "rt"] }

tracing-test = "0.2.4"

# TODO(https://github.com/awslabs/smithy-rs/issues/2619): Remove this

# workaround once the fixed is upstreamed.

regex = { version = "1.0", features = ["unicode-case", "unicode-perl"] }



[package.metadata.docs.rs]

all-features = true

aws/rust-runtime/aws-credential-types/src/credentials_impl.rs

+34 −12
Original line number Diff line number Diff line
@@ -10,6 +10,8 @@ use std::sync::Arc; 
use std::time::{SystemTime, UNIX_EPOCH};

use zeroize::Zeroizing;



use aws_smithy_runtime_api::client::identity::Identity;



/// AWS SDK Credentials

///

/// An opaque struct representing credentials that may be used in an AWS SDK, modeled on
@@ -140,18 +142,6 @@ impl Credentials { 
        )

    }



    /// Creates a test `Credentials`.

    #[cfg(feature = "test-util")]

    pub fn for_tests() -> Self {

        Self::new(

            "ANOTREAL",

            "notrealrnrELgWzOk3IfjzDKtFBhDby",

            Some("notarealsessiontoken".to_string()),

            None,

            "test",

        )

    }



    /// Returns the access key ID.

    pub fn access_key_id(&self) -> &str {

        &self.0.access_key_id
@@ -178,6 +168,38 @@ impl Credentials { 
    }

}



#[cfg(feature = "test-util")]

impl Credentials {

    /// Creates a test `Credentials` with no session token.

    pub fn for_tests() -> Self {

        Self::new(

            "ANOTREAL",

            "notrealrnrELgWzOk3IfjzDKtFBhDby",

            None,

            None,

            "test",

        )

    }



    /// Creates a test `Credentials` that include a session token.

    pub fn for_tests_with_session_token() -> Self {

        Self::new(

            "ANOTREAL",

            "notrealrnrELgWzOk3IfjzDKtFBhDby",

            Some("notarealsessiontoken".to_string()),

            None,

            "test",

        )

    }

}



impl From<Credentials> for Identity {

    fn from(val: Credentials) -> Self {

        let expiry = val.expiry();

        Identity::new(val, expiry)

    }

}



#[cfg(test)]

mod test {

    use crate::Credentials;

aws/rust-runtime/aws-inlineable/tests/middleware_e2e_test.rs

+3 −2
Original line number Diff line number Diff line
@@ -89,8 +89,9 @@ fn test_operation() -> Operation<TestOperationParser, AwsResponseRetryClassifier 
        ));

        aws_http::auth::set_credentials_cache(

            conf,

            CredentialsCache::lazy()

                .create_cache(SharedCredentialsProvider::new(Credentials::for_tests())),

            CredentialsCache::lazy().create_cache(SharedCredentialsProvider::new(

                Credentials::for_tests_with_session_token(),

            )),

        );

        conf.insert(SigningRegion::from_static("test-region"));

        conf.insert(OperationSigningConfig::default_config());

aws/rust-runtime/aws-runtime/src/auth/sigv4.rs

+32 −29
Original line number Diff line number Diff line
@@ -217,12 +217,16 @@ impl SigV4Signer { 


    fn signing_params<'a>(

        settings: SigningSettings,

        credentials: &'a Credentials,

        identity: &'a Identity,

        operation_config: &'a SigV4OperationSigningConfig,

        request_timestamp: SystemTime,

    ) -> Result<SigningParams<'a>, SigV4SigningError> {

        let creds = identity

            .data::<Credentials>()

            .ok_or_else(|| SigV4SigningError::WrongIdentityType(identity.clone()))?;



        if let Some(expires_in) = settings.expires_in {

            if let Some(creds_expires_time) = credentials.expiry() {

            if let Some(creds_expires_time) = creds.expiry() {

                let presigned_expires_time = request_timestamp + expires_in;

                if presigned_expires_time > creds_expires_time {

                    tracing::warn!(EXPIRATION_WARNING);
@@ -230,9 +234,8 @@ impl SigV4Signer { 
            }

        }



        let mut builder = SigningParams::builder()

            .access_key(credentials.access_key_id())

            .secret_key(credentials.secret_access_key())

        let builder = SigningParams::builder()

            .identity(identity)

            .region(

                operation_config

                    .region
@@ -249,7 +252,6 @@ impl SigV4Signer { 
            )

            .time(request_timestamp)

            .settings(settings);

        builder.set_security_token(credentials.session_token());

        Ok(builder.build().expect("all required fields set"))

    }
@@ -324,18 +326,18 @@ impl Signer for SigV4Signer { 
            Self::extract_operation_config(auth_scheme_endpoint_config, config_bag)?;

        let request_time = runtime_components.time_source().unwrap_or_default().now();



        let credentials = if let Some(creds) = identity.data::<Credentials>() {

            creds

        } else if operation_config.signing_options.signing_optional {

        if identity.data::<Credentials>().is_none() {

            if operation_config.signing_options.signing_optional {

                tracing::debug!("skipped SigV4 signing since signing is optional for this operation and there are no credentials");

                return Ok(());

            } else {

                return Err(SigV4SigningError::WrongIdentityType(identity.clone()).into());

            }

        };



        let settings = Self::settings(&operation_config);

        let signing_params =

            Self::signing_params(settings, credentials, &operation_config, request_time)?;

            Self::signing_params(settings, identity, &operation_config, request_time)?;



        let (signing_instructions, _signature) = {

            // A body that is already in memory can be signed directly. A body that is not in memory
@@ -382,7 +384,7 @@ impl Signer for SigV4Signer { 
                signer_sender

                    .send(Box::new(SigV4MessageSigner::new(

                        _signature,

                        credentials.clone(),

                        identity.clone(),

                        Region::new(signing_params.region().to_string()).into(),

                        signing_params.name().to_string().into(),

                        time_source,
@@ -397,11 +399,11 @@ impl Signer for SigV4Signer { 


#[cfg(feature = "event-stream")]

mod event_stream {

    use aws_credential_types::Credentials;

    use aws_sigv4::event_stream::{sign_empty_message, sign_message};

    use aws_sigv4::SigningParams;

    use aws_smithy_async::time::SharedTimeSource;

    use aws_smithy_eventstream::frame::{Message, SignMessage, SignMessageError};

    use aws_smithy_runtime_api::client::identity::Identity;

    use aws_types::region::SigningRegion;

    use aws_types::SigningName;
@@ -409,7 +411,7 @@ mod event_stream { 
    #[derive(Debug)]

    pub(super) struct SigV4MessageSigner {

        last_signature: String,

        credentials: Credentials,

        identity: Identity,

        signing_region: SigningRegion,

        signing_name: SigningName,

        time: SharedTimeSource,
@@ -418,14 +420,14 @@ mod event_stream { 
    impl SigV4MessageSigner {

        pub(super) fn new(

            last_signature: String,

            credentials: Credentials,

            identity: Identity,

            signing_region: SigningRegion,

            signing_name: SigningName,

            time: SharedTimeSource,

        ) -> Self {

            Self {

                last_signature,

                credentials,

                identity,

                signing_region,

                signing_name,

                time,
@@ -433,14 +435,12 @@ mod event_stream { 
        }



        fn signing_params(&self) -> SigningParams<'_, ()> {

            let mut builder = SigningParams::builder()

                .access_key(self.credentials.access_key_id())

                .secret_key(self.credentials.secret_access_key())

            let builder = SigningParams::builder()

                .identity(&self.identity)

                .region(self.signing_region.as_ref())

                .name(self.signing_name.as_ref())

                .time(self.time.now())

                .settings(());

            builder.set_security_token(self.credentials.session_token());

            builder.build().unwrap()

        }

    }
@@ -467,9 +467,11 @@ mod event_stream { 


    #[cfg(test)]

    mod tests {

        use super::*;

        use crate::auth::sigv4::event_stream::SigV4MessageSigner;

        use aws_credential_types::Credentials;

        use aws_smithy_async::time::SharedTimeSource;

        use aws_smithy_eventstream::frame::{HeaderValue, Message, SignMessage};



        use aws_types::region::Region;

        use aws_types::region::SigningRegion;

        use aws_types::SigningName;
@@ -484,7 +486,7 @@ mod event_stream { 
            let region = Region::new("us-east-1");

            let mut signer = check_send_sync(SigV4MessageSigner::new(

                "initial-signature".into(),

                Credentials::for_tests(),

                Credentials::for_tests_with_session_token().into(),

                SigningRegion::from(region),

                SigningName::from_static("transcribe"),

                SharedTimeSource::new(UNIX_EPOCH + Duration::new(1611160427, 0)),
@@ -534,13 +536,14 @@ mod tests { 
        let mut settings = SigningSettings::default();

        settings.expires_in = Some(creds_expire_in - Duration::from_secs(10));



        let credentials = Credentials::new(

        let identity = Credentials::new(

            "test-access-key",

            "test-secret-key",

            Some("test-session-token".into()),

            Some(now + creds_expire_in),

            "test",

        );

        )

        .into();

        let operation_config = SigV4OperationSigningConfig {

            region: Some(SigningRegion::from_static("test")),

            service: Some(SigningName::from_static("test")),
@@ -555,13 +558,13 @@ mod tests { 
                payload_override: None,

            },

        };

        SigV4Signer::signing_params(settings, &credentials, &operation_config, now).unwrap();

        SigV4Signer::signing_params(settings, &identity, &operation_config, now).unwrap();

        assert!(!logs_contain(EXPIRATION_WARNING));



        let mut settings = SigningSettings::default();

        settings.expires_in = Some(creds_expire_in + Duration::from_secs(10));



        SigV4Signer::signing_params(settings, &credentials, &operation_config, now).unwrap();

        SigV4Signer::signing_params(settings, &identity, &operation_config, now).unwrap();

        assert!(logs_contain(EXPIRATION_WARNING));

    }