You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by Jess Holle <je...@ptc.com> on 2014/11/14 20:25:46 UTC

JNDIRealm bug

I just moved from Tomcat 7.0.56 to 8.0.15.

I tried out form-based authentication using CombinedRealm of 2 JNDIRealms.

I get:

java.lang.NullPointerException
	org.apache.catalina.realm.JNDIRealm.getUser(JNDIRealm.java:1286)
	org.apache.catalina.realm.JNDIRealm.getUser(JNDIRealm.java:1236)
	org.apache.catalina.realm.JNDIRealm.authenticate(JNDIRealm.java:1177)
	org.apache.catalina.realm.JNDIRealm.authenticate(JNDIRealm.java:1052)
	org.apache.catalina.realm.CombinedRealm.authenticate(CombinedRealm.java:157)
	org.apache.catalina.authenticator.FormAuthenticator.authenticate(FormAuthenticator.java:272)
	org.apache.catalina.authenticator.AuthenticatorBase.invoke(AuthenticatorBase.java:452)
	org.apache.catalina.valves.ErrorReportValve.invoke(ErrorReportValve.java:79)
	org.apache.catalina.connector.CoyoteAdapter.service(CoyoteAdapter.java:537)
	org.apache.coyote.ajp.AbstractAjpProcessor.process(AbstractAjpProcessor.java:831)
	org.apache.coyote.AbstractProtocol$AbstractConnectionHandler.process(AbstractProtocol.java:658)
	org.apache.tomcat.util.net.JIoEndpoint$SocketProcessor.run(JIoEndpoint.java:277)
	java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1145)
	java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:615)
	org.apache.tomcat.util.threads.TaskThread$WrappingRunnable.run(TaskThread.java:61)
	java.lang.Thread.run(Thread.java:745)

This tracks directly to the following code added between these two releases:

         if (userPassword == null && credentials != null) {
             // The password is available. Insert it since it may be required for
             // role searches.
             return new User(user.getUserName(), user.getDN(), credentials,
                     user.getRoles(), user.getUserRoleId());
         }

The new User(...) line is the bad one.  Earlier in the method there was 
a search for a user:

         if (userPatternFormatArray != null && curUserPattern >= 0) {
             user = getUserByPattern(context, username, credentials, attrIds, curUserPattern);
         } else {
             user = getUserBySearch(context, username, attrIds);
         }

but there's no null check in between these two bodies of code, so this 
is an obvious source of NPE's.

I'll patch this in my own Tomcat, but I wanted to notify the community so:

 1. I don't have to maintain this patch long term and
 2. The Tomcat community can review the correctness/sanity of this code
    change and decide whether the correct fix is simply adding a null
    check or something larger.

--
Jess Holle


Re: JNDIRealm bug

Posted by Christopher Schultz <ch...@christopherschultz.net>.
Jess,

On 11/14/14 2:25 PM, Jess Holle wrote:
> I just moved from Tomcat 7.0.56 to 8.0.15.
> 
> I tried out form-based authentication using CombinedRealm of 2 JNDIRealms.
> 
> I get:
> 
> java.lang.NullPointerException
>     org.apache.catalina.realm.JNDIRealm.getUser(JNDIRealm.java:1286)
>     org.apache.catalina.realm.JNDIRealm.getUser(JNDIRealm.java:1236)
>     org.apache.catalina.realm.JNDIRealm.authenticate(JNDIRealm.java:1177)
>     org.apache.catalina.realm.JNDIRealm.authenticate(JNDIRealm.java:1052)
>     org.apache.catalina.realm.CombinedRealm.authenticate(CombinedRealm.java:157)
> 
>     org.apache.catalina.authenticator.FormAuthenticator.authenticate(FormAuthenticator.java:272)
> 
>     org.apache.catalina.authenticator.AuthenticatorBase.invoke(AuthenticatorBase.java:452)
> 
>     org.apache.catalina.valves.ErrorReportValve.invoke(ErrorReportValve.java:79)
> 
>     org.apache.catalina.connector.CoyoteAdapter.service(CoyoteAdapter.java:537)
> 
>     org.apache.coyote.ajp.AbstractAjpProcessor.process(AbstractAjpProcessor.java:831)
> 
>     org.apache.coyote.AbstractProtocol$AbstractConnectionHandler.process(AbstractProtocol.java:658)
> 
>     org.apache.tomcat.util.net.JIoEndpoint$SocketProcessor.run(JIoEndpoint.java:277)
> 
>     java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1145)
> 
>     java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:615)
> 
>     org.apache.tomcat.util.threads.TaskThread$WrappingRunnable.run(TaskThread.java:61)
> 
>     java.lang.Thread.run(Thread.java:745)
> 
> This tracks directly to the following code added between these two
> releases:
> 
>         if (userPassword == null && credentials != null) {
>             // The password is available. Insert it since it may be
> required for
>             // role searches.
>             return new User(user.getUserName(), user.getDN(), credentials,
>                     user.getRoles(), user.getUserRoleId());
>         }
> 
> The new User(...) line is the bad one.  Earlier in the method there was
> a search for a user:
> 
>         if (userPatternFormatArray != null && curUserPattern >= 0) {
>             user = getUserByPattern(context, username, credentials,
> attrIds, curUserPattern);
>         } else {
>             user = getUserBySearch(context, username, attrIds);
>         }
> 
> but there's no null check in between these two bodies of code, so this
> is an obvious source of NPE's.
> 
> I'll patch this in my own Tomcat, but I wanted to notify the community so:
> 
> 1. I don't have to maintain this patch long term and
> 2. The Tomcat community can review the correctness/sanity of this code
>    change and decide whether the correct fix is simply adding a null
>    check or something larger.

Please file a bug and attach your patch it.

-chris


Re: JNDIRealm bug

Posted by Christopher Schultz <ch...@christopherschultz.net>.
Jess,

https://issues.apache.org/bugzilla/show_bug.cgi?id=57208

On 11/14/14 2:25 PM, Jess Holle wrote:
> I just moved from Tomcat 7.0.56 to 8.0.15.
> 
> I tried out form-based authentication using CombinedRealm of 2 JNDIRealms.
> 
> I get:
> 
> java.lang.NullPointerException
>     org.apache.catalina.realm.JNDIRealm.getUser(JNDIRealm.java:1286)
>     org.apache.catalina.realm.JNDIRealm.getUser(JNDIRealm.java:1236)
>     org.apache.catalina.realm.JNDIRealm.authenticate(JNDIRealm.java:1177)
>     org.apache.catalina.realm.JNDIRealm.authenticate(JNDIRealm.java:1052)
>     org.apache.catalina.realm.CombinedRealm.authenticate(CombinedRealm.java:157)
> 
>     org.apache.catalina.authenticator.FormAuthenticator.authenticate(FormAuthenticator.java:272)
> 
>     org.apache.catalina.authenticator.AuthenticatorBase.invoke(AuthenticatorBase.java:452)
> 
>     org.apache.catalina.valves.ErrorReportValve.invoke(ErrorReportValve.java:79)
> 
>     org.apache.catalina.connector.CoyoteAdapter.service(CoyoteAdapter.java:537)
> 
>     org.apache.coyote.ajp.AbstractAjpProcessor.process(AbstractAjpProcessor.java:831)
> 
>     org.apache.coyote.AbstractProtocol$AbstractConnectionHandler.process(AbstractProtocol.java:658)
> 
>     org.apache.tomcat.util.net.JIoEndpoint$SocketProcessor.run(JIoEndpoint.java:277)
> 
>     java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1145)
> 
>     java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:615)
> 
>     org.apache.tomcat.util.threads.TaskThread$WrappingRunnable.run(TaskThread.java:61)
> 
>     java.lang.Thread.run(Thread.java:745)
> 
> This tracks directly to the following code added between these two
> releases:
> 
>         if (userPassword == null && credentials != null) {
>             // The password is available. Insert it since it may be
> required for
>             // role searches.
>             return new User(user.getUserName(), user.getDN(), credentials,
>                     user.getRoles(), user.getUserRoleId());
>         }
> 
> The new User(...) line is the bad one.  Earlier in the method there was
> a search for a user:
> 
>         if (userPatternFormatArray != null && curUserPattern >= 0) {
>             user = getUserByPattern(context, username, credentials,
> attrIds, curUserPattern);
>         } else {
>             user = getUserBySearch(context, username, attrIds);
>         }
> 
> but there's no null check in between these two bodies of code, so this
> is an obvious source of NPE's.
> 
> I'll patch this in my own Tomcat, but I wanted to notify the community so:
> 
> 1. I don't have to maintain this patch long term and
> 2. The Tomcat community can review the correctness/sanity of this code
>    change and decide whether the correct fix is simply adding a null
>    check or something larger.
> 
> -- 
> Jess Holle
> 
>