Re: [Openvpn-devel] [PATCH v4 4/5] Move NCP related function into a seperate file and add unit tests

2020-02-20 Thread David Sommerseth
On 17/02/2020 15:43, Arne Schwabe wrote:
> This allows unit test the NCP functions. The ssl.c file has too
> many dependencies to make unit testing of it viable.
> 
> Patch V2: Removing the include "ssl_ncp.h" from options.c for V2 of
>   implement dynamic NCP forces a new version of this patch to
>   add the #include in this patch. Merge VS studio file changes
>   for ssl_ncp.[ch] into this patch
> 
> Patch V3: Regenerate for changes in earlier patches, apply Lev's changes
>   to Visual Studio project file
> 
> Patch V4: Regenerate to also have the changes of earlier patches.
> 
> Signed-off-by: Arne Schwabe 
> ---
>  src/openvpn/Makefile.am  |   1 +
>  src/openvpn/init.c   |   1 +
>  src/openvpn/multi.c  |   1 +
>  src/openvpn/openvpn.vcxproj  |   2 +
>  src/openvpn/openvpn.vcxproj.filters  |   8 +-
>  src/openvpn/options.c|   1 +
>  src/openvpn/push.c   |   1 +
>  src/openvpn/ssl.c| 176 +---
>  src/openvpn/ssl.h|  65 
>  src/openvpn/ssl_ncp.c| 231 +++
>  src/openvpn/ssl_ncp.h| 101 
>  tests/unit_tests/openvpn/Makefile.am |  18 ++-
>  tests/unit_tests/openvpn/test_ncp.c  | 179 +
>  13 files changed, 544 insertions(+), 241 deletions(-)
>  create mode 100644 src/openvpn/ssl_ncp.c
>  create mode 100644 src/openvpn/ssl_ncp.h
>  create mode 100644 tests/unit_tests/openvpn/test_ncp.c
> 
Sorry, but this gets a NAK from me.

$ ./tests/unit_tests/openvpn/ncp_testdriver
[==] Running 4 test(s).
[ RUN  ] test_check_ncp_ciphers_list
Unsupported cipher in --ncp-ciphers: AES-256-GCM
Unsupported cipher in --ncp-ciphers: AES-128-GCM
[  ERROR   ] --- tls_check_ncp_cipher_list(aes_ciphers)
[   LINE   ] --- test_ncp.c:50: error: Failure!
[  FAILED  ] test_check_ncp_ciphers_list
[ RUN  ] test_extract_client_ciphers
[   OK ] test_extract_client_ciphers
[ RUN  ] test_poor_man
[   OK ] test_poor_man
[ RUN  ] test_ncp_best
[   OK ] test_ncp_best
[==] 4 test(s) run.
[  PASSED  ] 3 test(s).
[  FAILED  ] 1 test(s), listed below:
[  FAILED  ] test_check_ncp_ciphers_list

 1 FAILED TEST(S)

We can't have any failing tests ;-)

This is tested on RHEL-7.7 (openssl-1.0.2k-19) which I also do know have
AES-GCM support.


-- 
kind regards,

David Sommerseth
OpenVPN Inc



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


Re: [Openvpn-devel] [PATCH v4 4/5] Move NCP related function into a seperate file and add unit tests

2020-02-17 Thread Lev Stipakov
Stared at the code, built and tested on MSVC and Ubuntu 18.

Acked-by: Lev Stipakov 


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


[Openvpn-devel] [PATCH v4 4/5] Move NCP related function into a seperate file and add unit tests

2020-02-17 Thread Arne Schwabe
This allows unit test the NCP functions. The ssl.c file has too
many dependencies to make unit testing of it viable.

Patch V2: Removing the include "ssl_ncp.h" from options.c for V2 of
  implement dynamic NCP forces a new version of this patch to
  add the #include in this patch. Merge VS studio file changes
  for ssl_ncp.[ch] into this patch

Patch V3: Regenerate for changes in earlier patches, apply Lev's changes
  to Visual Studio project file

Patch V4: Regenerate to also have the changes of earlier patches.

Signed-off-by: Arne Schwabe 
---
 src/openvpn/Makefile.am  |   1 +
 src/openvpn/init.c   |   1 +
 src/openvpn/multi.c  |   1 +
 src/openvpn/openvpn.vcxproj  |   2 +
 src/openvpn/openvpn.vcxproj.filters  |   8 +-
 src/openvpn/options.c|   1 +
 src/openvpn/push.c   |   1 +
 src/openvpn/ssl.c| 176 +---
 src/openvpn/ssl.h|  65 
 src/openvpn/ssl_ncp.c| 231 +++
 src/openvpn/ssl_ncp.h| 101 
 tests/unit_tests/openvpn/Makefile.am |  18 ++-
 tests/unit_tests/openvpn/test_ncp.c  | 179 +
 13 files changed, 544 insertions(+), 241 deletions(-)
 create mode 100644 src/openvpn/ssl_ncp.c
 create mode 100644 src/openvpn/ssl_ncp.h
 create mode 100644 tests/unit_tests/openvpn/test_ncp.c

diff --git a/src/openvpn/Makefile.am b/src/openvpn/Makefile.am
index d1bb99c2..2ea47cda 100644
--- a/src/openvpn/Makefile.am
+++ b/src/openvpn/Makefile.am
@@ -116,6 +116,7 @@ openvpn_SOURCES = \
ssl.c ssl.h  ssl_backend.h \
ssl_openssl.c ssl_openssl.h \
ssl_mbedtls.c ssl_mbedtls.h \
+   ssl_ncp.c ssl_ncp.h \
ssl_common.h \
ssl_verify.c ssl_verify.h ssl_verify_backend.h \
ssl_verify_openssl.c ssl_verify_openssl.h \
diff --git a/src/openvpn/init.c b/src/openvpn/init.c
index 9363769f..d83d4366 100644
--- a/src/openvpn/init.c
+++ b/src/openvpn/init.c
@@ -49,6 +49,7 @@
 #include "ping.h"
 #include "mstats.h"
 #include "ssl_verify.h"
+#include "ssl_ncp.h"
 #include "tls_crypt.h"
 #include "forward.h"
 #include "auth_token.h"
diff --git a/src/openvpn/multi.c b/src/openvpn/multi.c
index d594dd25..b82c68c4 100644
--- a/src/openvpn/multi.c
+++ b/src/openvpn/multi.c
@@ -45,6 +45,7 @@
 #include "gremlin.h"
 #include "mstats.h"
 #include "ssl_verify.h"
+#include "ssl_ncp.h"
 #include "vlan.h"
 #include 
 
diff --git a/src/openvpn/openvpn.vcxproj b/src/openvpn/openvpn.vcxproj
index 614d720a..53ac5482 100644
--- a/src/openvpn/openvpn.vcxproj
+++ b/src/openvpn/openvpn.vcxproj
@@ -192,6 +192,7 @@
 
 
 
+
 
 
 
@@ -278,6 +279,7 @@
 
 
 
+
 
 
 
diff --git a/src/openvpn/openvpn.vcxproj.filters 
b/src/openvpn/openvpn.vcxproj.filters
index 41e62d14..80eb52b3 100644
--- a/src/openvpn/openvpn.vcxproj.filters
+++ b/src/openvpn/openvpn.vcxproj.filters
@@ -243,6 +243,9 @@
 
   Source Files
 
+
+  Source Files
+
   
   
 
@@ -506,10 +509,13 @@
 
   Header Files
 
+
+  Header Files
+
   
   
 
   Resource Files
 
   
-
+
\ No newline at end of file
diff --git a/src/openvpn/options.c b/src/openvpn/options.c
index c459b260..d057975c 100644
--- a/src/openvpn/options.c
+++ b/src/openvpn/options.c
@@ -45,6 +45,7 @@
 #include "shaper.h"
 #include "crypto.h"
 #include "ssl.h"
+#include "ssl_ncp.h"
 #include "options.h"
 #include "misc.h"
 #include "socket.h"
diff --git a/src/openvpn/push.c b/src/openvpn/push.c
index 8b634051..71f22e94 100644
--- a/src/openvpn/push.c
+++ b/src/openvpn/push.c
@@ -33,6 +33,7 @@
 #include "options.h"
 #include "ssl.h"
 #include "ssl_verify.h"
+#include "ssl_ncp.h"
 #include "manage.h"
 
 #include "memdbg.h"
diff --git a/src/openvpn/ssl.c b/src/openvpn/ssl.c
index 51b03c3b..e21279f1 100644
--- a/src/openvpn/ssl.c
+++ b/src/openvpn/ssl.c
@@ -59,6 +59,7 @@
 #include "ssl.h"
 #include "ssl_verify.h"
 #include "ssl_backend.h"
+#include "ssl_ncp.h"
 #include "auth_token.h"
 
 #include "memdbg.h"
@@ -67,6 +68,7 @@
 static const char ssl_default_options_string[] = "V0 UNDEF";
 #endif
 
+
 static inline const char *
 local_options_string(const struct tls_session *session)
 {
@@ -1914,119 +1916,6 @@ key_ctx_update_implicit_iv(struct key_ctx *ctx, uint8_t 
*key, size_t key_len)
 }
 }
 
-bool
-tls_item_in_cipher_list(const char *item, const char *list)
-{
-char *tmp_ciphers = string_alloc(list, NULL);
-char *tmp_ciphers_orig = tmp_ciphers;
-
-const char *token = strtok(tmp_ciphers, ":");
-while (token)
-{
-if (0 == strcmp(token, item))
-{
-break;
-}
-token = strtok(NULL, ":");
-}
-free(tmp_ciphers_orig);
-
-return token != NULL;
-}
-
-const char *
-tls_peer_ncp_list(const char *peer_info, struct gc_arena *gc)
-{
-/* Check if the peer