[PATCH 0/1] MR10174: bcryptprimitives: Abort if ProcessPrng fails

[Christopher Denton (＠cdenton)]

[`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...

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/10174

[PATCH 1/1] bcryptprimitives: Abort if ProcessPrng fails

[Chris Denton]

From: Chris Denton <[e-mail]>

`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..f57c96c9c9a 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(bcryptprim);
 
 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("ProcessPrng failed to generate random data\n");
+        abort();
+    }
+    return TRUE;
 }
-- 
GitLab

https://gitlab.winehq.org/wine/wine/-/merge_requests/10174

[Hans Leidekker (＠hans)]

Hans Leidekker (@hans) commented about dlls/bcryptprimitives/main.c:
> * 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(bcryptprim);
> Please use 'bcrypt' channel.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/10174#note_130335

[Hans Leidekker (＠hans)]

Hans Leidekker (@hans) commented about dlls/bcryptprimitives/main.c:
> +#include <stdlib.h>
> +
>  #include "windef.h"
>  #include "winbase.h"
>  #include "ntsecapi.h"
> +#include "wine/debug.h"
> +
> +WINE_DEFAULT_DEBUG_CHANNEL(bcryptprim);
>
> 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("ProcessPrng failed to generate random data\n");
> No need to include the function name, it's added by the ERR() macro.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/10174#note_130336