2

Is CryptGenRandom() thread-safe with a single global program-wide HCRYPTPROV instance?

MSDN appears to lack any info on this: https://msdn.microsoft.com/en-us/library/windows/desktop/aa379942(v=vs.85).aspx

Creating a separate HCRYPTPROV per thread and destroying it again would significantly complicate matters (and also risk more security-relevant bugs on my side), so this would be really useful to know. Sharing one global HCRYPTPROV would be a lot easier for sure.

So does anyone here know about the thread-safety of CryptGenRandom(), particularly with a single HCRYPTPROV instance?

E. T.
  • 847
  • 10
  • 26
  • Why don't you just lock it? Would be less complicated than separate instances. – Ajay Brahmakshatriya Sep 11 '17 at 08:23
  • Well, if you use a pool of a few app-lifetime threads, you could create one instance per thread and not need to continually create/destroy them? Would that not work for you? – Martin James Sep 11 '17 at 09:59
  • yes, this api call is thread-safe by fact. why you think that here can be some thread interference ? – RbMm Sep 11 '17 at 10:33
  • 2
    @RbMm probably because there is no explicit mention of thread-safety, and the OP does not realize that, in a preemptive multitasker like Windows, there would only be a mention of thread-safety if an API was thread-unsafe. Thread-safe must be, and is, the default in such a system. – Martin James Sep 11 '17 at 10:36
  • 1
    @AjayBrahmakshatriya: If you place a lock on a resource, that lock needs to go into your lock hierarchy (you do have a lock hierarchy, right?). Lock hierarchies are extremely difficult to implement properly, and require a lot of care when maintaining a system. Making sure that you do not require thread-safety by keeping non-thread-safe resources thread-local is by *far* the easier problem to solve. And a *lot* easier to maintain. – IInspectable Sep 11 '17 at 19:12
  • @AjayBrahmakshatriya I don't want to lock it for performance reasons. I don't want to create one HCRYPTPROV per thread either *unless* I really need to because it's too easy to have corner cases where one still hangs around and I leak HCRYPTPROV instances – E. T. Sep 11 '17 at 22:29
  • 4
    There *used* to be some discussion of thread safety, [see this archived article](https://web.archive.org/web/20101002143835/http://msdn.microsoft.com/en-us/library/aa388149(VS.85).aspx). I'm not sure why this was removed from MSDN, particularly since at least one other article still references it. Unfortunately it doesn't mention your scenario. – Harry Johnston Sep 11 '17 at 23:55
  • 2
    @RbMm, if you've examined the code, I'll take your word for it, but it isn't obvious to me that this particular call would necessarily be thread-safe, since it has to obtain the PRNG seed from common state. Are you certain that two simultaneous calls with the same handle might not return the same data, or data that is subtly correlated and therefore unsuitable for cryptographic use? – Harry Johnston Sep 12 '17 at 00:00
  • @Martin, I guess you trust MSDN rather more than I do. :-) Can you make any sense out of the archived article I link to above? – Harry Johnston Sep 12 '17 at 00:01
  • @HarryJohnston - really this is implementation defined, and exist different providers. even custom. so impossible exactly say how internal this implemented by concrete provider. but in general i not view serious basics think that this is somehow can be affected if several threads at once call this function (unlike for example when we work with key). for `BCryptGenRandom` also nothing said in msdn that it not safe in different threads call. in my test, when called at concurrent from different threads - anyway different random data returned to every thread. how minimum no any warning in docs – RbMm Sep 12 '17 at 00:29
  • 1
    so faster this is really *UB* because nothing clear said are safe and correct such use provider (state). if look for some (i look only random one from many) [implementation](https://prnt.sc/gk0yyy) - visible that critical sections internally used for access some states. but any way nothing prove. rather intuitively i guess that this will be thread safe – RbMm Sep 12 '17 at 00:54
  • and all well know ms providers internal use look like the same code (ProcessPrng) – RbMm Sep 12 '17 at 01:03
  • bit more compare msdn in this microsoft pdf - http://csrc.nist.gov/groups/STM/cmvp/documents/140-1/140sp/140sp2606.pdf . here mentioned `ProcessPrng` exported function, which do all job in user mode. in `BCryptGenRandom` description said about [`SystemPrng`](https://msdn.microsoft.com/en-us/library/windows/desktop/dd408060(v=vs.85).aspx) function.. but nothing clear about thread safe – RbMm Sep 12 '17 at 01:21

1 Answers1

2

Creating a separate HCRYPTPROV per thread doesn't make much sense. This is pointer to memory block from heap in all current implementations, primarily saved pointers to CSP entry points which used to call actual provider implementation (CPGenRandom in our case). The references themselves do not contain state of the CSP, unlike for example HCRYPTKEY which containing actual key state. So even if you create a separate HCRYPTPROV for every thread - this changes nothing.

There may be some global variables / data used by CSP internally during this call; this is however unknown as these would be implementation details. Of course we can serialize calls to CryptGenRandom in the code. However we cannot control that some other dll in our process also call CryptGenRandom concurrently. So serializing all calls to CryptGenRandom also impossible.

As result I think the CPGenRandom must be design to be thread-safe. and it my tests with a well known Microsoft CSP this is true. Internal synchronization is used in function, when need access global data and if multiple threads call CPGenRandom concurrently; every thread receives unique random data.

So my conclusion - CryptGenRandom is thread-safe, at least for all Microsoft CSP

Maarten Bodewes
  • 90,524
  • 13
  • 150
  • 263
RbMm
  • 31,280
  • 3
  • 35
  • 56
  • Thanks very much for this detailed analysis with very sound reasoning. This makes me very confident that this answer is indeed correct. Thanks! – E. T. Sep 13 '17 at 16:12