You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@apr.apache.org by Mladen Turk <mt...@mappingsoft.com> on 2001/10/29 19:45:50 UTC

[PATCH] apr_generate_random_bytes - WIN32

Hi,

The WIN-XP does not starts the cryptographic service provider (CSP) by
default (at least on my installation).
I think they move that out to boot faster.
Well, anyhow, the patch tries to create the new key container with the
default name if the CSP was not started allready.

Interesting is that the call takes couple of seconds to finish when called
for the first time.

MT.

Index: rand.c
===================================================================
RCS file: /home/cvspublic/apr/misc/win32/rand.c,v
retrieving revision 1.10
diff -u -r1.10 rand.c
--- rand.c	2001/02/16 04:15:58	1.10
+++ rand.c	2001/10/29 18:21:38
@@ -63,7 +63,11 @@
     apr_status_t res = APR_SUCCESS;

     if (!CryptAcquireContext(&hProv,NULL,NULL,PROV_RSA_FULL,0)) {
-	return apr_get_os_error();
+        /* Try to create the new key container */
+        if (CryptAcquireContext(&hProv,NULL,NULL,PROV_RSA_FULL,
+                                CRYPT_NEWKEYSET)) {
+        	return apr_get_os_error();
+        }
     }
     if (!CryptGenRandom(hProv,length,buf)) {
     	res = apr_get_os_error();



RE: [PATCH] apr_generate_random_bytes - WIN32

Posted by Cliff Woolley <cl...@yahoo.com>.
On Tue, 30 Oct 2001, Mladen Turk wrote:

> >  1    if (!CryptAcquireContext(&hProv,NULL,NULL,PROV_RSA_FULL,0)) {
> >  2        /* Try to create the new key container */
> >  3        if ((GetLastError() != NTE_BAD_KEYSET) ||
> >  4            (!CryptAcquireContext(&hProv,NULL,NULL,PROV_RSA_FULL,
> >  5                                  CRYPT_NEWKEYSET))) {
> >  6        	return apr_get_os_error();
> >  7        }
> >  8    }
>
> 1    if (!CryptAcquireContext(&hProv,NULL,NULL,PROV_RSA_FULL,0)) {
> 2        /* Try to create the new key container */
> 3        if (GetLastError() == NTE_BAD_KEYSET) {
> 4            if (!CryptAcquireContext(&hProv,NULL,NULL,PROV_RSA_FULL,
> 5                                  CRYPT_NEWKEYSET))
> 6        	return apr_get_os_error();
> 7        }
> 8        else
> 9        	return apr_get_os_error();
> 10    }
>

It's the same exact thing.  I just exploited the handy dandy
short-circuit boolean evaluation of C, which guarantees in my example that
if the one is !=, then the second CryptAcquireContext call will be
skipped.

Anyhow, yours might arguably be easier to read... so +1 to either variant
from the Win32 peanut gallery (I'm taking your word for it that this is
the right fix since I know next to nothing about the Win32 API... I'm just
saying +1 because I now believe the logic is correct).

--Cliff

--------------------------------------------------------------
   Cliff Woolley
   cliffwoolley@yahoo.com
   Charlottesville, VA



RE: [PATCH] apr_generate_random_bytes - WIN32

Posted by Mladen Turk <mt...@mappingsoft.com>.

> -----Original Message-----
> From: Cliff Woolley [mailto:cliffwoolley@yahoo.com]
> Sent: Monday, October 29, 2001 9:21 PM
> To: Mladen Turk
> Cc: Sander Striker; APR Dev List
> Subject: RE: [PATCH] apr_generate_random_bytes - WIN32
>
> Shouldn't there be an "else" in here that returns apr_get_os_error() if
> GetLastError() returned something OTHER than NTE_BAD_KEYSET?  That makes
> it a little funky to break out of the success case, though.  Seems like it
> should be something like this:
>
>  1    if (!CryptAcquireContext(&hProv,NULL,NULL,PROV_RSA_FULL,0)) {
>  2        /* Try to create the new key container */
>  3        if ((GetLastError() != NTE_BAD_KEYSET) ||
>  4            (!CryptAcquireContext(&hProv,NULL,NULL,PROV_RSA_FULL,
>  5                                  CRYPT_NEWKEYSET))) {
>  6        	return apr_get_os_error();
>  7        }
>  8    }
>
> Note in particular line 3.  We want to call apr_get_os_error() if _either_
> the last error was not NTE_BAD_KEYSET _or_ the CRYPT_NEWKEYSET thingy
> failed.
>
> Is that right?

Well, It is, but I intentionally put the && to be sure not to call the
CryptAcquireContex that will surely fall in case if the last error wasn't
NTE_BAD_KEYSET.
The proper code would be as follows:

1    if (!CryptAcquireContext(&hProv,NULL,NULL,PROV_RSA_FULL,0)) {
2        /* Try to create the new key container */
3        if (GetLastError() == NTE_BAD_KEYSET) {
4            if (!CryptAcquireContext(&hProv,NULL,NULL,PROV_RSA_FULL,
5                                  CRYPT_NEWKEYSET))
6        	return apr_get_os_error();
7        }
8        else
9        	return apr_get_os_error();
10    }


MT.


RE: [PATCH] apr_generate_random_bytes - WIN32

Posted by Cliff Woolley <cl...@yahoo.com>.
On Mon, 29 Oct 2001, Mladen Turk wrote:

> Ooops! It should be:
> if (!CryptAcquireContext(&hProv,NULL,NULL,PROV_RSA_FULL,

I'd wondered about that, too.

> Index: rand.c
> ===================================================================
> RCS file: /home/cvspublic/apr/misc/win32/rand.c,v
> retrieving revision 1.10
> diff -u -r1.10 rand.c
> --- rand.c	2001/02/16 04:15:58	1.10
> +++ rand.c	2001/10/29 19:24:55
> @@ -63,7 +63,12 @@
>      apr_status_t res = APR_SUCCESS;
>
>      if (!CryptAcquireContext(&hProv,NULL,NULL,PROV_RSA_FULL,0)) {
> -	return apr_get_os_error();
> +        /* Try to create the new key container */
> +        if ((GetLastError() == NTE_BAD_KEYSET) &&
> +            !CryptAcquireContext(&hProv,NULL,NULL,PROV_RSA_FULL,
> +                                CRYPT_NEWKEYSET)) {
> +        	return apr_get_os_error();
> +        }
>      }

Shouldn't there be an "else" in here that returns apr_get_os_error() if
GetLastError() returned something OTHER than NTE_BAD_KEYSET?  That makes
it a little funky to break out of the success case, though.  Seems like it
should be something like this:

 1    if (!CryptAcquireContext(&hProv,NULL,NULL,PROV_RSA_FULL,0)) {
 2        /* Try to create the new key container */
 3        if ((GetLastError() != NTE_BAD_KEYSET) ||
 4            (!CryptAcquireContext(&hProv,NULL,NULL,PROV_RSA_FULL,
 5                                  CRYPT_NEWKEYSET))) {
 6        	return apr_get_os_error();
 7        }
 8    }

Note in particular line 3.  We want to call apr_get_os_error() if _either_
the last error was not NTE_BAD_KEYSET _or_ the CRYPT_NEWKEYSET thingy
failed.

Is that right?

--Cliff

--------------------------------------------------------------
   Cliff Woolley
   cliffwoolley@yahoo.com
   Charlottesville, VA



RE: [PATCH] apr_generate_random_bytes - WIN32

Posted by Mladen Turk <mt...@mappingsoft.com>.

> -----Original Message-----
> From: Sander Striker [mailto:striker@apache.org]
> Sent: Monday, October 29, 2001 7:50 PM
> To: Mladen Turk; APR Dev List
> Subject: RE: [PATCH] apr_generate_random_bytes - WIN32
>
>
> > +        if (CryptAcquireContext(&hProv,NULL,NULL,PROV_RSA_FULL,
>                ^^^^
> Seems you have reversed the logic here.  Or was it wrong in the
> first place?

Ooops! It should be:
if (!CryptAcquireContext(&hProv,NULL,NULL,PROV_RSA_FULL,


Well, the all thing is very strange...
Once you create the  key container, the 'default' CryptAcquireContext passes
even after reboot.
All that is per-user basis, so if you switch the user you'll need to call
the CryptAcquireContext with the CRYPT_NEWKEYSET param because it returns
the NTE_BAD_KEYSET otherwise.
It seems that the key container doesn't exist when the user is created, and
that the CRYPT_NEWKEYSET needs to be called only once.

Here is the corrected one :)

MT.

Index: rand.c
===================================================================
RCS file: /home/cvspublic/apr/misc/win32/rand.c,v
retrieving revision 1.10
diff -u -r1.10 rand.c
--- rand.c	2001/02/16 04:15:58	1.10
+++ rand.c	2001/10/29 19:24:55
@@ -63,7 +63,12 @@
     apr_status_t res = APR_SUCCESS;

     if (!CryptAcquireContext(&hProv,NULL,NULL,PROV_RSA_FULL,0)) {
-	return apr_get_os_error();
+        /* Try to create the new key container */
+        if ((GetLastError() == NTE_BAD_KEYSET) &&
+            !CryptAcquireContext(&hProv,NULL,NULL,PROV_RSA_FULL,
+                                CRYPT_NEWKEYSET)) {
+        	return apr_get_os_error();
+        }
     }
     if (!CryptGenRandom(hProv,length,buf)) {
     	res = apr_get_os_error();


RE: [PATCH] apr_generate_random_bytes - WIN32

Posted by Sander Striker <st...@apache.org>.
> From: Mladen Turk [mailto:mturk@mappingsoft.com]
> Sent: 29 October 2001 19:46
> To: APR Dev List
> Subject: [PATCH] apr_generate_random_bytes - WIN32
>
>
> Hi,
>
> The WIN-XP does not starts the cryptographic service provider (CSP) by
> default (at least on my installation).
> I think they move that out to boot faster.
> Well, anyhow, the patch tries to create the new key container with the
> default name if the CSP was not started allready.
>
> Interesting is that the call takes couple of seconds to finish when called
> for the first time.
>
> MT.
>
> Index: rand.c
> ===================================================================
> RCS file: /home/cvspublic/apr/misc/win32/rand.c,v
> retrieving revision 1.10
> diff -u -r1.10 rand.c
> --- rand.c	2001/02/16 04:15:58	1.10
> +++ rand.c	2001/10/29 18:21:38
> @@ -63,7 +63,11 @@
>      apr_status_t res = APR_SUCCESS;
>
>      if (!CryptAcquireContext(&hProv,NULL,NULL,PROV_RSA_FULL,0)) {
> -	return apr_get_os_error();
> +        /* Try to create the new key container */
> +        if (CryptAcquireContext(&hProv,NULL,NULL,PROV_RSA_FULL,
               ^^^^
Seems you have reversed the logic here.  Or was it wrong in the
first place?

> +                                CRYPT_NEWKEYSET)) {
> +        	return apr_get_os_error();
> +        }
>      }
>      if (!CryptGenRandom(hProv,length,buf)) {
>      	res = apr_get_os_error();