taskcluster icon indicating copy to clipboard operation
taskcluster copied to clipboard

enhancement: add history of when a given secret was changed to the secrets UI

Open escapewindow opened this issue 2 years ago • 3 comments

Use case: Releng was asked if we knew when a given secret was last changed. We didn't, but Cloudops was able to retrieve this information from the logs.

This could be as simple as "this secret was last changed on datestring" (though we may have to store this information in the db if it's not there already).

Bonus points for being able to view a history of the last n changes to a given secret, or being able to blame the clientId responsible.

escapewindow avatar May 09 '22 21:05 escapewindow

Is this a good-first-bug?

djmitche avatar May 09 '22 23:05 djmitche

I see several ways we can achieve this.

  1. Log all activities (POST/PUT/DELETE) + user info. It would allow to see when specific resource was changed.
  2. Keep real history of secret values. It would allow to see previous values as well.
  3. Trivially add lastModified and lastModifiedBy fields to the secret (that would always be last change only).

Question is, if we implement 1. or 2. what other entities are worth being tracked? Roles? Worker pool configurations?

lotas avatar May 10 '22 08:05 lotas

@escapewindow there's also an older ticket about quarantine #4343

It feels like we should implement a proper audit log for most of the entities. Common approach I've seen in the past is to have a second table for each entity to store all actions: create/update/delete, but this might be too much for our use-case.

Maybe we can implement single audit table for all (most) entities to log such events in form of:

timestamp
entity_type
entity_id (or ids for compound pk's)
client_id (who triggered this change)
diff (where makes sense)

So we would be able to attach audit log to secrets, hooks, clients, roles, worker-pools, workers..

lotas avatar Jun 20 '22 12:06 lotas

Another angle to this problem is using mozilla-history for this purpose. We are already "tracking" most of the entities this way (except secrets) by scraping them and storing in git repository. It provides some answers as to when something was changed and what exactly was changed. But it cannot answer who made that change

lotas avatar Jun 28 '23 06:06 lotas

A model here might be tc-admin, which uses a hash to represent secrets in the diff. Some considerations at the time were:

  • We used a very short hash, opting to take a greater likelihood of a collision (which would just result in an incorrect diff) vs. revealing information about the secret
  • The hashes are salted differently on every run, so that for example a secret changing and then being reverted doesn't result in HASH1 -> HASH2 -> HASH1
  • The hashes include the secretId, so that if secret/1 and secret/2 have the same value, they do not have the same hash.

As for permissions, tc-admin already has to have permission to write all managed secrets, so permissions aren't an issue. It also has a --with-secrets flag without which it won't try to manage secrets, allowing it to be run without those permissions.

In the case of mozilla-history, I can see it being technically possible to invent a way to hash secrets such that the hashes could be stored in a Git repository and only reveal when a secret had been changed, but not its value or that it is equal to any other secret value past-or-present. But, I suspect that having a hook running every hour with secrets:get:* is a risk that outweighs the benefit.

djmitche avatar Jun 29 '23 00:06 djmitche