-
Notifications
You must be signed in to change notification settings - Fork 0
Add support for base64:… arg for unlock
#10
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
Conversation
To make it easier to unlock a repository locally given the base64 of the key (which is how the key would likely be shared between coworkers)
|
|
||
| # Option 2: Provide a path to a from file containing the raw binary, 32 bytes key | ||
| # Option 2: Provide the Base64-encoded key as command line argument. (Only use locally, as on CI this could leak the key in logs). | ||
| git-conceal unlock "base64:c3VwcG9zZWRseS15b3VyLWJpbmFyeS1zZWNyZXRrZXk=" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I imagine this isn't caused by this PR, but I've tried to run it in Automattic/pocket-casts-android#4841 and deliberately used a wrong key, just for testing:
git-conceal unlock "base64:c3VwcG9zZWRseS15b3VyLWJpbmFyeS1zZWNyZXRrZXk="
Then I got:
Re-checking out encrypted files...
Will re-checkout: app/google-services.json
Will re-checkout: automotive/google-services.json
Will re-checkout: firebase.secrets.json
Will re-checkout: google-upload-credentials.json
Will re-checkout: release.keystore
Will re-checkout: secret.properties
Will re-checkout: sentry.properties
Will re-checkout: wear/google-services.json
Error: Failed to re-checkout encrypted files
Caused by:
git checkout HEAD -- <files> failed: exit status: 128
stderr: Error: Failed to decrypt data
Caused by:
HMAC verification failed - the decryption key may be incorrect, or the file may have been tampered with. Please verify you're using the correct key for this repository.
error: external filter '"(...)git-conceal" filter smudge' failed 1
error: external filter '"(...)git-conceal" filter smudge' failed
fatal: app/google-services.json: smudge filter git-conceal failed
Which led to some changed files:
Changes to be committed:
(use "git restore --staged <file>..." to unstage)
deleted: app/google-services.json
deleted: automotive/google-services.json
deleted: firebase.secrets.json
deleted: google-upload-credentials.json
deleted: release.keystore
deleted: secret.properties
deleted: sentry.properties
deleted: wear/google-services.json
Untracked files:
(use "git add <file>..." to include in what will be committed)
automotive/google-services.json
firebase.secrets.json
google-upload-credentials.json
release.keystore
secret.properties
sentry.properties
wear/google-services.json
My first reflex was running git reset, which also failed:
git reset --hard
Error: Failed to decrypt data
Caused by:
HMAC verification failed - the decryption key may be incorrect, or the file may have been tampered with. Please verify you're using the correct key for this repository.
error: external filter '"(...)git-conceal" filter smudge' failed 1
error: external filter '"(...)git-conceal" filter smudge' failed
fatal: app/google-services.json: smudge filter git-conceal failed
git-conceal lock failed and then git-conceal lock --force restored the repo to its locked state, and unlocking with the right key worked again.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah it is indeed not related to this PR, and kinda the expected behavior so far.
I agree that we should ideally improve on that unhappy path (ie make unlock automatically revert changes in the git index if it detected decryption failed with the provided key during re-checkout, as well as make lock --force take care of the git restore --staged + git restore of secret files to undo any odd state in those situations. Noted for a future improvement 👍
(PS: for the record that's also what you get with git-crypt and unlocking with a wrong key, except git-crypt doesn't use HMAC for integrity verification iinm so with git-crypt it might happen that you could be (un-)lucky enough to use a bad key that still doesn't crash the decryption cipher (resulting in garbage decrypted output, but that git-crypt wouldn't realize is invalid). And it's only if you happen to use an incorrect key that makes the decryption impossible that git-crypt will leave you in that odd git state.
While at least git-conceal has integrity verification with HMAC so detects if you're using the wrong key when decrypting a file and is thus guaranteed to fail in those cases (as opposed to report success but generate garbage)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
git-cryptdoesn't use HMAC for integrity verification iinm
at least
git-concealhas integrity verification with HMAC so detects if you're using the wrong key when decrypting a file and is thus guaranteed to fail in those cases
That's a great improvement 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Brainstorm: what would be a good name for a subcommand to get out of such a messy status after unlocking with the wrong key?
I'm thinking that, while we could execute such a revert/cleanup automatically during unlock if provided with wrong key, to make it transparent and restore to a good state in those cases, maybe it could also be useful to have a dedicated command for also running it manually in case such thing happen in other situations?
(The most likely being if someone switches from one git branch that uses a given key to a different branch in which the secrets have been encrypted with a different key because a key rotation happened in between)
Or maybe git lock --force (which was actually meant to handle those kind of situations too, but apparently isn't enough currently) would be that command? Though would be annoying to have to lock/unlock the repo just to get out of bad git state… although I think such a case should only happen in practice if decryption failed due to bad key, so maybe forcing to lock then re-unlock (with the right key) makes sense to get out of those?
iangmaia
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tested it on PCAndroid and it worked great 👍
|
@AliSoftware Oops, didn't realize we had auto-merge here (to give you chance of updating the docs for this comment) 🤦♂️ |
|
I can create a follow-up PR for adding the tip about histognorespace 👍 In retrospect I'm starting to regret that the unlock command has evolved to require So maybe I'll make a corrective PR that switches the format and:
That would make base64-value the default and file input the exception, instead of the other way around. Footnotes
|
I've implemented that change in #14. In fact, I decided to completely drop the option to read the raw binary key from a file, because:
So now after #14 it'll only be either |
Why?
To make it easier to unlock a repository locally given the base64 of the key (which is how the key would likely be shared between coworkers, e.g. this is how it's shared in our internal Secret Store), this adds support for
git conceal unlock "base64:…"syntax.How?
Similar to the
git conceal unlock env:<VARNAME>syntax to read the base64-encoded key from an env var (ideal for CI), thisgit conceal unlock "base64:<base64key>syntax is most suitable for developers that need to unlock their local working copy from the base64 key they'd retrive from our Secret Store.Without that change, providing the base64 key from the Secret Store to
git concealrequired to use something likeecho "<base64key>" | base64 -d | git conceal unlock -, which worked but was not super natural. Another possible way was toexport GIT_CONCEAL_KEY="<base64key>"thengit conceal unlock env:GIT_CONCEAL_KEY, but that required setting a one-off env var for little benefit. With that new syntax, it'll be easier to unlock a repo from a base64-encoded key like the ones shared in Secret Store, making the onboarding instructions easier.Testing
cargo build --releasefrom this PR's head branchgit-conceal, like of [AINFRA-1533] Adoptgit-concealin this repo woocommerce/woocommerce-android#14979 or [AINFRA-1539] [Internal] Migrate fromconfigure_applytogit-concealpocket-casts-android#4841, so you have a repo on which to test that command on$ git conceal unlock "base64:$(pbpaste)"(or alternatively, paste the key manually in your terminal in that command, i.e.
git conceal unlock "base64:<paste key here>")