mvdan / bitw

Minimalist BitWarden client
BSD 3-Clause "New" or "Revised" License
169 stars 15 forks source link

Error during sync: could not sync: invalid cipher string #22

Open esiqveland opened 4 years ago

esiqveland commented 4 years ago

Hi, thanks for building this. An implementation with a Dbus secret service sounds like a great idea!

I am having some trouble with my (fairly huge) dataset from bitwarden.

Doing bitw sync fails with:

could not sync: invalid cipher string <<<REDACTED>>>

Should this happen, and is there a way I should go about debugging this?

mvdan commented 4 years ago

I assume you're running into this check here:

https://github.com/mvdan/bitw/blob/aa676adb187161afb340e699a5a80b181052b011/sync.go#L46-L49

As you can see earlier in that file, the cipher text is generally in the form ${type}.${iv}|${ct}|${mac}, so that's what we are trying to parse. I assume your cipher text doesn't look like that. Without disclosing it, what does it look like? Any idea what format it might be in?

Unfortunately, the bitwarden API isn't documented, so the current implementation is partially reverse-engineered.

esiqveland commented 4 years ago

Ok thanks, that is helpful. The format of the cipher string does not match what you describe, it looks like two strings of base64 separated by a pipe |: {base64}|{base64} but I have not verified it.

I might do some debugging and dig out which entry it is.

Is the api json based?

esiqveland commented 4 years ago

I see now that the cipher text comes from the field SyncData.Profile.Key:

https://github.com/mvdan/bitw/blob/aa676adb187161afb340e699a5a80b181052b011/sync.go#L114

and is prefixed with 0., giving the full value something like: 0.{base64}|{base64}

esiqveland commented 4 years ago

Thinking about this, maybe the {mac} is missing from the Key on my account? That would be really weird?

mvdan commented 4 years ago

I honestly don't know. It's hard to figure these bugs out because the API has no spec and it's difficult to test all the edge cases against the real server :)

slurdge commented 4 years ago

@mvdan I'm currently trying to write an exporter from bw to kdbx and your repository has come useful. Now, for this issue, I believe I've read somewhere that old keys are not encrypted (see https://github.com/bitwarden/browser/blob/f1262147a33f302b5e569f13f56739f05bbec362/src/services/constantsService.js#L13) with AesCbc256_HmacSha256_B64 but with AesCbc256_B64. See https://github.com/bitwarden/jslib/blob/f30d6f8027055507abfdefd1eeb5d9aab25cc601/src/models/domain/cipherString.ts#L71

I've also seen (somewhere...) the key read for older keys, but I can't find it. Here is something related. https://github.com/bitwarden/jslib/blob/f30d6f8027055507abfdefd1eeb5d9aab25cc601/src/models/domain/symmetricCryptoKey.ts#L35

mvdan commented 4 years ago

Thanks, that's a good find. We certainly don't support multiple kinds of keys. If someone wants to work on this, a PR with some sort of unit or integration tests would be welcome.

slurdge commented 4 years ago

Found it: https://github.com/bitwarden/jslib/blob/f30d6f8027055507abfdefd1eeb5d9aab25cc601/src/services/crypto.service.ts#L133

        let decEncKey: ArrayBuffer;
        const encKeyCipher = new CipherString(encKey);
        if (encKeyCipher.encryptionType === EncryptionType.AesCbc256_B64) {
            decEncKey = await this.decryptToBytes(encKeyCipher, key);
        } else if (encKeyCipher.encryptionType === EncryptionType.AesCbc256_HmacSha256_B64) {
            const newKey = await this.stretchKey(key);
            decEncKey = await this.decryptToBytes(encKeyCipher, newKey);
        } else {
            throw new Error('Unsupported encKey type.');
        }

I believe right now you are doing as me and stretching the key. However in the case of @esiqveland the key shouldn't be stretched.

kryptt commented 3 years ago

was just tryin bitw and ran into this issue....

mvdan commented 3 years ago

@esiqveland reported that changing his password resulted in him getting new cipher strings, which use the format understood by bitw.

His PR above is the ground work towards supporting multiple cipher string formats. Unfortunately, I don't have any valid cipher string in this different format that we could unit test for decryption. If someone can provide that (perhaps with a dummy secret and dummy password), then we'd be able to implement and test it.

slurdge commented 3 years ago

I think that's difficult because probably the old keys are like this. So if someone wanted to have a dummy secret/password, they would change their current password and make it stretched. Given the fact only the stretch part is to be skipped, I think you could try to blind code it and ask @kryptt to test it. No ideal, but could work well enough.

mvdan commented 3 years ago

Indeed, it's a bit of a chicken and egg situation :) It's hard to properly support all of bitwarden's API and features when they don't document any of it.

Thanks for pointing out the js/ts code, though, that sounds straightforward enough. I'll push a branch for @kryptt to try.

mvdan commented 3 years ago

Okay, I've very carefully implemented this, and hopefully in a correct way. I've refactored the crypto code to not depend on global state, and written some table-driven unit tests with dummy keys.

@esiqveland @kryptt could either of you please try the latest master and let me know if it works? If not, then I'd have to investigate what I did wrong, because I assume neither of you wants to give me your keys and passwords :)

kryptt commented 3 years ago

Still getting this now:

✦ ❯ bitw dump
Password: 
error: decrypt: MAC mismatch
mvdan commented 3 years ago

Thanks for checking. I'm a bit confused, because supposedly the AesCbc256_B64 cipher does not use a mac/hmac at all, so we should not be validating any mac either. The code would have also errored out earlier if the mac key or cipher mac component were empty. Perhaps your issue is unrelated to the different cipher type.

Anything else you can tell me about your setup or issue? For example, what the format of your cipher strings looks like. I can't debug further without having access to input to reproduce the problem, but I also assume you don't want to give me your encrypted secrets and password. Perhaps you could debug the code yourself, too, since you can reproduce the issue.

@esiqveland were you able to try your setup again? You definitely did have AesCbc256_B64 ciphers, so it would be useful to know if the issue was indeed fixed for you or not :)