Skip to content

Commit 039f59b

Browse files
fix: missing user policy enforcement in PostPolicyHandler (#11682)
1 parent c6a120d commit 039f59b

6 files changed

+63
-24
lines changed

Diff for: cmd/auth-handler.go

+4-4
Original file line numberDiff line numberDiff line change
@@ -185,12 +185,12 @@ func getSessionToken(r *http.Request) (token string) {
185185
// Fetch claims in the security token returned by the client, doesn't return
186186
// errors - upon errors the returned claims map will be empty.
187187
func mustGetClaimsFromToken(r *http.Request) map[string]interface{} {
188-
claims, _ := getClaimsFromToken(r, getSessionToken(r))
188+
claims, _ := getClaimsFromToken(getSessionToken(r))
189189
return claims
190190
}
191191

192192
// Fetch claims in the security token returned by the client.
193-
func getClaimsFromToken(r *http.Request, token string) (map[string]interface{}, error) {
193+
func getClaimsFromToken(token string) (map[string]interface{}, error) {
194194
claims := xjwt.NewMapClaims()
195195
if token == "" {
196196
return claims.Map(), nil
@@ -237,7 +237,7 @@ func getClaimsFromToken(r *http.Request, token string) (map[string]interface{},
237237
if err != nil {
238238
// Base64 decoding fails, we should log to indicate
239239
// something is malforming the request sent by client.
240-
logger.LogIf(r.Context(), err, logger.Application)
240+
logger.LogIf(GlobalContext, err, logger.Application)
241241
return nil, errAuthentication
242242
}
243243
claims.MapClaims[iampolicy.SessionPolicyName] = string(spBytes)
@@ -258,7 +258,7 @@ func checkClaimsFromToken(r *http.Request, cred auth.Credentials) (map[string]in
258258
if subtle.ConstantTimeCompare([]byte(token), []byte(cred.SessionToken)) != 1 {
259259
return nil, ErrInvalidToken
260260
}
261-
claims, err := getClaimsFromToken(r, token)
261+
claims, err := getClaimsFromToken(token)
262262
if err != nil {
263263
return nil, toAPIErrorCode(r.Context(), err)
264264
}

Diff for: cmd/bucket-handlers.go

+46-6
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
package cmd
1818

1919
import (
20+
"crypto/subtle"
2021
"encoding/base64"
2122
"encoding/xml"
2223
"fmt"
@@ -25,7 +26,6 @@ import (
2526
"net/textproto"
2627
"net/url"
2728
"path"
28-
"path/filepath"
2929
"sort"
3030
"strconv"
3131
"strings"
@@ -337,7 +337,7 @@ func (api objectAPIHandlers) ListBucketsHandler(w http.ResponseWriter, r *http.R
337337

338338
// err will be nil here as we already called this function
339339
// earlier in this request.
340-
claims, _ := getClaimsFromToken(r, getSessionToken(r))
340+
claims, _ := getClaimsFromToken(getSessionToken(r))
341341
n := 0
342342
// Use the following trick to filter in place
343343
// https://github.com/golang/go/wiki/SliceTricks#filter-in-place
@@ -797,13 +797,15 @@ func (api objectAPIHandlers) PostPolicyBucketHandler(w http.ResponseWriter, r *h
797797
writeErrorResponse(ctx, w, errorCodes.ToAPIErr(ErrMissingContentLength), r.URL, guessIsBrowserReq(r))
798798
return
799799
}
800+
800801
resource, err := getResource(r.URL.Path, r.Host, globalDomainNames)
801802
if err != nil {
802803
writeErrorResponse(ctx, w, errorCodes.ToAPIErr(ErrInvalidRequest), r.URL, guessIsBrowserReq(r))
803804
return
804805
}
805-
// Make sure that the URL does not contain object name.
806-
if bucket != filepath.Clean(resource[1:]) {
806+
807+
// Make sure that the URL does not contain object name.
808+
if bucket != path.Clean(resource[1:]) {
807809
writeErrorResponse(ctx, w, errorCodes.ToAPIErr(ErrMethodNotAllowed), r.URL, guessIsBrowserReq(r))
808810
return
809811
}
@@ -846,7 +848,6 @@ func (api objectAPIHandlers) PostPolicyBucketHandler(w http.ResponseWriter, r *h
846848
defer fileBody.Close()
847849

848850
formValues.Set("Bucket", bucket)
849-
850851
if fileName != "" && strings.Contains(formValues.Get("Key"), "${filename}") {
851852
// S3 feature to replace ${filename} found in Key form field
852853
// by the filename attribute passed in multipart
@@ -866,12 +867,51 @@ func (api objectAPIHandlers) PostPolicyBucketHandler(w http.ResponseWriter, r *h
866867
}
867868

868869
// Verify policy signature.
869-
errCode := doesPolicySignatureMatch(formValues)
870+
cred, errCode := doesPolicySignatureMatch(formValues)
870871
if errCode != ErrNone {
871872
writeErrorResponse(ctx, w, errorCodes.ToAPIErr(errCode), r.URL, guessIsBrowserReq(r))
872873
return
873874
}
874875

876+
// Once signature is validated, check if the user has
877+
// explicit permissions for the user.
878+
{
879+
token := formValues.Get(xhttp.AmzSecurityToken)
880+
if token != "" && cred.AccessKey == "" {
881+
writeErrorResponse(ctx, w, errorCodes.ToAPIErr(ErrNoAccessKey), r.URL, guessIsBrowserReq(r))
882+
return
883+
}
884+
885+
if cred.IsServiceAccount() && token == "" {
886+
token = cred.SessionToken
887+
}
888+
889+
if subtle.ConstantTimeCompare([]byte(token), []byte(cred.SessionToken)) != 1 {
890+
writeErrorResponse(ctx, w, errorCodes.ToAPIErr(ErrInvalidToken), r.URL, guessIsBrowserReq(r))
891+
return
892+
}
893+
894+
// Extract claims if any.
895+
claims, err := getClaimsFromToken(token)
896+
if err != nil {
897+
writeErrorResponse(ctx, w, toAPIError(ctx, err), r.URL, guessIsBrowserReq(r))
898+
return
899+
}
900+
901+
if !globalIAMSys.IsAllowed(iampolicy.Args{
902+
AccountName: cred.AccessKey,
903+
Action: iampolicy.PutObjectAction,
904+
ConditionValues: getConditionValues(r, "", cred.AccessKey, claims),
905+
BucketName: bucket,
906+
ObjectName: object,
907+
IsOwner: globalActiveCred.AccessKey == cred.AccessKey,
908+
Claims: claims,
909+
}) {
910+
writeErrorResponse(ctx, w, errorCodes.ToAPIErr(ErrAccessDenied), r.URL, guessIsBrowserReq(r))
911+
return
912+
}
913+
}
914+
875915
policyBytes, err := base64.StdEncoding.DecodeString(formValues.Get("Policy"))
876916
if err != nil {
877917
writeErrorResponse(ctx, w, errorCodes.ToAPIErr(ErrMalformedPOSTRequest), r.URL, guessIsBrowserReq(r))

Diff for: cmd/signature-v2.go

+4-6
Original file line numberDiff line numberDiff line change
@@ -75,20 +75,18 @@ const (
7575

7676
// AWS S3 Signature V2 calculation rule is give here:
7777
// http://docs.aws.amazon.com/AmazonS3/latest/dev/RESTAuthentication.html#RESTAuthenticationStringToSign
78-
79-
func doesPolicySignatureV2Match(formValues http.Header) APIErrorCode {
80-
cred := globalActiveCred
78+
func doesPolicySignatureV2Match(formValues http.Header) (auth.Credentials, APIErrorCode) {
8179
accessKey := formValues.Get(xhttp.AmzAccessKeyID)
8280
cred, _, s3Err := checkKeyValid(accessKey)
8381
if s3Err != ErrNone {
84-
return s3Err
82+
return cred, s3Err
8583
}
8684
policy := formValues.Get("Policy")
8785
signature := formValues.Get(xhttp.AmzSignatureV2)
8886
if !compareSignatureV2(signature, calculateSignatureV2(policy, cred.SecretKey)) {
89-
return ErrSignatureDoesNotMatch
87+
return cred, ErrSignatureDoesNotMatch
9088
}
91-
return ErrNone
89+
return cred, ErrNone
9290
}
9391

9492
// Escape encodedQuery string into unescaped list of query params, returns error

Diff for: cmd/signature-v2_test.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -265,7 +265,7 @@ func TestDoesPolicySignatureV2Match(t *testing.T) {
265265
formValues.Set("Awsaccesskeyid", test.accessKey)
266266
formValues.Set("Signature", test.signature)
267267
formValues.Set("Policy", test.policy)
268-
errCode := doesPolicySignatureV2Match(formValues)
268+
_, errCode := doesPolicySignatureV2Match(formValues)
269269
if errCode != test.errCode {
270270
t.Fatalf("(%d) expected to get %s, instead got %s", i+1, niceError(test.errCode), niceError(errCode))
271271
}

Diff for: cmd/signature-v4.go

+7-6
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@ import (
3939
"github.com/minio/minio-go/v7/pkg/s3utils"
4040
"github.com/minio/minio-go/v7/pkg/set"
4141
xhttp "github.com/minio/minio/cmd/http"
42+
"github.com/minio/minio/pkg/auth"
4243
)
4344

4445
// AWS Signature Version '4' constants.
@@ -149,7 +150,7 @@ func getSignature(signingKey []byte, stringToSign string) string {
149150
}
150151

151152
// Check to see if Policy is signed correctly.
152-
func doesPolicySignatureMatch(formValues http.Header) APIErrorCode {
153+
func doesPolicySignatureMatch(formValues http.Header) (auth.Credentials, APIErrorCode) {
153154
// For SignV2 - Signature field will be valid
154155
if _, ok := formValues["Signature"]; ok {
155156
return doesPolicySignatureV2Match(formValues)
@@ -169,19 +170,19 @@ func compareSignatureV4(sig1, sig2 string) bool {
169170
// doesPolicySignatureMatch - Verify query headers with post policy
170171
// - http://docs.aws.amazon.com/AmazonS3/latest/API/sigv4-HTTPPOSTConstructPolicy.html
171172
// returns ErrNone if the signature matches.
172-
func doesPolicySignatureV4Match(formValues http.Header) APIErrorCode {
173+
func doesPolicySignatureV4Match(formValues http.Header) (auth.Credentials, APIErrorCode) {
173174
// Server region.
174175
region := globalServerRegion
175176

176177
// Parse credential tag.
177178
credHeader, s3Err := parseCredentialHeader("Credential="+formValues.Get(xhttp.AmzCredential), region, serviceS3)
178179
if s3Err != ErrNone {
179-
return s3Err
180+
return auth.Credentials{}, s3Err
180181
}
181182

182183
cred, _, s3Err := checkKeyValid(credHeader.accessKey)
183184
if s3Err != ErrNone {
184-
return s3Err
185+
return cred, s3Err
185186
}
186187

187188
// Get signing key.
@@ -192,11 +193,11 @@ func doesPolicySignatureV4Match(formValues http.Header) APIErrorCode {
192193

193194
// Verify signature.
194195
if !compareSignatureV4(newSignature, formValues.Get(xhttp.AmzSignature)) {
195-
return ErrSignatureDoesNotMatch
196+
return cred, ErrSignatureDoesNotMatch
196197
}
197198

198199
// Success.
199-
return ErrNone
200+
return cred, ErrNone
200201
}
201202

202203
// doesPresignedSignatureMatch - Verify query headers with presigned signature

Diff for: cmd/signature-v4_test.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -84,7 +84,7 @@ func TestDoesPolicySignatureMatch(t *testing.T) {
8484

8585
// Run each test case individually.
8686
for i, testCase := range testCases {
87-
code := doesPolicySignatureMatch(testCase.form)
87+
_, code := doesPolicySignatureMatch(testCase.form)
8888
if code != testCase.expected {
8989
t.Errorf("(%d) expected to get %s, instead got %s", i, niceError(testCase.expected), niceError(code))
9090
}

0 commit comments

Comments
 (0)