Merge pull request #1546 from versity/sis/incorrect-md5

fix: adds BadDigest error for incorrect Content-Md5 s
This commit is contained in:
Ben McClelland
2025-09-22 10:44:56 -07:00
committed by GitHub
17 changed files with 566 additions and 452 deletions

View File

@@ -35,42 +35,42 @@ func (ar *S3AdminRouter) Init(app *fiber.App, be backend.Backend, iam auth.IAMSe
// CreateUser admin api
app.Patch("/create-user",
controllers.ProcessHandlers(ctrl.CreateUser, metrics.ActionAdminCreateUser, services,
middlewares.VerifyV4Signature(root, iam, region),
middlewares.VerifyV4Signature(root, iam, region, false),
middlewares.IsAdmin(metrics.ActionAdminCreateUser),
))
// DeleteUsers admin api
app.Patch("/delete-user",
controllers.ProcessHandlers(ctrl.DeleteUser, metrics.ActionAdminDeleteUser, services,
middlewares.VerifyV4Signature(root, iam, region),
middlewares.VerifyV4Signature(root, iam, region, false),
middlewares.IsAdmin(metrics.ActionAdminDeleteUser),
))
// UpdateUser admin api
app.Patch("/update-user",
controllers.ProcessHandlers(ctrl.UpdateUser, metrics.ActionAdminUpdateUser, services,
middlewares.VerifyV4Signature(root, iam, region),
middlewares.VerifyV4Signature(root, iam, region, false),
middlewares.IsAdmin(metrics.ActionAdminUpdateUser),
))
// ListUsers admin api
app.Patch("/list-users",
controllers.ProcessHandlers(ctrl.ListUsers, metrics.ActionAdminListUsers, services,
middlewares.VerifyV4Signature(root, iam, region),
middlewares.VerifyV4Signature(root, iam, region, false),
middlewares.IsAdmin(metrics.ActionAdminListUsers),
))
// ChangeBucketOwner admin api
app.Patch("/change-bucket-owner",
controllers.ProcessHandlers(ctrl.ChangeBucketOwner, metrics.ActionAdminChangeBucketOwner, services,
middlewares.VerifyV4Signature(root, iam, region),
middlewares.VerifyV4Signature(root, iam, region, false),
middlewares.IsAdmin(metrics.ActionAdminChangeBucketOwner),
))
// ListBucketsAndOwners admin api
app.Patch("/list-buckets",
controllers.ProcessHandlers(ctrl.ListBuckets, metrics.ActionAdminListBuckets, services,
middlewares.VerifyV4Signature(root, iam, region),
middlewares.VerifyV4Signature(root, iam, region, false),
middlewares.IsAdmin(metrics.ActionAdminListBuckets),
))
}

View File

@@ -37,7 +37,7 @@ type RootUserConfig struct {
Secret string
}
func VerifyV4Signature(root RootUserConfig, iam auth.IAMService, region string) fiber.Handler {
func VerifyV4Signature(root RootUserConfig, iam auth.IAMService, region string, streamBody bool) fiber.Handler {
acct := accounts{root: root, iam: iam}
return func(ctx *fiber.Ctx) error {
@@ -112,7 +112,7 @@ func VerifyV4Signature(root RootUserConfig, iam auth.IAMService, region string)
if !utils.IsValidSh256PayloadHeader(hashPayload) {
return s3err.GetAPIError(s3err.ErrInvalidSHA256Paylod)
}
if utils.IsBigDataAction(ctx) {
if streamBody {
// for streaming PUT actions, authorization is deferred
// until end of stream due to need to get length and
// checksum of the stream to validate authorization
@@ -160,7 +160,7 @@ func VerifyV4Signature(root RootUserConfig, iam auth.IAMService, region string)
}
}
err = utils.CheckValidSignature(ctx, authData, account.Secret, hashPayload, tdate, contentLength)
err = utils.CheckValidSignature(ctx, authData, account.Secret, hashPayload, tdate, contentLength, false)
if err != nil {
return err
}

View File

@@ -16,6 +16,7 @@ package middlewares
import (
"crypto/md5"
"encoding/base64"
"io"
"github.com/gofiber/fiber/v2"
@@ -23,14 +24,18 @@ import (
"github.com/versity/versitygw/s3err"
)
func VerifyMD5Body() fiber.Handler {
func VerifyMD5Body(streamBody bool) fiber.Handler {
return func(ctx *fiber.Ctx) error {
incomingSum := ctx.Get("Content-Md5")
if incomingSum == "" {
return nil
}
if utils.IsBigDataAction(ctx) {
if !isValidMD5(incomingSum) {
return s3err.GetAPIError(s3err.ErrInvalidDigest)
}
if streamBody {
var err error
wrapBodyReader(ctx, func(r io.Reader) io.Reader {
r, err = utils.NewHashReader(r, incomingSum, utils.HashTypeMd5)
@@ -46,9 +51,18 @@ func VerifyMD5Body() fiber.Handler {
calculatedSum := utils.Base64SumString(sum[:])
if incomingSum != calculatedSum {
return s3err.GetAPIError(s3err.ErrInvalidDigest)
return s3err.GetAPIError(s3err.ErrBadDigest)
}
return nil
}
}
func isValidMD5(s string) bool {
decoded, err := base64.StdEncoding.DecodeString(s)
if err != nil {
return false
}
return len(decoded) == 16
}

View File

@@ -0,0 +1,41 @@
// Copyright 2023 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 middlewares
import (
"testing"
"github.com/stretchr/testify/assert"
)
func Test_isValidMD5(t *testing.T) {
tests := []struct {
name string
s string
want bool
}{
{"invalid", "hello world", false},
{"valid base64", "aGVsbCBzLGRham5mamFuc2Zhc2RmZHNhZmRzYWY=", false},
{"valid 1", "CY9rzUYh03PK3k6DJie09g==", true},
{"valid 2", "uU0nuZNNPgilLlLX2n2r+s==", true},
{"valid 3", "7Qdih1MuhjZehB6Sv8UNjA==", true},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
got := isValidMD5(tt.s)
assert.Equal(t, tt.want, got)
})
}
}

View File

@@ -24,7 +24,7 @@ import (
"github.com/versity/versitygw/s3err"
)
func VerifyPresignedV4Signature(root RootUserConfig, iam auth.IAMService, region string) fiber.Handler {
func VerifyPresignedV4Signature(root RootUserConfig, iam auth.IAMService, region string, streamBody bool) fiber.Handler {
acct := accounts{root: root, iam: iam}
return func(ctx *fiber.Ctx) error {
@@ -71,7 +71,7 @@ func VerifyPresignedV4Signature(root RootUserConfig, iam auth.IAMService, region
}
}
if utils.IsBigDataAction(ctx) {
if streamBody {
// Content-Length has to be set for data uploads: PutObject, UploadPart
if contentLengthStr == "" {
return s3err.GetAPIError(s3err.ErrMissingContentLength)
@@ -88,7 +88,7 @@ func VerifyPresignedV4Signature(root RootUserConfig, iam auth.IAMService, region
return nil
}
err = utils.CheckPresignedSignature(ctx, authData, account.Secret)
err = utils.CheckPresignedSignature(ctx, authData, account.Secret, streamBody)
if err != nil {
return err
}

View File

@@ -28,7 +28,7 @@ import (
// AuthorizePublicBucketAccess checks if the bucket grants public
// access to anonymous requesters
func AuthorizePublicBucketAccess(be backend.Backend, s3action string, policyPermission auth.Action, permission auth.Permission) fiber.Handler {
func AuthorizePublicBucketAccess(be backend.Backend, s3action string, policyPermission auth.Action, permission auth.Permission, streamBody bool) fiber.Handler {
return func(ctx *fiber.Ctx) error {
// skip for authenticated requests
if utils.IsPresignedURLAuth(ctx) || ctx.Get("Authorization") != "" {
@@ -60,7 +60,7 @@ func AuthorizePublicBucketAccess(be backend.Backend, s3action string, policyPerm
return err
}
if utils.IsBigDataAction(ctx) {
if streamBody {
payloadType := ctx.Get("X-Amz-Content-Sha256")
if utils.IsUnsignedStreamingPayload(payloadType) {
checksumType, err := utils.ExtractChecksumType(ctx)

File diff suppressed because it is too large Load Diff

View File

@@ -106,7 +106,7 @@ func (ar *AuthReader) validateSignature() error {
return s3err.GetAPIError(s3err.ErrMissingDateHeader)
}
return CheckValidSignature(ar.ctx, ar.auth, ar.secret, hashPayload, tdate, int64(ar.size))
return CheckValidSignature(ar.ctx, ar.auth, ar.secret, hashPayload, tdate, int64(ar.size), true)
}
const (
@@ -114,11 +114,11 @@ const (
)
// CheckValidSignature validates the ctx v4 auth signature
func CheckValidSignature(ctx *fiber.Ctx, auth AuthData, secret, checksum string, tdate time.Time, contentLen int64) error {
func CheckValidSignature(ctx *fiber.Ctx, auth AuthData, secret, checksum string, tdate time.Time, contentLen int64, streamBody bool) error {
signedHdrs := strings.Split(auth.SignedHeaders, ";")
// Create a new http request instance from fasthttp request
req, err := createHttpRequestFromCtx(ctx, signedHdrs, contentLen)
req, err := createHttpRequestFromCtx(ctx, signedHdrs, contentLen, streamBody)
if err != nil {
return fmt.Errorf("create http request from context: %w", err)
}

View File

@@ -92,7 +92,7 @@ func Test_Client_UserAgent(t *testing.T) {
}
app.Get("/", func(c *fiber.Ctx) error {
req, err := createHttpRequestFromCtx(c, signedHdrs, int64(c.Request().Header.ContentLength()))
req, err := createHttpRequestFromCtx(c, signedHdrs, int64(c.Request().Header.ContentLength()), true)
if err != nil {
t.Fatal(err)
}

View File

@@ -115,7 +115,7 @@ func (hr *HashReader) Read(p []byte) (int, error) {
case HashTypeMd5:
sum := hr.Sum()
if sum != hr.sum {
return n, s3err.GetAPIError(s3err.ErrInvalidDigest)
return n, s3err.GetAPIError(s3err.ErrBadDigest)
}
case HashTypeSha256Hex:
sum := hr.Sum()

View File

@@ -64,7 +64,7 @@ func (pr *PresignedAuthReader) Read(p []byte) (int, error) {
n, err := pr.r.Read(p)
if errors.Is(err, io.EOF) {
cerr := CheckPresignedSignature(pr.ctx, pr.auth, pr.secret)
cerr := CheckPresignedSignature(pr.ctx, pr.auth, pr.secret, true)
if cerr != nil {
return n, cerr
}
@@ -74,7 +74,7 @@ func (pr *PresignedAuthReader) Read(p []byte) (int, error) {
}
// CheckPresignedSignature validates presigned request signature
func CheckPresignedSignature(ctx *fiber.Ctx, auth AuthData, secret string) error {
func CheckPresignedSignature(ctx *fiber.Ctx, auth AuthData, secret string, streamBody bool) error {
signedHdrs := strings.Split(auth.SignedHeaders, ";")
var contentLength int64
@@ -88,7 +88,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)
req, err := createPresignedHttpRequestFromCtx(ctx, signedHdrs, contentLength, streamBody)
if err != nil {
return fmt.Errorf("create http request from context: %w", err)
}

View File

@@ -57,10 +57,10 @@ func GetUserMetaData(headers *fasthttp.RequestHeader) (metadata map[string]strin
return
}
func createHttpRequestFromCtx(ctx *fiber.Ctx, signedHdrs []string, contentLength int64) (*http.Request, error) {
func createHttpRequestFromCtx(ctx *fiber.Ctx, signedHdrs []string, contentLength int64, streamBody bool) (*http.Request, error) {
req := ctx.Request()
var body io.Reader
if IsBigDataAction(ctx) {
if streamBody {
body = req.BodyStream()
} else {
body = bytes.NewReader(req.Body())
@@ -112,10 +112,10 @@ var (
}
)
func createPresignedHttpRequestFromCtx(ctx *fiber.Ctx, signedHdrs []string, contentLength int64) (*http.Request, error) {
func createPresignedHttpRequestFromCtx(ctx *fiber.Ctx, signedHdrs []string, contentLength int64, streamBody bool) (*http.Request, error) {
req := ctx.Request()
var body io.Reader
if IsBigDataAction(ctx) {
if streamBody {
body = req.BodyStream()
} else {
body = bytes.NewReader(req.Body())
@@ -236,15 +236,6 @@ func includeHeader(hdr string, signedHdrs []string) bool {
return false
}
func IsBigDataAction(ctx *fiber.Ctx) bool {
if ctx.Method() == http.MethodPut && len(strings.Split(ctx.Path(), "/")) >= 3 {
if !ctx.Request().URI().QueryArgs().Has("tagging") && ctx.Get("X-Amz-Copy-Source") == "" && !ctx.Request().URI().QueryArgs().Has("acl") {
return true
}
}
return false
}
// expiration time window
// https://docs.aws.amazon.com/AmazonS3/latest/userguide/RESTAuthentication.html#RESTAuthenticationTimeStamp
const timeExpirationSec = 15 * 60

View File

@@ -81,7 +81,7 @@ func TestCreateHttpRequestFromCtx(t *testing.T) {
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
got, err := createHttpRequestFromCtx(tt.args.ctx, tt.hdrs, 0)
got, err := createHttpRequestFromCtx(tt.args.ctx, tt.hdrs, 0, true)
if (err != nil) != tt.wantErr {
t.Errorf("CreateHttpRequestFromCtx() error = %v, wantErr %v", err, tt.wantErr)
return