You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by bu...@apache.org on 2019/04/10 16:24:27 UTC

[Bug 63336] New: Currently there is no way to know in form error page that the user was not authenticated because it was locked out

https://bz.apache.org/bugzilla/show_bug.cgi?id=63336

            Bug ID: 63336
           Summary: Currently there is no way to know in form error page
                    that the user was not authenticated because it was
                    locked out
           Product: Tomcat 8
           Version: 8.5.x-trunk
          Hardware: PC
            Status: NEW
          Severity: normal
          Priority: P2
         Component: Catalina
          Assignee: dev@tomcat.apache.org
          Reporter: jchobantonov@yahoo.com
  Target Milestone: ----

If a user is locked out from LockOutRealm or if there are some specific
exceptions in backend user realms like user is locked in the backend, user is
required to change it's password first etc (see JAAS exception like
AccountExpiredException, AccountLockedException, AccountNotFoundException or
CredentialExpiredException - in case the password is valid but because it was
requested that the user should change it's password because forgot password has
been requested)

So we need some way to inform the user of the web app that the account has been
locked up in the login error page instead of just saying the username/password
is invalid as it is confusing and users are going to request forgot password
flow which will change their password and they are going to still not be able
to login if LockOutRealm has triggered lockout for 5 min.

What I'm suggesting is to provide custom configurable HttpServletRequest
attribute for example "login.error.message" of type String that describes why
the user was not able to login along with the exception itself so that we could
pass additional information into the exception itself in an attribute
"login.error.exception" (again configurable request attribute name in
server.xml as it is not standard - please do not use standard servlet error
message and error attributes as some frameworks will clear those attributes and
the login error page will not be able to get the correct message/exception)

Because LockOutRealm do not have the HttpServletRequest passed into the user
realms we need to have a Valve that will put the HttpServletRequest/Response
into thread local variable so that user realms/JAAS modules could obtain the
HttpServletRequest and inject the user attribute to be used by the login error
page

Note that currently basic authenticator will report 401 error but it will not
put into the body the reason why it was rejected so it could be a good thing to
refactor that as well and if request have the attribute to pull the value and
when sending 401 Http error from basic authentication to also put the error
message in the response body.

Here is an example that I'm using for LockOutRealm in order to report to the
user that the account is locked up and not that the username/password is
incorrect and having the user wonder what's wrong:

import java.security.Principal;
import java.security.cert.X509Certificate;

import javax.security.jacc.PolicyContext;
import javax.security.jacc.PolicyContextException;
import javax.servlet.http.HttpServletRequest;

import org.apache.catalina.realm.LockOutRealm;
import org.apache.juli.logging.Log;
import org.apache.juli.logging.LogFactory;
import org.ietf.jgss.GSSContext;
import org.ietf.jgss.GSSException;
import org.ietf.jgss.GSSName;

public class ExtendedLockOutRealm extends LockOutRealm {

    public static final String REQUEST_ATTRIBUTE_LOGIN_ERROR_MESSAGE =
"login.error.message"; 

    public static final String REQUEST_ATTRIBUTE_LOCKOUT_MESSAGE =
ExtendedLockOutRealm.class.getName() + ".REQUEST_ATTRIBUTE_LOCKOUT_MESSAGE"; 
    public static final String REQUEST_ATTRIBUTE_LOCKOUT_USERNAME =
ExtendedLockOutRealm.class.getName() + ".REQUEST_ATTRIBUTE_LOCKOUT_USERNAME"; 
    public static final String REQUEST_ATTRIBUTE_LOCKOUT_TIME =
ExtendedLockOutRealm.class.getName() + ".REQUEST_ATTRIBUTE_LOCKOUT_TIME"; 

    private static final Log log = LogFactory.getLog(LockOutRealm.class);

    @Override
    public Principal authenticate(String username, String clientDigest,
            String nonce, String nc, String cnonce, String qop,
            String realmName, String md5a2) {
        if (isLocked(username)) {
            processLockOutUser(username, null);
            return null;
        }
        Principal principal = super.authenticate(username, clientDigest, nonce,
nc, cnonce, qop, realmName, md5a2);
        processLockOutUser(username, principal);
        return principal;
    }

    @Override
    public Principal authenticate(String username, String credentials) {
        if (isLocked(username)) {
            processLockOutUser(username, null);
            return null;
        }
        Principal principal = super.authenticate(username, credentials);
        processLockOutUser(username, principal);
        return principal;
    }

    @Override
    public Principal authenticate(X509Certificate[] certs) {
        String username = null;
        if (certs != null && certs.length >0) {
            username = certs[0].getSubjectDN().getName();
        }
        if (isLocked(username)) {
            processLockOutUser(username, null);
            return null;
        }

        Principal principal = super.authenticate(certs);
        processLockOutUser(username, principal);
        return principal;
    }

    @Override
    public Principal authenticate(GSSContext gssContext, boolean storeCreds) {
        String username = null;
        if (gssContext.isEstablished()) {
            GSSName name = null;
            try {
                name = gssContext.getSrcName();
            } catch (GSSException e) {
                log.warn(sm.getString("realmBase.gssNameFail"), e);
                return null;
            }

            username = name.toString();

            if (isLocked(username)) {
                processLockOutUser(username, null);
                return null;
            }
        }
        Principal principal = super.authenticate(gssContext, storeCreds);
        processLockOutUser(username, principal);
        return principal;
    }

    private void processLockOutUser(String username, Principal principal) {
        if (principal == null && username != null && isLocked(username)) {

            log.warn(sm.getString("lockOutRealm.authLockedUser", username));

            if
(PolicyContext.getHandlerKeys().contains("javax.servlet.http.HttpServletRequest"))
{
                try {
                    Object object =
PolicyContext.getContext("javax.servlet.http.HttpServletRequest");
                    if (object instanceof HttpServletRequest) {
                        HttpServletRequest httpServletRequest =
(HttpServletRequest) object;
                        int lockoutMin = lockOutTime / 60;
                        String message = "Account has been locked for " +
(lockoutMin > 0 ? (lockoutMin + (lockoutMin > 1 ? " minutes" : " minute")) :
(lockOutTime + (lockOutTime > 1 ? " seconds" : " second")));

                       
httpServletRequest.setAttribute(REQUEST_ATTRIBUTE_LOGIN_ERROR_MESSAGE,
message);

                       
httpServletRequest.setAttribute(REQUEST_ATTRIBUTE_LOCKOUT_MESSAGE, message);
                       
httpServletRequest.setAttribute(REQUEST_ATTRIBUTE_LOCKOUT_USERNAME, username);
                       
httpServletRequest.setAttribute(REQUEST_ATTRIBUTE_LOCKOUT_TIME, lockOutTime);
                    }
                } catch (PolicyContextException ignored) {
                }
            }
        }
    }
}


the override authenticate methods are because I want if the user is locked out
to return immediately and not invoke the inner user realms 
basically adding the check:
if (isLocked(username)) {
}
before the actual super.authenticate method

I'm using javax.security.jacc.PolicyContext in order to store http
request/response so that security classes be able to obtain them


Here is the valve to register the request and reponse with PolicyContext:

import java.io.IOException;

import javax.security.jacc.PolicyContext;
import javax.security.jacc.PolicyContextException;
import javax.servlet.ServletException;

import org.apache.catalina.LifecycleException;
import org.apache.catalina.connector.Request;
import org.apache.catalina.connector.Response;
import org.apache.catalina.valves.ValveBase;

public class JACCPolicyContextValve extends ValveBase {

    private static final String ALREADY_FILTERED_ATTRIBUTE =
JACCPolicyContextValve.class.getName() + ".FILTERED";

    private boolean debug = false;

    public void setDebug(final Boolean debug) {
        if (debug != null) {
            this.debug = debug;
        }
    }

    @Override
    protected void initInternal() throws LifecycleException {
        if (debug) {
            System.out.println("[JACCPolicyContextValve] initInternal
invoked.");
        }
        super.initInternal();
        if
(!PolicyContext.getHandlerKeys().contains(HttpServletRequestPolicyContextHandler.KEY))
{
            try {
               
PolicyContext.registerHandler(HttpServletRequestPolicyContextHandler.KEY, new
HttpServletRequestPolicyContextHandler(), false);
                if (debug) {
                    System.out.println("[JACCPolicyContextValve] Register " +
HttpServletRequestPolicyContextHandler.class.getName() + " handler for key " +
HttpServletRequestPolicyContextHandler.KEY);
                }
            } catch (PolicyContextException ex) {
                if (debug) {
                    ex.printStackTrace();
                }
            }
        } else {
            if (debug) {
                System.out.println("[JACCPolicyContextValve] PolicyContext
already have " + HttpServletRequestPolicyContextHandler.KEY + " handler key.");
            }
        }
        if
(!PolicyContext.getHandlerKeys().contains(HttpServletResponsePolicyContextHandler.KEY))
{
            try {
               
PolicyContext.registerHandler(HttpServletResponsePolicyContextHandler.KEY, new
HttpServletResponsePolicyContextHandler(), false);
                if (debug) {
                    System.out.println("[JACCPolicyContextValve] Register " +
HttpServletResponsePolicyContextHandler.class.getName() + " handler for key " +
HttpServletResponsePolicyContextHandler.KEY);
                }
            } catch (PolicyContextException ex) {
                if (debug) {
                    ex.printStackTrace();
                }
            }
        } else {
            if (debug) {
                System.out.println("[JACCPolicyContextValve] PolicyContext
already have " + HttpServletResponsePolicyContextHandler.KEY + " handler
key.");
            }
        }
    }

    @Override
    public void invoke(Request request, Response response) throws IOException,
ServletException {
        if (request.getAttribute(ALREADY_FILTERED_ATTRIBUTE) == null) {
            request.setAttribute(ALREADY_FILTERED_ATTRIBUTE, Boolean.TRUE);
            try {
                if (debug) {
                    System.out.println("[JACCPolicyContextValve] Set request of
class: " + request.getClass().getName());
                }
                SecurityContextAssociationHandler.activeRequest.set(request);
                if (debug) {
                    System.out.println("[JACCPolicyContextValve] Set response
of class: " + response.getClass().getName());
                }
                SecurityContextAssociationHandler.activeResponse.set(response);
                getNext().invoke(request, response);
            } finally {
                if (debug) {
                    System.out.println("[JACCPolicyContextValve] Unset
request.");
                }
                SecurityContextAssociationHandler.activeRequest.set(null);
                if (debug) {
                    System.out.println("[JACCPolicyContextValve] Unset
response.");
                }
                SecurityContextAssociationHandler.activeResponse.set(null);
            }
        } else {
            getNext().invoke(request, response);
        }
    }
}


Now having request obtained from thread local will allow JAAS modules to put
information why it did not login so that even when the JAASRealm catches the
exceptions thrown from JAAS modules and just return null for the Principal to
still know why it failed.

Having catalina's standard way to obtain the request in security related
structure that do not have http request passed as parameter will allow security
providers to put security related information that is needed to inform the user
as to why he/she can't login

This issue is related to 
https://bz.apache.org/bugzilla/show_bug.cgi?id=63334

-- 
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


[Bug 63336] Currently there is no way to know in form error page that the user was not authenticated because it was locked out

Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=63336

Mark Thomas <ma...@apache.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 OS|                            |All
         Resolution|---                         |WONTFIX
             Status|NEW                         |RESOLVED

--- Comment #1 from Mark Thomas <ma...@apache.org> ---
This has been discussed previously and will not be implemented in Tomcat since
informing an attacker that an account has been locked is a (minor) security
vulnerability.

Users are free to extend Tomcat to provide this functionality in their apps if
they wish.

Requests to modify Tomcat to make this sort of extension easier are likely to
be looked on favourably - especially if patches are provided.

-- 
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


[Bug 63336] Currently there is no way to know in form error page that the user was not authenticated because it was locked out

Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=63336

--- Comment #3 from Mark Thomas <ma...@apache.org> ---
See this thread in the archives:
http://tomcat.markmail.org/thread/4garqvcph2ci3j5m

The isLocked() method of the Realm was made public and exposed via JMX to
support this sort of custom feature. unlock() is also available.

-- 
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


[Bug 63336] Currently there is no way to know in form error page that the user was not authenticated because it was locked out

Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=63336

--- Comment #4 from jchobantonov@yahoo.com ---
Thank you for pointing out that isLocked() and unlock() methods are public - I
already know that. Even with this information I need to provide custom
LockOutRealm in order to see the real reason why my form error page is been
displayed

And again I know I could provide Valves, extensions etc to fix that and I
already did that for my application but I thought someone else could benefit
from this as well 

As you are the one that provide LockOutRealm you could just add some extra
information (either using request.setAttribute()) or some other ways so that in
web application's form error page (jsp or whatever) you could be able to tell
the user if there is a lockout or not if you choose to do so.

I'm not aware how from we application error login page I could obtain the realm
in order to ask isLocked() and the error page do not get the login user id to
pass it to - even if this was possible it will make web application not
portable across different app servers and we don't want that - we want to have
a way to get this information preferably using the request attribute - then if
the attribute is not standard (as there is no such thing in the servlet spec)
we could use servlet init parameters to pass what is the name of the request
attribute to look for and then our web applications will be more portable

The issue is that right now unless I supply tomcat specific classes and
configuration I can't develop application to tell me that

And also just an example - amazon cognito also is using lockout if you have too
many error login attempts and it will tell you that straight up - so a failed
login could result because cognito also is in lockout - so I need to query
JAASRealm somehow to see what is the condition there - too much things to check
- it would be better if everyone participate - LockOutRealm, JAASRealm, JAAS
modules to tell you why something happens - not to mention that you might want
to have some ability in the error login page to log the reason why something
happens in one place (probably some audit framework - not just a log message
from LockOutRealm) - as there a many places why something happens -
LockOutRealm, JAASRealm, modules, ldap realm etc it is better whoever report
the error to put it in some standard place for us to consume

-- 
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


[Bug 63336] Currently there is no way to know in form error page that the user was not authenticated because it was locked out

Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=63336

--- Comment #2 from jchobantonov@yahoo.com ---
Ok, forget about modifying the basic ream to report the error - the application
could have 401 error page and put that information itself - again the request
is to add http request attribute so that error page of the application could
expose that to the end user if they choose to - I’m not asking tomcat to report
the security issue as I’m well aware of the security concerns there but there
is a real business use case that all applications could benefit - I’m asking
for the support - it is still up to the application whether to expose this to
the end user or not as it is only set in request attribute that is not going to
be transmitted to the client unless the application do something about it

-- 
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org