4 ms·

Currently using Bitwarden right now. Really good to see that the security assessment is relatively positive: > All in all, while the client and backend code ar

by udia 8y ago

Currently using Bitwarden right now. Really good to see that the security assessment is relatively positive: > All in all, while the client and backend code are vulnerable to some issues, all of the problems can be easily fixed without a lot of effort. In that sense, Cure53 believes these items of the Bitwarden scope to be fully capable of reaching the desired standards of security in a rather short time. To reiterate, the results of this autumn 2018 assessment are positive for the client and code. Wondering how they will address the current cryptographic scheme though.

CiPHPerCoder 8y ago
> Wondering how they will address the current cryptographic scheme though. The only cryptographic weakness Cure53 identified was that a malicious API server could exfiltrate encryption keys. Cure53 deemed it a hard problem to solve. I wrote a proposed strategy for mitigating it: https://github.com/bitwarden/core/issues/392 https://github.com/bitwarden/core/issues/392 Regarding Bitwarden's cryptographic security, a cursory read through their code yields the following: * It's using RSA-OAEP to encrypt AES keys (EDIT: formerly "some data") https://github.com/bitwarden/jslib/blob/b4fad203b94da53d33693f4283d7249e3a8f1afe/src/services/crypto.service.ts#L382-L399 https://github.com/bitwarden/jslib/blob/b4fad203b94da53d3369... * It's using AES-256-CBC https://github.com/bitwarden/jslib/blob/b4fad203b94da53d33693f4283d7249e3a8f1afe/src/services/crypto.service.ts#L345-L380 https://github.com/bitwarden/jslib/blob/b4fad203b94da53d3369... + https://github.com/bitwarden/jslib/blob/b4fad203b94da53d33693f4283d7249e3a8f1afe/src/services/crypto.service.ts#L494-L508 https://github.com/bitwarden/jslib/blob/b4fad203b94da53d3369... + https://github.com/bitwarden/jslib/blob/2045e7047a66599b2c8a92b88cd0d1b8bfc5186f/src/services/nodeCryptoFunction.service.ts#L71-L78 https://github.com/bitwarden/jslib/blob/2045e7047a66599b2c8a... It doesn't appear to be authenticating the AES-CBC-encrypted ciphertexts in all cases, which makes me suspect padding oracles are still in-scope. https://robertheaton.com/2013/07/29/padding-oracle-attack/ https://robertheaton.com/2013/07/29/padding-oracle-attack/ RSA-OAEP is the better RSA mode. (You don't want PKCS1v1.5) In closing: As long as you're not for some reason storing unauthenticated AES-CBC ciphertexts in the server, the encryption is really boring. (Boring is good for encryption.)
- tialaramex 8y ago
  "to encrypt some data" ? Actually data? You'd usually expect RSA to be protecting a symmetric key in this sort of setup - is that what the data is, or something else?
  - CiPHPerCoder 8y ago
    Yes, it's using RSA to encrypt a key, as one would hope. https://github.com/bitwarden/jslib/blob/b4fad203b94da53d33693f4283d7249e3a8f1afe/src/services/crypto.service.ts#L312 https://github.com/bitwarden/jslib/blob/b4fad203b94da53d3369... Usually when I see RSA-OAEP in a casual stroll through something's code, I stop there and move onto looking for other issues. Reason: Very few users of RSA encryption bother to use a secure padding mode. If they're doing that much, the chances of doing something very stupid (a.k.a. "RSA-ECB") is low enough to discount for the purposes of message board discussions. (Obviously, if I'm being paid to review something, I spend a lot more time on it.) When I wrote my post above, all I cared about was the modes being used. That's why I vaguely said "some data". A further analysis (i.e. where rsaEncrypt() is invoked) yields: They're only using RSA for encrypting AES keys, which is a sane design. Hopefully my lazy word choice didn't cause you (or anyone else) any undue alarm.
    - GordonS 8y ago
      When you said 'data' I assumed you meant a hybrid RSA-AES scheme - of course keys are technically 'data', but when talking about data in the context of cryptography, it usually means 'data that isn't a key' :)
    - tialaramex 8y ago
      You clearly deal with more competent people than me. Literally the last piece of code I read that specified RSA OAEP was trying to shove user session data into it. Thanks for replying to put my mind at ease on this.
- xxkylexx 8y ago
  All AES-CBC data is authenticated with HMAC SHA-256. This was highlighted in the BWN-01-011 issue (which was determined to be a false positive since it was deemed that authentication was properly done).

by udia 8y ago

CiPHPerCoder 8y ago

> Wondering how they will address the current cryptographic scheme though. The only cryptographic weakness Cure53 identified was that a malicious API server could exfiltrate encryption keys. Cure53 deemed it a hard problem to solve. I wrote a proposed strategy for mitigating it: https://github.com/bitwarden/core/issues/392 https://github.com/bitwarden/core/issues/392 Regarding Bitwarden's cryptographic security, a cursory read through their code yields the following: * It's using RSA-OAEP to encrypt AES keys (EDIT: formerly "some data") https://github.com/bitwarden/jslib/blob/b4fad203b94da53d33693f4283d7249e3a8f1afe/src/services/crypto.service.ts#L382-L399 https://github.com/bitwarden/jslib/blob/b4fad203b94da53d3369... * It's using AES-256-CBC https://github.com/bitwarden/jslib/blob/b4fad203b94da53d33693f4283d7249e3a8f1afe/src/services/crypto.service.ts#L345-L380 https://github.com/bitwarden/jslib/blob/b4fad203b94da53d3369... + https://github.com/bitwarden/jslib/blob/b4fad203b94da53d33693f4283d7249e3a8f1afe/src/services/crypto.service.ts#L494-L508 https://github.com/bitwarden/jslib/blob/b4fad203b94da53d3369... + https://github.com/bitwarden/jslib/blob/2045e7047a66599b2c8a92b88cd0d1b8bfc5186f/src/services/nodeCryptoFunction.service.ts#L71-L78 https://github.com/bitwarden/jslib/blob/2045e7047a66599b2c8a... It doesn't appear to be authenticating the AES-CBC-encrypted ciphertexts in all cases, which makes me suspect padding oracles are still in-scope. https://robertheaton.com/2013/07/29/padding-oracle-attack/ https://robertheaton.com/2013/07/29/padding-oracle-attack/ RSA-OAEP is the better RSA mode. (You don't want PKCS1v1.5) In closing: As long as you're not for some reason storing unauthenticated AES-CBC ciphertexts in the server, the encryption is really boring. (Boring is good for encryption.)

tialaramex 8y ago
"to encrypt some data" ? Actually data? You'd usually expect RSA to be protecting a symmetric key in this sort of setup - is that what the data is, or something else?
- CiPHPerCoder 8y ago
  Yes, it's using RSA to encrypt a key, as one would hope. https://github.com/bitwarden/jslib/blob/b4fad203b94da53d33693f4283d7249e3a8f1afe/src/services/crypto.service.ts#L312 https://github.com/bitwarden/jslib/blob/b4fad203b94da53d3369... Usually when I see RSA-OAEP in a casual stroll through something's code, I stop there and move onto looking for other issues. Reason: Very few users of RSA encryption bother to use a secure padding mode. If they're doing that much, the chances of doing something very stupid (a.k.a. "RSA-ECB") is low enough to discount for the purposes of message board discussions. (Obviously, if I'm being paid to review something, I spend a lot more time on it.) When I wrote my post above, all I cared about was the modes being used. That's why I vaguely said "some data". A further analysis (i.e. where rsaEncrypt() is invoked) yields: They're only using RSA for encrypting AES keys, which is a sane design. Hopefully my lazy word choice didn't cause you (or anyone else) any undue alarm.
  - GordonS 8y ago
    When you said 'data' I assumed you meant a hybrid RSA-AES scheme - of course keys are technically 'data', but when talking about data in the context of cryptography, it usually means 'data that isn't a key' :)
  - tialaramex 8y ago
    You clearly deal with more competent people than me. Literally the last piece of code I read that specified RSA OAEP was trying to shove user session data into it. Thanks for replying to put my mind at ease on this.
xxkylexx 8y ago
All AES-CBC data is authenticated with HMAC SHA-256. This was highlighted in the BWN-01-011 issue (which was determined to be a false positive since it was deemed that authentication was properly done).