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

Add configuration for secret related checks #3905

Closed
pr3l14t0r opened this issue Apr 30, 2024 · 13 comments · Fixed by #4915
Closed

Add configuration for secret related checks #3905

pr3l14t0r opened this issue Apr 30, 2024 · 13 comments · Fixed by #4915
Assignees
Labels
feature-request New feature request for Prowler. provider/aws Issues/PRs related with the AWS provider

Comments

@pr3l14t0r
Copy link

New feature motivation

All checks related to secret detection lead to a massive amount of false positives. This could get prevented if there would be a possibility to configure those checks, since most of them use the detect-secrets package.

Use case:
The checks awslambda_function_no_secrets_in_variables and ecs_task_definitions_no_environment_secrets do use the detect-secrets package.

Unfortunately there is no possibility (at least no documented one) that allows to configure the checks done with that package. The repository documents how you can use a customized settings file, which can for example adjust the threshold of Base64HighEntropyString etc.

Could it be possible to add the JSON like config to the prowler's config.yaml?

Furthermore it would be nice to have an option to filter out environment variable names.

Example:
In an AWS TaskDefinition i use a variable named GCP_SECRET_ARN. This contains an ARN to AWS_SECRETS_MANAGER service which will be used during runtime of a script to retrieve a JSON, since it is not really an option to mount a JSON with newline characters into an environment variable as secret. Thus i would like to filter out for name patterns like "*SECRET_ARN".
Would this make sense to you, too?

As documented for detect-secrets, one could use the following parameter:

  --exclude-secrets EXCLUDE_SECRETS
                        If secrets match this regex, it will be ignored.

This could already help me out for my use-case.

Solution Proposed

Add an option to the config.yaml that would apply to all scans which use detect-secrets.

My pseudo example:

aws:
  # configure entropy threshold
  awslambda_function_no_secrets_in_variables_Base64HighEntropyString_limit: 7.0
  # Configure a  regex-based ignore-pattern for variable names
  ecs_task_definitions_no_environment_secrets_ignore_patterns:
    - ".*SECRET_ARN"
    - ".*PASSWORD_ARN"

And then fetch those values in the corresponding python scripts...

I know: configuring this is more a global option than a "per-check" option. So maybe it makes even more sense to create a default config-file for detect-secrets which someone could adjust. Just as with the yaml files.

Describe alternatives you've considered

I did consider to overwrite the python scripts for the mentioned checks, but this is somewhat unfeasible to have it within a deployment CI-CD deployment.. It would be more easy if i could just specify or change a global settings file for the detect-secrets package.

Additional context

Maybe i have overseen something in the documentation or something in the code itself. So if there already is a solution for my presented use cases, i would already like to apologize.

@pr3l14t0r pr3l14t0r added feature-request New feature request for Prowler. status/needs-triage Issue pending triage labels Apr 30, 2024
@toniblyx
Copy link
Member

Thanks @pr3l14t0r, this is a good idea to enhance our Mutelist feature! what do you think @jfagoagas @sergargar ?

@jfagoagas
Copy link
Member

jfagoagas commented May 7, 2024

Hi @pr3l14t0r, I'm going to answer you inline:

  • Regarding the detect-secrets Python package configuration, as far as we know there is no way to configure BASE64_LIMIT and HEX_LIMIT but we can find a workaround to do that.
  • About the environment variables I think the Mutelist should be the way to filter out those environment variables but if the value within is an ARN, detect-secrets shouldn't report this as a secret.

Thanks for using Prowler 🚀

@pr3l14t0r
Copy link
Author

pr3l14t0r commented May 8, 2024

[..]but if the value within is an ARN, detect-secrets shouldn't report this as a secret.

@jfagoagas Well... the check ecs_task_definitions_no_environment_secrets detects a secret in my task-definition config with the following STATUS_EXTENDED explanation:

Potential secret found in variables of ECS task definition REDACTED-name-of-taskdefinition-resource with revision 1 -> Secret Keyword on line 2.

I don't know what exactly the Secret Keyword on line 2 is referring to, but my environment variables are the following:

            "environment": [
                {
                    "name": "GCP_SECRET_ARN",
                    "value": "arn:aws:secretsmanager:<aws-region>:<aws-account-id>:secret:<name-of-my-secret>"
                },
                {
                    "name": "S3_BUCKET",
                    "value": "<REDACTED-name-of-my-output-bucket>"
                },
                {
                    "name": "AWS_STACK",
                    "value": "<name-of-my-aws-stack>"
                }
            ]

does it get flagged because if have named the environment variable GCP_SECRET_ARN - containing the term *secret* ?

@pr3l14t0r
Copy link
Author

About the environment variables I think the Mutelist should be the way to filter out those environment variables

That's not possible at the moment... In my example from above the ResourceName is the name of the TaskDefinition, which makes sense. But this won't allow me to filter for the specific variable name

@jfagoagas jfagoagas self-assigned this May 8, 2024
@jfagoagas
Copy link
Member

Hi @pr3l14t0r,

I don't know what exactly the Secret Keyword on line 2 is referring to, but my environment variables are the following:

That line number comes from the AWS ECS API response, sometimes there are differences between the format returned by the API and how is shown in the AWS Console.

does it get flagged because if have named the environment variable GCP_SECRET_ARN - containing the term secret ?

I need to review the detect-secrets engine but it seems to be the reason and also due to the format of the value.

That's not possible at the moment... In my example from above the ResourceName is the name of the TaskDefinition, which makes sense. But this won't allow me to filter for the specific variable name

You are right, we have identified some other scenarios like this and we are thinking about ways to improve the mutelist to allow combinations but for that we need to include more in the finding object, like the secret name for this use case.

The only I can recommend you now is to move that secret from the environment object in the ECS Task Definition to the secrets key. With that, ECS will get the secret value and inject it into the container's runtime as an environment variable but that would need you to change a little bit your logic since I suppose you are calling the AWS Secrets Manager API from your code to retrieve that secret value. You can read more about here.

Another option we are evaluating is to include the configuration you suggested with ecs_task_definitions_no_environment_secrets_ignore_patterns or something similar.

Thanks for using Prowler 🚀

@jfagoagas jfagoagas added the provider/aws Issues/PRs related with the AWS provider label May 9, 2024
@jchrisfarris
Copy link
Contributor

I came here to open a similar issue - Specifically around the keys that are considered secret in Lambda envars

Potential secret found in Lambda function guardduty-to-splunk-lambda variables -> Secret Keyword in variable SECRET_REGION.

Just because I have the word "SECRET" doesn't mean there is a secret. In this case I'm just pointing my function to Secrets Manager.

I'm not sure the Mutelist would help. As currently configured, the mutelist would allow you to mute specific resources, not attributes of a specific resource. This does seem to fit better as an array of IGNORE_SECRET_WORDS

@pedrooot
Copy link
Member

pedrooot commented Sep 2, 2024

Hi! @pr3l14t0r you can see the final approach for this issue here. Finally, we think that the best way to go is to improve the config from detect_secrets. Please, try it and tell me your thoughts from this to make improvements. Thanks!

P.D. In any case, we do not rule out adding additional configuration to the checks you mentioned.

@pedrooot
Copy link
Member

pedrooot commented Sep 4, 2024

hey! @pr3l14t0r @jchrisfarris finally we added a field on the config.yaml file to manage your known secrets. On secrets_ignore_patterns you can add your regex patterns to ignore from secrets detection. Thanks for the idea!

@pr3l14t0r
Copy link
Author

Heyho @pedrooot !!
This is awesome! I will try that out as soon as i can. :)
Thank you so much for your efforts!

@pr3l14t0r
Copy link
Author

pr3l14t0r commented Oct 10, 2024

Heyho @pedrooot !
I was able to test this feature now, sorry for the late response on this.

The feature is available in version 4.4.0, right? Because while not being mentioned on the docs page the configuration seems to work when using it there. But i have tested also on 4.5.0 building it from master.

While testing, i have encountered a few errors and the results would be wrong.

Example: Let's say i have a lambda with a variable configured like this:

{
  "Variables": {
    "DATABASE_PASSWORD": "123456789ABCEF"
    "SOME_API_KEY": "1234-4567-890-1234"
  }
}

Both 4.4.0 and 4.5.0 would flag these variables in check awslambda_function_no_secrets_in_variables. So far, so good.

Now i run the same thing again with using a config that looks like this:

  secrets_ignore_patterns:
    [
      "*ARN*",
      "*SECRET_ARN*",
      "arn:aws:secretsmanager",
      "secretsmanager",
      "arn:aws:ssm"
    ]

Suddenly, all the secret-related checks do pass. And when i check the results for the functions' ARN, i see this in the description:

Description     : No secrets found in Lambda function FUNCTIONNAME variables.

While the secrets should technically have been detected.

Furthermore during runtime, i see a lot of these errors when i run with the configuration:

[...]
2024-10-10 12:28:16,891 [File: utils.py:172]    [Module: utils]  ERROR: Error scanning for secrets: nothing to repeat at position 0

2024-10-10 12:28:16,901 [File: utils.py:172]    [Module: utils]  ERROR: Error scanning for secrets: nothing to repeat at position 0

2024-10-10 12:28:16,916 [File: utils.py:172]    [Module: utils]  ERROR: Error scanning for secrets: nothing to repeat at position 0
[...]

There are like hundreds of these lines.. Both on 4.4.0 and 4.5.0

So unfortunately the configuration does not work for me, and even more bad: Findings won't get flagged correctly using the configuration...

Any idea how i can sort this out? If it helps i can create a new issue for this.

Thanks for any help in advance. :)

Cheers!

@jfagoagas
Copy link
Member

jfagoagas commented Oct 10, 2024

Hello @pr3l14t0r the issue here is that you are setting a list of regexp starting with an * which is not valid since the * means "repeat the previous character N times. So for that regexp list to work you'd need to adjust it to:

secrets_ignore_patterns:
    [
      ".*ARN",
      ".*SECRET_ARN",
      "arn:aws:secretsmanager",
      "secretsmanager",
      "arn:aws:ssm"
    ]

Could you please try that? We will need also to add validation to inform that the regexp is not valid.

The secrets_ignore_patterns is present in the Prowler documentation under the configuration file: https://docs.prowler.com/projects/prowler-open-source/en/latest/tutorials/configuration_file/

@jfagoagas jfagoagas removed the status/needs-triage Issue pending triage label Oct 10, 2024
@pr3l14t0r
Copy link
Author

pr3l14t0r commented Oct 10, 2024

@jfagoagas thank you for your response!

The secrets_ignore_patterns is present in the Prowler documentation under the configuration file

Ah.. sorry, i must have completely overseen that... 🤦🏼 So stupid..

I am quite certain that i have already tested without the asterisks.. I will invoke another test according to your guidance and will let you know. :)

Thanks again for your response, you guys are awesome!

@jfagoagas
Copy link
Member

jfagoagas commented Oct 10, 2024

@jfagoagas thank you for your response!

The secrets_ignore_patterns is present in the Prowler documentation under the configuration file

Ah.. sorry, i must have completely overseen that... 🤦🏼 So stupid..

It is fine, we are here to always help you!

I am quite certain that i have already tested without the asterisks.. I will invoke another test according to your guidance and will let you know. :)

Thanks again for your response, you guys are awesome!

Thank you 🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature-request New feature request for Prowler. provider/aws Issues/PRs related with the AWS provider
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants