Skip to content
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
27 changes: 27 additions & 0 deletions policy/admin-action.go
Original file line number Diff line number Diff line change
Expand Up @@ -387,6 +387,33 @@ var SupportedAdminActions = map[AdminAction]struct{}{
AllAdminActions: {},
}

// AdminActionsWithResource enumerates admin actions that operate on
// a specific bucket resource. When a policy statement contains one of
// these actions *and* specifies a Resource, the resource is enforced
// against the target bucket. All other admin actions are resource-less;
// any Resource specified in the statement is ignored for them.
var AdminActionsWithResource = map[AdminAction]struct{}{
SetBucketQuotaAdminAction: {},
GetBucketQuotaAdminAction: {},
SetBucketTargetAction: {},
GetBucketTargetAction: {},
ReplicationDiff: {},
ImportBucketMetadataAction: {},
ExportBucketMetadataAction: {},
HealAdminAction: {},
InventoryControlAction: {},
}

// HasResource reports whether this admin action operates on a bucket resource.
func (action AdminAction) HasResource() bool {
for a := range AdminActionsWithResource {
if action.Match(a) {
return true
}
}
return false
}

// Match - matches action name with action pattern.
func (action AdminAction) Match(a AdminAction) bool {
return wildcard.Match(string(action), string(a))
Expand Down
32 changes: 32 additions & 0 deletions policy/policy.go
Original file line number Diff line number Diff line change
Expand Up @@ -493,6 +493,23 @@ func (iamp Policy) Validate() error {
return iamp.isValid()
}

// ValidateStrict applies strict validation rules suitable for new
// policy creation. It rejects policies that would be accepted by
// Validate for backward compatibility but are invalid going forward
// (e.g. admin statements with both Resource and NotResource).
func (iamp Policy) ValidateStrict() error {
if iamp.Version != DefaultVersion && iamp.Version != "" {
return Errorf("invalid version '%v'", iamp.Version)
}

for _, statement := range iamp.Statements {
if err := statement.isValidStrict(); err != nil {
return err
}
}
return nil
}

// updateActionIndex with latest statements()
// maintains a reverse map of Action -> []Statements
// for faster lookup and short-circuit.
Expand Down Expand Up @@ -533,6 +550,21 @@ func ParseConfig(reader io.Reader) (*Policy, error) {
return &iamp, iamp.Validate()
}

// ParseConfigStrict parses and validates with strict rules. Use this
// when the server is creating or updating policies — it rejects
// constructs that Validate allows for backward compatibility.
func ParseConfigStrict(reader io.Reader) (*Policy, error) {
var iamp Policy

decoder := json.NewDecoder(reader)
decoder.DisallowUnknownFields()
if err := decoder.Decode(&iamp); err != nil {
return nil, Errorf("%w", err)
}

return &iamp, iamp.ValidateStrict()
}

// Equals returns true if the two policies are identical
func (iamp *Policy) Equals(p Policy) bool {
if iamp.ID != p.ID || iamp.Version != p.Version {
Expand Down
178 changes: 178 additions & 0 deletions policy/policy_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2716,3 +2716,181 @@ func TestS3VectorsActionsAllowed(t *testing.T) {
})
}
}

func TestAdminActionResourceScoping(t *testing.T) {
// Policy that scopes GetBucketQuota to a specific bucket via Resource.
scopedPolicy := `{
"Version": "2012-10-17",
"Statement": [
{
"Effect": "Allow",
"Action": ["admin:GetBucketQuota"],
"Resource": ["arn:aws:s3:::mybucket"]
}
]
}`
p, err := ParseConfig(strings.NewReader(scopedPolicy))
if err != nil {
t.Fatal(err)
}

// Matching bucket must be allowed.
allowed := p.IsAllowedActions("mybucket", "", map[string][]string{})
if !allowed.Match(GetBucketQuotaAdminAction) {
t.Fatal("expected GetBucketQuota allowed for mybucket")
}

// Different bucket must be denied.
allowed = p.IsAllowedActions("otherbucket", "", map[string][]string{})
if allowed.Match(GetBucketQuotaAdminAction) {
t.Fatal("expected GetBucketQuota denied for otherbucket")
}

// Resource-less admin actions ignore Resource even if specified.
resourcelessPolicy := `{
"Version": "2012-10-17",
"Statement": [
{
"Effect": "Allow",
"Action": ["admin:ServerInfo"]
}
]
}`
p, err = ParseConfig(strings.NewReader(resourcelessPolicy))
if err != nil {
t.Fatal(err)
}
allowed = p.IsAllowedActions("", "", map[string][]string{})
if !allowed.Match(ServerInfoAdminAction) {
t.Fatal("expected ServerInfo allowed without resource")
}

// admin:* without Resource stays backward compatible for resource-less actions.
wildcardPolicy := `{
"Version": "2012-10-17",
"Statement": [
{
"Effect": "Allow",
"Action": ["admin:*"]
}
]
}`
p, err = ParseConfig(strings.NewReader(wildcardPolicy))
if err != nil {
t.Fatal(err)
}
allowed = p.IsAllowedActions("", "", map[string][]string{})
if !allowed.Match(ServerInfoAdminAction) {
t.Fatal("expected ServerInfo allowed with admin:* and no resource")
}
if !allowed.Match(GetBucketQuotaAdminAction) {
t.Fatal("expected GetBucketQuota allowed with admin:* and no resource constraint")
}
}

func TestAdminActionHasResource(t *testing.T) {
// Bucket-scoped actions must report HasResource.
bucketScoped := []AdminAction{
SetBucketQuotaAdminAction,
GetBucketQuotaAdminAction,
SetBucketTargetAction,
GetBucketTargetAction,
ReplicationDiff,
ImportBucketMetadataAction,
ExportBucketMetadataAction,
HealAdminAction,
InventoryControlAction,
}
for _, a := range bucketScoped {
if !a.HasResource() {
t.Errorf("expected %s to have resource", a)
}
}

// Resource-less actions must not.
resourceless := []AdminAction{
ServerInfoAdminAction,
ListUsersAdminAction,
ConfigUpdateAdminAction,
CreateUserAdminAction,
}
for _, a := range resourceless {
if a.HasResource() {
t.Errorf("expected %s to not have resource", a)
}
}
}

func TestParseConfigStrictRejectsResourceConflict(t *testing.T) {
// Strict parsing rejects admin statements with both Resource and NotResource.
conflicting := `{
"Version": "2012-10-17",
"Statement": [
{
"Effect": "Allow",
"Action": ["admin:GetBucketQuota"],
"Resource": ["arn:aws:s3:::mybucket"],
"NotResource": ["arn:aws:s3:::otherbucket"]
}
]
}`
// Permissive parse succeeds (backward compat).
_, err := ParseConfig(strings.NewReader(conflicting))
if err != nil {
t.Fatalf("permissive parse should succeed, got: %v", err)
}

// Strict parse rejects.
_, err = ParseConfigStrict(strings.NewReader(conflicting))
if err == nil {
t.Fatal("strict parse should reject Resource + NotResource in admin statement")
}

// Resource-less admin action with no resources is fine in strict mode.
clean := `{
"Version": "2012-10-17",
"Statement": [
{
"Effect": "Allow",
"Action": ["admin:ServerInfo"]
}
]
}`
_, err = ParseConfigStrict(strings.NewReader(clean))
if err != nil {
t.Fatalf("strict parse should accept clean admin statement, got: %v", err)
}

// Bucket-scoped admin action with valid Resource is fine in strict mode.
scoped := `{
"Version": "2012-10-17",
"Statement": [
{
"Effect": "Allow",
"Action": ["admin:GetBucketQuota"],
"Resource": ["arn:aws:s3:::mybucket"]
}
]
}`
_, err = ParseConfigStrict(strings.NewReader(scoped))
if err != nil {
t.Fatalf("strict parse should accept scoped admin statement, got: %v", err)
}
}

func TestMixedActionTypesRejected(t *testing.T) {
mixed := `{
"Version": "2012-10-17",
"Statement": [
{
"Effect": "Allow",
"Action": ["s3:GetObject", "admin:ServerInfo"],
"Resource": ["arn:aws:s3:::mybucket/*"]
}
]
}`
_, err := ParseConfig(strings.NewReader(mixed))
if err == nil {
t.Fatal("expected error for mixed s3+admin actions in same statement")
}
}
Loading
Loading