[fix-signed-headers] aws signatures v4: don't actually check Content-Type is signed
This page of the AWS docs indicate that Content-Type should be part of the CanonicalHeaders (and therefore SignedHeaders) strings in signature calculation: https://docs.aws.amazon.com/AmazonS3/latest/API/sig-v4-header-based-auth.html However, testing with Minio Client revealed that it did not sign the Content-Type header, and therefore we broke CI by expecting it to be signed. With this commit, we don't mandate Content-Type to be signed anymore, for better compatibility with the ecosystem. Testing against the official behavior of S3 on AWS has not been done.
This commit is contained in:
parent
b8c7a560ef
commit
a36248a169
2 changed files with 9 additions and 14 deletions
|
@ -81,11 +81,9 @@ if [ -z "$SKIP_AWS" ]; then
|
||||||
echo "Invalid multipart upload"
|
echo "Invalid multipart upload"
|
||||||
exit 1
|
exit 1
|
||||||
fi
|
fi
|
||||||
|
aws s3api delete-object --bucket eprouvette --key upload
|
||||||
fi
|
fi
|
||||||
|
|
||||||
echo "OK!!"
|
|
||||||
exit 0
|
|
||||||
|
|
||||||
# S3CMD
|
# S3CMD
|
||||||
if [ -z "$SKIP_S3CMD" ]; then
|
if [ -z "$SKIP_S3CMD" ]; then
|
||||||
echo "🛠️ Testing with s3cmd"
|
echo "🛠️ Testing with s3cmd"
|
||||||
|
|
|
@ -3,7 +3,7 @@ use std::convert::TryFrom;
|
||||||
|
|
||||||
use chrono::{DateTime, Duration, NaiveDateTime, TimeZone, Utc};
|
use chrono::{DateTime, Duration, NaiveDateTime, TimeZone, Utc};
|
||||||
use hmac::Mac;
|
use hmac::Mac;
|
||||||
use hyper::header::{HeaderMap, HeaderName, HeaderValue, AUTHORIZATION, CONTENT_TYPE, HOST};
|
use hyper::header::{HeaderMap, HeaderName, HeaderValue, AUTHORIZATION, HOST};
|
||||||
use hyper::{body::Incoming as IncomingBody, Method, Request};
|
use hyper::{body::Incoming as IncomingBody, Method, Request};
|
||||||
use sha2::{Digest, Sha256};
|
use sha2::{Digest, Sha256};
|
||||||
|
|
||||||
|
@ -74,12 +74,13 @@ async fn check_standard_signature(
|
||||||
let authorization = Authorization::parse_header(request.headers())?;
|
let authorization = Authorization::parse_header(request.headers())?;
|
||||||
|
|
||||||
// Verify that all necessary request headers are included in signed_headers
|
// Verify that all necessary request headers are included in signed_headers
|
||||||
// For standard AWSv4 signatures, the following must be included:
|
// The following must be included for all signatures:
|
||||||
// - the Host header (mandatory)
|
// - the Host header (mandatory)
|
||||||
// - the Content-Type header, if it is used in the request
|
|
||||||
// - all x-amz-* headers used in the request
|
// - all x-amz-* headers used in the request
|
||||||
|
// AWS also indicates that the Content-Type header should be signed if
|
||||||
|
// it is used, but Minio client doesn't sign it so we don't check it for compatibility.
|
||||||
let signed_headers = split_signed_headers(&authorization)?;
|
let signed_headers = split_signed_headers(&authorization)?;
|
||||||
verify_signed_headers(request.headers(), &signed_headers, &[CONTENT_TYPE])?;
|
verify_signed_headers(request.headers(), &signed_headers)?;
|
||||||
|
|
||||||
let canonical_request = canonical_request(
|
let canonical_request = canonical_request(
|
||||||
service,
|
service,
|
||||||
|
@ -129,7 +130,7 @@ async fn check_presigned_signature(
|
||||||
// - the Host header (mandatory)
|
// - the Host header (mandatory)
|
||||||
// - all x-amz-* headers used in the request
|
// - all x-amz-* headers used in the request
|
||||||
let signed_headers = split_signed_headers(&authorization)?;
|
let signed_headers = split_signed_headers(&authorization)?;
|
||||||
verify_signed_headers(request.headers(), &signed_headers, &[])?;
|
verify_signed_headers(request.headers(), &signed_headers)?;
|
||||||
|
|
||||||
// The X-Amz-Signature value is passed as a query parameter,
|
// The X-Amz-Signature value is passed as a query parameter,
|
||||||
// but the signature cannot be computed from a string that contains itself.
|
// but the signature cannot be computed from a string that contains itself.
|
||||||
|
@ -229,16 +230,12 @@ fn split_signed_headers(authorization: &Authorization) -> Result<Vec<HeaderName>
|
||||||
Ok(signed_headers)
|
Ok(signed_headers)
|
||||||
}
|
}
|
||||||
|
|
||||||
fn verify_signed_headers(
|
fn verify_signed_headers(headers: &HeaderMap, signed_headers: &[HeaderName]) -> Result<(), Error> {
|
||||||
headers: &HeaderMap,
|
|
||||||
signed_headers: &[HeaderName],
|
|
||||||
extra_headers: &[HeaderName],
|
|
||||||
) -> Result<(), Error> {
|
|
||||||
if !signed_headers.contains(&HOST) {
|
if !signed_headers.contains(&HOST) {
|
||||||
return Err(Error::bad_request("Header `Host` should be signed"));
|
return Err(Error::bad_request("Header `Host` should be signed"));
|
||||||
}
|
}
|
||||||
for (name, _) in headers.iter() {
|
for (name, _) in headers.iter() {
|
||||||
if name.as_str().starts_with("x-amz-") || extra_headers.contains(name) {
|
if name.as_str().starts_with("x-amz-") {
|
||||||
if !signed_headers.contains(name) {
|
if !signed_headers.contains(name) {
|
||||||
return Err(Error::bad_request(format!(
|
return Err(Error::bad_request(format!(
|
||||||
"Header `{}` should be signed",
|
"Header `{}` should be signed",
|
||||||
|
|
Loading…
Reference in a new issue