This is an automated email from the ASF dual-hosted git repository. markt pushed a commit to branch 8.5.x in repository https://gitbox.apache.org/repos/asf/tomcat.git
The following commit(s) were added to refs/heads/8.5.x by this push: new bb88e99 Fix BZ 64715. Add PasswordValidationCallback support. bb88e99 is described below commit bb88e9995aff950bdd73fbed949f3396f2c89b01 Author: Mark Thomas <ma...@apache.org> AuthorDate: Fri Sep 18 09:12:53 2020 +0100 Fix BZ 64715. Add PasswordValidationCallback support. https://bz.apache.org/bugzilla/show_bug.cgi?id=64715 Patch provided by Robert Rodewald --- .../catalina/authenticator/AuthenticatorBase.java | 55 +++++---- .../authenticator/jaspic/CallbackHandlerImpl.java | 52 +++++---- .../authenticator/jaspic/LocalStrings.properties | 2 + .../TestJaspicCallbackHandlerInAuthenticator.java | 129 +++++++++++++++++++-- webapps/docs/changelog.xml | 4 + 5 files changed, 190 insertions(+), 52 deletions(-) diff --git a/java/org/apache/catalina/authenticator/AuthenticatorBase.java b/java/org/apache/catalina/authenticator/AuthenticatorBase.java index b053e49..b20b4b6 100644 --- a/java/org/apache/catalina/authenticator/AuthenticatorBase.java +++ b/java/org/apache/catalina/authenticator/AuthenticatorBase.java @@ -42,6 +42,7 @@ import javax.servlet.http.HttpServletRequest; import javax.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; @@ -50,7 +51,6 @@ import org.apache.catalina.Realm; 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; @@ -221,7 +221,7 @@ public abstract class AuthenticatorBase extends ValveBase * 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 @@ -248,6 +248,7 @@ public abstract class AuthenticatorBase extends ValveBase private volatile String jaspicAppContextID = null; private volatile AuthConfigProvider jaspicProvider = null; + private volatile CallbackHandler jaspicCallbackHandler = null; // ------------------------------------------------------------- Properties @@ -774,7 +775,7 @@ public abstract class AuthenticatorBase extends ValveBase 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); @@ -788,29 +789,41 @@ public abstract class AuthenticatorBase extends ValveBase 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(); + } catch (ReflectiveOperationException e) { + throw new SecurityException(e); + } + + if (callbackHandler instanceof Contained) { + ((Contained) callbackHandler).setContainer(getContainer()); } + jaspicCallbackHandler = callbackHandler; return callbackHandler; } @@ -1306,7 +1319,7 @@ public abstract class AuthenticatorBase extends ValveBase 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 08f70c7..b1acaa5 100644 --- a/java/org/apache/catalina/authenticator/jaspic/CallbackHandlerImpl.java +++ b/java/org/apache/catalina/authenticator/jaspic/CallbackHandlerImpl.java @@ -28,35 +28,24 @@ import javax.security.auth.callback.CallbackHandler; import javax.security.auth.callback.UnsupportedCallbackException; import javax.security.auth.message.callback.CallerPrincipalCallback; import javax.security.auth.message.callback.GroupPrincipalCallback; +import javax.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. */ -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; - - - static { - instance = new CallbackHandlerImpl(); - } - - - public static CallbackHandler getInstance() { - return instance; - } - - - private CallbackHandlerImpl() { - // Hide default constructor - } + private Container container; @Override @@ -80,10 +69,19 @@ public class CallbackHandlerImpl implements CallbackHandler { } 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())); } @@ -118,4 +116,16 @@ public class CallbackHandlerImpl implements CallbackHandler { return new GenericPrincipal(name, null, roles, principal); } + + // Contained interface methods + @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..b86acb1 100644 --- a/java/org/apache/catalina/authenticator/jaspic/LocalStrings.properties +++ b/java/org/apache/catalina/authenticator/jaspic/LocalStrings.properties @@ -19,7 +19,9 @@ authConfigFactoryImpl.registerInstance=Registering instance of type[{0}] for lay authConfigFactoryImpl.zeroLengthAppContext=A zero length application context name is not valid authConfigFactoryImpl.zeroLengthMessageLayer=A zero length message layer name is not valid +callbackHandlerImpl.containerMissing=Missing container for JASPIC callback of type [{0}] which was ignored 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 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 51807de..cd63ed8 100644 --- a/test/org/apache/catalina/authenticator/TestJaspicCallbackHandlerInAuthenticator.java +++ b/test/org/apache/catalina/authenticator/TestJaspicCallbackHandlerInAuthenticator.java @@ -19,17 +19,30 @@ package org.apache.catalina.authenticator; 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 javax.security.auth.message.callback.CallerPrincipalCallback; +import javax.security.auth.message.callback.GroupPrincipalCallback; +import javax.security.auth.message.callback.PasswordValidationCallback; import javax.servlet.http.HttpServletResponse; import org.junit.Assert; import org.junit.Test; +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; public class TestJaspicCallbackHandlerInAuthenticator { @@ -39,26 +52,91 @@ public class TestJaspicCallbackHandlerInAuthenticator { TestCallbackHandlerImpl.class); } + @Test public void testDefaultCallbackHandlerCreation() throws Exception { testCallbackHandlerCreation(null, CallbackHandlerImpl.class); } - private void testCallbackHandlerCreation(String callbackHandlerImplClassName, - Class<?> callbackHandlerImplClass) - throws NoSuchMethodException, SecurityException, IllegalAccessException, - IllegalArgumentException, InvocationTargetException { + private void testCallbackHandlerCreation(String callbackHandlerImplClassName, Class<?> callbackHandlerImplClass) + throws NoSuchMethodException, SecurityException, IllegalAccessException, IllegalArgumentException, + InvocationTargetException { + CallbackHandler callbackHandler = createCallbackHandler(callbackHandlerImplClassName); + Assert.assertTrue(callbackHandlerImplClass.isInstance(callbackHandler)); + } + + + @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<>(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 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 { @Override @@ -73,8 +151,39 @@ public class TestJaspicCallbackHandlerInAuthenticator { } } + + + 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, null, null); + } + } } + class TestCallbackHandlerImpl implements CallbackHandler { public TestCallbackHandlerImpl() { diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml index 36eda75..ac004dc 100644 --- a/webapps/docs/changelog.xml +++ b/webapps/docs/changelog.xml @@ -58,6 +58,10 @@ Correct numerous spellings throughout the code base. Based on a pull request from John Bampton. (markt) </fix> + <fix> + <bug>64715</bug>: Add PasswordValidationCallback to the JASPIC + implementation. Patch provided by Robert Rodewald. (markt) + </fix> </changelog> </subsection> <subsection name="Coyote"> --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org