test,win: speedup tls-server-verify #1836
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
cc @nodejs/crypto |
Fishrock123
added
windows
|
<3 |
|
what are the chances of getting these upstreamed to openssl? |
|
@rvagg No chance because this makes a client connection be in a low entropy state. This change only applied to |
|
@shigeki Why is it a low entropy? RAND_poll(CryptGenRandom) still gets called and for the CI the screen content is always the same, so not much entropy added in this case. |
| @@ -446,6 +447,10 @@ static void sc_usage(void) | |||
| " -keymatexport label - Export keying material using label\n"); | |||
| BIO_printf(bio_err, | |||
| " -keymatexportlen len - Export len bytes of keying material (default 20)\n"); | |||
| #ifdef OPENSSL_SYS_WINDOWS | |||
| BIO_printf(bio_err, | |||
| " -no-rand-screen - Do not use RAND_screen() to initialize random state\n"); | |||
There was a problem hiding this comment.
Nitpick: name it -no_rand_screen for consistency with other options.
|
@mathiask88 I should have written it as "lower than the current". I don't think it is absolutely low for RAND_add with CryptGenRandom but I can't say it is enough high without RAND_screen for general uses. In CI, I agree that we need not it for this server-verify tests. |
|
@orangemocha According to Ben's comments, my commits were updated in shigeki@833b236 , shigeki@9f0f7c3 and shigeki@2ff517e . |
OpenSSL s_client introduces some delay on Windows. With all clients running sequentially, this delay is big enough to break CI. This fix runs the clients in parallel (unless the test includes renegotiation), reducing the total run time. (cherry picked from commit 12d4ea3c8ae7f8011de9ca9f50bdc08ee8cfb8bf)
Different servers must use different ports. Since we can count only on common.PORT and common.PORT+1, run only 2 servers in parallel. (cherry picked from commit efb0075ee0e1f4123974fe5b74ded8727bad4a9e)
When running in parallel, it is not easy to identify what server and client failed when the test fails. This adds identifiers to all lines of console output. (cherry picked from commit 29c651522df2f47c4360660264eb857fb6232b23)
For better performance of the test, the parent kills child processes so as not to wait them to be ended. (cherry picked from commit 833b23636045f7afc929196139021630a390391a)
In openssl s_client on Windows, RAND_screen() is invoked to initialize random state but it takes several seconds in each connection. This added -no_rand_screen to openssl s_client on Windows to skip RAND_screen() and gets a better performance in the unit test of test-tls-server-verify. Do not enable this except to use in the unit test. (cherry picked from commit 9f0f7c38e6df975dd39735d0e9ef968076369c74)
This improves the performance of openssl s_client on Windows and gains several seconds to finish test-tls-server-verify. (cherry picked from commit 2ff517e0e410ea33ba5a3d289a82fc315d120e8e)
joaocgreis
force-pushed
the
joaocgreis-io-tls-server-verify
branch
from
ec14721
to
e809f01
Compare
|
@bnoordhuis Thanks for your comments, I updated my commits. I also picked the updated commits from @shigeki . |
|
Thanks, @bnoordhuis . I will land this shortly. |
joaocgreis
added a commit
to JaneaSystems/node
that referenced
this pull request
Jun 3, 2015
OpenSSL s_client introduces some delay on Windows. With all clients running sequentially, this delay is big enough to break CI. This fix runs the clients in parallel (unless the test includes renegotiation), reducing the total run time. Fixes: nodejs#1461 PR-URL: nodejs#1836 Reviewed-By: Ben Noordhuis <[email protected]>
joaocgreis
added a commit
to JaneaSystems/node
that referenced
this pull request
Jun 3, 2015
Different servers must use different ports. Since we can count only on common.PORT and common.PORT+1, run only 2 servers in parallel. Fixes: nodejs#1461 PR-URL: nodejs#1836 Reviewed-By: Ben Noordhuis <[email protected]>
MylesBorins
pushed a commit
that referenced
this pull request
Dec 7, 2017
In openssl s_client on Windows, RAND_screen() is invoked to initialize random state but it takes several seconds in each connection. This added -no_rand_screen to openssl s_client on Windows to skip RAND_screen() and gets a better performance in the unit test of test-tls-server-verify. Do not enable this except to use in the unit test. Fixes: #1461 PR-URL: #1836 Reviewed-By: Ben Noordhuis <[email protected]>
evanlucas
pushed a commit
that referenced
this pull request
Dec 7, 2017
In openssl s_client on Windows, RAND_screen() is invoked to initialize random state but it takes several seconds in each connection. This added -no_rand_screen to openssl s_client on Windows to skip RAND_screen() and gets a better performance in the unit test of test-tls-server-verify. Do not enable this except to use in the unit test. Fixes: #1461 PR-URL: #1836 Reviewed-By: Ben Noordhuis <[email protected]>
This was referenced Dec 7, 2017
Merged
Merged
Merged
2 tasks
shigeki
pushed a commit
to shigeki/node
that referenced
this pull request
Mar 27, 2018
In openssl s_client on Windows, RAND_screen() is invoked to initialize random state but it takes several seconds in each connection. This added -no_rand_screen to openssl s_client on Windows to skip RAND_screen() and gets a better performance in the unit test of test-tls-server-verify. Do not enable this except to use in the unit test. Fixes: nodejs#1461 PR-URL: nodejs#1836 Reviewed-By: Ben Noordhuis <[email protected]>
MylesBorins
pushed a commit
that referenced
this pull request
Mar 28, 2018
In openssl s_client on Windows, RAND_screen() is invoked to initialize random state but it takes several seconds in each connection. This added -no_rand_screen to openssl s_client on Windows to skip RAND_screen() and gets a better performance in the unit test of test-tls-server-verify. Do not enable this except to use in the unit test. Backport-PR-URL: #19638 Fixes: #1461 PR-URL: #1836 Reviewed-By: Ben Noordhuis <[email protected]>
shigeki
pushed a commit
to shigeki/node
that referenced
this pull request
Aug 15, 2018
In openssl s_client on Windows, RAND_screen() is invoked to initialize random state but it takes several seconds in each connection. This added -no_rand_screen to openssl s_client on Windows to skip RAND_screen() and gets a better performance in the unit test of test-tls-server-verify. Do not enable this except to use in the unit test. Fixes: nodejs#1461 PR-URL: nodejs#1836 Reviewed-By: Ben Noordhuis <[email protected]>
This was referenced Aug 16, 2018
Merged
rvagg
pushed a commit
that referenced
this pull request
Nov 23, 2018
In openssl s_client on Windows, RAND_screen() is invoked to initialize random state but it takes several seconds in each connection. This added -no_rand_screen to openssl s_client on Windows to skip RAND_screen() and gets a better performance in the unit test of test-tls-server-verify. Do not enable this except to use in the unit test. Fixes: #1461 PR-URL: #1836 Reviewed-By: Ben Noordhuis <[email protected]>
rvagg
pushed a commit
that referenced
this pull request
Nov 23, 2018
In openssl s_client on Windows, RAND_screen() is invoked to initialize random state but it takes several seconds in each connection. This added -no_rand_screen to openssl s_client on Windows to skip RAND_screen() and gets a better performance in the unit test of test-tls-server-verify. Do not enable this except to use in the unit test. Fixes: #1461 PR-URL: #1836 Reviewed-By: Ben Noordhuis <[email protected]>
rvagg
pushed a commit
that referenced
this pull request
Nov 24, 2018
In openssl s_client on Windows, RAND_screen() is invoked to initialize random state but it takes several seconds in each connection. This added -no_rand_screen to openssl s_client on Windows to skip RAND_screen() and gets a better performance in the unit test of test-tls-server-verify. Do not enable this except to use in the unit test. Fixes: #1461 PR-URL: #1836 Reviewed-By: Ben Noordhuis <[email protected]>
shigeki
pushed a commit
to shigeki/node
that referenced
this pull request
Feb 26, 2019
In openssl s_client on Windows, RAND_screen() is invoked to initialize random state but it takes several seconds in each connection. This added -no_rand_screen to openssl s_client on Windows to skip RAND_screen() and gets a better performance in the unit test of test-tls-server-verify. Do not enable this except to use in the unit test. Fixes: nodejs#1461 PR-URL: nodejs#1836 Reviewed-By: Ben Noordhuis <[email protected]>
sam-github
pushed a commit
to sam-github/node
that referenced
this pull request
Sep 5, 2019
In openssl s_client on Windows, RAND_screen() is invoked to initialize random state but it takes several seconds in each connection. This added -no_rand_screen to openssl s_client on Windows to skip RAND_screen() and gets a better performance in the unit test of test-tls-server-verify. Do not enable this except to use in the unit test. Fixes: nodejs#1461 PR-URL: nodejs#1836 Reviewed-By: Ben Noordhuis <[email protected]>
BethGriggs
pushed a commit
that referenced
this pull request
Sep 19, 2019
In openssl s_client on Windows, RAND_screen() is invoked to initialize random state but it takes several seconds in each connection. This added -no_rand_screen to openssl s_client on Windows to skip RAND_screen() and gets a better performance in the unit test of test-tls-server-verify. Do not enable this except to use in the unit test. Fixes: #1461 Backport-PR-URL: #28230 PR-URL: #1836 Reviewed-By: Ben Noordhuis <[email protected]>
Merged
abhishekumar-tyagi
pushed a commit
to abhishekumar-tyagi/node
that referenced
this pull request
May 5, 2024
In openssl s_client on Windows, RAND_screen() is invoked to initialize random state but it takes several seconds in each connection. This added -no_rand_screen to openssl s_client on Windows to skip RAND_screen() and gets a better performance in the unit test of test-tls-server-verify. Do not enable this except to use in the unit test. Fixes: nodejs/node#1461 PR-URL: nodejs/node#1836 Reviewed-By: Ben Noordhuis <[email protected]>
abhishekumar-tyagi
pushed a commit
to abhishekumar-tyagi/node
that referenced
this pull request
May 5, 2024
In openssl s_client on Windows, RAND_screen() is invoked to initialize random state but it takes several seconds in each connection. This added -no_rand_screen to openssl s_client on Windows to skip RAND_screen() and gets a better performance in the unit test of test-tls-server-verify. Do not enable this except to use in the unit test. Fixes: nodejs/node#1461 PR-URL: nodejs/node#1836 Reviewed-By: Ben Noordhuis <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Addresses #1461 (and nodejs/node-v0.x-archive#25279 once ported there).
With the -no-rand-screen changes, the test is now much much faster (less than 1s on my machine). I am not sure however if speeding up a test is a good justification for adding a floating patch in openssl. I added all the commits in here to get more feedback.