Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Question] Custom TrustManager and performance impact ? #7862

Closed
sbernard31 opened this issue Apr 11, 2018 · 8 comments · Fixed by #8847
Closed

[Question] Custom TrustManager and performance impact ? #7862

sbernard31 opened this issue Apr 11, 2018 · 8 comments · Fixed by #8847

Comments

@sbernard31
Copy link

If I want to define a custom behavior to validate certificate, I should implement a custom X509TrustManager, right ?

Now imagine, that my TrustManager needs to do some "slow" operation like calling a micro service or maybe doing some OCSP check. As TrustManager is a sync API, I ask my self how would this impact netty "performance".

I know that Netty is design for Better throughput, lower latency so how could be the right way to manage this use case ?

Thx :)

@normanmaurer
Copy link
Member

normanmaurer commented Apr 11, 2018 via email

@sbernard31
Copy link
Author

Ok ok, but checking certificate using OCSP for example is not so eccentric and this could take time, So where should I do this kind of check if this is not in TrustManager ?

Thx again :)

@johnou
Copy link
Contributor

johnou commented Apr 11, 2018

@sbernard31
Copy link
Author

@johnou, maybe I missed something, but this is an example about how doing OCSP request, right ?
But my question is more about where to plug the OCSP validation in a server. The natural way seems to use a custom TrustManager, but as I suspected this could be an issue if OCSP request take too many time as TrustManager is a sync API.

Does it could make sense to add a new async API like TrustManager ?

@normanmaurer
Copy link
Member

@sbernard31 sadly enough there is not a good solution for "long-running" tasks during verification atm :(

@sbernard31
Copy link
Author

@normanmaurer thx for the answer. Should I close the issue ?

Is there plan about introduce a way to do that later ? (I can open an issue/improvement If you want ?)

@rkapsi
Copy link
Member

rkapsi commented May 1, 2018

To chime in here in regards OCSP. Normally it's the client that does the validation. The server fetches it out-of-band from the CA's OCSP responder server and caches it for up to the TTL that is configured in CA's response (I believe 10 days is the max). That is something you wouldn't want to do inline during the handshake.

As for the client side. If you were to write a Netty based SSL client and wanted to do OCSP validation that involves needing to reach out to the CA then it's indeed a problem. The underlying OpenSSL API is synchronous.

I believe a solution for dealing with this could be a dedicated SSL handhsake Thread pool so that the normal I/O event loop doesn't get blocked for other connections that are past the handshake (I think there is an open ticket for that).

normanmaurer added a commit that referenced this issue Feb 6, 2019
…en processing TLS / SSL via the SslHandler.

Motivation:

The SSLEngine does provide a way to signal to the caller that it may need to execute a blocking / long-running task which then can be offloaded to an Executor to ensure the I/O thread is not blocked. Currently how we handle this in SslHandler is not really optimal as while we offload to the Executor we still block the I/O Thread.

Modifications:

- Correctly support offloading the task to the Executor while suspending processing of SSL in the I/O Thread
- Add new methods to SslContext to specify the Executor when creating a SslHandler
- Remove @deprecated annotations from SslHandler constructor that takes an Executor
- Adjust tests to also run with the Executor to ensure all works as expected.

Result:

Be able to offload long running tasks to an Executor when using SslHandler. Partly fixes #7862 and #7020.
normanmaurer added a commit that referenced this issue Feb 8, 2019
…en processing TLS / SSL via the SslHandler.

Motivation:

The SSLEngine does provide a way to signal to the caller that it may need to execute a blocking / long-running task which then can be offloaded to an Executor to ensure the I/O thread is not blocked. Currently how we handle this in SslHandler is not really optimal as while we offload to the Executor we still block the I/O Thread.

Modifications:

- Correctly support offloading the task to the Executor while suspending processing of SSL in the I/O Thread
- Add new methods to SslContext to specify the Executor when creating a SslHandler
- Remove @deprecated annotations from SslHandler constructor that takes an Executor
- Adjust tests to also run with the Executor to ensure all works as expected.

Result:

Be able to offload long running tasks to an Executor when using SslHandler. Partly fixes #7862 and #7020.
normanmaurer added a commit that referenced this issue Feb 11, 2019
#8847)

Motivation:

The SSLEngine does provide a way to signal to the caller that it may need to execute a blocking / long-running task which then can be offloaded to an Executor to ensure the I/O thread is not blocked. Currently how we handle this in SslHandler is not really optimal as while we offload to the Executor we still block the I/O Thread.

Modifications:

- Correctly support offloading the task to the Executor while suspending processing of SSL in the I/O Thread
- Add new methods to SslContext to specify the Executor when creating a SslHandler
- Remove @deprecated annotations from SslHandler constructor that takes an Executor
- Adjust tests to also run with the Executor to ensure all works as expected.

Result:

Be able to offload long running tasks to an Executor when using SslHandler. Partly fixes #7862 and #7020.
normanmaurer added a commit that referenced this issue Feb 11, 2019
#8847)

Motivation:

The SSLEngine does provide a way to signal to the caller that it may need to execute a blocking / long-running task which then can be offloaded to an Executor to ensure the I/O thread is not blocked. Currently how we handle this in SslHandler is not really optimal as while we offload to the Executor we still block the I/O Thread.

Modifications:

- Correctly support offloading the task to the Executor while suspending processing of SSL in the I/O Thread
- Add new methods to SslContext to specify the Executor when creating a SslHandler
- Remove @deprecated annotations from SslHandler constructor that takes an Executor
- Adjust tests to also run with the Executor to ensure all works as expected.

Result:

Be able to offload long running tasks to an Executor when using SslHandler. Partly fixes #7862 and #7020.
@ZhenLian
Copy link
Contributor

ZhenLian commented Nov 4, 2019

@normanmaurer

Hello Norman, thanks for the excellent work! I've noticed that you had several following PRs to solve this problem, but I am still not super clear after reading the codes and comments in those PRs. May I know what exactly do I need to do in order to offload the work if I am using my custom TrustManager and KeyManager, and some of the operations such as peer cert selection or trust cert verification takes longer time? Currently, IIUC, I need to:

  • Set system property useTasks to true

  • Pass in another parameter Executor when creating SSLHandler

But it seems that I didn't specify which operations might take longer time yet. Can the system recognize the time-consuming tasks after certain amount of time and offload them "automatically"? I've read the code in ReferenceCountedOpenSslEngine and it looks like this is the case. Just want to double-check the above two steps are all the things I need to do.

Thanks again for your time!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants