mirror of
https://github.com/versity/versitygw.git
synced 2026-08-17 20:56:21 +00:00
fix: enforce required SignedHeaders validation for SigV4 requests
Validate required signed headers for both Authorization-header SigV4 requests and presigned URLs. The required signed header set is now `host` plus every incoming header with the `x-amz-` prefix. During request reconstruction, signed headers and explicitly ignored headers are copied into the generated request used for signature verification. If an incoming `x-amz-*` header is present but missing from the client-provided `SignedHeaders`, return `AccessDenied` with a `HeadersNotSigned` field. The `host` header remains part of the canonical request and signed header calculation. Previously, a client could sign a request without an S3 control header and then add that header after signing. For example, a presigned `PUT` URL could be generated with only `host` signed, then the actual request could include an unsigned `X-Amz-Tagging` or `X-Amz-Copy-Source` header. Because the verifier reconstructed the request only from `SignedHeaders`, that extra header was omitted from signature calculation and could pass authentication even though it changed the request semantics. This is now rejected with `AccessDenied`. Expose v4 helper methods for checking required and ignored headers, and update canonical header signing so ignored headers can still be included when a client explicitly lists them in `SignedHeaders`, while `Authorization` remains excluded from signature calculation.
This commit is contained in:
@@ -59,7 +59,7 @@ func CheckValidSignature(ctx *fiber.Ctx, auth AuthData, secret, checksum string,
|
||||
// Create a new http request instance from fasthttp request
|
||||
req, err := createHttpRequestFromCtx(ctx, signedHdrs, contentLen)
|
||||
if err != nil {
|
||||
return "", fmt.Errorf("create http request from context: %w", err)
|
||||
return "", err
|
||||
}
|
||||
|
||||
signer := v4.NewSigner()
|
||||
|
||||
@@ -54,7 +54,7 @@ func CheckPresignedSignature(ctx *fiber.Ctx, auth AuthData, secret string) error
|
||||
// Create a new http request instance from fasthttp request
|
||||
req, err := createPresignedHttpRequestFromCtx(ctx, signedHdrs, contentLength)
|
||||
if err != nil {
|
||||
return fmt.Errorf("create http request from context: %w", err)
|
||||
return err
|
||||
}
|
||||
|
||||
date, _ := time.Parse(iso8601Format, auth.Date)
|
||||
|
||||
@@ -0,0 +1,227 @@
|
||||
// Copyright 2026 Versity Software
|
||||
// This file is licensed under the Apache License, Version 2.0
|
||||
// (the "License"); you may not use this file except in compliance
|
||||
// with the License. You may obtain a copy of the License at
|
||||
//
|
||||
// http://www.apache.org/licenses/LICENSE-2.0
|
||||
//
|
||||
// Unless required by applicable law or agreed to in writing, software
|
||||
// distributed under the License is distributed on an "AS IS" BASIS,
|
||||
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
|
||||
// See the License for the specific language governing permissions and
|
||||
// limitations under the License.
|
||||
|
||||
package utils
|
||||
|
||||
import (
|
||||
"context"
|
||||
"net/http"
|
||||
"net/url"
|
||||
"testing"
|
||||
"time"
|
||||
|
||||
"github.com/aws/aws-sdk-go-v2/aws"
|
||||
"github.com/gofiber/fiber/v2"
|
||||
"github.com/stretchr/testify/require"
|
||||
"github.com/valyala/fasthttp"
|
||||
v4 "github.com/versity/versitygw/aws/signer/v4"
|
||||
"github.com/versity/versitygw/s3err"
|
||||
)
|
||||
|
||||
const signedHeadersTestRegion = "us-east-1"
|
||||
|
||||
var signedHeadersTestCreds = aws.Credentials{
|
||||
AccessKeyID: "AKID",
|
||||
SecretAccessKey: "SECRET",
|
||||
}
|
||||
|
||||
func TestCheckPresignedSignatureRejectsUnsignedAmzHeader(t *testing.T) {
|
||||
signedURL := buildPresignedURL(t, nil)
|
||||
ctx := fiberCtxFromURL(t, http.MethodPut, signedURL, http.Header{
|
||||
"X-Amz-Copy-Source": []string{"source/key"},
|
||||
})
|
||||
authData, err := ParsePresignedURIParts(ctx, signedHeadersTestRegion)
|
||||
require.NoError(t, err)
|
||||
|
||||
err = CheckPresignedSignature(ctx, authData, signedHeadersTestCreds.SecretAccessKey)
|
||||
requireHeadersNotSigned(t, err, "x-amz-copy-source")
|
||||
}
|
||||
|
||||
func TestCheckPresignedSignatureAllowsSignedAmzHeader(t *testing.T) {
|
||||
headers := http.Header{
|
||||
"X-Amz-Copy-Source": []string{"source/key"},
|
||||
}
|
||||
signedURL := buildPresignedURL(t, headers)
|
||||
ctx := fiberCtxFromURL(t, http.MethodPut, signedURL, headers)
|
||||
authData, err := ParsePresignedURIParts(ctx, signedHeadersTestRegion)
|
||||
require.NoError(t, err)
|
||||
|
||||
err = CheckPresignedSignature(ctx, authData, signedHeadersTestCreds.SecretAccessKey)
|
||||
require.NoError(t, err)
|
||||
}
|
||||
|
||||
func TestCheckPresignedSignatureAllowsUnsignedNonAmzHeader(t *testing.T) {
|
||||
signedURL := buildPresignedURL(t, nil)
|
||||
ctx := fiberCtxFromURL(t, http.MethodPut, signedURL, http.Header{
|
||||
"Content-Type": []string{"text/plain"},
|
||||
"X-Custom-Header": []string{"value"},
|
||||
})
|
||||
authData, err := ParsePresignedURIParts(ctx, signedHeadersTestRegion)
|
||||
require.NoError(t, err)
|
||||
|
||||
err = CheckPresignedSignature(ctx, authData, signedHeadersTestCreds.SecretAccessKey)
|
||||
require.NoError(t, err)
|
||||
}
|
||||
|
||||
func TestCheckValidSignatureRejectsUnsignedAmzHeader(t *testing.T) {
|
||||
ctx, authData, signingTime := signedHeaderAuthCtx(t, nil, http.Header{
|
||||
"X-Amz-Tagging": []string{"a=b"},
|
||||
})
|
||||
|
||||
_, err := CheckValidSignature(ctx, authData, signedHeadersTestCreds.SecretAccessKey, unsignedPayload, signingTime, 0)
|
||||
requireHeadersNotSigned(t, err, "x-amz-tagging")
|
||||
}
|
||||
|
||||
func TestCheckValidSignatureAllowsSignedAmzHeader(t *testing.T) {
|
||||
ctx, authData, signingTime := signedHeaderAuthCtx(t, http.Header{
|
||||
"X-Amz-Tagging": []string{"a=b"},
|
||||
}, nil)
|
||||
|
||||
_, err := CheckValidSignature(ctx, authData, signedHeadersTestCreds.SecretAccessKey, unsignedPayload, signingTime, 0)
|
||||
require.NoError(t, err)
|
||||
}
|
||||
|
||||
func TestCheckValidSignatureAllowsUnsignedNonAmzHeader(t *testing.T) {
|
||||
ctx, authData, signingTime := signedHeaderAuthCtx(t, nil, http.Header{
|
||||
"Content-Type": []string{"text/plain"},
|
||||
"X-Custom-Header": []string{"value"},
|
||||
})
|
||||
|
||||
_, err := CheckValidSignature(ctx, authData, signedHeadersTestCreds.SecretAccessKey, unsignedPayload, signingTime, 0)
|
||||
require.NoError(t, err)
|
||||
}
|
||||
|
||||
func TestCheckPresignedSignatureRejectsUnsignedAmzHeaderPattern(t *testing.T) {
|
||||
signedURL := buildPresignedURL(t, nil)
|
||||
ctx := fiberCtxFromURL(t, http.MethodPut, signedURL, http.Header{
|
||||
"X-Amz-Some-Other-Header": []string{"value"},
|
||||
})
|
||||
authData, err := ParsePresignedURIParts(ctx, signedHeadersTestRegion)
|
||||
require.NoError(t, err)
|
||||
|
||||
err = CheckPresignedSignature(ctx, authData, signedHeadersTestCreds.SecretAccessKey)
|
||||
requireHeadersNotSigned(t, err, "x-amz-some-other-header")
|
||||
}
|
||||
|
||||
func TestCheckValidSignatureRejectsUnsignedAmzHeaderPattern(t *testing.T) {
|
||||
ctx, authData, signingTime := signedHeaderAuthCtx(t, nil, http.Header{
|
||||
"X-Amz-Some-Other-Header": []string{"value"},
|
||||
})
|
||||
|
||||
_, err := CheckValidSignature(ctx, authData, signedHeadersTestCreds.SecretAccessKey, unsignedPayload, signingTime, 0)
|
||||
requireHeadersNotSigned(t, err, "x-amz-some-other-header")
|
||||
}
|
||||
|
||||
func buildPresignedURL(t *testing.T, headers http.Header) string {
|
||||
t.Helper()
|
||||
|
||||
req, err := http.NewRequest(http.MethodPut, "http://example.com/bucket/key?X-Amz-Expires=600", nil)
|
||||
require.NoError(t, err)
|
||||
req.Header = headers.Clone()
|
||||
if req.Header == nil {
|
||||
req.Header = make(http.Header)
|
||||
}
|
||||
|
||||
signer := v4.NewSigner()
|
||||
signedURL, _, _, err := signer.PresignHTTP(
|
||||
context.Background(),
|
||||
signedHeadersTestCreds,
|
||||
req,
|
||||
unsignedPayload,
|
||||
service,
|
||||
signedHeadersTestRegion,
|
||||
time.Now().UTC(),
|
||||
nil,
|
||||
func(options *v4.SignerOptions) {
|
||||
options.DisableURIPathEscaping = true
|
||||
},
|
||||
)
|
||||
require.NoError(t, err)
|
||||
|
||||
return signedURL
|
||||
}
|
||||
|
||||
func signedHeaderAuthCtx(t *testing.T, signedHeaders, extraHeaders http.Header) (*fiber.Ctx, AuthData, time.Time) {
|
||||
t.Helper()
|
||||
|
||||
signingTime := time.Now().UTC()
|
||||
req, err := http.NewRequest(http.MethodPut, "http://example.com/bucket/key", nil)
|
||||
require.NoError(t, err)
|
||||
req.Header = signedHeaders.Clone()
|
||||
if req.Header == nil {
|
||||
req.Header = make(http.Header)
|
||||
}
|
||||
|
||||
signer := v4.NewSigner()
|
||||
_, err = signer.SignHTTP(
|
||||
context.Background(),
|
||||
signedHeadersTestCreds,
|
||||
req,
|
||||
unsignedPayload,
|
||||
service,
|
||||
signedHeadersTestRegion,
|
||||
signingTime,
|
||||
nil,
|
||||
func(options *v4.SignerOptions) {
|
||||
options.DisableURIPathEscaping = true
|
||||
},
|
||||
)
|
||||
require.NoError(t, err)
|
||||
|
||||
headers := req.Header.Clone()
|
||||
for key, values := range extraHeaders {
|
||||
for _, value := range values {
|
||||
headers.Add(key, value)
|
||||
}
|
||||
}
|
||||
|
||||
ctx := fiberCtxFromURL(t, http.MethodPut, req.URL.String(), headers)
|
||||
authData, err := ParseAuthorization(ctx.Get("Authorization"))
|
||||
require.NoError(t, err)
|
||||
|
||||
return ctx, authData, signingTime
|
||||
}
|
||||
|
||||
func fiberCtxFromURL(t *testing.T, method, rawURL string, headers http.Header) *fiber.Ctx {
|
||||
t.Helper()
|
||||
|
||||
parsedURL, err := url.Parse(rawURL)
|
||||
require.NoError(t, err)
|
||||
|
||||
app := fiber.New()
|
||||
ctx := app.AcquireCtx(&fasthttp.RequestCtx{})
|
||||
t.Cleanup(func() {
|
||||
app.ReleaseCtx(ctx)
|
||||
})
|
||||
|
||||
ctx.Request().Header.SetMethod(method)
|
||||
ctx.Request().SetRequestURI(parsedURL.RequestURI())
|
||||
ctx.Request().Header.SetHost(parsedURL.Host)
|
||||
for key, values := range headers {
|
||||
for _, value := range values {
|
||||
ctx.Request().Header.Add(key, value)
|
||||
}
|
||||
}
|
||||
|
||||
return ctx
|
||||
}
|
||||
|
||||
func requireHeadersNotSigned(t *testing.T, err error, expected string) {
|
||||
t.Helper()
|
||||
|
||||
require.Error(t, err)
|
||||
serr, ok := err.(s3err.HeadersNotSignedError)
|
||||
require.Truef(t, ok, "expected HeadersNotSignedError, got %T", err)
|
||||
require.Equal(t, "AccessDenied", serr.Code)
|
||||
require.Equal(t, expected, serr.HeadersNotSigned)
|
||||
}
|
||||
+29
-16
@@ -25,6 +25,7 @@ import (
|
||||
"net/http"
|
||||
"net/url"
|
||||
"regexp"
|
||||
"slices"
|
||||
"strconv"
|
||||
"strings"
|
||||
"sync/atomic"
|
||||
@@ -33,6 +34,7 @@ import (
|
||||
"github.com/aws/aws-sdk-go-v2/service/s3/types"
|
||||
"github.com/gofiber/fiber/v2"
|
||||
"github.com/valyala/fasthttp"
|
||||
signerV4 "github.com/versity/versitygw/aws/signer/v4"
|
||||
"github.com/versity/versitygw/debuglogger"
|
||||
"github.com/versity/versitygw/s3err"
|
||||
"github.com/versity/versitygw/s3response"
|
||||
@@ -133,12 +135,8 @@ func createHttpRequestFromCtx(ctx *fiber.Ctx, signedHdrs []string, contentLength
|
||||
return nil, errors.New("error in creating an http request")
|
||||
}
|
||||
|
||||
// Set the request headers
|
||||
for key, value := range req.Header.All() {
|
||||
keyStr := string(key)
|
||||
if includeHeader(keyStr, signedHdrs) {
|
||||
httpReq.Header.Add(keyStr, string(value))
|
||||
}
|
||||
if err := addRequestHeadersFromCtx(ctx, httpReq, signedHdrs); err != nil {
|
||||
return nil, err
|
||||
}
|
||||
|
||||
// make sure all headers in the signed headers are present
|
||||
@@ -195,12 +193,8 @@ func createPresignedHttpRequestFromCtx(ctx *fiber.Ctx, signedHdrs []string, cont
|
||||
if err != nil {
|
||||
return nil, errors.New("error in creating an http request")
|
||||
}
|
||||
// Set the request headers
|
||||
for key, value := range req.Header.All() {
|
||||
keyStr := string(key)
|
||||
if includeHeader(keyStr, signedHdrs) {
|
||||
httpReq.Header.Add(keyStr, string(value))
|
||||
}
|
||||
if err := addRequestHeadersFromCtx(ctx, httpReq, signedHdrs); err != nil {
|
||||
return nil, err
|
||||
}
|
||||
|
||||
// Check if Content-Length in signed headers
|
||||
@@ -344,12 +338,31 @@ func IsValidBucketName(bucket string) bool {
|
||||
}
|
||||
|
||||
func includeHeader(hdr string, signedHdrs []string) bool {
|
||||
for _, shdr := range signedHdrs {
|
||||
if strings.EqualFold(hdr, shdr) {
|
||||
return true
|
||||
return slices.ContainsFunc(signedHdrs, func(shdr string) bool {
|
||||
return strings.EqualFold(hdr, shdr)
|
||||
})
|
||||
}
|
||||
|
||||
func addRequestHeadersFromCtx(ctx *fiber.Ctx, httpReq *http.Request, signedHdrs []string) error {
|
||||
headersNotSigned := []string{}
|
||||
for key, value := range ctx.Request().Header.All() {
|
||||
keyStr := string(key)
|
||||
if includeHeader(keyStr, signedHdrs) || signerV4.IsIgnoredHeader(keyStr) {
|
||||
httpReq.Header.Add(keyStr, string(value))
|
||||
continue
|
||||
}
|
||||
if signerV4.IsRequiredSignedHeader(keyStr) {
|
||||
lowerKey := strings.ToLower(keyStr)
|
||||
headersNotSigned = append(headersNotSigned, lowerKey)
|
||||
}
|
||||
}
|
||||
return false
|
||||
|
||||
if len(headersNotSigned) != 0 {
|
||||
debuglogger.Logf("headers present in request but not included in SignedHeaders: %q", strings.Join(headersNotSigned, ", "))
|
||||
return s3err.GetHeadersNotSignedErr(headersNotSigned)
|
||||
}
|
||||
|
||||
return nil
|
||||
}
|
||||
|
||||
// expiration time window
|
||||
|
||||
Reference in New Issue
Block a user