Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

rfc to disable payload signing #3583

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions design/src/SUMMARY.md
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,7 @@
- [RFC-0041: Improve client error ergonomics](./rfcs/rfc0041_improve_client_error_ergonomics.md)
- [RFC-0042: File-per-change changelog](./rfcs/rfc0042_file_per_change_changelog.md)
- [RFC-0043: Identity Cache Partitions](./rfcs/rfc0043_identity_cache_partitions.md)
- [RFC-0044: Disable payload signing](./rfcs/rfc0044_disable_payload_signing.md)

- [Contributing](./contributing/overview.md)
- [Writing and debugging a low-level feature that relies on HTTP](./contributing/writing_and_debugging_a_low-level_feature_that_relies_on_HTTP.md)
3 changes: 1 addition & 2 deletions design/src/rfcs/overview.md
Original file line number Diff line number Diff line change
Expand Up @@ -53,5 +53,4 @@
- [RFC-0041: Improve client error ergonomics](./rfc0041_improve_client_error_ergonomics.md)
- [RFC-0042: File-per-change changelog](./rfc0042_file_per_change_changelog.md)
- [RFC-0043: Identity Cache Partitions](./rfc0043_identity_cache_partitions.md)

>>>>>>> 4a8757c23 (add RFC to fix identity cache partitioning and default cache behaviors)
- [RFC-0044: Disable payload signing](./rfc0044_disable_payload_signing.md)
86 changes: 86 additions & 0 deletions design/src/rfcs/rfc0044_disable_payload_signing.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,86 @@
RFC: Disable Payload Signing
=============================

> Status: RFC
>
> Applies to: client

For a summarized list of proposed changes, see the [Changes Checklist](#changes-checklist) section.

This RFC proposes a way for users to disable payload signing for requests with a streaming binary payload.


The user experience if this RFC is implemented
----------------------------------------------


In the current version of the SDK, users are unable to control how a request is signed.
Most requests must include a hash of the payload as part of the signature. However, requests with a
[streaming](https://smithy.io/2.0/spec/streaming.html#smithy-api-streaming-trait) `blob` shape as the payload can
sometimes (e.g. S3 `PutObject`) choose to skip payload hashing and exclude it from the signature. When calculating the
signature the body hash is instead set to the literal constant `UNSIGNED-PAYLOAD`. This has the advantage of not paying
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What services does this actually work for? I was under the impression this only worked for S3.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't have a comprehensive list and I doubt we can definitively know without actually trying to send an unsigned payload. There are only something like ~30 or less binary streaming operations across all of AWS services IIRC.

We could limit this to just S3 for now if we wanted to be conservative (and likely the only use case most will care about).

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I think we should allow list this until we've verified it actually works in other services.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm good with that, will update the RFC to mention this.

the cost of reading the payload into memory or hashing it at the cost of giving up some integrity checks.
This may be a desired tradeoff though when performance is the top concern or when other checksums are already being used
(e.g. [S3 checksums](https://aws.amazon.com/blogs/aws/new-additional-checksum-algorithms-for-amazon-s3/)).



Once this RFC is implemented, users will have the ability to selectively disable payload signing by customizing the
request.

```rust,ignore
let resp = s3.put_object()
.bucket(bucket_name)
.key(key_name)
.body(body)
.customize()
.disable_payload_signing();
```

This new `disable_payload_signing()` method will only be added selectively to requests with a binary stream as the
payload.


How to actually implement this RFC
----------------------------------

Each service client operation can be customized to (e.g.) override specific configuration for just that operation
invocation. See [RFC-0017](./rfc0017_customizable_client_operations.html)
for more details.

In each generated service a [`CustomizableOperation<T, E, B>`](https://github.com/awslabs/aws-sdk-rust/blob/release-2024-04-11/sdk/s3/src/client/customize.rs#L3)
is generated. These are exposed as part of the [fluent builder](https://github.com/awslabs/aws-sdk-rust/blob/release-2024-04-11/sdk/s3/src/operation/put_object/builders.rs#L152) for each operation.


A new `impl` block specific to only binary streaming requests will be generated that
exposes this new `disable_payload_signing()` method.

```rust,ignore

impl CustomizableOperation<
crate::operation::put_object::PutObjectOutput,
crate::operation::put_object::PutObjectError,
PutObjectFluentBuilder
> {

/// Disable payload signing for a single operation invocation.
fn disable_payload_signing(mut self) -> Self {
// register a plugin that overrides the default signing parameters
// and sets `signing_options.payload_override` to `SignableBody::UnsignedPayload`
self.runtime_plugins
.push(::aws_runtime::auth::DisablePayloadSigningPlugin::new());
}
}

```



Changes checklist
-----------------

- [ ] Create a new runtime plugin that disables payload signing
- [ ] Update codegen to generate a specific `impl` block for requests with a `@streaming blob` payload that
registers the new plugin
- [ ] Add new integration test(s)
- [ ] Update developer guide(s)