July 2019 - openldap-bugs

Re: (ITS#9055) contrib/slapd-modules/passwd/totp improvements

by gv＠members.scinet.supercomputing.org

On Fri, Jul 19, 2019 at 02:30:48PM -0400, Greg Veldman wrote: > I'll submit an updated patch shortly to fix > this, as well as some documentation updates for issues pointed > out. I have generated an updated version of the patch, with the following changes from the original: - Avoid the side effects of short-circuit && - Point out the need for one initial authentication in the current window to verify clocks before the previous window logic kicks in - Document as a bug the lack of hash functions for the new schemes Updated patch: https://scinet.supercomputing.org/~gv/slapd-totp-v2.txt -- Greg Veldman

4 years, 2 months

1
0
0 / 0

Re: (ITS#9055) contrib/slapd-modules/passwd/totp improvements

by gv＠members.scinet.supercomputing.org

On Fri, Jul 19, 2019 at 07:21:35PM +0200, Ond??ej Kuzn??k wrote: > > if (chk_totp(&passwd_otp, &cred_otp, mech, text) == LUTIL_PASSWD_OK > > && lutil_passwd(&passwd_pass, &cred_pass, NULL, text) > > == LUTIL_PASSWD_OK) > > rc = LUTIL_PASSWD_OK; > > This only checks the password if OTP check passed, right? So if checking > the password takes a measurable amount of time, an attacker can see if > they hit the right OTP token without it being voided. Ah, yes, sorry I didn't quite catch what you were getting at previously there. I'll submit an updated patch shortly to fix this, as well as some documentation updates for issues pointed out. -- Greg Veldman

4 years, 2 months

1
0
0 / 0

Re: (ITS#9055) contrib/slapd-modules/passwd/totp improvements

by ondra＠mistotebe.net

On Fri, Jul 19, 2019 at 04:34:44PM +0000, gv(a)members.scinet.supercomputing.org wrote: > On Fri, Jul 19, 2019 at 05:58:37PM +0200, Ond??ej Kuzn??k wrote: >> BTW, why did you choose to check that the user has already logged in >> rather than "(told == 0 || t - 1 > told)"? > > There are two circumstances where a user may transmit a code > from the previous time window: > > 1. The time window rolls over as the code is being typed in > or transmitted over the network. > > 2. The clock on the user's prover is off. > > I made this condition because on the very first attempt at TOTP > auth, I wanted to be more strict about accepting expired codes > to ensure that the clock on the user's prover is in fact in sync > with the server. Since this implementation has no mechanism to > adjust for clock skew, if both clocks weren't in sync that would > cause many issues for the user down the road, and the possibility > of a false negative on the first attempt seemed a small price to > pay to ensure they were. OK, probably worth mentioning then. >> It might be better to check both the token and password independently >> and then return success iff both of them succeeded to avoid timing leaks >> of OTP success/failure? > > I do. The issue Quanah raised is in relation to the change > in chk_totp(). This is only called directly for the {TOTP1}, > {TOTP256}, and {TOTP512} schemes (e.g. TOTP only and no password). > If a password is in play, the checks use the new chk_totp_and_pw() > function, which does validate each piece independently: > > if (chk_totp(&passwd_otp, &cred_otp, mech, text) == LUTIL_PASSWD_OK > && lutil_passwd(&passwd_pass, &cred_pass, NULL, text) > == LUTIL_PASSWD_OK) > rc = LUTIL_PASSWD_OK; This only checks the password if OTP check passed, right? So if checking the password takes a measurable amount of time, an attacker can see if they hit the right OTP token without it being voided. Regards, -- Ondřej Kuzník Senior Software Engineer Symas Corporation http://www.symas.com Packaged, certified, and supported LDAP solutions powered by OpenLDAP

4 years, 2 months

1
0
0 / 0

Re: (ITS#7326) additional notes

by quanah＠symas.com

The related glibc bugs are: <https://sourceware.org/bugzilla/show_bug.cgi?id=12398> <https://sourceware.org/bugzilla/show_bug.cgi?id=12377> RedHat patched around this as noted in <https://bugzilla.redhat.com/show_bug.cgi?id=505105> Other useful information in <https://bugzilla.redhat.com/show_bug.cgi?id=699576> --Quanah -- Quanah Gibson-Mount Product Architect Symas Corporation Packaged, certified, and supported LDAP solutions powered by OpenLDAP: <http://www.symas.com>

4 years, 2 months

1
0
0 / 0

Re: (ITS#9055) contrib/slapd-modules/passwd/totp improvements

by gv＠members.scinet.supercomputing.org

On Fri, Jul 19, 2019 at 05:58:37PM +0200, Ond??ej Kuzn??k wrote: > BTW, why did you choose to check that the user has already logged in > rather than "(told == 0 || t - 1 > told)"? There are two circumstances where a user may transmit a code from the previous time window: 1. The time window rolls over as the code is being typed in or transmitted over the network. 2. The clock on the user's prover is off. I made this condition because on the very first attempt at TOTP auth, I wanted to be more strict about accepting expired codes to ensure that the clock on the user's prover is in fact in sync with the server. Since this implementation has no mechanism to adjust for clock skew, if both clocks weren't in sync that would cause many issues for the user down the road, and the possibility of a false negative on the first attempt seemed a small price to pay to ensure they were. > It might be better to check both the token and password independently > and then return success iff both of them succeeded to avoid timing leaks > of OTP success/failure? I do. The issue Quanah raised is in relation to the change in chk_totp(). This is only called directly for the {TOTP1}, {TOTP256}, and {TOTP512} schemes (e.g. TOTP only and no password). If a password is in play, the checks use the new chk_totp_and_pw() function, which does validate each piece independently: if (chk_totp(&passwd_otp, &cred_otp, mech, text) == LUTIL_PASSWD_OK && lutil_passwd(&passwd_pass, &cred_pass, NULL, text) == LUTIL_PASSWD_OK) rc = LUTIL_PASSWD_OK; -- Greg Veldman

4 years, 2 months

1
0
0 / 0

Re: (ITS#7326) [PATCH] use AI_ADDRCONFIG if defined in the environment

by quanah＠symas.com

This patch is rejected as it requires RedHat's custom version of glibc that has been patched to address issues like: <https://sourceware.org/bugzilla/show_bug.cgi?id=12398> <https://sourceware.org/bugzilla/show_bug.cgi?id=12377> In the future, I would ask that RedHat not submit patches that depend on custom RedHat implementations of core libraries. Thanks. Regards, Quanah -- Quanah Gibson-Mount Product Architect Symas Corporation Packaged, certified, and supported LDAP solutions powered by OpenLDAP: <http://www.symas.com>

4 years, 2 months

1
0
0 / 0

Re: (ITS#9055) contrib/slapd-modules/passwd/totp improvements

by ondra＠mistotebe.net

On Fri, Jul 19, 2019 at 11:45:02AM +0000, gv(a)members.scinet.supercomputing.org wrote: > On Thu, Jul 18, 2019 at 05:53:54PM -0400, Greg Veldman wrote: >> Would a default-off ifdef to activate that code block work for >> this? I did intentionally keep that part of the change self >> contained, so it wouldn't be hard to add that... > > Actually, re-reading RFC 6238 this morning (section 5.2, > especially paragraph 1), I believe this part of the proposed > change is both compliant and in accordance with recommended > practice in its current form. Yes, it makes sense to check the previous interval in case you entered the token just at the end of its validity even if you don't choose to test them in a broader window for resynchronisation (section 6). BTW, why did you choose to check that the user has already logged in rather than "(told == 0 || t - 1 > told)"? It might be better to check both the token and password independently and then return success iff both of them succeeded to avoid timing leaks of OTP success/failure? Sounds like a powerful oracle with PBKDF2 and similar hashes if you don't aggressively lock out users. Also probably needs to track the last successful OTP token somewhere else than the lastbind attribute even if the overall bind failed to mitigate the opposite attack. Also maybe use op->o_time instead of time(0L). Regards, -- Ondřej Kuzník Senior Software Engineer Symas Corporation http://www.symas.com Packaged, certified, and supported LDAP solutions powered by OpenLDAP

4 years, 2 months

1
0
0 / 0

Re: (ITS#9055) contrib/slapd-modules/passwd/totp improvements

by gv＠members.scinet.supercomputing.org

On Thu, Jul 18, 2019 at 05:53:54PM -0400, Greg Veldman wrote: > Would a default-off ifdef to activate that code block work for > this? I did intentionally keep that part of the change self > contained, so it wouldn't be hard to add that... Actually, re-reading RFC 6238 this morning (section 5.2, especially paragraph 1), I believe this part of the proposed change is both compliant and in accordance with recommended practice in its current form. -- Greg Veldman

4 years, 2 months

1
0
0 / 0

Re: (ITS#9055) contrib/slapd-modules/passwd/totp improvements

by gv＠members.scinet.supercomputing.org

On Thu, Jul 18, 2019 at 01:37:11PM -0700, Quanah Gibson-Mount wrote: > This should be a configuration item that is an integer value of the number > of seconds to allow outside of the timeslice, with 0 meaning only the > default time slice is allowed. Allowing people to authenticate outside of > the time slice is of course a security issue and should not be allowed by > default (So the default value of the parameter should be 0). I don't disagree, but by that logic so should the actual size of the time window, the number of digits, etc. I saw a lot of these parameters were hard-coded in this module and proceeded in kind. I wasn't really trying to recreate the full functionality of the OpenLDAP Gold implementation[1]. Would a default-off ifdef to activate that code block work for this? I did intentionally keep that part of the change self contained, so it wouldn't be hard to add that... [1] https://symas.com/two-factor-authentication-everywhere -- Greg Veldman IT Infrastructure Services, Purdue University gv(a)purdue.edu | (765)-496-2456

4 years, 2 months

1
0
0 / 0

Re: (ITS#9055) contrib/slapd-modules/passwd/totp improvements

by gv＠members.scinet.supercomputing.org

On Thu, Jul 18, 2019 at 08:32:22PM +0100, Howard Chu wrote: > it doesn't support setting > the password using the PasswordModify exop. That seems to imply that users are > required to generate their passwords using some other tool, and set them using a > normal Modify op, but doing so is deprecated. That is correct. > Password changes should only be done > using the PasswordModify exop. I wasn't sure how to do this. My implementation is essentially two password checks in one, so I need a way to tell the data apart. For the check functions this was easy as the OTP key is stored base32 encoded, and the DELIM character should not appear in a base32 string. For actual password input, I don't think I can make that assumption. I think the safest way to hash this scheme would be to have two separate input prompts, but I didn't see in the existing code a provision for this. It's possible I wasn't looking in the right places. If you have any suggestions I'd be happy to try writing that code. -- Greg Veldman IT Infrastructure Services, Purdue University gv(a)purdue.edu | (765)-496-2456

4 years, 2 months

1
0
0 / 0

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

2012

2011

2010

2009

2008

2007

2006

openldap-bugs July 2019