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

Refactor github repository module to use encrypted secret values #1421

Closed
3 tasks done
dms1981 opened this issue Jan 27, 2022 · 1 comment · Fixed by #1520
Closed
3 tasks done

Refactor github repository module to use encrypted secret values #1421

dms1981 opened this issue Jan 27, 2022 · 1 comment · Fixed by #1520
Assignees
Labels
enhancement New feature or request security technical debt This issue is either technical debt or an issue that will lead to technical debt as time goes by.

Comments

@dms1981
Copy link
Contributor

dms1981 commented Jan 27, 2022

User Story

As a modernisation platform engineer

I want to use encrypted_secret rather than plaintext_secret in the github repository module (terraform/github/modules/repository)

So that secret values are not accidentally exposed

Value

This issue is related to #133 ; this was brought to our attention by a Checkov test while adding details for a new application environment (PPUD). It might be enough to change the attribute reference for aws_secretmanager_version.$name.secret_string to aws_secretmanager_version.$name.secret_binary so that we get a base64 string that will be accepted by github_actions_secret.$name.encrypted_value but in all likelihood it will be more complicated than this.

Questions / Assumptions

  • Does CKV_GIT_4 meaningfully apply to this module?
  • Are we at risk of inadvertently exposing secret information?
  • Is automatic rotation of secrets enough to mitigate any risk from failing?
  • Who has access to the state files containing the plaintext values?

Definition of done

  • questions have been answered
  • code has been refactored so that CKV_GIT_4 passes
  • team member has reviewed pull request

Reference

How to write good user stories
CKV_GIT_4

@dms1981 dms1981 added enhancement New feature or request security labels Jan 27, 2022
@davidkelliott davidkelliott added the technical debt This issue is either technical debt or an issue that will lead to technical debt as time goes by. label Feb 1, 2022
@dms1981 dms1981 moved this from Backlog to In Progress in Modernisation Platform Mar 8, 2022
@dms1981 dms1981 self-assigned this Mar 8, 2022
@dms1981
Copy link
Contributor Author

dms1981 commented Mar 8, 2022

From some investigation it appears that this Checkov test may not be applicable to the situation where the test fails. There's a very informative comment in the Chekov repository - bridgecrewio/checkov#2374 - that discusses the context for this check.

It appears that while the secret is passed in plaintext when terraform runs, it is not stored in plaintext at rest.
Encrypted values are not stored in terraform state files, but as the state files themselves are controlled and restricted the risk of exposure here is low as only a limited subset of legitimate users have access to those state files.

Finally, scheduled rotation of secrets would be a positive step, but would also require some consideration outside this task.

@dms1981 dms1981 linked a pull request Mar 9, 2022 that will close this issue
Repository owner moved this from In Progress to Done in Modernisation Platform Mar 9, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request security technical debt This issue is either technical debt or an issue that will lead to technical debt as time goes by.
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

2 participants