AES without IV

[BenjaminHae]

We are using AES without setting an initialization vector. "If the CBC is not properly initialized, data that is encrypted can be compromised and therefore be read." (http://cwe.mitre.org/data/definitions/329.html) An attacker could for example see which accounts share passwords(I know nobody should use the same password for multiple services but it happens..).

To solve this we should implement an iv that is different for every string that is encrypted. I propose the following solution:

We change the password table to have 4 columns: index, userid, data, iv. The data of one account will be included in a aes encrypted json dictionary. This is easy to implement as the frontend already contains the data in this format. Keeping the data separate brings no benefit as you can't read the database contents because they are encrypted. When saving an account a new IV get's randomly created the json dict gets encrypted using the aes key and the iv. The IV and the encrypted data get's saved to the database.

These changes will take a while to implement, but if you're all onboard I'd be happy to make the necessary changes over the next months.

As some might have users already using their instances I'm also planning to implement an update routine.

[zeruniverse]

I'll take a look.

On Sat, Jan 14, 2017 at 5:06 AM Benjamin Häublein notifications@github.com wrote:

We are using AES without setting an initialization vector. "If the CBC is not properly initialized, data that is encrypted can be compromised and therefore be read." (http://cwe.mitre.org/data/definitions/329.html)

An attacker could for example see which accounts share passwords(I know nobody should use the same password for multiple services but it happens..).

To solve this we should implement an iv that is different for every string that is encrypted. I propose the following solution:

We change the password table to have 4 columns: index, userid, data, iv.

The data of one account will be included in a json dictionary. This is easy to implement as the frontend already contains the data in this format. Keeping the data separate brings no benefit as you can't read the database contents because they are encrypted.

When saving an account a new IV get's randomly created the json dict gets encrypted using the aes key and the iv. The IV and the encrypted data get's saved to the database.

These changes will take a while to implement, but if you're all onboard I'd be happy to make the necessary changes over the next months.

As some might have users already using their instances I'm also planning to implement an update routine.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/zeruniverse/Password-Manager/issues/121, or mute the thread https://github.com/notifications/unsubscribe-auth/AEbvNF0x-BUsdIVKIgtL-7blUJEAqRvHks5rSMg5gaJpZM4LjnnU .

[zeruniverse]

I looked into it. Though we didn't use iv, we did use salt, which aims to prevent the same type of attack. If you try encryptchar("encryptch","key") multiple times, you'll get different results.

[BenjaminHae]

Wow. Thanks! I didn't expect that and I don't understand it. Where is the salt referenced? For me encryptchar always looked deterministic on the two input variables...

And now I think I found a little bug in encryptchar: if(encryptchar==""||key==""){ The first condition is always true, as encryptchar is the function. It probably should be if(encryptch==""||key==""){

[BenjaminHae]

Could you please elaborate on where the salt comes from?

[zeruniverse]

salt is randomly generated and stored in the encrypted string (salt is not encrypted). You can see the salt by following:

• Go to https://phppasswordmanager.sourceforge.io/
• Open console

• Paste the following code:

  
  function tosend(cipherParams) {
          // create json object with ciphertext
          var send="";
             send+=cipherParams.ciphertext.toString(CryptoJS.enc.Base64)+"-";
  
          // optionally add iv and salt
          if (cipherParams.iv) {
              send+=cipherParams.iv.toString()+"-";
          }
          if (cipherParams.salt) {
              send+=cipherParams.salt.toString()+"-";
          }
  
          // stringify json object
          return send;
      }

function encryptchar_s(encryptch,key){
if(encryptchar==""||key==""){
alert("ERROR: empty key detected!");
return;
} var p=tosend(CryptoJS.AES.encrypt(encryptch,key)); return p; }

+ Use `encryptchar_s(encryptch,key)`, the output is something like   
`DTKeN5wmJuLzjE/Z06wsKQ==-200e4fdc9b7b67a4e7a67d450e459fe2-0d78c98ee85329cb-`  separated by `-`.  
+ `DTKeN5wmJuLzjE/Z06wsKQ==` is encrypted string, `200e4fdc9b7b67a4e7a67d450e459fe2` is `iv`, though not used, `0d78c98ee85329cb` is random salt

[BenjaminHae]

alright thank you very much.