-
Notifications
You must be signed in to change notification settings - Fork 181
RUST-1529 Use AWS SDK for sigv4 signing #1438
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?
Changes from 44 commits
bb11533
3d78f80
1f4c590
6fe85bc
b6e1d9a
18c3bc8
915b087
f5a65af
e2a5080
cf3f3a5
8df943f
6b9d129
e99fff2
73d3a82
dd94b48
e10a3ba
f365624
408ca2f
f528c00
678cdcb
90d5156
eac9bcf
cb26899
725d373
b4c06e8
46e171d
0cd8dfa
a482936
2416644
ee4f602
6890532
f2a4a62
d41a5b7
cd470fb
f841841
c64a610
06183ea
e5549b2
12b9c33
9a7d14c
c1b3f82
55fb305
2d71fc4
2bffc01
9c96dde
070b5b5
dde6a5c
92d0cd3
7018a69
923cee4
2fbd690
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -31,6 +31,15 @@ use aws_config::BehaviorVersion; | |
#[cfg(feature = "aws-auth")] | ||
use aws_credential_types::{provider::ProvideCredentials, Credentials}; | ||
|
||
#[cfg(feature = "aws-auth")] | ||
use aws_sigv4::{ | ||
http_request::{sign, SignableBody, SignableRequest, SigningSettings}, | ||
sign::v4::SigningParams, | ||
}; | ||
|
||
#[cfg(feature = "aws-auth")] | ||
use http::Request; | ||
|
||
const AWS_ECS_IP: &str = "169.254.170.2"; | ||
const AWS_EC2_IP: &str = "169.254.169.254"; | ||
const AWS_LONG_DATE_FMT: &str = "%Y%m%dT%H%M%SZ"; | ||
|
@@ -117,24 +126,32 @@ async fn authenticate_stream_inner( | |
let creds = get_aws_credentials(credential).await.map_err(|e| { | ||
Error::authentication_error(MECH_NAME, &format!("failed to get creds: {e}")) | ||
})?; | ||
let aws_credential = AwsCredential::from_sdk_creds(creds); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In a previous PR, we introduced |
||
|
||
let date = Utc::now(); | ||
|
||
let authorization_header = aws_credential.compute_authorization_header( | ||
// Generate authorization header using original implementation without AWS SDK | ||
// let authorization_header = aws_credential.compute_authorization_header( | ||
// date, | ||
// &server_first.sts_host, | ||
// &server_first.server_nonce, | ||
// )?; | ||
|
||
// let mut client_second_payload = doc! { | ||
// "a": authorization_header, | ||
// "d": date.format(AWS_LONG_DATE_FMT).to_string(), | ||
// }; | ||
|
||
// if let Some(security_token) = aws_credential.session_token { | ||
// client_second_payload.insert("t", security_token); | ||
// } | ||
|
||
let client_second_payload = compute_aws_sigv4_payload( | ||
creds, | ||
date, | ||
&server_first.sts_host, | ||
&server_first.server_nonce, | ||
)?; | ||
|
||
let mut client_second_payload = doc! { | ||
"a": authorization_header, | ||
"d": date.format(AWS_LONG_DATE_FMT).to_string(), | ||
}; | ||
|
||
if let Some(security_token) = aws_credential.session_token { | ||
client_second_payload.insert("t", security_token); | ||
} | ||
) | ||
.await?; | ||
|
||
let mut client_second_payload_bytes = vec![]; | ||
client_second_payload.to_writer(&mut client_second_payload_bytes)?; | ||
|
@@ -197,6 +214,118 @@ pub(crate) async fn get_aws_credentials(credential: &Credential) -> Result<Crede | |
} | ||
} | ||
|
||
pub async fn compute_aws_sigv4_payload( | ||
JamieTsai1024 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
creds: Credentials, | ||
date: DateTime<Utc>, | ||
host: &str, | ||
server_nonce: &[u8], | ||
) -> Result<Document> { | ||
let region = if host == "sts.amazonaws.com" { | ||
"us-east-1" | ||
} else { | ||
let parts: Vec<_> = host.split('.').collect(); | ||
parts.get(1).copied().unwrap_or("us-east-1") | ||
}; | ||
|
||
let url = format!("https://{}", host); | ||
let date_str = date.format("%Y%m%dT%H%M%SZ").to_string(); | ||
let body_str = "Action=GetCallerIdentity&Version=2011-06-15"; | ||
let body_bytes = body_str.as_bytes(); | ||
let nonce_b64 = base64::encode(server_nonce); | ||
|
||
// Create the HTTP request | ||
let mut builder = Request::builder() | ||
.method("POST") | ||
.uri(&url) | ||
.header("host", host) | ||
.header("content-type", "application/x-www-form-urlencoded") | ||
.header("content-length", body_bytes.len()) | ||
.header("x-amz-date", &date_str) | ||
.header("x-mongodb-gs2-cb-flag", "n") | ||
.header("x-mongodb-server-nonce", &nonce_b64); | ||
|
||
if let Some(token) = creds.session_token() { | ||
builder = builder.header("x-amz-security-token", token); | ||
} | ||
|
||
let mut request = builder.body(body_str.to_string()).map_err(|e| { | ||
Error::authentication_error(MECH_NAME, &format!("Failed to build request: {e}")) | ||
})?; | ||
|
||
let service = "sts"; | ||
let identity = creds.into(); | ||
|
||
// Set up signing parameters | ||
let signing_settings = SigningSettings::default(); | ||
let signing_params = SigningParams::builder() | ||
.identity(&identity) | ||
.region(region) | ||
.name(service) | ||
.time(date.into()) | ||
.settings(signing_settings) | ||
.build() | ||
.map_err(|e| { | ||
Error::authentication_error(MECH_NAME, &format!("Failed to build signing params: {e}")) | ||
})? | ||
.into(); | ||
|
||
let signable_request = SignableRequest::new( | ||
request.method().as_str(), | ||
request.uri().to_string(), | ||
request | ||
.headers() | ||
.iter() | ||
.map(|(k, v)| { | ||
let value = std::str::from_utf8(v.as_bytes()).map_err(|_| { | ||
Error::authentication_error( | ||
MECH_NAME, | ||
"Failed to convert header value to valid UTF-8", | ||
) | ||
})?; | ||
Ok((k.as_str(), value)) | ||
}) | ||
.filter_map(Result::ok), | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This will ignore any failures that occur when converting the strings and proceed with authentication. This would likely cause the signature to be computed incorrectly, and the reported error might be confusing for users to parse. Instead, I suggest doing the following to propagate an error here: let headers: Result<Vec<_>> = request
.headers()
.iter()
.map(|(k, v)| {
let v = v.to_str().map_err(|_| {
Error::authentication_error(
MECH_NAME,
"Failed to convert header value to valid UTF-8",
)
})?;
Ok((k.as_str(), v))
})
.collect(); This uses a handy iterator trick: if an iterator is mapped into We then need to return an error if needed using headers?.into_iter() This should make what happened more clear if something goes wrong with the headers. |
||
SignableBody::Bytes(request.body().as_bytes()), | ||
) | ||
.map_err(|e| { | ||
Error::authentication_error(MECH_NAME, &format!("Failed to create SignableRequest: {e}")) | ||
})?; | ||
|
||
let (signing_instructions, _signature) = sign(signable_request, &signing_params) | ||
.map_err(|e| Error::authentication_error(MECH_NAME, &format!("Signing failed: {e}")))? | ||
.into_parts(); | ||
Comment on lines
+294
to
+296
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: since we're not using the returned signature, I think this can call There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks Isabel! I tried switching to The There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ah I missed the reference vs. owned value distinction, feel free to leave as-is There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. sounds good, thanks! |
||
signing_instructions.apply_to_request_http1x(&mut request); | ||
|
||
let headers = request.headers(); | ||
let authorization_header = headers | ||
.get("authorization") | ||
.ok_or_else(|| Error::authentication_error(MECH_NAME, "Missing authorization header"))? | ||
.to_str() | ||
.map_err(|e| { | ||
Error::authentication_error(MECH_NAME, &format!("Invalid header value: {e}")) | ||
})?; | ||
|
||
let token_header = headers | ||
.get("x-amz-security-token") | ||
.map(|v| { | ||
v.to_str().map_err(|e| { | ||
Error::authentication_error(MECH_NAME, &format!("Invalid token header: {e}")) | ||
}) | ||
}) | ||
.transpose()?; | ||
|
||
let mut payload = doc! { | ||
"a": authorization_header, | ||
"d": date_str, | ||
}; | ||
|
||
if let Some(token) = token_header { | ||
payload.insert("t", token); | ||
} | ||
|
||
Ok(payload) | ||
} | ||
|
||
/// Contains the credentials for MONGODB-AWS authentication. | ||
// RUST-1529 note: dead_code tag added to avoid unused warnings on expiration field | ||
#[allow(dead_code)] | ||
|
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.
will uncomment once PR is approved