[BUG][FG_R00276] - It doesn't fail if we didn't set the source_account

[rsareth]

Hi,

We are still testing regula on a huge amount of repositories. So we have so many lines of code and problems detected to analyze. That's why we are creating issues by analyzing the regula results.

A - Description
We think there might a bug in the rule FG_R00276 - Lambda function policies should not allow global access. Regula doesn't fail when it applies the rule but in our AWS Config console, we are seeing an alarm: securityhub-lambda-function-public-access-prohibited The description is there: [Lambda.1] Lambda function policies should prohibit public access Of course, we might be wrong on the understanding of both rules.

B - Context
In the same AWS subscription, we have created a s3 bucket that invokes our lambda. Both resources are created in the same repository. But in our aws_lambda_permission resource, we have only set the source_arn and we haven't set the source_account.

C - Version

$ regula version
v1.3.2, build baa7df6, built with OPA v0.28.0

D - Test
Put the below code in the file lambda_a.tf

resource "aws_s3_bucket" "bucket_a" {

  bucket = "bucket_a"

  server_side_encryption_configuration {
    rule {
      apply_server_side_encryption_by_default {
        sse_algorithm = "AES256"
      }
    }
  }

}

resource "aws_s3_bucket_public_access_block" "bucket_a" {
  bucket                  = aws_s3_bucket.bucket_a.id
  block_public_acls       = true
  block_public_policy     = true
  ignore_public_acls      = true
  restrict_public_buckets = true
}

resource "aws_s3_bucket_policy" "bucket_a" {
  depends_on = [aws_s3_bucket_public_access_block.bucket_a]

  bucket = aws_s3_bucket.bucket_a.id
  policy = jsonencode({
    Version = "2012-10-17"
    Statement : [
      {
        Sid       = "DenyUnsecureAccess"
        Effect    = "Deny"
        Principal = "*"
        Action    = "s3:*"
        Resource = [
          aws_s3_bucket.bucket_a.arn,
          "${aws_s3_bucket.bucket_a.arn}/*",
        ]
        Condition = {
          Bool = {
            "aws:SecureTransport" = "false"
          }
        }
      }
    ]
  })
}

resource "aws_s3_bucket_notification" "bucket_a" {
  bucket = aws_s3_bucket.bucket_a.id

  lambda_function {
    lambda_function_arn = aws_lambda_function.lambda_a.arn
    events = [
      "s3:ObjectCreated:*"
    ]
  }

  depends_on = [aws_lambda_permission.lambda_a]
}

resource "aws_iam_role" "lambda_a" {
  name = "lambda_a-exec"

  assume_role_policy = <<EOF
{
  "Version": "2012-10-17",
  "Statement": [
    {
      "Action": "sts:AssumeRole",
      "Principal": {
        "Service": "lambda.amazonaws.com"
      },
      "Effect": "Allow",
      "Sid": ""
    }
  ]
}
EOF

}

resource "aws_lambda_function" "lambda_a" {
  function_name = "lambda_a"
  role          = aws_iam_role.lambda_a.arn
  handler       = "com.company.LambdaHandler::handleRequest"
  runtime       = "java11"
  timeout       = 30
  memory_size   = 1048
}

data "aws_caller_identity" "current" {
}

resource "aws_lambda_permission" "lambda_a" { # <---- We should get an error detected on this resource
  statement_id   = "AllowExecutionFromS3Bucket"
  action         = "lambda:InvokeFunction"
  function_name  = aws_lambda_function.lambda_a.arn
  principal      = "s3.amazonaws.com"
  source_arn     = aws_s3_bucket.bucket_a.arn
  #source_account = data.aws_caller_identity.current.account_id # <--- Following the AWS best practices, we need to set the source_account
}

Command to run and the output:

$ regula run regula/lambda_a.tf

FG_R00101: S3 bucket versioning and lifecycle policies should be enabled [Medium]
           https://docs.fugue.co/FG_R00101.html

  [1]: aws_s3_bucket.bucket_a
       in regula/lambda_a.tf:1:1

FG_R00274: S3 bucket access logging should be enabled [Medium]
           https://docs.fugue.co/FG_R00274.html

  [1]: aws_s3_bucket.bucket_a
       in regula/lambda_a.tf:1:1

FG_R00275: S3 bucket replication (cross-region or same-region) should be enabled [Medium]
           https://docs.fugue.co/FG_R00275.html

  [1]: aws_s3_bucket.bucket_a
       in regula/lambda_a.tf:1:1

Found 3 problems.

We expected to see a problem for the rule FG_R00276 but nothing.

E - AWS Web Console
And in the console, the bad policy looks like this:

{
  "Version": "2012-10-17",
  "Id": "default",
  "Statement": [
    {
      "Sid": "AllowExecutionFromS3Bucket",
      "Effect": "Allow",
      "Principal": {
        "Service": "s3.amazonaws.com"
      },
      "Action": "lambda:InvokeFunction",
      "Resource": "arn:aws:lambda:eu-west-1:<AWS_ACCOUNT_ID>:function:lambda_a",
      "Condition": {
        "ArnLike": {
          "AWS:SourceArn": "arn:aws:s3:::bucket_a"
        }
      }
    }
  ]
}

instead of:

{
  "Version": "2012-10-17",
  "Id": "default",
  "Statement": [
    {
      "Sid": "AllowExecutionFromS3Bucket",
      "Effect": "Allow",
      "Principal": {
        "Service": "s3.amazonaws.com"
      },
      "Action": "lambda:InvokeFunction",
      "Resource": "arn:aws:lambda:us-east-1:<AWS_ACCOUNT_ID>:function:lambda_a",
      "Condition": {
        "StringEquals": {
          "AWS:SourceAccount": "<AWS_ACCOUNT_ID>"
        },
        "ArnLike": {
          "AWS:SourceArn": "arn:aws:s3:::bucket_a"
        }
      }
    }
  ]
}

What do you think ?

Thank you in advance.

Regards,
Rasmey

[rsareth]

BTW, we don't know how to read properly rego code. The rule FG_R00276 is described in here: https://github.com/fugue/regula/blob/master/rego/rules/tf/aws/lambda/function_not_public.rego

[jason-fugue]

Hi, @rsareth! Thank you so much again for using Regula and opening these issues. It's incredibly helpful for us! There's definitely a discrepancy between our interpretation of "publicly accessible" and Security Hub's. Right now, we're just checking for Principal set to *. The key line is here: https://github.com/fugue/regula/blob/master/rego/rules/tf/aws/lambda/function_not_public.rego#L64

From the Security Hub docs, it looks like lambda-function-public-access-prohibited is both checking for Principal set to * and enforcing the SourceAccount condition that you pointed out. I'll reply back after I've discussed this issue with our team.

[jason-fugue]

We think this might be a bug in that Security Hub rule or, at the very least, a discrepancy between their docs and their rules. In their remediation steps for that rule, they say:

To remediate the issue, you must update the policy to remove the permissions or to add the AWS:SourceAccount condition.

Which sounds like the intent is to deny when Principal is set to * unless an AWS:SourceAccount condition is present. From our perspective, it doesn't make sense to always require that condition.

After discussing this issue, we do intend to change our rule to allow Principal set to * when that condition is present. That will make us consistent with Security Hub's documentation, but unfortunately we'll still be inconsistent with this particular rule result.

If you just want to avoid triggering that alarm, you could enforce that condition with a small custom rule that you include with the --include option on regula run:

package rules.lambda_permission_needs_source_account

__rego__metadoc__ := {
  "custom": {
    "severity": "Medium"
  },
  "title": "Lambda permission resources should always have a condition for AWS:SourceAccount"
}

resource_type = "aws_lambda_permission"

default allow = false

allow {
  input.source_account != ""
}

for example if that rule was saved as rules/lambda_permission_needs_source_account.rego, you would use it like:

regula run --include rules/lambda_permission_needs_source_account.rego <your inputs>

# or you can just specify the parent directory
regula run --include rules <your inputs>

[jason-fugue]

Sorry to keep spamming on this ticket: after discussing this issue more, it does seem like we should have a rule that enforces AWS:SourceArn and AWS:SourceAccount conditions on lambda permissions in some cases like in your example. We'll probably implement this as a new rule instead of changing the behavior of FG_R00276. I'll reference this ticket when we PR that change.

[rsareth]

Hi @jason-fugue

Thank you for your answer. But if we understand it properly, this situation annoys us a little bit.

If you are right, AWS needs to fix their managed rule, lambda-function-public-access-prohibited, and/or update their documentation. In that case, your rego rule looks like a temporary technical workaround. How can we notify AWS to do something properly ? Have you ever talk to them about a discrepancy between their rules and their docs ? Can they fix quickly ? In our case, those rules are activated by another team that creates AWS subscriptions for us. The subscriptions are pre-configured / pre-provisionned with mandatory stuff that we can't say anything about them.

Anyway, having this rule will make us to think of how to integrate custom rules in our ci/cd workflows.

Regards,
Rasmey

[rsareth]

Sorry to keep spamming on this ticket: after discussing this issue more, it does seem like we should have a rule that enforces AWS:SourceArn and AWS:SourceAccount conditions on lambda permissions in some cases like in your example. We'll probably implement this as a new rule instead of changing the behavior of FG_R00276. I'll reference this ticket when we PR that change.

So, we posted a response at the same time. Don't read that much my response but I prefer to let it be for transparency.

Thank you again

Rasmey

[jason-fugue]

Hi, @rsareth! We added a new rule (FG_R00499) in the latest v1.6.0 release that checks for source ARN and source account conditions on lambda permissions that use a service principal. Sorry it took a while to get it scheduled and written. Please let us know if that resolves this issue whenever you get a chance.

[rsareth]

Hi @jason-fugue,

Thank you for the new release. I've tried the release with the same test provided previously but it didn't raise the error for the rule FG_R00499.

Also, I wanted to run the make command on the tag v1.6.0 and it failed:

$ make test
go test -v -cover ./...
?   	github.com/fugue/regula	[no test files]
?   	github.com/fugue/regula/cmd	[no test files]
?   	github.com/fugue/regula/pkg/git	[no test files]
=== RUN   TestCfnDetector
--- PASS: TestCfnDetector (0.00s)
=== RUN   TestCfnDetectorNotCfnContents
--- PASS: TestCfnDetectorNotCfnContents (0.00s)
[...]
=== RUN   TestTf
    tf_test.go:54:
[...]
--- FAIL: TestTf (0.06s)
[...]
--- FAIL: TestTfResourceLocation (0.00s)
panic: runtime error: invalid memory address or nil pointer dereference [recovered]
	panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x48 pc=0x14003c0]

goroutine 9 [running]:
testing.tRunner.func1.2({0x146a040, 0x190c9d0})
	/usr/local/Cellar/go/1.17.2/libexec/src/testing/testing.go:1209 +0x24e
[...]
coverage: 97.1% of statements
ok  	github.com/fugue/regula/pkg/topsort	(cached)	coverage: 97.1% of statements
?   	github.com/fugue/regula/pkg/version	[no test files]
?   	github.com/fugue/regula/rego	[no test files]
FAIL
make: *** [test] Error 1

I'm not really sure if it is my mac. But the README.md doesn't explain the requirements to have for building and testing regula from the source code.

[jason-fugue]

Hi, @rsareth! The reason your configuration still did not trigger a failure is because of a deficiency in how we relate lambda functions and permissions. It would trigger a failure if you changed the function_name property to the lambda's function_name property instead of aws_lambda_function.lambda_a.arn, like:

resource "aws_lambda_permission" "lambda_a" { # <---- We should get an error detected on this resource
  statement_id   = "AllowExecutionFromS3Bucket"
  action         = "lambda:InvokeFunction"
  function_name  = aws_lambda_function.lambda_a.function_name
  principal      = "s3.amazonaws.com"
  source_arn     = aws_s3_bucket.bucket_a.arn
  #source_account = data.aws_caller_identity.current.account_id # <--- Following the AWS best practices, we need to set the source_account
}

Using an ARN there is valid and there are some other ways to specify function_name that we wouldn't catch in the current version. We'll need to make an improvement there.

For your build failure - there's a git submodule that we use in one of the test suites. You'll need to run this first to fetch it:

git submodule update --init --recursive

Our current README.md is more tailored for users rather than contributors. It probably makes sense for us to add a separate document (linked from the readme) that covers how to develop regula.

[rsareth]

Hi @jason-fugue,

Thank you for the quick answer. First, I can confirm that I was able to rebuild from the source code. I hope you will provide a contributor markdown file.

Second, after your explanation, I can confirm that everything is ok. I check in our source code, we can see that we are using the arn instead of the function name. I guess in a previous version of the provider, it was the arn that was needed to set.

We hope that you can provide a fix that can handle both the arn and the function name of the lambda. BTW, you don't provide the link to your rule web page. I didn't see the web page. I saw a 404 page. When do you think you can publish this web page ?

Do you want me to close the ticket or should we let open ?

Thank you again,
Rasmey

[jason-fugue]

Hi, @rsareth! I think it makes sense to leave this issue open until we make the change to accommodate function ARNs and the other valid values in the lambda permission function name field.

I'll reply back once I've got an time estimate for adding documentation for this rule.

[jason-fugue]

Hi again, @rsareth! Sorry for the very long turnaround on this issue. The latest release (v2.1.0) has a fix to support the use of other function properties (like arn) in the function_name field on lambda permissions. It also has a remediation doc link for the new FG_R00499 rule that we added a while back. Could you please give it a try whenever you get a chance and see if it resolves this issue?

[rsareth]

Hi @jason-fugue,

I can confirm that the 2 issues are solved with regula v2.1.0. Thank you for the fixes!

Rasmey