Improve secret management

[marcoieni]

Current situation

At the moment, we store secrets in AWS KMS or GitHub Actions secrets.

KMS:

• Pros:
  • logging into aws leaves a trail
• Cons:
  • an admin can read the value of the secret easily without leaving a trail

GitHub Actions secrets:

• Pros:
  • GitHub secrets are not readable from the UI
• Cons:
  • One can read GitHub secrets by pushing code that edits them and print them (e.g. write the secret to a file, separate each character with a , and print the content of the file). In theory this leaves a trail in GitHub Actions logs, but an admin can delete a workflow run from the UI.

Solution

Consider using a secret manager that audits when secrets are accessed. E.g. GitHub Actions read secrets from this manager and the manager logs that a secret was accessed from the action.

Example: AWS Secrets Manager

Context

discussion started in zulip#t-infra > meeting 2025-03-24 @ 💬

[Kobzol]

Note that even admins won't be able to push to master. So they have to create a PR to modify code, which leaves a trail. In team, this would automatically ping all other admins. Also, to access the secrets (which will be protected by a GitHub environment), the PR would have to be approved and moved to a merge queue (but this is not so hard, as a compromised admin account could just send the PR from a different GH account and then approve it themselves, unless we will require 2 admin approvals for CI changes).

[marcoieni]

So they have to create a PR to modify code

what if they just push to a branch without creating a PR?

[Kobzol]

We could forbid that with branch protections, but if we use environment secrets properly, then the branch won't get access to the secrets until it gets to the merge queue, and it shouldn't be possible to add stuff to the merge queue without a PR.

[Mark-Simulacrum]

A few notes:

We currently primarily use AWS Parameter Store for secrets (but not exclusively! e.g. the Rust signing key is stored in an S3 bucket with restricted bucket-level resource policies, it's also stored in 1pass and a few other places probably).

All KMS actions do persist into CloudTrail a record of actions (https://docs.aws.amazon.com/kms/latest/developerguide/logging-using-cloudtrail.html):

CloudTrail logs all AWS KMS operations, including read-only operations, such as ListAliases and GetKeyRotationStatus, operations that manage KMS keys, such as CreateKey and PutKeyPolicy, and cryptographic operations, such as GenerateDataKey and Decrypt. It also logs internal operations that AWS KMS calls for you, such as DeleteExpiredKeyMaterial, DeleteKey, SynchronizeMultiRegionKey, and RotateKey.

That includes transitive calls through other services (e.g., Parameter Store secrets).

This is currently retained for 90 days ("You can view, search, and download the most recent 90-day history of your account’s control plane activity at no additional cost using CloudTrail in the CloudTrail console." pricing) in our accounts but can be retained up to our desires if we opt-in to delivering to S3 the raw data ($0.10 per 100,000 data events delivered + object storage costs, likely pretty trivial in our case).

I didn't look for a GitHub-actions specific case, but e.g. when the docs-rs ECS task started it logged this event to decrypt the AWS SSM Parameter Store Secret for the environment variable.

{
    "eventVersion": "1.11",
    "userIdentity": {
        "type": "AssumedRole",
        "principalId": "[... redacted by Mark ...]:71dbc24fd3ab4c55941c8d81a3e0e7ee",
        "arn": "arn:aws:sts::[... redacted by Mark ...]:assumed-role/ecs-task-execution--docs-rs-web/71dbc24fd3ab4c55941c8d81a3e0e7ee",
        "accountId": "[... redacted by Mark ...]",
        "accessKeyId": "[... redacted by Mark ...]",
        "sessionContext": {
            "sessionIssuer": {
                "type": "Role",
                "principalId": "AROA46X5W6CZE35K4QS44",
                "arn": "arn:aws:iam::[... redacted by Mark ...]:role/ecs-task-execution--docs-rs-web",
                "accountId": "[... redacted by Mark ...]",
                "userName": "ecs-task-execution--docs-rs-web"
            },
            "attributes": {
                "creationDate": "2025-03-25T00:04:31Z",
                "mfaAuthenticated": "false"
            }
        },
        "invokedBy": "AWS Internal"
    },
    "eventTime": "2025-03-25T00:04:32Z",
    "eventSource": "kms.amazonaws.com",
    "eventName": "Decrypt",
    "awsRegion": "us-west-1",
    "sourceIPAddress": "AWS Internal",
    "userAgent": "AWS Internal",
    "requestParameters": {
        "encryptionContext": {
            "PARAMETER_ARN": "[... redacted by Mark ...]"
        },
        "encryptionAlgorithm": "SYMMETRIC_DEFAULT"
    },
    "responseElements": null,
    "requestID": "4270db56-4870-4a1f-bd26-e2bd63900cda",
    "eventID": "ee62a05e-dbb2-4ef9-aaff-a1e879c9baa8",
    "readOnly": true,
    "resources": [
        {
            "accountId": "[... redacted by Mark ...]",
            "type": "AWS::KMS::Key",
            "ARN": "[... redacted by Mark ...]"
        }
    ],
    "eventType": "AwsApiCall",
    "managementEvent": true,
    "recipientAccountId": "[... redacted by Mark ...]",
    "eventCategory": "Management"
}

We'd see similar events from humans (admins etc) accessing the secret values.

---

they have to create a PR to modify code, which leaves a trail

FWIW, the way I'd try to do this if I was an attacker is modifying one of our Rust dependencies and waiting for that to get cargo update'd (or even a GHA pre-installed dep). We're not network jailing GHA -- not sure that's even possible -- so it's relatively easy to leak a secret out to some remote endpoint if you have code execution where the secret is set in the environment.

---

So I think GHA secrets are better in the sense that there's less moving parts and AFAIK they're not human-readable after the initial setup, but they're strictly worse in that there's no persisted audit log of access, rotation, etc. -- at least not in the relatively easy way that AWS gives us by e.g. sorting the parameter store by "last modified" date. AWS could also give us a cleaner path to eventually issuing the tokens, where possible, from a GH App with the private key for it imported into KMS, making it impossible (after the initial import) for humans or robots to get access to the long-lived private key.

[marcoieni]

We could forbid that with branch protections, but if we use environment secrets properly, then the branch won't get access to the secrets until it gets to the merge queue, and it shouldn't be possible to add stuff to the merge queue without a PR.

But if someone has admin access they can temporarily disable branch protections 🙈 I'm not sure if this leaves a trail in some github audit log.

[Kobzol]

Yeah, branch protections are not enough. Environment secrets should be, although even that doesn't protect against the dependency attack.

[marcoieni]

Restricting the tokens to environment secrets is a great idea. E.g. we might limit the sync team write token to the master branch only 👍
dependency attack are orthogonal to this issue. The network jailing mentioned by Mark is possible and we could discuss it in another issue if we want 👍