[PATCH v2 0/1] MR10174: bcryptprimitives: Abort if ProcessPrng fails
[`ProcessPrng`] is documented as always returning TRUE. So I've modified `ProcessPrng` to abort on errors. This is I believe an import aspect of the function. See Microsoft's [The Windows 10 random number generation infrastructure] whitepaper.
We also have the property that a request for random bytes never fails. In the past our RNG functions could return an error code. We have observed that there are many callers that never check for the error code, even if they are generating cryptographic key material. This can lead to serious security vulnerabilities if an attacker manages to create a situation in which the RNG infrastructure returns an error. For that reason, the Win10 RNG infrastructure will never return an error code and always produce high-quality random bytes for every request.
[`ProcessPrng`]: https://learn.microsoft.com/en-us/windows/win32/seccng/processprng [The Windows 10 random number generation infrastructure]: https://www.microsoft.com/en-us/security/blog/2019/11/25/going-in-depth-on-t... -- v2: bcryptprimitives: Abort if ProcessPrng fails https://gitlab.winehq.org/wine/wine/-/merge_requests/10174
From: Chris Denton <chris@chrisdenton.dev> `ProcessPrng` is documented as always returning TRUE[1]. So I've modified `ProcessPrng` to abort on errors. This is I believe an import aspect of the function. See Microsoft's "The Windows 10 random number generation infrastructure" whitepaper[2].
We also have the property that a request for random bytes never fails. In the past our RNG functions could return an error code. We have observed that there are many callers that never check for the error code, even if they are generating cryptographic key material. This can lead to serious security vulnerabilities if an attacker manages to create a situation in which the RNG infrastructure returns an error. For that reason, the Win10 RNG infrastructure will never return an error code and always produce high-quality random bytes for every request.
[1]: https://learn.microsoft.com/en-us/windows/win32/seccng/processprng [2]: https://www.microsoft.com/en-us/security/blog/2019/11/25/going-in-depth-on-t... --- dlls/bcryptprimitives/main.c | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/dlls/bcryptprimitives/main.c b/dlls/bcryptprimitives/main.c index 6562d672389..7be38f84b72 100644 --- a/dlls/bcryptprimitives/main.c +++ b/dlls/bcryptprimitives/main.c @@ -16,12 +16,24 @@ * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301, USA */ +#include <assert.h> #include <stdarg.h> +#include <stdlib.h> + #include "windef.h" #include "winbase.h" #include "ntsecapi.h" +#include "wine/debug.h" + +WINE_DEFAULT_DEBUG_CHANNEL(bcrypt); BOOL WINAPI ProcessPrng(BYTE *data, SIZE_T size) { - return RtlGenRandom(data, size); + if (!RtlGenRandom( data, size )) + { + /* ProcessPrng is documented as always returning TRUE so abort the process if it fails */ + ERR( "failed to generate random data\n" ); + abort(); + } + return TRUE; } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/10174
On Tue Feb 24 10:39:33 2026 +0000, Christopher Denton wrote:
changed this line in [version 2 of the diff](/wine/wine/-/merge_requests/10174/diffs?diff_id=246461&start_sha=ce921f1717ce4fd3b15ca86792f48fdd4e1bb091#2fb1afd848730dac705e8d369fa7be3de33a9e3a_35_35) I also slightly changed the style to more closely match the uses of `ERR` in `bcrypt_main.c`.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/10174#note_130342
This merge request was approved by Hans Leidekker. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/10174
Nikolay Sivov (@nsivov) commented about dlls/bcryptprimitives/main.c:
#include "windef.h" #include "winbase.h" #include "ntsecapi.h" +#include "wine/debug.h" + +WINE_DEFAULT_DEBUG_CHANNEL(bcrypt);
BOOL WINAPI ProcessPrng(BYTE *data, SIZE_T size) { - return RtlGenRandom(data, size); + if (!RtlGenRandom( data, size )) + { + /* ProcessPrng is documented as always returning TRUE so abort the process if it fails */ + ERR( "failed to generate random data\n" ); + abort(); + } Why would we ever want to abort here?
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/10174#note_130354
On Tue Feb 24 13:17:31 2026 +0000, Nikolay Sivov wrote:
Why would we ever want to abort here? You are thinking it should be `ExitProcess`? My reasoning is that it's intended as a security feature so continuing with the normal exit flow may not be appropriate.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/10174#note_130356
On Tue Feb 24 13:23:21 2026 +0000, Christopher Denton wrote:
You are thinking it should be `ExitProcess`? My reasoning is that it's intended as a security feature so continuing with the normal exit flow may not be appropriate. No, I think if you want to return success unconditionally, that's fine, but terminating isn't.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/10174#note_130357
On Tue Feb 24 13:53:44 2026 +0000, Nikolay Sivov wrote:
No, I think if you want to return success unconditionally, that's fine, but terminating isn't. Ok, I've updated it to simply return `TRUE`. From looking at `get_random` in system.c, it doesn't look like it can fail in any case (assuming the data buffer is writeable of course).
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/10174#note_130363
participants (4)
-
Chris Denton -
Christopher Denton (＠cdenton)
-
Hans Leidekker (＠hans)
-
Nikolay Sivov (＠nsivov)