Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Blackbox security/integrity can be undermined in several ways #378

Open
Lightning- opened this issue Oct 31, 2023 · 3 comments
Open

Blackbox security/integrity can be undermined in several ways #378

Lightning- opened this issue Oct 31, 2023 · 3 comments

Comments

@Lightning-
Copy link
Contributor

This is gonna be a long one with some complexity, please feel free to get in touch with me for further clarification if needed :).

Summarizing the core problem: Blackbox does not ensure validity/plausibility of the delivered keyring and "admin files"; as well "collisions" (let's call it that way for now) with the personal keyring are not prevented.

It starts with blackbox-admins.txt which can simply be edited by everyone with repository write access, even if that person is not meant to administer Blackbox secrets at all; here an issue ticket already exists → #9
Basically there's no big need to explain how to exploit this: think about a repository for a wiki where people do have read and write access to the content but not everybody should be able to read also-stored password files. Create a big "fix syntax/formatting" commit, hide changes to blackbox-admins.txt in there and be patient till things get re-encrypted (exploit chain follows).
Beside that, blackbox-files.txt can be edited in a similar way, this just doesn't harm anyone as there potentially will be files encrypted that shouldn't.

Almost similar to those: delivered pubring (which simply gets imported to the personal pubring in _blackbox_common.sh) can be "enriched" by additional keys (or even subkeys, to make it less obvious) the same way.
Beyond combining manipulation of blackbox-admins.txt and the pubring with fresh entries, you also can inject a forged (sub)key for an existing entry of blackbox-admins.txt and replace the current one in the keyring. As well, dependent on sort order of GPG keys/subkeys, you might get things encrypted for you even if there are 2 different keys for the same list entry.

Next: Blackbox doesn't care about whether the used keys are actually the ones that should be used according to the delivered keyring. After importing delivered keys into the personal keyring, the key selection is left to GPG based on the eMail address (or whatever selection criteria is used in blackbox-admins.txt; which doesn't need to be unique/pseudo-unique). So if I have 2 keys for the same address in my personal keyring, this might lead to kicking out somebody from encryption unintentionally (in best case). Worst case (think of previous forgery ideas) is, that I encrypt secrets to people that simply shouldn't get them.

Last, but not least important: Blackbox explicitly/intentionally bypasses the web of trust (see here). This means that even in a fully trusted environment, where people do proper keysigning, building up clean trustdbs, maybe do have a company signing key that signs keys for employees and hands them over on smartcard/Yubikey etc. Blackbox kills the strongest safety net that PGP/GPG has. While the web of trust would have caught some of the potential security issues or problems before, this possibility is eliminated here.

Combining these problems there are several ways to play around, trick around, obfuscate evil things, ... and get secrets that you shouldn't have!
Some problems additionally lead to unexpected behavior ("this is encrypted for the wrong key").

Asking questions and suggesting solutions

Checksum + Signature

Checksum and cryptographically sign the checksums the same way that is common for any random open source project.
So place a checksums-file in keyrings/live or .blackbox directory that holds the current checksums for blackbox-admins.txt, blackbox-files.txt and pubring.gpg and make the editor cryptographically sign that checksums-file whenever changes are done here. This speaks as "I am a legit BB admin, made changes to those files, sign this with my key and everyone can verify this".
Before encrypting any data, verify the checksums, the signature and the trustworthiness of the signing key and bail out if something is wrong here!
This is barely more than sha256sum blackbox-admins.txt blackbox-files.txt pubring.gpg > checksums.sha256, gpg2 --local-user identifier_of_used_blackbox_key --detach-sig -a checksums.sha256, gpg2 --verify checksums.sha256.asc and sha256sum -c checksums.sha256.

Why importing the delivered keyring and not using it directly?

As BB already delivers a maintained keyring (which is checksummed and signed ;)) containing all used pubkeys, why importing this into the personal keyring and not use it directly by --keyring along with --no-default-keyring? It might be beneficial for people to use their personal pubrings anyways, but making this an option (ENV-VAR, cli parameter, config file, ...) and not the default would feel way more intuitive and kill many wanted/unwanted misbehaviors.

Identify keys by ID or fingerprint not by eMail

Of course, randomly looking numbers don't please a user, but maintaining a file that maps the configured addresses to IDs/fingerprints and just use those IDs/fingerprints in the gpg command lines would kill many collisions or mis-picks of keys. Receive valid IDs/fingerprints from pubring (checksummed, signed, ...), dump them to a file, checksum it, sign it and you're pretty safe.

Fingers off the web of trust

If a user wants to bypass the web of trust in some way, he's free to set his favorite trust model in his gpg.conf, but bypassing the trust model hardcoded is imho a no-go. Don't kill essential security features of PGP/GPG that would have saved you from (security) issues!

@TomOnTime
Copy link
Collaborator

Checksum + Signature

I agree and I disagree.

I agree with your assessment. Those files can be modified by anyone and this is bad.

I disagree with your assessment. The only way to modify those files is to submit a PR, and the PR must be approved by someone who would not permit such changes.

Yes, that's a lazy answer but that's how Blackbox was designed to be used.

Why importing the delivered keyring and not using it directly?

I hadn't thought of that. Good idea!

Identify keys by ID or fingerprint not by eMail

Agreed. We've had significant problems as a result of using email addresses. ID or fingerprint would be better.

Fingers off the web of trust

I hadn't realized this was hardcoded. Can you explain?


The bottom line is that Blackbox started out as a simple Bash wrapper around a few common GPG commands so that people didn't have to type (literally) hundreds of keystrokes to do simple things like encrypt a file so that only their co-workers could decrypt them. It has gown too large and people are expecting too much of it.

Meanwhile systems like git-crypt have evolved to be much more complete solutions. I think it is time to decomm this package or hand it off to someone who has more time to support it.

@Lightning-
Copy link
Contributor Author

I agree and I disagree.

I agree with your assessment. Those files can be modified by anyone and this is bad.

I disagree with your assessment. The only way to modify those files is to submit a PR, and the PR must be approved by someone who would not permit such changes.

Yes, that's a lazy answer but that's how Blackbox was designed to be used.

Ok, I didn't realize that PR/four-eyes is a basic concept of BB.
But thinking about "an evil (non-BB) admin tricking around", a repository where dozens of people do have write access and where PRs aren't practical and/or unwanted (wiki repository example) and the general spirit of "make it as safe as possible, even if there are other ways to secure it as well (signed git commits as a basic requirement for the "admin files" e.g.)", I definitely think that implementing checksums+signatures is sane. If nobody manipulates things this would be completely transparent and a BB user won't even notice.

I hadn't realized this was hardcoded. Can you explain?

Here you're passing --trust-model=always as a hardcoded cli parameter to gpg. This says "don't use the web of trust at all, even if people usually do (cli-parameter overriding config settings and built-in defaults)".
So if I have a properly built trustdb and settings that require those trustlevels, which together would have warded from encrypting to a "sneaked in fake-key", this kills all of that.
Beside that, WOT should always be used with PGP/GPG unless a different validation system is in place.


The bottom line is that Blackbox started out as a simple Bash wrapper around a few common GPG commands so that people didn't have to type (literally) hundreds of keystrokes to do simple things like encrypt a file so that only their co-workers could decrypt them. It has gown too large and people are expecting too much of it.

Meanwhile systems like git-crypt have evolved to be much more complete solutions. I think it is time to decomm this package or hand it off to someone who has more time to support it.

Well ... I didn't mean to blow the whole project up ... :(
But I know what you're talking about. Actually I wrote a ~200 line bash wrapper around git+gpg many many years ago (basically same purpose as BB, just encrypting a single file and just supporting git). What to say ... have been using this for 13 years in 3 different companies right now =D

So if you're willing to take some minor but breaking changes in BB's handling, which should be sanely catchable by documentation/explanation, I'm absolutely willing to submit patches for all of the mentioned issues.

@TomOnTime
Copy link
Collaborator

Hey there!

Yeah, I'd definitely be interested in patches! It may take me a while to review them but I'll get to it.

I'm particularly interested in the --keyring idea and using IDs/fingerprints instead of email addresses.

Lightning- pushed a commit to Lightning-/blackbox that referenced this issue Nov 2, 2023
the choice whether to use a web of trust and on which trust level is up
to the user of PGP/GPG and must not be overriden by tools that are set
on top

users can decide to ignore this safety net by setting their gpg.conf
adequately, defining an alias for `gpg --trust-model=always` or passing
the env GPG to blackbox in this way but we should not override their
preferences hardcoded
Lightning- added a commit to Lightning-/blackbox that referenced this issue Jan 10, 2024
the choice whether to use a web of trust and on which trust level is up
to the user of PGP/GPG and must not be overriden by tools that are set
on top

users can decide to ignore this safety net by setting their gpg.conf
adequately, defining an alias for `gpg --trust-model=always` or passing
the env GPG to blackbox in this way but we should not override their
preferences hardcoded
tlimoncelli pushed a commit that referenced this issue Jan 17, 2024
* don't bypass the web of trust (#378)

the choice whether to use a web of trust and on which trust level is up
to the user of PGP/GPG and must not be overriden by tools that are set
on top

users can decide to ignore this safety net by setting their gpg.conf
adequately, defining an alias for `gpg --trust-model=always` or passing
the env GPG to blackbox in this way but we should not override their
preferences hardcoded

* update README

add note about the web of trust

* fix broken test

assume that we have `--quick-generate-key` if we run gpg2 instead of
doing a dry run for that (which has side effects that break the test)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants