On Wed, Jan 15, 2020 at 5:35 PM Mark Thomas <ma...@apache.org> wrote:
> On 15/01/2020 16:01, Rémy Maucherat wrote: > > On Wed, Jan 15, 2020 at 4:37 PM <ma...@apache.org > > <mailto:ma...@apache.org>> wrote: > > > > This is an automated email from the ASF dual-hosted git repository. > > > > markt pushed a commit to branch master > > in repository https://gitbox.apache.org/repos/asf/tomcat.git > > > > commit c64ccf3fd2bd58949360ab05b2f20da610b2c999 > > Author: Mark Thomas <ma...@apache.org <mailto:ma...@apache.org>> > > AuthorDate: Wed Jan 15 15:36:05 2020 +0000 > > > > Update tests to use SSLHostConfig for TLS configuration > > > > > > I was doing this removal as well at the same time, predictably it has a > > large impact on embedded TLS (which was already quite nightmarish). Oh > > well, it had to happen. > > Sorry if I caused you to waste time on this. > No problem, you did it better. > > While I was doing this I did wonder about deprecating/removing > [get|set]Attribute on Connector (and any other element where we have > both [get|set]Attribute() and [get|set]Property(). Thoughts? Something > to add to the TODO list? > Ok, I forgot the real use of that [get|set]Attribute() to be honest. > > I'm currently working on ensuring master, 9.0.x and 8.5.x are as aligned > as possible (with a view to keeping them that way). Hopefully that won't > conflict. > Rémy > > > > > > Rémy > > > > > > --- > > test/org/apache/tomcat/util/net/TestCustomSsl.java | 35 > > +++++++++------- > > test/org/apache/tomcat/util/net/TesterSupport.java | 49 > > ++++++++++------------ > > .../util/net/jsse/TesterBug50640SslImpl.java | 1 - > > 3 files changed, 40 insertions(+), 45 deletions(-) > > > > diff --git a/test/org/apache/tomcat/util/net/TestCustomSsl.java > > b/test/org/apache/tomcat/util/net/TestCustomSsl.java > > index 60dbf00..f036931 100644 > > --- a/test/org/apache/tomcat/util/net/TestCustomSsl.java > > +++ b/test/org/apache/tomcat/util/net/TestCustomSsl.java > > @@ -32,6 +32,7 @@ import org.apache.catalina.startup.TomcatBaseTest; > > import org.apache.coyote.ProtocolHandler; > > import org.apache.coyote.http11.AbstractHttp11JsseProtocol; > > import org.apache.tomcat.util.buf.ByteChunk; > > +import org.apache.tomcat.util.net > > <http://org.apache.tomcat.util.net>.SSLHostConfigCertificate.Type; > > import org.apache.tomcat.util.net > > <http://org.apache.tomcat.util.net>.jsse.TesterBug50640SslImpl; > > import org.apache.tomcat.websocket.server.WsContextListener; > > > > @@ -59,20 +60,22 @@ public class TestCustomSsl extends > TomcatBaseTest { > > Assume.assumeFalse("This test is only for JSSE based SSL > > connectors", > > > > connector.getProtocolHandlerClassName().contains("Apr")); > > > > + SSLHostConfig sslHostConfig = new SSLHostConfig(); > > + SSLHostConfigCertificate certificate = new > > SSLHostConfigCertificate(sslHostConfig, Type.UNDEFINED); > > + sslHostConfig.addCertificate(certificate); > > + connector.addSslHostConfig(sslHostConfig); > > + > > Assert.assertTrue(connector.setProperty( > > "sslImplementationName", > > "org.apache.tomcat.util.net > > <http://org.apache.tomcat.util.net>.jsse.TesterBug50640SslImpl")); > > > > // This setting will break ssl configuration unless the > custom > > // implementation is used. > > - Assert.assertTrue(connector.setProperty( > > - TesterBug50640SslImpl.PROPERTY_NAME, > > TesterBug50640SslImpl.PROPERTY_VALUE)); > > + > > sslHostConfig.setProtocols(TesterBug50640SslImpl.PROPERTY_VALUE); > > > > - Assert.assertTrue(connector.setProperty("sslProtocol", > "tls")); > > + sslHostConfig.setSslProtocol("tls"); > > > > - File keystoreFile = > > - new File(TesterSupport.LOCALHOST_RSA_JKS); > > - connector.setAttribute( > > - "keystoreFile", keystoreFile.getAbsolutePath()); > > + File keystoreFile = new > File(TesterSupport.LOCALHOST_RSA_JKS); > > + > > > certificate.setCertificateKeystoreFile(keystoreFile.getAbsolutePath()); > > > > connector.setSecure(true); > > Assert.assertTrue(connector.setProperty("SSLEnabled", > "true")); > > @@ -109,23 +112,25 @@ public class TestCustomSsl extends > > TomcatBaseTest { > > Tomcat tomcat = getTomcatInstance(); > > > > Assume.assumeTrue("SSL renegotiation has to be supported > > for this test", > > - > > TesterSupport.isRenegotiationSupported(getTomcatInstance())); > > + TesterSupport.isRenegotiationSupported(tomcat)); > > > > TesterSupport.configureClientCertContext(tomcat); > > > > + Connector connector = tomcat.getConnector(); > > + > > // Override the defaults > > - ProtocolHandler handler = > > tomcat.getConnector().getProtocolHandler(); > > + ProtocolHandler handler = connector.getProtocolHandler(); > > if (handler instanceof AbstractHttp11JsseProtocol) { > > - ((AbstractHttp11JsseProtocol<?>) > > handler).setTruststoreFile(null); > > + > connector.findSslHostConfigs()[0].setTruststoreFile(null); > > } else { > > // Unexpected > > Assert.fail("Unexpected handler type"); > > } > > if (trustType.equals(TrustType.ALL)) { > > - > tomcat.getConnector().setAttribute("trustManagerClassName", > > + > connector.findSslHostConfigs()[0].setTrustManagerClassName( > > "org.apache.tomcat.util.net > > <http://org.apache.tomcat.util.net>.TesterSupport$TrustAllCerts"); > > } else if (trustType.equals(TrustType.CA)) { > > - > tomcat.getConnector().setAttribute("trustManagerClassName", > > + > connector.findSslHostConfigs()[0].setTrustManagerClassName( > > "org.apache.tomcat.util.net > > <http://org.apache.tomcat.util.net > >.TesterSupport$SequentialTrustManager"); > > } > > > > @@ -135,16 +140,14 @@ public class TestCustomSsl extends > > TomcatBaseTest { > > TesterSupport.configureClientSsl(); > > > > // Unprotected resource > > - ByteChunk res = > > - getUrl("https://localhost:"; + getPort() + > > "/unprotected"); > > + ByteChunk res = getUrl("https://localhost:"; + getPort() + > > "/unprotected"); > > Assert.assertEquals("OK", res.toString()); > > > > // Protected resource > > res.recycle(); > > int rc = -1; > > try { > > - rc = getUrl("https://localhost:"; + getPort() + > > "/protected", res, > > - null, null); > > + rc = getUrl("https://localhost:"; + getPort() + > > "/protected", res, null, null); > > } catch (SocketException se) { > > if (!trustType.equals(TrustType.NONE)) { > > Assert.fail(se.getMessage()); > > diff --git a/test/org/apache/tomcat/util/net/TesterSupport.java > > b/test/org/apache/tomcat/util/net/TesterSupport.java > > index 49b8de7..37d69c8 100644 > > --- a/test/org/apache/tomcat/util/net/TesterSupport.java > > +++ b/test/org/apache/tomcat/util/net/TesterSupport.java > > @@ -64,6 +64,7 @@ import org.apache.tomcat.util.compat.JrePlatform; > > import org.apache.tomcat.util.descriptor.web.LoginConfig; > > import org.apache.tomcat.util.descriptor.web.SecurityCollection; > > import org.apache.tomcat.util.descriptor.web.SecurityConstraint; > > +import org.apache.tomcat.util.net > > <http://org.apache.tomcat.util.net>.SSLHostConfigCertificate.Type; > > > > public final class TesterSupport { > > > > @@ -137,47 +138,39 @@ public final class TesterSupport { > > protected static void initSsl(Tomcat tomcat, String keystore, > > String keystorePass, String keyPass) { > > > > + Connector connector = tomcat.getConnector(); > > + connector.setSecure(true); > > + Assert.assertTrue(connector.setProperty("SSLEnabled", > "true")); > > + > > + SSLHostConfig sslHostConfig = new SSLHostConfig(); > > + SSLHostConfigCertificate certificate = new > > SSLHostConfigCertificate(sslHostConfig, Type.UNDEFINED); > > + sslHostConfig.addCertificate(certificate); > > + connector.addSslHostConfig(sslHostConfig); > > + > > String protocol = > > tomcat.getConnector().getProtocolHandlerClassName(); > > if (!protocol.contains("Apr")) { > > - Connector connector = tomcat.getConnector(); > > String sslImplementation = > > System.getProperty("tomcat.test.sslImplementation"); > > if (sslImplementation != null && > > !"${test.sslImplementation}".equals(sslImplementation)) { > > StandardServer server = (StandardServer) > > tomcat.getServer(); > > AprLifecycleListener listener = new > > AprLifecycleListener(); > > listener.setSSLRandomSeed("/dev/urandom"); > > server.addLifecycleListener(listener); > > - > > tomcat.getConnector().setAttribute("sslImplementationName", > > sslImplementation); > > + connector.setAttribute("sslImplementationName", > > sslImplementation); > > } > > - Assert.assertTrue(connector.setProperty("sslProtocol", > > "tls")); > > - File keystoreFile = > > - new File(keystore); > > - connector.setAttribute("keystoreFile", > > - keystoreFile.getAbsolutePath()); > > - File truststoreFile = new File(CA_JKS); > > - connector.setAttribute("truststoreFile", > > - truststoreFile.getAbsolutePath()); > > + sslHostConfig.setSslProtocol("tls"); > > + certificate.setCertificateKeystoreFile(new > > File(keystore).getAbsolutePath()); > > + sslHostConfig.setTruststoreFile(new > > File(CA_JKS).getAbsolutePath()); > > if (keystorePass != null) { > > - connector.setAttribute("keystorePass", > keystorePass); > > + > > certificate.setCertificateKeystorePassword(keystorePass); > > } > > if (keyPass != null) { > > - connector.setAttribute("keyPass", keyPass); > > + certificate.setCertificateKeyPassword(keyPass); > > } > > } else { > > - File keystoreFile = new File( > > - LOCALHOST_RSA_CERT_PEM); > > - tomcat.getConnector().setAttribute("SSLCertificateFile", > > - keystoreFile.getAbsolutePath()); > > - keystoreFile = new File( > > - LOCALHOST_RSA_KEY_PEM); > > - > tomcat.getConnector().setAttribute("SSLCertificateKeyFile", > > - keystoreFile.getAbsolutePath()); > > - keystoreFile = new File( > > - CA_CERT_PEM); > > - > tomcat.getConnector().setAttribute("SSLCACertificateFile", > > - keystoreFile.getAbsolutePath()); > > - } > > - tomcat.getConnector().setSecure(true); > > - > > Assert.assertTrue(tomcat.getConnector().setProperty("SSLEnabled", > > "true")); > > + certificate.setCertificateFile(new > > File(LOCALHOST_RSA_CERT_PEM).getAbsolutePath()); > > + certificate.setCertificateKeyFile(new > > File(LOCALHOST_RSA_KEY_PEM).getAbsolutePath()); > > + sslHostConfig.setCaCertificateFile(new > > File(CA_CERT_PEM).getAbsolutePath()); > > + } > > } > > > > protected static KeyManager[] getUser1KeyManagers() throws > > Exception { > > @@ -266,7 +259,7 @@ public final class TesterSupport { > > * depend. Therefore, force these tests to use TLSv1.2 so > > that they pass > > * when running on TLSv1.3. > > */ > > - > > > Assert.assertTrue(tomcat.getConnector().setProperty("sslEnabledProtocols", > > Constants.SSL_PROTO_TLSv1_2)); > > + > > > > tomcat.getConnector().findSslHostConfigs()[0].setProtocols(Constants.SSL_PROTO_TLSv1_2); > > > > // Need a web application with a protected and unprotected > URL > > // No file system docBase required > > diff --git > > a/test/org/apache/tomcat/util/net/jsse/TesterBug50640SslImpl.java > > b/test/org/apache/tomcat/util/net/jsse/TesterBug50640SslImpl.java > > index 6865b9d..478bbfa 100644 > > --- a/test/org/apache/tomcat/util/net/jsse/TesterBug50640SslImpl.java > > +++ b/test/org/apache/tomcat/util/net/jsse/TesterBug50640SslImpl.java > > @@ -23,7 +23,6 @@ import org.apache.tomcat.util.net > > <http://org.apache.tomcat.util.net>.SSLUtil; > > > > public class TesterBug50640SslImpl extends JSSEImplementation { > > > > - public static final String PROPERTY_NAME = > "sslEnabledProtocols"; > > public static final String PROPERTY_VALUE = "magic"; > > > > > > > > > > --------------------------------------------------------------------- > > To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org > > <mailto:dev-unsubscr...@tomcat.apache.org> > > For additional commands, e-mail: dev-h...@tomcat.apache.org > > <mailto:dev-h...@tomcat.apache.org> > > > > > --------------------------------------------------------------------- > To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org > For additional commands, e-mail: dev-h...@tomcat.apache.org > >
