The following may or may not be intended operation. It is merely an observation.
KeePass 2.53
Windows 10 Pro 21H2 64-bit
Intel i5-3380M
When choosing database security settings, there is useful Test button for benchmarking the current settings. There is also a Cancel button that appears along the bottom button area as needed to interrupt the process.
Pressing Cancel does bring the KeePass window back into active focus and makes it responsive to input, but it doesn't actually stop the benchmark that has been started; cancelling merely causes the KeePass window to stop waiting for the benchmark to return a result. This appears to occur for all KDF options.
While the prior benchmark runs to completion, the user is able to start a new benchmark using the same or different settings. The benchmarks all occur in parallel and it's unclear if there's any limit on how many can be started. I successfully tested 3 parallel operations at once, as indicated by memory being committed to each test for the requested duration. KeePass became less responsive for a short time after starting the 3rd benchmark, so I was reluctant to add more.
This would not be noticed with reasonable KDF settings that benchmark rapidly, but could cause confusion or system unresponsiveness if users attempt to benchmark multiple long KDF sequences simultaneously. This may simply be one of those "dumb" things that Dominik never expected the user to do ;).
Perhaps the benchmark/test thread cannot be easily interrupted, but I wanted to report this behavior in case it was unintended. It would seem best if the running benchmark could actually be stopped or that a second test could not be started until the first one had ended, both for safety and to avoid the inaccurate test results that will occur when a prior test is still silently running. However, there may be factors involved that I'm unaware of.
It's possible that this behavior could freeze or crash KeePass or cause system memory exhaustion and CPU saturation, but I don't know if this is a relevant problem or not as it would seem to require self-sabotage and is unlikely to occur during normal usage.
Thanks for your time and many years of awesome work! KeePass is a masterpiece, in my opinion.
Last edit: RobAllenB 2023-01-11
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
Thanks for reporting this! There indeed is room for improvement (but a proper solution for this unfortunately isn't easy to implement). I've added it to my (huge) to-do list for a future version.
Best regards,
Dominik
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
a proper solution for this unfortunately isn't easy to implement
I was afraid of that ;). I was hoping that an "easy" solution might be to not allow a second test to begin until the first had completed, even if the completion and results aren't reported to the user.
Thanks for the reply and for everything that you do. Many of us really appreciate your decades of work on both 1.x and 2.x and I hope that you feel appreciated.
I'm currently having numerous conversations with people now interested in KeePass after the sometimes-overblown reaction to recent LastPass breaches and behavior, so you may see increased activity or increased feature requests from newcomers :(.
I often encourage these users to donate if they find the software useful, as that appears to be an Achilles Heel of open source projects; people equate "open source" with "free" and that's simply not true; it costs time and money to develop, support, and distribute any software project, but doesn't cost money to use. I'm preaching to the choir here, but existing KeePass users might politely encourage new users to support whichever open source password manager that they settle upon in the wake of LastPass.
Last edit: RobAllenB 2023-01-13
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
Disabling the 'Test' button until a previous test has completed would be possible, but this would probably confuse some users. Unfortunately, terminating/exiting the KDF computation threads isn't simple. There are 6 KDF implementations (AES-KDF managed, AES-KDF native Windows, AES-KDF native Unix, Argon2 managed, Argon2 native Windows, Argon2 native Unix) and each of them creates threads. Identifying these threads is difficult, and letting the main thread terminate native computation threads could also be problematic (memory leaks, ...). One solution would be to implement a way how the main thread can signal the computation threads to exit (e.g. an event). The official Argon2 implementation (which KeePass uses) doesn't support aborting a computation though. On Windows, I'm compiling the Argon2 code myself, so I could add support for an event here (I wouldn't really like this though), but on Unix-like systems, KeePass uses 'libargon2.so', where I cannot add it...
However, I had a different idea and have implemented it now. When clicking the 'Test' button, KeePass now spawns a child process that performs the KDF test computation. Clicking the 'Cancel Operation' button terminates the child process (the operating system terminates all threads of it).
After the KDF computation finished, KeePass displays a success message that mentions the time that it took. Here, KeePass measures only the time that the KDF computation takes; the time for spawning the child process isn't included.
If no child process can be started (e.g. blocked by some security software), KeePass automatically falls back to computing the KDF in the main process.
The new approach is used only for the 'Test' button, not for any other KDF computations.
The '1 Second Delay' button uses a strategy with reasonable parameters, terminating within a few seconds anyway.
In the case of KDF computations for opening/saving a database, I prefer to not use the new approach for security reasons. KeePass would need to securely transfer a cryptographic key (derived from the master key) to/from the child process. Although this may be possible, there would be a risk of introducing security vulnerabilities, and I don't think that the rather minor benefit in this case justifies the risk. For the 'Test' button, this is not a problem, because KDF test computations (and the '1 Second Delay' button) use dummy keys, not the actual master key.
I've checked carefully that terminating the child process prematurely is safe (it doesn't corrupt the configuration file, etc.).
The new approach works only for the built-in KDFs (currently AES-KDF, Argon2d and Argon2id), not for KDFs provided by plugins (because the child process doesn't load plugins; in general, plugins are loaded during the construction of the main window, and the child process doesn't construct a main window). In the case of a KDF provided by a plugin, KeePass computes it in the main process, like before. I'm not aware of any plugin that provides a KDF though, so this is probably irrelevant right now anyway.
Thanks and best regards,
Dominik
PS: I'm glad that you like KeePass; thanks for spreading the word and suggesting to donate :-)
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
Sorry for the delayed response. I tested the build and the Cancel button does quickly end the computation thread and I found no apparent issues with it. If there's any chance that this approach could cause corruption, instability, or a security risk then you should probably leave the behavior unchanged, but your proposed strategy seems to work fine for me in Win10.
This has obviously never a problem for anyone before and might never be, but I did find it interesting that the user can start many ongoing tests at once, though doing so would be their own fault. Choose whichever behavior is best for the overall integrity of KeePass and thanks again for the wonderful program and for the KDBX format now used by many others.
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
Although you're the first one who reported this issue, I think many users encountered it. The way it is implemented now, there probably won't be any problems; it's a nice improvement :-)
Best regards,
Dominik
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
The following may or may not be intended operation. It is merely an observation.
KeePass 2.53
Windows 10 Pro 21H2 64-bit
Intel i5-3380M
When choosing database security settings, there is useful Test button for benchmarking the current settings. There is also a Cancel button that appears along the bottom button area as needed to interrupt the process.
Pressing Cancel does bring the KeePass window back into active focus and makes it responsive to input, but it doesn't actually stop the benchmark that has been started; cancelling merely causes the KeePass window to stop waiting for the benchmark to return a result. This appears to occur for all KDF options.
While the prior benchmark runs to completion, the user is able to start a new benchmark using the same or different settings. The benchmarks all occur in parallel and it's unclear if there's any limit on how many can be started. I successfully tested 3 parallel operations at once, as indicated by memory being committed to each test for the requested duration. KeePass became less responsive for a short time after starting the 3rd benchmark, so I was reluctant to add more.
This would not be noticed with reasonable KDF settings that benchmark rapidly, but could cause confusion or system unresponsiveness if users attempt to benchmark multiple long KDF sequences simultaneously. This may simply be one of those "dumb" things that Dominik never expected the user to do ;).
Perhaps the benchmark/test thread cannot be easily interrupted, but I wanted to report this behavior in case it was unintended. It would seem best if the running benchmark could actually be stopped or that a second test could not be started until the first one had ended, both for safety and to avoid the inaccurate test results that will occur when a prior test is still silently running. However, there may be factors involved that I'm unaware of.
It's possible that this behavior could freeze or crash KeePass or cause system memory exhaustion and CPU saturation, but I don't know if this is a relevant problem or not as it would seem to require self-sabotage and is unlikely to occur during normal usage.
Thanks for your time and many years of awesome work! KeePass is a masterpiece, in my opinion.
Last edit: RobAllenB 2023-01-11
Thanks for reporting this! There indeed is room for improvement (but a proper solution for this unfortunately isn't easy to implement). I've added it to my (huge) to-do list for a future version.
Best regards,
Dominik
I was afraid of that ;). I was hoping that an "easy" solution might be to not allow a second test to begin until the first had completed, even if the completion and results aren't reported to the user.
Thanks for the reply and for everything that you do. Many of us really appreciate your decades of work on both 1.x and 2.x and I hope that you feel appreciated.
I'm currently having numerous conversations with people now interested in KeePass after the sometimes-overblown reaction to recent LastPass breaches and behavior, so you may see increased activity or increased feature requests from newcomers :(.
I often encourage these users to donate if they find the software useful, as that appears to be an Achilles Heel of open source projects; people equate "open source" with "free" and that's simply not true; it costs time and money to develop, support, and distribute any software project, but doesn't cost money to use. I'm preaching to the choir here, but existing KeePass users might politely encourage new users to support whichever open source password manager that they settle upon in the wake of LastPass.
Last edit: RobAllenB 2023-01-13
Disabling the 'Test' button until a previous test has completed would be possible, but this would probably confuse some users. Unfortunately, terminating/exiting the KDF computation threads isn't simple. There are 6 KDF implementations (AES-KDF managed, AES-KDF native Windows, AES-KDF native Unix, Argon2 managed, Argon2 native Windows, Argon2 native Unix) and each of them creates threads. Identifying these threads is difficult, and letting the main thread terminate native computation threads could also be problematic (memory leaks, ...). One solution would be to implement a way how the main thread can signal the computation threads to exit (e.g. an event). The official Argon2 implementation (which KeePass uses) doesn't support aborting a computation though. On Windows, I'm compiling the Argon2 code myself, so I could add support for an event here (I wouldn't really like this though), but on Unix-like systems, KeePass uses 'libargon2.so', where I cannot add it...
However, I had a different idea and have implemented it now. When clicking the 'Test' button, KeePass now spawns a child process that performs the KDF test computation. Clicking the 'Cancel Operation' button terminates the child process (the operating system terminates all threads of it).
Here's the latest development snapshot for testing:
https://keepass.info/filepool/KeePass_230116.zip
Notes:
Thanks and best regards,
Dominik
PS: I'm glad that you like KeePass; thanks for spreading the word and suggesting to donate :-)
Sorry for the delayed response. I tested the build and the Cancel button does quickly end the computation thread and I found no apparent issues with it. If there's any chance that this approach could cause corruption, instability, or a security risk then you should probably leave the behavior unchanged, but your proposed strategy seems to work fine for me in Win10.
This has obviously never a problem for anyone before and might never be, but I did find it interesting that the user can start many ongoing tests at once, though doing so would be their own fault. Choose whichever behavior is best for the overall integrity of KeePass and thanks again for the wonderful program and for the KDBX format now used by many others.
Great, thanks for testing it!
Although you're the first one who reported this issue, I think many users encountered it. The way it is implemented now, there probably won't be any problems; it's a nice improvement :-)
Best regards,
Dominik