Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

security: allow setting conditions on the request region #13 #14

Merged
merged 1 commit into from
Dec 6, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
57 changes: 55 additions & 2 deletions cmd/almost-e2e_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -280,11 +280,34 @@ var testPolicyAllowTeamFolder string = fmt.Sprintf(`{
]
}`, testingBucketNameBackenddetails, testTeamFile, testTeamTag, testAllowedTeam)

var testPolicyAllowAllInRegion1ARN string = "arn:aws:iam::000000000000:role/AllowAllInRegion1"

//This policy is to test whether a policy can be scoped to a specific region
//since our proxy uses region to determine a backend this makes sure to be able
//to have different permissions for different backends. This is used in test cases
//that start with TestPolicyAllowAllInRegion1
var testPolicyAllowAllInRegion1 string = fmt.Sprintf(`{
"Version": "2012-10-17",
"Statement": [
{
"Effect": "Allow",
"Action": "s3:*",
"Resource": "*",
"Condition" : {
"StringLike" : {
"aws:RequestedRegion": "%s"
}
}
}
]
}`, testRegion1)

func NewTestPolicyManagerAlmostE2EPolicies() *PolicyManager {
return NewPolicyManager(
TestPolicyRetriever{
testPolicies: map[string]string{
testPolicyAllowTeamFolderARN: testPolicyAllowTeamFolder,
testPolicyAllowAllInRegion1ARN: testPolicyAllowAllInRegion1,
},
},
)
Expand Down Expand Up @@ -327,7 +350,7 @@ func TestPolicyAllowTeamFolderIDPClaimsCanBeUsedInPolicyEvaluationPrincipalWithI
t.Error("We should have gotten a Forbidden error but no error was raised.")
}
if err.ErrorCode() != "AccessDenied" {
t.Errorf("Expected Forbidden, got %s", err.ErrorCode())
t.Errorf("Expected AccessDenied, got %s", err.ErrorCode())
}
}

Expand All @@ -347,6 +370,36 @@ func TestPolicyAllowTeamFolderIDPClaimsCanBeUsedInPolicyEvaluationPrincipalWitho
t.Error("We should have gotten a Forbidden error but no error was raised.")
}
if err.ErrorCode() != "AccessDenied" {
t.Errorf("Expected Forbidden, got %s", err.ErrorCode())
t.Errorf("Expected AccessDenied, got %s", err.ErrorCode())
}
}

func TestPolicyAllowAllInRegion1ConditionsOnRegionAreEnforced(t *testing.T) {
tearDown, getSignedToken := testingFixture(t)
defer tearDown()
token := getSignedToken("mySubject", time.Minute * 20, AWSSessionTags{PrincipalTags: map[string][]string{"org": {"a"}}})
//Given the policy Manager that has our test policies
pm = *NewTestPolicyManagerAlmostE2EPolicies()
//Given credentials that use the policy that allow everything in Region1
creds := getCredentialsFromTestStsProxy(t, token, "my-session", testPolicyAllowAllInRegion1ARN)

//WHEN we get an object in region 1
regionContent, err := getRegionObjectContent(t, testRegion1, creds)
//THEN it should just succeed as any action is allowed
if err != nil {
t.Errorf("Could not get region content due to error %s", err)
} else if regionContent != testRegion1 {
t.Errorf("when retrieving region file for %s we got %s", testRegion1, regionContent)
}

//WHEN we get an object in region2
regionContent2, err2 := getRegionObjectContent(t, testRegion2, creds)
//THEN we expect it to give an access denied as no explicit allow exists for which region is not excluded via a condition
if err2 == nil {
t.Errorf("Could get region content %s but policy should have limited to %s", regionContent2, testRegion1)
} else {
if err2.ErrorCode() != "AccessDenied" {
t.Errorf("Expected AccessDenied, got %s", err.ErrorCode())
}
}
}
7 changes: 4 additions & 3 deletions cmd/handler-builder.go
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ func cleanHeadersThatAreNotSignedInAuthHeader(ctx context.Context, req *http.Req

// Authorize an S3 action
// maxExpiryTime is an upperbound for the expiry of the session token
func authorizeS3Action(ctx context.Context, sessionToken string, action S3ApiAction, w http.ResponseWriter, r *http.Request, maxExpiryTime time.Time) (allowed bool) {
func authorizeS3Action(ctx context.Context, sessionToken, targetRegion string, action S3ApiAction, w http.ResponseWriter, r *http.Request, maxExpiryTime time.Time) (allowed bool) {
allowed = false
sessionClaims, err := ExtractTokenClaims(sessionToken, s3ProxyKeyFunc)
if err != nil {
Expand All @@ -112,6 +112,7 @@ func authorizeS3Action(ctx context.Context, sessionToken string, action S3ApiAct
}

policySessionData := GetPolicySessionDataFromClaims(sessionClaims)
policySessionData.RequestedRegion = targetRegion
policyStr, err := pm.GetPolicy(sessionClaims.RoleARN, policySessionData)
if err != nil {
slog.Error("Could not get policy for temporary credentials", "error", err, xRequestIDStr, getRequestID(ctx), "role_arn", sessionClaims.RoleARN)
Expand Down Expand Up @@ -223,7 +224,7 @@ func (hb handlerBuilder) Build(action S3ApiAction, presigned bool) (http.Handler

//To have a valid signature
r.Header.Add(constants.AmzContentSHAKey, constants.EmptyStringSHA256)
if authorizeS3Action(ctx, creds.SessionToken, action, w, r, getCutoffForPresignedUrl()){
if authorizeS3Action(ctx, creds.SessionToken, targetBackendId, action, w, r, getCutoffForPresignedUrl()){
hb.proxyFunc(ctx, w, r, targetBackendId)
}
return
Expand Down Expand Up @@ -271,7 +272,7 @@ func (hb handlerBuilder) Build(action S3ApiAction, presigned bool) (http.Handler
targetRegion := requestutils.GetRegionFromRequest(r, globalBackendsConfig.defaultBackend)

//Authn done time to perform authorization
if authorizeS3Action(ctx, creds.SessionToken, action, w, r, time.Now().UTC()){
if authorizeS3Action(ctx, creds.SessionToken, targetRegion, action, w, r, time.Now().UTC()){
hb.proxyFunc(ctx, w, r, targetRegion)
}
return
Expand Down
1 change: 1 addition & 0 deletions cmd/policy-generation.go
Original file line number Diff line number Diff line change
Expand Up @@ -145,6 +145,7 @@ type PolicySessionClaims struct {
type PolicySessionData struct {
Claims PolicySessionClaims
Tags AWSSessionTags
RequestedRegion string
}

func GetPolicySessionDataFromClaims(claims *SessionClaims) *PolicySessionData {
Expand Down
12 changes: 12 additions & 0 deletions cmd/policy-iam-action.go
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@ func getS3ObjectFromRequest(req *http.Request) (bucketName string, objectKey str
//Add context keys that are added to nearly all requests that contain information about the current session
func addGenericSessionContextKeys(context map[string]*policy.ConditionValue, session *PolicySessionData) {
addAwsPrincipalTagConditionKeys(context, session)
addAwsRequestedRegionConditionKey(context, session)
}

//Add aws:PrincipalTag/tag-key keys that are added to nearly all requests that contain information about the current session
Expand All @@ -76,6 +77,17 @@ func addAwsPrincipalTagConditionKeys(context map[string]*policy.ConditionValue,
}
}

//Add aws:RequestedRegion key that are added to all requests
//https://docs.aws.amazon.com/IAM/latest/UserGuide/reference_policies_examples_aws_deny-requested-region.html
func addAwsRequestedRegionConditionKey(context map[string]*policy.ConditionValue, session *PolicySessionData) {
if session == nil {
return
}
if session.RequestedRegion != "" {
context["aws:RequestedRegion"] = policy.NewConditionValueString(true, session.RequestedRegion)
}
}

//Buid a new IAM action based out of an HTTP Request. The IAM action should resemble the required
//Permissions. The api_action is passed in as a string argument
func newIamActionsFromS3Request(api_action S3ApiAction, req *http.Request, session *PolicySessionData) (actions []iamAction, err error) {
Expand Down
Loading