openvpn-devel — OpenVPN developers list

[Openvpn-devel] [S] Change in openvpn[master]: dco_linux: enable extended netlink error reporting

[cron2 (C. Review) <ge...@op...>]

Attention is currently required from: flichtenheld, ordex, plaisthos.

cron2 has posted comments on this change. ( http://gerrit.openvpn.net/c/openvpn/+/1040?usp=email )

Change subject: dco_linux: enable extended netlink error reporting
......................................................................


Patch Set 2: Code-Review+2


-- 
To view, visit http://gerrit.openvpn.net/c/openvpn/+/1040?usp=email
To unsubscribe, or for help writing mail filters, visit http://gerrit.openvpn.net/settings

Gerrit-Project: openvpn
Gerrit-Branch: master
Gerrit-Change-Id: I4457b1d7262e0a39c275d33aaef6c4bcbeae6ab3
Gerrit-Change-Number: 1040
Gerrit-PatchSet: 2
Gerrit-Owner: ordex <an...@ma...>
Gerrit-Reviewer: cron2 <ge...@gr...>
Gerrit-Reviewer: flichtenheld <fr...@li...>
Gerrit-Reviewer: plaisthos <arn...@rf...>
Gerrit-CC: openvpn-devel <ope...@li...>
Gerrit-Attention: plaisthos <arn...@rf...>
Gerrit-Attention: flichtenheld <fr...@li...>
Gerrit-Attention: ordex <an...@ma...>
Gerrit-Comment-Date: Mon, 09 Jun 2025 10:01:09 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment

[Openvpn-devel] [S] Change in openvpn[master]: dco_linux: enable extended netlink error reporting

[ordex (C. Review) <ge...@op...>]

Attention is currently required from: cron2, flichtenheld, ordex, plaisthos.

Hello cron2, flichtenheld, plaisthos, 

I'd like you to reexamine a change. Please visit

    http://gerrit.openvpn.net/c/openvpn/+/1040?usp=email

to look at the new patch set (#2).

The following approvals got outdated and were removed:
Code-Review-1 by cron2


Change subject: dco_linux: enable extended netlink error reporting
......................................................................

dco_linux: enable extended netlink error reporting

The ovpn netlink code reports more verbose error
strings to help userspace understand what went wrong,
rather than just returning, for example, -EINVAL.

However, userspace must instruct the kernel netlink
subsystem that it wants to receive such strings.

code for parsing such strings has always been present
but it was never used.

Set the socket option which enables such reporting.

Change-Id: I4457b1d7262e0a39c275d33aaef6c4bcbeae6ab3
Signed-off-by: Antonio Quartulli <an...@ma...>
---
M src/openvpn/dco_linux.c
1 file changed, 8 insertions(+), 3 deletions(-)


  git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/40/1040/2

diff --git a/src/openvpn/dco_linux.c b/src/openvpn/dco_linux.c
index 49dbdad..0345413 100644
--- a/src/openvpn/dco_linux.c
+++ b/src/openvpn/dco_linux.c
@@ -367,19 +367,19 @@
     {
         len = strnlen((char *)nla_data(tb_msg[NLMSGERR_ATTR_MSG]),
                       nla_len(tb_msg[NLMSGERR_ATTR_MSG]));
-        msg(M_WARN, "kernel error: %*s\n", len,
+        msg(M_WARN, "kernel error: %*s", len,
             (char *)nla_data(tb_msg[NLMSGERR_ATTR_MSG]));
     }
 
     if (tb_msg[OVPN_NLMSGERR_ATTR_MISS_NEST])
     {
-        msg(M_WARN, "kernel error: missing required nesting type %u\n",
+        msg(M_WARN, "kernel error: missing required nesting type %u",
             nla_get_u32(tb_msg[OVPN_NLMSGERR_ATTR_MISS_NEST]));
     }
 
     if (tb_msg[OVPN_NLMSGERR_ATTR_MISS_TYPE])
     {
-        msg(M_WARN, "kernel error: missing required attribute type %u\n",
+        msg(M_WARN, "kernel error: missing required attribute type %u",
             nla_get_u32(tb_msg[OVPN_NLMSGERR_ATTR_MISS_TYPE]));
     }
 
@@ -405,6 +405,11 @@
             nl_geterror(ret));
     }
 
+    /* enable Extended ACK for detailed error reporting */
+    ret = 1;
+    setsockopt(nl_socket_get_fd(dco->nl_sock), SOL_NETLINK, NETLINK_EXT_ACK,
+               &ret, sizeof(ret));
+
     /* set close on exec and non-block on the netlink socket */
     set_cloexec(nl_socket_get_fd(dco->nl_sock));
     set_nonblock(nl_socket_get_fd(dco->nl_sock));

-- 
To view, visit http://gerrit.openvpn.net/c/openvpn/+/1040?usp=email
To unsubscribe, or for help writing mail filters, visit http://gerrit.openvpn.net/settings

Gerrit-Project: openvpn
Gerrit-Branch: master
Gerrit-Change-Id: I4457b1d7262e0a39c275d33aaef6c4bcbeae6ab3
Gerrit-Change-Number: 1040
Gerrit-PatchSet: 2
Gerrit-Owner: ordex <an...@ma...>
Gerrit-Reviewer: cron2 <ge...@gr...>
Gerrit-Reviewer: flichtenheld <fr...@li...>
Gerrit-Reviewer: plaisthos <arn...@rf...>
Gerrit-CC: openvpn-devel <ope...@li...>
Gerrit-Attention: plaisthos <arn...@rf...>
Gerrit-Attention: cron2 <ge...@gr...>
Gerrit-Attention: flichtenheld <fr...@li...>
Gerrit-Attention: ordex <an...@ma...>
Gerrit-MessageType: newpatchset

[Openvpn-devel] [XS] Change in openvpn[master]: dco_linux: enable extended netlink error reporting

[cron2 (C. Review) <ge...@op...>]

Attention is currently required from: flichtenheld, ordex, plaisthos.

cron2 has posted comments on this change. ( http://gerrit.openvpn.net/c/openvpn/+/1040?usp=email )

Change subject: dco_linux: enable extended netlink error reporting
......................................................................


Patch Set 1: Code-Review-1

(1 comment)

Patchset:

PS1: 
This patch works and does what it says on the lid.  Alas, when we're starting to fix error message printing, we also need to fix it for good...

```
    if (tb_msg[NLMSGERR_ATTR_MSG])
    {
        len = strnlen((char *)nla_data(tb_msg[NLMSGERR_ATTR_MSG]),
                      nla_len(tb_msg[NLMSGERR_ATTR_MSG]));
        msg(M_WARN, "kernel error: %*s\n", len,
            (char *)nla_data(tb_msg[NLMSGERR_ATTR_MSG]));
    }
```

there is an extra `\n` which should not be there.  Please :-)



-- 
To view, visit http://gerrit.openvpn.net/c/openvpn/+/1040?usp=email
To unsubscribe, or for help writing mail filters, visit http://gerrit.openvpn.net/settings

Gerrit-Project: openvpn
Gerrit-Branch: master
Gerrit-Change-Id: I4457b1d7262e0a39c275d33aaef6c4bcbeae6ab3
Gerrit-Change-Number: 1040
Gerrit-PatchSet: 1
Gerrit-Owner: ordex <an...@ma...>
Gerrit-Reviewer: cron2 <ge...@gr...>
Gerrit-Reviewer: flichtenheld <fr...@li...>
Gerrit-Reviewer: plaisthos <arn...@rf...>
Gerrit-CC: openvpn-devel <ope...@li...>
Gerrit-Attention: plaisthos <arn...@rf...>
Gerrit-Attention: flichtenheld <fr...@li...>
Gerrit-Attention: ordex <an...@ma...>
Gerrit-Comment-Date: Fri, 06 Jun 2025 12:54:08 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment

[Openvpn-devel] [XS] Change in openvpn[master]: dco_linux: enable extended netlink error reporting

[ordex (C. Review) <ge...@op...>]

Attention is currently required from: flichtenheld, plaisthos.

Hello plaisthos, flichtenheld,

I'd like you to do a code review.
Please visit

    http://gerrit.openvpn.net/c/openvpn/+/1040?usp=email

to review the following change.


Change subject: dco_linux: enable extended netlink error reporting
......................................................................

dco_linux: enable extended netlink error reporting

The ovpn netlink code reports more verbose error
strings to help userspace understand what went wrong,
rather than just returning, for example, -EINVAL.

However, userspace must instruct the kernel netlink
subsystem that it wants to receive such strings.

code for parsing such strings has always been present
but it was never used.

Set the socket option which enables such reporting.

Change-Id: I4457b1d7262e0a39c275d33aaef6c4bcbeae6ab3
Signed-off-by: Antonio Quartulli <an...@ma...>
---
M src/openvpn/dco_linux.c
1 file changed, 5 insertions(+), 0 deletions(-)



  git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/40/1040/1

diff --git a/src/openvpn/dco_linux.c b/src/openvpn/dco_linux.c
index 49dbdad..fcca9dc 100644
--- a/src/openvpn/dco_linux.c
+++ b/src/openvpn/dco_linux.c
@@ -405,6 +405,11 @@
             nl_geterror(ret));
     }
 
+    /* enable Extended ACK for detailed error reporting */
+    ret = 1;
+    setsockopt(nl_socket_get_fd(dco->nl_sock), SOL_NETLINK, NETLINK_EXT_ACK,
+               &ret, sizeof(ret));
+
     /* set close on exec and non-block on the netlink socket */
     set_cloexec(nl_socket_get_fd(dco->nl_sock));
     set_nonblock(nl_socket_get_fd(dco->nl_sock));

-- 
To view, visit http://gerrit.openvpn.net/c/openvpn/+/1040?usp=email
To unsubscribe, or for help writing mail filters, visit http://gerrit.openvpn.net/settings

Gerrit-Project: openvpn
Gerrit-Branch: master
Gerrit-Change-Id: I4457b1d7262e0a39c275d33aaef6c4bcbeae6ab3
Gerrit-Change-Number: 1040
Gerrit-PatchSet: 1
Gerrit-Owner: ordex <an...@ma...>
Gerrit-Reviewer: flichtenheld <fr...@li...>
Gerrit-Reviewer: plaisthos <arn...@rf...>
Gerrit-CC: openvpn-devel <ope...@li...>
Gerrit-Attention: plaisthos <arn...@rf...>
Gerrit-Attention: flichtenheld <fr...@li...>
Gerrit-MessageType: newchange

[Openvpn-devel] [S] Change in openvpn[master]: Multi-socket: local_list clean-up

[its_Giaan (C. Review) <ge...@op...>]

Attention is currently required from: flichtenheld, plaisthos.

Hello plaisthos, flichtenheld,

I'd like you to do a code review.
Please visit

    http://gerrit.openvpn.net/c/openvpn/+/1039?usp=email

to review the following change.


Change subject: Multi-socket: local_list clean-up
......................................................................

Multi-socket: local_list clean-up

Optimize the current local_list implementation
by replacing the static array with a resizable
one, as the static allocation serves no real
purpose, particularly on the client side.

Github: #682

Change-Id: I32effed9e273fbe8986d1f4e8da4a4d0ac216463
Signed-off-by: Gianmarco De Gregori <gia...@ma...>
---
M src/openvpn/options.c
M src/openvpn/options.h
2 files changed, 14 insertions(+), 5 deletions(-)



  git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/39/1039/1

diff --git a/src/openvpn/options.c b/src/openvpn/options.c
index 6ea01d4..70337b1 100644
--- a/src/openvpn/options.c
+++ b/src/openvpn/options.c
@@ -2212,12 +2212,20 @@
     struct local_list *l = alloc_local_list_if_undef(ce, gc);
     struct local_entry *e;
 
-    if (l->len >= CONNECTION_LIST_SIZE)
+    if (l->len >= l->capacity)
     {
-        msg(msglevel, "Maximum number of 'local' options (%d) exceeded",
-            CONNECTION_LIST_SIZE);
+        const int new_cap = l->capacity + 1;
+        const size_t elem_size = sizeof(*l->array);
 
-        return NULL;
+        struct local_entry **new_array = gc_realloc(l->array, new_cap * elem_size, gc);
+        if (!new_array)
+        {
+            msg(msglevel, "Unable to process more local options: out of memory. Number of entries = %d", l->len);
+            return NULL;
+        }
+
+        l->array = new_array;
+        l->capacity = new_cap;
     }
 
     ALLOC_OBJ_CLEAR_GC(e, struct local_entry, gc);
diff --git a/src/openvpn/options.h b/src/openvpn/options.h
index b28ad58..46ec32b 100644
--- a/src/openvpn/options.h
+++ b/src/openvpn/options.h
@@ -188,8 +188,9 @@
 
 struct local_list
 {
+    int capacity;
     int len;
-    struct local_entry *array[CONNECTION_LIST_SIZE];
+    struct local_entry **array;
 };
 
 struct connection_list

-- 
To view, visit http://gerrit.openvpn.net/c/openvpn/+/1039?usp=email
To unsubscribe, or for help writing mail filters, visit http://gerrit.openvpn.net/settings

Gerrit-Project: openvpn
Gerrit-Branch: master
Gerrit-Change-Id: I32effed9e273fbe8986d1f4e8da4a4d0ac216463
Gerrit-Change-Number: 1039
Gerrit-PatchSet: 1
Gerrit-Owner: its_Giaan <gia...@ma...>
Gerrit-Reviewer: flichtenheld <fr...@li...>
Gerrit-Reviewer: plaisthos <arn...@rf...>
Gerrit-CC: openvpn-devel <ope...@li...>
Gerrit-Attention: plaisthos <arn...@rf...>
Gerrit-Attention: flichtenheld <fr...@li...>
Gerrit-MessageType: newchange

[Openvpn-devel] [L] Change in openvpn[master]: dns: deal with --dhcp-options when --dns is active

[d12fk (C. Review) <ge...@op...>]

Attention is currently required from: cron2, flichtenheld, plaisthos, stipa.

d12fk has posted comments on this change. ( http://gerrit.openvpn.net/c/openvpn/+/904?usp=email )

Change subject: dns: deal with --dhcp-options when --dns is active
......................................................................


Patch Set 22:

(1 comment)

File src/openvpn/options.c:

http://gerrit.openvpn.net/c/openvpn/+/904/comment/99c9aa4d_22bf37c4 :
PS15, Line 4299:         tuntap_options_postprocess_dns(o);
> I have thought about this a while now, and would suggest to do the following […]
Think all this is covered in the latest push, please check.



-- 
To view, visit http://gerrit.openvpn.net/c/openvpn/+/904?usp=email
To unsubscribe, or for help writing mail filters, visit http://gerrit.openvpn.net/settings

Gerrit-Project: openvpn
Gerrit-Branch: master
Gerrit-Change-Id: I635c4018fb43b5976a39b6a90cb2e9cb2570cd6a
Gerrit-Change-Number: 904
Gerrit-PatchSet: 22
Gerrit-Owner: d12fk <he...@op...>
Gerrit-Reviewer: flichtenheld <fr...@li...>
Gerrit-Reviewer: plaisthos <arn...@rf...>
Gerrit-Reviewer: stipa <lst...@gm...>
Gerrit-CC: cron2 <ge...@gr...>
Gerrit-CC: openvpn-devel <ope...@li...>
Gerrit-Attention: plaisthos <arn...@rf...>
Gerrit-Attention: cron2 <ge...@gr...>
Gerrit-Attention: flichtenheld <fr...@li...>
Gerrit-Attention: stipa <lst...@gm...>
Gerrit-Comment-Date: Thu, 05 Jun 2025 10:21:24 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: cron2 <ge...@gr...>
Comment-In-Reply-To: d12fk <he...@op...>
Comment-In-Reply-To: stipa <lst...@gm...>
Gerrit-MessageType: comment

[Openvpn-devel] [L] Change in openvpn[master]: dns: deal with --dhcp-options when --dns is active

[d12fk (C. Review) <ge...@op...>]

Attention is currently required from: d12fk, flichtenheld, plaisthos, stipa.

Hello flichtenheld, plaisthos, stipa, 

I'd like you to reexamine a change. Please visit

    http://gerrit.openvpn.net/c/openvpn/+/904?usp=email

to look at the new patch set (#22).

The following approvals got outdated and were removed:
Code-Review+1 by stipa


Change subject: dns: deal with --dhcp-options when --dns is active
......................................................................

dns: deal with --dhcp-options when --dns is active

Since --dns settings overrule DNS related --dhcp-options,
remove the latter when values were defined via --dns.

To stay as backward compatible as possible, we add foreign_options to
the script hook environment from the --dns values when a --up script
is defined. In that case the default --dns-updown is not run, even
when --dns values are present, to prevent double DNS configuration.
This way an existing --up script that deals with DNS can run, without
the immediate need to change after an openvpn upgrade and a server
pushing --dns options.

If you specify a custom --dns-updown, or force running the default
dns-updown that comes with openvpn, those compat env vars are not set
for --up scripts and the dns-updown command is run, even when there's
an --up script present.

Since Android uses the DNS values from tuntap_options, we always
override those with --dns stuff unconditionally. Also on Windows when
--ip-win32 is dynamic or adaptive, since DHCP relies on these as well.

Change-Id: I635c4018fb43b5976a39b6a90cb2e9cb2570cd6a
Signed-off-by: Heiko Hund <he...@is...>
---
M src/openvpn/dns.c
M src/openvpn/dns.h
M src/openvpn/options.c
3 files changed, 323 insertions(+), 184 deletions(-)


  git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/04/904/22

diff --git a/src/openvpn/dns.c b/src/openvpn/dns.c
index 7cf1b63..939ae09 100644
--- a/src/openvpn/dns.c
+++ b/src/openvpn/dns.c
@@ -691,7 +691,8 @@
 static void
 run_up_down_command(bool up, struct options *o, const struct tuntap *tt, struct dns_updown_runner_info *updown_runner)
 {
-    if (!o->dns_options.updown)
+    struct dns_options *dns = &o->dns_options;
+    if (!dns->updown || (o->up_script && !dns->user_set_updown))
     {
         return;
     }
@@ -701,7 +702,7 @@
     if (!updown_runner->required)
     {
         /* Run dns updown directly */
-        status = do_run_up_down_command(up, NULL, &o->dns_options, tt);
+        status = do_run_up_down_command(up, NULL, dns, tt);
     }
     else
     {
@@ -852,6 +853,14 @@
     {
         return;
     }
+#ifdef _WIN32
+    /* Don't use iservice in DHCP mode */
+    struct tuntap_options *tto = &o->tuntap_options;
+    if (tto->ip_win32_type == IPW32_SET_DHCP_MASQ || tto->ip_win32_type == IPW32_SET_ADAPTIVE)
+    {
+        return;
+    }
+#endif
 
     /* Warn about adding servers of unsupported AF */
     const struct dns_server *s = o->dns_options.servers;
diff --git a/src/openvpn/dns.h b/src/openvpn/dns.h
index 60f5471..688daa7 100644
--- a/src/openvpn/dns.h
+++ b/src/openvpn/dns.h
@@ -76,7 +76,28 @@
 #endif
 };
 
+#ifndef N_DHCP_ADDR
+#define N_DHCP_ADDR 4
+#endif
+
+#ifndef N_SEARCH_LIST_LEN
+#define N_SEARCH_LIST_LEN 10
+#endif
+
+struct dhcp_options {
+    in_addr_t dns[N_DHCP_ADDR];
+    int dns_len;
+
+    struct in6_addr dns6[N_DHCP_ADDR];
+    int dns6_len;
+
+    const char *domain;
+    const char *domain_search_list[N_SEARCH_LIST_LEN];
+    int domain_search_list_len;
+};
+
 struct dns_options {
+    struct dhcp_options from_dhcp;
     struct dns_domain *search_domains;
     struct dns_server *servers_prepull;
     struct dns_server *servers;
diff --git a/src/openvpn/options.c b/src/openvpn/options.c
index 6ea01d4..33f387c 100644
--- a/src/openvpn/options.c
+++ b/src/openvpn/options.c
@@ -1328,7 +1328,6 @@
 #endif /* ifndef ENABLE_SMALL */
 #endif /* ifdef _WIN32 */
 
-#if defined(_WIN32) || defined(TARGET_ANDROID)
 static void
 dhcp_option_dns6_parse(const char *parm, struct in6_addr *dns6_list, int *len, int msglevel)
 {
@@ -1371,150 +1370,6 @@
     }
 }
 
-/*
- * If DNS options are set use these for TUN/TAP options as well.
- * Applies to DNS, DNS6 and DOMAIN-SEARCH.
- * Existing options will be discarded.
- */
-static void
-tuntap_options_copy_dns(struct options *o)
-{
-    struct tuntap_options *tt = &o->tuntap_options;
-    struct dns_options *dns = &o->dns_options;
-
-    if (dns->search_domains)
-    {
-        tt->domain_search_list_len = 0;
-        const struct dns_domain *domain = dns->search_domains;
-        while (domain && tt->domain_search_list_len < N_SEARCH_LIST_LEN)
-        {
-            tt->domain_search_list[tt->domain_search_list_len++] = domain->name;
-            domain = domain->next;
-        }
-        if (domain)
-        {
-            msg(M_WARN, "WARNING: couldn't copy all --dns search-domains to --dhcp-option");
-        }
-        tt->dhcp_options |= DHCP_OPTIONS_DHCP_REQUIRED;
-    }
-
-    if (dns->servers)
-    {
-        tt->dns_len = 0;
-        tt->dns6_len = 0;
-        bool overflow = false;
-        const struct dns_server *server = dns->servers;
-        while (server)
-        {
-            for (int i = 0; i < server->addr_count; ++i)
-            {
-                if (server->addr[i].family == AF_INET)
-                {
-                    if (tt->dns_len >= N_DHCP_ADDR)
-                    {
-                        overflow = true;
-                        continue;
-                    }
-                    tt->dns[tt->dns_len++] = ntohl(server->addr[i].in.a4.s_addr);
-                }
-                else
-                {
-                    if (tt->dns6_len >= N_DHCP_ADDR)
-                    {
-                        overflow = true;
-                        continue;
-                    }
-                    tt->dns6[tt->dns6_len++] = server->addr[i].in.a6;
-                }
-            }
-            server = server->next;
-        }
-        if (overflow)
-        {
-            msg(M_WARN, "WARNING: couldn't copy all --dns server addresses to --dhcp-option");
-        }
-        tt->dhcp_options |= DHCP_OPTIONS_DHCP_OPTIONAL;
-    }
-}
-#else /* if defined(_WIN32) || defined(TARGET_ANDROID) */
-static void
-foreign_options_copy_dns(struct options *o, struct env_set *es)
-{
-    const struct dns_domain *domain = o->dns_options.search_domains;
-    const struct dns_server *server = o->dns_options.servers;
-    if (!domain && !server)
-    {
-        return;
-    }
-
-    /* reset the index since we're starting all over again */
-    int opt_max = o->foreign_option_index;
-    o->foreign_option_index = 0;
-
-    for (int i = 1; i <= opt_max; ++i)
-    {
-        char name[32];
-        snprintf(name, sizeof(name), "foreign_option_%d", i);
-
-        const char *env_str = env_set_get(es, name);
-        const char *value = strchr(env_str, '=') + 1;
-        if ((domain && strstr(value, "dhcp-option DOMAIN-SEARCH") == value)
-            || (server && strstr(value, "dhcp-option DNS") == value))
-        {
-            setenv_del(es, name);
-        }
-        else
-        {
-            setenv_foreign_option(o, &value, 1, es);
-        }
-    }
-
-    struct gc_arena gc = gc_new();
-
-    while (server)
-    {
-        for (size_t i = 0; i < server->addr_count; ++i)
-        {
-            if (server->addr[i].family == AF_INET)
-            {
-                const char *argv[] = {
-                    "dhcp-option",
-                    "DNS",
-                    print_in_addr_t(server->addr[i].in.a4.s_addr, 0, &gc)
-                };
-                setenv_foreign_option(o, argv, 3, es);
-            }
-            else
-            {
-                const char *argv[] = {
-                    "dhcp-option",
-                    "DNS6",
-                    print_in6_addr(server->addr[i].in.a6, 0, &gc)
-                };
-                setenv_foreign_option(o, argv, 3, es);
-            }
-        }
-        server = server->next;
-    }
-    while (domain)
-    {
-        const char *argv[] = { "dhcp-option", "DOMAIN-SEARCH", domain->name };
-        setenv_foreign_option(o, argv, 3, es);
-        domain = domain->next;
-    }
-
-    gc_free(&gc);
-
-    /* remove old leftover entries */
-    while (o->foreign_option_index < opt_max)
-    {
-        char name[32];
-        snprintf(name, sizeof(name), "foreign_option_%d", opt_max--);
-        setenv_del(es, name);
-    }
-}
-#endif /* if defined(_WIN32) || defined(TARGET_ANDROID) */
-
 #ifndef ENABLE_SMALL
 static const char *
 print_vlan_accept(enum vlan_acceptable_frames mode)
@@ -3603,6 +3458,260 @@
     }
 }
 
+#if defined(_WIN32) || defined(TARGET_ANDROID)
+/**
+ * @brief Postprocess DNS related settings
+ *
+ * Set TUN/TAP DNS options with values from either --dns
+ * or --dhcp-option.
+ *
+ * @param o     pointer to the options struct
+ */
+static void
+tuntap_options_postprocess_dns(struct options *o)
+{
+    struct dns_options *dns = &o->dns_options;
+    struct tuntap_options *tt = &o->tuntap_options;
+    if (!dns->servers)
+    {
+        /* Copy --dhcp-options to tuntap_options */
+        struct dhcp_options *dhcp = &dns->from_dhcp;
+        assert(sizeof(dhcp->dns) == sizeof(tt->dns));
+        assert(sizeof(dhcp->dns6) == sizeof(tt->dns6));
+        assert(sizeof(dhcp->domain_search_list) == sizeof(tt->domain_search_list));
+
+        tt->domain = dhcp->domain;
+        tt->dns_len = dhcp->dns_len;
+        tt->dns6_len = dhcp->dns6_len;
+
+        memcpy(tt->dns, dhcp->dns, sizeof(tt->dns));
+        memcpy(tt->dns6, dhcp->dns6, sizeof(tt->dns6));
+
+        tt->domain_search_list_len = dhcp->domain_search_list_len;
+        for (size_t i = 0; i < SIZE(tt->domain_search_list); ++i)
+        {
+            tt->domain_search_list[i] = dhcp->domain_search_list[i];
+        }
+
+        return;
+    }
+
+#if defined(_WIN32)
+    if (tt->ip_win32_type != IPW32_SET_DHCP_MASQ && tt->ip_win32_type != IPW32_SET_ADAPTIVE)
+    {
+        return;     /* Not in DHCP mode */
+    }
+#endif /* if defined(_WIN32) */
+
+    /* Copy --dns options to tuntap_options */
+    const struct dns_domain *d = dns->search_domains;
+    while (d && tt->domain_search_list_len + 1 < N_SEARCH_LIST_LEN)
+    {
+        tt->domain_search_list[tt->domain_search_list_len++] = d->name;
+        d = d->next;
+    }
+    if (d)
+    {
+        msg(M_WARN, "WARNING: couldn't copy all --dns search-domains to TUN/TAP");
+    }
+
+    const struct dns_server *s = dns->servers;
+    while (s)
+    {
+        bool non_standard_server_port = false;
+        for (int i = 0; i < s->addr_count; ++i)
+        {
+            if (s->addr[i].port && s->addr[i].port != 53)
+            {
+                non_standard_server_port = true;
+                break;
+            }
+        }
+        if ((s->transport && s->transport != DNS_TRANSPORT_PLAIN)
+            || (s->dnssec && s->dnssec != DNS_SECURITY_NO)
+            || non_standard_server_port)
+        {
+            /* Skip servers requiring unsupported config to be set */
+            s = s->next;
+        }
+        else
+        {
+            bool overflow = false;
+            for (int i = 0; i < s->addr_count; ++i)
+            {
+                if (s->addr[i].family == AF_INET && tt->dns_len + 1 < N_DHCP_ADDR)
+                {
+                    tt->dns[tt->dns_len++] = s->addr[i].in.a4.s_addr;
+                }
+                else if (tt->dns6_len + 1 < N_DHCP_ADDR)
+                {
+                    tt->dns6[tt->dns6_len] = s->addr[i].in.a6;
+                }
+                else
+                {
+                    overflow = true;
+                }
+            }
+            if (overflow)
+            {
+                msg(M_WARN, "WARNING: couldn't copy all --dns server addresses to TUN/TAP");
+            }
+            return;
+        }
+    }
+}
+
+#else /* if defined(_WIN32) || defined(TARGET_ANDROID) */
+
+/**
+ * @brief Postprocess DNS related settings
+ *
+ * Discard existing --dhcp-options from the env if needed and possibly
+ * replace them with values from --dns. If no --dns servers are set copy
+ * the --dhcp-option values over for --dns-updown runs.
+ *
+ * @param o     pointer to the options struct
+ * @param es    env set to modify potentially
+ */
+static void
+dhcp_options_postprocess_dns(struct options *o, struct env_set *es)
+{
+    struct gc_arena gc = gc_new();
+    struct dns_options *dns = &o->dns_options;
+
+    if (dns->servers || dns->user_set_updown)
+    {
+        /* Clean up env from --dhcp-option DNS config */
+        struct buffer name = alloc_buf_gc(OPTION_PARM_SIZE, &gc);
+        struct buffer value = alloc_buf_gc(OPTION_PARM_SIZE, &gc);
+
+        const int fo_count = o->foreign_option_index;
+        o->foreign_option_index = 0;
+
+        for (int i = 1; i <= fo_count; ++i)
+        {
+            buf_clear(&name);
+            buf_printf(&name, "foreign_option_%d", i);
+            const char *env_str = env_set_get(es, BSTR(&name));
+            const char *item_val = strchr(env_str, '=') + 1;
+            buf_clear(&value);
+            buf_printf(&value, "%s", item_val);
+
+            /* Remove foreign option item from env set */
+            env_set_del(es, BSTR(&name));
+
+            item_val = BSTR(&value);
+            if (strncmp(item_val, "dhcp-option ", 12) != 0
+                || (strncmp(item_val + 12, "ADAPTER-DOMAIN-SUFFIX ", 22) != 0
+                    && strncmp(item_val + 12, "DOMAIN-SEARCH ", 14) != 0
+                    && strncmp(item_val + 12, "DOMAIN ", 7) != 0
+                    && strncmp(item_val + 12, "DNS6 ", 5) != 0
+                    && strncmp(item_val + 12, "DNS ", 4) != 0))
+            {
+                /* Re-set the item with potentially updated name */
+                buf_clear(&name);
+                buf_printf(&name, "foreign_option_%d", ++o->foreign_option_index);
+                setenv_str(es, BSTR(&name), BSTR(&value));
+            }
+        }
+    }
+
+    if (!dns->servers)
+    {
+        /* Copy --dhcp-options to dns_options */
+        struct dhcp_options *dhcp = &dns->from_dhcp;
+
+        if (dhcp->dns_len || dhcp->dns6_len)
+        {
+            struct dns_domain **entry = &dns->search_domains;
+            ALLOC_OBJ_CLEAR_GC(*entry, struct dns_domain, &dns->gc);
+            struct dns_domain *new = *entry;
+            new->name = dhcp->domain;
+            entry = &new->next;
+
+            for (size_t i = 0; i < dhcp->domain_search_list_len; ++i)
+            {
+                ALLOC_OBJ_CLEAR_GC(*entry, struct dns_domain, &dns->gc);
+                struct dns_domain *new = *entry;
+                new->name = dhcp->domain_search_list[i];
+                entry = &new->next;
+            }
+
+            struct dns_server *server = dns_server_get(&dns->servers, 0, &dns->gc);
+            const size_t max_addrs = SIZE(server->addr);
+            for (size_t i = 0; i < dhcp->dns_len && server->addr_count < max_addrs; ++i)
+            {
+                server->addr[server->addr_count].in.a4.s_addr = htonl(dhcp->dns[i]);
+                server->addr[server->addr_count].family = AF_INET;
+                server->addr_count += 1;
+            }
+            for (size_t i = 0; i < dhcp->dns6_len && server->addr_count < max_addrs; ++i)
+            {
+                server->addr[server->addr_count].in.a6 = dhcp->dns6[i];
+                server->addr[server->addr_count].family = AF_INET6;
+                server->addr_count += 1;
+            }
+        }
+    }
+    else if (o->up_script && !dns->user_set_updown)
+    {
+        /* Set foreign option env vars from --dns config */
+        const char *p[] = { "dhcp-option", NULL, NULL };
+        size_t p_len = sizeof(p) / sizeof(p[0]);
+
+        p[1] = "DOMAIN";
+        const struct dns_domain *d = dns->search_domains;
+        while (d)
+        {
+            p[2] = d->name;
+            setenv_foreign_option(o, (const char **)p, p_len, es);
+            d = d->next;
+        }
+
+        const struct dns_server *s = dns->servers;
+        while (s)
+        {
+            bool non_standard_server_port = false;
+            for (int i = 0; i < s->addr_count; ++i)
+            {
+                if (s->addr[i].port && s->addr[i].port != 53)
+                {
+                    non_standard_server_port = true;
+                    break;
+                }
+            }
+            if ((s->transport && s->transport != DNS_TRANSPORT_PLAIN)
+                || (s->dnssec && s->dnssec != DNS_SECURITY_NO)
+                || non_standard_server_port)
+            {
+                /* Skip servers requiring unsupported config to be set */
+                s = s->next;
+            }
+            else
+            {
+                for (int i = 0; i < s->addr_count; ++i)
+                {
+                    if (s->addr[i].family == AF_INET)
+                    {
+                        p[1] = "DNS";
+                        p[2] = print_in_addr_t(s->addr[i].in.a4.s_addr, IA_NET_ORDER, &gc);
+                    }
+                    else
+                    {
+                        p[1] = "DNS6";
+                        p[2] = print_in6_addr(s->addr[i].in.a6, 0, &gc);
+                    }
+                    setenv_foreign_option(o, (const char **)p, p_len, es);
+                }
+                break;
+            }
+        }
+    }
+
+    gc_free(&gc);
+}
+#endif /* if defined(_WIN32) || defined(TARGET_ANDROID) */
+
 static void
 options_postprocess_mutate(struct options *o, struct env_set *es)
 {
@@ -3786,9 +3895,9 @@
     else
     {
 #if defined(_WIN32) || defined(TARGET_ANDROID)
-        tuntap_options_copy_dns(o);
+        tuntap_options_postprocess_dns(o);
 #else
-        foreign_options_copy_dns(o, es);
+        dhcp_options_postprocess_dns(o, es);
 #endif
     }
     if (o->auth_token_generate && !o->auth_token_renewal)
@@ -4171,9 +4280,9 @@
     {
         dns_options_postprocess_pull(&o->dns_options);
 #if defined(_WIN32) || defined(TARGET_ANDROID)
-        tuntap_options_copy_dns(o);
+        tuntap_options_postprocess_dns(o);
 #else
-        foreign_options_copy_dns(o, es);
+        dhcp_options_postprocess_dns(o, es);
 #endif
     }
     return success;
@@ -8162,18 +8271,43 @@
             goto err;
         }
     }
-#if defined(_WIN32) || defined(TARGET_ANDROID)
     else if (streq(p[0], "dhcp-option") && p[1])
     {
+        struct dhcp_options *dhcp = &options->dns_options.from_dhcp;
+#if defined(_WIN32) || defined(TARGET_ANDROID)
         struct tuntap_options *o = &options->tuntap_options;
+#endif
         VERIFY_PERMISSION(OPT_P_DHCPDNS);
 
-        if ((streq(p[1], "DOMAIN") || streq(p[1], "ADAPTER_DOMAIN_SUFFIX"))
-            && p[2] && !p[3])
+        if ((streq(p[1], "DOMAIN") || streq(p[1], "ADAPTER_DOMAIN_SUFFIX")) && p[2] && !p[3])
         {
-            o->domain = p[2];
-            o->dhcp_options |= DHCP_OPTIONS_DHCP_OPTIONAL;
+            dhcp->domain = p[2];
         }
+        else if (streq(p[1], "DOMAIN-SEARCH") && p[2] && !p[3])
+        {
+            if (dhcp->domain_search_list_len < N_SEARCH_LIST_LEN)
+            {
+                dhcp->domain_search_list[dhcp->domain_search_list_len++] = p[2];
+            }
+            else
+            {
+                msg(msglevel, "--dhcp-option %s: maximum of %d search entries can be specified",
+                    p[1], N_SEARCH_LIST_LEN);
+            }
+        }
+        else if ((streq(p[1], "DNS") || streq(p[1], "DNS6")) && p[2] && !p[3]
+                 && (!strstr(p[2], ":") || ipv6_addr_safe(p[2])))
+        {
+            if (strstr(p[2], ":"))
+            {
+                dhcp_option_dns6_parse(p[2], dhcp->dns6, &dhcp->dns6_len, msglevel);
+            }
+            else
+            {
+                dhcp_option_address_parse("DNS", p[2], dhcp->dns, &dhcp->dns_len, msglevel);
+            }
+        }
+#if defined(_WIN32) || defined(TARGET_ANDROID)
         else if (streq(p[1], "NBS") && p[2] && !p[3])
         {
             o->netbios_scope = p[2];
@@ -8191,23 +8325,9 @@
             o->netbios_node_type = t;
             o->dhcp_options |= DHCP_OPTIONS_DHCP_REQUIRED;
         }
-        else if ((streq(p[1], "DNS") || streq(p[1], "DNS6")) && p[2] && !p[3]
-                 && (!strstr(p[2], ":") || ipv6_addr_safe(p[2])))
-        {
-            if (strstr(p[2], ":"))
-            {
-                dhcp_option_dns6_parse(p[2], o->dns6, &o->dns6_len, msglevel);
-            }
-            else
-            {
-                dhcp_option_address_parse("DNS", p[2], o->dns, &o->dns_len, msglevel);
-                o->dhcp_options |= DHCP_OPTIONS_DHCP_OPTIONAL;
-            }
-        }
         else if (streq(p[1], "WINS") && p[2] && !p[3])
         {
             dhcp_option_address_parse("WINS", p[2], o->wins, &o->wins_len, msglevel);
-            o->dhcp_options |= DHCP_OPTIONS_DHCP_OPTIONAL;
         }
         else if (streq(p[1], "NTP") && p[2] && !p[3])
         {
@@ -8219,19 +8339,6 @@
             dhcp_option_address_parse("NBDD", p[2], o->nbdd, &o->nbdd_len, msglevel);
             o->dhcp_options |= DHCP_OPTIONS_DHCP_REQUIRED;
         }
-        else if (streq(p[1], "DOMAIN-SEARCH") && p[2] && !p[3])
-        {
-            if (o->domain_search_list_len < N_SEARCH_LIST_LEN)
-            {
-                o->domain_search_list[o->domain_search_list_len++] = p[2];
-            }
-            else
-            {
-                msg(msglevel, "--dhcp-option %s: maximum of %d search entries can be specified",
-                    p[1], N_SEARCH_LIST_LEN);
-            }
-            o->dhcp_options |= DHCP_OPTIONS_DHCP_OPTIONAL;
-        }
         else if (streq(p[1], "DISABLE-NBT") && !p[2])
         {
             o->disable_nbt = 1;
@@ -8249,8 +8356,10 @@
             msg(msglevel, "--dhcp-option: unknown option type '%s' or missing or unknown parameter", p[1]);
             goto err;
         }
-    }
+#else /* if defined(_WIN32) || defined(TARGET_ANDROID) */
+        setenv_foreign_option(options, (const char **)p, 3, es);
 #endif /* if defined(_WIN32) || defined(TARGET_ANDROID) */
+    }
 #ifdef _WIN32
     else if (streq(p[0], "show-adapters") && !p[1])
     {

-- 
To view, visit http://gerrit.openvpn.net/c/openvpn/+/904?usp=email
To unsubscribe, or for help writing mail filters, visit http://gerrit.openvpn.net/settings

Gerrit-Project: openvpn
Gerrit-Branch: master
Gerrit-Change-Id: I635c4018fb43b5976a39b6a90cb2e9cb2570cd6a
Gerrit-Change-Number: 904
Gerrit-PatchSet: 22
Gerrit-Owner: d12fk <he...@op...>
Gerrit-Reviewer: flichtenheld <fr...@li...>
Gerrit-Reviewer: plaisthos <arn...@rf...>
Gerrit-Reviewer: stipa <lst...@gm...>
Gerrit-CC: cron2 <ge...@gr...>
Gerrit-CC: openvpn-devel <ope...@li...>
Gerrit-Attention: plaisthos <arn...@rf...>
Gerrit-Attention: flichtenheld <fr...@li...>
Gerrit-Attention: d12fk <he...@op...>
Gerrit-Attention: stipa <lst...@gm...>
Gerrit-MessageType: newpatchset

[Openvpn-devel] [PATCH ovpn-net-next] ovpn: propagate socket mark to skb in UDP

[Ralf L. <ra...@ma...>]

openvpn implements the `--mark` option, which utilizes the `SO_MARK`
Linux socket option. However, in the UDP code path, the socket's
`sk_mark` value is not currently propagated to `skb` objects that pass
through our socket.

This commit ensures proper inheritance of the field by assigning
`sk_mark` to `skb->mark` before handing the `skb` to the network stack
for transmission.

Signed-off-by: Ralf Lici <ra...@ma...>
---
 drivers/net/ovpn/udp.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/ovpn/udp.c b/drivers/net/ovpn/udp.c
index b4fbebad8f45..4f9c9a9a889a 100644
--- a/drivers/net/ovpn/udp.c
+++ b/drivers/net/ovpn/udp.c
@@ -344,6 +344,7 @@ void ovpn_udp_send_skb(struct ovpn_peer *peer, struct sock *sk,
 	int ret;
 
 	skb->dev = peer->ovpn->dev;
+	skb->mark = READ_ONCE(sk->sk_mark);
 	/* no checksum performed at this layer */
 	skb->ip_summed = CHECKSUM_NONE;
 
-- 
2.49.0

[Openvpn-devel] [S] Change in openvpn[master]: Fix various badly placed comments in preparation for reformat

[cron2 (C. Review) <ge...@op...>]

cron2 has uploaded a new patch set (#2) to the change originally created by flichtenheld. ( http://gerrit.openvpn.net/c/openvpn/+/1030?usp=email )

The following approvals got outdated and were removed:
Code-Review+2 by cron2


Change subject: Fix various badly placed comments in preparation for reformat
......................................................................

Fix various badly placed comments in preparation for reformat

Change-Id: I83831060fdf5588a0ada8d6abbedc7ce3ded4182
Signed-off-by: Frank Lichtenheld <fr...@li...>
Acked-by: Gert Doering <ge...@gr...>
Message-Id: <202...@gr...>
URL: https://www.mail-archive.com/ope...@li.../msg31872.html
Signed-off-by: Gert Doering <ge...@gr...>
---
M src/openvpn/buffer.c
M src/openvpn/multi_io.c
M src/openvpn/networking.h
M src/openvpn/options.c
M src/openvpn/otime.c
M src/openvpn/otime.h
M src/openvpn/tun.h
7 files changed, 15 insertions(+), 8 deletions(-)


  git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/30/1030/2

diff --git a/src/openvpn/buffer.c b/src/openvpn/buffer.c
index fd81323..dd6044b 100644
--- a/src/openvpn/buffer.c
+++ b/src/openvpn/buffer.c
@@ -910,7 +910,8 @@
     {
         return true;
     }
-    if ((flags & CC_PRINT) && (c >= 32 && c != 127)) /* allow ascii non-control and UTF-8, consider DEL to be a control */
+    /* allow ascii non-control and UTF-8, consider DEL to be a control */
+    if ((flags & CC_PRINT) && (c >= 32 && c != 127))
     {
         return true;
     }
diff --git a/src/openvpn/multi_io.c b/src/openvpn/multi_io.c
index 2bce272..4854f4b 100644
--- a/src/openvpn/multi_io.c
+++ b/src/openvpn/multi_io.c
@@ -247,7 +247,8 @@
         case TA_TUN_WRITE:
             looking_for = TUN_WRITE;
             tun_input_pending = NULL;
-            c->c2.timeval.tv_sec = 1; /* For some reason, the Linux 2.2 TUN/TAP driver hits this timeout */
+            /* For some reason, the Linux 2.2 TUN/TAP driver hits this timeout */
+            c->c2.timeval.tv_sec = 1;
             perf_push(PERF_PROC_OUT_TUN_MTCP);
             io_wait(c, IOW_TO_TUN);
             perf_pop();
diff --git a/src/openvpn/networking.h b/src/openvpn/networking.h
index 6f5a6d6..0ba4963 100644
--- a/src/openvpn/networking.h
+++ b/src/openvpn/networking.h
@@ -302,7 +302,7 @@
                      int metric);
 
 /**
- * Delete a route for an IPv4 address/network
+ * Delete a route for an IPv6 address/network
  *
  * @param ctx       the implementation specific context
  * @param dst       the destination of the route
diff --git a/src/openvpn/options.c b/src/openvpn/options.c
index b9708343..6ea01d4 100644
--- a/src/openvpn/options.c
+++ b/src/openvpn/options.c
@@ -8392,9 +8392,10 @@
         VERIFY_PERMISSION(OPT_P_DHCPDNS);
         setenv_foreign_option(options, (const char **)p, 3, es);
     }
-    else if (streq(p[0], "route-method") && p[1] && !p[2]) /* ignore when pushed to non-Windows OS */
+    else if (streq(p[0], "route-method") && p[1] && !p[2])
     {
         VERIFY_PERMISSION(OPT_P_ROUTE_EXTRAS);
+        /* ignore when pushed to non-Windows OS */
     }
 #endif /* ifdef _WIN32 */
 #if PASSTOS_CAPABILITY
diff --git a/src/openvpn/otime.c b/src/openvpn/otime.c
index e604a28..d423067 100644
--- a/src/openvpn/otime.c
+++ b/src/openvpn/otime.c
@@ -44,8 +44,10 @@
 void
 update_now(const time_t system_time)
 {
-    const int forward_threshold = 86400; /* threshold at which to dampen forward jumps */
-    const int backward_trigger  = 10;  /* backward jump must be >= this many seconds before we adjust */
+    /* threshold at which to dampen forward jumps */
+    const int forward_threshold = 86400;
+    /* backward jump must be >= this many seconds before we adjust */
+    const int backward_trigger = 10;
     time_t real_time = system_time + now_adj;
 
     if (real_time > now)
diff --git a/src/openvpn/otime.h b/src/openvpn/otime.h
index 5c3e86f..448389d 100644
--- a/src/openvpn/otime.h
+++ b/src/openvpn/otime.h
@@ -246,7 +246,8 @@
 static inline bool
 tv_within_sigma(const struct timeval *t1, const struct timeval *t2, unsigned int sigma)
 {
-    const int delta = tv_subtract(t1, t2, TV_WITHIN_SIGMA_MAX_SEC); /* sigma should be less than 10 minutes */
+    /* sigma should be less than 10 minutes */
+    const int delta = tv_subtract(t1, t2, TV_WITHIN_SIGMA_MAX_SEC);
     return -(int)sigma <= delta && delta <= (int)sigma;
 }
 
diff --git a/src/openvpn/tun.h b/src/openvpn/tun.h
index ec0f4da..5407e47 100644
--- a/src/openvpn/tun.h
+++ b/src/openvpn/tun.h
@@ -212,7 +212,8 @@
 
 #ifdef _WIN32
     HANDLE hand;
-    OVERLAPPED dco_new_peer_ov; /* used for async NEW_PEER dco call, which might wait for TCP connect */
+    /* used for async NEW_PEER dco call, which might wait for TCP connect */
+    OVERLAPPED dco_new_peer_ov;
     struct overlapped_io reads;
     struct overlapped_io writes;
     struct rw_handle rw_handle;

-- 
To view, visit http://gerrit.openvpn.net/c/openvpn/+/1030?usp=email
To unsubscribe, or for help writing mail filters, visit http://gerrit.openvpn.net/settings

Gerrit-Project: openvpn
Gerrit-Branch: master
Gerrit-Change-Id: I83831060fdf5588a0ada8d6abbedc7ce3ded4182
Gerrit-Change-Number: 1030
Gerrit-PatchSet: 2
Gerrit-Owner: flichtenheld <fr...@li...>
Gerrit-Reviewer: cron2 <ge...@gr...>
Gerrit-Reviewer: plaisthos <arn...@rf...>
Gerrit-CC: openvpn-devel <ope...@li...>
Gerrit-MessageType: newpatchset

[Openvpn-devel] [S] Change in openvpn[master]: Fix various badly placed comments in preparation for reformat

[cron2 (C. Review) <ge...@op...>]

cron2 has submitted this change. ( http://gerrit.openvpn.net/c/openvpn/+/1030?usp=email )

Change subject: Fix various badly placed comments in preparation for reformat
......................................................................

Fix various badly placed comments in preparation for reformat

Change-Id: I83831060fdf5588a0ada8d6abbedc7ce3ded4182
Signed-off-by: Frank Lichtenheld <fr...@li...>
Acked-by: Gert Doering <ge...@gr...>
Message-Id: <202...@gr...>
URL: https://www.mail-archive.com/ope...@li.../msg31872.html
Signed-off-by: Gert Doering <ge...@gr...>
---
M src/openvpn/buffer.c
M src/openvpn/multi_io.c
M src/openvpn/networking.h
M src/openvpn/options.c
M src/openvpn/otime.c
M src/openvpn/otime.h
M src/openvpn/tun.h
7 files changed, 15 insertions(+), 8 deletions(-)




diff --git a/src/openvpn/buffer.c b/src/openvpn/buffer.c
index fd81323..dd6044b 100644
--- a/src/openvpn/buffer.c
+++ b/src/openvpn/buffer.c
@@ -910,7 +910,8 @@
     {
         return true;
     }
-    if ((flags & CC_PRINT) && (c >= 32 && c != 127)) /* allow ascii non-control and UTF-8, consider DEL to be a control */
+    /* allow ascii non-control and UTF-8, consider DEL to be a control */
+    if ((flags & CC_PRINT) && (c >= 32 && c != 127))
     {
         return true;
     }
diff --git a/src/openvpn/multi_io.c b/src/openvpn/multi_io.c
index 2bce272..4854f4b 100644
--- a/src/openvpn/multi_io.c
+++ b/src/openvpn/multi_io.c
@@ -247,7 +247,8 @@
         case TA_TUN_WRITE:
             looking_for = TUN_WRITE;
             tun_input_pending = NULL;
-            c->c2.timeval.tv_sec = 1; /* For some reason, the Linux 2.2 TUN/TAP driver hits this timeout */
+            /* For some reason, the Linux 2.2 TUN/TAP driver hits this timeout */
+            c->c2.timeval.tv_sec = 1;
             perf_push(PERF_PROC_OUT_TUN_MTCP);
             io_wait(c, IOW_TO_TUN);
             perf_pop();
diff --git a/src/openvpn/networking.h b/src/openvpn/networking.h
index 6f5a6d6..0ba4963 100644
--- a/src/openvpn/networking.h
+++ b/src/openvpn/networking.h
@@ -302,7 +302,7 @@
                      int metric);
 
 /**
- * Delete a route for an IPv4 address/network
+ * Delete a route for an IPv6 address/network
  *
  * @param ctx       the implementation specific context
  * @param dst       the destination of the route
diff --git a/src/openvpn/options.c b/src/openvpn/options.c
index b9708343..6ea01d4 100644
--- a/src/openvpn/options.c
+++ b/src/openvpn/options.c
@@ -8392,9 +8392,10 @@
         VERIFY_PERMISSION(OPT_P_DHCPDNS);
         setenv_foreign_option(options, (const char **)p, 3, es);
     }
-    else if (streq(p[0], "route-method") && p[1] && !p[2]) /* ignore when pushed to non-Windows OS */
+    else if (streq(p[0], "route-method") && p[1] && !p[2])
     {
         VERIFY_PERMISSION(OPT_P_ROUTE_EXTRAS);
+        /* ignore when pushed to non-Windows OS */
     }
 #endif /* ifdef _WIN32 */
 #if PASSTOS_CAPABILITY
diff --git a/src/openvpn/otime.c b/src/openvpn/otime.c
index e604a28..d423067 100644
--- a/src/openvpn/otime.c
+++ b/src/openvpn/otime.c
@@ -44,8 +44,10 @@
 void
 update_now(const time_t system_time)
 {
-    const int forward_threshold = 86400; /* threshold at which to dampen forward jumps */
-    const int backward_trigger  = 10;  /* backward jump must be >= this many seconds before we adjust */
+    /* threshold at which to dampen forward jumps */
+    const int forward_threshold = 86400;
+    /* backward jump must be >= this many seconds before we adjust */
+    const int backward_trigger = 10;
     time_t real_time = system_time + now_adj;
 
     if (real_time > now)
diff --git a/src/openvpn/otime.h b/src/openvpn/otime.h
index 5c3e86f..448389d 100644
--- a/src/openvpn/otime.h
+++ b/src/openvpn/otime.h
@@ -246,7 +246,8 @@
 static inline bool
 tv_within_sigma(const struct timeval *t1, const struct timeval *t2, unsigned int sigma)
 {
-    const int delta = tv_subtract(t1, t2, TV_WITHIN_SIGMA_MAX_SEC); /* sigma should be less than 10 minutes */
+    /* sigma should be less than 10 minutes */
+    const int delta = tv_subtract(t1, t2, TV_WITHIN_SIGMA_MAX_SEC);
     return -(int)sigma <= delta && delta <= (int)sigma;
 }
 
diff --git a/src/openvpn/tun.h b/src/openvpn/tun.h
index ec0f4da..5407e47 100644
--- a/src/openvpn/tun.h
+++ b/src/openvpn/tun.h
@@ -212,7 +212,8 @@
 
 #ifdef _WIN32
     HANDLE hand;
-    OVERLAPPED dco_new_peer_ov; /* used for async NEW_PEER dco call, which might wait for TCP connect */
+    /* used for async NEW_PEER dco call, which might wait for TCP connect */
+    OVERLAPPED dco_new_peer_ov;
     struct overlapped_io reads;
     struct overlapped_io writes;
     struct rw_handle rw_handle;

-- 
To view, visit http://gerrit.openvpn.net/c/openvpn/+/1030?usp=email
To unsubscribe, or for help writing mail filters, visit http://gerrit.openvpn.net/settings

Gerrit-Project: openvpn
Gerrit-Branch: master
Gerrit-Change-Id: I83831060fdf5588a0ada8d6abbedc7ce3ded4182
Gerrit-Change-Number: 1030
Gerrit-PatchSet: 2
Gerrit-Owner: flichtenheld <fr...@li...>
Gerrit-Reviewer: cron2 <ge...@gr...>
Gerrit-Reviewer: plaisthos <arn...@rf...>
Gerrit-CC: openvpn-devel <ope...@li...>
Gerrit-MessageType: merged

[Openvpn-devel] [PATCH applied] Re: Fix various badly placed comments in preparation for reformat

[Gert D. <ge...@gr...>]

Only whitespace changes, but beyond what automatic tools will do
(without making them reformat everything).  One comment bugfix.

Your patch has been applied to the master branch.

commit 9cc7c68bd8d42b9e1c02fd3f069d404b5c056b57
Author: Frank Lichtenheld
Date:   Tue Jun 3 18:30:34 2025 +0200

     Fix various badly placed comments in preparation for reformat

     Signed-off-by: Frank Lichtenheld <fr...@li...>
     Acked-by: Gert Doering <ge...@gr...>
     Message-Id: <202...@gr...>
     URL: https://www.mail-archive.com/ope...@li.../msg31872.html
     Signed-off-by: Gert Doering <ge...@gr...>


--
kind regards,

Gert Doering

[Openvpn-devel] [PATCH v1] Fix various badly placed comments in preparation for reformat

[Gert D. <ge...@gr...>]

From: Frank Lichtenheld <fr...@li...>

Change-Id: I83831060fdf5588a0ada8d6abbedc7ce3ded4182
Signed-off-by: Frank Lichtenheld <fr...@li...>
Acked-by: Gert Doering <ge...@gr...>
---

This change was reviewed on Gerrit and approved by at least one
developer. I request to merge it to master.

Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/1030
This mail reflects revision 1 of this Change.

Acked-by according to Gerrit (reflected above):
Gert Doering <ge...@gr...>

        
diff --git a/src/openvpn/buffer.c b/src/openvpn/buffer.c
index b2a5bf5..4b83eeb 100644
--- a/src/openvpn/buffer.c
+++ b/src/openvpn/buffer.c
@@ -910,7 +910,8 @@
     {
         return true;
     }
-    if ((flags & CC_PRINT) && (c >= 32 && c != 127)) /* allow ascii non-control and UTF-8, consider DEL to be a control */
+    /* allow ascii non-control and UTF-8, consider DEL to be a control */
+    if ((flags & CC_PRINT) && (c >= 32 && c != 127))
     {
         return true;
     }
diff --git a/src/openvpn/multi_io.c b/src/openvpn/multi_io.c
index 7f47319..6ab06bb 100644
--- a/src/openvpn/multi_io.c
+++ b/src/openvpn/multi_io.c
@@ -247,7 +247,8 @@
         case TA_TUN_WRITE:
             looking_for = TUN_WRITE;
             tun_input_pending = NULL;
-            c->c2.timeval.tv_sec = 1; /* For some reason, the Linux 2.2 TUN/TAP driver hits this timeout */
+            /* For some reason, the Linux 2.2 TUN/TAP driver hits this timeout */
+            c->c2.timeval.tv_sec = 1;
             perf_push(PERF_PROC_OUT_TUN_MTCP);
             io_wait(c, IOW_TO_TUN);
             perf_pop();
diff --git a/src/openvpn/networking.h b/src/openvpn/networking.h
index f06d6df..d9aca1e 100644
--- a/src/openvpn/networking.h
+++ b/src/openvpn/networking.h
@@ -302,7 +302,7 @@
                      int metric);
 
 /**
- * Delete a route for an IPv4 address/network
+ * Delete a route for an IPv6 address/network
  *
  * @param ctx       the implementation specific context
  * @param dst       the destination of the route
diff --git a/src/openvpn/options.c b/src/openvpn/options.c
index bcc18a5..4528edd 100644
--- a/src/openvpn/options.c
+++ b/src/openvpn/options.c
@@ -8398,9 +8398,10 @@
         VERIFY_PERMISSION(OPT_P_DHCPDNS);
         setenv_foreign_option(options, (const char **)p, 3, es);
     }
-    else if (streq(p[0], "route-method") && p[1] && !p[2]) /* ignore when pushed to non-Windows OS */
+    else if (streq(p[0], "route-method") && p[1] && !p[2])
     {
         VERIFY_PERMISSION(OPT_P_ROUTE_EXTRAS);
+        /* ignore when pushed to non-Windows OS */
     }
 #endif /* ifdef _WIN32 */
 #if PASSTOS_CAPABILITY
diff --git a/src/openvpn/otime.c b/src/openvpn/otime.c
index d77c99e..7a362e0 100644
--- a/src/openvpn/otime.c
+++ b/src/openvpn/otime.c
@@ -44,8 +44,10 @@
 void
 update_now(const time_t system_time)
 {
-    const int forward_threshold = 86400; /* threshold at which to dampen forward jumps */
-    const int backward_trigger  = 10;  /* backward jump must be >= this many seconds before we adjust */
+    /* threshold at which to dampen forward jumps */
+    const int forward_threshold = 86400;
+    /* backward jump must be >= this many seconds before we adjust */
+    const int backward_trigger = 10;
     time_t real_time = system_time + now_adj;
 
     if (real_time > now)
diff --git a/src/openvpn/otime.h b/src/openvpn/otime.h
index 9543732..04aa5c6 100644
--- a/src/openvpn/otime.h
+++ b/src/openvpn/otime.h
@@ -246,7 +246,8 @@
 static inline bool
 tv_within_sigma(const struct timeval *t1, const struct timeval *t2, unsigned int sigma)
 {
-    const int delta = tv_subtract(t1, t2, TV_WITHIN_SIGMA_MAX_SEC); /* sigma should be less than 10 minutes */
+    /* sigma should be less than 10 minutes */
+    const int delta = tv_subtract(t1, t2, TV_WITHIN_SIGMA_MAX_SEC);
     return -(int)sigma <= delta && delta <= (int)sigma;
 }
 
diff --git a/src/openvpn/tun.h b/src/openvpn/tun.h
index de1876a..379a31a 100644
--- a/src/openvpn/tun.h
+++ b/src/openvpn/tun.h
@@ -212,7 +212,8 @@
 
 #ifdef _WIN32
     HANDLE hand;
-    OVERLAPPED dco_new_peer_ov; /* used for async NEW_PEER dco call, which might wait for TCP connect */
+    /* used for async NEW_PEER dco call, which might wait for TCP connect */
+    OVERLAPPED dco_new_peer_ov;
     struct overlapped_io reads;
     struct overlapped_io writes;
     struct rw_handle rw_handle;

[Openvpn-devel] [S] Change in openvpn[master]: Fix various badly placed comments in preparation for reformat

[cron2 (C. Review) <ge...@op...>]

Attention is currently required from: flichtenheld, plaisthos.

cron2 has posted comments on this change. ( http://gerrit.openvpn.net/c/openvpn/+/1030?usp=email )

Change subject: Fix various badly placed comments in preparation for reformat
......................................................................


Patch Set 1: Code-Review+2


-- 
To view, visit http://gerrit.openvpn.net/c/openvpn/+/1030?usp=email
To unsubscribe, or for help writing mail filters, visit http://gerrit.openvpn.net/settings

Gerrit-Project: openvpn
Gerrit-Branch: master
Gerrit-Change-Id: I83831060fdf5588a0ada8d6abbedc7ce3ded4182
Gerrit-Change-Number: 1030
Gerrit-PatchSet: 1
Gerrit-Owner: flichtenheld <fr...@li...>
Gerrit-Reviewer: cron2 <ge...@gr...>
Gerrit-Reviewer: plaisthos <arn...@rf...>
Gerrit-CC: openvpn-devel <ope...@li...>
Gerrit-Attention: plaisthos <arn...@rf...>
Gerrit-Attention: flichtenheld <fr...@li...>
Gerrit-Comment-Date: Tue, 03 Jun 2025 16:30:25 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment

[Openvpn-devel] [XS] Change in openvpn[master]: t_server_null: Test different permutations of --dh

[cron2 (C. Review) <ge...@op...>]

cron2 has submitted this change. ( http://gerrit.openvpn.net/c/openvpn/+/1038?usp=email )

Change subject: t_server_null: Test different permutations of --dh
......................................................................

t_server_null: Test different permutations of --dh

Do not include --dh by default, since we do not actually
need it. Use the different servers for different ways
of specifying it.

Change-Id: I480442a55025bfcce7cb68ec7564ff33b0b780e2
Signed-off-by: Frank Lichtenheld <fr...@li...>
Acked-by: Gert Doering <ge...@gr...>
Message-Id: <202...@gr...>
URL: https://www.mail-archive.com/ope...@li.../msg31868.html
Signed-off-by: Gert Doering <ge...@gr...>
---
M tests/t_server_null_default.rc
1 file changed, 3 insertions(+), 3 deletions(-)




diff --git a/tests/t_server_null_default.rc b/tests/t_server_null_default.rc
index ca8004a..365b5a8 100755
--- a/tests/t_server_null_default.rc
+++ b/tests/t_server_null_default.rc
@@ -40,7 +40,7 @@
 SERVER_EXEC="${top_builddir}/src/openvpn/openvpn"
 SERVER_BASE_OPTS="--daemon --local 127.0.0.1 --dev tun --topology subnet --max-clients $MAX_CLIENTS --persist-tun --verb 3 --duplicate-cn"
 SERVER_CIPHER_OPTS=""
-SERVER_CERT_OPTS="--ca ${CA} --dh ${DH} --cert ${SERVER_CERT} --key ${SERVER_KEY} --tls-auth ${TA} 0"
+SERVER_CERT_OPTS="--ca ${CA} --cert ${SERVER_CERT} --key ${SERVER_KEY} --tls-auth ${TA} 0"
 SERVER_CONF_BASE="${SERVER_BASE_OPTS} ${SERVER_CIPHER_OPTS} ${SERVER_CERT_OPTS}"
 
 TEST_SERVER_LIST="1 2 3"
@@ -55,13 +55,13 @@
 SERVER_SERVER_2="--server 10.29.42.0 255.255.255.0"
 SERVER_MGMT_PORT_2="11195"
 SERVER_EXEC_2="${SERVER_EXEC}"
-SERVER_CONF_2="${SERVER_CONF_BASE} ${SERVER_SERVER_2} --lport 1195 --proto tcp --management 127.0.0.1 ${SERVER_MGMT_PORT_2}"
+SERVER_CONF_2="${SERVER_CONF_BASE} ${SERVER_SERVER_2} --lport 1195 --proto tcp --management 127.0.0.1 ${SERVER_MGMT_PORT_2} --dh ${DH}"
 
 SERVER_NAME_3="t_server_null_server-1196_udp"
 SERVER_SERVER_3="--server 10.29.43.0 255.255.255.0"
 SERVER_MGMT_PORT_3="11196"
 SERVER_EXEC_3="${SERVER_EXEC}"
-SERVER_CONF_3="${SERVER_CONF_BASE} ${SERVER_SERVER_3} --lport 1196 --proto udp --management 127.0.0.1 ${SERVER_MGMT_PORT_3} --cipher AES-192-CBC --data-ciphers DEFAULT:AES-192-CBC"
+SERVER_CONF_3="${SERVER_CONF_BASE} ${SERVER_SERVER_3} --lport 1196 --proto udp --management 127.0.0.1 ${SERVER_MGMT_PORT_3} --dh none --cipher AES-192-CBC --data-ciphers DEFAULT:AES-192-CBC"
 
 # Test client configurations
 CLIENT_EXEC="${top_builddir}/src/openvpn/openvpn"

-- 
To view, visit http://gerrit.openvpn.net/c/openvpn/+/1038?usp=email
To unsubscribe, or for help writing mail filters, visit http://gerrit.openvpn.net/settings

Gerrit-Project: openvpn
Gerrit-Branch: master
Gerrit-Change-Id: I480442a55025bfcce7cb68ec7564ff33b0b780e2
Gerrit-Change-Number: 1038
Gerrit-PatchSet: 3
Gerrit-Owner: flichtenheld <fr...@li...>
Gerrit-Reviewer: cron2 <ge...@gr...>
Gerrit-Reviewer: plaisthos <arn...@rf...>
Gerrit-CC: openvpn-devel <ope...@li...>
Gerrit-MessageType: merged

[Openvpn-devel] [XS] Change in openvpn[master]: t_server_null: Test different permutations of --dh

[cron2 (C. Review) <ge...@op...>]

cron2 has uploaded a new patch set (#3) to the change originally created by flichtenheld. ( http://gerrit.openvpn.net/c/openvpn/+/1038?usp=email )

The following approvals got outdated and were removed:
Code-Review+2 by cron2


Change subject: t_server_null: Test different permutations of --dh
......................................................................

t_server_null: Test different permutations of --dh

Do not include --dh by default, since we do not actually
need it. Use the different servers for different ways
of specifying it.

Change-Id: I480442a55025bfcce7cb68ec7564ff33b0b780e2
Signed-off-by: Frank Lichtenheld <fr...@li...>
Acked-by: Gert Doering <ge...@gr...>
Message-Id: <202...@gr...>
URL: https://www.mail-archive.com/ope...@li.../msg31868.html
Signed-off-by: Gert Doering <ge...@gr...>
---
M tests/t_server_null_default.rc
1 file changed, 3 insertions(+), 3 deletions(-)


  git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/38/1038/3

diff --git a/tests/t_server_null_default.rc b/tests/t_server_null_default.rc
index ca8004a..365b5a8 100755
--- a/tests/t_server_null_default.rc
+++ b/tests/t_server_null_default.rc
@@ -40,7 +40,7 @@
 SERVER_EXEC="${top_builddir}/src/openvpn/openvpn"
 SERVER_BASE_OPTS="--daemon --local 127.0.0.1 --dev tun --topology subnet --max-clients $MAX_CLIENTS --persist-tun --verb 3 --duplicate-cn"
 SERVER_CIPHER_OPTS=""
-SERVER_CERT_OPTS="--ca ${CA} --dh ${DH} --cert ${SERVER_CERT} --key ${SERVER_KEY} --tls-auth ${TA} 0"
+SERVER_CERT_OPTS="--ca ${CA} --cert ${SERVER_CERT} --key ${SERVER_KEY} --tls-auth ${TA} 0"
 SERVER_CONF_BASE="${SERVER_BASE_OPTS} ${SERVER_CIPHER_OPTS} ${SERVER_CERT_OPTS}"
 
 TEST_SERVER_LIST="1 2 3"
@@ -55,13 +55,13 @@
 SERVER_SERVER_2="--server 10.29.42.0 255.255.255.0"
 SERVER_MGMT_PORT_2="11195"
 SERVER_EXEC_2="${SERVER_EXEC}"
-SERVER_CONF_2="${SERVER_CONF_BASE} ${SERVER_SERVER_2} --lport 1195 --proto tcp --management 127.0.0.1 ${SERVER_MGMT_PORT_2}"
+SERVER_CONF_2="${SERVER_CONF_BASE} ${SERVER_SERVER_2} --lport 1195 --proto tcp --management 127.0.0.1 ${SERVER_MGMT_PORT_2} --dh ${DH}"
 
 SERVER_NAME_3="t_server_null_server-1196_udp"
 SERVER_SERVER_3="--server 10.29.43.0 255.255.255.0"
 SERVER_MGMT_PORT_3="11196"
 SERVER_EXEC_3="${SERVER_EXEC}"
-SERVER_CONF_3="${SERVER_CONF_BASE} ${SERVER_SERVER_3} --lport 1196 --proto udp --management 127.0.0.1 ${SERVER_MGMT_PORT_3} --cipher AES-192-CBC --data-ciphers DEFAULT:AES-192-CBC"
+SERVER_CONF_3="${SERVER_CONF_BASE} ${SERVER_SERVER_3} --lport 1196 --proto udp --management 127.0.0.1 ${SERVER_MGMT_PORT_3} --dh none --cipher AES-192-CBC --data-ciphers DEFAULT:AES-192-CBC"
 
 # Test client configurations
 CLIENT_EXEC="${top_builddir}/src/openvpn/openvpn"

-- 
To view, visit http://gerrit.openvpn.net/c/openvpn/+/1038?usp=email
To unsubscribe, or for help writing mail filters, visit http://gerrit.openvpn.net/settings

Gerrit-Project: openvpn
Gerrit-Branch: master
Gerrit-Change-Id: I480442a55025bfcce7cb68ec7564ff33b0b780e2
Gerrit-Change-Number: 1038
Gerrit-PatchSet: 3
Gerrit-Owner: flichtenheld <fr...@li...>
Gerrit-Reviewer: cron2 <ge...@gr...>
Gerrit-Reviewer: plaisthos <arn...@rf...>
Gerrit-CC: openvpn-devel <ope...@li...>
Gerrit-MessageType: newpatchset

[Openvpn-devel] [PATCH applied] Re: t_server_null: Test different permutations of --dh

[Gert D. <ge...@gr...>]

Thanks, this is a very welcome addition to catch future oversights around
--dh variants.

The buildbots have tested this, and found it to their liking :-) - I have
just stared at it and liked it as well.

Your patch has been applied to the master branch.

commit 4d104a3857a21cf22774ba50b66fe575a682ae32
Author: Frank Lichtenheld
Date:   Tue Jun 3 16:20:29 2025 +0200

     t_server_null: Test different permutations of --dh

     Signed-off-by: Frank Lichtenheld <fr...@li...>
     Acked-by: Gert Doering <ge...@gr...>
     Message-Id: <202...@gr...>
     URL: https://www.mail-archive.com/ope...@li.../msg31868.html
     Signed-off-by: Gert Doering <ge...@gr...>


--
kind regards,

Gert Doering

[Openvpn-devel] [PATCH v2] t_server_null: Test different permutations of --dh

[Gert D. <ge...@gr...>]

From: Frank Lichtenheld <fr...@li...>

Do not include --dh by default, since we do not actually
need it. Use the different servers for different ways
of specifying it.

Change-Id: I480442a55025bfcce7cb68ec7564ff33b0b780e2
Signed-off-by: Frank Lichtenheld <fr...@li...>
Acked-by: Gert Doering <ge...@gr...>
---

This change was reviewed on Gerrit and approved by at least one
developer. I request to merge it to master.

Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/1038
This mail reflects revision 2 of this Change.

Acked-by according to Gerrit (reflected above):
Gert Doering <ge...@gr...>

        
diff --git a/tests/t_server_null_default.rc b/tests/t_server_null_default.rc
index ca8004a..365b5a8 100755
--- a/tests/t_server_null_default.rc
+++ b/tests/t_server_null_default.rc
@@ -40,7 +40,7 @@
 SERVER_EXEC="${top_builddir}/src/openvpn/openvpn"
 SERVER_BASE_OPTS="--daemon --local 127.0.0.1 --dev tun --topology subnet --max-clients $MAX_CLIENTS --persist-tun --verb 3 --duplicate-cn"
 SERVER_CIPHER_OPTS=""
-SERVER_CERT_OPTS="--ca ${CA} --dh ${DH} --cert ${SERVER_CERT} --key ${SERVER_KEY} --tls-auth ${TA} 0"
+SERVER_CERT_OPTS="--ca ${CA} --cert ${SERVER_CERT} --key ${SERVER_KEY} --tls-auth ${TA} 0"
 SERVER_CONF_BASE="${SERVER_BASE_OPTS} ${SERVER_CIPHER_OPTS} ${SERVER_CERT_OPTS}"
 
 TEST_SERVER_LIST="1 2 3"
@@ -55,13 +55,13 @@
 SERVER_SERVER_2="--server 10.29.42.0 255.255.255.0"
 SERVER_MGMT_PORT_2="11195"
 SERVER_EXEC_2="${SERVER_EXEC}"
-SERVER_CONF_2="${SERVER_CONF_BASE} ${SERVER_SERVER_2} --lport 1195 --proto tcp --management 127.0.0.1 ${SERVER_MGMT_PORT_2}"
+SERVER_CONF_2="${SERVER_CONF_BASE} ${SERVER_SERVER_2} --lport 1195 --proto tcp --management 127.0.0.1 ${SERVER_MGMT_PORT_2} --dh ${DH}"
 
 SERVER_NAME_3="t_server_null_server-1196_udp"
 SERVER_SERVER_3="--server 10.29.43.0 255.255.255.0"
 SERVER_MGMT_PORT_3="11196"
 SERVER_EXEC_3="${SERVER_EXEC}"
-SERVER_CONF_3="${SERVER_CONF_BASE} ${SERVER_SERVER_3} --lport 1196 --proto udp --management 127.0.0.1 ${SERVER_MGMT_PORT_3} --cipher AES-192-CBC --data-ciphers DEFAULT:AES-192-CBC"
+SERVER_CONF_3="${SERVER_CONF_BASE} ${SERVER_SERVER_3} --lport 1196 --proto udp --management 127.0.0.1 ${SERVER_MGMT_PORT_3} --dh none --cipher AES-192-CBC --data-ciphers DEFAULT:AES-192-CBC"
 
 # Test client configurations
 CLIENT_EXEC="${top_builddir}/src/openvpn/openvpn"

[Openvpn-devel] [XS] Change in openvpn[master]: t_server_null: Test different permutations of --dh

[cron2 (C. Review) <ge...@op...>]

Attention is currently required from: flichtenheld, plaisthos.

cron2 has posted comments on this change. ( http://gerrit.openvpn.net/c/openvpn/+/1038?usp=email )

Change subject: t_server_null: Test different permutations of --dh
......................................................................


Patch Set 2: Code-Review+2


-- 
To view, visit http://gerrit.openvpn.net/c/openvpn/+/1038?usp=email
To unsubscribe, or for help writing mail filters, visit http://gerrit.openvpn.net/settings

Gerrit-Project: openvpn
Gerrit-Branch: master
Gerrit-Change-Id: I480442a55025bfcce7cb68ec7564ff33b0b780e2
Gerrit-Change-Number: 1038
Gerrit-PatchSet: 2
Gerrit-Owner: flichtenheld <fr...@li...>
Gerrit-Reviewer: cron2 <ge...@gr...>
Gerrit-Reviewer: plaisthos <arn...@rf...>
Gerrit-CC: openvpn-devel <ope...@li...>
Gerrit-Attention: plaisthos <arn...@rf...>
Gerrit-Attention: flichtenheld <fr...@li...>
Gerrit-Comment-Date: Tue, 03 Jun 2025 14:20:17 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment

[Openvpn-devel] [S] Change in openvpn[master]: mbedtls: Allow TLS 1.3 if available

[cron2 (C. Review) <ge...@op...>]

cron2 has submitted this change. ( http://gerrit.openvpn.net/c/openvpn/+/1042?usp=email )

Change subject: mbedtls: Allow TLS 1.3 if available
......................................................................

mbedtls: Allow TLS 1.3 if available

We need mbedtls_ssl_export_keying_material() to support TLS 1.3. The
workaround we use for TLS 1.2 does not work for TLS 1.3.

Change-Id: If5e832866b312a2f8a1ce6b4e00d40e3dcf63681
Signed-off-by: Max Fillinger <max...@fo...>
Acked-by: Frank Lichtenheld <fr...@li...>
Message-Id: <202...@gr...>
URL: https://www.mail-archive.com/ope...@li.../msg31858.html
Signed-off-by: Gert Doering <ge...@gr...>
---
M README.mbedtls
M src/openvpn/ssl_mbedtls.c
2 files changed, 13 insertions(+), 6 deletions(-)




diff --git a/README.mbedtls b/README.mbedtls
index c4f3924..a1012e9 100644
--- a/README.mbedtls
+++ b/README.mbedtls
@@ -26,5 +26,9 @@
 
 *************************************************************************
 
-Mbed TLS 3 has implemented (parts of) the TLS 1.3 protocol, but we have disabled
-support in OpenVPN because the TLS-Exporter function is not yet implemented.
+Mbed TLS 3 has implemented TLS 1.3, but support in OpenVPN requires the
+function mbedtls_ssl_export_keying_material() which is currently not in
+any released version. It is available when building mbed TLS from source
+(mbedtls-3.6 or development branch).
+
+Without this function, only TLS 1.2 is available.
diff --git a/src/openvpn/ssl_mbedtls.c b/src/openvpn/ssl_mbedtls.c
index 7452c30..ecccc26 100644
--- a/src/openvpn/ssl_mbedtls.c
+++ b/src/openvpn/ssl_mbedtls.c
@@ -1048,11 +1048,14 @@
 int
 tls_version_max(void)
 {
-#if defined(MBEDTLS_SSL_PROTO_TLS1_2)
+    /* We need mbedtls_ssl_export_keying_material() to support TLS 1.3. */
+#if defined(MBEDTLS_SSL_PROTO_TLS1_3) && defined(MBEDTLS_SSL_KEYING_MATERIAL_EXPORT)
+    return TLS_VER_1_3;
+#elif defined(MBEDTLS_SSL_PROTO_TLS1_2)
     return TLS_VER_1_2;
-#else /* defined(MBEDTLS_SSL_PROTO_TLS1_2) */
-    #error "mbedtls is compiled without support for TLS 1.2."
-#endif /* defined(MBEDTLS_SSL_PROTO_TLS1_2) */
+#else
+    #error mbedtls is compiled without support for TLS 1.2 or 1.3
+#endif
 }
 
 /**

-- 
To view, visit http://gerrit.openvpn.net/c/openvpn/+/1042?usp=email
To unsubscribe, or for help writing mail filters, visit http://gerrit.openvpn.net/settings

Gerrit-Project: openvpn
Gerrit-Branch: master
Gerrit-Change-Id: If5e832866b312a2f8a1ce6b4e00d40e3dcf63681
Gerrit-Change-Number: 1042
Gerrit-PatchSet: 4
Gerrit-Owner: MaxF <ma...@ma...>
Gerrit-Reviewer: cron2 <ge...@gr...>
Gerrit-Reviewer: flichtenheld <fr...@li...>
Gerrit-Reviewer: plaisthos <arn...@rf...>
Gerrit-CC: openvpn-devel <ope...@li...>
Gerrit-MessageType: merged

[Openvpn-devel] [S] Change in openvpn[master]: mbedtls: Allow TLS 1.3 if available

[cron2 (C. Review) <ge...@op...>]

cron2 has uploaded a new patch set (#4) to the change originally created by MaxF. ( http://gerrit.openvpn.net/c/openvpn/+/1042?usp=email )

The following approvals got outdated and were removed:
Code-Review+2 by cron2, Code-Review+2 by flichtenheld


Change subject: mbedtls: Allow TLS 1.3 if available
......................................................................

mbedtls: Allow TLS 1.3 if available

We need mbedtls_ssl_export_keying_material() to support TLS 1.3. The
workaround we use for TLS 1.2 does not work for TLS 1.3.

Change-Id: If5e832866b312a2f8a1ce6b4e00d40e3dcf63681
Signed-off-by: Max Fillinger <max...@fo...>
Acked-by: Frank Lichtenheld <fr...@li...>
Message-Id: <202...@gr...>
URL: https://www.mail-archive.com/ope...@li.../msg31858.html
Signed-off-by: Gert Doering <ge...@gr...>
---
M README.mbedtls
M src/openvpn/ssl_mbedtls.c
2 files changed, 13 insertions(+), 6 deletions(-)


  git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/42/1042/4

diff --git a/README.mbedtls b/README.mbedtls
index c4f3924..a1012e9 100644
--- a/README.mbedtls
+++ b/README.mbedtls
@@ -26,5 +26,9 @@
 
 *************************************************************************
 
-Mbed TLS 3 has implemented (parts of) the TLS 1.3 protocol, but we have disabled
-support in OpenVPN because the TLS-Exporter function is not yet implemented.
+Mbed TLS 3 has implemented TLS 1.3, but support in OpenVPN requires the
+function mbedtls_ssl_export_keying_material() which is currently not in
+any released version. It is available when building mbed TLS from source
+(mbedtls-3.6 or development branch).
+
+Without this function, only TLS 1.2 is available.
diff --git a/src/openvpn/ssl_mbedtls.c b/src/openvpn/ssl_mbedtls.c
index 7452c30..ecccc26 100644
--- a/src/openvpn/ssl_mbedtls.c
+++ b/src/openvpn/ssl_mbedtls.c
@@ -1048,11 +1048,14 @@
 int
 tls_version_max(void)
 {
-#if defined(MBEDTLS_SSL_PROTO_TLS1_2)
+    /* We need mbedtls_ssl_export_keying_material() to support TLS 1.3. */
+#if defined(MBEDTLS_SSL_PROTO_TLS1_3) && defined(MBEDTLS_SSL_KEYING_MATERIAL_EXPORT)
+    return TLS_VER_1_3;
+#elif defined(MBEDTLS_SSL_PROTO_TLS1_2)
     return TLS_VER_1_2;
-#else /* defined(MBEDTLS_SSL_PROTO_TLS1_2) */
-    #error "mbedtls is compiled without support for TLS 1.2."
-#endif /* defined(MBEDTLS_SSL_PROTO_TLS1_2) */
+#else
+    #error mbedtls is compiled without support for TLS 1.2 or 1.3
+#endif
 }
 
 /**

-- 
To view, visit http://gerrit.openvpn.net/c/openvpn/+/1042?usp=email
To unsubscribe, or for help writing mail filters, visit http://gerrit.openvpn.net/settings

Gerrit-Project: openvpn
Gerrit-Branch: master
Gerrit-Change-Id: If5e832866b312a2f8a1ce6b4e00d40e3dcf63681
Gerrit-Change-Number: 1042
Gerrit-PatchSet: 4
Gerrit-Owner: MaxF <ma...@ma...>
Gerrit-Reviewer: cron2 <ge...@gr...>
Gerrit-Reviewer: flichtenheld <fr...@li...>
Gerrit-Reviewer: plaisthos <arn...@rf...>
Gerrit-CC: openvpn-devel <ope...@li...>
Gerrit-MessageType: newpatchset

[Openvpn-devel] [PATCH applied] Re: mbedtls: Allow TLS 1.3 if available

[Gert D. <ge...@gr...>]

A twisty nightmare of passages, all alike...  (but ignoring the eye sore,
these particular #ifdef are not actually that complex, well described,
and well contained).

I have removed my Acked-By: from the commit - this was more a gerrit
artefact due to -1'ing and then +2'ing the patch again.  Frank and the BBs
tested it ;-)

Your patch has been applied to the master branch.

commit abed088c9bf3d6ab479dbe815d4d307b21b816b6
Author: Max Fillinger
Date:   Tue Jun 3 16:06:24 2025 +0200

     mbedtls: Allow TLS 1.3 if available

     Signed-off-by: Max Fillinger <max...@fo...>
     Acked-by: Frank Lichtenheld <fr...@li...>
     Message-Id: <202...@gr...>
     URL: https://www.mail-archive.com/ope...@li.../msg31858.html
     Signed-off-by: Gert Doering <ge...@gr...>


--
kind regards,

Gert Doering

[Openvpn-devel] [S] Change in openvpn[master]: mbedtls: Allow TLS 1.3 if available

[cron2 (C. Review) <ge...@op...>]

Attention is currently required from: MaxF, plaisthos.

cron2 has posted comments on this change. ( http://gerrit.openvpn.net/c/openvpn/+/1042?usp=email )

Change subject: mbedtls: Allow TLS 1.3 if available
......................................................................


Patch Set 3:

(1 comment)

File README.mbedtls:

http://gerrit.openvpn.net/c/openvpn/+/1042/comment/f309799d_61275c63 :
PS3, Line 32: (mbedtls-3.6 or development branch).
> It is not released yet. You need the mbed TLS 3. […]
Done



-- 
To view, visit http://gerrit.openvpn.net/c/openvpn/+/1042?usp=email
To unsubscribe, or for help writing mail filters, visit http://gerrit.openvpn.net/settings

Gerrit-Project: openvpn
Gerrit-Branch: master
Gerrit-Change-Id: If5e832866b312a2f8a1ce6b4e00d40e3dcf63681
Gerrit-Change-Number: 1042
Gerrit-PatchSet: 3
Gerrit-Owner: MaxF <ma...@ma...>
Gerrit-Reviewer: cron2 <ge...@gr...>
Gerrit-Reviewer: flichtenheld <fr...@li...>
Gerrit-Reviewer: plaisthos <arn...@rf...>
Gerrit-CC: openvpn-devel <ope...@li...>
Gerrit-Attention: plaisthos <arn...@rf...>
Gerrit-Attention: MaxF <ma...@ma...>
Gerrit-Comment-Date: Tue, 03 Jun 2025 14:18:36 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: plaisthos <arn...@rf...>
Comment-In-Reply-To: cron2 <ge...@gr...>
Gerrit-MessageType: comment

[Openvpn-devel] [S] Change in openvpn[master]: Use mbedtls_ssl_export_keying_material()

[cron2 (C. Review) <ge...@op...>]

cron2 has submitted this change. ( http://gerrit.openvpn.net/c/openvpn/+/1041?usp=email )

Change subject: Use mbedtls_ssl_export_keying_material()
......................................................................

Use mbedtls_ssl_export_keying_material()

Mbed TLS now has an implementation of the TLS-Exporter feature (though
not yet in a released version). Use it if it's available.

v2: Rebased, changed feature detection in configure.ac

Change-Id: I1204bc2ff85952160a86f0b9d1caae90e5065bc4
Signed-off-by: Max Fillinger <max...@fo...>
Acked-by: Frank Lichtenheld <fr...@li...>
Message-Id: <202...@gr...>
URL: https://www.mail-archive.com/ope...@li.../msg31856.html
Signed-off-by: Gert Doering <ge...@gr...>
---
M configure.ac
M src/openvpn/ssl_mbedtls.c
M src/openvpn/ssl_mbedtls.h
3 files changed, 33 insertions(+), 8 deletions(-)




diff --git a/configure.ac b/configure.ac
index c70892a..8bdec32 100644
--- a/configure.ac
+++ b/configure.ac
@@ -1072,7 +1072,10 @@
 			[AC_DEFINE([HAVE_MBEDTLS_SSL_SET_EXPORT_KEYS_CB], [0], [no])]
 		)
 		if test "x$ac_cv_func_mbedtls_ssl_set_export_keys_cb" != xyes; then
-			AC_MSG_ERROR(This version of mbed TLS has no support for exporting key material.)
+			AC_CHECK_FUNC([mbedtls_ssl_export_keying_material])
+			if test "x$ac_cv_func_mbedtls_ssl_export_keying_material" != xyes; then
+				AC_MSG_ERROR(This version of mbed TLS has no support for exporting key material.)
+			fi
 		fi
 	fi
 
diff --git a/src/openvpn/ssl_mbedtls.c b/src/openvpn/ssl_mbedtls.c
index 5936ca3..7452c30 100644
--- a/src/openvpn/ssl_mbedtls.c
+++ b/src/openvpn/ssl_mbedtls.c
@@ -251,8 +251,8 @@
     memcpy(cache->master_secret, secret, sizeof(cache->master_secret));
     cache->tls_prf_type = tls_prf_type;
 }
-#else  /* if HAVE_MBEDTLS_SSL_CONF_EXPORT_KEYS_EXT_CB */
-#error either mbedtls_ssl_conf_export_keys_ext_cb or mbedtls_ssl_set_export_keys_cb must be available in mbed TLS
+#elif !defined(MBEDTLS_SSL_KEYING_MATERIAL_EXPORT)
+#error mbedtls_ssl_conf_export_keys_ext_cb, mbedtls_ssl_set_export_keys_cb or mbedtls_ssl_export_keying_material must be available in mbed TLS
 #endif /* HAVE_MBEDTLS_SSL_CONF_EXPORT_KEYS_EXT_CB */
 
 bool
@@ -262,6 +262,20 @@
 {
     ASSERT(strlen(label) == label_size);
 
+#if defined(MBEDTLS_SSL_KEYING_MATERIAL_EXPORT)
+    /* Our version of mbed TLS has a built-in TLS-Exporter. */
+
+    mbedtls_ssl_context *ctx = session->key[KS_PRIMARY].ks_ssl.ctx;
+    if (mbed_ok(mbedtls_ssl_export_keying_material(ctx, ekm, ekm_size, label, label_size, NULL, 0, 0)))
+    {
+        return true;
+    }
+    else
+    {
+        return false;
+    }
+
+#else  /* defined(MBEDTLS_SSL_KEYING_MATERIAL_EXPORT) */
     struct tls_key_cache *cache = &session->key[KS_PRIMARY].ks_ssl.tls_key_cache;
 
     /* If the type is NONE, we either have no cached secrets or
@@ -286,6 +300,7 @@
         secure_memzero(ekm, session->opt->ekm_size);
         return false;
     }
+#endif  /* defined(MBEDTLS_SSL_KEYING_MATERIAL_EXPORT) */
 }
 
 bool
@@ -1226,7 +1241,7 @@
         mbedtls_ssl_conf_max_tls_version(ks_ssl->ssl_config, version);
     }
 
-#if HAVE_MBEDTLS_SSL_CONF_EXPORT_KEYS_EXT_CB
+#if HAVE_MBEDTLS_SSL_CONF_EXPORT_KEYS_EXT_CB && !defined(MBEDTLS_SSL_KEYING_MATERIAL_EXPORT)
     /* Initialize keying material exporter, old style. */
     mbedtls_ssl_conf_export_keys_ext_cb(ks_ssl->ssl_config,
                                         mbedtls_ssl_export_keys_cb, session);
@@ -1241,7 +1256,7 @@
      * verification. */
     ASSERT(mbed_ok(mbedtls_ssl_set_hostname(ks_ssl->ctx, NULL)));
 
-#if HAVE_MBEDTLS_SSL_SET_EXPORT_KEYS_CB
+#if HAVE_MBEDTLS_SSL_SET_EXPORT_KEYS_CB && !defined(MBEDTLS_SSL_KEYING_MATERIAL_EXPORT)
     /* Initialize keying material exporter, new style. */
     mbedtls_ssl_set_export_keys_cb(ks_ssl->ctx, mbedtls_ssl_export_keys_cb, session);
 #endif
diff --git a/src/openvpn/ssl_mbedtls.h b/src/openvpn/ssl_mbedtls.h
index 3841eff..9f124cd 100644
--- a/src/openvpn/ssl_mbedtls.h
+++ b/src/openvpn/ssl_mbedtls.h
@@ -85,14 +85,21 @@
     void *sign_ctx;
 };
 
-/** struct to cache TLS secrets for keying material exporter (RFC 5705).
- * The constants (64 and 48) are inherent to TLS version and
- * the whole keying material export will likely change when they change */
+#if !defined(MBEDTLS_SSL_KEYING_MATERIAL_EXPORT)
+/**
+ * struct to cache TLS secrets for keying material exporter (RFC 5705).
+ * Not needed if the library itself implements the keying material exporter.
+ *
+ * The constants 64 and 48 are inherent to TLS 1.2. For TLS 1.3, it is not
+ * possible to obtain the exporter master secret from mbed TLS. */
 struct tls_key_cache {
     unsigned char client_server_random[64];
     mbedtls_tls_prf_types tls_prf_type;
     unsigned char master_secret[48];
 };
+#else  /* !defined(MBEDTLS_SSL_KEYING_MATERIAL_EXPORT) */
+struct tls_key_cache { };
+#endif
 
 /**
  * Structure that wraps the TLS context. Contents differ depending on the

-- 
To view, visit http://gerrit.openvpn.net/c/openvpn/+/1041?usp=email
To unsubscribe, or for help writing mail filters, visit http://gerrit.openvpn.net/settings

Gerrit-Project: openvpn
Gerrit-Branch: master
Gerrit-Change-Id: I1204bc2ff85952160a86f0b9d1caae90e5065bc4
Gerrit-Change-Number: 1041
Gerrit-PatchSet: 4
Gerrit-Owner: MaxF <ma...@ma...>
Gerrit-Reviewer: flichtenheld <fr...@li...>
Gerrit-Reviewer: plaisthos <arn...@rf...>
Gerrit-CC: cron2 <ge...@gr...>
Gerrit-CC: openvpn-devel <ope...@li...>
Gerrit-MessageType: merged

[Openvpn-devel] [S] Change in openvpn[master]: Use mbedtls_ssl_export_keying_material()

[cron2 (C. Review) <ge...@op...>]

cron2 has uploaded a new patch set (#4) to the change originally created by MaxF. ( http://gerrit.openvpn.net/c/openvpn/+/1041?usp=email )

The following approvals got outdated and were removed:
Code-Review+2 by flichtenheld


Change subject: Use mbedtls_ssl_export_keying_material()
......................................................................

Use mbedtls_ssl_export_keying_material()

Mbed TLS now has an implementation of the TLS-Exporter feature (though
not yet in a released version). Use it if it's available.

v2: Rebased, changed feature detection in configure.ac

Change-Id: I1204bc2ff85952160a86f0b9d1caae90e5065bc4
Signed-off-by: Max Fillinger <max...@fo...>
Acked-by: Frank Lichtenheld <fr...@li...>
Message-Id: <202...@gr...>
URL: https://www.mail-archive.com/ope...@li.../msg31856.html
Signed-off-by: Gert Doering <ge...@gr...>
---
M configure.ac
M src/openvpn/ssl_mbedtls.c
M src/openvpn/ssl_mbedtls.h
3 files changed, 33 insertions(+), 8 deletions(-)


  git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/41/1041/4

diff --git a/configure.ac b/configure.ac
index c70892a..8bdec32 100644
--- a/configure.ac
+++ b/configure.ac
@@ -1072,7 +1072,10 @@
 			[AC_DEFINE([HAVE_MBEDTLS_SSL_SET_EXPORT_KEYS_CB], [0], [no])]
 		)
 		if test "x$ac_cv_func_mbedtls_ssl_set_export_keys_cb" != xyes; then
-			AC_MSG_ERROR(This version of mbed TLS has no support for exporting key material.)
+			AC_CHECK_FUNC([mbedtls_ssl_export_keying_material])
+			if test "x$ac_cv_func_mbedtls_ssl_export_keying_material" != xyes; then
+				AC_MSG_ERROR(This version of mbed TLS has no support for exporting key material.)
+			fi
 		fi
 	fi
 
diff --git a/src/openvpn/ssl_mbedtls.c b/src/openvpn/ssl_mbedtls.c
index 5936ca3..7452c30 100644
--- a/src/openvpn/ssl_mbedtls.c
+++ b/src/openvpn/ssl_mbedtls.c
@@ -251,8 +251,8 @@
     memcpy(cache->master_secret, secret, sizeof(cache->master_secret));
     cache->tls_prf_type = tls_prf_type;
 }
-#else  /* if HAVE_MBEDTLS_SSL_CONF_EXPORT_KEYS_EXT_CB */
-#error either mbedtls_ssl_conf_export_keys_ext_cb or mbedtls_ssl_set_export_keys_cb must be available in mbed TLS
+#elif !defined(MBEDTLS_SSL_KEYING_MATERIAL_EXPORT)
+#error mbedtls_ssl_conf_export_keys_ext_cb, mbedtls_ssl_set_export_keys_cb or mbedtls_ssl_export_keying_material must be available in mbed TLS
 #endif /* HAVE_MBEDTLS_SSL_CONF_EXPORT_KEYS_EXT_CB */
 
 bool
@@ -262,6 +262,20 @@
 {
     ASSERT(strlen(label) == label_size);
 
+#if defined(MBEDTLS_SSL_KEYING_MATERIAL_EXPORT)
+    /* Our version of mbed TLS has a built-in TLS-Exporter. */
+
+    mbedtls_ssl_context *ctx = session->key[KS_PRIMARY].ks_ssl.ctx;
+    if (mbed_ok(mbedtls_ssl_export_keying_material(ctx, ekm, ekm_size, label, label_size, NULL, 0, 0)))
+    {
+        return true;
+    }
+    else
+    {
+        return false;
+    }
+
+#else  /* defined(MBEDTLS_SSL_KEYING_MATERIAL_EXPORT) */
     struct tls_key_cache *cache = &session->key[KS_PRIMARY].ks_ssl.tls_key_cache;
 
     /* If the type is NONE, we either have no cached secrets or
@@ -286,6 +300,7 @@
         secure_memzero(ekm, session->opt->ekm_size);
         return false;
     }
+#endif  /* defined(MBEDTLS_SSL_KEYING_MATERIAL_EXPORT) */
 }
 
 bool
@@ -1226,7 +1241,7 @@
         mbedtls_ssl_conf_max_tls_version(ks_ssl->ssl_config, version);
     }
 
-#if HAVE_MBEDTLS_SSL_CONF_EXPORT_KEYS_EXT_CB
+#if HAVE_MBEDTLS_SSL_CONF_EXPORT_KEYS_EXT_CB && !defined(MBEDTLS_SSL_KEYING_MATERIAL_EXPORT)
     /* Initialize keying material exporter, old style. */
     mbedtls_ssl_conf_export_keys_ext_cb(ks_ssl->ssl_config,
                                         mbedtls_ssl_export_keys_cb, session);
@@ -1241,7 +1256,7 @@
      * verification. */
     ASSERT(mbed_ok(mbedtls_ssl_set_hostname(ks_ssl->ctx, NULL)));
 
-#if HAVE_MBEDTLS_SSL_SET_EXPORT_KEYS_CB
+#if HAVE_MBEDTLS_SSL_SET_EXPORT_KEYS_CB && !defined(MBEDTLS_SSL_KEYING_MATERIAL_EXPORT)
     /* Initialize keying material exporter, new style. */
     mbedtls_ssl_set_export_keys_cb(ks_ssl->ctx, mbedtls_ssl_export_keys_cb, session);
 #endif
diff --git a/src/openvpn/ssl_mbedtls.h b/src/openvpn/ssl_mbedtls.h
index 3841eff..9f124cd 100644
--- a/src/openvpn/ssl_mbedtls.h
+++ b/src/openvpn/ssl_mbedtls.h
@@ -85,14 +85,21 @@
     void *sign_ctx;
 };
 
-/** struct to cache TLS secrets for keying material exporter (RFC 5705).
- * The constants (64 and 48) are inherent to TLS version and
- * the whole keying material export will likely change when they change */
+#if !defined(MBEDTLS_SSL_KEYING_MATERIAL_EXPORT)
+/**
+ * struct to cache TLS secrets for keying material exporter (RFC 5705).
+ * Not needed if the library itself implements the keying material exporter.
+ *
+ * The constants 64 and 48 are inherent to TLS 1.2. For TLS 1.3, it is not
+ * possible to obtain the exporter master secret from mbed TLS. */
 struct tls_key_cache {
     unsigned char client_server_random[64];
     mbedtls_tls_prf_types tls_prf_type;
     unsigned char master_secret[48];
 };
+#else  /* !defined(MBEDTLS_SSL_KEYING_MATERIAL_EXPORT) */
+struct tls_key_cache { };
+#endif
 
 /**
  * Structure that wraps the TLS context. Contents differ depending on the

-- 
To view, visit http://gerrit.openvpn.net/c/openvpn/+/1041?usp=email
To unsubscribe, or for help writing mail filters, visit http://gerrit.openvpn.net/settings

Gerrit-Project: openvpn
Gerrit-Branch: master
Gerrit-Change-Id: I1204bc2ff85952160a86f0b9d1caae90e5065bc4
Gerrit-Change-Number: 1041
Gerrit-PatchSet: 4
Gerrit-Owner: MaxF <ma...@ma...>
Gerrit-Reviewer: flichtenheld <fr...@li...>
Gerrit-Reviewer: plaisthos <arn...@rf...>
Gerrit-CC: cron2 <ge...@gr...>
Gerrit-CC: openvpn-devel <ope...@li...>
Gerrit-MessageType: newpatchset

[Openvpn-devel] [PATCH applied] Re: Use mbedtls_ssl_export_keying_material()

[Gert D. <ge...@gr...>]

I could not test this myself, and neither did the buildbots (as far as
I know) because we all have older mbedTLS versions - so the patch does
not break anything there, at least :-)

Arne has tested this with an mbedTLS development build, and it works
as it says on the lid - the new function is used, the API is called
correctly, things work.

Your patch has been applied to the master branch.

commit 4c2022ab9044d4449d2f6480fcf845461f02c114
Author: Max Fillinger
Date:   Tue Jun 3 16:01:01 2025 +0200

     Use mbedtls_ssl_export_keying_material()

     Signed-off-by: Max Fillinger <max...@fo...>
     Acked-by: Frank Lichtenheld <fr...@li...>
     Message-Id: <202...@gr...>
     URL: https://www.mail-archive.com/ope...@li.../msg31856.html
     Signed-off-by: Gert Doering <ge...@gr...>


--
kind regards,

Gert Doering