Re: [Openvpn-devel] [PATCH v2] Add missing check for nl_socket_alloc failure

2023-03-29 Thread Antonio Quartulli




On 29/03/2023 14:46, Arne Schwabe wrote:

This can happen if the memory alloc fails.

Patch V2: add goto error

Change-Id: Iee66caa794d267ac5f8bee584633352893047171
Signed-off-by: Arne Schwabe 
---
  src/openvpn/dco_linux.c | 7 +++
  1 file changed, 7 insertions(+)

diff --git a/src/openvpn/dco_linux.c b/src/openvpn/dco_linux.c
index 41540c0f8..95fe94848 100644
--- a/src/openvpn/dco_linux.c
+++ b/src/openvpn/dco_linux.c
@@ -83,6 +83,13 @@ resolve_ovpn_netlink_id(int msglevel)
  int ret;
  struct nl_sock *nl_sock = nl_socket_alloc();
  
+if (!nl_sock)

+{
+msg(msglevel, "Allocating net link socket failed");
+ret = -1;


Please use -ENOMEM here - it is always better to return an actual reason 
rather than just "failed".



+goto err_sock;


There is no need to jump to cleanup.
You can just return -1 here and save one line.
(this is what we do in other functions of this file)

Cheers,

--
Antonio Quartulli


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


[Openvpn-devel] [PATCH v2] Add missing check for nl_socket_alloc failure

2023-03-29 Thread Arne Schwabe
This can happen if the memory alloc fails.

Patch V2: add goto error

Change-Id: Iee66caa794d267ac5f8bee584633352893047171
Signed-off-by: Arne Schwabe 
---
 src/openvpn/dco_linux.c | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/src/openvpn/dco_linux.c b/src/openvpn/dco_linux.c
index 41540c0f8..95fe94848 100644
--- a/src/openvpn/dco_linux.c
+++ b/src/openvpn/dco_linux.c
@@ -83,6 +83,13 @@ resolve_ovpn_netlink_id(int msglevel)
 int ret;
 struct nl_sock *nl_sock = nl_socket_alloc();
 
+if (!nl_sock)
+{
+msg(msglevel, "Allocating net link socket failed");
+ret = -1;
+goto err_sock;
+}
+
 ret = genl_connect(nl_sock);
 if (ret)
 {
-- 
2.37.1 (Apple Git-137.1)



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


[Openvpn-devel] [PATCH] Route: add support for user defined routing table

2023-03-29 Thread Gianmarco De Gregori
Add the ability for users to specify a custom
routing table where routes should be installed in.
As of now routes are always installed in the main
routing table of the operating system, however,
with the new --route-table option it is possibile
to specify the ID of the default routing table
to be used by --route(-ipv6).

The --route(-ipv6) directives have been extended
with an additional argument (5th for --route)
(4th for --route-ipv6) so that each of them
can possibly use an independent routing table.

Please note: this feature is currently supported
only by Linux/SITNL.
Support for other platforms should be added in related backends.

Signed-off-by: Gianmarco De Gregori 
---
 doc/man-sections/vpn-network-options.rst |  16 +++-
 src/openvpn/helper.c |   1 +
 src/openvpn/init.c   |  15 +++-
 src/openvpn/options.c|  44 +-
 src/openvpn/options.h|   1 +
 src/openvpn/route.c  | 101 +--
 src/openvpn/route.h  |  17 +++-
 7 files changed, 179 insertions(+), 16 deletions(-)

diff --git a/doc/man-sections/vpn-network-options.rst 
b/doc/man-sections/vpn-network-options.rst
index 8e3c92ee..c25bbf31 100644
--- a/doc/man-sections/vpn-network-options.rst
+++ b/doc/man-sections/vpn-network-options.rst
@@ -367,6 +367,14 @@ routing.
   Like ``--redirect-gateway``, but omit actually changing the default gateway.
   Useful when pushing private subnets.
 
+--route-table id
+  Specify a default table id for use with --route.
+  By default, OpenVPN installs routes in the main routing
+  table of the operating system, but with this option,
+  a user defined routing table can be used instead.
+
+  (Supported on Linux only, on other platforms this is a no-op).
+
 --route args
   Add route to routing table after connection is established. Multiple
   routes can be specified. Routes will be automatically torn down in
@@ -379,6 +387,7 @@ routing.
   route network/IP netmask
   route network/IP netmask gateway
   route network/IP netmask gateway metric
+  route network/IP netmask gateway metric table-id
 
   This option is intended as a convenience proxy for the ``route``\(8)
   shell command, while at the same time providing portable semantics
@@ -394,6 +403,9 @@ routing.
   ``metric``
 default taken from ``--route-metric`` if set, otherwise :code:`0`.
 
+  ``table-id`` (Supported on Linux only, on other platforms this is a no-op).
+   default taken from ``--route-table`` if set, otherwise :code:`0`.
+
   The default can be specified by leaving an option blank or setting it to
   :code:`default`.
 
@@ -444,12 +456,14 @@ routing.
   Valid syntax:
   ::
 
- route-ipv6 ipv6addr/bits [gateway] [metric]
+ route-ipv6 ipv6addr/bits [gateway] [metric] [table-id]
 
   The gateway parameter is only used for IPv6 routes across *tap* devices,
   and if missing, the ``ipv6remote`` field from ``--ifconfig-ipv6`` or
   ``--route-ipv6-gateway`` is used.
 
+  (table-id supported on Linux only, on other platforms this is a no-op).
+
 --route-gateway arg
   Specify a default *gateway* for use with ``--route``.
 
diff --git a/src/openvpn/helper.c b/src/openvpn/helper.c
index 7c219fdf..4a0e0d85 100644
--- a/src/openvpn/helper.c
+++ b/src/openvpn/helper.c
@@ -120,6 +120,7 @@ helper_add_route(const in_addr_t network, const in_addr_t 
netmask, struct option
  print_in_addr_t(network, 0, >gc),
  print_in_addr_t(netmask, 0, >gc),
  NULL,
+ NULL,
  NULL);
 }
 
diff --git a/src/openvpn/init.c b/src/openvpn/init.c
index d358ad00..8220eb93 100644
--- a/src/openvpn/init.c
+++ b/src/openvpn/init.c
@@ -1504,6 +1504,7 @@ do_init_route_list(const struct options *options,
 const char *gw = NULL;
 int dev = dev_type_enum(options->dev, options->dev_type);
 int metric = 0;
+uint32_t table_id = 0; /* unspec table */
 
 /* if DCO is enabled we have both regular routes and iroutes in the system
  * routing table, and normal routes must have a higher metric for that to
@@ -1522,6 +1523,10 @@ do_init_route_list(const struct options *options,
 {
 gw = options->route_default_gateway;
 }
+if (options->route_default_table_id)
+{
+table_id = options->route_default_table_id;
+}
 if (options->route_default_metric)
 {
 metric = options->route_default_metric;
@@ -1530,6 +1535,7 @@ do_init_route_list(const struct options *options,
 if (init_route_list(route_list,
 options->routes,
 gw,
+table_id,
 metric,
 link_socket_current_remote(link_socket_info),
 es,
@@ -1549,6 +1555,7 @@ do_init_route_ipv6_list(const struct options *options,
 {
 const 

[Openvpn-devel] [PATCH applied] Re: Make cert_data.h and test_cryptoapi/pkcs11.c MSVC compliant

2023-03-29 Thread Gert Doering
I have only loosely followed the discussion, but since this has an ACK
*and* passes all GHA compiles and tests, this is good enough for me :-)

I have added the "co-authored-by" as requested.

Your patch has been applied to the master branch.

commit 846951665a60424b98097ad0a77ec6cb1c3d05ac
Author: Selva Nair
Date:   Mon Mar 27 07:49:37 2023 -0400

 Make cert_data.h and test_cryptoapi/pkcs11.c MSVC compliant

 Signed-off-by: Selva Nair 
 Acked-by: Frank Lichtenheld 
 Message-Id: <20230327114937.28246-1-selva.n...@gmail.com>
 URL: 
https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg26525.html
 Signed-off-by: Gert Doering 


--
kind regards,

Gert Doering



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


[Openvpn-devel] [PATCH applied] Re: Unit tests: Test for PKCS#11 using a softhsm2 token

2023-03-29 Thread Gert Doering
Having pkcs#11 tests available is most welcome.  This said, I have not
really looked into "how can I make this do things for my test beds?",
but will do...

With the #if HAVE_SOFTHSM2, this does not actually do anything for
most build environments yet, but patch 3/3 will enable it for GHA Ubuntu
20/22 builds.

Your patch has been applied to the master branch.

commit 3013fde1c8290830d424b9f4ea84ee7c7dbfb75e
Author: Selva Nair
Date:   Wed Mar 22 18:14:55 2023 -0400

 Unit tests: Test for PKCS#11 using a softhsm2 token

 Signed-off-by: Selva Nair 
 Acked-by: Frank Lichtenheld 
 Message-Id: <2023031456.1660425-2-selva.n...@gmail.com>
 URL: 
https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg26483.html
 Signed-off-by: Gert Doering 


--
kind regards,

Gert Doering



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


[Openvpn-devel] [PATCH applied] Re: Enable pkcs11 an dtest_pkcs11 in github actions

2023-03-29 Thread Gert Doering
Ran this through GHA, it claims success, and claims having tested this...

[==] Running 3 test(s).
Slot 0 has a free/uninitialized token.
The token has been initialized and is reassigned to slot 347291425
[ RUN  ] test_pkcs11_ids
[   OK ] test_pkcs11_ids
[ RUN  ] test_tls_ctx_use_pkcs11
[   OK ] test_tls_ctx_use_pkcs11
[ RUN  ] test_tls_ctx_use_pkcs11__management
[   OK ] test_tls_ctx_use_pkcs11__management
Found token (b36c3fa5-a027-3fc0-2be4-05ee94b33f21) with matching token label.
The token (softhsm2_tokens_53u42S/b36c3fa5-a027-3fc0-2be4-05ee94b33f21) has 
been deleted.
[  PASSED  ] 3 test(s).
[==] 3 test(s) run.
PASS: pkcs11_testdriver

.. very nice!

Your patch has been applied to the master branch.

commit 9283c3980ff543e20f76fdfb4f4e59d5a9162d62
Author: Selva Nair
Date:   Wed Mar 22 18:14:56 2023 -0400

 Enable pkcs11 an dtest_pkcs11 in github actions

 Signed-off-by: Selva Nair 
 Acked-by: Frank Lichtenheld 
 Message-Id: <2023031456.1660425-3-selva.n...@gmail.com>
 URL: 
https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg26485.html
 Signed-off-by: Gert Doering 


--
kind regards,

Gert Doering



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


[Openvpn-devel] [PATCH 2/2] test_buffer: add tests for buf_catrunc and its caller format_hex_ex

2023-03-29 Thread Frank Lichtenheld
Just some very basic tests.

Signed-off-by: Frank Lichtenheld 
---
 tests/unit_tests/openvpn/test_buffer.c | 59 ++
 1 file changed, 59 insertions(+)

diff --git a/tests/unit_tests/openvpn/test_buffer.c 
b/tests/unit_tests/openvpn/test_buffer.c
index 9e3b1d2e..5e61fb07 100644
--- a/tests/unit_tests/openvpn/test_buffer.c
+++ b/tests/unit_tests/openvpn/test_buffer.c
@@ -56,6 +56,63 @@ test_buffer_strprefix(void **state)
 assert_int_equal(BLEN(buf), strlen(str)); \
 assert_memory_equal(BPTR(buf), str, BLEN(buf));
 
+static void
+test_buffer_printf_catrunc(void **state)
+{
+struct gc_arena gc = gc_new();
+struct buffer buf = alloc_buf_gc(16, );
+
+buf_printf(, "%d", 123);
+buf_printf(, "%s", "some text, too long to fit");
+assert_buf_equals_str(, "123some text, t");
+
+buf_catrunc(, "...");
+assert_buf_equals_str(, "123some text...");
+
+buf_catrunc(, "some other text, much too long to fit");
+assert_buf_equals_str(, "123some text...");
+
+buf_catrunc(, "something other");
+assert_buf_equals_str(, "something other");
+
+gc_free();
+}
+
+static void
+test_buffer_format_hex_ex(void **state)
+{
+const int input_size = 10;
+const uint8_t input[] = {
+0x01, 0x00, 0xff, 0x10, 0xff, 0x00, 0xf0, 0x0f, 0x09, 0x0a
+};
+char *output;
+struct gc_arena gc = gc_new();
+
+int maxoutput = 0;
+unsigned int blocksize = 5;
+char *separator = " ";
+output = format_hex_ex(input, input_size, maxoutput, blocksize, separator, 
);
+assert_string_equal(output, "0100ff10ff 00f00f090a");
+
+maxoutput = 14;
+output = format_hex_ex(input, input_size, maxoutput, blocksize, separator, 
);
+assert_string_equal(output, "0100[more...]");
+
+maxoutput = 10;
+output = format_hex_ex(input, input_size, maxoutput, blocksize, separator, 
);
+assert_string_equal(output, "[more...]");
+
+maxoutput = 9;
+output = format_hex_ex(input, input_size, maxoutput, blocksize, separator, 
);
+assert_string_equal(output, "0100ff10");
+
+maxoutput = 8;
+output = format_hex_ex(input, input_size, maxoutput, blocksize, separator, 
);
+assert_string_equal(output, "0100ff1");
+
+gc_free();
+}
+
 struct test_buffer_list_aggregate_ctx {
 struct buffer_list *empty;
 struct buffer_list *one_two_three;
@@ -267,6 +324,8 @@ main(void)
 {
 const struct CMUnitTest tests[] = {
 cmocka_unit_test(test_buffer_strprefix),
+cmocka_unit_test(test_buffer_printf_catrunc),
+cmocka_unit_test(test_buffer_format_hex_ex),
 
cmocka_unit_test_setup_teardown(test_buffer_list_aggregate_separator_empty,
 test_buffer_list_setup,
 test_buffer_list_teardown),
-- 
2.34.1



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


[Openvpn-devel] [PATCH 1/2 v2] buffer: use memcpy in buf_catrunc

2023-03-29 Thread Frank Lichtenheld
Since we use strlen() to determine the length
and then check it ourselves, there is really
no point in using strncpy.

But the compiler might complain that we use
the output of strlen() for the length of
strncpy which is usually a sign for bugs:

error: ‘strncpy’ specified bound depends
 on the length of the source argument
 [-Werror=stringop-overflow=]

Warning was at least triggered for
mingw-gcc version 10-win32 20220113.

Signed-off-by: Frank Lichtenheld 
---
 src/openvpn/buffer.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

v2:
 - make len size_t and change code to avoid any theoretical overflows
 - remove useless casts

diff --git a/src/openvpn/buffer.c b/src/openvpn/buffer.c
index d099795b..886eb2c3 100644
--- a/src/openvpn/buffer.c
+++ b/src/openvpn/buffer.c
@@ -316,10 +316,10 @@ buf_catrunc(struct buffer *buf, const char *str)
 {
 if (buf_forward_capacity(buf) <= 1)
 {
-int len = (int) strlen(str) + 1;
+size_t len = strlen(str);
 if (len < buf_forward_capacity_total(buf))
 {
-strncpynt((char *)(buf->data + buf->capacity - len), str, len);
+memcpy(buf->data + buf->capacity - len - 1, str, len + 1);
 }
 }
 }
-- 
2.34.1



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


[Openvpn-devel] [PATCH applied] Re: Move digest_sign_verify out of test_cryptoapi.c

2023-03-29 Thread Gert Doering
"git show --color-moved=zebra" agrees with "this is just moving stuff
around".  I have tested that "make dist" picks up the new file (this
is something we tend to get wrong), and ran this by GHA (pass).

At this point in the 2.6 life cycle, I think we can go back to the
"everything goes to master, only bugfixes go to release/2.6" way of
doing things - so this patchset (and subsequent new features for
cryptoapi that benefit from the unit test) would only be "in master".

I am bringing this up at the community meeting today (and here in the
mail) so you all have a chance to yell at me and convince me otherwise.

Your patch has been applied to the master branch.

commit 3bc071a8e3e21250f9be9c29273de0d96288056e (master)
Author: Selva Nair
Date:   Wed Mar 22 18:14:54 2023 -0400

 Move digest_sign_verify out of test_cryptoapi.c

 Signed-off-by: Selva Nair 
 Acked-by: Frank Lichtenheld 
 Message-Id: <2023031456.1660425-1-selva.n...@gmail.com>
 URL: 
https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg26484.html
 Signed-off-by: Gert Doering 


--
kind regards,

Gert Doering



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