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.

Sign up for GitHub

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

🐛 fix aws.s3.bucket.policy resource #5228

Merged
merged 5 commits into from
Feb 21, 2025
Merged

🐛 fix aws.s3.bucket.policy resource #5228

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
75b2543
🐛 fix `aws.s3.bucket.policy` resource
afiune Feb 15, 2025
01b8dd5
🔄 do not create bucket policy if not exists
afiune Feb 20, 2025
99d3c83
🐛 set policy to `plugin.StateIsNull` when not found
afiune Feb 20, 2025
958b932
🐛 fix accessing aws.s3.bucket.policy
afiune Feb 21, 2025
d4dfcef
🧹 update aws.lr
afiune Feb 21, 2025
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
7 changes: 4 additions & 3 deletions providers/aws/resources/aws.lr
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1822,11 +1822,13 @@ private aws.s3.bucket.corsrule @defaults("name") {
}

// Amazon S3 bucket policy
private aws.s3.bucket.policy @defaults("name version") {
private aws.s3.bucket.policy @defaults("bucketName version") {
// Unique ID for the policy
id string
// Name for the policy
// Deprecated. Use `bucketName` instead
name string
// Bucket name that this policy belongs
bucketName string
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This might be a breaking change, but a bucket policy doesn't have a name. It belongs to a bucket and, to calculate the bucket policy ID we need the bucket name, so I re purpose this field.

If this is not acceptable, I am ok creating an internal struct to pass the bucket name but, we need to do something about this namefield that says it is the name of the policy, but it's not.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we don't know whether this is used by someone or not. Perhaps keep both for a while and add a comment to name that it's deprecated. We can remove it in next major release

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If someone is using it, they are using it incorrectly but I agree, I'll put it back with a deprecation.

// Document for the policy
document string
// Version of the policy
Expand All @@ -1835,7 +1837,6 @@ private aws.s3.bucket.policy @defaults("name version") {
statements() []dict
}


// AWS Application Auto Scaling
aws.applicationAutoscaling @defaults("namespace") {
init(namespace string)
Expand Down
14 changes: 13 additions & 1 deletion providers/aws/resources/aws.lr.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 4 additions & 0 deletions providers/aws/resources/aws.lr.manifest.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2845,7 +2845,11 @@ resources:
desc: |
Bucket policies grant permission to your Amazon S3 resources
fields:
bucketName:
min_mondoo_version: 9.0.0
document: {}
exists:
min_mondoo_version: 9.0.0
id: {}
name: {}
statements: {}
Expand Down
114 changes: 65 additions & 49 deletions providers/aws/resources/aws_s3.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,31 @@ func (a *mqlAwsS3) buckets() ([]interface{}, error) {
return res, nil
}

func initAwsS3BucketPolicy(runtime *plugin.Runtime, args map[string]*llx.RawData) (map[string]*llx.RawData, plugin.Resource, error) {
// reuse the init func for the bucket
_, s3bucketResource, err := initAwsS3Bucket(runtime, args)
if err != nil {
return args, nil, err
}
// then use it to get its policy
policyResource := s3bucketResource.(*mqlAwsS3Bucket).GetPolicy()
if policyResource != nil {
return args, policyResource.Data, nil
}

// no policy found
resource := &mqlAwsS3BucketPolicy{}
resource.Id.State = plugin.StateIsNull | plugin.StateIsSet
resource.Name.State = plugin.StateIsNull | plugin.StateIsSet
resource.Document.State = plugin.StateIsNull | plugin.StateIsSet
resource.Version.State = plugin.StateIsNull | plugin.StateIsSet
resource.Statements.State = plugin.StateIsNull | plugin.StateIsSet
resource.BucketName = plugin.TValue[string]{
Data: s3bucketResource.(*mqlAwsS3Bucket).GetName().Data, State: plugin.StateIsSet,
}
return args, resource, nil
}

func initAwsS3Bucket(runtime *plugin.Runtime, args map[string]*llx.RawData) (map[string]*llx.RawData, plugin.Resource, error) {
// NOTE: bucket only initializes with arn and name
if len(args) >= 2 {
Expand All @@ -133,6 +158,9 @@ func initAwsS3Bucket(runtime *plugin.Runtime, args map[string]*llx.RawData) (map
var arn string
if args["arn"] != nil {
arn = args["arn"].Value.(string)
if !strings.HasPrefix(arn, "arn:aws:s3:") {
return nil, nil, errors.Newf("not a valid bucket ARN '%s'", arn)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

During testing I detected that, if you are NOT in discovery and, you try to access aws.s3.bucket then we try to "create a bucket resource" of the AWS account, which is incorrect.

Now, we will exit with helpful error message:

cnquery> aws.s3.bucket
not a valid bucket ARN 'arn:aws:sts::123456789012'
aws.s3.bucket: no data available

}
} else {
nameVal := args["name"].Value.(string)
arn = fmt.Sprintf(s3ArnPattern, nameVal)
Expand Down Expand Up @@ -180,20 +208,6 @@ func (a *mqlAwsS3Bucket) id() (string, error) {
return a.Arn.Data, nil
}

func emptyAwsS3BucketPolicy(runtime *plugin.Runtime) (*mqlAwsS3BucketPolicy, error) {
res, err := CreateResource(runtime, "aws.s3.bucket.policy", map[string]*llx.RawData{
"name": llx.StringData(""),
"document": llx.StringData("{}"),
"version": llx.StringData(""),
"id": llx.StringData(""),
"statements": llx.ArrayData([]interface{}{}, types.Dict),
})
if err != nil {
return nil, err
}
return res.(*mqlAwsS3BucketPolicy), nil
}

func (a *mqlAwsS3Bucket) policy() (*mqlAwsS3BucketPolicy, error) {
conn := a.MqlRuntime.Connection.(*connection.AwsConnection)

Expand All @@ -207,26 +221,35 @@ func (a *mqlAwsS3Bucket) policy() (*mqlAwsS3BucketPolicy, error) {
})
if err != nil {
if isNotFoundForS3(err) {
return emptyAwsS3BucketPolicy(a.MqlRuntime)
a.Policy.State = plugin.StateIsNull | plugin.StateIsSet
return nil, nil
}
return nil, err
}

if policy != nil && policy.Policy != nil {
parsedPolicy, err := parseS3BucketPolicy(*policy.Policy)
if err != nil {
return nil, err
}
// create the policy resource
mqlS3BucketPolicy, err := CreateResource(a.MqlRuntime, "aws.s3.bucket.policy",
map[string]*llx.RawData{
"name": llx.StringData(bucketname),
"document": llx.StringDataPtr(policy.Policy),
"id": llx.StringData(parsedPolicy.Id),
"name": llx.StringData(bucketname),
"bucketName": llx.StringData(bucketname),
"version": llx.StringData(parsedPolicy.Version),
"document": llx.StringDataPtr(policy.Policy),
})
if err != nil {
return nil, err
}

return mqlS3BucketPolicy.(*mqlAwsS3BucketPolicy), nil
}

// no bucket policy found, return nil for the policy
return emptyAwsS3BucketPolicy(a.MqlRuntime)
return nil, nil
}

func (a *mqlAwsS3Bucket) tags() (map[string]interface{}, error) {
Expand Down Expand Up @@ -438,7 +461,7 @@ func (a *mqlAwsS3Bucket) public() (bool, error) {
statusOutput, err := svc.GetBucketPolicyStatus(ctx, &s3.GetBucketPolicyStatusInput{
Bucket: &bucketname,
})
if err != nil {
if err != nil && !isNotFoundForS3(err) {
return false, err
}
if statusOutput != nil &&
Expand All @@ -448,23 +471,21 @@ func (a *mqlAwsS3Bucket) public() (bool, error) {
}

// If that didn't work, fetch the bucket policy manually and parse it
bucketPolicyResource, err := a.policy()
if err != nil {
return false, err
}

bucketPolicy, err := bucketPolicyResource.parsePolicyDocument()
if err != nil {
return false, err
}

for _, statement := range bucketPolicy.Statements {
if statement.Effect != "Allow" {
continue
bucketPolicyResource := a.GetPolicy()
if bucketPolicyResource.State == plugin.StateIsSet {
bucketPolicy, err := bucketPolicyResource.Data.parsePolicyDocument()
if err != nil {
return false, err
}
if awsPrincipal, ok := statement.Principal["AWS"]; ok {
if slices.Contains(awsPrincipal, "*") {
return true, nil

for _, statement := range bucketPolicy.Statements {
if statement.Effect != "Allow" {
continue
}
if awsPrincipal, ok := statement.Principal["AWS"]; ok {
if slices.Contains(awsPrincipal, "*") {
return true, nil
}
}
}
}
Expand Down Expand Up @@ -698,25 +719,20 @@ func (a *mqlAwsS3BucketCorsrule) id() (string, error) {
}

func (a *mqlAwsS3BucketPolicy) id() (string, error) {
policy, err := a.parsePolicyDocument()
if err != nil || policy == nil {
return "none", err
}

a.Id = plugin.TValue[string]{Data: policy.Id, State: plugin.StateIsSet}
return policy.Id, nil
// NOTE that `policy.Id` might or might not exist and,
// it is NOT unique for s3 bucket policies. what we need
// here is the bucket name, which is unique globally.
return fmt.Sprintf("aws.s3.bucket/%s/policy", a.BucketName.Data), nil
}

func (a *mqlAwsS3BucketPolicy) parsePolicyDocument() (*awspolicy.S3BucketPolicy, error) {
data := a.Document.Data
return parseS3BucketPolicy(a.Document.Data)
}

func parseS3BucketPolicy(document string) (*awspolicy.S3BucketPolicy, error) {
var policy awspolicy.S3BucketPolicy
err := json.Unmarshal([]byte(data), &policy)
if err != nil {
return nil, err
}

return &policy, nil
err := json.Unmarshal([]byte(document), &policy)
return &policy, err
}

func (a *mqlAwsS3BucketPolicy) version() (string, error) {
Expand Down