diff mbox series

[Openvpn-devel] crypto.c: fix Visual Studio build

Message ID 1563442503-11119-1-git-send-email-lstipakov@gmail.com
State Accepted
Headers show
Series [Openvpn-devel] crypto.c: fix Visual Studio build | expand

Commit Message

Lev Stipakov July 17, 2019, 11:35 p.m. UTC 
From: Lev Stipakov <lev@openvpn.net>

Commit fb4e8ab added variable-length array which
is C99 feature and is not supported by Visual Studio.

This removes VLA and writes data directly into passed buffer.

Signed-off-by: Lev Stipakov <lev@openvpn.net>
---
 src/openvpn/crypto.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

Comments

Antonio Quartulli July 18, 2019, 1:55 a.m. UTC | #1 
Hi,

On 18/07/2019 11:35, Lev Stipakov wrote:
> From: Lev Stipakov <lev@openvpn.net>
> 
> Commit fb4e8ab added variable-length array which
> is C99 feature and is not supported by Visual Studio.
> 
> This removes VLA and writes data directly into passed buffer.
> 
> Signed-off-by: Lev Stipakov <lev@openvpn.net>
> ---
>  src/openvpn/crypto.c | 10 +++++++---
>  1 file changed, 7 insertions(+), 3 deletions(-)
> 
> diff --git a/src/openvpn/crypto.c b/src/openvpn/crypto.c
> index 69877d1..8bf33e7 100644
> --- a/src/openvpn/crypto.c
> +++ b/src/openvpn/crypto.c
> @@ -1895,14 +1895,18 @@ cleanup:
>  bool
>  generate_ephemeral_key(struct buffer *key, const char *key_name)
>  {
> +    const int len = BCAP(key);
> +
>      msg(M_INFO, "Using random %s.", key_name);
> -    uint8_t rand[BCAP(key)];
> -    if (!rand_bytes(rand, BCAP(key)))
> +
> +    if (!rand_bytes(BEND(key), len))

Are we sure we can't call this function with 'len' longer than the
available space in 'key' ?


Cheers,

>      {
>          msg(M_WARN, "ERROR: could not generate random key");
>          return false;
>      }
> -    buf_write(key, rand, BCAP(key));
> +
> +    buf_inc_len(key, len);
> +
>      return true;
>  }
>  
>
Antonio Quartulli July 18, 2019, 2:20 a.m. UTC | #2 
On 18/07/2019 14:20, Lev Stipakov wrote:
> Hi,
> 
> 
>> Are we sure we can't call this function with 'len' longer than the
>> available space in 'key' ?
> 
> 
> Yep, because we get available space here:
> 
>> +    const int len = BCAP(key);
>>
> 
> and then pass it to rand_bytes:
> 

Ok, that sounds good, thanks!

> 
>>> +    if (!rand_bytes(BEND(key), len))
> 
>
Lev Stipakov July 18, 2019, 2:20 a.m. UTC | #3 
Hi,


> Are we sure we can't call this function with 'len' longer than the
> available space in 'key' ?


Yep, because we get available space here:

> +    const int len = BCAP(key);
>

and then pass it to rand_bytes:


> > +    if (!rand_bytes(BEND(key), len))
Lev Stipakov July 21, 2019, 9:14 p.m. UTC | #4 
Moving discussion to -devel.

By the way, are you aware that we already perform Visual Studio builds in
appveyor?

https://ci.appveyor.com/project/mattock/openvpn-build/history

Do you think we should have both?

What would be nice to have is to trigger VS build on every commit to
openvpn repo, since openvpn-build is something we run manually.

---------- Forwarded message ---------
Lähettäjä: Илья Шипицин <chipitsine@gmail.com>
Date: su 21. heinäk. 2019 klo 18.17
Subject: Re: [Openvpn-devel] [PATCH] crypto.c: fix Visual Studio build
To: Lev Stipakov <lstipakov@gmail.com>




чт, 18 июл. 2019 г. в 16:26, Lev Stipakov <lstipakov@gmail.com>:

> Hi,
>
> to 18. heinäk. 2019 klo 14.15 Илья Шипицин (chipitsine@gmail.com)
> kirjoitti:
>
>> btw, can you share steps required for Visual Studio builds ?
>>
>
> Just clone
>
> https://github.com/openvpn/openvpn-build
>
> and run
>


I used to think we can run build from 'openvpn/openvpn' repo.
well, openvpn-build is good as well


> c:\Users\lev\Projects\openvpn-build\msvc>set MODE=DEPS&& build.bat
> c:\Users\lev\Projects\openvpn-build\msvc>set MODE=OPENVPN&& build.bat
>
> It depends on VS2017, maybe it is time to switch to VS2019.
>

travis-ci comes without VS installed. I'll figure out is it legal to
install community edition or not

currently it already caught nasty bug

"rc" was removed by endlocal:
https://github.com/OpenVPN/openvpn-build/pull/152


>
>
>> travis-ci supports windows builds. I would like to add VS to travis-ci
>>
>
> That would be cool.
>
>>
> --
> -Lev
>
Arne Schwabe July 24, 2019, 12:29 a.m. UTC | #5 
Am 18.07.19 um 11:35 schrieb Lev Stipakov:
> From: Lev Stipakov <lev@openvpn.net>
> 
> Commit fb4e8ab added variable-length array which
> is C99 feature and is not supported by Visual Studio.
> 
> This removes VLA and writes data directly into passed buffer.
> 

After some confusion of the various buffer related function on my side,
this looks good.

Acked-By: Arne Schwabe <arne@rfc2549.org>
Gert Doering July 28, 2019, 10:22 a.m. UTC | #6 
Your patch has been applied to the master branch.

(The code is not in 2.4, so no need to backport).  Slightly tested with
a "make check" run.

commit 2df27b2fa3932372075afd0db6b706a38e0a9dc3
Author: Lev Stipakov
Date:   Thu Jul 18 12:35:03 2019 +0300

     crypto.c: fix Visual Studio build

     Signed-off-by: Lev Stipakov <lev@openvpn.net>
     Acked-by: Arne Schwabe <arne@rfc2549.org>
     Message-Id: <1563442503-11119-1-git-send-email-lstipakov@gmail.com>
     URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg18676.html
     Signed-off-by: Gert Doering <gert@greenie.muc.de>


--
kind regards,

Gert Doering
diff mbox series

Patch

diff --git a/src/openvpn/crypto.c b/src/openvpn/crypto.c
index 69877d1..8bf33e7 100644
--- a/src/openvpn/crypto.c
+++ b/src/openvpn/crypto.c
@@ -1895,14 +1895,18 @@  cleanup:
 bool
 generate_ephemeral_key(struct buffer *key, const char *key_name)
 {
+    const int len = BCAP(key);
+
     msg(M_INFO, "Using random %s.", key_name);
-    uint8_t rand[BCAP(key)];
-    if (!rand_bytes(rand, BCAP(key)))
+
+    if (!rand_bytes(BEND(key), len))
     {
         msg(M_WARN, "ERROR: could not generate random key");
         return false;
     }
-    buf_write(key, rand, BCAP(key));
+
+    buf_inc_len(key, len);
+
     return true;
 }