Anti bruteforce
Stefano Gargiulo
Peter Schober
> Does anyone implemented a module to avoid bruteforce login attemps
> into SS IdP?
Since a SAML IdP usually is not the only thing that talks to your
actual authentication service (e.g. LDAP DSAs) it's probably best to
implement such a thing there. Otherwise you'd still be vulnerable to
attempts coming in via IMAP, SMTPA, HTTPS, etc (or you'd be requried
to implement these limits in each of those applications).
For the case of SimpleSAMLphp being the authentication source itself
(by way of the selfregister module) there is currently no code to
limit authentication requests. Not sure it's worth the effort though,
for essentially no-trust self-service userids ;)
-peter
Stefano Gargiulo
Since a SAML IdP usually is not the only thing that talks to your
actual authentication service (e.g. LDAP DSAs) it's probably best to
implement such a thing there
Otherwise you'd still be vulnerable to
attempts coming in via IMAP, SMTPA, HTTPS, etc (or you'd be requried
to implement these limits in each of those applications).
--
You received this message because you are subscribed to the Google Groups "simpleSAMLphp" group.
To post to this group, send email to simple...@googlegroups.com.
To unsubscribe from this group, send email to simplesamlph...@googlegroups.com.
For more options, visit this group at http://groups.google.com/group/simplesamlphp?hl=en.
Richard Gomes
I hope my answer is in context, in spite the solution does not involve
programming.
Once a failure will possibly redirect to an specific page which tells
"login failure" or something like this, you can easily detect these
situations and block the offending IP for a certain amount of time.
I've written a couple of articles which show how you can protect Apache
and Tomcat against unpleasant, unwanted, undesired spambots using
"fail2ban" on Debian boxes.
* http://www.jquantlib.org/index.php/LazyAdminApacheFail2Ban
*
http://www.jquantlib.org/index.php/Protecting_Tomcat_with_Fail2Ban_on_Debian
I hope it helps.
Cheers :)
--
Richard Gomes
http://www.jquantlib.org/index.php/User:RichardGomes
twitter: frgomes
JQuantLib is a library for Quantitative Finance written in Java.
http://www.jquantlib.org/
twitter: jquantlib
Stefano Gargiulo
Stefano Gargiulo
Stefano Gargiulo
Thijs Kinkhorst
On Friday 28 January 2011 16:40:15 Stefano Gargiulo wrote:
> this is my implementation, thanks to the robustness and flexibility of the
> simplesamlphp framework was very easy to inculde it, make it configurable,
> and show graceful errors to users (also an alert when only 1 attempt is
> left)
I'm not a simpleSAMLphp developer but I like the idea of this; Peter's
suggestion to implement it in the backend is a good idea to some extent but
it does not allow to limit attemts from a username,IP combination which I
think is the useful thing about your patch.
Still, here's some feedback on it:
> here the plain diff text for who wanna implement it
>
> Index: GarrSSO/usr/local/simplesamlphp-1.7.0/config/config.php
> ===================================================================
> --- GarrSSO/usr/local/simplesamlphp-1.7.0/config/config.php (revision 83)
> +++ GarrSSO/usr/local/simplesamlphp-1.7.0/config/config.php (revision 87)
> @@ -22,6 +22,12 @@
> * SimpleSAMLphp will attempt to create this directory if it doesn't
> exist. */
> 'tempdir' => '/tmp/simplesamlphp',
> +
> + /** anti bruteforce for password cracking prevention **/
> + "security.antibruteforce.timewindow.minuites" => 10,
Probably you mean "minutes".
> + "security.antibruteforce.timewindow.max_attempts" => 5,
> + "security.antibruteforce.logfile" =>
> "/tmp-www/simplesaml.antibruteforce.data",
Perhaps /tmp-www is a secured directory where only simpleSAMLphp can write.
But if the user changes that to /tmp you've created a tempfilerace. Probably
best to make this logfile location relative to simpleSAMLphp's loggingdir or
datadir setting.
> + "security.antibruteforce.username_check" => false,
The options could use some documentatoin. The ones earlier may speak a bit for
themselves but you really need to read the code to understand what this one
does.
> + $deny_login = false;
> +
> + if (!file_exists(MM_BRUTE_FILE))
> + touch(MM_BRUTE_FILE);
> + $cache = unserialize(self::fileToString(MM_BRUTE_FILE));
> + if (!$cache)
> + $cache = array();
> +
> + if ($failed_attempt) { //register the new failed attempt and
> timestamp
> + if (!isset($cache[$username.$_SERVER['REMOTE_ADDR']])) {
I'll readily admit that it's unlikely, but by just contatenating them you can
get the situation that attempts from 'user' on '12.34.56.78' are conflated
with attempts from 'user1' on '2.34.56.78'. Perhaps for correctness sake it
helps to glue them together with a character that doesn't appear in
IP-addresses, e.g.: $username.'_'.$_SERVER['REMOTE_ADDR'] (and create that
string once at the start of the function, and use it in a variable in all the
places throughout the code).
> +//get the number of failed attempts in the last 15 minutes
> + if (!isset($cache[$username.$_SERVER['REMOTE_ADDR']])) {
> + $deny_login = false;
> +
> + } else {
> + $attempts = $cache[$username.$_SERVER['REMOTE_ADDR']];
> + if (count($attempts) < MM_BRUTE_ATTEMPTS) {
> + $deny_login = false;
> + } else {
> + if ($attempts[0] + MM_BRUTE_WINDOW > time())
> + $deny_login = true;
> + else
> + $deny_login = false;
> + }
> + }
$deny_login is already set to false at the start of the function, so you may
skip all the conditions where you set it to false again here (and only drill
down to the part where it's true).
> + public static function fileToString($filename) {
> + return file_get_contents($filename);
> + }
This just calls file_get_contents, why can't that function be used directly?
> + public static function stringToFile($filename, $data) {
> + $file = fopen($filename, "w");
> + fwrite($file, $data);
> + fclose($file);
> + return true;
> + }
This seems to implement PHP builtin file_put_contents(), so why not use that
instead?
One last thing is that I'm wondering if the writeout of the cache could be
problematic on really busy installations, causing a race condition. But
perhaps its small enough not to be a problem in practice.
Cheers,
Thijs
--
Thijs Kinkhorst <th...@uvt.nl> – LIS Unix
Universiteit van Tilburg – Library and IT Services • Postbus 90153, 5000 LE
Bezoekadres > Warandelaan 2 • Tel. 013 466 3035 • G 236 • http://www.uvt.nl
Olav Morken
> This seems to implement PHP builtin file_put_contents(), so why not use that
> instead?
>
> One last thing is that I'm wondering if the writeout of the cache could be
> problematic on really busy installations, causing a race condition. But
> perhaps its small enough not to be a problem in practice.
If I understand the code correctly, it is only a single file for all
users & IPs, and that file is rewritten every time a username/password
combination fails. Combine that with a server under load (for example
because someone is trying lots of username/password combinations at the
same time), and you are almost guaranteed to hit this race condition.
What is worse is that the file write isn't atomic, so you can expect
some of the reads to read a truncated file, which in turn causes the
counters to be reset.
We have an function for atomically writing a file that can be used:
SimpleSAML_Utilities::writeFile($filename, $data);
It writes the data to a temporary file, and then renames that over the
existing file. Some data will still be lost, when we have multiple
simultaneous requests, but at least some data should survive.
An alternative is to implement this on top of memcache. Memcache has
an "increment" function, which allows you to atomically increase an
counter.
Regards,
Olav Morken
UNINETT / Feide
Olav Morken
> this is my implementation, thanks to the robustness and flexibility of the
> simplesamlphp framework was very easy to inculde it, make it configurable,
> and show graceful errors to users (also an alert when only 1 attempt is
> left)
Generally, I think that this could be better implemented as a wrapper
around an existing authentication source, e.g. configured like:
'mylogin' => array(
'antibruteforce:File',
"timewindow.minuites" => 10,
"timewindow.max_attempts" => 5,
"logfile" => "/tmp-www/simplesaml.antibruteforce.data",
"username_check" => false,
'auth' => array(
'ldap:LDAP',
[...],
),
),
That would require some small modifications to SimpleSAML_Auth_Source
to allow one authentication source to "wrap" a different one, but
should be doable - at least for all that are based on UserPassBase.
It would also make it easier to implement custom backends for
validation.
> + define('MM_BRUTE_FILE',
> SimpleSAML_Configuration::getConfig()->getString("security.antibruteforce.logfile"));
> + define('MM_BRUTE_WINDOW',
> SimpleSAML_Configuration::getConfig()->getInteger("security.antibruteforce.timewindow.minuites")
> * 60);
> + define('MM_BRUTE_ATTEMPTS',
> SimpleSAML_Configuration::getConfig()->getInteger("security.antibruteforce.timewindow.max_attempts")
> );
These define's could be replaced with normal variables?
> +
> + $deny_login = false;
> +
> + if (!file_exists(MM_BRUTE_FILE))
> + touch(MM_BRUTE_FILE);
This will only cause unserialization of an empty string - you might
as well just initialize $cache to an empty array.
> + $cache = unserialize(self::fileToString(MM_BRUTE_FILE));
> + if (!$cache)
> + $cache = array();
Stefano Gargiulo
Hi thank you for the replies. Yes the class i posted is a very ugly code, and not mine (i used a sample i found on the blog post i linked before on this thread, sorry but i was in an hurry i know that is to be reviewed to fit simplesamlphp framework, that was just to give inspiration). Anyway in my installation i will revwrite it maybe using an something like SQLlite database to prevent concurrency.
Ps. The idea of authsource wrapper sounds very good to me, maybe also useful to implement other interesting things (is the complement to the authprocfilter feature: a filtrer that can be executed before authentication) so, my 50 cents.
Best regards,
Stefano.
Il giorno 31/gen/2011 13:40, "Olav Morken" <olav....@uninett.no> ha scritto:
On Fri, Jan 28, 2011 at 16:40:15 +0100, Stefano Gargiulo wrote:
> this is my implementation, thanks ...
Generally, I think that this could be better implemented as a wrapper
around an existing authentication source, e.g. configured like:
'mylogin' => array(
'antibruteforce:File',
"timewindow.minuites" => 10,
"timewindow.max_attempts" => 5,
"logfile" => "/tmp-www/simplesaml.antibruteforce.data",
"username_check" => false,
'auth' => array(
'ldap:LDAP',
[...],
),
),
That would require some small modifications to SimpleSAML_Auth_Source
to allow one authentication source to "wrap" a different one, but
should be doable - at least for all that are based on UserPassBase.
It would also make it easier to implement custom backends for
validation.
> + define('MM_BRUTE_FILE',
> SimpleSAML_Configuration::getConfig()->getString("security.an...
These define's could be replaced with normal variables?
> +
> + $deny_login = false;
> +
> + if (!file_exists(MM_BRUTE_FILE))
> + ...
This will only cause unserialization of an empty string - you might
as well just initialize $cache to an empty array.
> + $cache = unserialize(self::fileToString(MM_BRUTE_FILE));
> + if (!$cache)
> + ...
Dyonisius Visser
> authentication) so, my 50 cents.
I think the saying is "my 2 cents".
You might have been listen too much to hiphip music ;-)
--
Dyonisius Visser
System & Networking Engineer
TERENA Secretariat
Singel 468 D, 1017 AW Amsterdam
The Netherlands
T +31 20 530 44 88 F +31 20 530 44 99
vis...@terena.org | www.terena.org