search

Kunzisoft / KeePassDX

Lightweight vault and password manager for Android, KeePassDX allows editing encrypted data in a single file in KeePass format and fill in the forms in a secure way.
https://www.keepassdx.com/
GNU General Public License v3.0
4.7k stars 276 forks source link

Feature request: quick unlock #102

Closed Fred-Vatin closed 3 years ago

Fred-Vatin commented 6 years ago

I use the Keepass2Android app and this feature make it very pleasant to use. When DB is auto lock, then you only have to type the 3 last letter of your password to unlock. I have a long pass phrase so it is really helpful.

ghost commented 6 years ago

I'd also like to suggest some kind of PIN lock as an addition to the Fingerprint lock. There are a bunch of Android devices flashed to custom ROMs, which usually make fingerprint scanner non-working.

bluepostit commented 4 years ago

I see this is an old request, but as a new user I'd really appreciate having a quick unlock like on Keepass2Android. It seems to be well thought-out: you can only use the quick unlock once you've already unlocked with the full password, and if your quick unlock fails, you are forced to unlock with the full password. Once that's done, you can use quick unlock again.

J-Jamet commented 4 years ago

For me this functionality is not useful for this application because the quick unlocking of KP2A implies that the data is temporarily stored in another intermediate database, which is not the case for KeePassDX. I am thinking of other methods such as PIN or patterns to get the password in the keystore (same as biometric). Recovering a password by a piece of this shorter password has no interest, put a short password as the main credential has the same effect.

EDIT : After studying how K2A works, there is no intermediate database but only one parameter which indicates that an open database is locked. Other solutions are discussed later in this conversation to encrypt the database during the quick lock.

TheOtherDoctor commented 4 years ago

Does that mean, the architecture of KeePassDX hinders to go the same approach as KP2A, or do you consider the approach of KP2A to be inherently insecure?

I am also using a long complex passphase for my vault, and make daily usage efficient via the quickunlock feature. As KeePass2Android does not make it into the f-droid since years (see https://github.com/PhilippC/keepass2android/issues/38), I'd be very happy to have a alternative for my google-free LineageOS devices... So, for my usage approach, a PIN or pattern would be also fine but yet I am unclear how this does improve, compared to allowing quickunlock via the end-of-passphrase.

J-Jamet commented 4 years ago

Does that mean, the architecture of KeePassDX hinders to go the same approach as KP2A, or do you consider the approach of KP2A to be inherently insecure?

Edit : My apologies, I come back to my explanation, I wrongly thought that the operation of the K2A quick lock involved encryption. Refer to "https://github.com/Kunzisoft/KeePassDX/issues/102#issuecomment-636650198" for more information.

Old response: They simply weren't built the same way. I cannot give an objective opinion. A study should be made of each advantage and disadvantage of each method. For me, the redundancy of information in an internal database is not necessary in this case because it simply offers a new way to obtain encrypted information but it is secure with what you call the quick unlock.

If you want a comparison, in my mind, it's like opening a house, on the main door of the house we added big locks and we have to make lots of turns of keys.

Advanced unlock (Fingerprint recognition/Pin/Pattern in the future) uses the same feature in both apps. It is like having a muscular butler who keeps the main key on him (keystore on the device), when you ask him, he opens the door for you.

Tell me if it helps to understand, obviously it only represents my vision. I am open to criticism.

shadow00 commented 4 years ago

Would there be a way to store part of the password in memory, encrypted in some way? And when the user comes back and types the missing characters, the main password gets decrypted and is used to open the full database. That way, you don't need to change the way you handle the entire database, but only worry about how you fetch the decryption key.

Example: Full db password is 20 chars; Quickunlock length (set by user) is 4 chars; After unlocking the db the first time, you also encrypt the first 16 chars of the password using a key derived from the last 4 (using a PBKDF), and store the encrypted result in memory as long as the app is open.

When the user comes back, ask for the Quickunlock missing chars, decrypt the first 16 chars, combine them with the QU input, and try to use that to decrypt the main db. If db decryption is successful, all good; if not, delete the encrypted partial password from memory and revert to asking for the full password.

This way, you still the cost/benefit tradeoff of the Quickunlock function, without having to reimplement the way you handle data internally.

rugk commented 4 years ago

Well… in any case – in addition to maybe use that – you can likely use the fingerprint authentication API in new Android versions (AFAIK also works with other unlock screens) and the Android keystore, which you can configure to require user authentication and you can also enforce the time-locking mechanism there, apparently.

Pretty much what we want…

Disclaimer: no Android developer here

J-Jamet commented 4 years ago

@rugk It's already in the code https://github.com/Kunzisoft/KeePassDX/blob/master/app/src/main/java/com/kunzisoft/keepass/biometric/BiometricUnlockDatabaseHelper.kt#L63. But I have not yet managed to operate the native unlocking of Android (pin / etc). I will debug this part when I have time.

ghost commented 4 years ago

@shadow00 I'd say this is quite a nice idea, at least in theory... I know that fingerprint unlock is in place, and maybe instead of fully unlocking the database with the Fingerprint, we could store partial password, and then have to input last (or first) n chars...

Maybe this could be available as additional security option, where we can also select how many chars we should add, and which side (left or right)...

J-Jamet commented 4 years ago

As I said earlier, storing the main password to recover it with part of the same password has no security interest in the case of KeePassDX. It is the same as directly storing a n-letter password as the master password. You can add a key file if you want a second credential.

(After if you want, we can think about storing part of the part of the part of the password part. To be able to open it with a single letter. :D It's a little joke to make you understand that it's useless. Just increase the number of derivations in the KDF setting with a small password, it will have the same effect.)

shadow00 commented 4 years ago

@J-Jamet

Just increase the number of derivations in the KDF setting with a small password, it will have the same effect.

I disagree with this statement.

Perhaps you've misunderstood my suggestion: I don't mean 'store' the encrypted partial password in file storage; I mean store it in RAM while the application is open/in the background. Unlocking the db when launching the app from zero will always require the full password.
The point of QuickUnlock is to avoid having to re-type the full main password every time the db gets auto-locked after a few minutes but the app is still open. That way, if someone grabs my phone and tries to unlock my db while it's in QU mode, they have exactly one attempt at guessing the missing piece of the password, and if they fail they have to start guessing the full password.
If I completely close the app, the encrypted password gets wiped from memory, and next time I open the app I have to use the full password again.

This is not the same as using a n-letter password as the master password.

If the full password is short (eg: 4 characters), it would be easy to just enumerate all the possible combinations. And increasing the number of iterations in the KDF to a "safe" level would mean waiting a long time every time I unlock/save the database (which becomes even longer on mobile devices that share the db with a computer).

On the other hand, if the main password is long and secure (eg: 25 characters), I can keep the number of iterations down to a reasonable level (eg 1-second unlock time). Then I enable QuickUnlock and set it to 4 characters: now, if someone tries to guess the QU shortened password, they have only 1 in 624 (14.77 million) chances[1] of getting access. After they fail, the partial password gets wiped from memory and they're back to guessing the full password - which is never gonna happen with a brute-force attack.

The only way to attack someone using this feature would be to extract the encrypted partial password from RAM before it gets wiped (either by fully closing the app or after a wrong guess), and try to bruteforce the QU combination - which would still take almost three months on a single machine[2].
And let's be honest: if you're worried about such an attack scenario, you're probably not the target audience for this feature. For everyone else, I think it is a reasonable compromise between security and usability.

Bonus: the length of the QU password is easily configurable in code, since all you have to do is encrypt the first n - m characters using the last m. 624 is not enough security for you? No problem, set the length to 6, 8, or however much you feel comfortable re-typing in a pinch when you need to keep the db quickly available.
You could also decide to hide the length of the required QU string for some extra obsuscation, so if someone grabs your phone from your hands they won't know how many characters are required.

[1] assuming English alphabet, uppercase, lowercase, and numbers.
[2] assuming each decryption of the main password is instantaneous and a 1-second db unlock time on a reference machine, the attacker would, on average, have to go through 50% of 624 attempts, taking 7388168 total machine-seconds, or ~85.5 machine-days.

shadow00 commented 4 years ago

This would also give a quick access method users who don't have/don't want to use biometric unlock (at least after unlocking the db the first time).

J-Jamet commented 4 years ago

Indeed, we are not talking about the same thing. I understand what you mean, but the problem in this case comes from the workflow usage. We are supposed to close the main database in KeePassDX at the end of each use. So for me, putting the password in Ram is useless since it will be deleted each time. I plan to implement Ripple to close the database in case of panic and turning off the screen also closes the database.

The point of QuickUnlock is to avoid having to re-type the full main password every time the db gets auto-locked after a few minutes but the app is still open.

KeePassDX never worked like this. The auto-lock has always closed the database.as I explained earlier, there is no temporary database to protect temporarily. Here when we talk about quick unlock or advanced unlock, it is to use alternative methods which use the keystore of the device to automatically manage the password. So always available methods which are not deleted. Next, we can think about using these alternative methods with passphrases that are not linked to the main password (https://github.com/Kunzisoft/KeePassDX/issues/106). But we would lose compatibility with other software.

If you want a comparison, the workflow usage is the same as KeePass on PC. There is no quick unlock like KP2A because there is no temporary database to protect. If you close the database, it is closed, it is not kept in memory.

Of course I am also thinking about deleting the Keystore keys if necessary. (https://github.com/Kunzisoft/KeePassDX/issues/437)

For what you said before:

Just increase the number of derivations in the KDF setting with a small password, it will have the same effect.

I disagree with this statement.

I therefore maintain what I say, if we go in this way of storing parts of passwords in KeePassDX, it will be as secure as if we used small master passwords.

If the full password is short (eg: 4 characters), it would be easy to just enumerate all the possible combinations. And increasing the number of iterations in the KDF to a "safe" level would mean waiting a long time every time I unlock/save the database (which becomes even longer on mobile devices that share the db with a computer).

You're right, it will be easy for a hacker to decode the base, so it's why I did not understand why everyone wanted this.

rugk commented 4 years ago

I did not understand why everyone wanted this.

Usability of course. The problem is: I cannot enter my long master password on a mobile device, each time I access just one password. How that is solved technically is another thing.

And security is always a trade-off, between usability and security here. (As long as each user can choose what they want and know the security implications, it's all fine.)

If you want a comparison, the workflow usage is the same as KeePass on PC. There is no quick unlock like KP2A because there is no temporary database to protect. If you close the database, it is closed, it is not kept in memory.

Well… this comparison is like comparing Apples und Bananas. The workflows on a mobile phone vs a desktop are totally different. Also the application behaves differently. First, when using the magic keyboard e.g. (when I've used it the last time), I did have to re-enter the password at each password request. That is totally different than on a desktop PC.

Also Android frequently kills the app.

KeePassDX never worked like this. The auto-lock has always closed the database.as I explained earlier, there is no temporary database to protect temporarily.

Well… that's the problem then. The proposal as far as I see is to have two security levels:

The thread model is different for both.

J-Jamet commented 4 years ago

Usability of course. The problem is: I cannot enter my long master password on a mobile device, each time I access just one password. How that is solved technically is another thing.

Yes this is the purpose of this discussion, find a way to quickly open a database in KeePassDX (without biometric). But the quick unlock of K2A with password part is not a technical solution here.

I did not understand why everyone wanted this.

When I say this sentence, it is that I do not understand why the users want to stay on an unlocking solution which is not adapted to KeePassDX. There is no technical or functional reason to use an existing part of the main password in KeePassDX as advanced unlock. I prefer to think about the native Android unlocking implementation to access the keystore (schema, etc...).

Well… this comparison is like comparing Apples und Bananas. The workflows on a mobile phone vs a desktop are totally different. Also the application behaves differently. First, when using the magic keyboard e.g. (when I've used it the last time), I did have to re-enter the password at each password request. That is totally different than on a desktop PC.

By workflow, I mean of course the workflow of opening the database. The magikeyboard can be used when a database is open, that's all. But I don't understand the link with the discussion. You can open a new issue if you have a problem with the magikeyboard.

Also Android frequently kills the app.

This is not normal, KeePassDX use a foreground service to not killed the app if the database is open. If you have this problem, please open an issue. There may be Android systems from manufacturers that force the application to close.

There is no "semi-lock" in KeePassDX, a database is either open or closed. To implement the semi lock, we have to change the complete architecture of the app and the behavior of the lock methods, we must add another database to copy all the content of the main database which will be managed separately. If it is to have only a shorter temporary password based on a part of the main password, I do not see the interest to completely modify KeePassDX to be identical to KeePass2Android, just use K2A. (use the one that suits your needs.)

KeePassDX is not made for the semi-lock. But the discussion is to find a way to open the database quickly using the Keystore on the phone without using the fingerprint (with a schema, or a PIN, etc..)

Sorry, I may be misunderstanding, I still use the translation tools.

ghost commented 4 years ago

@J-Jamet I might have not expressed myself clearly in my comment.

@shadow00 I'd say this is quite a nice idea, at least in theory... I know that fingerprint unlock is in place, and maybe instead of fully unlocking the database with the Fingerprint, we could store partial password, and then have to input last (or first) n chars...

Maybe this could be available as additional security option, where we can also select how many chars we should add, and which side (left or right)...

What I meant is actually using the Fingerprint combined with the last letters of the password. I mean I'm not aware how the Fingerprint feature works, I just assumed that it stores the password somewhere in order to decrypt the DB, and that's why I suggested what I suggested :slightly_smiling_face:

J-Jamet commented 4 years ago

@Sk1pp3r Indeed, I did not fully understand what you meant. What you are proposing is a technical solution idea for issue #106. I will add a comment in the corresponding issue! ;)

shadow00 commented 4 years ago

@J-Jamet

Here when we talk about quick unlock or advanced unlock, it is to use alternative methods which use the keystore of the device to automatically manage the password.

This is not what I, and the OP who requested this feature, are talking about. The QuickUnlock feature in KP2A only applies after you unlock the db the first time, either by typing your full password or by using biometric authentication. When you unlock, you also choose whether to enable QuickUnlock or not.
After that, if QU is enabled, when the db gets auto-locked after the chosen timeout, you have one chance to unlock it with the partial password without having to go through the keystore and whatnot.

Now: you say KP2A does it by storing data in an intermediate database, and that's inefficient and would require major internal rework, which would not be good. I agree with that.
What I suggest is to only store the master password itself in ram in encrypted form, while the database itself gets fully locked away as usual - no intermediate/partial state or anything.

This is my suggested flow for a user with QuickUnlock enabled: 1) The user opens the app and attempts to unlock the database by providing the full master password. 2) If the unlock is successful, take a copy of the master password and encrypt it through a KDF[1] using the last n chars of the password as key, then store that output in ram. This can task be performed in the background while the user interacts with the app. 3) If at any point the user fully closes the app (swipes it away), lock the db and close the app (thus wiping the encrypted password too). Goto (1). 4) After the timeout for user inactivity, the db gets auto-locked. The database is now fully locked, in the same state as (1). We still have the encrypted master password in ram. 5) Now, the user exactly one chance to unlock through the QU feature: acquire QuickUnlock input, use it as a key to decrypt the master password that we have in memory, then take that decryption output and try to use it as the master password. Note that we have no way to know if the decryption output is the actual master password before attempting to decrypt the database with it. 6a. If the decryption of the database is successful, we are now back to the same state as (2). Goto (2). 6b. If the decryption of the database fails, wipe away from memory the encrypted password. We are now back in the same state as (1). Goto (1). 6c. The user fully closes the app. The database was already locked in step (4), so we only need to wipe the encrypted password and close the app. Goto (1).

[1] For better security, the KDF should include a different random salt each time this operation is performed

KeePassDX never worked like this. The auto-lock has always closed the database.as I explained earlier, there is no temporary database to protect temporarily.

There is no temporary/semi-locked database in my proposal either. The database is either unlocked (when it's open), or fully locked with the full password (when the app is not running in the background AND when it gets auto-locked after the set timeout and the app is still open in the background). We're using the same name as KP2A, but the implementation I'm suggesting is fundamentally different from the way KP2A does it.

Any transformation done to the user's input happens prior to the unlock function call.
Therefore, the method I suggested does not break compatibility with other implementations, because it does not change the fundamental mechanism of how the database gets unlocked (ie: by decrypting a db through the master password), but only how you acquire the master password itself in a specific scenario (by decrypting the partial master password with the QU input after a fully unlocked db was auto-locked). In fact, now that I think about it, this could be implemented on any KeePass implementation on any device.

TheOtherDoctor commented 4 years ago

@shadow00 has perfectly the state flow concept formulated as I had it in mind, thanks a lot for making this so clear.

As far as I understood by now, the proposed PIN or fingerprint "quick unlocks" all rely on the android keystore. I am not really familiar with the keystore details, thus that may be just a personal feeling resulting from lack of understanding - but I'm not yet clear why we'd have to use this keystore and why I should trust it to hold my KeePass master key. I understood the sketch by @shadow00 to somewhat do an internal keystore instead, which I'd like much better.

p.s. First: thank you all for the helpful discussion, providing great critical insight into the design concepts - and the resulting challenges.

TheOtherDoctor commented 4 years ago

We are mostly talking about using a substring of the password as a "reopener" - for obvious pragmatic reason (not having to ask the user to provide a re-opener pass). But in general, the "quickunlock" is just an independent key to recover a cached master passwd from application memory.

From the TFA concept of "things the user knows" and "things the user has", I'd actually see the state/event, that the DB has been recently unlocked, as "something the user has". Seing it like this might make is easier to take this as a key component in a multi-path unlocking scenario to cover several multi-factor combinations.

As a sidenote, my highly preferred solution for quickunlock would be a physical token (Yubikey or the like), which then could be used as a second factor in combination with a short PIN - I'd expect this should be quickest; I'd be willing to invest for a trial, if I'd only see a realistic change to get the functionality on all relevant platforms (Ubuntu, Win10, Googledroid, LineageOS, ...); as not all my devices have a (working) fingerprint scanner, referring to #106, I'd be happy to use a hardware key as the 2nd factor along with a PIN. Pragmatically, I'd wonder if it would be feasible to smoothly use a key stored on a wire-bound device, like a USB stick? If that scenario means lots of clicks, it's useless - but if it could work on my mobile device just like on my PC, with automount, it would be a great (and very cheap) thing.

rugk commented 4 years ago

but I'm not yet clear why we'd have to use this keystore and why I should trust it to hold my KeePass master key.

You don't have to, but it's very useful. 1. it's used anyway, already. 2. you should trust your OS anyway, but most importantly 3. the way the keystore works AFAIK is that you get an (asymmetric) encryption key from it, so you have the secret in your application anyway and encrypt your own things, …

J-Jamet commented 4 years ago

Okay, I can see more clearly the reasons why we don't understand each other. Finally, this is only an interpretation of the concepts. I will try to explain each point in more detail.

I think I have not been clear on several subjects, sorry for that, I have my nose in the code very often so it can be difficult to get out of it a few times.

Keystore: First, let's talk about the Keystore (java doc). It is a class that allows keys to be stored securely, it uses internally the same encryption algorithms used by KeePass (https://github.com/Kunzisoft/KeePassDX/blob/master/app/src/main/java/com/kunzisoft/keepass/biometric/BiometricUnlockDatabaseHelper.kt#L290). The device's keystore advantage : it is connected to the identification systems (fingeprint and unlocking screen). And natively, the google API offers utility methods to use these locking methods. It is of course possible to use separate Keystore, which we just initialize for KeePassDX. The storage of a key can be done by any other key (a hash of a file, a hash of retinal print or fingerprint, a passphrase, etc ...), this is why I did not understand the desire to keep only a piece of the same key, might as well use a more intuitive way compatible with all devices. ;)

@shadow00 There is a concept that is not clear: closing the application. Concretely, the closing of an android app is not so easy to define, it is not because visually it is not visible that it is closed and vice versa, it is not because we have access to variables in the app that it is open. KeePassDX uses simple concepts because I didn't want data redundancy. So for simplicity, let's say that if a database is closed, the application is closed. The application is only opened if there is a database notification which indicates that a database is open (this indicates that a service is active and it can retrieve action commands). If there is no screen and no notification, it is considered closed for the issue explanation. If you only see the screen for selecting recent databases, the application is considered to be also closed, since no action in the background. I set up these concepts because in Android, we have to manage the cycles of construction and destruction of context (Application / Activity / Service) and keeping an active variable in the context of the application involves storing it in a space memory which is not guaranteed to be destroyed. It is not possible to communicate directly with the RAM for security reasons.

What you suggest is to store the master key in a separate keystore stored only in RAM. Which is feasible if it is stored in the context of the application class (but no guarantee of destruction), I hadn't thought of it because simply using the Keystore on the phone was more appropriate and I didn't see the point of creating another keystore. Why? Because it is possible to delete a key from the keystore, so deleting the encrypted password in RAM after an event would be equivalent to deleting a key from the keystore of the device after the same event. So finally, deleting a Keystore key after each bad recognition is a technical solution to this issue. I just also opened this one which is linked for fingerprint: #548 And it's an improvement to https://github.com/Kunzisoft/KeePassDX/issues/437.

Your scenario (Points 1 to 4) is already in place for fingerprints (just replace RAM with Keystore), so it would have to be adapted for other opening methods (Pattern, PIN, Passphrase or other https://github.com/Kunzisoft/KeePassDX/issues/102#issuecomment-624738030), and add point 5 (#548 ).

J-Jamet commented 4 years ago

I do not know if it is clearer? I try to have an objective point of view compatible with the technical needs of the app. So this is my interpretation of the solution you describe. It is never easy if the request is based on another application whose internal functioning is not identical.

shadow00 commented 4 years ago

@J-Jamet sorry, I've been busy the last two days and could not reply sooner.

I was not very familiar with the Android Keystore, but from what I read it should be the best place to store the encrypted master password - no need to implement your own version of the Keystore.
Now that I look at it, I realize my proposal is similar to storing the encrypted master password in the Keystore and requiring user authentication - but instead of using the phone's Pin/Password/Biometric unlock as a form of authentication, it uses part of the database's master password.

If we encrypt our copy of the password before we place it in the Keystore, and we request the Keystore to remove it after one failed unlock attempt, I think we don't really need to require user authentication to access it.
Therefore, implementing this feature would allow a user to quickly unlock the database even if they have not enabled any form of security/authentication on their phone.
(Note that this still allows you to implement the "traditional" Pin/Password/Biometric unlock methods through UserAuthentication as an alternative way to access the db.)

I thought that Android/Java had a method to securely wipe a variable from ram (something like Windows' SecureZeroMemory function), but it seems this does not exist in Java. Therefore, the Keystore seems to be the best place to store the encrypted password.

I guess the best way to implement this would be through a Service running in the background, which gets created right after the user unlocks the database (encrypting the master password with the last n chars and then adding it to the Keystore - if the QuickUnlock feature is enabled), and gets destroyed (removing the encrypted master password from the Keystore) when the user fails the QU unlock attempt, or the application is terminated (along with all its services).

J-Jamet commented 4 years ago

No worries, we respond when we have time ;)

If we encrypt our copy of the password before we place it in the Keystore, and we request the Keystore to remove it after one failed unlock attempt, I think we don't really need to require user authentication to access it.

I still have to study the identification methods that I can combine with the Keystore, so I cannot answer this part for the moment but yes it is doable.

I guess the best way to implement this would be through a Service running in the background, which gets created right after the user unlocks the database (encrypting the master password with the last n chars and then adding it to the Keystore - if the QuickUnlock feature is enabled)

Operation is roughly similar with biometric decryption. So we have to look at which methods to reuse to add key in the Keystore.

... and gets destroyed (removing the encrypted master password from the Keystore) when the user fails the QU unlock attempt, or the application is terminated (along with all its services).

This is where it gets complicated, because there is no event that indicates that an application is terminated (as explained earlier). When the databases are closed, there are no more services running, we must therefore consider the application closed. So the key will remain in the keystore until it is told expressly to destroy it. We can destroy it after receiving the screen extinction broadcast for example.

shadow00 commented 4 years ago

When the databases are closed, there are no more services running, we must therefore consider the application closed.

This is something that would have to change: the service that keeps the encrypted password in memory after the initial unlock must stay alive after the db gets auto-locked. KP2A does this through a foreground service, that keeps a permanent notification enabled as long as QuickUnlock is available. So if you can manage to tie the lifetime of the encrypted password with the lifetime of the foreground service, you have everything you need to build this feature.

A naive implementation I thought of would be to use the foreground service to keep extending the temporal validity interval of the key (eg: add 1 extra second of validity every second). As long as the service is alive, the key will be kept in the keystore, and the user will be able to authenticate with it. If the service gets killed (either by the system, or because the user pressed the 'Close' button in the notification like in KP2A), then the encrypted password in the Keystore will expire, and the only way to unlock the db again will be with the full master password/standard biometric authentication.

I don't know if services are allowed to perform some "cleanup" when they get sent the killing signal by the system or not?
Or maybe something like await (UserCloseCommand || SystemKillSignal) (pseudo-code) could be implemented? So the foreground service just sits there waiting for one of those signals from the user or the system, and when it does, it removes the password the Keystore. That way you don't need to worry about extending the validity of the key every second - you just place it there, and you wait until it is either used or deleted.

J-Jamet commented 4 years ago

The foreground service seems the right way for the feature, but there are conditions that bother me. Adding 1 extra second of validity would consume a lot of battery if the validity period of the key is long. A better approach is to define a configurable time in a setting and send a closing request broadcast with a receiver in the service. But there is another problem with this technique (if using time validity), the documentation says:

Temporal validity interval authorizations are unlikely to be enforced by the secure hardware because it normally doesn't have an independent secure real-time clock.

So we can't really count on the clock of this feature. And if the service is no longer accessible, the problem remains the same the easily crackable key would still be valid. If a service is killed by the system, it is normally restarted if the return value of the onStartCommand() method is adapted. But it will not restart if it is killed by the user (If the user manually kills the app). There is no foolproof way to keep active service.

Or we come back to one of the previous ideas of creating a secondary Keystore included in the service, we are sure that it is erased when the app is killed.

If this quick unlock method is chosen. We will have to indicate to the user that there will be 2 types of unlocking assistance (short and long). I'm afraid it's too confusing.

For me, the main purpose is to quickly open a database, if we add more secure alternative unlocking methods like patterns and add a reasonable timer setting to remove the key from the keystore. It would be less confusing. It does not matter if the method of deleting or invalidating the key is not precise because it is stored more securely in the Keystore than a 4-char code. What do you think?

rugk commented 4 years ago

the main purpose is to quickly open a database, if we add more secure alternative unlocking methods like patterns and add a reasonable timer setting to remove the key from the keystore. It would be less confusing. It does not matter if the method of deleting or invalidating the key is not precise because it is stored more securely in the Keystore than a 4-char code.

You mean it does not matter if the key is deleted after 10min or 10½ minutes or even 12 minutes? If so, yes, I totally agree. If you cannot achieve it and would need to trade-off it with a secure keystore storage, then certainly ignore the time limit. It does not need to be exact.

IMHO the TL;DR should be: Store the key as secure as possible while still allowing a convenient quick unlock mechanism (limited by time).

schlomie commented 4 years ago

Something else to consider - and this may be more of a concern when Hardware Tokens come to more fruition.

keepass2android's quick unlock also allows you to quickly unlock a db secured with an hmac challenge response from a Yubikey, without needing the Yubikey to unlock. Only when the db is fully locked is the Yubikey required.

This is a huge plus. I use a second, older phone, (not connected to any network,) paired with an InputStick, as an entirely offline password manager that I use frequently throughout the day. When I need to step away for a while, I fully lock down the kdbx again, requiring use of the Yubikey. Requiring the Yubikey on every unlock would be overly inconvenient in this use case.

Just something to consider when designing the quick unlock interface with the keystore.

leondzn commented 4 years ago

@J-Jamet I might be able to help you implement this quick unlock feature. I understood completely what @shadow00 meant about the possible approach to take this on. However, I am not at all familiar with the Android Keystore implementation as I've never used it before in any of my previous projects. But I am very willing to study this one.

I also believe a quick unlock feature is an important compromise between security and usability. I personally would prefer to have a really strong master password in case my .kdbx gets physically stolen for some reason. But if I'm only accessing the database in my phone, I want it to be convenient enough so that's it's not a chore to open the database every single time. Although I use the fingerprint unlock, there are users who doesn't want to use it, or maybe some of them doesn't have a fingerprint sensor on their phones and have no other options for quick-unlocking the database.

K2A just happened to use the last n number of characters of the master password but it could be anything, really; a PIN, or separate shorter passphrase. Last n characters just happened to be more convenient since the user doesn't have to remember anything new.

I also came from K2A and have used it for 3 years. Only recently switched to KeePassDX and I absolutely love it so far.

J-Jamet commented 4 years ago

@leondzn ~I think you haven't read the whole conversation. ;)~ It is not only a question of use, there are also technical constraints.

leondzn commented 4 years ago

@J-Jamet Yeah. I understood the technical constraints. I think I also just need to read about how to use the Android Keystore and how to store and access data there. I also understand that the best case scenario here is to not use a separate keystore as I agree that it will become too confusing and by extension, harder to maintain.

But in regards to deleting an entry to the keystore in case the a foreground service is killed (either by the user manually force-stopping the app, or the OS deciding to kill it), doesn't the Service class have an onDestroy callback that enables for cleanup? Or is it not possible to access the Android Keystore in that case?

J-Jamet commented 4 years ago

Sorry, I'm the one who reads too fast ... I know that it is important to implement it but it is the question of the technical choice which will determine the roadmap of the developments. I'm more in favor of keeping only the app Keystore with timer. Adding another KeyStore to the foreground service does too much work and maintenance and will add confusion to the user. The onDestroy() method is not necessarily called, that is the whole problem. https://stackoverflow.com/questions/7236782/is-an-android-service-guaranteed-to-call-ondestroy

@schlomie I agree, we must think in advance about the integration of physical keys, I have not yet fully thought about it in integration. It must be done absolutely before getting started. Edit: Linked to Hardware Key

leondzn commented 4 years ago

Yep. It's clearer now. And yeah, I suppose onDestroy isn't guaranteed to be called each time. Faced with that problem in the past as well but I just assumed it was just a result of a really bad code that I've worked with in the past.

Was also not aware about the Keystore with timer. Just noticed it now that you mentioned it. I literally just started using this app yesterday. Lol. Anyhow, I'll take that into consideration. Thanks for mentioning.

In any case, since you intend to implement a PIN unlock feature anyway (152), a quick unlock feature would also still be implemented one way or another.

I think I see your point about this. While it is entirely doable, it's a matter of design choice based on the current technical constraints. We just have to find better ways to implement this without over-complicating the process.

J-Jamet commented 4 years ago

The PIN unlock is directly linked to the choice we will make in this discussion. For the moment, I don't know if you can use the PIN unlocking system built into the phone. We have to do tests, and check the constraints of the Keystore to use this feature. It is this PIN unlock that I had in mind, so it is another question of knowing if we integrate it with a personalized UI or not in a secondary Keystore or not.

shadow00 commented 4 years ago

@J-Jamet @leondzn what if the password is also salted with a random "session id" that is owned by the service? That way, the password will be protected by something you know (the last n characters) and something you have (the random session id that belongs to the service).

If the service gets killed by the system and we fail to remove the encrypted password from the keystore, an attacker will still be unable to attack it, because the lifetime of the "session id" is tied to the lifetime of the service, and the system takes care of deleting it when reclaiming resources.
An attacker would have to find a way to extract this information from the service before it gets destroyed (either by the user or by the system), just to be able to begin mounting a brute force attack against the QU password.

This way, we exploit the way the OS deallocates resources in our favour.

Does that make sense? Or are there other hurdles in the way?

shadow00 commented 4 years ago

The only (minor) problem to solve would be checking if an old copy of the encrypted password (for which we don't have the session id anymore) was still left in the Keystore when you open the app again, and removing it to avoid confusion the new copy that will be created after the new first unlock. This is to avoid filling the Keystore with abandoned copies of our password.

Since that value does not exist yet in State 1 (when the QuickUnlock service is being created again), we could simply check if there's anything already in the Keystore and safely delete it. I'm just not sure how the "checking" process would work, or how actually feasible it would be: can a Keystore item be "identified" by the app that created it? If there is a way for the app/service to say "yes, this item in the Keystore belonged to me, and it is the QuickUnlock password", then the deletion method I suggested is feasible. If not... well, you guys would have to figure something out xD.

J-Jamet commented 4 years ago

Does that make sense? Or are there other hurdles in the way?

Your solution of a salted password in a session is a very good idea if we use a service. The key/alias must be deleted as soon as possible in the KeyStore when the service is no longer available (and before creating another one). It will be necessary to test but I don't think there is a big problem at this level.

Sunnyji commented 4 years ago

For me this functionality is not useful for this application because the quick unlocking of KP2A implies that the data is temporarily stored in another intermediate database, which is not the case for KeePassDX. I am thinking of other methods such as PIN or patterns to get the password in the keystore (same as biometric). Recovering a password by a piece of this shorter password has no interest, put a short password as the main credential has the same effect.

I have rooted phone. I checked Keepass2Android's data folder before and after loading the keepass database. I made copy of the data folder in my internal SD card before and after loading the keepass database and checked each file's MD5. The only different files that I found were cache file and Keepass2android (SQLite db file). The db file had nothing but path of the opened keepass database file. I locked the loaded keepass, deleted the cache file and quick unlocked the keepass App. The app unlocked quickly for last 4 password characters. I also checked sdcard0/Android/data folder. K2PA doesn't create any files in this folder. This shows K2PA doesn't store password or part of password or copy of Keepass database anywhere but RAM.

J-Jamet commented 4 years ago

@Sunnyji I do not understand the purpose of this message and how it helps the implementation of the functionality in KeePassDX? And especially the link with what I said. If you want to do a security audit of KeePass2Android that's fine, but it does nothing to help resolve the technical constraints of KeePassDX. I said that KeePassDX does not have an intermediate database. It still hasn't changed.

EDIT: The K2A technique of using a variable on an open database will not be used because it suggests that the database is encrypted when it is just not accessible by the UI.

J-Jamet commented 4 years ago

For those who have questions about how the K2A QuickUnlock works, I pushed my analysis further: https://github.com/PhilippC/keepass2android/blob/master/src/keepass2android/QuickUnlock.cs#L345 https://github.com/PhilippC/keepass2android/blob/master/src/keepass2android/app/App.cs#L415

The most interesting lines are:

public Database GetDbForQuickUnlock() {
    return OpenDatabases.FirstOrDefault();
}

,

public void Lock(bool allowQuickUnlock = true)
        {
            if (OpenDatabases.Any())
            {
                if (QuickUnlockEnabled && allowQuickUnlock &&
                    GetDbForQuickUnlock().KpDatabase.MasterKey.ContainsType(typeof(KcpPassword)) &&
                    !((KcpPassword)App.Kp2a.GetDbForQuickUnlock().KpDatabase.MasterKey.GetUserKey(typeof(KcpPassword))).Password.IsEmpty)
                {
                    if (!QuickLocked)
                    {
                        Kp2aLog.Log("QuickLocking database");
                        QuickLocked = true;
                        LastOpenedEntry = null;
                        BroadcastDatabaseAction(Application.Context, Strings.ActionLockDatabase);
                    }
                    else
                    {
                        Kp2aLog.Log("Database already QuickLocked");
                    }
                }
                else
                {
                    ...
            }
            else
            {
                ...
            }
            ...
        Application.Context.SendBroadcast(new Intent(Intents.DatabaseLocked));
 }

and

/// <summary>
/// If true, the database must be regarded as locked and not exposed to the user.
/// </summary>
public bool QuickLocked { get; private set; }

This code partially shows how K2A works. I thought wrongly (excuse me for that, I should have pushed the analysis further) that the data was encrypted during the Quick unlock (which seemed logical to me).

But by referring to the code, on K2A it's a simple variable on an open database and a visual lock so that a physical user does not access the data by the UI. (Broadcasts seem to close the activities and services related to the use of the entries, tell me if I'm wrong.)

Personally, this operation does not satisfy me, because ultimately there are the same theoretical problems as what I describe in this conversation. For me, the most secure solution is still to use the idea of ​​the salted password in the KeyStore.

J-Jamet commented 4 years ago

In truth, what upsets me most is that I did not understand that KeePass2Android simply did not encrypt the database during a quick lock.

I do not know all the hack techniques but there are much more chances of accessing memory areas of an unencrypted database to recover passwords even if the user only has a chance by the user interface (the database is potentially always open with this technique.). It is not just a question of probability. From a theoretical point of view, if it's possible to access the memory areas (RAM) of the app (with decrypted database), it is won for the hacker. No even need to go through a decipher.

I don't know if everyone has that in mind. Perhaps I was the only one who did not understand the technical functioning of the first request and that you really want to rapid use on a database that is always open but visually inaccessible? This is a real question because it is not my vision but many people ask me the functionality of K2A. So maybe I'm wrong somewhere?

shadow00 commented 4 years ago

Holy shit, I thought K2A actually encrypted the database when it was locked with QuickUnlock, not just hidden away from user access! If I had known that was how K2A actually did this, I would have never used that feature.

Perhaps I was the only one who did not understand the technical functioning of the first request and that you really want to rapid use on a database that is always open but visually inaccessible?

No, that is definitely not something I had in mind either. I assumed it was actually encrypted, and that it used some method like the one I suggested to quickly unlock the db without asking for the full password. In terms of "observable behaviour" it looks the same to the user, but my proposal actually requires encrypting the database while it's locked with QuickUnlock - not just hiding access away from the user interface.

Personally, this operation does not satisfy me, because ultimately there are the same theoretical problems as what I describe in this conversation. For me, the most secure solution is still to use the idea of ​​the salted password in the KeyStore.

100% agree. Please implement the method we've been discussing here asap - I really need a new password manager D:

Sunnyji commented 4 years ago

My point was when a person is very tired and sleeping, his friends can make use of his finger for first unlocking the phone and then the keepass database. In this respect quick unlock with PIN or last X characters of password is more secure than biometric unlock. By the way, is key or password saved by KeePassDX somewhere, for decrypting database using Biometric Authentication?

J-Jamet commented 4 years ago

My point was when a person is very tired and sleeping, his friends can make use of his finger for first unlocking the phone and then the keepass database. In this respect quick unlock with PIN or last X characters of password is more secure than biometric unlock.

@Sunnyji In this case, you want a programmed obsolescence of the biometric key. I plan to implement this feature soon. It's related to https://github.com/Kunzisoft/KeePassDX/issues/566

By the way, is key or password saved by KeePassDX somewhere, for decrypting database using Biometric Authentication?

The master password fingerprint is encrypted in the KeyStore with a biometric key (https://github.com/Kunzisoft/KeePassDX/blob/master/app/src/main/java/com/kunzisoft/keepass/biometric/BiometricUnlockDatabaseHelper.kt).

J-Jamet commented 3 years ago

I am currently developing the feature that uses the phone's credentials to unlock the database. This way there will be no more problems for those who do not want to use biometric recognition and it will be as secure as using the fingerprint (with the same keystore method internally). The traditional quick unlock with an open database will clearly not be implemented (for all the reasons mentioned above). But this new method will allow you to go faster when opening a database (will be available from Android 11).

rugk commented 3 years ago

Awesome! :tada: For me this solves this issue here. :+1:

shadow00 commented 3 years ago

Will there be an option to follow the same authentication flow as we discussed here? (First unlock with password only, then store the password in the keystore to allow quick unlocking, and remove any old entries if/when the app service gets killed by the system or after a failed authentication attempt)

Or will this be a "permanent" method once I enable it, so anyone who knows my system pin/password/pattern could unlock my db at any time without ever knowing my db password?

J-Jamet commented 3 years ago

I reused the biometric workflow as it is suitable for device identification methods. And I took a good look, there is normally no need to use a service, the Keystore is managed well enough by the system to invalidate keys based on specific criteria. (and nothing prevents us from doing a new service afterwards)

Or will this be a "permanent" method once I enable it, so anyone who knows my system pin/password/pattern could unlock my db at any time without ever knowing my db password?

A setting will allow the keys to be invalidates after a set time for biometric and device credentials. #566 A setting will allow the key to be deleted if a bad biometric / credential is attempted. #548 ~A setting will allow the keys to be deleted after a device shutdown. #437~

There will be no more worries at this level and will be configurable according to needs.

To stop using the advanced unlock, just delete the key manually. (It was the crossed out fingerprint icon, I replaced it with a crossed out key). You must validate the advanced unlocking when you want to reuse the feature after entering your password. Which is not a problem because it is quick.

Edit: After careful analysis, it appears that new Android devices apply restrictions in "receiver" for device optimization that will not allow keys to be deleted in many cases. And also to satisfy users who do not want to store encrypted content, a new method using a foreground service can be applied.

shadow00 commented 3 years ago

Woohoo!! This is awesome! Can't wait to try it when it's released!