newbi3 Posted December 1, 2015 Share Posted December 1, 2015 I know that there are a lot of password managers out there but it never hurts to have options. Also I don't trust any application with my passwords if I can't review the code. So I took it upon myself to write my own password manager for Android and you can find it in the Play Store now. Its completely free and requires 0 permissions. All passwords are encrypted with 128bit AES encryption and the app requires you to enter your encryption key before you can view the "vault". Your encryption key is never stored in the app - just a SHA256 hash of it so the App can verify that you entered the correct key. The code is open source and up on my Github. If you want to contribute to it that would be really cool :D Also you can find it on the Play Store. I'm completely open to suggestions and criticisms for it. Git Hub https://github.com/frozenjava/VaultAndroid Google Play https://play.google.com/store/apps/details?id=codeit.space.vault Quote Link to comment Share on other sites More sharing options...
cooper Posted December 1, 2015 Share Posted December 1, 2015 I know that there are a lot of password managers out there but it never hurts to have options. Also I don't trust any application with my passwords if I can't review the code. So I took it upon myself to write my own password manager for Android and you can find it in the Play Store now. Its completely free and requires 0 permissions. All passwords are encrypted with 128bit AES encryption and the app requires you to enter your encryption key before you can view the "vault". Your encryption key is never stored in the app - just a SHA256 hash of it so the App can verify that you entered the correct key. The code is open source and up on my Github. If you want to contribute to it that would be really cool :D Also you can find it on the Play Store. I'm completely open to suggestions and criticisms for it. Git Hub https://github.com/frozenjava/VaultAndroid Google Play https://play.google.com/store/apps/details?id=codeit.space.vault I'll look at the code itself in a bit, but the highlighted section looks suspect. There should be no need for you to store the hash to compare anything. What you probably should be doing is hash the key to something and then use that something as a key to encrypt the actual keys. If your key doesn't produce a decrypted password store, clearly it's not the right key. Quote Link to comment Share on other sites More sharing options...
cooper Posted December 1, 2015 Share Posted December 1, 2015 Looked through a few of the files. codeit/space/vault/crypto/Encryption.java Your secret key is allowed to be an empty string. You don't provide any means of seeding the IV prior to encrypting, or salting the hash before hashing, even though the uid you use for a password is an excellent value to use for this. codeit/space/vault/db/VaultDatabaseHandler.java 51: Your method 'vaultKeyExists' actually tests if the vault key *table* exists, since no key is included anywhere. 89: Let the database do the work for you - SELECT count(1) FROM table where vaultKeyHash = 'hash'; 113: In the database column name you suggest the password is encrypted, but your password model doesn't seem to think so. 114: All data from this model should be encrypted. If I get access to your vault I shouldn't be able to prioritize my cracking efforts based on the name you gave to a password nor should I know the places where you have an account. Why is it relevant to retain a creation date? 136: You shouldn't need a getAllPasswords method. You should use a getAllPasswordTitles() method and only when a password is selected do you read it from your database. Quote Link to comment Share on other sites More sharing options...
newbi3 Posted December 1, 2015 Author Share Posted December 1, 2015 (edited) Looked through a few of the files. codeit/space/vault/crypto/Encryption.java Your secret key is allowed to be an empty string. You don't provide any means of seeding the IV prior to encrypting, or salting the hash before hashing, even though the uid you use for a password is an excellent value to use for this. codeit/space/vault/db/VaultDatabaseHandler.java 51: Your method 'vaultKeyExists' actually tests if the vault key *table* exists, since no key is included anywhere. 89: Let the database do the work for you - SELECT count(1) FROM table where vaultKeyHash = 'hash'; 113: In the database column name you suggest the password is encrypted, but your password model doesn't seem to think so. 114: All data from this model should be encrypted. If I get access to your vault I shouldn't be able to prioritize my cracking efforts based on the name you gave to a password nor should I know the places where you have an account. Why is it relevant to retain a creation date? 136: You shouldn't need a getAllPasswords method. You should use a getAllPasswordTitles() method and only when a password is selected do you read it from your database. The password can't be an empty string. Line 44 of AccessActivity checks the length of the key. I am going to have a password policy in the next version but I just didn't get around to it yet. I'm not sure the proper way to do the salting. I shouldn't store the salt in the DB correct? If not then I can't use a random salt and if it can't be random then I should use something unique like the phones identity but I don't want to have to have special permissions on the app. 89 - Thats the query I was running but it would crash the app if certain characters were in the hash I just changed the query last night actually 113 - The password is encrypted before being put into PasswordModel (line 78 of NewPasswordActivity) 114 - Yes I agree next release 136 - I also agree with this - next release Thanks for the review I'll update it soon Edited December 1, 2015 by newbi3 Quote Link to comment Share on other sites More sharing options...
cooper Posted December 2, 2015 Share Posted December 2, 2015 (edited) I'm not sure the proper way to do the salting. I shouldn't store the salt in the DB correct? Incorrect. The problem seeding/salting defends against is that when someone gets access to the vault contents, they can run a wordlist by the encrypted passwords and see if anything matches. When your stored passwords are all encrypted from a single state, the attacker is, with a single run of his wordlist, attacking all encrypted passwords simultaneously. When the encryption algorithm was seeded with a value that's unique for each separate password, the attacker can only attack that 1 password from the list with each wordlist run. If the attacker doesn't know the seed value, all the better because he now not only has to guess the encryption key, but that aswell. If he does have the seed because you stored it along with the password, he still has to guess the encryption key and use a full wordlist run on that single password. The problem with not storing the seed value means that it has to come from somewhere else which might not be the same value when someone, for instance, switches phones but copies over his data before taking the new phone in use. For the proper way of seeding your encryption, in your code, file src/main/java/codeit/space/vault/crypto/Encryption.java lines 26 and 42 the keybytes value should've been the seed and I would personally recommend you either use the unsalted hash of the UID, assuming it's effectively the output of UUID.randomUUID(), or alternatively the also unsalted hash of the name of the key, which because of that gains a few additional requirements regarding length and such. From a database perspective, I believe your vault should be a single table with 2 columns: id (UUID) and data (BLOB or Base64 text for the binary data). When the password was correct, you can decrypt each 'data' element using the id column to produce the seed value of each entry. The data is the name of the title, a separator char (like \r, \t or \n) and then the password. Downside is when you acquire the titles, you also put into memory the keys - you can quickly discard them again (decrypt to a single byte buffer and overwrite/reuse the password part unless you know you need it to mitigate somewhat). Upside is that the value to encrypt/decrypt becomes longer due to the concatenation which means crypto-analysis based on the visible value becomes harder. When you first display the list of stored keys, you show the sorted set of titles but retain a map of title=>id. When a key is selected, take the id, decrypt the record and provide the password. Edited December 2, 2015 by cooper Quote Link to comment Share on other sites More sharing options...
newbi3 Posted December 21, 2015 Author Share Posted December 21, 2015 I'm almost ready to release the next version which has a ton of improvements. 1. All data (not just passwords) is encrypted and salted 2. Data is only shown in plain text when it needs to be 3. Access key is now salted in addition to being hashed 4. New "Access History" feature lets you see the dates and times that a specific password has been viewed 5. New random password generator using SecureRandom Its not released yet and probably won't be for about a week or two as I am on vacation and still have some features to add - however if you want to view the code look at the "development" branch. Quote Link to comment Share on other sites More sharing options...
cooper Posted December 23, 2015 Share Posted December 23, 2015 I like the Access History feature and would recommend you also include any failed login attempts. Might be fun to include a snap from the front-facing camera with that. Quote Link to comment Share on other sites More sharing options...
newbi3 Posted December 24, 2015 Author Share Posted December 24, 2015 I like the Access History feature and would recommend you also include any failed login attempts. Might be fun to include a snap from the front-facing camera with that. I like that idea. I would do the camera snap but I am trying to not use any permissions and that would break that model. Ill add a log of failed login attempts though. Quote Link to comment Share on other sites More sharing options...
cooper Posted December 24, 2015 Share Posted December 24, 2015 Here's a tricky one though: How do you ensure the log doesn't get tampered with? Quote Link to comment Share on other sites More sharing options...
newbi3 Posted December 24, 2015 Author Share Posted December 24, 2015 Good question - I'm not sure. I am not going to allow for the log to be cleared. If someone gets the password right on the 100th time they won't be able to clear the login and the owner would see it next time they logged in. Now that doesn't stop someone from modifying the log with a 3rd party application or ADB or something. Without sending the log to a server somewhere and then pulling it down when needed I'm not sure how I could keep it from being tampered with. Maybe the alternative option is to have a kill switch built in where if your password is entered incorrectly x times in x timespan then all of the passwords are dropped. Quote Link to comment Share on other sites More sharing options...
Karit Posted December 25, 2015 Share Posted December 25, 2015 Here's a tricky one though: How do you ensure the log doesn't get tampered with? Good question - I'm not sure. I am not going to allow for the log to be cleared. If someone gets the password right on the 100th time they won't be able to clear the login and the owner would see it next time they logged in. Now that doesn't stop someone from modifying the log with a 3rd party application or ADB or something. Without sending the log to a server somewhere and then pulling it down when needed I'm not sure how I could keep it from being tampered with. Maybe the alternative option is to have a kill switch built in where if your password is entered incorrectly x times in x timespan then all of the passwords are dropped. With a lot of these things need to consider your threats, risk and advisories and pick the most important to target. If someone has third party access they can make a copy of everything (especially if phone is rooted) and they can brute force offline. If wipe after X wrong passwords either go back to safe copy each time or just update the counter back to zero. Main thing if lose the phone or if some else accesses it the brute time should be longer than the time it takes you to change all your passwords. The phone's password and full disk crypto are important here. Quote Link to comment Share on other sites More sharing options...
newbi3 Posted December 27, 2015 Author Share Posted December 27, 2015 Login history is all done and will be included in the next version. It doesn't take a picture with the front facing camera because I don't want to add the permissions but it does warn you when there are new failed login attempts and you can view all, successful, or failed attempts individually. Quote Link to comment Share on other sites More sharing options...
cooper Posted December 27, 2015 Share Posted December 27, 2015 Should there be a limit on the amount of successive failed login attempts within a set timeframe? Like the first x logins will fail and instantly allow a retry, but after that each successive attempt must be, say, 5 minutes later until a succesful login occurs. Quote Link to comment Share on other sites More sharing options...
Recommended Posts
Join the conversation
You can post now and register later. If you have an account, sign in now to post with your account.