SSL improvements

[vietj]

A set of improvements for SSL handling

• SSL configuration is performed
  • on a worker thread instead of event-loop thread
  • lazily in NetClient as validation is asynchronous
• SslContext is now delegated to an SPI to enable alternative way to create an SslContext (e.g reuse an existing SSLContext) provided on the SSLEngineOptions classes
• remove the suppliedSSLContext temporary machinery in favor of the SSLEngineOptions configuration mechanism
• ssl handler (SslHandler or SniHandler) is now created by the SSLHelper instead of the pipeline setup
• internal cleanups

The SPI will be documented in the Vert.x Advanced Guide

Thanks @MikeEdgar for the SSL blocking contribution

[vietj]

@kabir have a look, it changes the temporary supplied SSLContext machinery for a cleaner API, NetTest#testNetClientInternalTLS shows how the API can be used for NetClient. I will provide a PR for vertx-proton for this matter.

[kabir]

@vietj Looks good. My only concern is what the timeframe is to get this in? Since I have some work further down the chain (in SR Reactive Messaging) which needs merging/adjusting.

CC @cescoffier @ozangunalp

[vietj]

I think the next release. I'm working actively on it.

…

On Wed, Aug 31, 2022 at 11:25 AM Kabir Khan ***@***.***> wrote: @vietj <https://github.com/vietj> Looks good. My only concern is what the timeframe is to get this in? Since I have some work further down the chain (in SR Reactive Messaging) which needs merging/adjusting. CC @cescoffier <https://github.com/cescoffier> @ozangunalp <https://github.com/ozangunalp> — Reply to this email directly, view it on GitHub <#4468 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AABXDCWEICERUA6KOY5TVTDV34QKPANCNFSM6AAAAAAQAZVVLA> . You are receiving this because you were mentioned.Message ID: ***@***.***>

[kabir]

@vietj ok, great - the needed work in SmallRye Reactive Messaging will need updating. But since that isn't merged yet, assuming this comes soon-ish, I can update that PR.

[MikeEdgar]

@vietj - this looks good to me as it relates to issue #4439. I no longer see the blocked threads during application startup with this branch.

[vietj]

@kabir this is I think the most up to date PR for the feature

Now the vertx-proton and vertx-amqp-client changes are not necessary anymore as the SSLContext can be configured from the SSLEngineOptions, e.g

    AmqpClientOptions options = new AmqpClientOptions()
      .setSsl(true)
      .setHost("localhost")
      .setPort(server.actualPort())
      .setSslEngineOptions(new JdkSSLEngineOptions() {
        @Override
        public SslContextFactory sslContextFactory() {
          return new SslContextFactory() {
            @Override
            public SslContext create() throws SSLException {
              return new JdkSslContext(
                sslContext,
                true,
                null,
                IdentityCipherSuiteFilter.INSTANCE,
                ApplicationProtocolConfig.DISABLED,
                io.netty.handler.ssl.ClientAuth.NONE,
                null,
                false);
            }
          };
        }
      });

    AmqpClient client = AmqpClient.create(vertx, options);

After this is merged, I'll revert the changes in vertx-proton and vertx-amqp-client (but I'll keep the SSL tests).

[kabir]

Thanks @vietj - the changes look good to me

[InfoSec812]

I added some comments from my perspective as a user of the toolkit who is trying to learn more about the internals.

[InfoSec812]

Might be helpful to add JavaDoc to this method and the next?

[vietj]

will do

[InfoSec812]

I am having issues tracing through this method and understanding the reasoning. Any chance of adding comments or decomposing into smaller methods?

[vietj]

I'll add comments

[InfoSec812]

Thanks for the additional comments and documentation @vietj - Looks good!

[tsegismont]

@vietj great job