Ideas2IT / cordova-aes256

MIT License
13 stars 20 forks source link

Feature request: Add method to generate random bytes #2

Closed BorntraegerMarc closed 6 years ago

BorntraegerMarc commented 6 years ago

I think for performance it would be better to generate the random bytes on the native side and pass them through cordova once it's finished.

This issue is a feature request to add a method "randomBytes(bytes: number)" to generate random bytes for the secureKey & secureIV string

pandiarajan-i2i commented 6 years ago

Hi @BorntraegerMarc , Thanks for your suggestion. We will add this feature to the native layer.

FYI, In front end, we have used "crypto-js/pbkdf2" js library to generate random "secureKey" & "secureIV".

Example,

import PBKDF2 from 'crypto-js/pbkdf2';
let secureKey = PBKDF2("password","salt", { keySize: 2, iterations: 1000 });

keySize -> out key length
password -> user input
salt -> random string
iterations -> random value (For more security this value should be >1000)
BorntraegerMarc commented 6 years ago

Thanks a lot for the fast reply & tip. It's uncommon nowadays to see such engagement from the contributers in a cordova plugin!

Looking forward to the update :)

BorntraegerMarc commented 6 years ago

Hey @pandiarajan-i2i just checking in of you have an ETA on this feature request and if I can help you in any way?

pandiarajan-i2i commented 6 years ago

Hi @BorntraegerMarc ,

We have completed the android part and working on the iOS part. So hopefully this weekend we will give a new release.

Thanks for your support.

BorntraegerMarc commented 6 years ago

Cool! :) I'm happy to test it out as soon as you have released it. Just hit me with a message :)

pandiarajan-i2i commented 6 years ago

sure @BorntraegerMarc . We will update you once we complete it.

BorntraegerMarc commented 6 years ago

Hey @pandiarajan-i2i Just checking in if there's anything I can test yet? :)

pandiarajan-i2i commented 6 years ago

Hi @BorntraegerMarc, We have added the secure key and secure IV generation method to the android code and tested it. Currently testing the iOS code. We will release it soon.

You can clone the repo and add can able to tested the android build.

FYI, Add a local plugin ionic cordova plugin add cloned repository folder path

BorntraegerMarc commented 6 years ago

Alright, thanks! I'll start with testing next week. Have a nice time!

BorntraegerMarc commented 6 years ago

Hi @pandiarajan-i2i I checked out the API for android and I have one question: Why is it still needed to pass a password to the library? Is there a specific use case that you have in mind? Because honestly I don't know when a user has a specific password but doesn't want to generate a IV or key himself.

Would it maybe make sense to make the password attribute optional and if it's not provided to generate also some random bytes for the password?

pandiarajan-i2i commented 6 years ago

Hi @BorntraegerMarc , We have used the PBKDF2 (Password-Based Key Derivation Function 2) for generating the IV and secureKey. The PBKDF2 function requires a password as a input. To ensure more security, instead of generating a random password from the library, we have exposed the password param so that user can set the password.

BorntraegerMarc commented 6 years ago

Why would not having a random password be more secure?

To my understanding: more random inputs = harder to guess for an attacker = more Security

pandiarajan-i2i commented 6 years ago

@BorntraegerMarc , Yes, you're correct. The random input is more secure. But, instead of generating the random password from the library with some random algorithm user can able to generate the password string using any random algorithm.

If the library generates a random password then the attacker will know about a random algorithm(from the source). It's hard to guess but it will give way to the attacker.

FYI... password = sometext + Date.now().toString() + randomString

BorntraegerMarc commented 6 years ago

Hmmm ok, I do see your point that one could have different ways on how to generate a random password.

The password does not need to have a specific length, right? Because it's going to be fun through a KDF anyway.

pandiarajan-i2i commented 6 years ago

@BorntraegerMarc Yes, Password doesn't have a specific length.

pandiarajan-i2i commented 6 years ago

Hi @BorntraegerMarc , We have released a new version. Please check it out and let us know if any issues.

BorntraegerMarc commented 6 years ago

I will, soon! Thanks!

BorntraegerMarc commented 6 years ago

I can confirm that encryption / decryption works like a charm for me :) Thanks guys, really love the new API!

PS: added this PR https://github.com/ionic-team/ionic-native/pull/2672 to update the ionic typings

pandiarajan-i2i commented 6 years ago

Hi @BorntraegerMarc,

Thanks for your support and your contribution is really great.

We have checked your pull request in the ionic-native repo and seems ci has failed. We also have some filed changes in the ionic-native source code so we will check and update the source.

BorntraegerMarc commented 6 years ago

Yeah I noticed the CI tests are failing. But I don't think it's because of me, as they are also failing on master...

OK. What changes do you have for ionic-native?

pandiarajan-i2i commented 6 years ago

We have some variable name changes.

pandiarajan-i2i commented 6 years ago

Hi @BorntraegerMarc ,

We updated the pull request with your changes and added some additional details. Ci check has passed. We will wait for the ionic-native developers to review it.

https://github.com/ionic-team/ionic-native/pull/2675

Thanks for your contribution.

BorntraegerMarc commented 6 years ago

cool, thanks :) closed my PR...

pandiarajan-i2i commented 6 years ago

@BorntraegerMarc ,

Please feel free to post any issues or new feature if you want to add.

pandiarajan-i2i commented 6 years ago

Hi @BorntraegerMarc , The ionic team has merged the pull request(ionic-team/ionic-native#2675) and updated it the official docs.

FYI... https://ionicframework.com/docs/native/aes256/

BorntraegerMarc commented 6 years ago

Thanks! I have already been using the new version since couple of days 😉