Skip to content

Rake tasks to verify encrypted data through secrets

Catalin Irimie requested to merge 20069-ci-secrets-rake-task into master

What does this MR do?

Introduces one rake tasks: gitlab:doctor:secrets that scan all columns across all models that use attr_encrypted or our TokenAuthenticatable class and verifies if they can be decrypted, respectively "fixes" them (re-encrypts if the token is still available, or clears the value if not).

/cc @stanhu @nick.thomas @dblessing @dstanley @lbot @ashmckenzie for thoughts based on the discussion in #20069 (closed) 😁

Possible discussions:

  1. Should we also have specific data (or at least a --debug or some parameter that would show it) like IDs for bad values?

    Later edit: this was added through a VERBOSE flag consistent with other rake tasks.

  2. I've also considered Stan's thought about HA/Geo setups:

    Perhaps we need to do something similar where the first node stores a known encrypted value into a column, and all nodes should be able to decrypt it.

    As a first iteration, I think this should replace the scripts we're currently using in Support that are getting outdated (due to new/different columns with encrypted values).

    For HA/Geo I'd say we can add a new task that does exactly the above, store a key in Redis with ~1 hour or so expiration time if it doesn't exist, and then if it exists, attempt to decrypt it - so the rake task can be ran on all nodes in an HA/Geo setup, the first of them setting the key initially - if that makes sense I can create a new issue.

Does this MR meet the acceptance criteria?

Conformity

Availability and Testing

Security

If this MR contains changes to processing or storing of credentials or tokens, authorization and authentication methods and other items described in the security review guidelines:

  • Label as security and @ mention @gitlab-com/gl-security/appsec
  • The MR includes necessary changes to maintain consistency between UI, API, email, or other methods
  • Security reports checked/validated by a reviewer from the AppSec team
Edited by Catalin Irimie

Merge request reports