forked from Deuxfleurs/garage
k2v signature verification: double urlencoding (see comment in source code)
This commit is contained in:
parent
c26a4308b4
commit
746b0090e4
2 changed files with 39 additions and 2 deletions
|
@ -19,7 +19,7 @@ use crate::signature::error::*;
|
|||
|
||||
pub async fn check_payload_signature(
|
||||
garage: &Garage,
|
||||
service: &str,
|
||||
service: &'static str,
|
||||
request: &Request<Body>,
|
||||
) -> Result<(Option<Key>, Option<Hash>), Error> {
|
||||
let mut headers = HashMap::new();
|
||||
|
@ -51,6 +51,7 @@ pub async fn check_payload_signature(
|
|||
};
|
||||
|
||||
let canonical_request = canonical_request(
|
||||
service,
|
||||
request.method(),
|
||||
request.uri(),
|
||||
&headers,
|
||||
|
@ -231,15 +232,50 @@ pub fn string_to_sign(datetime: &DateTime<Utc>, scope_string: &str, canonical_re
|
|||
}
|
||||
|
||||
pub fn canonical_request(
|
||||
service: &'static str,
|
||||
method: &Method,
|
||||
uri: &hyper::Uri,
|
||||
headers: &HashMap<String, String>,
|
||||
signed_headers: &str,
|
||||
content_sha256: &str,
|
||||
) -> String {
|
||||
// There seems to be evidence that in AWSv4 signatures, the path component is url-encoded
|
||||
// a second time when building the canonical request, as specified in this documentation page:
|
||||
// -> https://docs.aws.amazon.com/rolesanywhere/latest/userguide/authentication-sign-process.html
|
||||
// However this documentation page is for a specific service ("roles anywhere"), and
|
||||
// in the S3 service we know for a fact that there is no double-urlencoding, because all of
|
||||
// the tests we made with external software work without it.
|
||||
//
|
||||
// The theory is that double-urlencoding occurs for all services except S3,
|
||||
// which is what is implemented in rusoto_signature:
|
||||
// -> https://docs.rs/rusoto_signature/latest/src/rusoto_signature/signature.rs.html#464
|
||||
//
|
||||
// Digging into the code of the official AWS Rust SDK, we learn that double-URI-encoding can
|
||||
// be set or unset on a per-request basis (the signature crates, aws-sigv4 and aws-sig-auth,
|
||||
// are agnostic to this). Grepping the codebase confirms that S3 is the only API for which
|
||||
// double_uri_encode is set to false, meaning it is true (its default value) for all other
|
||||
// AWS services. We will therefore implement this behavior in Garage as well.
|
||||
//
|
||||
// Note that this documentation page, which is touted as the "authoritative reference" on
|
||||
// AWSv4 signatures, makes no mention of either single- or double-urlencoding:
|
||||
// -> https://docs.aws.amazon.com/IAM/latest/UserGuide/create-signed-request.html
|
||||
// This page of the S3 documentation does also not mention anything specific:
|
||||
// -> https://docs.aws.amazon.com/AmazonS3/latest/API/sig-v4-header-based-auth.html
|
||||
//
|
||||
// Note that there is also the issue of path normalization, which I hope is unrelated to the
|
||||
// one of URI-encoding. At least in aws-sigv4 both parameters can be set independently,
|
||||
// and rusoto_signature does not seem to do any effective path normalization, even though
|
||||
// it mentions it in the comments (same link to the souce code as above).
|
||||
// We make the explicit choice of NOT normalizing paths in the K2V API because doing so
|
||||
// would make non-normalized paths invalid K2V partition keys, and we don't want that.
|
||||
let path: std::borrow::Cow<str> = if service != "s3" {
|
||||
uri_encode(uri.path(), false).into()
|
||||
} else {
|
||||
uri.path().into()
|
||||
};
|
||||
[
|
||||
method.as_str(),
|
||||
uri.path(),
|
||||
&path,
|
||||
&canonical_query_string(uri),
|
||||
&canonical_header_string(headers, signed_headers),
|
||||
"",
|
||||
|
|
|
@ -209,6 +209,7 @@ impl<'a> RequestBuilder<'a> {
|
|||
all_headers.extend(self.unsigned_headers.clone());
|
||||
|
||||
let canonical_request = signature::payload::canonical_request(
|
||||
self.service,
|
||||
&self.method,
|
||||
&Uri::try_from(&uri).unwrap(),
|
||||
&all_headers,
|
||||
|
|
Loading…
Reference in a new issue