New build option to allow reuse of the windows crypt provider handle … #8706
Conversation
|
Retest this please: "Found unhandled org.jenkinsci.plugins.workflow.support.steps.AgentOfflineException exception:" |
| @@ -2741,14 +2745,26 @@ int wc_GenerateSeed(OS_Seed* os, byte* output, word32 sz) | |||
| } | |||
| #endif /* HAVE_INTEL_RDSEED */ | |||
|
|
|||
| #ifdef WIN_REUSE_CRYPT_HANDLE | |||
There was a problem hiding this comment.
OS_Seed already contains ProviderHandle, wouldn't it be enough not to release it with CryptReleaseContext and init only when not initialised? This will avoid global static
There was a problem hiding this comment.
There could be multiple OS_Seed provider handles and without a global static there would be no way to properly cleanup handles.
| @@ -2711,6 +2711,10 @@ int wc_GenerateSeed(OS_Seed* os, byte* output, word32 sz) | |||
|
|
|||
| #elif defined(USE_WINDOWS_API) | |||
|
|
|||
| #ifdef WIN_REUSE_CRYPT_HANDLE | |||
There was a problem hiding this comment.
if it is safe optimisation, can it be enabled unconditionally without build option? If it is not and users must make choice what are consequences here?
There was a problem hiding this comment.
@redbaron , in order to consider making this default I'll have to do some more testing with multiple threads to confirm there aren't any issues.
wolfcrypt/src/random.c
Outdated
| CRYPT_VERIFYCONTEXT)) | ||
| return WINCRYPT_E; | ||
| } | ||
| os->handle = gHandle; |
There was a problem hiding this comment.
if WIN_REUSE_CRYPT_HANDLE is enabled os->handle seems to be unused and can be removed from OS_Seed
There was a problem hiding this comment.
Keeping for compatibility (structure needs a member)
wolfcrypt/src/random.c
Outdated
| @@ -2741,14 +2745,26 @@ int wc_GenerateSeed(OS_Seed* os, byte* output, word32 sz) | |||
| } | |||
| #endif /* HAVE_INTEL_RDSEED */ | |||
|
|
|||
| #ifdef WIN_REUSE_CRYPT_HANDLE | |||
| if (gHandle == 0) { | |||
There was a problem hiding this comment.
is wc_GenerateSeed supposed to be thread safe? if it is, then this check is not safe.
There was a problem hiding this comment.
Hi @redbaron , no this is not thread-safe and it will need fixed before this can be considered. It was already pointed out by a few other wolfSSL engineers.
…ws crypt provider handle. Seeding happens on any new RNG or after `WC_RESEED_INTERVAL`. If using threads make sure wolfSSL_Init() or wolfCrypt_Init() is called before spinning up threads. ZD 19754. Fixed minor implicit cast warnings in internal.c. Add missing `hpke.c` to wolfssl VS project.
|
Retest this please: "Found unhandled org.jenkinsci.plugins.workflow.support.steps.AgentOfflineException exception:" |
Description
New build option to allow reuse of the windows crypt provider handle (
WIN_REUSE_CRYPT_HANDLE). Documentation from Windows suggests using the same handle from different threads in the same process is safe whenCRYPT_VERIFYCONTEXTis used.Fixes ZD 19754
Testing
Tested on Windows 10 with Visual Studio 2022 with and without
WIN_REUSE_CRYPT_HANDLE.Checklist