square/keywhiz

Creating a secret with the same name as a deleted secret should not reuse the ID

jbpeirce opened this issue · 1 comments

When a secret is created (in secretDAO.createSecret() or secretDAO.createOrUpdateSecret()), if the new secret has the same name as a secret which was previously deleted, the old entry in the secrets table is reused for the new secret. This means that the new secret will have the same secret ID, creator, and creation date as the old secret.

Previous "versions" of a secret are identified by retrieving entries from the secrets_content table whose secretid field matches the secret's ID from the secrets table. Since this ID is reused when a secret with the same name as a deleted secret is created, the owner of the new secret can view and roll back to versions of the deleted secret, allowing them to view the deleted secret's contents. Because the new secret has the deleted secret's creation date, the creation date of the secret series vs. the creation date of the secret contents cannot be used to identify secret contents corresponding to a deleted secret.

We should update Keywhiz to create a new entry when a secret is created, regardless of whether an entry in the secrets table with that name already exists. This will require updating code in Keywhiz which currently assumes that there will be at most one entry in the secrets table with a given name; instead, it should enforce the condition that there will be at most one entry in the secrets table with a given name and a current_version field which is not null. This may cause the secrets table to grow more rapidly than before, since rows are no longer being reused, but will prevent content leaks from deleted secrets through this edge case.

Historically, since rolling back has been an administrator-only operation, this hasn't been a concern, but we'd like to build out a tool internally to allow users to roll back. This is a blocker for that.