[ovs-dev] [Windows thread 3]

Ben Pfaff blp at nicira.com
Wed Dec 18 17:44:45 UTC 2013 

Saurabh, can you post a final patch?

On Fri, Dec 13, 2013 at 11:18:17AM -0800, Saurabh Shah wrote:
> I don't have a strong preference about the structuring. Anything that is intuitive is fine with me. With regards to keeping all pragma comments separate, I actually like keeping them close to the files that use them. However, since these are DLLs, I don't see any major downside in declaring them in a separate file (except that the pragma declare file might get out-of-sync with the actual libraries used by the code).
> 
> Thanks!
> Saurabh
> 
> From: Alin Serdean <aserdean at cloudbasesolutions.com<mailto:aserdean at cloudbasesolutions.com>>
> Date: Friday, December 13, 2013 10:28 AM
> 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]
> 
> I was thinking to include all pragmas and other needed defines in a common header used through all of the source files(http://openvswitch.org/pipermail/dev/2013-December/034983.html), but this is just my thought.
> 
> The way you want to structure it is up too you :-).
> 
> Alin.
> ________________________________
> From: Saurabh Shah [ssaurabh at vmware.com<mailto:ssaurabh at vmware.com>]
> Sent: Friday, December 13, 2013 2:41 AM
> 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]
> 
> 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
>

More information about the dev mailing list