> On Aug. 10, 2017, 2:26 p.m., Robert Levas wrote: > > Would it have been possble to add the lockout logic in > > `org.apache.ambari.server.security.authentication.AmbariAuthenticationEventHandlerImpl#onSuccessfulAuthentication`? > > I am not sure if chaning the _success_ to a _failure_ is possible here, > > though. > > > > If the code stays as is, does the consective failure count increase when > > the user fails to authenticate becuase the failure count was passed the > > threshhold? If so, do we care? > > Attila Magyar wrote: > First I wanted to put it there, but I didn't find a way to move to the > failure case from there. > > The counter will keep increasing after it passed the threashold. This can > lead to a possible overflow after ~2 billion failures, which will eventually > reset the counter. So maybe we shouldn't increase it beyond a certain limit. > I can add overflow detection to UserEntity>incrementConsecutiveFailures and > maximize the counter to Integer.MAX_VALUE. Or do you have a better idea? > > Robert Levas wrote: > Can we check the AmbariAuthenticationException value and not increment if > the cause is/contains a TooManyLoginFailuresException? > > Attila Magyar wrote: > The number of consecutive failures is shown on the UI, wouldn't it be > confusing to display a number that doesn't reflect the real number of > failures?
Ideally, the login faulure does not indicate that the user is locked out. The error message should be the same as any other authentication failure. We don't want to give away any information about the user account in the even someone is trying to brute force their way in. If we are telling the user that they have failed to authenticate X number of times, then constantly incremeneting the failure count is preferred - no matter if the user was locked out or not. Therefore the current implementation is correct.. however maybe a little note somewhere in the code would be nice to have to make sure no one "fixes" the issue potentially allowing a hacker to know they have successfully authenticated to a locked account. - Robert ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/61501/#review182616 ----------------------------------------------------------- On Aug. 11, 2017, 3:32 a.m., Attila Magyar wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/61501/ > ----------------------------------------------------------- > > (Updated Aug. 11, 2017, 3:32 a.m.) > > > Review request for Ambari, Balázs Bence Sári, Laszlo Puskas, Robert Levas, > and Sebastian Toader. > > > Bugs: AMBARI-21680 > https://issues.apache.org/jira/browse/AMBARI-21680 > > > Repository: ambari > > > Description > ------- > > Prevent users from authenticating if they exceed a configured number of login > failures, which is set as a configuration in the ambari.properties file - > authentication.max.failures. > > After a users successfully authenticates, check the value of > org.apache.ambari.server.orm.entities.UserEntity#getConsecutiveFailures. > > If it exceeds the value set in authentication.max.failures, then fail > authentication. Else allow authentication to proceed. > If failing authentication due to being "locked out", do not indicate this to > the user; however an Ambari server log message will be useful. > The normal "authentication failed" message should be returned as to not give > away any information about a user's authentication. If a special "locked out" > message is shown, then a hacker will be able to attempt a brute force attack > on a user's account since the returned error message will be different if > they eventually succeed in guessing the password. > > To "unlock" the user, a user administrator (a user with the > AMBARI.MANAGE_USERS authorization) needs to reset the user's consecutive > failure count to 0. > By default the authentication.max.failures should be 10; however 0 should > indicate that no lockout is desired. > Options > > > Diffs > ----- > > > ambari-admin/src/main/resources/ui/admin-web/app/scripts/controllers/users/UsersShowCtrl.js > 200872e > ambari-admin/src/main/resources/ui/admin-web/app/scripts/i18n.config.js > 43b32da > ambari-admin/src/main/resources/ui/admin-web/app/scripts/services/User.js > ac50653 > ambari-admin/src/main/resources/ui/admin-web/app/views/users/show.html > f965c5d > ambari-server/docs/configuration/index.md 9dbe9c4 > > ambari-server/src/main/java/org/apache/ambari/server/configuration/Configuration.java > 4f787c6 > > ambari-server/src/main/java/org/apache/ambari/server/controller/AmbariServer.java > b52e2b1 > > ambari-server/src/main/java/org/apache/ambari/server/controller/UserRequest.java > d0836a9 > > ambari-server/src/main/java/org/apache/ambari/server/controller/internal/UserResourceProvider.java > a2d9917 > > ambari-server/src/main/java/org/apache/ambari/server/security/authentication/AmbariAuthenticationEventHandlerImpl.java > 3a5a66b > > ambari-server/src/main/java/org/apache/ambari/server/security/authentication/TooManyLoginFailuresException.java > PRE-CREATION > > ambari-server/src/main/java/org/apache/ambari/server/security/authorization/AmbariLocalUserProvider.java > 2c8bf12 > > ambari-server/src/test/java/org/apache/ambari/server/security/authorization/AmbariLocalUserProviderTest.java > 133fc9f > > > Diff: https://reviews.apache.org/r/61501/diff/3/ > > > Testing > ------- > > 1 > - tried to log in 10 times with an incorrect password > - tried to log in with the correct password and received authentication > failure > - reseted the counter > - logged in successfully with the correct password > > 2 > - tried to log in 3 times with incorrect password > - checked that UI showed Login failures=3 > - logged in successfully with the correct password > - checked the UI showed Login failures=0 > > 3 > - added authentication.max.failures=3 to ambari.properties > - repeated the first test > > 4 > - added authentication.max.failures=0 to ambari.properties > - tried to log in 11 times with an incorrect password > - successfully logged in with the correct password > > Results : > Tests run: 4841, Failures: 0, Errors: 0, Skipped: 33 > ---------------------------------------------------------------------- > Total run:1145 > Total errors:0 > Total failures:0 > OK > Ran 470 tests in 108.068s > > > Thanks, > > Attila Magyar > >
