[PATCH] Add offline failed login counter

[Sumit Bose]

Hi,

this patch adds a counter for failed login attempts when offline. This
should fix https://fedorahosted.org/sssd/ticket/78.

bye,
Sumit

[Stephen Gallagher]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 01/25/2010 11:51 AM, Sumit Bose wrote:
>  Hi,
>  
>  this patch adds a counter for failed login attempts when offline. This
>  should fix https://fedorahosted.org/sssd/ticket/78.
>  
>  bye,
>  Sumit
>   
We discussed on IRC that we should really have an additional option
"failed_login_delay". If we receive "failed_login_count" tries, the
login delay should kick in and prevent logins for n minutes. If set to
zero, we lock the user out completely.

This is probably a better solution than the current one, which always
locks the user out. I'd like to avoid the inevitable situation where
someone is locked out while out of the country. Performing an online
auth in this situation may be infeasible.

I also think that the failed_login_count+delay should be available for
online auths as well, to prevent brute-force attacks against the server.

- -- 
Stephen Gallagher
RHCE 804006346421761

Delivering value year after year.
Red Hat ranks #1 in value among software vendors.
http://www.redhat.com/promo/vendor/
> -----BEGIN PGP SIGNATURE-----
> Version: GnuPG v1.4.10 (GNU/Linux)
> Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org/
>
> iEYEARECAAYFAktfK7EACgkQeiVVYja6o6PzAwCfQNYlPYbHg1lYZW6Vq5d28QMO
> GA0AnRyysjsLTuy7CKItBolKoZD1e+v+
> =QctR
> -----END PGP SIGNATURE-----

[Sumit Bose]

On Tue, Jan 26, 2010 at 12:51:45PM -0500, Stephen Gallagher wrote:
>  -----BEGIN PGP SIGNED MESSAGE-----
>  Hash: SHA1
>  
>  On 01/25/2010 11:51 AM, Sumit Bose wrote:
>  > Hi,
>  > 
>  > this patch adds a counter for failed login attempts when offline. This
>  > should fix https://fedorahosted.org/sssd/ticket/78.
>  > 
>  > bye,
>  > Sumit
>  > 
>  
>  We discussed on IRC that we should really have an additional option
>  "failed_login_delay". If we receive "failed_login_count" tries, the
>  login delay should kick in and prevent logins for n minutes. If set to
>  zero, we lock the user out completely.
>  
>  This is probably a better solution than the current one, which always
>  locks the user out. I'd like to avoid the inevitable situation where
>  someone is locked out while out of the country. Performing an online
>  auth in this situation may be infeasible.
>  
>  I also think that the failed_login_count+delay should be available for
>  online auths as well, to prevent brute-force attacks against the server.
>  
>  - -- 
>  Stephen Gallagher
>  RHCE 804006346421761
>   
This new version of the patch adds the option offline_failed_login_delay. I
think we should this scheme for the local domain, too, but with a
separate set of config options (failed_login_count and
failed_login_delay in the domain section).

I'm not sure about the general online case because we might interfere
with policies on the server.

bye,
Sumit

[Stephen Gallagher]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 01/27/2010 11:48 AM, Sumit Bose wrote:
>  This new version of the patch adds the option
> offline_failed_login_delay. I
>  think we should this scheme for the local domain, too, but with a
>  separate set of config options (failed_login_count and
>  failed_login_delay in the domain section).
>  
>  I'm not sure about the general online case because we might interfere
>  with policies on the server. 
Nack. Please specify in the SSSDConfig API that the time is in minutes.

I think it would be more correct to return EIO if we fail to get the
login delay or allowed attempts from confdb. EACCESS implies that we
explicitly failed.

The DEBUG messages after failure to save SYSDB_LAST_LOGIN and
SYSDB_FAILED_LOGIN_ATTEMPTS are wrong.

I'm not sure if this was intentional or not: if I'm reading this
correctly, we're still saving the last failed login time every time it
fails, even if we're already in the 5-minute waiting period. So,
effectively, it's going to deny forever unless they wait five minutes.

I think we probably want to stop storing the last failed login time once
we hit our failed login counter, otherwise things might be frustrating
for the user.

On the other hand, this is a great way to defeat brute-force attacks, so
I'm not sure how we want to proceed on this.

Also, would it be possible to return a message to the user that they've
passed the maximum attempts and that they need to wait five minutes?
(I'm thinking something similar to your "Warn the user if authentication
happens offline" patch.
- -- 
Stephen Gallagher
RHCE 804006346421761

Delivering value year after year.
Red Hat ranks #1 in value among software vendors.
http://www.redhat.com/promo/vendor/
> -----BEGIN PGP SIGNATURE-----
> Version: GnuPG v1.4.10 (GNU/Linux)
> Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org/
>
> iEYEARECAAYFAkth6u8ACgkQeiVVYja6o6NC/QCfavL52lGlMjmggte/NeK8onPv
> zZwAmgJHhaKywKvltDZh/wmtt49aDmB4
> =RtX3
> -----END PGP SIGNATURE-----

[Sumit Bose]

On Thu, Jan 28, 2010 at 02:52:21PM -0500, Stephen Gallagher wrote:
>  -----BEGIN PGP SIGNED MESSAGE-----
>  Hash: SHA1
>  
>  On 01/27/2010 11:48 AM, Sumit Bose wrote:
>  > This new version of the patch adds the option offline_failed_login_delay. I
>  > think we should this scheme for the local domain, too, but with a
>  > separate set of config options (failed_login_count and
>  > failed_login_delay in the domain section).
>  > 
>  > I'm not sure about the general online case because we might interfere
>  > with policies on the server.
>  
>  Nack. Please specify in the SSSDConfig API that the time is in minutes. 
fixed

>  
>  I think it would be more correct to return EIO if we fail to get the
>  login delay or allowed attempts from confdb. EACCESS implies that we
>  explicitly failed. 
fixed

>  
>  The DEBUG messages after failure to save SYSDB_LAST_LOGIN and
>  SYSDB_FAILED_LOGIN_ATTEMPTS are wrong. 
fixed

>  
>  I'm not sure if this was intentional or not: if I'm reading this
>  correctly, we're still saving the last failed login time every time it
>  fails, even if we're already in the 5-minute waiting period. So,
>  effectively, it's going to deny forever unless they wait five minutes. 
no, you are wrong, if check_failed_login_attempts() fails or denies
access nothing is changed or updated.

>  
>  I think we probably want to stop storing the last failed login time once
>  we hit our failed login counter, otherwise things might be frustrating
>  for the user.
>  
>  On the other hand, this is a great way to defeat brute-force attacks, so
>  I'm not sure how we want to proceed on this. 
I think this would be an unexpected behaviour and it might be a bit
annoying to a user who didn't look too closely on his watch and retried
after 4 minutes and 55 seconds.

>  
>  Also, would it be possible to return a message to the user that they've
>  passed the maximum attempts and that they need to wait five minutes?
>  (I'm thinking something similar to your "Warn the user if authentication
>  happens offline" patch. 
This would be possible, but wouldn't this disclose too much information
to an attacker?

bye,
Sumit

>  - -- 
>  Stephen Gallagher
>  RHCE 804006346421761
>  
>  Delivering value year after year.
>  Red Hat ranks #1 in value among software vendors.
>  http://www.redhat.com/promo/vendor/
> >  -----BEGIN PGP SIGNATURE-----
> >  Version: GnuPG v1.4.10 (GNU/Linux)
> >  Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org/
> >  
> >  iEYEARECAAYFAkth6u8ACgkQeiVVYja6o6NC/QCfavL52lGlMjmggte/NeK8onPv
> >  zZwAmgJHhaKywKvltDZh/wmtt49aDmB4
> >  =RtX3
> >  -----END PGP SIGNATURE----- 
>  _______________________________________________
>  sssd-devel mailing list
>  sssd-devel(a)lists.fedorahosted.org
>  https://fedorahosted.org/mailman/listinfo/sssd-devel

[Stephen Gallagher]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Nack

On 01/29/2010 05:51 AM, Sumit Bose wrote:
>  On Thu, Jan 28, 2010 at 02:52:21PM -0500, Stephen Gallagher wrote:
>  
>  Nack. Please specify in the SSSDConfig API that the time is in minutes.
>  
> > fixed
>  
>  
>  I think it would be more correct to return EIO if we fail to get the
>  login delay or allowed attempts from confdb. EACCESS implies that we
>  explicitly failed.
>  
> > fixed
>  
>  
>  The DEBUG messages after failure to save SYSDB_LAST_LOGIN and
>  SYSDB_FAILED_LOGIN_ATTEMPTS are wrong.
>  
> > fixed 
No it's not:
+        ret = sysdb_attrs_add_time_t(state->update_attrs, SYSDB_LAST_LOGIN,
+                                     time(NULL));
+        if (ret != EOK) {
+            DEBUG(3, ("sysdb_attrs_add_long failed, "
+                      "but authentication is successful.\n"));
+            ret = EOK;
+            goto done;
+        }

And several other similar ones.

>  
>  I'm not sure if this was intentional or not: if I'm reading this
>  correctly, we're still saving the last failed login time every time it
>  fails, even if we're already in the 5-minute waiting period. So,
>  effectively, it's going to deny forever unless they wait five minutes.
>  
> > no, you are wrong, if check_failed_login_attempts() fails or denies
> > access nothing is changed or updated. 
Yeah, I see it now. No problem.
>  
>  
>  I think we probably want to stop storing the last failed login time once
>  we hit our failed login counter, otherwise things might be frustrating
>  for the user.
>  
>  On the other hand, this is a great way to defeat brute-force attacks, so
>  I'm not sure how we want to proceed on this.
>  
> > I think this would be an unexpected behaviour and it might be a bit
> > annoying to a user who didn't look too closely on his watch and retried
> > after 4 minutes and 55 seconds.
>   
Agreed.

>  
>  Also, would it be possible to return a message to the user that they've
>  passed the maximum attempts and that they need to wait five minutes?
>  (I'm thinking something similar to your "Warn the user if authentication
>  happens offline" patch.
>  
> > This would be possible, but wouldn't this disclose too much information
> > to an attacker?
>   

I'm not sure this is harmful information to disclose. The attacker now
knows that the previous passwords didn't work, and that they now need to
wait five minute more to try the next set of passwords that probably
won't work.

If anything, I think it would be better, because it might discourage the
attacker from hammering the site and wasting bandwidth and resources.

I'll open an RFE for this and we can discuss it separately. This is out
of scope for this patch.

- -- 
Stephen Gallagher
RHCE 804006346421761

Delivering value year after year.
Red Hat ranks #1 in value among software vendors.
http://www.redhat.com/promo/vendor/
> -----BEGIN PGP SIGNATURE-----
> Version: GnuPG v1.4.10 (GNU/Linux)
> Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org/
>
> iEYEARECAAYFAktm1yUACgkQeiVVYja6o6NF0QCdGUl45qiO4nN7EEnWcwsheDqd
> g/sAoK8fnCwAUf7mHYV6v4/LZLOjnfWP
> =nk9f
> -----END PGP SIGNATURE-----

[Sumit Bose]

On Mon, Feb 01, 2010 at 08:29:09AM -0500, Stephen Gallagher wrote:
>  -----BEGIN PGP SIGNED MESSAGE-----
>  Hash: SHA1
>  
>  Nack
>  
>  On 01/29/2010 05:51 AM, Sumit Bose wrote:
>  > On Thu, Jan 28, 2010 at 02:52:21PM -0500, Stephen Gallagher wrote:
>  > The DEBUG messages after failure to save SYSDB_LAST_LOGIN and
>  > SYSDB_FAILED_LOGIN_ATTEMPTS are wrong.
>  > 
>  >> fixed
>  
>  No it's not:
>  +        ret = sysdb_attrs_add_time_t(state->update_attrs, SYSDB_LAST_LOGIN,
>  +                                     time(NULL));
>  +        if (ret != EOK) {
>  +            DEBUG(3, ("sysdb_attrs_add_long failed, "
>  +                      "but authentication is successful.\n"));
>  +            ret = EOK;
>  +            goto done;
>  +        }
>  
>  And several other similar ones.
>   
oh, sorry, I thought you've meant a different DEBUG message. I hope I've
found all of them now.

bye,
Sumit

[Stephen Gallagher]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 02/01/2010 09:02 AM, Sumit Bose wrote:
>  
>  oh, sorry, I thought you've meant a different DEBUG message. I hope I've
>  found all of them now.
>  
>  bye,
>  Sumit
>  
>   
Ack.

- -- 
Stephen Gallagher
RHCE 804006346421761

Delivering value year after year.
Red Hat ranks #1 in value among software vendors.
http://www.redhat.com/promo/vendor/
> -----BEGIN PGP SIGNATURE-----
> Version: GnuPG v1.4.10 (GNU/Linux)
> Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org/
>
> iEYEARECAAYFAktm4K0ACgkQeiVVYja6o6NY7QCgomXTGdlA0XcWUYljLaEf9xQq
> n4cAniri3/+6wMg5e7S4VEsTg5zqI+1O
> =UIew
> -----END PGP SIGNATURE-----

[Stephen Gallagher]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 02/01/2010 09:09 AM, Stephen Gallagher wrote:
>  On 02/01/2010 09:02 AM, Sumit Bose wrote:
>  
> > oh, sorry, I thought you've meant a different DEBUG message. I hope I've
> > found all of them now.
>  
> > bye,
> > Sumit
>  
>  
>  
>  Ack.
>   
Pushed to master.

- -- 
Stephen Gallagher
RHCE 804006346421761

Delivering value year after year.
Red Hat ranks #1 in value among software vendors.
http://www.redhat.com/promo/vendor/
> -----BEGIN PGP SIGNATURE-----
> Version: GnuPG v1.4.10 (GNU/Linux)
> Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org/
>
> iEYEARECAAYFAktogHAACgkQeiVVYja6o6NEcgCfZXAbulj6778QS9JvE5PoD7vx
> 6rwAoI2zGYJYTfww/DoTs7VyAhV/oB7n
> =YMdU
> -----END PGP SIGNATURE-----