Request to support Hot Reload of Cipher Suites about javalin HOT 14 CLOSED

Thank you for providing the context, I makes more sense now.

I have a WIP pull requests here: Hakky54/sslcontext-kickstart#465
Please have a look at it, I am curious what both of your opinion is on the usability of it.

@zugazagoitia What do you think of updating those properties? Would you like me to hide it away behind the SSLFactoryUtils#reload method or do you think getting the sslparameters from the sslfactory and setting the properties there like the example in the PR is better for you/Javalin? The only downside of hiding behind the SSLFactoryUtils#reload will be that a new SSLFactory is needed when just only the cipher needs to be adjusted which does not make sense to me... What do you think?

I personally have no opinion on this, however you choose to implement it will be fine, I'll adapt to it. Thank you very much for the work!

from javalin.

I just released version 8.3.3 which contains the reloadable ciphers

from javalin.

@Hakky54 is this feasible using your library?

from javalin.

The current version does not support it, but I can add this feature for the next version. I never added it previously as I thought nobody would use that kind of option. Just out of curiosity, can you explain your use case @venkateshdomakonda I am wondering why you would require such an option at runtime. With key and trust material I can understand it needs to be reloaded, because they can expire. But not sure why the reloading of ciphers can be useful. Looking forward to your response

from javalin.

Hello @Hakky54 ,

Thank you for considering adding support for runtime configuration of cipher suites in the next version. The ability to dynamically adjust cipher suites at runtime can greatly enhance flexibility and security in various deployment scenarios. Use case for runtime cipher suite configuration is in environments where specific security requirements or compliance standards dictate the use of particular cipher suites.

For instance, in our telco applications, there are security protocols that need to be followed. Being able to fine-tune cipher suite selection on certain interfaces or endpoints based on these requirements can ensure that the application remains compliant.

from javalin.

Thank you for providing the context, I makes more sense now.

I have a WIP pull requests here: Hakky54/sslcontext-kickstart#465
Please have a look at it, I am curious what both of your opinion is on the usability of it.

@zugazagoitia What do you think of updating those properties? Would you like me to hide it away behind the SSLFactoryUtils#reload method or do you think getting the sslparameters from the sslfactory and setting the properties there like the example in the PR is better for you/Javalin? The only downside of hiding behind the SSLFactoryUtils#reload will be that a new SSLFactory is needed when just only the cipher needs to be adjusted which does not make sense to me... What do you think?

from javalin.

Hey, @Hakky54, I'm currently implementing this, but I'm unsure that Jetty will allow us to do it. See this.

from javalin.

It seems like it won't be that easy to have it working for Jetty. I am investigating the possibilities. In my earlier tests I have only tested it implicitly with Netty and it worked and to be honest I forgot to test it on a Jetty server...

Jetty's internal server configuration with ssl is slightly different and I need to reverse engineer it to understand how it works. It will take some time, so I hope to ping you back in couple of days.

from javalin.

I found a way to get it working. Currently testing in-dept different cases to validate and it seems promising.

So basically the link which you have shared, here will be used as a base ciphers/protocols and for every connection Jetty server will create a new SSLEngine or SSLServerSocket and apply the base ciphers and protocols and other settings. See here:

• https://github.com/jetty/jetty.project/blob/2067ec81e8e947a7e65955e04cc2b8d637bfbbd9/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/ssl/SslContextFactory.java#L1646
• https://github.com/jetty/jetty.project/blob/2067ec81e8e947a7e65955e04cc2b8d637bfbbd9/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/ssl/SslContextFactory.java#L1658
• https://github.com/jetty/jetty.project/blob/2067ec81e8e947a7e65955e04cc2b8d637bfbbd9/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/ssl/SslContextFactory.java#L1827
• https://github.com/jetty/jetty.project/blob/2067ec81e8e947a7e65955e04cc2b8d637bfbbd9/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/ssl/SslContextFactory.java#L1848
• https://github.com/jetty/jetty.project/blob/2067ec81e8e947a7e65955e04cc2b8d637bfbbd9/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/ssl/SslContextFactory.java#L1877
• https://github.com/jetty/jetty.project/blob/2067ec81e8e947a7e65955e04cc2b8d637bfbbd9/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/ssl/SslContextFactory.java#L1891

With the changes from this PR Hakky54/sslcontext-kickstart#468 it should work. Can you give it a try on your side? Can you maybe build the repo locally and try the snapshot version and ping here whether it also works on your side before I make a new release?

from javalin.

The fix is now available in version 8.3.4 Can you retry on your side? An integration test is available which tests this option with just plain jetty server, here: https://github.com/Hakky54/sslcontext-kickstart/blob/64f9194c430cc8797155f22124c2824ada891ba1/sslcontext-kickstart/src/test/java/nl/altindag/ssl/SSLFactoryIT.java#L455

from javalin.

Thank you! I haven't had the time to try during the week, I will see about doing it this weekend.

from javalin.

Hi again @Hakky54 , sorry for the delay in this reply...

When testing locally (corretto-21, aarch64, macOS 14) I get:

[JettyServerThreadPool-1068] WARN org.eclipse.jetty.io.ManagedSelector - Could not accept java.nio.channels.SocketChannel[closed]: java.lang.IllegalStateException: Connection rejected: No ALPN Processor for nl.altindag.ssl.sslengine.FenixSSLEngine from [org.eclipse.jetty.alpn.java.server.JDK9ServerALPNProcessor@41217b62]

You can check the sources to replicate here (mvn test on the parent pom)

Which makes a lot of sense:
https://github.com/jetty/jetty.project/blob/31581cf7e2e62d8d8fe25a8daaa4569bee3b60ec/jetty-alpn/jetty-alpn-java-server/src/main/java/org/eclipse/jetty/alpn/java/server/JDK9ServerALPNProcessor.java#L43
And was motivated by: jetty/jetty.project#215

Someone would need to provide the Jetty ALPN implementation supporting your FenixSSLEngine. Would it make sense for an extension with the appliesTo(SSLEngine sslEngine) method overridden? Since this is starting to get a bit too "hacky" (pun intended) I would consider dropping support for this altogether on our side, a change of ciphers warrants a server restart IMO.

This change has not been implemented on Javalin, but for anyone using HTTP/2 (ALPN) alongside this feature in your plugin it will be breaking.

UPDATE: You can check the failing build here: https://github.com/zugazagoitia/javalin/actions/runs/8680529665

from javalin.

In my opinion this is an edge case to have the cipher suite reloadable. I can understand the OP that this feature will be useful for him. I liked the challange and added it to the library, however it is already a bit hacky on the library side as I created wrapper classes to get it working, however these wrapper classes are compatible with Java 8. So newer methods in later Java versions won't be compatible within the wrapper class. So even though I added the capability it has some drawback/limitations. I was expecting some limitations/issues already and it seems like that is now indeed the case.

If it would have worked out of the box on your side I would advise to just go for it, however it seems like you also need to do some hacky stuff to get it working and I don't think that would be a good way to proceed with this feature. Maintaining the hacky solution is also not very pleasent as it tends to be more error prone.

Looking at the source code I think having a wrapper class of JDK9ServerALPNProcessor which overrides appliesTo method would maybe do the trick. But I think having Jetty adjusting their code would be better instead of us creating wrappers and hacky solution.

So I think it is indeed better not to support this feature in Javalin, however it is up to you to decide. I will add a disclaimer in the library that using this kind of feature will have drawback/limitations.

Thank you @zugazagoitia , nice to see you put some effort into it and testing it!
And thank you @venkateshdomakonda for coming up this with very challanging feature request!

from javalin.

Closing as not planned due to the complexity of the implementation, it is not worth the added value.

from javalin.