[libcxx] random_device for Windows needs more storage

classic Classic list List threaded Threaded
13 messages Options
Reply | Threaded
Open this post in threaded view
|

[libcxx] random_device for Windows needs more storage

Yaron Keren
I'm trying to implement random_device for Windows using the CryptGenRandom API, as in the attached non-working code.

The trouble is, I need to store a crypto handle sized 64 bits in random_device whereas it has only an int (likely less than 64 bits) private member for storage.

Is it OK to modify the private member __f_ type from int to uint64_t?

A uint64_t it can hold both the crypto handle on Windows and file handles on Linux.

Yaron
 

_______________________________________________
cfe-dev mailing list
[hidden email]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev

random.cpp (2K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [libcxx] random_device for Windows needs more storage

Richard-2

In article <[hidden email]>,
    Yaron Keren <[hidden email]> writes:

> Is it OK to modify the private member __f_ type from int to uint64_t?

Wouldn't intptr_t or uintptr_t be more appropriate?

"Handles" in Windows are more likely to be pointers masquerading as an
integer for opacity purposes.
--
"The Direct3D Graphics Pipeline" free book <http://tinyurl.com/d3d-pipeline>
     The Computer Graphics Museum <http://computergraphicsmuseum.org>
         The Terminals Wiki <http://terminals.classiccmp.org>
  Legalize Adulthood! (my blog) <http://legalizeadulthood.wordpress.com>
_______________________________________________
cfe-dev mailing list
[hidden email]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev
Reply | Threaded
Open this post in threaded view
|

Re: [libcxx] random_device for Windows needs more storage

Yaron Keren
The actual data type is typedef ULONG_PTR HCRYPTPROV;
I found two conflicting descriptions in microsoft.com for ULONG_PTR:



#if defined(_WIN64)
 typedef unsigned __int64 ULONG_PTR;
#else
 typedef unsigned long ULONG_PTR;
#endif
The second http://msdn.microsoft.com/en-us/library/cc230394.aspx says it's essentially int, 32 bit or 64 bit.

Looking in the MingW header file basestd.h, ULONG_PTR is defined to either __int64 in WIN64 or long in WIN32, so it is always 64 bit.

uintptr_t will be 32 bits in WIN32.

The ideal would be to use the correct typdef HCRYPTPROV but this would require exposing it in random header which is probably wrong. So my question is if private member __f_ must be an int or is it an implementation detail and a uint64_t will be OK.

Yaron


2013/10/7 Richard <[hidden email]>

In article <[hidden email]>,
    Yaron Keren <[hidden email]> writes:

> Is it OK to modify the private member __f_ type from int to uint64_t?

Wouldn't intptr_t or uintptr_t be more appropriate?

"Handles" in Windows are more likely to be pointers masquerading as an
integer for opacity purposes.
--
"The Direct3D Graphics Pipeline" free book <http://tinyurl.com/d3d-pipeline>
     The Computer Graphics Museum <http://computergraphicsmuseum.org>
         The Terminals Wiki <http://terminals.classiccmp.org>
  Legalize Adulthood! (my blog) <http://legalizeadulthood.wordpress.com>
_______________________________________________
cfe-dev mailing list
[hidden email]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev


_______________________________________________
cfe-dev mailing list
[hidden email]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev
Reply | Threaded
Open this post in threaded view
|

Re: [libcxx] random_device for Windows needs more storage

Nico Rieck
In reply to this post by Yaron Keren
On 07.10.2013 20:57, Yaron Keren wrote:
> I'm trying to implement random_device for Windows using the CryptGenRandom
> API, as in the attached non-working code.

There's no need for CryptGenRandom and its overhead here. RtlGenRandom
is just fine for random_device. MSVCRT does the same.

-Nico
_______________________________________________
cfe-dev mailing list
[hidden email]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev
Reply | Threaded
Open this post in threaded view
|

Re: [libcxx] random_device for Windows needs more storage

Yaron Keren
Nick, I have seen RtlGenRandom, but I didn't initally use it since Microsoft is recommending to use CryptGenRandom instead:


[The RtlGenRandom function is available for use in the operating systems specified in the Requirements section. It may be altered or unavailable in subsequent versions. Instead, use the CryptGenRandom function.]

Due to backward compatibility it's very low chance RtlGenRandom will actually go away, I'll use it.

Yaron



2013/10/7 Nico Rieck <[hidden email]>
On 07.10.2013 20:57, Yaron Keren wrote:
I'm trying to implement random_device for Windows using the CryptGenRandom
API, as in the attached non-working code.

There's no need for CryptGenRandom and its overhead here. RtlGenRandom is just fine for random_device. MSVCRT does the same.

-Nico


_______________________________________________
cfe-dev mailing list
[hidden email]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev
Reply | Threaded
Open this post in threaded view
|

Re: [libcxx] random_device for Windows needs more storage

Aaron Ballman
On Mon, Oct 7, 2013 at 5:31 PM, Yaron Keren <[hidden email]> wrote:

> Nick, I have seen RtlGenRandom, but I didn't initally use it since Microsoft
> is recommending to use CryptGenRandom instead:
>
> http://msdn.microsoft.com/en-us/library/windows/desktop/aa387694(v=vs.85).aspx
>
> [The RtlGenRandom function is available for use in the operating systems
> specified in the Requirements section. It may be altered or unavailable in
> subsequent versions. Instead, use the CryptGenRandom function.]
>
> Due to backward compatibility it's very low chance RtlGenRandom will
> actually go away, I'll use it.

I don't believe we should be using this function.  Technically, the
documentation states this is available for use with the operating
systems listed in the Requirements section, which only lists XP and
Server 2003.  Relying on this outside of those platforms is not only
bad form, but to boot, in order to use them, you need to dynamically
load SystemFunction036 from AdvApi32.dll.

~Aaron
_______________________________________________
cfe-dev mailing list
[hidden email]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev
Reply | Threaded
Open this post in threaded view
|

Re: [libcxx] random_device for Windows needs more storage

Yaron Keren
Hi,

Just found out both Visual C++ and MingW supply the rand_s() function (implemented in MSVCRT), which wraps one of the above functions in a very easy to use and version:


Microsoft own random_device uses it,

Yaron



2013/10/8 Aaron Ballman <[hidden email]>
On Mon, Oct 7, 2013 at 5:31 PM, Yaron Keren <[hidden email]> wrote:
> Nick, I have seen RtlGenRandom, but I didn't initally use it since Microsoft
> is recommending to use CryptGenRandom instead:
>
> http://msdn.microsoft.com/en-us/library/windows/desktop/aa387694(v=vs.85).aspx
>
> [The RtlGenRandom function is available for use in the operating systems
> specified in the Requirements section. It may be altered or unavailable in
> subsequent versions. Instead, use the CryptGenRandom function.]
>
> Due to backward compatibility it's very low chance RtlGenRandom will
> actually go away, I'll use it.

I don't believe we should be using this function.  Technically, the
documentation states this is available for use with the operating
systems listed in the Requirements section, which only lists XP and
Server 2003.  Relying on this outside of those platforms is not only
bad form, but to boot, in order to use them, you need to dynamically
load SystemFunction036 from AdvApi32.dll.

~Aaron


_______________________________________________
cfe-dev mailing list
[hidden email]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev
Reply | Threaded
Open this post in threaded view
|

Re: [libcxx] random_device for Windows needs more storage

Aaron Ballman
rand_s() is a much better solution.  :-)

~Aaron

On Mon, Oct 7, 2013 at 5:53 PM, Yaron Keren <[hidden email]> wrote:

> Hi,
>
> Just found out both Visual C++ and MingW supply the rand_s() function
> (implemented in MSVCRT), which wraps one of the above functions in a very
> easy to use and version:
>
> http://msdn.microsoft.com/en-us/library/sxtz2fa8.aspx
>
> Microsoft own random_device uses it,
>
> Yaron
>
>
>
> 2013/10/8 Aaron Ballman <[hidden email]>
>>
>> On Mon, Oct 7, 2013 at 5:31 PM, Yaron Keren <[hidden email]> wrote:
>> > Nick, I have seen RtlGenRandom, but I didn't initally use it since
>> > Microsoft
>> > is recommending to use CryptGenRandom instead:
>> >
>> >
>> > http://msdn.microsoft.com/en-us/library/windows/desktop/aa387694(v=vs.85).aspx
>> >
>> > [The RtlGenRandom function is available for use in the operating systems
>> > specified in the Requirements section. It may be altered or unavailable
>> > in
>> > subsequent versions. Instead, use the CryptGenRandom function.]
>> >
>> > Due to backward compatibility it's very low chance RtlGenRandom will
>> > actually go away, I'll use it.
>>
>> I don't believe we should be using this function.  Technically, the
>> documentation states this is available for use with the operating
>> systems listed in the Requirements section, which only lists XP and
>> Server 2003.  Relying on this outside of those platforms is not only
>> bad form, but to boot, in order to use them, you need to dynamically
>> load SystemFunction036 from AdvApi32.dll.
>>
>> ~Aaron
>
>
_______________________________________________
cfe-dev mailing list
[hidden email]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev
Reply | Threaded
Open this post in threaded view
|

Re: [libcxx] random_device for Windows needs more storage

Nico Rieck
In reply to this post by Aaron Ballman
On 07.10.2013 23:37, Aaron Ballman wrote:
> I don't believe we should be using this function.  Technically, the
> documentation states this is available for use with the operating
> systems listed in the Requirements section, which only lists XP and
> Server 2003.  Relying on this outside of those platforms is not only
> bad form, but to boot, in order to use them, you need to dynamically
> load SystemFunction036 from AdvApi32.dll.

For newer SDKs, when targeting XP and up (_WIN32_WINNT >= 0x0501),
ntsecapi.h defines it as a macro to SystemFunction036 so it works
without any additional steps. And the documentation lists the minimum
required OS version.

-Nico
_______________________________________________
cfe-dev mailing list
[hidden email]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev
Reply | Threaded
Open this post in threaded view
|

Re: [libcxx] random_device for Windows needs more storage

David Chisnall-4
In reply to this post by Yaron Keren
On 7 Oct 2013, at 21:20, Yaron Keren <[hidden email]> wrote:

> Looking in the MingW header file basestd.h, ULONG_PTR is defined to either __int64 in WIN64 or long in WIN32, so it is always 64 bit.

Did I miss something?  Long is 32-bits on win32 (and on win64, which breaks a lot of code that assumes that you can round trip pointers through long, because even though the standard has never guaranteed this it's been possible on pretty much every system that isn't an IBM mainframe for decades).  On Win32, long is the size of a pointer, on Win64, __int64 is the size of a pointer.

David



_______________________________________________
cfe-dev mailing list
[hidden email]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev
Reply | Threaded
Open this post in threaded view
|

Re: [libcxx] random_device for Windows needs more storage

Yaron Keren
Hi David, 

You are correct, I was under the wrong impression that longs are 64 bit on Win32.  So indeed all references above map (with differing typdefs) HCRYPTPROV to 32 bits on Win32 and 64 bits on Win64.

In any case, I submitted a patch using rand_s() from MSVCRT.

Yaron



2013/10/8 David Chisnall <[hidden email]>
On 7 Oct 2013, at 21:20, Yaron Keren <[hidden email]> wrote:

> Looking in the MingW header file basestd.h, ULONG_PTR is defined to either __int64 in WIN64 or long in WIN32, so it is always 64 bit.

Did I miss something?  Long is 32-bits on win32 (and on win64, which breaks a lot of code that assumes that you can round trip pointers through long, because even though the standard has never guaranteed this it's been possible on pretty much every system that isn't an IBM mainframe for decades).  On Win32, long is the size of a pointer, on Win64, __int64 is the size of a pointer.

David




_______________________________________________
cfe-dev mailing list
[hidden email]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev
Reply | Threaded
Open this post in threaded view
|

Re: [libcxx] random_device for Windows needs more storage

Sebastian Redl
In reply to this post by Yaron Keren
On 2013-10-07 22:20, Yaron Keren wrote:
The actual data type is typedef ULONG_PTR HCRYPTPROV;
I found two conflicting descriptions in microsoft.com for ULONG_PTR:


#if defined(_WIN64)
 typedef unsigned __int64 ULONG_PTR;
#else
 typedef unsigned long ULONG_PTR;
#endif

unsigned long is always 32 bits on Microsoft's compiler.

Sebastian

_______________________________________________
cfe-dev mailing list
[hidden email]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev
Reply | Threaded
Open this post in threaded view
|

Re: [libcxx] random_device for Windows needs more storage

Yaron Keren
Thanks, Sebastian, David Chisnall corrected me.
random_device() was finally implemented based on rand_s() from MSVCRT.

Yaron



2013/10/8 Sebastian Redl <[hidden email]>
On 2013-10-07 22:20, Yaron Keren wrote:
The actual data type is typedef ULONG_PTR HCRYPTPROV;
I found two conflicting descriptions in microsoft.com for ULONG_PTR:


#if defined(_WIN64)
 typedef unsigned __int64 ULONG_PTR;
#else
 typedef unsigned long ULONG_PTR;
#endif

unsigned long is always 32 bits on Microsoft's compiler.

Sebastian

_______________________________________________
cfe-dev mailing list
[hidden email]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev



_______________________________________________
cfe-dev mailing list
[hidden email]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev