Re: [PR] Simplify usage of custom ssl configuration [tomcat]
Hakky54 commented on PR #706: URL: https://github.com/apache/tomcat/pull/706#issuecomment-2212556516 After couple of review rounds it is not clear whether it would get approved/merged, so I decided to close this PR. Please ping me if you guys still consider and and I will create a new PR. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
Re: [PR] Simplify usage of custom ssl configuration [tomcat]
Hakky54 closed pull request #706: Simplify usage of custom ssl configuration URL: https://github.com/apache/tomcat/pull/706 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
Re: [PR] Simplify usage of custom ssl configuration [tomcat]
Hakky54 commented on PR #706: URL: https://github.com/apache/tomcat/pull/706#issuecomment-2173747924 The wrapper class is package protected, so no one can access it. It can only be created using the sslutil which returns the interface. I am not sure whether sslutil is the right place and whether the sslutil is part of the public api. What do you think? -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
Re: [PR] Simplify usage of custom ssl configuration [tomcat]
ChristopherSchultz commented on PR #706: URL: https://github.com/apache/tomcat/pull/706#issuecomment-2173636514 > > The constructor captures its arguments and then ignores them forever after that. Why bother capturing them in the first place? > > Well actually it is using the KeyManager and TrustManager, see here: https://github.com/apache/tomcat/pull/706/files#diff-8ed2a43a8b2f354b707c0fdb8cd5b794e5a476ecbf603b2ba69af5eea18b3cc4R73-R81 Oops, I seem to have totally missed that part of the code. My apologies. > So the reloading of tomcat was just an example but I use it also for different use cases, such as: > > * Combining custom truststore, cacert and System keystore as a TrustManager > * Fetching certificates as pem from a database and constructing the KeyManager and TrustManager > * Using a custom TrustManager which can prompt when the certificate is not trusted yet and whether it needs to be trusted, ss it can be added to the exusting list of trusted certificates Really, using any custom SSLContext for whatever reason is a valid use case. It's not reasonable for Tomcat to provide all of these various combinations of features, so extensibility is certainly useful. I think the only question is whether this wrapper is really useful to ship with Tomcat. It's certainly not useful *outside* of Tomcat since it uses Tomcat's internal interface. But it does bridge the gap between Java-provided APIs and Tomcat's APIs. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
Re: [PR] Simplify usage of custom ssl configuration [tomcat]
Hakky54 commented on PR #706: URL: https://github.com/apache/tomcat/pull/706#issuecomment-2168908756 > This wrapper class appears to specifically discard the key managers and trust managers. I used my own example from my own project to demonstrate the usage of it within the PR description, however the code changes are slightly different. It has a constructor requireding the SSLContext, KeyManager and TrustManager. > The constructor captures its arguments and then ignores them forever after that. Why bother capturing them in the first place? Well actually it is using the KeyManager and TrustManager, see here: https://github.com/apache/tomcat/pull/706/files#diff-8ed2a43a8b2f354b707c0fdb8cd5b794e5a476ecbf603b2ba69af5eea18b3cc4R73-R81 So it is using all of the objects from the constructor even the SSLContext itself. It just acts as a wrapper to simplifies the usage of a custom `org.apache.tomcat.util.net.SSLContext` > If the goal is to allow "instant" reloading of the SSL configuration... that capability already exists in Tomcat. In my pull request description I mentioned the usage of ssl reloading, but this was an example to demonstrate how I used it with the resulting wrapper which I needed to add to provide a custom ssl configuration. So I wanted to point out there that the developer who want's to have a custom ssl configurationn always needs to create a wrapper on their side or else it won't work. So the reloading of tomcat was just an example but I use it also for different use cases, such as: - Combining custom truststore, cacert and System keystore as a TrustManager - Fetching certificates as pem from a database and constructing the KeyManager and TrustManager - Using a custom TrustManager which can prompt when the certificate is not trusted yet and whether it needs to be trusted, ss it can be added to the exusting list of trusted certificates - Managing ssl sessions It might be that I am the only developer which is working in this kind of edge cases... So all of these scenario's are working for me already actually while using the [wrapper class](https://github.com/apache/tomcat/pull/706/files#diff-8ed2a43a8b2f354b707c0fdb8cd5b794e5a476ecbf603b2ba69af5eea18b3cc4), I thought it would be nice to move it to apache tomcat to simplify the usage of it. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
Re: [PR] Simplify usage of custom ssl configuration [tomcat]
ChristopherSchultz commented on PR #706: URL: https://github.com/apache/tomcat/pull/706#issuecomment-2168824675 This wrapper class appears to specifically discard the key managers and trust managers. The constructor captures its arguments and then ignores them forever after that. Why bother capturing them in the first place? If the goal is to allow "instant" reloading of the SSL configuration... that capability already exists in Tomcat. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
Re: [PR] Simplify usage of custom ssl configuration [tomcat]
Hakky54 commented on PR #706: URL: https://github.com/apache/tomcat/pull/706#issuecomment-2168789527 Hi guys, any news regarding this topic? -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
Re: [PR] Simplify usage of custom ssl configuration [tomcat]
rmaucher commented on PR #706: URL: https://github.com/apache/tomcat/pull/706#issuecomment-2049169784 I had left the PR open since others could have been willing to go through with it (or not, I don't know). -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
Re: [PR] Simplify usage of custom ssl configuration [tomcat]
Hakky54 commented on PR #706: URL: https://github.com/apache/tomcat/pull/706#issuecomment-2049130432 Ah so your feeling is that this also might cause some regression while this wrapper does not add that much value to the project itself. I can understand that. Okay, thank you for your time for reviewing this PR. Let's close it then. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
Re: [PR] Simplify usage of custom ssl configuration [tomcat]
Hakky54 closed pull request #706: Simplify usage of custom ssl configuration URL: https://github.com/apache/tomcat/pull/706 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
Re: [PR] Simplify usage of custom ssl configuration [tomcat]
rmaucher commented on PR #706: URL: https://github.com/apache/tomcat/pull/706#issuecomment-2049093782 Your previous PR, which was integrated, unfortunately caused a very high number of regressions that had to be fixed over multiple Tomcat releases. This PR similarly seems uninteresting to me, so as a result I will not integrate it. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
Re: [PR] Simplify usage of custom ssl configuration [tomcat]
Hakky54 commented on PR #706: URL: https://github.com/apache/tomcat/pull/706#issuecomment-2049034933 Hi @markt-asf What do you think of this PR, would it make sense to have this kind of wrapper, or does it needs to be adjusted or would you like me to close it and disregard it? Looking forward to get your feedback on this -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[PR] Simplify usage of custom ssl configuration [tomcat]
Hakky54 opened a new pull request, #706:
URL: https://github.com/apache/tomcat/pull/706
This PR is a followup of the following earlier PR
https://github.com/apache/tomcat/pull/673 Although that pull request didn't get
merged, the code changes has been comitted to the main branch by the main
developer, see here for the specific commit:
https://github.com/apache/tomcat/commit/b0df9819c8d130adab0490b89dce1ab4ca6a3448
**Context**
With the earlier commit it is now possible to programatically configure the
ssl configuration of tomcat instead of using properties and delegating to
tomcat to construct the ssl configuration. This opens the possibility of
reloading the ssl configuration or other sorts of customizations.
I want to provide an example with my own code to give a better
understanding, the project is also available here: [Instant SSL Reloading with
Tomcat](https://github.com/Hakky54/java-tutorials/tree/main/instant-ssl-reloading-with-spring-tomcat)
**SSLContext wrapper**
```java
import nl.altindag.ssl.SSLFactory;
import org.apache.tomcat.util.net.SSLContext;
import javax.net.ssl.KeyManager;
import javax.net.ssl.SSLEngine;
import javax.net.ssl.SSLParameters;
import javax.net.ssl.SSLServerSocketFactory;
import javax.net.ssl.SSLSessionContext;
import javax.net.ssl.TrustManager;
import java.security.SecureRandom;
import java.security.cert.X509Certificate;
public final class TomcatSSLContext implements SSLContext {
private final SSLFactory sslFactory;
public TomcatSSLContext(SSLFactory sslFactory) {
this.sslFactory = sslFactory;
}
@Override
public void init(KeyManager[] kms, TrustManager[] tms, SecureRandom sr) {
// not needed to initialize as it is already initialized
}
@Override
public void destroy() {
}
@Override
public SSLSessionContext getServerSessionContext() {
return sslFactory.getSslContext().getServerSessionContext();
}
@Override
public SSLEngine createSSLEngine() {
return sslFactory.getSSLEngine();
}
@Override
public SSLServerSocketFactory getServerSocketFactory() {
return sslFactory.getSslServerSocketFactory();
}
@Override
public SSLParameters getSupportedSSLParameters() {
return sslFactory.getSslParameters();
}
@Override
public X509Certificate[] getCertificateChain(String alias) {
return sslFactory.getKeyManager()
.map(keyManager -> keyManager.getCertificateChain(alias))
.orElseThrow();
}
@Override
public X509Certificate[] getAcceptedIssuers() {
return sslFactory.getTrustedCertificates().toArray(new
X509Certificate[0]);
}
}
```
```java
import nl.altindag.ssl.SSLFactory;
import org.apache.catalina.connector.Connector;
import org.apache.coyote.http11.AbstractHttp11Protocol;
import org.apache.tomcat.util.net.SSLHostConfig;
import org.apache.tomcat.util.net.SSLHostConfigCertificate;
import org.apache.tomcat.util.net.SSLHostConfigCertificate.Type;
import org.springframework.beans.factory.annotation.Value;
import
org.springframework.boot.web.embedded.tomcat.TomcatConnectorCustomizer;
import org.springframework.context.annotation.Configuration;
@Configuration
public class SSLConnectorCustomizer implements TomcatConnectorCustomizer {
private final SSLFactory sslFactory;
private final int port;
public SSLConnectorCustomizer(SSLFactory sslFactory,
@Value("${server.port}") int port) {
this.sslFactory = sslFactory;
this.port = port;
}
@Override
public void customize(Connector connector) {
connector.setScheme("https");
connector.setSecure(true);
connector.setPort(port);
AbstractHttp11Protocol protocol = (AbstractHttp11Protocol)
connector.getProtocolHandler();
protocol.setSSLEnabled(true);
SSLHostConfig sslHostConfig = new SSLHostConfig();
SSLHostConfigCertificate certificate = new
SSLHostConfigCertificate(sslHostConfig, Type.UNDEFINED);
certificate.setSslContext(new TomcatSSLContext(sslFactory));
sslHostConfig.addCertificate(certificate);
protocol.addSslHostConfig(sslHostConfig);
}
}
```
**Problem statement**
Boilerplate code is needed by the end-user to provide a custom ssl
configuration. Tomcat takes a custom SSLContext, the full name is
`org.apache.tomcat.util.net.SSLContext` while the end-user has
`javax.net.ssl.SSLContext`. So the end-user is required to create an
implementation of `org.apache.tomcat.util.net.SSLContext` which acts as a
wrapper. This sslcontext needs to be passed to
