aws-sdk-rust icon indicating copy to clipboard operation
aws-sdk-rust copied to clipboard

[request]: aws-sig-auth - Support for `omit_session_token` for `iotdevicegateway`

Open MathiasKoch opened this issue 3 years ago • 11 comments
trafficstars

A note for the community

Community Note

  • Please vote on this issue by adding a 👍 reaction to the original issue to help the community and maintainers prioritize this request
  • Please do not leave "+1" or "me too" comments, they generate extra noise for issue followers and do not help prioritize the request
  • If you are interested in working on this issue, please leave a comment

Tell us about your request

I would like support for omit_session_token here: https://github.com/awslabs/smithy-rs/blob/ab0306b2a178ccd236b217c06de684172b667f70/aws/rust-runtime/aws-sig-auth/src/signer.rs#L88-L96 and here: https://github.com/awslabs/aws-sdk-rust/blob/63fc7ab18c9fdd7b6f981d5d1bf223ced0225e05/sdk/aws-sig-auth/src/signer.rs#L88-L96

And obviously in the underlying implementation in aws-sigv4

Tell us about the problem you're trying to solve.

It is currently not possible to create a presigned SigV4 request for certain services, due to the fact that they need to exclude the X-Amz-Security-Token from the query string used for calculating the signature.

As per the documentation note here: http://docs.aws.amazon.com/general/latest/gr/sigv4-add-signature-to-request.html.

When you add the X-Amz-Security-Token parameter to the query string, some services require that you include this parameter in the canonical (signed) request. For other services, you add this parameter at the end, after you calculate the signature. For details, see the API reference documentation for that service.

The only service i have yet to encounter is iotdevicegateway, but judging from AWS's documentation note it appears as if there could be more.

Are you currently working around this issue?

We are currently using a patched version of rusoto for this, which is the last thing blocking us from migrating fully to aws-sdk-rust

Additional context

This was also part of the original feature request for AWS IoT in https://github.com/awslabs/aws-sdk-rust/issues/115, but seems to have been missed.

MathiasKoch avatar Mar 01 '22 14:03 MathiasKoch

ah thanks! We tried to figure out why this was actually needed but couldn't reproduce it. Presigning was the missing link. We'll get this fixed (and would also happily accept a PR)

rcoh avatar Mar 01 '22 15:03 rcoh

Awesome! I will see if i can find the time to open a PR before you get it fixed, but i have a lot on my plate right now :)

Just to be sure it would be a PR to smithy-rs, and one for aws-sdk-rust, changing smithy-rs/aws-sig-auth, smithy-rs/aws-sigv4, aws-sdk-rust/aws-sig-auth, aws-sdk-rust/aws-sigv4?

Anything more that needs to be affected?

MathiasKoch avatar Mar 01 '22 15:03 MathiasKoch

I am a bit unsure if the aws_sdk_iotdataplane client uses Sigv4 presigned connections, but if it does this might be the underlying issue behind https://github.com/awslabs/aws-sdk-rust/issues/468#issuecomment-1055389607 as well?

MathiasKoch avatar Mar 01 '22 16:03 MathiasKoch

yep correct, changes would go here: https://github.com/awslabs/smithy-rs/tree/main/aws/rust-runtime/aws-sigv4 & in the sibling folder aws-sig-auth.

No PR for aws-sdk-rust this is purely a product of codegen (after you merge to smithy-rs, you'll see your changes in the next branch of this repo in a couple of hours)

rcoh avatar Mar 01 '22 16:03 rcoh

Awesome! That definitely makes it a bit easier :+1:

MathiasKoch avatar Mar 01 '22 16:03 MathiasKoch

currently aws_sdk_iotdataplane doesn't model any presigned operations, but that is also not a huge lift I think. There is actually no universal model of what operations can be presigned, so generating presigned operations is accomplished by manually augmenting the model: https://github.com/awslabs/smithy-rs/blob/main/aws/sdk-codegen/src/main/kotlin/software/amazon/smithy/rustsdk/AwsPresigningDecorator.kt#L63-L80

We would also need to add a configuration to the PresignedTrait to mark operations as omitting the X-Amz-Security-Token (In the meantime though, you can always work around the issue by manually using aws-sig-auth once it supports omit_session_token)

rcoh avatar Mar 01 '22 16:03 rcoh

(In the meantime though, you can always work around the issue by manually using aws-sig-auth once it supports omit_session_token)

Yeah, that is exactly what i am doing now:

let region_provider = RegionProviderChain::default_provider().or_else("eu-west-1");
let credentials_provider = CredentialsProviderChain::default_provider().await;
let region = region_provider
    .region()
    .await
    .ok_or(anyhow!("No region?"))?;

let config = aws_config::from_env().region(region_provider).load().await;
let iot_client = aws_sdk_iot::Client::new(&config);

let endpoint_address = iot_client
    .describe_endpoint()
    .endpoint_type(String::from("iot:Data-ATS"))
    .send()
    .await?
    .endpoint_address
    .ok_or(anyhow!("No endpoint address?"))?;

let port = 443;

let request_config = RequestConfig {
    request_ts: SystemTime::now(),
    region: &SigningRegion::from(region),
    service: &SigningService::from_static("iotdevicegateway"),
    payload_override: None,
};

let uri = Uri::builder()
    .scheme("wss")
    .authority(format!("{}:{}", endpoint_address, port))
    .path_and_query("/mqtt")
    .build()?;

let mut request = Request::builder()
    .method("GET")
    .uri(uri)
    .body(SdkBody::empty())?;

let mut operation_config = OperationSigningConfig::default_config();
operation_config.signature_type = HttpSignatureType::HttpRequestQueryParams;
operation_config.expires_in = Some(Duration::from_secs(86400));

// FIXME: Currently missing! (https://github.com/awslabs/aws-sdk-rust/issues/477)
// operation_config.signing_options.omit_session_token = true;

SigV4Signer::new()
    .sign(
        &operation_config,
        &request_config,
        &credentials_provider.provide_credentials().await?,
        &mut request,
    )
    .map_err(|e| anyhow!("{:?}", e))?;

let mut opts = MqttOptions::new(
    format!("bbctl-{}", uuid::Uuid::new_v4()),
    request.uri().to_string(),
    port,
);

MathiasKoch avatar Mar 01 '22 16:03 MathiasKoch

you might be able to work around it by dropping the header, signing, then re-adding the header

rcoh avatar Mar 01 '22 16:03 rcoh

@itsnottoolate you are more than welcome to see if you can make a PR tackling this 👍

MathiasKoch avatar Mar 02 '22 15:03 MathiasKoch

Any updates on this issue?

MathiasKoch avatar May 17 '22 05:05 MathiasKoch

Hey @MathiasKoch, We don't have any updates to share on this issue as we haven't gotten around to looking at it. We prioritize issues based on the number of upvotes they receive (counted in 👍s on the initial issue description.) If you'd like us to get to this issue sooner, encourage people to upvote it. You can check here to see which issues are currently our highest priority.

Velfi avatar May 17 '22 16:05 Velfi

This was fixed by https://github.com/awslabs/smithy-rs/pull/2473

MathiasKoch avatar Jun 22 '23 10:06 MathiasKoch

⚠️COMMENT VISIBILITY WARNING⚠️

Comments on closed issues are hard for our team to see. If you need more assistance, please either tag a team member or open a new issue that references this one. If you wish to keep having a conversation with other community members under this issue feel free to do so.

github-actions[bot] avatar Jun 22 '23 10:06 github-actions[bot]