Re: [Openvpn-devel] [PATCH] msvc: mark x64 release binaries as compatible with CET shadow stack

2022-01-07 Thread Lev Stipakov
Please disregard this patch,

I've sent two separate ones for 2.5 and master which fix multiple
issues found by binskim, including HW-enforced stack protection.

  https://patchwork.openvpn.net/patch/2209/
  https://patchwork.openvpn.net/patch/2210/

Note that before applying 2.5 patch, one needs to cherry-pick this commit

  
https://github.com/openvpn/openvpn/commit/e5e9a07e8baee4065b7dfd65736bfa77b8329cfc

from the master.

-- 
-Lev


___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel


[Openvpn-devel] [PATCH] msvc: adjust build options to harden binaries

2022-01-07 Thread Lev Stipakov
From: Lev Stipakov 

 - enable hardware-enforced stack protection on
compatible hardware/software (/CETCOMPAT linker option)

 - hash object files with SHA256 (/ZH:SHA_256 compiler option)

 - enable SDL. The required to add

_CRT_NONSTDC_NO_DEPRECATE
_CRT_SECURE_NO_WARNINGS
_WINSOCK_DEPRECATED_NO_WARNINGS

preprocessor definitions. I don't feel like replacing strdup (which is
correct POSIX function) and inet_ntoa (we always pass IPv4 address to
it, inet_ntop will make code more complex)

Above issues were discovered by bitskim.

Signed-off-by: Lev Stipakov 
---
 src/openvpn/openvpn.vcxproj   | 35 ---
 src/openvpnmsica/openvpnmsica.vcxproj | 37 +
 src/openvpnserv/openvpnserv.vcxproj   | 14 
 src/tapctl/tapctl.vcxproj | 48 +++
 4 files changed, 116 insertions(+), 18 deletions(-)

diff --git a/src/openvpn/openvpn.vcxproj b/src/openvpn/openvpn.vcxproj
index 65ee6839..55ad7197 100644
--- a/src/openvpn/openvpn.vcxproj
+++ b/src/openvpn/openvpn.vcxproj
@@ -147,11 +147,12 @@
   
   
 
-  
_CONSOLE;%(PreprocessorDefinitions)
+  
_CRT_NONSTDC_NO_DEPRECATE;_CRT_SECURE_NO_WARNINGS;_WINSOCK_DEPRECATED_NO_WARNINGS;_CONSOLE;%(PreprocessorDefinitions)
   
%(UndefinePreprocessorDefinitions)
-  Level2
   true
   
..\compat;$(SolutionDir);%(AdditionalIncludeDirectories)
+  Level2
+  /ZH:SHA_256 %(AdditionalOptions)
 
 
 
@@ -162,11 +163,12 @@
   
   
 
-  
_CONSOLE;%(PreprocessorDefinitions)
+  
_CRT_NONSTDC_NO_DEPRECATE;_CRT_SECURE_NO_WARNINGS;_WINSOCK_DEPRECATED_NO_WARNINGS;_CONSOLE;%(PreprocessorDefinitions)
   
%(UndefinePreprocessorDefinitions)
-  Level2
   true
   
..\compat;$(SolutionDir)include;$(SolutionDir);%(AdditionalIncludeDirectories)
+  Level2
+  /ZH:SHA_256 %(AdditionalOptions)
 
 
 
@@ -177,11 +179,12 @@
   
   
 
-  
_CONSOLE;%(PreprocessorDefinitions)
+  
_CRT_NONSTDC_NO_DEPRECATE;_CRT_SECURE_NO_WARNINGS;_WINSOCK_DEPRECATED_NO_WARNINGS;_CONSOLE;%(PreprocessorDefinitions)
   
%(UndefinePreprocessorDefinitions)
-  Level2
   true
   
..\compat;$(SolutionDir);%(AdditionalIncludeDirectories)
+  Level2
+  /ZH:SHA_256 %(AdditionalOptions)
 
 
 
@@ -192,44 +195,52 @@
   
   
 
-  
_CONSOLE;%(PreprocessorDefinitions)
+  
_CRT_NONSTDC_NO_DEPRECATE;_CRT_SECURE_NO_WARNINGS;_WINSOCK_DEPRECATED_NO_WARNINGS;_CONSOLE;%(PreprocessorDefinitions)
   
%(UndefinePreprocessorDefinitions)
-  Level2
   true
   
..\compat;$(SolutionDir);%(AdditionalIncludeDirectories)
   Guard
+  Level2
+  /ZH:SHA_256 %(AdditionalOptions)
+  true
 
 
 
   
Ncrypt.lib;gdi32.lib;ws2_32.lib;wininet.lib;crypt32.lib;iphlpapi.lib;winmm.lib;Fwpuclnt.lib;Rpcrt4.lib;setupapi.lib;Advapi32.lib
   
$(OPENSSL_HOME)/lib;$(LZO_HOME)/lib;$(PKCS11H_HOME)/lib;%(AdditionalLibraryDirectories)
   Console
+  true
 
   
   
 
-  
_CONSOLE;%(PreprocessorDefinitions)
+  
_CRT_NONSTDC_NO_DEPRECATE;_CRT_SECURE_NO_WARNINGS;_WINSOCK_DEPRECATED_NO_WARNINGS;_CONSOLE;%(PreprocessorDefinitions)
   
%(UndefinePreprocessorDefinitions)
-  Level2
   true
   
..\compat;$(SolutionDir);%(AdditionalIncludeDirectories)
   Guard
+  true
+  Level2
+  /ZH:SHA_256 %(AdditionalOptions)
 
 
 
   
Ncrypt.lib;gdi32.lib;ws2_32.lib;wininet.lib;crypt32.lib;iphlpapi.lib;winmm.lib;Fwpuclnt.lib;Rpcrt4.lib;setupapi.lib;Advapi32.lib
   
$(OPENSSL_HOME)/lib;$(LZO_HOME)/lib;$(PKCS11H_HOME)/lib;%(AdditionalLibraryDirectories)
   Console
+  true
 
   
   
 
-  
_CONSOLE;%(PreprocessorDefinitions)
+  
_CRT_NONSTDC_NO_DEPRECATE;_CRT_SECURE_NO_WARNINGS;_WINSOCK_DEPRECATED_NO_WARNINGS;_CONSOLE;%(PreprocessorDefinitions)
   
%(UndefinePreprocessorDefinitions)
-  Level2
   true
   
..\compat;$(SolutionDir);%(AdditionalIncludeDirectories)
   Guard
+  Level2
+  /ZH:SHA_256 %(AdditionalOptions)
+  true
 
 
 
diff --git a/src/openvpnmsica/openvpnmsica.vcxproj 
b/src/openvpnmsica/openvpnmsica.vcxproj
index 11aa78bb..1af8899e 100644
--- a/src/openvpnmsica/openvpnmsica.vcxproj
+++ b/src/openvpnmsica/openvpnmsica.vcxproj
@@ -135,6 +135,43 @@
   
 true
   
+  
+
+  true
+
+
+  /ZH:SHA_256 %(AdditionalOptions)
+
+  
+  
+
+  true
+
+
+  /ZH:SHA_256 %(AdditionalOptions)
+  true
+
+  
+  
+
+  /ZH:SHA_256 %(AdditionalOptions)
+
+  
+  
+
+  /ZH:SHA_256 %(AdditionalOptions)
+
+  
+  
+
+  /ZH:SHA_256 %(AdditionalOptions)
+
+  
+  
+
+  /ZH:SHA_256 %(AdditionalOptions)
+
+  
   
 
 
diff --git a/src/openvpnserv/openvpnserv.vcxproj 
b/src/openvpnserv/openvpnserv.vcxproj
index 5fd7d60b..d42e9642 100644
--- a/src/openvpnserv/openvpnserv.vcxproj
+++ 

[Openvpn-devel] [PATCH 2.5] msvc: adjust build options to harden binaries

2022-01-07 Thread Lev Stipakov
From: Lev Stipakov 

 - enable hardware-enforced stack protection on
compatible hardware/software (/CETCOMPAT linker option)

 - hash object files with SHA256 (/ZH:SHA_256 compiler option)

 - enable SDL. The required to add

_CRT_NONSTDC_NO_DEPRECATE
_CRT_SECURE_NO_WARNINGS
_WINSOCK_DEPRECATED_NO_WARNINGS

preprocessor definitions. I don't feel like replacing strdup (which is
correct POSIX function) and inet_ntoa (we always pass IPv4 address to
it, inet_ntop will make code more complex)

Above issues were discovered by bitskim.

Signed-off-by: Lev Stipakov 
---

 Note that one needs to cherry-pick commit

 "e5e9a07" (tapctl: Resolve MSVC C4996 warnings)

 before applying this patch.

 src/openvpn/openvpn.vcxproj   | 35 +++--
 src/openvpnmsica/openvpnmsica.vcxproj | 43 +
 src/openvpnserv/openvpnserv.vcxproj   | 26 ++---
 src/tapctl/tapctl.vcxproj | 54 ---
 4 files changed, 134 insertions(+), 24 deletions(-)

diff --git a/src/openvpn/openvpn.vcxproj b/src/openvpn/openvpn.vcxproj
index 33b8f19a..a540ec22 100644
--- a/src/openvpn/openvpn.vcxproj
+++ b/src/openvpn/openvpn.vcxproj
@@ -147,11 +147,12 @@
   
   
 
-  
_CONSOLE;%(PreprocessorDefinitions)
+  
_CRT_NONSTDC_NO_DEPRECATE;_CRT_SECURE_NO_WARNINGS;_WINSOCK_DEPRECATED_NO_WARNINGS;_CONSOLE;%(PreprocessorDefinitions)
   
%(UndefinePreprocessorDefinitions)
-  Level2
   true
   
..\compat;$(SolutionDir);%(AdditionalIncludeDirectories)
+  Level2
+  /ZH:SHA_256 %(AdditionalOptions)
 
 
 
@@ -162,11 +163,12 @@
   
   
 
-  
_CONSOLE;%(PreprocessorDefinitions)
+  
_CRT_NONSTDC_NO_DEPRECATE;_CRT_SECURE_NO_WARNINGS;_WINSOCK_DEPRECATED_NO_WARNINGS;_CONSOLE;%(PreprocessorDefinitions)
   
%(UndefinePreprocessorDefinitions)
-  Level2
   true
   
..\compat;$(SolutionDir)include;$(SolutionDir);%(AdditionalIncludeDirectories)
+  Level2
+  /ZH:SHA_256 %(AdditionalOptions)
 
 
 
@@ -177,11 +179,12 @@
   
   
 
-  
_CONSOLE;%(PreprocessorDefinitions)
+  
_CRT_NONSTDC_NO_DEPRECATE;_CRT_SECURE_NO_WARNINGS;_WINSOCK_DEPRECATED_NO_WARNINGS;_CONSOLE;%(PreprocessorDefinitions)
   
%(UndefinePreprocessorDefinitions)
-  Level2
   true
   
..\compat;$(SolutionDir);%(AdditionalIncludeDirectories)
+  Level2
+  /ZH:SHA_256 %(AdditionalOptions)
 
 
 
@@ -192,44 +195,52 @@
   
   
 
-  
_CONSOLE;%(PreprocessorDefinitions)
+  
_CRT_NONSTDC_NO_DEPRECATE;_CRT_SECURE_NO_WARNINGS;_WINSOCK_DEPRECATED_NO_WARNINGS;_CONSOLE;%(PreprocessorDefinitions)
   
%(UndefinePreprocessorDefinitions)
-  Level2
   true
   
..\compat;$(SolutionDir);%(AdditionalIncludeDirectories)
   Guard
+  Level2
+  /ZH:SHA_256 %(AdditionalOptions)
+  true
 
 
 
   
Ncrypt.lib;gdi32.lib;ws2_32.lib;wininet.lib;crypt32.lib;iphlpapi.lib;winmm.lib;Fwpuclnt.lib;Rpcrt4.lib;setupapi.lib;Advapi32.lib
   
$(OPENSSL_HOME)/lib;$(LZO_HOME)/lib;$(PKCS11H_HOME)/lib;%(AdditionalLibraryDirectories)
   Console
+  true
 
   
   
 
-  
_CONSOLE;%(PreprocessorDefinitions)
+  
_CRT_NONSTDC_NO_DEPRECATE;_CRT_SECURE_NO_WARNINGS;_WINSOCK_DEPRECATED_NO_WARNINGS;_CONSOLE;%(PreprocessorDefinitions)
   
%(UndefinePreprocessorDefinitions)
-  Level2
   true
   
..\compat;$(SolutionDir);%(AdditionalIncludeDirectories)
   Guard
+  true
+  Level2
+  /ZH:SHA_256 %(AdditionalOptions)
 
 
 
   
Ncrypt.lib;gdi32.lib;ws2_32.lib;wininet.lib;crypt32.lib;iphlpapi.lib;winmm.lib;Fwpuclnt.lib;Rpcrt4.lib;setupapi.lib;Advapi32.lib
   
$(OPENSSL_HOME)/lib;$(LZO_HOME)/lib;$(PKCS11H_HOME)/lib;%(AdditionalLibraryDirectories)
   Console
+  true
 
   
   
 
-  
_CONSOLE;%(PreprocessorDefinitions)
+  
_CRT_NONSTDC_NO_DEPRECATE;_CRT_SECURE_NO_WARNINGS;_WINSOCK_DEPRECATED_NO_WARNINGS;_CONSOLE;%(PreprocessorDefinitions)
   
%(UndefinePreprocessorDefinitions)
-  Level2
   true
   
..\compat;$(SolutionDir);%(AdditionalIncludeDirectories)
   Guard
+  Level2
+  /ZH:SHA_256 %(AdditionalOptions)
+  true
 
 
 
diff --git a/src/openvpnmsica/openvpnmsica.vcxproj 
b/src/openvpnmsica/openvpnmsica.vcxproj
index 11aa78bb..5e774430 100644
--- a/src/openvpnmsica/openvpnmsica.vcxproj
+++ b/src/openvpnmsica/openvpnmsica.vcxproj
@@ -135,6 +135,49 @@
   
 true
   
+  
+
+  true
+
+
+  /ZH:SHA_256 %(AdditionalOptions)
+  
%(PreprocessorDefinitions)
+
+  
+  
+
+  true
+
+
+  /ZH:SHA_256 %(AdditionalOptions)
+  true
+  
%(PreprocessorDefinitions)
+
+  
+  
+
+  /ZH:SHA_256 %(AdditionalOptions)
+  
%(PreprocessorDefinitions)
+
+  
+  
+
+  /ZH:SHA_256 %(AdditionalOptions)
+  
%(PreprocessorDefinitions)
+
+  
+  
+
+  /ZH:SHA_256 %(AdditionalOptions)
+  

Re: [Openvpn-devel] [PATCH] auth_token.c: add NULL initialization

2022-01-07 Thread Arne Schwabe

Am 07.01.22 um 13:35 schrieb Lev Stipakov:

From: Lev Stipakov 

This fixes

   error C4703: potentially uninitialized local pointer variable 'b64output' 
used

found by arm64 msvc compiler with SDL enabled.

Not sure why this is not triggered on x86/x64.

Signed-off-by: Lev Stipakov 
---
  src/openvpn/auth_token.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/openvpn/auth_token.c b/src/openvpn/auth_token.c
index e8875464..ceae68f6 100644
--- a/src/openvpn/auth_token.c
+++ b/src/openvpn/auth_token.c
@@ -259,7 +259,7 @@ generate_auth_token(const struct user_pass *up, struct 
tls_multi *multi)
  ASSERT(buf_write(, , sizeof(timestamp)));
  ASSERT(buf_write(, hmac_output, sizeof(hmac_output)));
  
-char *b64output;

+char *b64output = NULL;
  openvpn_base64_encode(BPTR(), BLEN(), );
  
  struct buffer session_token = alloc_buf_gc(


Fine with me

Acked-By: Arne Schwabe 


___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel


Re: [Openvpn-devel] [PATCH] auth_token.c: add NULL initialization

2022-01-07 Thread Antonio Quartulli

Hi,

On 07/01/2022 13:35, Lev Stipakov wrote:

From: Lev Stipakov 

This fixes

   error C4703: potentially uninitialized local pointer variable 'b64output' 
used

found by arm64 msvc compiler with SDL enabled.

Not sure why this is not triggered on x86/x64.

Signed-off-by: Lev Stipakov 
---
  src/openvpn/auth_token.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/openvpn/auth_token.c b/src/openvpn/auth_token.c
index e8875464..ceae68f6 100644
--- a/src/openvpn/auth_token.c
+++ b/src/openvpn/auth_token.c
@@ -259,7 +259,7 @@ generate_auth_token(const struct user_pass *up, struct 
tls_multi *multi)
  ASSERT(buf_write(, , sizeof(timestamp)));
  ASSERT(buf_write(, hmac_output, sizeof(hmac_output)));
  
-char *b64output;

+char *b64output = NULL;
  openvpn_base64_encode(BPTR(), BLEN(), );


It's impossible to leave b64output uninitialized, but the compiler is 
probably not smart enough to understand it.


On the other hand, passing uninitialized variables by reference to a 
function (without checking its return value) and using them later is 
never a good pattern..


Acked-by: Antonio Quartulli 


--
Antonio Quartulli


___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel


[Openvpn-devel] [PATCH] auth_token.c: add NULL initialization

2022-01-07 Thread Lev Stipakov
From: Lev Stipakov 

This fixes

  error C4703: potentially uninitialized local pointer variable 'b64output' used

found by arm64 msvc compiler with SDL enabled.

Not sure why this is not triggered on x86/x64.

Signed-off-by: Lev Stipakov 
---
 src/openvpn/auth_token.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/openvpn/auth_token.c b/src/openvpn/auth_token.c
index e8875464..ceae68f6 100644
--- a/src/openvpn/auth_token.c
+++ b/src/openvpn/auth_token.c
@@ -259,7 +259,7 @@ generate_auth_token(const struct user_pass *up, struct 
tls_multi *multi)
 ASSERT(buf_write(, , sizeof(timestamp)));
 ASSERT(buf_write(, hmac_output, sizeof(hmac_output)));
 
-char *b64output;
+char *b64output = NULL;
 openvpn_base64_encode(BPTR(), BLEN(), );
 
 struct buffer session_token = alloc_buf_gc(
-- 
2.23.0.windows.1



___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel


[Openvpn-devel] [PATCH] msvc: mark x64 release binaries as compatible with CET shadow stack

2022-01-07 Thread Lev Stipakov
From: Lev Stipakov 

This provides hardware-enforced stack protection on compatible 
hardware/software.

This is based on patch from Ilya Shipitsin 
https://patchwork.openvpn.net/patch/1987/

See 
https://techcommunity.microsoft.com/t5/windows-kernel-internals-blog/developer-guidance-for-hardware-enforced-stack-protection/ba-p/2163340
 for more info.

Signed-off-by: Lev Stipakov 
---
 src/openvpn/openvpn.vcxproj   | 1 +
 src/openvpnmsica/openvpnmsica.vcxproj | 5 +
 src/openvpnserv/openvpnserv.vcxproj   | 1 +
 src/tapctl/tapctl.vcxproj | 6 +-
 4 files changed, 12 insertions(+), 1 deletion(-)

diff --git a/src/openvpn/openvpn.vcxproj b/src/openvpn/openvpn.vcxproj
index d583c281..fb08c1c7 100644
--- a/src/openvpn/openvpn.vcxproj
+++ b/src/openvpn/openvpn.vcxproj
@@ -220,6 +220,7 @@
   
Ncrypt.lib;gdi32.lib;ws2_32.lib;wininet.lib;crypt32.lib;iphlpapi.lib;winmm.lib;Fwpuclnt.lib;Rpcrt4.lib;setupapi.lib;Advapi32.lib
   
$(OPENSSL_HOME)/lib;$(LZO_HOME)/lib;$(PKCS11H_HOME)/lib;%(AdditionalLibraryDirectories)
   Console
+  true
 
   
   
diff --git a/src/openvpnmsica/openvpnmsica.vcxproj 
b/src/openvpnmsica/openvpnmsica.vcxproj
index 11aa78bb..e7186e70 100644
--- a/src/openvpnmsica/openvpnmsica.vcxproj
+++ b/src/openvpnmsica/openvpnmsica.vcxproj
@@ -135,6 +135,11 @@
   
 true
   
+  
+
+  true
+
+  
   
 
 
diff --git a/src/openvpnserv/openvpnserv.vcxproj 
b/src/openvpnserv/openvpnserv.vcxproj
index 5fd7d60b..deed8db1 100644
--- a/src/openvpnserv/openvpnserv.vcxproj
+++ b/src/openvpnserv/openvpnserv.vcxproj
@@ -174,6 +174,7 @@
 
   
legacy_stdio_definitions.lib;Userenv.lib;Iphlpapi.lib;ntdll.lib;Fwpuclnt.lib;Netapi32.lib;Shlwapi.lib;%(AdditionalDependencies)
   Console
+  true
 
   
   
diff --git a/src/tapctl/tapctl.vcxproj b/src/tapctl/tapctl.vcxproj
index 79da9d33..da9f2703 100644
--- a/src/tapctl/tapctl.vcxproj
+++ b/src/tapctl/tapctl.vcxproj
@@ -140,7 +140,11 @@
   
   
   
-  
+  
+
+  true
+
+  
   
 
 
-- 
2.23.0.windows.1



___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel