[ovs-dev] [Windows thread 3]

Saurabh Shah ssaurabh at vmware.com
Fri Dec 13 00:41:40 UTC 2013


Looks good to me. Just one minor comment. You probably also want to include the following?

#include <windows.h>
#pragma comment(lib, "advapi32.lib")

Btw, printing the error message will require a small change which maps strerror_r to  strerror_s. Not sure, if we should just add it to this commit.

Saurabh

From: Alin Serdean <aserdean at cloudbasesolutions.com<mailto:aserdean at cloudbasesolutions.com>>
Date: Thursday, December 12, 2013 1:59 PM
To: Saurabh Shah <ssaurabh at vmware.com<mailto:ssaurabh at vmware.com>>, "blp at nicira.com<mailto:blp at nicira.com>" <blp at nicira.com<mailto:blp at nicira.com>>, "shettyg at nicira.com<mailto:shettyg at nicira.com>" <shettyg at nicira.com<mailto:shettyg at nicira.com>>
Cc: "dev at openvswitch.org<mailto:dev at openvswitch.org>" <dev at openvswitch.org<mailto:dev at openvswitch.org>>
Subject: RE: [ovs-dev] [Windows thread 3]

Updated. Thanks for the feedback :-).

Final version would look something like:

 lib/entropy.c |   13 +++++++++++++
 1 files changed, 13 insertions(+), 0 deletions(-)
---
 diff --git a/lib/entropy.c b/lib/entropy.c
index 02f56e0..45e83ec 100644
--- a/lib/entropy.c
+++ b/lib/entropy.c
@@ -33,6 +33,7 @@ static const char urandom[] = "/dev/urandom";
 int
 get_entropy(void *buffer, size_t n)
 {
+#ifndef _WIN32
     size_t bytes_read;
     int error;
     int fd;
@@ -49,6 +50,18 @@ get_entropy(void *buffer, size_t n)
     if (error) {
         VLOG_ERR("%s: read error (%s)", urandom, ovs_retval_to_string(error));
     }
+#else
+       int error = 0;
+       HCRYPTPROV   crypt_prov = 0;
+       CryptAcquireContext(&crypt_prov, NULL, NULL, PROV_RSA_FULL, CRYPT_VERIFYCONTEXT);
+
+       if (!CryptGenRandom(crypt_prov, n, buffer)) {
+               error = GetLastError();
+               VLOG_ERR("CryptGenRandom: read error (%s)", ovs_retval_to_string(error));
+       }
+
+       CryptReleaseContext(crypt_prov, 0);
+#endif
     return error;
 }
---

If there are no further remarks I would post in a patch.

Alin.

________________________________
From: Saurabh Shah [ssaurabh at vmware.com<mailto:ssaurabh at vmware.com>]
Sent: Wednesday, December 11, 2013 9:53 PM
To: Alin Serdean; blp at nicira.com<mailto:blp at nicira.com>; shettyg at nicira.com<mailto:shettyg at nicira.com>
Cc: dev at openvswitch.org<mailto:dev at openvswitch.org>
Subject: Re: [ovs-dev] [Windows thread 3]


Hey,


The following is a quick patch for secure pseudorandom number generator on windows. I split the functionality with a brutal ifdef macro. Feedback on the code and suggestions for a nicer implementation is appreciated :).

diff --git a/lib/entropy.c b/lib/entropy.c
index 02f56e0..ec9d95c 100644
--- a/lib/entropy.c
+++ b/lib/entropy.c
@@ -20,6 +20,9 @@
#include <errno.h>
#include <fcntl.h>
#include <unistd.h>
+#ifdef _WIN32
+#include <Wincrypt.h>
+#endif

#include "socket-util.h"
#include "vlog.h"
@@ -33,6 +36,7 @@ static const char urandom[] = "/dev/urandom";
int
get_entropy(void *buffer, size_t n)
{
+#ifndef _WIN32
     size_t bytes_read;
     int error;
     int fd;
@@ -49,6 +53,20 @@ get_entropy(void *buffer, size_t n)
     if (error) {
         VLOG_ERR("%s: read error (%s)", urandom, ovs_retval_to_string(error));
     }
+#else
+     int error = 1;
+     HCRYPTPROV   crypt_prov = 0;
+     CryptAcquireContext(&crypt_prov, NULL, NULL, PROV_RSA_FULL, 0);
+

Microsoft documentation suggests using CRYPT_VERIFYCONTEXT. Although, I haven't tested to see what sort of an impact this will have.

http://msdn.microsoft.com/en-us/library/windows/desktop/aa379886(v=vs.85).aspx<https://urldefense.proofpoint.com/v1/url?u=http://msdn.microsoft.com/en-us/library/windows/desktop/aa379886%28v%3Dvs.85%29.aspx&k=oIvRg1%2BdGAgOoM1BIlLLqw%3D%3D%0A&r=pEkjsHfytvHEWufeZPpgqSOJMdMjuZPbesVsNhCUc0E%3D%0A&m=zoQn%2BOKvRPDqVVu5wfXEQ7LzlyDiCzTd%2BoRL3B70vuQ%3D%0A&s=bf3a9912332ddbdb2c2109e0613776296f4ecc3c50b51a489f75b67da27a015f>
For performance reasons, we recommend that you set the pszContainer parameter to NULL and the dwFlags parameter to CRYPT_VERIFYCONTEXT in all situations where you do not require a persisted key. In particular, consider setting the pszContainer parameter to NULL and the dwFlags parameter to CRYPT_VERIFYCONTEXT for the following scenarios:

+     if (CryptGenRandom(crypt_prov, n, buffer)) {
+         error = 0;
+     }
+
+     if (error) {
+         VLOG_ERR("CryptGenRandom: read error (%s)", urandom, ovs_retval_to_string(error));
+     }

How about doing instead -

int error = 0;
If (! CryptGetRandom(crypt_prov, n, buffer)) {
    error = GetLastError();
     VLOG_ERR("CryptGenRandom: read error (%s)", urandom, ovs_retval_to_string(error));
}

+     CryptReleaseContext(crypt_prov, 0);
+#endif
     return error;
}


Kind Regards,

Alin.

_______________________________________________
dev mailing list
dev at openvswitch.org<mailto:dev at openvswitch.org>
https://urldefense.proofpoint.com/v1/url?u=http://openvswitch.org/mailman/listinfo/dev&k=oIvRg1%2BdGAgOoM1BIlLLqw%3D%3D%0A&r=pEkjsHfytvHEWufeZPpgqSOJMdMjuZPbesVsNhCUc0E%3D%0A&m=KlUcJXE7spv5Cm%2FmexYFbql6rLI%2BJfpjXWgtb05Lero%3D%0A&s=ccb6c3872370fa5ee60d07509dba3d0a07ebc526274c18553e02ab434de8bcdb

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openvswitch.org/pipermail/ovs-dev/attachments/20131212/3a3be4ed/attachment-0003.html>


More information about the dev mailing list