blackbox-admins.txt can be manipulated by non-admins

[TomOnTime]

A non-admin can gain access to the encrypted data as follows:

1. Run blackbox_addadmin as normal but do not ask someone to approve their membership.
2. Wait until someone runs blackbox_update_all_files

This can happen if a newbie runs blackbox_addadmin by mistake. This situation can be fixed by moving the change to blackbox-admins.txt out of blackbox_addadmin, and to the "step 2" part (which should be automated via a new command such as blackbox_acceptadmin).

This can happen maliciously but someone simply editing blackbox-admins.txt. However this is generally prevented by repo access controls. However, this assumes anyone with write access to the repo is trusted.

NOTE: This is safe as long as anyone with write access to the repo is also an admin (or soon to become an admit). This is the use case currently at Stack Exchange, but it should be fixed so other use-cases are safe.

Suggested solution: GPG sign the blackbox-admins.txt and only permit encryption/re-encryption to happen if the signature matches. Alternatively, I think keeping the blackbox-admins.txt file encrypted might be sufficient. I'll have to think about it.

[christophebiocca]

You current solutions don't deal with admin revocation and replay attacks:

1. blackbox-admins.txt includes particular admin.
2. Admin gets access revoked, all secrets are rotated.
3. Someone re-checks out the old blackbox-admins.txt. It's still valid (either properly encrypted, or properly signed).
4. Whoever runs blackbox_update_all_files just gave the revoked admin access to all the new secrets.

Replay attacks are evil and hard to plan for.

Alternate solution:

• Every signing admin also keeps a local (.gitignore'd or outside the repo) hash of the blackbox-admins.txt file.
• If it doesn't exist, blackbox_update_all_files confirms the list with the operator, then creates and stores the hash.
• If it does and matches, proceed as normal.
• If it doesn't match, prompt the operator to confirm the new list.

This will work as long as admins are careful about checking such changes. It also breaks down if the blackbox-admins.txt is reverted before all admins have updated their hash.

Alternatively, use the encrypted files themselves as authoritative records. Whenever an update would add (or remove too?) someone's access, prompt the operator. You can be clever and merge these warnings together when multiple files are potentially affected.

[dwickwire]

Is this really a bug? My understanding is that re-encryption is the point where security enters the scene so the blackbox-admins.txt is just a helper file. Its not nice that anyone can edit it, but presumably you control who has access to the repo and trust them somewhat not to break things on master, or if they can push to master you trust them already.