[patch] Proposed patch for bug 64715 (second attempt), Tomcat 10

tomcat10_bug_64715.patch (text/plain), 16.57 KB, created by Robert Rodewald on 2020-09-09 10:17:59 UTC

(hide)

Description: Proposed patch for bug 64715 (second attempt), Tomcat 10

Filename:

MIME Type:

Creator: Robert Rodewald

Created: 2020-09-09 10:17:59 UTC

Size: 16.57 KB

patch

obsolete

>diff --git a/java/org/apache/catalina/authenticator/AuthenticatorBase.java b/java/org/apache/catalina/authenticator/AuthenticatorBase.java >index 66b9038..783d96e 100644 >--- a/java/org/apache/catalina/authenticator/AuthenticatorBase.java >+++ b/java/org/apache/catalina/authenticator/AuthenticatorBase.java >@@ -43,6 +43,7 @@ > import jakarta.servlet.http.HttpServletResponse; > > import org.apache.catalina.Authenticator; >+import org.apache.catalina.Contained; > import org.apache.catalina.Container; > import org.apache.catalina.Context; > import org.apache.catalina.Globals; >@@ -51,7 +52,6 @@ > import org.apache.catalina.Session; > import org.apache.catalina.TomcatPrincipal; > import org.apache.catalina.Valve; >-import org.apache.catalina.authenticator.jaspic.CallbackHandlerImpl; > import org.apache.catalina.authenticator.jaspic.MessageInfoImpl; > import org.apache.catalina.connector.Request; > import org.apache.catalina.connector.Response; >@@ -220,7 +220,7 @@ > * default {@link org.apache.catalina.authenticator.jaspic.CallbackHandlerImpl} > * will be used. > */ >- protected String jaspicCallbackHandlerClass = null; >+ protected String jaspicCallbackHandlerClass = "org.apache.catalina.authenticator.jaspic.CallbackHandlerImpl"; > > /** > * Should the auth information (remote user and auth type) be returned as response >@@ -247,6 +247,7 @@ > > private volatile String jaspicAppContextID = null; > private volatile Optional<AuthConfigProvider> jaspicProvider = null; >+ private volatile CallbackHandler jaspicCallbackHandler = null; > > > // ------------------------------------------------------------- Properties >@@ -773,7 +774,7 @@ > new MessageInfoImpl(request.getRequest(), response.getResponse(), authMandatory); > > try { >- CallbackHandler callbackHandler = createCallbackHandler(); >+ CallbackHandler callbackHandler = getCallbackHandler(); > ServerAuthConfig serverAuthConfig = jaspicProvider.getServerAuthConfig( > "HttpServlet", jaspicAppContextID, callbackHandler); > String authContextID = serverAuthConfig.getAuthContextID(jaspicState.messageInfo); >@@ -787,29 +788,37 @@ > return jaspicState; > } > >+ private CallbackHandler getCallbackHandler() { >+ CallbackHandler handler = jaspicCallbackHandler; >+ if (handler == null) { >+ handler = createCallbackHandler(); >+ } >+ return handler; >+ } >+ > private CallbackHandler createCallbackHandler() { > CallbackHandler callbackHandler = null; >- if (jaspicCallbackHandlerClass == null) { >- callbackHandler = CallbackHandlerImpl.getInstance(); >- } else { >- Class<?> clazz = null; >- try { >- clazz = Class.forName(jaspicCallbackHandlerClass, true, >- Thread.currentThread().getContextClassLoader()); >- } catch (ClassNotFoundException e) { >- // Proceed with the retry below >- } >- >- try { >- if (clazz == null) { >- clazz = Class.forName(jaspicCallbackHandlerClass); >- } >- callbackHandler = (CallbackHandler)clazz.getConstructor().newInstance(); >- } catch (ReflectiveOperationException e) { >- throw new SecurityException(e); >- } >+ Class<?> clazz = null; >+ try { >+ clazz = Class.forName(jaspicCallbackHandlerClass, true, >+ Thread.currentThread().getContextClassLoader()); >+ } catch (ClassNotFoundException e) { >+ // Proceed with the retry below > } > >+ try { >+ if (clazz == null) { >+ clazz = Class.forName(jaspicCallbackHandlerClass); >+ } >+ callbackHandler = (CallbackHandler) clazz.getConstructor().newInstance(); >+ if (callbackHandler instanceof Contained) { >+ ((Contained)callbackHandler).setContainer(context); >+ } >+ } catch (ReflectiveOperationException e) { >+ throw new SecurityException(e); >+ } >+ >+ jaspicCallbackHandler = callbackHandler; > return callbackHandler; > } > >@@ -1305,7 +1314,7 @@ > ServerAuthContext serverAuthContext; > try { > ServerAuthConfig serverAuthConfig = provider.getServerAuthConfig("HttpServlet", >- jaspicAppContextID, CallbackHandlerImpl.getInstance()); >+ jaspicAppContextID, getCallbackHandler()); > String authContextID = serverAuthConfig.getAuthContextID(messageInfo); > serverAuthContext = serverAuthConfig.getAuthContext(authContextID, null, null); > serverAuthContext.cleanSubject(messageInfo, client); >diff --git a/java/org/apache/catalina/authenticator/jaspic/CallbackHandlerImpl.java b/java/org/apache/catalina/authenticator/jaspic/CallbackHandlerImpl.java >index dc539c7..0834abb 100644 >--- a/java/org/apache/catalina/authenticator/jaspic/CallbackHandlerImpl.java >+++ b/java/org/apache/catalina/authenticator/jaspic/CallbackHandlerImpl.java >@@ -29,35 +29,26 @@ > > import jakarta.security.auth.message.callback.CallerPrincipalCallback; > import jakarta.security.auth.message.callback.GroupPrincipalCallback; >- >+import jakarta.security.auth.message.callback.PasswordValidationCallback; >+import org.apache.catalina.Contained; >+import org.apache.catalina.Container; > import org.apache.catalina.realm.GenericPrincipal; > import org.apache.juli.logging.Log; > import org.apache.juli.logging.LogFactory; > import org.apache.tomcat.util.res.StringManager; > > /** >- * Implemented as a singleton since the class is stateless. >+ * Default implementation of a JASPIC CallbackHandler. >+ * >+ * @see AuthenticatorBase#setJaspicCallbackHandlerClass(String) > */ >-public class CallbackHandlerImpl implements CallbackHandler { >+public class CallbackHandlerImpl implements CallbackHandler, Contained { > >- private static final StringManager sm = StringManager.getManager(CallbackHandlerImpl.class); >+ private final Log log = LogFactory.getLog(CallbackHandlerImpl.class); // must not be static > >- private static CallbackHandler instance; >+ protected static final StringManager sm = StringManager.getManager(CallbackHandlerImpl.class); > >- >- static { >- instance = new CallbackHandlerImpl(); >- } >- >- >- public static CallbackHandler getInstance() { >- return instance; >- } >- >- >- private CallbackHandlerImpl() { >- // Hide default constructor >- } >+ protected Container container; > > > @Override >@@ -81,10 +72,20 @@ > } else if (callback instanceof GroupPrincipalCallback) { > GroupPrincipalCallback gpc = (GroupPrincipalCallback) callback; > groups = gpc.getGroups(); >+ } else if (callback instanceof PasswordValidationCallback) { >+ if (container == null) { >+ log.warn(sm.getString("callbackHandlerImpl.containerMissing", >+ callback.getClass().getName())); >+ } else if (container.getRealm() == null) { >+ log.warn(sm.getString("callbackHandlerImpl.realmMissing", >+ callback.getClass().getName(), container.getName())); >+ } else { >+ PasswordValidationCallback pvc = (PasswordValidationCallback) callback; >+ principal = container.getRealm().authenticate(pvc.getUsername(), >+ String.valueOf(pvc.getPassword())); >+ subject = pvc.getSubject(); >+ } > } else { >- // This is a singleton so need to get correct Logger for >- // current TCCL >- Log log = LogFactory.getLog(CallbackHandlerImpl.class); > log.error(sm.getString("callbackHandlerImpl.jaspicCallbackMissing", > callback.getClass().getName())); > } >@@ -119,4 +120,16 @@ > > return new GenericPrincipal(name, roles, principal); > } >+ >+ >+ @Override >+ public Container getContainer() { >+ return this.container; >+ } >+ >+ >+ @Override >+ public void setContainer(Container container) { >+ this.container = container; >+ } > } >diff --git a/java/org/apache/catalina/authenticator/jaspic/LocalStrings.properties b/java/org/apache/catalina/authenticator/jaspic/LocalStrings.properties >index 1f395a0..a702645 100644 >--- a/java/org/apache/catalina/authenticator/jaspic/LocalStrings.properties >+++ b/java/org/apache/catalina/authenticator/jaspic/LocalStrings.properties >@@ -20,6 +20,8 @@ > authConfigFactoryImpl.zeroLengthMessageLayer=A zero length message layer name is not valid > > callbackHandlerImpl.jaspicCallbackMissing=Unsupported JASPIC callback of type [{0}] received which was ignored >+callbackHandlerImpl.realmMissing=Missing realm for JASPIC callback of type [{0}] in container [{1}] which was ignored >+callbackHandlerImpl.containerMissing=Missing container for JASPIC callback of type [{0}] which was ignored > > jaspicAuthenticator.authenticate=Authenticating request for [{0}] via JASPIC > >diff --git a/test/org/apache/catalina/authenticator/TestJaspicCallbackHandlerInAuthenticator.java b/test/org/apache/catalina/authenticator/TestJaspicCallbackHandlerInAuthenticator.java >index 0502ea4..1f1c9b5 100644 >--- a/test/org/apache/catalina/authenticator/TestJaspicCallbackHandlerInAuthenticator.java >+++ b/test/org/apache/catalina/authenticator/TestJaspicCallbackHandlerInAuthenticator.java >@@ -19,18 +19,30 @@ > import java.io.IOException; > import java.lang.reflect.InvocationTargetException; > import java.lang.reflect.Method; >+import java.security.Principal; >+import java.util.Arrays; >+import java.util.HashSet; >+import java.util.Set; > >+import javax.security.auth.Subject; > import javax.security.auth.callback.Callback; > import javax.security.auth.callback.CallbackHandler; > import javax.security.auth.callback.UnsupportedCallbackException; > >-import jakarta.servlet.http.HttpServletResponse; >- >+import org.apache.catalina.Contained; >+import org.apache.catalina.Container; >+import org.apache.catalina.authenticator.jaspic.CallbackHandlerImpl; >+import org.apache.catalina.connector.Request; >+import org.apache.catalina.core.ContainerBase; >+import org.apache.catalina.realm.GenericPrincipal; >+import org.apache.catalina.realm.RealmBase; > import org.junit.Assert; > import org.junit.Test; > >-import org.apache.catalina.authenticator.jaspic.CallbackHandlerImpl; >-import org.apache.catalina.connector.Request; >+import jakarta.security.auth.message.callback.CallerPrincipalCallback; >+import jakarta.security.auth.message.callback.GroupPrincipalCallback; >+import jakarta.security.auth.message.callback.PasswordValidationCallback; >+import jakarta.servlet.http.HttpServletResponse; > > public class TestJaspicCallbackHandlerInAuthenticator { > >@@ -45,19 +57,79 @@ > testCallbackHandlerCreation(null, CallbackHandlerImpl.class); > } > >+ @Test >+ public void testCallerPrincipalCallback() throws Exception { >+ CallbackHandler callbackHandler = createCallbackHandler(null); >+ Subject clientSubject = new Subject(); >+ CallerPrincipalCallback cpc1 = new CallerPrincipalCallback(clientSubject, "name1"); >+ callbackHandler.handle(new Callback[] { cpc1 }); >+ CallerPrincipalCallback cpc2 = new CallerPrincipalCallback(clientSubject, new Principal() { >+ @Override >+ public String getName() { >+ return "name2"; >+ } >+ }); >+ callbackHandler.handle(new Callback[] { cpc2 }); >+ Set<Object> credentials = clientSubject.getPrivateCredentials(); >+ Assert.assertTrue(credentials.size() == 2); >+ Set<String> names = new HashSet<String>(Arrays.asList(new String[] { "name1", "name2" })); >+ for (Object o : credentials) >+ names.remove(((GenericPrincipal) o).getName()); >+ Assert.assertTrue(names.isEmpty()); >+ } >+ >+ @Test >+ public void testGroupPrincipalCallback() throws Exception { >+ CallbackHandler callbackHandler = createCallbackHandler(null); >+ Subject clientSubject = new Subject(); >+ CallerPrincipalCallback cpc = new CallerPrincipalCallback(clientSubject, "name"); >+ GroupPrincipalCallback gpc = new GroupPrincipalCallback(clientSubject, >+ new String[] { "group1", "group2" }); >+ callbackHandler.handle(new Callback[] { cpc, gpc }); >+ Set<Object> credentials = clientSubject.getPrivateCredentials(); >+ Assert.assertTrue(credentials.size() == 1); >+ GenericPrincipal gp = (GenericPrincipal) credentials.iterator().next(); >+ Assert.assertEquals("name", gp.getName()); >+ Assert.assertTrue(gp.hasRole("group1")); >+ Assert.assertTrue(gp.hasRole("group2")); >+ } >+ >+ @Test >+ public void testPasswordValidationCallback() throws Exception { >+ CallbackHandler callbackHandler = createCallbackHandler(null); >+ Container container = new TestContainer(); >+ container.setRealm(new TestRealm()); >+ ((Contained) callbackHandler).setContainer(container); >+ Subject clientSubject = new Subject(); >+ PasswordValidationCallback pvc1 = new PasswordValidationCallback(clientSubject, "name1", >+ "password".toCharArray()); >+ callbackHandler.handle(new Callback[] { pvc1 }); >+ PasswordValidationCallback pvc2 = new PasswordValidationCallback(clientSubject, "name2", >+ "invalid".toCharArray()); >+ callbackHandler.handle(new Callback[] { pvc2 }); >+ Set<Object> credentials = clientSubject.getPrivateCredentials(); >+ Assert.assertTrue(credentials.size() == 1); >+ GenericPrincipal gp = (GenericPrincipal) credentials.iterator().next(); >+ Assert.assertEquals("name1", gp.getName()); >+ } > > private void testCallbackHandlerCreation(String callbackHandlerImplClassName, >- Class<?> callbackHandlerImplClass) >+ Class<?> callbackHandlerImplClass) throws NoSuchMethodException, SecurityException, >+ IllegalAccessException, IllegalArgumentException, InvocationTargetException { >+ CallbackHandler callbackHandler = createCallbackHandler(callbackHandlerImplClassName); >+ Assert.assertTrue(callbackHandlerImplClass.isInstance(callbackHandler)); >+ } >+ >+ private CallbackHandler createCallbackHandler(String callbackHandlerImplClassName) > throws NoSuchMethodException, SecurityException, IllegalAccessException, > IllegalArgumentException, InvocationTargetException { > TestAuthenticator authenticator = new TestAuthenticator(); >- authenticator.setJaspicCallbackHandlerClass(callbackHandlerImplClassName); >- Method createCallbackHandlerMethod = >- AuthenticatorBase.class.getDeclaredMethod("createCallbackHandler"); >+ if (callbackHandlerImplClassName != null) >+ authenticator.setJaspicCallbackHandlerClass(callbackHandlerImplClassName); >+ Method createCallbackHandlerMethod = AuthenticatorBase.class >+ .getDeclaredMethod("createCallbackHandler"); > createCallbackHandlerMethod.setAccessible(true); >- CallbackHandler callbackHandler = >- (CallbackHandler) createCallbackHandlerMethod.invoke(authenticator); >- Assert.assertTrue(callbackHandlerImplClass.isInstance(callbackHandler)); >+ return (CallbackHandler) createCallbackHandlerMethod.invoke(authenticator); > } > > private static class TestAuthenticator extends AuthenticatorBase { >@@ -72,7 +144,34 @@ > protected String getAuthMethod() { > return null; > } >+ } > >+ private static class TestContainer extends ContainerBase { >+ >+ @Override >+ protected String getObjectNameKeyProperties() { >+ return null; >+ } >+ } >+ >+ private static class TestRealm extends RealmBase { >+ >+ @Override >+ public Principal authenticate(String username, String password) { >+ if (getPassword(username).equals(password)) >+ return getPrincipal(username); >+ return null; >+ } >+ >+ @Override >+ protected String getPassword(String username) { >+ return "password"; >+ } >+ >+ @Override >+ protected Principal getPrincipal(String username) { >+ return new GenericPrincipal(username); >+ } > } > } > >@@ -82,7 +181,6 @@ > // Default constructor required by reflection > } > >- > @Override > public void handle(Callback[] callbacks) throws IOException, UnsupportedCallbackException { > // don't have to do anything; needed only for instantiation View Attachment As Diff View Attachment As Raw

This is ASF Bugzilla: the Apache Software Foundation bug system. In case of problems with the functioning of ASF Bugzilla, please contact bugzilla-admin@apache.org. Please Note: this e-mail address is only for reporting problems with ASF Bugzilla. Mail about any other subject will be silently ignored.

• • Home
  • | New
  • | Browse
  • | Search
  • |
    [?]
  • | Reports
  • | Help
  • | New Account
  • | Log In
    Remember [x]
  • | Forgot Password
    Login: [x]