-
Notifications
You must be signed in to change notification settings - Fork 345
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Mitigate bruteforce attempts #708
base: develop
Are you sure you want to change the base?
Conversation
Add "login.fail_delay" config option
Add delay on failed attempt to mitigate bruteforce
I guess these checks are broken x) The project should build perfectly, and has been tested on Debian 11. Also to add on my initial post : I think it doesn't cost much to merge this PR, and it can help raise the security level of SoftHSM in some use-cases (not all cases, I hear you). |
@@ -51,6 +51,7 @@ const struct config Configuration::valid_config[] = { | |||
{ "slots.removable", CONFIG_TYPE_BOOL }, | |||
{ "slots.mechanisms", CONFIG_TYPE_STRING }, | |||
{ "library.reset_on_fork", CONFIG_TYPE_BOOL }, | |||
{ "login.fail_delay", CONFIG_TYPE_INT }, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd recommend to add a default in softhsm2.conf.in
and softhsm2.conf.5.in
to document this new feature as well, e.g. somewhere here:
SoftHSMv2/src/lib/common/softhsm2.conf.in
Line 17 in a8cabf5
library.reset_on_fork = false |
.SH LIBRARY.RESET_ON_FORK |
Also to indicate which units (e.g. seconds or milliseconds) these are supposed to be set to.
@@ -263,6 +270,7 @@ bool SecureDataManager::login(const ByteString& passphrase, const ByteString& en | |||
|
|||
if (!RFC4880::PBEDeriveKey(passphrase, salt, &pbeKey)) | |||
{ | |||
sleep(Configuration::i()->getInt("login.fail_delay", 0)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
On Windows I would expect this to be a capitalized Sleep
(https://learn.microsoft.com/en-us/windows/win32/api/synchapi/nf-synchapi-sleep) which takes in milliseconds, instead of the POSIX sleep
which takes seconds, so I'm not sure how this could work for both platforms.
(I guess a utility function elsewhere would help.)
Please rebase on develop and mark as ready when ready. |
Hi,
I have seen past issues about this topic, which have all been disregarded because "if an attacker has access to the SoftHSM library, he could also access the filesystem and loot the token file, to try and bruteforce locally".
However, as a QubesOS user, I use extensively SoftHSM in combination with socat, so that my SoftHSM server is on a virtual machine with all my certificates (a vault VM), and my clients (for instance a Firefox process) are all in other VMs, with no direct access to the vault's file system. In such cases, I feel it makes the most sense to implement a kind of rate limiting in SoftHSM as it would drastically reduce the bruteforce pace of an attacker in a client VM. The way I implemented it is very trivial, feel free to make it cleaner or to tell me what to do in order to make it so.
By default, when no configuration option is added to the config file, it does not change the behavior of SoftHSM (no delay on failed attempts). However, if you add the
login.fail_delay
(here again, the name might not be in line with the configuration options naming scheme, feel free to tell me about it) in your config file, the value of the option you provided (in seconds) will be waited on a failed attempt.Feel free to reach out if you have any question, or to make any modification to the code of this PR.
Best regards,
Antoine