-
Couldn't load subscription status.
- Fork 220
Add support for Unframing SigV4 signed messages for Servers #4356
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
base: main
Are you sure you want to change the base?
Conversation
a97453e to
017463e
Compare
|
A new generated diff is ready to view.
A new doc preview is ready to view. |
017463e to
f43b21f
Compare
|
A new generated diff is ready to view.
A new doc preview is ready to view. |
|
A new generated diff is ready to view.
A new doc preview is ready to view. |
f0700da to
fedbf82
Compare
|
A new generated diff is ready to view.
A new doc preview is ready to view. |
fedbf82 to
266592d
Compare
|
A new generated diff is ready to view.
A new doc preview is ready to view. |
...amazon/smithy/rust/codegen/server/smithy/customizations/SigV4EventStreamSupportStructures.kt
Show resolved
Hide resolved
| use aws.auth#sigv4 | ||
|
|
||
| @rpcv2Cbor | ||
| @sigv4(name: "rpcv2-cbor") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there an existing test service without @sigv4 to verify the code generator works for unsigned services? If not, perhaps instead of adding this annotation directly to the .smithy file, we could add it as part of a separate test to ensure both signed and unsigned services are covered?
| } | ||
|
|
||
| override fun symbolProvider(base: RustSymbolProvider): RustSymbolProvider { | ||
| // We need access to the service shape to check for SigV4 trait, but the base interface doesn't provide it. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: We can use the function transformModel to detect whether the service uses sigV4 authentication or not.
| } | ||
|
|
||
| internal fun RustSymbolProvider.usesSigAuth(): Boolean = | ||
| ServiceIndex.of(model).getAuthSchemes(moduleProviderContext.serviceShape!!).containsKey(SigV4Trait.ID) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we need to consider SigV4ATrait as well?
| ##[derive(Debug, Clone)] | ||
| pub struct SignatureInfo { | ||
| /// The chunk signature bytes from the `:chunk-signature` header | ||
| pub chunk_signature: Vec<u8>, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
#{Vec} instead of Vec
| /// Information extracted from a signed event stream message | ||
| ##[non_exhaustive] | ||
| ##[derive(Debug, Clone)] | ||
| pub struct SignatureInfo { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I assume the handler code is responsible for determining whether the signature is sigv4 or sigv4a based by inspecting the initial request headers?
| })) | ||
| } | ||
| #{UnmarshalledMessage}::Error(err) => { | ||
| Ok(#{UnmarshalledMessage}::Error(#{SignedEventError}::Event(err))) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should SignedEventError::Event include the signature when the error message itself was signed? Currently the signature is being extracted but then discarded for error messages.
| match header.name().as_str() { | ||
| ":chunk-signature" => { | ||
| if let #{HeaderValue}::ByteArray(bytes) = header.value() { | ||
| chunk_signature = Some(bytes.as_ref().to_vec()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are there limits on event stream message/chunk sizes to prevent memory exhaustion attacks? or is this the service's responsibility via Hyper configuration? Are individual header values size-limited? or should we enforce some check here that the value is maximum 256 bytes long before we .to_vec() it?
| Err(err) => Err(err), | ||
| } | ||
| } | ||
| Ok(MaybeSignedMessage::Unsigned) => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If a service has the @sigv4 trait applied to it:
- Can a client send unsigned messages?
- Does the service handler have to raise an error if it doesn't want to deal with unsigned messages?
SigV4 Event Stream Support for Server SDK
Problem
Clients wrap event stream messages in SigV4 envelopes with signature headers (
:chunk-signature,:date), but servers couldn't parse these signed messages because they expected the raw event shape, not the envelope.Solution
Added server-side SigV4 event stream unsigning support that automatically extracts inner messages from signed envelopes while maintaining compatibility with unsigned messages.
Implementation
Type System Changes
Event stream types are wrapped to handle both signed and unsigned messages:
Receiver<Events, Error>→Receiver<SignedEvent<Events>, SignedEventError<Error>>SignedEvent<T>provides access to both the inner message and signature informationSignedEventError<E>wraps both extraction errors and underlying event errorsRuntime Components
SigV4Unmarshaller: Wraps the base event stream unmarshaller to handle SigV4 extraction:
Code Generation Integration
SigV4EventStreamDecorator:
@sigv4trait and event streamsSigV4EventStreamSymbolProviderSignedEvent,SigV4Unmarshaller, etc.)HTTP Binding Customization:
BeforeCreatingEventStreamReceiversection toHttpBindingGeneratorReceivercreationlet unmarshaller = SigV4Unmarshaller::new(unmarshaller);Usage
Server handlers receive
SignedEvent<T>and extract the inner message:Testing
test_sigv4_signed_event_streamthat sends SigV4-wrapped eventsArchitecture Benefits
@sigv4trait and event streamsChecklist
.changelogdirectory, specifying "client," "server," or both in theapplies_tokey..changelogdirectory, specifying "aws-sdk-rust" in theapplies_tokey.By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.