Deuxfleurs
/
garage
43
37
Fork 55
Code Issues 69 Pull Requests 4 Releases 29 Wiki Activity

Merge pull request 'K2V: double urlencoding' (#574) from fix-k2v-urlencoding into main
continuous-integration/drone/push Build is passing Details
continuous-integration/drone Build is passing Details 

Reviewed-on: #574
This commit is contained in:
Alex 2023-05-18 09:33:03 +00:00
parent c26a4308b4 4420db7310
commit 03efc191c1
6 changed files with 63 additions and 9 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
Cargo.lock generated
View File

@ -1891,7 +1891,7 @@ dependencies = [
[[package]]
name = "k2v-client"
version = "0.0.2"
version = "0.0.3"
dependencies = [
"base64 0.21.0",
"clap 4.2.7",
@ -1906,6 +1906,8 @@ dependencies = [
"serde_json",
"thiserror",
"tokio",
"tracing",
"tracing-subscriber",
]
[[package]]

12
Cargo.nix
View File

@ -33,7 +33,7 @@ args@{
ignoreLockHash,
}:
let
nixifiedLockHash = "b57cac5992bf6a665ce564a43f629f165268920af9035aa11fda2a8dfe9738f6";
nixifiedLockHash = "73f681ad7edf7fe539579e25d09d2ad962b9b7d64b018d421951d3cde43f5f77";
workspaceSrc = if args.workspaceSrc == null then ./. else args.workspaceSrc;
currentLockHash = builtins.hashFile "sha256" (workspaceSrc + /Cargo.lock);
lockHashIgnored = if ignoreLockHash
@ -67,7 +67,7 @@ in
garage_web = rustPackages.unknown.garage_web."0.8.2";
garage = rustPackages.unknown.garage."0.8.2";
format_table = rustPackages.unknown.format_table."0.1.1";
k2v-client = rustPackages.unknown.k2v-client."0.0.2";
k2v-client = rustPackages.unknown.k2v-client."0.0.3";
};
"registry+https://github.com/rust-lang/crates.io-index".addr2line."0.19.0" = overridableMkRustCrate (profileName: rec {
name = "addr2line";
@ -2637,15 +2637,17 @@ in
};
});
"unknown".k2v-client."0.0.2" = overridableMkRustCrate (profileName: rec {
"unknown".k2v-client."0.0.3" = overridableMkRustCrate (profileName: rec {
name = "k2v-client";
version = "0.0.2";
version = "0.0.3";
registry = "unknown";
src = fetchCrateLocal (workspaceSrc + "/src/k2v-client");
features = builtins.concatLists [
(lib.optional (rootFeatures' ? "k2v-client/clap" || rootFeatures' ? "k2v-client/cli") "clap")
(lib.optional (rootFeatures' ? "k2v-client/cli") "cli")
(lib.optional (rootFeatures' ? "k2v-client/cli" || rootFeatures' ? "k2v-client/format_table") "format_table")
(lib.optional (rootFeatures' ? "k2v-client/cli" || rootFeatures' ? "k2v-client/tracing") "tracing")
(lib.optional (rootFeatures' ? "k2v-client/cli" || rootFeatures' ? "k2v-client/tracing-subscriber") "tracing-subscriber")
];
dependencies = {
base64 = (rustPackages."registry+https://github.com/rust-lang/crates.io-index".base64."0.21.0" { inherit profileName; }).out;
@ -2661,6 +2663,8 @@ in
serde_json = (rustPackages."registry+https://github.com/rust-lang/crates.io-index".serde_json."1.0.91" { inherit profileName; }).out;
thiserror = (rustPackages."registry+https://github.com/rust-lang/crates.io-index".thiserror."1.0.38" { inherit profileName; }).out;
tokio = (rustPackages."registry+https://github.com/rust-lang/crates.io-index".tokio."1.28.0" { inherit profileName; }).out;
${ if rootFeatures' ? "k2v-client/cli" || rootFeatures' ? "k2v-client/tracing" then "tracing" else null } = (rustPackages."registry+https://github.com/rust-lang/crates.io-index".tracing."0.1.37" { inherit profileName; }).out;
${ if rootFeatures' ? "k2v-client/cli" || rootFeatures' ? "k2v-client/tracing-subscriber" then "tracing_subscriber" else null } = (rustPackages."registry+https://github.com/rust-lang/crates.io-index".tracing-subscriber."0.3.16" { inherit profileName; }).out;
};
});

40
src/api/signature/payload.rs
View File

@ -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),
"",

1
src/garage/tests/common/custom_requester.rs
View File

@ -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,

6
src/k2v-client/Cargo.toml
View File

@ -1,6 +1,6 @@
[package]
name = "k2v-client"
version = "0.0.2"
version = "0.0.3"
authors = ["Trinity Pointard <trinity.pointard@gmail.com>", "Alex Auvolat <alex@adnab.me>"]
edition = "2018"
license = "AGPL-3.0"
@ -24,10 +24,12 @@ tokio = { version = "1.0", default-features = false, features = ["rt", "rt-multi
# cli deps
clap = { version = "4.1", optional = true, features = ["derive", "env"] }
format_table = { workspace = true, optional = true }
tracing = { version = "0.1", optional = true }
tracing-subscriber = { version = "0.3", optional = true, features = ["env-filter"] }
[features]
cli = ["clap", "tokio/fs", "tokio/io-std", "format_table"]
cli = ["clap", "tokio/fs", "tokio/io-std", "tracing", "tracing-subscriber", "format_table"]
[lib]
path = "lib.rs"

9
src/k2v-client/bin/k2v-cli.rs
View File

@ -397,6 +397,15 @@ impl Filter {
#[tokio::main]
async fn main() -> Result<(), Error> {
if std::env::var("RUST_LOG").is_err() {
std::env::set_var("RUST_LOG", "warn")
}
tracing_subscriber::fmt()
.with_writer(std::io::stderr)
.with_env_filter(tracing_subscriber::filter::EnvFilter::from_default_env())
.init();
let args = Args::parse();
let region = Region::Custom {