diff --git a/pkg/s3perm/s3perm.go b/pkg/s3perm/s3perm.go index b1069fee..2e57935e 100644 --- a/pkg/s3perm/s3perm.go +++ b/pkg/s3perm/s3perm.go @@ -5,6 +5,8 @@ package s3perm import ( + "slices" + "github.com/fil-forge/libforge/commands/blob" "github.com/fil-forge/libforge/commands/content" "github.com/fil-forge/libforge/commands/index" @@ -111,6 +113,9 @@ var permissionCommands = map[string][]ucan.Command{ "s3:CreateBucket": nil, "s3:ListAllMyBuckets": nil, "s3:DeleteBucket": cmdsDeleteBucket, + "s3:GetBucketPolicy": nil, + "s3:PutBucketPolicy": nil, + "s3:DeleteBucketPolicy": nil, // Multipart uploads. Initiating an upload, uploading a part and completing an // upload all require s3:PutObject, so they need no permission of their own. @@ -119,12 +124,55 @@ var permissionCommands = map[string][]ucan.Command{ "s3:ListBucketMultipartUploads": cmdsRetrieve, } +// policyActions is the allowlist of permissions a bucket policy statement may +// carry, sorted. The rest of the vocabulary stays out on purpose: a key holding +// s3:CreateBucket or s3:DeleteBucket acts outside the policy that granted it, a +// key holding a policy operation could rewrite the policy that granted it, and +// every principal holds s3:ListAllMyBuckets. A new permission is not a policy +// action until it is added here, which TestEveryPermissionIsClassified +// enforces. +var policyActions = []string{ + "s3:AbortMultipartUpload", + "s3:DeleteObject", + "s3:DeleteObjectVersion", + "s3:GetObject", + "s3:GetObjectLegalHold", + "s3:GetObjectRetention", + "s3:GetObjectVersion", + "s3:ListBucket", + "s3:ListBucketMultipartUploads", + "s3:ListBucketVersions", + "s3:ListMultipartUploadParts", + "s3:PutObject", + "s3:PutObjectLegalHold", + "s3:PutObjectRetention", +} + // Valid reports whether p is a recognized S3 permission. func Valid(p string) bool { _, ok := permissionCommands[p] return ok } +// PolicyAction reports whether p may appear in a bucket policy statement: one +// of the allowlisted policyActions. The wildcard PolicyWildcard is not a +// permission and is expanded by the policy package; it is not a PolicyAction. +func PolicyAction(p string) bool { + _, ok := slices.BinarySearch(policyActions, p) + return ok +} + +// PolicyWildcard is the action wildcard a bucket policy statement may carry in +// place of named actions. It stands for every permission PolicyAction accepts +// and never for a bucket-level action. +const PolicyWildcard = "s3:*" + +// PolicyActions returns a copy of the allowlist: every permission a bucket +// policy may grant, sorted, so that expanding PolicyWildcard is deterministic. +func PolicyActions() []string { + return slices.Clone(policyActions) +} + // CommandsFor returns the deduplicated set of Forge commands to delegate for the // given S3 permissions, preserving first-seen order. func CommandsFor(permissions ...string) []ucan.Command { diff --git a/pkg/s3perm/s3perm_internal_test.go b/pkg/s3perm/s3perm_internal_test.go new file mode 100644 index 00000000..37a88e2e --- /dev/null +++ b/pkg/s3perm/s3perm_internal_test.go @@ -0,0 +1,33 @@ +package s3perm + +import ( + "slices" + "testing" + + "github.com/stretchr/testify/require" +) + +// TestEveryPermissionIsClassified pins the policy allowlist against the +// permission table: every recognized permission is either a policy action or +// one of the six named here, so a new permission is not a policy action until +// someone lists it on one side. +func TestEveryPermissionIsClassified(t *testing.T) { + notPolicy := []string{ + "s3:CreateBucket", "s3:DeleteBucket", "s3:ListAllMyBuckets", + "s3:GetBucketPolicy", "s3:PutBucketPolicy", "s3:DeleteBucketPolicy", + } + require.True(t, slices.IsSorted(policyActions), "policyActions must be sorted") + for _, p := range policyActions { + require.True(t, Valid(p), "%q is listed as a policy action but is not a permission", p) + require.NotContains(t, notPolicy, p) + } + for p := range permissionCommands { + require.True(t, slices.Contains(policyActions, p) || slices.Contains(notPolicy, p), + "%q is neither a policy action nor listed here as excluded", p) + } + for _, p := range notPolicy { + require.True(t, Valid(p), p) + require.False(t, PolicyAction(p), p) + } + require.Len(t, policyActions, len(permissionCommands)-len(notPolicy)) +} diff --git a/pkg/s3perm/s3perm_test.go b/pkg/s3perm/s3perm_test.go index d57a6888..e5e21157 100644 --- a/pkg/s3perm/s3perm_test.go +++ b/pkg/s3perm/s3perm_test.go @@ -14,6 +14,7 @@ func TestValid(t *testing.T) { for _, p := range []string{ "s3:GetObject", "s3:PutObject", "s3:DeleteObject", "s3:CreateBucket", "s3:ListAllMyBuckets", "s3:DeleteBucket", + "s3:GetBucketPolicy", "s3:PutBucketPolicy", "s3:DeleteBucketPolicy", "s3:AbortMultipartUpload", "s3:ListMultipartUploadParts", "s3:ListBucketMultipartUploads", } { require.True(t, s3perm.Valid(p), p) @@ -104,7 +105,7 @@ func TestCommandsFor(t *testing.T) { }) t.Run("bucket-level permissions map to no commands", func(t *testing.T) { - require.Empty(t, strs("s3:CreateBucket", "s3:ListAllMyBuckets")) + require.Empty(t, strs("s3:CreateBucket", "s3:ListAllMyBuckets", "s3:GetBucketPolicy", "s3:PutBucketPolicy", "s3:DeleteBucketPolicy")) }) t.Run("deleting a bucket unwinds the blobs its space holds", func(t *testing.T) { @@ -171,6 +172,40 @@ func TestClassifiedPermissionsCoverTheirShapes(t *testing.T) { } } +// TestPolicyAction pins the policy vocabulary: every recognized permission except +// the three bucket-level actions, which a policy can neither grant nor withhold. +func TestPolicyAction(t *testing.T) { + for _, p := range []string{ + "s3:GetObject", "s3:PutObject", "s3:DeleteObject", "s3:ListBucket", + "s3:AbortMultipartUpload", "s3:ListMultipartUploadParts", "s3:ListBucketMultipartUploads", + } { + require.True(t, s3perm.PolicyAction(p), p) + } + for _, p := range []string{ + "s3:CreateBucket", "s3:DeleteBucket", "s3:ListAllMyBuckets", + "s3:GetBucketPolicy", "s3:PutBucketPolicy", "s3:DeleteBucketPolicy", + "", "s3:Frobnicate", "s3:getobject", + } { + require.False(t, s3perm.PolicyAction(p), p) + } +} + +// TestPolicyActions pins the wildcard expansion: sorted, every PolicyAction and +// nothing else. +func TestPolicyActions(t *testing.T) { + actions := s3perm.PolicyActions() + require.True(t, slices.IsSorted(actions)) + require.Len(t, actions, 14) + for _, p := range actions { + require.True(t, s3perm.PolicyAction(p), p) + } + for _, p := range []string{"s3:CreateBucket", "s3:DeleteBucket", "s3:ListAllMyBuckets", + "s3:GetBucketPolicy", "s3:PutBucketPolicy", "s3:DeleteBucketPolicy", s3perm.PolicyWildcard} { + require.NotContains(t, actions, p) + } + require.False(t, s3perm.PolicyAction(s3perm.PolicyWildcard)) +} + // TestOperationPermissionsAreValid keeps the two hardcoded permission lists in // lockstep: every permission an operation requires must be one this package can map // to Forge commands, otherwise an authorized request would be re-delegated nothing.