Bug 62036

Summary: Roles stripped when using programmatic login() in tomcat 8.5 but not 8.0
Product: Tomcat 8 Reporter: Konstantin Kolinko <knst.kolinko>
Component: CatalinaAssignee: Tomcat Developers Mailing List <dev>
Status: RESOLVED FIXED    
Severity: normal    
Priority: P2    
Version: 8.5.27   
Target Milestone: ----   
Hardware: PC   
OS: All   
Attachments: test.war
catalina_2018-01-23_log.txt - Cumulative log file, Tomcat 8.5.27

Description Konstantin Kolinko 2018-01-23 17:11:29 UTC
Filing a Bugzilla entry for an issue reported by Robert J. Carr on the users@ list. See
http://markmail.org/message/rfm2qejzgcd2uwmh

I can confirm that the issue is reproducible in the current Tomcat 8.5 and 9.0.

Steps to reproduce:

[quote]
To reproduce the problem in tomcat 8.5.24 (for me):

 1)  make a user available with the role "testrole" (I just user
tomcat-users)

 2) startup tomcat, copy the war file into webapps

 3) go to the application homepage, index.jsp should auto load

 4) enter username and password and login; it should change to the username
you're authenticated with

 5) hit the auth test link and it should give you a success message

 6) hit the same link again and it should give you a 403

If you want to see how things are changing, I created an unprotected page
called /authinfo (no jsp) that shows the logged in user and role.  Here's
what it shows as you proceed through the test:

 * no user or role
 * user and role
 * user, but no role

If you do this same process in tomcat 8 (8.0.43, for me) it works fine,
particularly, the you can hit the link as many times as you want and the
roles never go away until you logout.  And generally, the login/test/logout
works perfectly, where in 8.5 even if you logout it doesn't always log you
back in the next time either.  Sometimes its takes several attempts.
[/quote]
Comment 1 Konstantin Kolinko 2018-01-23 17:17:56 UTC
Created attachment 35694 [details]
test.war

Test application.

It differs from original:
I corrected the values of xmlns and xsi:schemaLocation attributes on the root <web-app> element to have the correct values for a Servlet 3.1 application.

(I usually run with org.apache.catalina.STRICT_SERVLET_COMPLIANCE=true which enables validation of web.xml, thus those typos became noticable).
Comment 2 Konstantin Kolinko 2018-01-23 17:25:43 UTC
Created attachment 35695 [details]
catalina_2018-01-23_log.txt - Cumulative log file, Tomcat 8.5.27

To enable logging I add the following lines to conf/logging.properties:

org.apache.catalina.authenticator.level=FINE
org.apache.catalina.realm.level=FINE

I am attaching an excerpt from catalina.2018-01-23.log and access log when running on Tomcat 8.5.27.
Comment 3 Konstantin Kolinko 2018-01-23 17:32:34 UTC
(In reply to Konstantin Kolinko from comment #2)
> Created attachment 35695 [details]

The first /test/authtest call at 19:25:27.037 has the following lines:

[[[
- AuthenticatorBase.invoke We have cached auth type NONE for principal GenericPrincipal[testuser(testrole,)]

- RealmBase.hasResourcePermission   Checking roles GenericPrincipal[testuser(testrole,)]
- RealmBase.hasRole Username [testuser] has role [testrole]
]]]

The second /test/authtest call at 19:25:35.225 has the following lines:
[[[
- AuthenticatorBase.invoke We have cached auth type NONE for principal User username="testuser", roles="testrole"

- RealmBase.hasResourcePermission   Checking roles User username="testuser", roles="testrole"
- RealmBase.hasRole Username [testuser] does NOT have role [testrole]
]]]

Note the following:
1). The principal differs.

The first call operates on a GenericPrincipal.
The second call operates on a MemoryUser.

2). It is also odd that RealmBase.hasRole() check for a MemoryUser fails.
Comment 4 Konstantin Kolinko 2018-01-23 17:37:57 UTC
(In reply to Konstantin Kolinko from comment #3)
> Note the following:
> 1). The principal differs.
> 
> The first call operates on a GenericPrincipal.
> The second call operates on a MemoryUser.

Debugging current Tomcat 9, the change of the cached principal occurs in NonLoginAuthenticator.doAuthenticate(), with the following call stack:

[[[
at StandardSession.setPrincipal(Principal) - StandardSession:614	
at NonLoginAuthenticator.doAuthenticate(Request, HttpServletResponse) - NonLoginAuthenticator:86	
at NonLoginAuthenticator.invoke(Request, Response) - AuthenticatorBase:584
at StandardHostValve.invoke(Request, Response) - StandardHostValve:140	
]]]
Comment 5 Konstantin Kolinko 2018-01-23 17:51:31 UTC
(In reply to Konstantin Kolinko from comment #3)
> 2). It is also odd that RealmBase.hasRole() check for a MemoryUser fails.

Debugging Tomcat 9:

The following methods in RealmBase are called:
(the actual object is LockOutRealm)

1. hasResourcePermission(request, response, SecurityConstraint []constraints, context)
2. hasRole((Wrapper) null, principal, (String) "testrole")
3. hasRoleInternal(principal, (String) "testrole")

On successful call the 'principal' is GenericPrincipal.

On unsuccessful call the 'principal' is MemoryUser and RealmBase.hasRoleInternal() fails because of the following lines:

[[[
 // Should be overridden in JAASRealm - to avoid pretty inefficient conversions
 if (!(principal instanceof GenericPrincipal)) {
        return false;
 }
]]]
Comment 6 Mark Thomas 2018-01-24 14:59:25 UTC
I see the same behaviour with trunk 8.0.x. This looks like a regression between 8.0.43 and 8.0.49. Nothing obvious in the change log. Still looking.
Comment 7 Mark Thomas 2018-01-24 15:05:07 UTC
Hmm. 8.0.43 fails in the same way.

Maybe the 8.0.43 tested by the OP was configured in such a way that this worked?
Comment 8 Mark Thomas 2018-01-24 16:01:23 UTC
Regardless, this does look like a bug that needs to be fixed. Apart from NonLoginAuthenticator.doAuthenticate() everywhere else that caches the Principal in the session caches the TomcatPrincipal. Switching to s/getUserPrincipal()/getPrincipal()/ looks like it will work - but I want to see if we need to validate any GSSCredential at this point.
Comment 9 Mark Thomas 2018-01-24 16:37:37 UTC
The prior call to checkForCachedAuthentication() will trigger a logout if the GSSCredential has expired so the proposed fix looks good.

Fixed in:
- trunk for 9.0.5 onwards
- 8.5.x for 8.5.28 onwards
- 8.0.x for 8.0.50 onwards
- 7.0.x for 7.0.85 onwards