[Openvpn-devel] [PATCH] work around false positive warning with mingw 12
When cross compiling for Windows with Ubuntu 23.04 mingw complains about
route.c:344:26: warning: ‘special.S_un.S_addr’ may be used uninitialized
which is wrong technically. However the workaround isn't really
intrusive and while there are other warnings caused by libtool, the
cmake mingw build completes with -Werror now.
Change-Id: I8a0f59707570722eab41af2db76980ced04e6d54
Signed-off-by: Heiko Hund
---
src/openvpn/route.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/openvpn/route.c b/src/openvpn/route.c
index 0d04a5a3..0b369da4 100644
--- a/src/openvpn/route.c
+++ b/src/openvpn/route.c
@@ -319,7 +319,7 @@ init_route(struct route_ipv4 *r,
const in_addr_t default_netmask = IPV4_NETMASK_HOST;
bool status;
int ret;
-struct in_addr special;
+struct in_addr special = {0};
CLEAR(*r);
r->option = ro;
--
2.39.2
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel
Re: [Openvpn-devel] [PATCH] [RFC] fix warning with gcc 12.2.0 (compiler bug?)
On Sonntag, 27. November 2022 09:59:33 CEST Arne Schwabe wrote: > Signed-off-by: Arne Schwabe Acked-by: Heiko Hund This one is ugly, but required to build with cmake and mingw with -Werror Cheers, Heiko ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
Re: [Openvpn-devel] [PATCH] Support --inactive option for DCO
On Mittwoch, 15. März 2023 14:38:08 CET Lev Stipakov wrote: > Change-Id: Ib417b965bc4a2c17b51935b43c9627b106716526 > Signed-off-by: Lev Stipakov Acked-by: Heiko Hund ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
[Openvpn-devel] [PATCH 2/3] dns option: remove support for exclude-domains
No DNS resolver currently supports this and it is not possible to
emulate the behavior without the chance of errors. Finding the
effective default system DNS server(s) to specify the exclude
DNS routes is not trivial and cannot be verified to be correct
without resolver internal knowledge. So, it is better to not
support this instead of supporting it, but incorrectly.
Change-Id: I7f422add22f3f01e9f47985065782dd67bca46eb
Signed-off-by: Heiko Hund
---
doc/man-sections/client-options.rst | 14 +-
doc/man-sections/script-options.rst | 1 -
src/openvpn/dns.c | 13 ++---
src/openvpn/dns.h | 7 ---
src/openvpn/options.c | 17 -
5 files changed, 7 insertions(+), 45 deletions(-)
diff --git a/doc/man-sections/client-options.rst
b/doc/man-sections/client-options.rst
index fe9ffa6a..434e 100644
--- a/doc/man-sections/client-options.rst
+++ b/doc/man-sections/client-options.rst
@@ -169,7 +169,7 @@ configuration.
dns search-domains domain [domain ...]
dns server n address addr[:port] [addr[:port] ...]
- dns server n resolve-domains|exclude-domains domain [domain ...]
+ dns server n resolve-domains domain [domain ...]
dns server n dnssec yes|optional|no
dns server n transport DoH|DoT|plain
dns server n sni server-name
@@ -191,14 +191,10 @@ configuration.
Optionally a port can be appended after a colon. IPv6 addresses need to
be enclosed in brackets if a port is appended.
- The ``resolve-domains`` and ``exclude-domains`` options take one or
- more DNS domains which are explicitly resolved or explicitly not resolved
- by a server. Only one of the options can be configured for a server.
- ``resolve-domains`` is used to define a split-dns setup, where only
- given domains are resolved by a server. ``exclude-domains`` is used to
- define domains which will never be resolved by a server (e.g. domains
- which can only be resolved locally). Systems which do not support fine
- grained DNS domain configuration, will ignore these settings.
+ The ``resolve-domains`` option takes one or more DNS domains used to define
+ a split-dns or dns-routing setup, where only the given domains are resolved
+ by the server. Systems which do not support fine grained DNS domain
+ configuration, will ignore this setting.
The ``dnssec`` option is used to configure validation of DNSSEC records.
While the exact semantics may differ for resolvers on different systems,
diff --git a/doc/man-sections/script-options.rst
b/doc/man-sections/script-options.rst
index d73231ed..8c0be0cd 100644
--- a/doc/man-sections/script-options.rst
+++ b/doc/man-sections/script-options.rst
@@ -663,7 +663,6 @@ instances.
dns_server_{n}_address_{m}
dns_server_{n}_port_{m}
dns_server_{n}_resolve_domain_{m}
- dns_server_{n}_exclude_domain_{m}
dns_server_{n}_dnssec
dns_server_{n}_transport
dns_server_{n}_sni
diff --git a/src/openvpn/dns.c b/src/openvpn/dns.c
index b7808db1..51fca2fb 100644
--- a/src/openvpn/dns.c
+++ b/src/openvpn/dns.c
@@ -402,11 +402,9 @@ setenv_dns_options(const struct dns_options *o, struct
env_set *es)
if (s->domains)
{
-const char *format = s->domain_type == DNS_RESOLVE_DOMAINS ?
- "dns_server_%d_resolve_domain_%d" :
"dns_server_%d_exclude_domain_%d";
for (j = 1, d = s->domains; d != NULL; j++, d = d->next)
{
-setenv_dns_option(es, format, i, j, d->name);
+setenv_dns_option(es, "dns_server_%d_resolve_domain_%d", i, j,
d->name);
}
}
@@ -484,14 +482,7 @@ show_dns_options(const struct dns_options *o)
struct dns_domain *domain = server->domains;
if (domain)
{
-if (server->domain_type == DNS_RESOLVE_DOMAINS)
-{
-msg(D_SHOW_PARMS, "resolve domains:");
-}
-else
-{
-msg(D_SHOW_PARMS, "exclude domains:");
-}
+msg(D_SHOW_PARMS, "resolve domains:");
while (domain)
{
msg(D_SHOW_PARMS, " %s", domain->name);
diff --git a/src/openvpn/dns.h b/src/openvpn/dns.h
index 162dec12..e4978579 100644
--- a/src/openvpn/dns.h
+++ b/src/openvpn/dns.h
@@ -27,12 +27,6 @@
#include "buffer.h"
#include "env_set.h"
-enum dns_domain_type {
-DNS_DOMAINS_UNSET,
-DNS_RESOLVE_DOMAINS,
-DNS_EXCLUDE_DOMAINS
-};
-
enum dns_security {
DNS_SECURITY_UNSET,
DNS_SECURITY_NO,
@@ -68,7 +62,6 @@ struct dns_server {
size_t addr_count;
struct dns_server_addr addr[8];
struct dns_domain *domains;
-enum dns_domain_type domain_type;
enum dns_security dnssec;
enum dns_server
[Openvpn-devel] [PATCH 3/3] dns option: make server id/priority optional
With the discovery that most of the time only one DNS server's settings
can be applied on various systems, the priority value will likely serve
no purpose most of the time.
This is to make it optional to give a --dns server priority, for cases
where you only specify one DNS server anyway. We keep the priority
because it still serves the case where you want to override pushed
server settings with local ones and when you run backends which do
support multiple server's settings like dnsmasq(8).
Change-Id: I1f97d8e5ae8f049d72db5c12ce627f601d87505c
Signed-off-by: Heiko Hund
---
doc/man-sections/client-options.rst | 11
src/openvpn/dns.c | 9 +--
src/openvpn/dns.h | 4 ++-
src/openvpn/options.c | 41 +++--
4 files changed, 37 insertions(+), 28 deletions(-)
diff --git a/doc/man-sections/client-options.rst
b/doc/man-sections/client-options.rst
index 434e..df8ac433 100644
--- a/doc/man-sections/client-options.rst
+++ b/doc/man-sections/client-options.rst
@@ -168,11 +168,11 @@ configuration.
::
dns search-domains domain [domain ...]
- dns server n address addr[:port] [addr[:port] ...]
- dns server n resolve-domains domain [domain ...]
- dns server n dnssec yes|optional|no
- dns server n transport DoH|DoT|plain
- dns server n sni server-name
+ dns server [n] address addr[:port] [addr[:port] ...]
+ dns server [n] resolve-domains domain [domain ...]
+ dns server [n] dnssec yes|optional|no
+ dns server [n] transport DoH|DoT|plain
+ dns server [n] sni server-name
The ``--dns search-domains`` directive takes one or more domain names
to be added as DNS domain suffixes. If it is repeated multiple times within
@@ -180,6 +180,7 @@ configuration.
a server will amend locally defined ones.
The ``--dns server`` directive is used to configure DNS server ``n``.
+ If the ``n`` parameter is omitted the directive configures DNS server ``0``.
The server id ``n`` must be a value between -128 and 127. For pushed
DNS server options it must be between 0 and 127. The server id is used
to group options and also for ordering the list of configured DNS servers;
diff --git a/src/openvpn/dns.c b/src/openvpn/dns.c
index 51fca2fb..5f5e06b6 100644
--- a/src/openvpn/dns.c
+++ b/src/openvpn/dns.c
@@ -159,13 +159,18 @@ dns_domain_list_append(struct dns_domain **entry, char
**domains, struct gc_aren
}
bool
-dns_server_priority_parse(long *priority, const char *str, bool pulled)
+dns_server_priority_parse(long *priority, size_t *subidx, const char *str,
bool pulled)
{
char *endptr;
const long min = pulled ? 0 : INT8_MIN;
const long max = INT8_MAX;
long prio = strtol(str, , 10);
-if (*endptr != '\0' || prio < min || prio > max)
+if (endptr == str)
+{
+/* No priority found, str isn't numeric */
+*subidx -= 1;
+}
+else if (*endptr != '\0' || prio < min || prio > max)
{
return false;
}
diff --git a/src/openvpn/dns.h b/src/openvpn/dns.h
index e4978579..d0258f75 100644
--- a/src/openvpn/dns.h
+++ b/src/openvpn/dns.h
@@ -78,11 +78,13 @@ struct dns_options {
* Parses a string DNS server priority and validates it.
*
* @param priorityPointer to where the priority should be stored
+ * @param subidx Pointer to the sub-option index, decremented if no
+ * priority value could be found
* @param str Priority string to parse
* @param pulled Whether this was pulled from a server
* @return True if priority in string is valid
*/
-bool dns_server_priority_parse(long *priority, const char *str, bool pulled);
+bool dns_server_priority_parse(long *priority, size_t *subidx, const char
*str, bool pulled);
/**
* Find or create DNS server with priority in a linked list.
diff --git a/src/openvpn/options.c b/src/openvpn/options.c
index 3e0cb62b..ea69ea7a 100644
--- a/src/openvpn/options.c
+++ b/src/openvpn/options.c
@@ -510,7 +510,7 @@ static const char usage_message[] =
" ignore or reject causes the option to be allowed,
removed or\n"
" rejected with error. May be specified multiple times,
and\n"
" each filter is applied in the order of appearance.\n"
-"--dns server[value ...] : Configure option for DNS
server #n\n"
+"--dns server [n] [value ...] : Configure option for DNS
server #n\n"
" Valid options are :\n"
" address [addr[:port] ...] : server
addresses 4/6\n"
" resolve-domains [domain ...] : split domains\n"
@@ -7997,10 +7997,11 @@ add_option(struct options *options,
{
dns_domain_list_append(>dns_options.search_domains,
[2], >dns_options.gc);
}
-else if (s
[Openvpn-devel] [PATCH 1/3] dns option: allow up to eight addresses per server
This change allows configuration of more than one address per family
for a DNS server. This way you can specify backup addresses in case a
server is not reachable. During closer inspection of the various DNS
backend in supported operation systems it turned out that our previous
idea to have more than one DNS server applied in order of priority does
not work in most cases. Thus it became important to be able to specify
backup addresses. So instead of doing
dns server 1 address 1.2.3.4 2001::1
dns server 2 address 5.6.7.8 2001::2
to specify a backup addresses, this is now done like so:
dns server 1 address 1.2.3.4 2001::1
dns server 1 address 5.6.7.8 2001::2
or you can have all the addresses on one line if you like:
dns server 1 address 1.2.3.4 2001::1 2001::2 5.6.7.8
This also saves some repeated options when (backup) servers share the
same settings like "resolve-domains" compared to the originally intended
way.
The order in which addresses are given is retained for backends that
support this sort of cross address family ordering.
Change-Id: I9bd3d6d05da4e61a5fa05c0e455fc770b1fe186a
Signed-off-by: Heiko Hund
---
doc/man-sections/client-options.rst | 9 ++--
doc/man-sections/script-options.rst | 6 +--
src/openvpn/dns.c | 83 +++--
src/openvpn/dns.h | 18 ---
src/openvpn/options.c | 76 ++
5 files changed, 103 insertions(+), 89 deletions(-)
diff --git a/doc/man-sections/client-options.rst
b/doc/man-sections/client-options.rst
index 974cc992..fe9ffa6a 100644
--- a/doc/man-sections/client-options.rst
+++ b/doc/man-sections/client-options.rst
@@ -168,7 +168,7 @@ configuration.
::
dns search-domains domain [domain ...]
- dns server n address addr[:port] [addr[:port]]
+ dns server n address addr[:port] [addr[:port] ...]
dns server n resolve-domains|exclude-domains domain [domain ...]
dns server n dnssec yes|optional|no
dns server n transport DoH|DoT|plain
@@ -186,9 +186,10 @@ configuration.
lower numbers come first. DNS servers being pushed to a client replace
already configured DNS servers with the same server id.
- The ``address`` option configures the IPv4 and / or IPv6 address of
- the DNS server. Optionally a port can be appended after a colon. IPv6
- addresses need to be enclosed in brackets if a port is appended.
+ The ``address`` option configures the IPv4 and / or IPv6 address(es) of
+ the DNS server. Up to eight addresses can be specified per DNS server.
+ Optionally a port can be appended after a colon. IPv6 addresses need to
+ be enclosed in brackets if a port is appended.
The ``resolve-domains`` and ``exclude-domains`` options take one or
more DNS domains which are explicitly resolved or explicitly not resolved
diff --git a/doc/man-sections/script-options.rst
b/doc/man-sections/script-options.rst
index 3d2c7445..d73231ed 100644
--- a/doc/man-sections/script-options.rst
+++ b/doc/man-sections/script-options.rst
@@ -660,10 +660,8 @@ instances.
::
dns_search_domain_{n}
- dns_server_{n}_address4
- dns_server_{n}_port4
- dns_server_{n}_address6
- dns_server_{n}_port6
+ dns_server_{n}_address_{m}
+ dns_server_{n}_port_{m}
dns_server_{n}_resolve_domain_{m}
dns_server_{n}_exclude_domain_{m}
dns_server_{n}_dnssec
diff --git a/src/openvpn/dns.c b/src/openvpn/dns.c
index 9f2a7d5e..b7808db1 100644
--- a/src/openvpn/dns.c
+++ b/src/openvpn/dns.c
@@ -64,7 +64,7 @@ dns_server_addr_parse(struct dns_server *server, const char
*addr)
char addrcopy[INET6_ADDRSTRLEN] = {0};
size_t copylen = 0;
in_port_t port = 0;
-int af;
+sa_family_t af;
char *first_colon = strchr(addr, ':');
char *last_colon = strrchr(addr, ':');
@@ -115,21 +115,26 @@ dns_server_addr_parse(struct dns_server *server, const
char *addr)
return false;
}
+if (server->addr_count >= SIZE(server->addr))
+{
+return false;
+}
+
if (ai->ai_family == AF_INET)
{
struct sockaddr_in *sin = (struct sockaddr_in *)ai->ai_addr;
-server->addr4_defined = true;
-server->addr4.s_addr = ntohl(sin->sin_addr.s_addr);
-server->port4 = port;
+server->addr[server->addr_count].in.a4.s_addr =
ntohl(sin->sin_addr.s_addr);
}
else
{
struct sockaddr_in6 *sin6 = (struct sockaddr_in6 *)ai->ai_addr;
-server->addr6_defined = true;
-server->addr6 = sin6->sin6_addr;
-server->port6 = port;
+server->addr[server->addr_count].in.a6 = sin6->sin6_addr;
}
+server->addr[server->addr_count].family = af;
+server->addr[server->addr_count].port = port;
+server->addr_count += 1;
+
freeaddrinfo(ai);
return true;
}
@@ -197,7 +202,7 @@ dns_options_ve
Re: [Openvpn-devel] [PATCH] dns option: remove support for exclude-domains
On Donnerstag, 2. März 2023 21:25:03 CET David Sommerseth wrote: > I've only glared at the code and quickly done a few compile tests. > LGTM. Change itself also makes sense. > > Acked-By: David Sommerseth Thanks, please note that there is a newer version of this patch in gerrit. The delta is rather small. You can find the current version at [1]. FYI: testing gerrit with (all) the dns option related things and like it. Will send the final version to this list again after it has been approved in gerrit. [1] https://gerrit.openvpn.net/c/openvpn/+/39 ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
[Openvpn-devel] [PATCH] dns option: remove support for exclude-domains
No DNS resolver currently supports this and it is not possible to
emulate the behavior without the chance of errors. Finding the
effective default system DNS server(s) to specify the exclude
DNS routes is not trivial and cannot be verified to be correct
without resolver internal knowledge. So, it is better to not
support this instead of supporting it, but incorrectly.
Signed-off-by: Heiko Hund
---
doc/man-sections/client-options.rst | 14 +-
src/openvpn/dns.c | 13 ++---
src/openvpn/dns.h | 7 ---
src/openvpn/options.c | 16
4 files changed, 7 insertions(+), 43 deletions(-)
diff --git a/doc/man-sections/client-options.rst
b/doc/man-sections/client-options.rst
index 0b973adf..85d5610f 100644
--- a/doc/man-sections/client-options.rst
+++ b/doc/man-sections/client-options.rst
@@ -169,7 +169,7 @@ configuration.
dns search-domains domain [domain ...]
dns server n address addr[:port] [addr[:port]] [addr[:port]] [addr[:port]]
- dns server n resolve-domains|exclude-domains domain [domain ...]
+ dns server n resolve-domains domain [domain ...]
dns server n dnssec yes|optional|no
dns server n transport DoH|DoT|plain
dns server n sni server-name
@@ -191,14 +191,10 @@ configuration.
Optionally a port can be appended after a colon. IPv6 addresses need to
be enclosed in brackets if a port is appended.
- The ``resolve-domains`` and ``exclude-domains`` options take one or
- more DNS domains which are explicitly resolved or explicitly not resolved
- by a server. Only one of the options can be configured for a server.
- ``resolve-domains`` is used to define a split-dns setup, where only
- given domains are resolved by a server. ``exclude-domains`` is used to
- define domains which will never be resolved by a server (e.g. domains
- which can only be resolved locally). Systems which do not support fine
- grained DNS domain configuration, will ignore these settings.
+ The ``resolve-domains`` option takes one or more DNS domains used to define
+ a split-dns or dns-routing setup, where only the given domains are resolved
+ by the server. Systems which do not support fine grained DNS domain
+ configuration, will ignore this setting.
The ``dnssec`` option is used to configure validation of DNSSEC records.
While the exact semantics may differ for resolvers on different systems,
diff --git a/src/openvpn/dns.c b/src/openvpn/dns.c
index 18f6e58b..b1486c86 100644
--- a/src/openvpn/dns.c
+++ b/src/openvpn/dns.c
@@ -408,11 +408,9 @@ setenv_dns_options(const struct dns_options *o, struct
env_set *es)
if (s->domains)
{
-const char *format = s->domain_type == DNS_RESOLVE_DOMAINS ?
- "dns_server_%d_resolve_domain_%d" :
"dns_server_%d_exclude_domain_%d";
for (j = 1, d = s->domains; d != NULL; j++, d = d->next)
{
-setenv_dns_option(es, format, i, j, d->name);
+setenv_dns_option(es, "dns_server_%d_resolve_domain_%d", i, j,
d->name);
}
}
@@ -491,14 +489,7 @@ show_dns_options(const struct dns_options *o)
struct dns_domain *domain = server->domains;
if (domain)
{
-if (server->domain_type == DNS_RESOLVE_DOMAINS)
-{
-msg(D_SHOW_PARMS, "resolve domains:");
-}
-else
-{
-msg(D_SHOW_PARMS, "exclude domains:");
-}
+msg(D_SHOW_PARMS, "resolve domains:");
while (domain)
{
msg(D_SHOW_PARMS, " %s", domain->name);
diff --git a/src/openvpn/dns.h b/src/openvpn/dns.h
index 34f864dd..22cb333b 100644
--- a/src/openvpn/dns.h
+++ b/src/openvpn/dns.h
@@ -27,12 +27,6 @@
#include "buffer.h"
#include "env_set.h"
-enum dns_domain_type {
-DNS_DOMAINS_UNSET,
-DNS_RESOLVE_DOMAINS,
-DNS_EXCLUDE_DOMAINS
-};
-
enum dns_security {
DNS_SECURITY_UNSET,
DNS_SECURITY_NO,
@@ -69,7 +63,6 @@ struct dns_server {
struct dns_server_addr addr4[2];
struct dns_server_addr addr6[2];
struct dns_domain *domains;
-enum dns_domain_type domain_type;
enum dns_security dnssec;
enum dns_server_transport transport;
const char *sni;
diff --git a/src/openvpn/options.c b/src/openvpn/options.c
index 7ea1994a..19731b53 100644
--- a/src/openvpn/options.c
+++ b/src/openvpn/options.c
@@ -8016,22 +8016,6 @@ add_option(struct options *options,
}
else if (streq(p[3], "resolve-domains"))
{
-if (server->domain_type == DNS_EXCLUDE_DOMAINS)
-{
-msg(msglevel, "--dns server %ld: cannot use
resolve-domains and ex
[Openvpn-devel] [PATCH] dns option: allow up to two addresses per family
Signed-off-by: Heiko Hund
---
doc/man-sections/client-options.rst | 7 +--
src/openvpn/dns.c | 70 -
src/openvpn/dns.h | 19 +---
src/openvpn/options.c | 30 +++--
4 files changed, 72 insertions(+), 54 deletions(-)
diff --git a/doc/man-sections/client-options.rst
b/doc/man-sections/client-options.rst
index 974cc992..0b973adf 100644
--- a/doc/man-sections/client-options.rst
+++ b/doc/man-sections/client-options.rst
@@ -168,7 +168,7 @@ configuration.
::
dns search-domains domain [domain ...]
- dns server n address addr[:port] [addr[:port]]
+ dns server n address addr[:port] [addr[:port]] [addr[:port]] [addr[:port]]
dns server n resolve-domains|exclude-domains domain [domain ...]
dns server n dnssec yes|optional|no
dns server n transport DoH|DoT|plain
@@ -187,8 +187,9 @@ configuration.
already configured DNS servers with the same server id.
The ``address`` option configures the IPv4 and / or IPv6 address of
- the DNS server. Optionally a port can be appended after a colon. IPv6
- addresses need to be enclosed in brackets if a port is appended.
+ the DNS server. Up to two addresses per address family can be specified.
+ Optionally a port can be appended after a colon. IPv6 addresses need to
+ be enclosed in brackets if a port is appended.
The ``resolve-domains`` and ``exclude-domains`` options take one or
more DNS domains which are explicitly resolved or explicitly not resolved
diff --git a/src/openvpn/dns.c b/src/openvpn/dns.c
index 9f2a7d5e..18f6e58b 100644
--- a/src/openvpn/dns.c
+++ b/src/openvpn/dns.c
@@ -117,17 +117,25 @@ dns_server_addr_parse(struct dns_server *server, const
char *addr)
if (ai->ai_family == AF_INET)
{
+if (server->addr4_count >= SIZE(server->addr4))
+{
+return false;
+}
struct sockaddr_in *sin = (struct sockaddr_in *)ai->ai_addr;
-server->addr4_defined = true;
-server->addr4.s_addr = ntohl(sin->sin_addr.s_addr);
-server->port4 = port;
+server->addr4[server->addr4_count].in.a4.s_addr =
ntohl(sin->sin_addr.s_addr);
+server->addr4[server->addr4_count].port = port;
+server->addr4_count += 1;
}
else
{
+if (server->addr6_count >= SIZE(server->addr6))
+{
+return false;
+}
struct sockaddr_in6 *sin6 = (struct sockaddr_in6 *)ai->ai_addr;
-server->addr6_defined = true;
-server->addr6 = sin6->sin6_addr;
-server->port6 = port;
+server->addr6[server->addr6_count].in.a6 = sin6->sin6_addr;
+server->addr6[server->addr6_count].port = port;
+server->addr6_count += 1;
}
freeaddrinfo(ai);
@@ -197,7 +205,7 @@ dns_options_verify(int msglevel, const struct dns_options
*o)
o->servers ? o->servers : o->servers_prepull;
while (server)
{
-if (!server->addr4_defined && !server->addr6_defined)
+if (server->addr4_count == 0 && server->addr6_count == 0)
{
msg(msglevel, "ERROR: dns server %ld does not have an address
assigned", server->priority);
return false;
@@ -376,26 +384,26 @@ setenv_dns_options(const struct dns_options *o, struct
env_set *es)
for (i = 1, s = o->servers; s != NULL; i++, s = s->next)
{
-if (s->addr4_defined)
-{
-setenv_dns_option(es, "dns_server_%d_address4", i, -1,
- print_in_addr_t(s->addr4.s_addr, 0, ));
-}
-if (s->port4)
+for (j = 0; j < s->addr4_count; ++j)
{
-setenv_dns_option(es, "dns_server_%d_port4", i, -1,
- print_in_port_t(s->port4, ));
+setenv_dns_option(es, "dns_server_%d_address4_%d", i, j + 1,
+ print_in_addr_t(s->addr4[j].in.a4.s_addr, 0,
));
+if (s->addr4[j].port)
+{
+setenv_dns_option(es, "dns_server_%d_port4_%d", i, j + 1,
+ print_in_port_t(s->addr4[j].port, ));
+}
}
-if (s->addr6_defined)
+for (j = 0; j < s->addr6_count; ++j)
{
-setenv_dns_option(es, "dns_server_%d_address6", i, -1,
- print_in6_addr(s->addr6, 0, ));
-}
-if (s->port6)
-{
-setenv_dns_option(es, "dns_server_%d_port6", i, -1,
- print_in_port_t(s->port6, ));
+setenv_dns_option(es, "dns_server_%d_address6_%d", i, j + 1,
+ print_in6_addr(s->addr6[j].in.a6, 0, ));
+
Re: [Openvpn-devel] [PATCH v7] Dynamic tls-crypt for secure soft_reset/session renegotiation
On Dienstag, 31. Januar 2023 14:52:48 CET Arne Schwabe wrote: > Patch v7: also xor tls-auth key data into the dynamic tls-crypt key like > tls-crypt key data I've tested this with the openvpn3 implementation thoroughly in combination with --tls-auth, --tls-crypt and without any wrapping of the control channel and found everything working as intended, hence: Acked-by: Heiko Hund Only this last little quirk: > +Dynamic TLS Crypt > +When both peers are OpenVPN 2.6.0+, OpenVPN will dynamically create maybe can be changed during merge to 2.6.1+ or whatever will fit. ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
Re: [Openvpn-devel] [PATCH 3/3] Introduce dynamic tls-crypt for secure soft_reset/session renegotiation
On Freitag, 9. September 2022 21:59:02 CEST Arne Schwabe wrote:
> --- a/src/openvpn/multi.c
> +++ b/src/openvpn/multi.c
> @@ -1803,6 +1803,10 @@ multi_client_set_protocol_options(struct context *c)
> {
> o->imported_protocol_flags |= CO_USE_TLS_KEY_MATERIAL_EXPORT;
> }
> +if (proto & IV_PROTO_SECURE_RENOG)
I think any occurrence of "renog" (i.e. short for renegotiation[s]) should be
changed to "reneg" throughout this patch. See --reneg-x options. Could be just
me, so a more opinions would be welcome.
> --- a/src/openvpn/options.c
> +++ b/src/openvpn/options.c
> @@ -8553,6 +8553,10 @@ add_option(struct options *options,
> {
> options->imported_protocol_flags |=
> CO_USE_TLS_KEY_MATERIAL_EXPORT; }
> +else if (streq(p[j], "secure-renog"))
Should be rewritten to use --protocol-flags instead.
> @@ -71,9 +71,77 @@ tls_crypt_init_key(struct key_ctx_bi *key, const char
> *key_file, msg(M_FATAL, "ERROR: --tls-crypt not supported");
> }
> crypto_read_openvpn_key(, key, key_file, key_inline, key_direction,
> -"Control Channel Encryption", "tls-crypt");
> +"Control Channel Encryption", "tls-crypt",
> keydata); }
>
> +/**
> + * Will produce dest = dest XOR data
> + */
> +static void
> +xor_key2_key(struct key2 *dest, const struct key2 *data)
I wonder if this would be more readable as
xor_key(struct key2 *key, const struct key2 *mask)
> +bool
> +tls_session_generate_secure_renegotition_key(struct tls_multi *multi,
> + struct tls_session *session)
typo renegotiation_key
> +struct key2 rengokeys;
typo renogkeys (renegkeys, IMHO)
> @@ -285,7 +354,7 @@ tls_crypt_v2_init_client_key(struct key_ctx_bi *key,
> struct buffer *wkc_buf, }
>
> tls_crypt_v2_load_client_key(key, , false);
> -secure_memzero(, sizeof(key2));
> +*original_key = key2;
We should do the zeroing in tls_session_generate_secure_renegotiation_key()
shortly after we used it to XOR then. And maybe only delay it if we need to
XOR anyways, could use original_key == NULL as indication.
> @@ -587,8 +655,8 @@ tls_crypt_v2_extract_client_key(struct buffer *buf,
> ctx->cleanup_key_ctx = true;
> ctx->opt.flags |= CO_PACKET_ID_LONG_FORM;
> memset(>opt.key_ctx_bi, 0, sizeof(ctx->opt.key_ctx_bi));
> -tls_crypt_v2_load_client_key(>opt.key_ctx_bi, _key, true);
> -secure_memzero(_key, sizeof(client_key));
> +tls_crypt_v2_load_client_key(>opt.key_ctx_bi,
> + >original_tlscrypt_keydata, true);
Same as above for the server side. Could zero here immediately if
original_tlscrypt_keydata == NULL
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel
Re: [Openvpn-devel] [PATCH 1/3] Allows renegotiation only to start if session is fully established
On Freitag, 9. September 2022 21:59:00 CEST Arne Schwabe wrote: > This change makes the state machine more strict in terms of transation *transitions > Signed-off-by: Arne Schwabe Acked-by: Heiko Hund For those who wonder what this is/does, my take on it: basically shields the calls to key_state_soft_reset() on both the sending and receiving side, and ensures both ends have completed the control channel negotiation fully (i.e. are in state S_GENERATED_KEYS). At that time pushed options are guaranteed to be processed/active, which was the main motivation for this change, as I understand it. ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
Re: [Openvpn-devel] [PATCH v3 4/5] Implement a function to calculate the default MTU
On Montag, 27. Juni 2022 10:36:02 CEST Frank Lichtenheld wrote: > As mentioned this is true for the specific options configured above. > But you can easily also get different values out of this function by > changing the options because frame_calculate_payload overhead does > not always return the same value. I don't get it. It produces the same output for the same input reliably. Do you want to see a test which forces something different to 1420 by modifying the inputs accordingly? Heiko ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
Re: [Openvpn-devel] [PATCH v3 2/5] Allow tun-mtu to be pushed
Doesn't apply to master anymore, please rebase.
On Sonntag, 26. Juni 2022 01:41:47 CEST Arne Schwabe wrote:
> --- a/doc/man-sections/vpn-network-options.rst
> +++ b/doc/man-sections/vpn-network-options.rst
> @@ -516,6 +516,11 @@ routing.
>It's best to use the ``--fragment`` and/or ``--mssfix`` options to deal
>with MTU sizing issues.
>
> +--tun-max-mtu maxmtu
--tun-mtu-max
> +if (c->options.ce.tun_mtu > frame->tun_max_mtu)
> +{
> +msg(D_PUSH_ERRORS, "Server pushed a large mtu, please add "
> +"tun-mtu-max %d in the client configuration", +
"a too large MTU, to accept it add" would describe it better
> @@ -222,6 +222,7 @@ frame_print(const struct frame *frame,
> buf_printf(, " max_frag:%d", frame->max_fragment_size);
> #endif
> buf_printf(, " tun_mtu:%d", frame->tun_mtu);
> +buf_printf(, " tun_max_mtu:%d", frame->tun_max_mtu);
should be printed as "tun_mtu_max" to be consistent with the option. Maybe it
Should even make sense to rename the frame element to match the option name as
they are closely related.
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel
Re: [Openvpn-devel] [PATCH v3 3/5] Push server mtu to client when support and support occ mtu
On Sonntag, 26. Juni 2022 01:41:48 CEST Arne Schwabe wrote: > To maximise compatibility allow to lie our MTU in the default OCC > message. > > Patch v2: improve documentation > Patch v3: split changing default MTU into its own patch Doesn't apply to master anymore. From what I have seen, besides the small typo below: Acked-by: Heiko Hund > + Starting with OpenVPN 2.6 when running server mode (``--mode server``, > + ``--server``, or ``-server-ipv6`` options present in the configuration), Should be ``--server-ipv6`` ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
Re: [Openvpn-devel] [PATCH v3 26/28] Allow setting control channel packet size with tls-mtu
This patch need to be rebased again, does not apply to master anymore: checking file src/openvpn/ssl.c Hunk #1 succeeded at 297 (offset 1 line). Hunk #2 FAILED at 321. Hunk #3 succeeded at 1256 (offset -42 lines). On Mittwoch, 11. Mai 2022 13:08:10 CEST Arne Schwabe wrote: > @@ -141,6 +147,8 @@ User-visible Changes > +- control channel packet maximum size is no longer influenced by ``--link- mtu``/``--tun-mtu`` > + and must be set by ``--tls-mtu`` now. Must it or is there still some cc-mtu derivation in place in case it is not given? If not, wouldn't it make sense to at least catch obvious breakage. Usually link/tun-mtu is lowered for that reason. Further down in the patch there are min/max_int() calls in tls_init_control_channel_frame_parameters(), do they do what I suggested here already? > diff --git a/doc/man-sections/link-options.rst > +--tls-mtu size Is --tls-mtu the best name for the option we can come up with? From my outside perspective (can hardly spell mtu right) the "tls" part is somewhat misnamed, as this is an SSL VPN after all. When you know that the framing for the data channel is not TLS you can make sense of it, but who does. > + The maximum packet size includes encapsulation overhead like UDP > and IP. > \ No newline at end of file ^ this > +int tls_mtu; /* Maximum MTU for the > control channel messages */ The "M" already stands for "Maximum". ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
Re: [Openvpn-devel] [PATCH v3] Implement exit notification via control channel
On Freitag, 19. August 2022 14:38:53 CEST Arne Schwabe wrote: > Current exit notification relies on data channel messages with specific > prefix. Adding these to new data channel modules (DCO) adds unncessary > complexity for the data for messages that from their idea belong to the > control channel anyway. > > This patch adds announcing support for control channel and sending/receving > it. We use the simple EXIT message for this. > > Patch V2: add comment about protocol-flags to be not a user visible option, > fix various grammar mistakes, remove unused argument to > receive_exit_message > > Patch V3: rename data_channel_crypto_flags to imported_protocol_flags > add tls-ekm to protocol-flags. Hunks #2 and #3 in push.c do not apply to master anymore, thus some rebase action is needed. Besides that: Acked-by: Heiko Hund ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
Re: [Openvpn-devel] [PATCH v3] Implement AUTH_FAIL, TEMP message support
On Mittwoch, 24. August 2022 14:58:58 CEST Arne Schwabe wrote:
> Patch v3: cleanup parse_auth_failed_temp to use a simple const string
> instead of a buffer
Besides the pending rebase and the one code smell below:
Acked-by: Heiko Hund
> src/openvpn/openvpn.vcxproj.filters | 3 +
This file is gone in the meantime
> src/openvpn/ssl.c| 13 ++--
Doesn't apply to master anymore
> +parse_auth_failed_temp(struct options *o, const char *reason)
> +{
> +struct gc_arena gc = gc_new();
> +/* skip TEMP */
> +const char *message = reason + strlen("TEMP");
This should be checked more. Or probably better, "reason" should be passed
from the outside as "reason + 4".
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel
Re: [Openvpn-devel] [PATCH v3 4/4] Allow scripts and plugins to set a custom AUTH_FAILED message
On Mittwoch, 24. August 2022 16:08:48 CEST Arne Schwabe wrote: > This is currently only possible when using the management interface > and the client-deny functionality. > > Patch v3: add missing gc_free Acked-by: Heiko Hund ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
Re: [Openvpn-devel] [PATCH v5] Implement --client-crresponse script options and plugin interface
On Mittwoch, 24. August 2022 13:09:30 CEST Arne Schwabe wrote: > This is allows scripts and pluginsto parse/react to a CR_RESPONSE message > > Patch V2: doc fixes, do not put script under ENABLE_PLUGIN > Patch V3: rebase > Patch V4: fix else branch of the verify_crresponse_script function > Patch V5: unify message when unable to create/write crresponse file > > Signed-off-by: Arne Schwabe Acked-by: Heiko Hund ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
Re: [Openvpn-devel] [PATCH v4] Implement --client-crresponse script options and plugin interface
On Freitag, 19. August 2022 11:51:32 CEST Arne Schwabe wrote:
> +verify_crresponse_script(struct tls_multi *multi, const char *cr_response)
> +{
[...]
> +if (!status_close(so))
> +{
> +msg(D_TLS_ERRORS, "TLS CR Response Error: could not write cr"
> +"responsed to file: %s",
> +tmp_file);
> +tls_deauthenticate(multi);
> +goto done;
> +}
> +}
> +else
> +{
> +msg(D_TLS_ERRORS, "TLS Auth Error: could not write "
> +"username/password to cr response temp file");
The message is still wrong. Which proves my point about unifying it.
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel
Re: [Openvpn-devel] [PATCH v2 2/4] Cleanup receive_auth_failed and simplify method
On Donnerstag, 18. August 2022 19:20:33 CEST Gert Doering wrote: > On Thu, Aug 18, 2022 at 04:39:07PM +0200, Heiko Hund wrote: > > On Freitag, 20. Mai 2022 23:32:48 CEST Arne Schwabe wrote: > > > Patch V2: remove uncessary ifdef/endif and unnecassary block > > > > Acked-by: Heiko Hund > > Thanks for the extra ACK :-) - this got already merged as > > commit 88823adebac31958cee83572241cff9fc775a601 Hm, was still showing in patchwork. ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
Re: [Openvpn-devel] [PATCH v2 4/4] Allow scripts and plugins to set a custom AUTH_FAILED message
What Frank said, and the nitpick about this hunk which should be removed:
On Freitag, 20. Mai 2022 23:32:50 CEST Arne Schwabe wrote:
> @@ -1376,6 +1440,7 @@ verify_user_pass_plugin(struct tls_session *session,
> struct tls_multi *multi, /* call command */
> retval = plugin_call(session->opt->plugins,
> OPENVPN_PLUGIN_AUTH_USER_PASS_VERIFY, NULL, NULL, session->opt->es);
>
> +
> if (retval == OPENVPN_PLUGIN_FUNC_DEFERRED)
> {
> /* Check if the plugin has written the pending auth control
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel
Re: [Openvpn-devel] [PATCH v2 1/4] Implement exit notification via control channel
Patch and thus series doesn't apply anymore, in addition to eventual changes
also please rebase.
On Freitag, 20. Mai 2022 23:32:47 CEST Arne Schwabe wrote:
> + If both server and client support sending this message using the control
> + channel, the message will be sent as control-channel message. Otherwise
> + the message is sent as data-channel message, which will be ignored by
> + data-channel offloaded peers.
> +
>The **n** parameter (default :code:`1` if not present) controls the
>maximum number of attempts that the client will try to resend the exit
> - notification message.
> + notification message if messages are sent in data-channel mode.
You don't need the "n" for the control channel exit notification because of
the reliability layer, right?!
> +if (proto & IV_PROTO_CC_EXIT_NOTIFY)
> +{
> +o->data_channel_crypto_flags |= CO_USE_CC_EXIT_NOTIFY;
> +}
The flags variable is named confusingly now. Is renaming an option?
> static void
> process_explicit_exit_notification_init(struct context *c)
> {
> msg(M_INFO, "SIGTERM received, sending exit notification to peer");
> event_timeout_init(>c2.explicit_exit_notification_interval, 1, 0);
> reset_coarse_timers(c);
Do we need to timer event if !cc_exit_notify_enabled(c)?
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel
Re: [Openvpn-devel] [PATCH v2 3/4] Implement AUTH_FAIL, TEMP message support
On Freitag, 20. Mai 2022 23:32:49 CEST Arne Schwabe wrote:
> This allows a server to indicate a temporary problem on the server and
> allows the server to indicate how to proceed (i.e. move to the next server,
> retry the same server, wait a certain time,...)
>
> This adds options_utils.c/h to be able to unit test the new function.
>
> Patch v2: Improve documentation, format man page better, comment that
> protocol-flags is not a user usable option.
> ---
> doc/man-sections/script-options.rst | 36 ++
> src/openvpn/Makefile.am | 1 +
> src/openvpn/init.c | 9 ++-
> src/openvpn/openvpn.vcxproj | 2 +
> src/openvpn/openvpn.vcxproj.filters | 3 +
> src/openvpn/options.h| 9 ++-
> src/openvpn/options_util.c | 104 +++
> src/openvpn/options_util.h | 33 +
> src/openvpn/push.c | 11 ++-
> src/openvpn/ssl.c| 13 ++--
> src/openvpn/ssl.h| 3 +
> tests/unit_tests/openvpn/Makefile.am | 1 +
> tests/unit_tests/openvpn/test_misc.c | 49 +
> 13 files changed, 266 insertions(+), 8 deletions(-)
> create mode 100644 src/openvpn/options_util.c
> create mode 100644 src/openvpn/options_util.h
>
> diff --git a/doc/man-sections/script-options.rst
This is more related to 4/4 and should go there for code archaeology reasons.
> +/* the server can suggest a backoff time to the client, it
> + * will still be capped by the max timeout between connections
> + * (300s by default) */
> +int server_backoff_time;
This should be (parsed as) an unsigned value. Negative backup requires too
advanced physics. ;-)
> +const char *
> +parse_auth_failed_temp(struct options *o, const struct buffer *buf)
> +{
> +struct gc_arena gc = gc_new();
> +char *m = string_alloc(BSTR(buf), );
> +/* skip TEMP */
> +m += strlen("TEMP");
> +const char *message = BSTR(buf) + 4;
I think if we string_alloc(BSTR(buf) + 4, ...) it's prettier. We have the
magic number 4 for message anyway, and it's not so magic if we keep the
comment around.
Generally I wanted to comment on the alloc only for reasons of \0 terminating
to options, but The code is not prettier if it is changed and performance if
not the issue here, so no comment (technically).
> \ No newline at end of file
That.
> +++ b/src/openvpn/options_util.h
> \ No newline at end of file
Again
> diff --git a/src/openvpn/push.c b/src/openvpn/push.c
> index 1c4e637e4..3b3dce642 100644
> --- a/src/openvpn/push.c
> +++ b/src/openvpn/push.c
> @@ -51,7 +52,6 @@ void
> receive_auth_failed(struct context *c, const struct buffer *buffer)
> {
> msg(M_VERB0, "AUTH: Received control message: %s", BSTR(buffer));
> -c->options.no_advance = true;
>
> if (!c->options.pull)
> {
I think we want to keep this as the default. If I read the code correctly, at
the moment the default behavior is "advance addr" if none is given, which feel
wrong to me, given what I have read about the feature.
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel
Re: [Openvpn-devel] [PATCH v2 1/4] Implement exit notification via control channel
On Freitag, 1. Juli 2022 00:42:55 CEST Arne Schwabe wrote: > Basically if I had been a bit more forwarding looking we would now have > protocol-flags ekm cc-exit instead of key-derivation ekm and > protocol-flags cc-exit Then maybe also add support for handling ekm via --protocol-flags and deprecate/remove sending --key-derivation? ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
Re: [Openvpn-devel] [PATCH v2 2/4] Cleanup receive_auth_failed and simplify method
On Freitag, 20. Mai 2022 23:32:48 CEST Arne Schwabe wrote: > This simplifies the buffer handling in the method and adds a quick > return instead of wrapping the whole method in a if (pull) block > > Patch V2: remove uncessary ifdef/endif and unnecassary block Acked-by: Heiko Hund ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
Re: [Openvpn-devel] [PATCH v3] Fix OpenVPN querying user/password if auth-token with user expires
On Donnerstag, 17. Februar 2022 19:22:34 CEST Arne Schwabe wrote:
> @@ -590,6 +590,7 @@ init_query_passwords(const struct context *c)
> /* Auth user/pass input */
> if (c->options.auth_user_pass_file)
> {
> +enable_auth_user_pass();
> #ifdef ENABLE_MANAGEMENT
> auth_user_pass_setup(c->options.auth_user_pass_file,
> >options.sc_info); #else
This should be inside the #ifdef to do exactly the same as before, i.e.
doesn't introduce side effects potentially. But then we want to get rid of
ENABLE_MANAGEMENT soonish, thus it may not matter.
Acked-by: Heiko Hund
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel
Re: [Openvpn-devel] [PATCH v3] Add example script demonstrating TOTP via auth-pending
On Donnerstag, 4. März 2021 12:40:18 CEST Arne Schwabe wrote: > + For a sample script that implement TOTP (RFC 6238) based two-factor > + authentication, see :code:`sample-scripts/totp.py`. This filename doesn't match with below. > diff --git a/sample/sample-scripts/totpauth.py > b/sample/sample-scripts/totpauth.py new file mode 100755 Have not looked at the Python code much, but David already did. More of a Perl guy anyway. ;-) ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
Re: [Openvpn-devel] [PATCH v3] Implement --client-crresponse script options and plugin interface
On Dienstag, 18. Mai 2021 14:26:35 CEST Arne Schwabe wrote:
> This is allows scripts and pluginsto parse/react to a CR_RESPONSE message
This commit message needs a makeover, I think.
> - If ``method`` is set to :code:`via-env`, OpenVPN will call ``script``
> + If ``method`` is set to :code:`via-env`, OpenVPN will call ``cmd``
These drive-by fixes Antonio spotted make sense and are not intrusive enough
that I care.
> +verify_crresponse_script(struct tls_multi *multi, const char *cr_response)
[...]
> +const char *tmp_file = platform_create_temp_file(session->opt->tmp_dir,
> "cr", );
> +if (tmp_file)
> +{
> +struct status_output *so = status_open(tmp_file, 0, -1, NULL,
> + STATUS_OUTPUT_WRITE);
> +status_printf(so, "%s", cr_response);
> +if (!status_close(so))
> +{
> +msg(D_TLS_ERRORS, "TLS CR Response Error: could not write cr"
> + "responsed to file: %s",
> +tmp_file);
> +tls_deauthenticate(multi);
> +goto done;
> +}
> +}
> +else
> +{
> +msg(D_TLS_ERRORS, "TLS Auth Error: could not create write "
> + "username/password to temp file");
> +}
This else branch should be the same as the "if (!status_close(so))" one above
I think, as you don't want to call the script without a valid tempfile.
Besides that the error message is copy/paste wrong anyway. So, maybe introduce
a bool and do the error handling in one place, might help future copy/pastes
as well.
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel
Re: [Openvpn-devel] [PATCH v2 14/25] dco: implement dco support for p2mp/server code path
On Donnerstag, 28. Juli 2022 21:55:01 CEST Antonio Quartulli wrote: > This change introduces ovpn-dco support along the p2mp/server code path. > Some code seems to be duplicate of the p2p version, but details are > different, so it couldn't be shared. > > Signed-off-by: Antonio Quartulli > --- > > Changes from v1: > * fix if condition P_DATA_V2 -> P_DATA_V1 > * fix unknown reason string Acked-by: Heiko Hund ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
Re: [Openvpn-devel] [PATCH] options: don't export local function pre_connect_save()
On Montag, 11. Juli 2022 14:23:48 CEST Antonio Quartulli wrote: > The pre_connect_save() function is not used outside of options.c, > therefore it should not be exported. > > Make it static and move definition before its invocation. > Move also pre_connect_restore() along with it in order to keep the two > close to each other. > > Cc: Arne Schwabe > Signed-off-by: Antonio Quartulli Noticed this as well before, but was too lazy to patch ;-) Acked-by: Heiko Hund ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
Re: [Openvpn-devel] [PATCH v2 23/23] dco-win: implement ovpn-dco support in P2P Windows code path
On Montag, 11. Juli 2022 16:19:44 CEST Antonio Quartulli wrote: > With this change it is possible to use ovpn-dco-win when running OpenVPN > in client or P2P mode. > > Signed-off-by: Arne Schwabe > Signed-off-by: Lev Stipakov > Signed-off-by: Antonio Quartulli > --- > > Changes from v1: > * use suffix _dco_win instead of _windco > * create helper function to retrieve last error from socket object Acked-by: Heiko Hund ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
Re: [Openvpn-devel] [PATCH] networking: fix doc for net_iface_new() API
On Dienstag, 5. Juli 2022 11:18:42 CEST Antonio Quartulli wrote: > Some auto correction must have sneaked in. > Restore proper wording. > > Signed-off-by: Antonio Quartulli > --- > src/openvpn/networking.h | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/src/openvpn/networking.h b/src/openvpn/networking.h > index 647718e0..79963756 100644 > --- a/src/openvpn/networking.h > +++ b/src/openvpn/networking.h > @@ -94,7 +94,8 @@ void net_ctx_free(openvpn_net_ctx_t *ctx); > * @param iface interface to create > * @param type string describing interface type > * @param arg extra data required by the specific type > - * @return int 0 on success, negative error code on error > + * > + * @return 0 on success, negative error code on error > */ > int net_iface_new(openvpn_net_ctx_t *ctx, const openvpn_net_iface_t *iface, > const char *type, void *arg); Acked-by: Heiko Hund ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
Re: [Openvpn-devel] [PATCH 02/25] dco: add helper function to detect if DCO is enabled or not
On Freitag, 24. Juni 2022 10:37:46 CEST Antonio Quartulli wrote:
> +/**
> + * Returns whether the current configuration has dco enabled.
> + */
> +static inline bool
> +dco_enabled(const struct options *o)
> +{
> +return !o->tuntap_options.disable_dco;
> +}
I think it would be beneficial if this took a tun_options* instead, as it
could be used more widely and replace the many "!tt->options.disable_dco"
occurrences and alike.
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel
Re: [Openvpn-devel] [PATCH 08/25] dco: allow user to disable it at runtime
On Freitag, 24. Juni 2022 10:37:52 CEST Antonio Quartulli wrote: > +else if (streq(p[0], "disable-dco") || streq(p[0], "dco-disable")) Don't think we need to be backwards compatible here, or do we? ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
Re: [Openvpn-devel] [PATCH 24/25] dco-win: add documentation to README.dco.md
On Freitag, 24. Juni 2022 10:38:08 CEST Antonio Quartulli wrote: > +Getting started (Windows) > +- > +Getting started under windows is currently for brave people having > experience > +with windows development. You need to compile openvpn yourself > and also need > +to get the test driver installed on your system. Not so brave anymore, maybe rephrase with more concrete steps to perform. ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
Re: [Openvpn-devel] [PATCH 23/25] dco-win: implement ovpn-dco support in P2P Windows code path
On Freitag, 24. Juni 2022 10:38:07 CEST Antonio Quartulli wrote:
> +if (!is_windco(c->c1.tuntap))
[...]
> +if ((options->windows_driver ==
> WINDOWS_DRIVER_WINTUN || options->windows_driver == WINDOWS_DRIVER_WINDCO)
[...]
> +create_socket_windco(struct context *c, struct link_socket *sock,
Thinking "_dco_win" would be a better suffix for consistency.
> -status = WSAGetLastError();
> +if (sock->info.dco_installed)
> +{
> +status = GetLastError();
> +}
> +else
> +{
> +status = WSAGetLastError();
> +}
Small wrapper would be good as this construct appears twice.
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel
Re: [Openvpn-devel] [PATCH 21/25] do_open_tun: restyle "can preserve TUN" check
On Freitag, 24. Juni 2022 10:38:05 CEST Antonio Quartulli wrote: > The current condition checking if the TUN interface was preserved is > dependant on the platform being Android or not. This makes the code > reasonably ugly, especially because uncrustify can't indent properly. > > On top of that, we will require an extra condition only for windows+DCO, > which will make the check even uglier. > > For this reason, factor out the check in a separate function which can > keep the ifdefs craziness well hidden, while do_open_tun becomes > (a bit) cleaner. > > Signed-off-by: Antonio Quartulli Looks mighty, but is rather trivial. Two things: I think the code could benefit from factoring out the call to run_up_down() and block_outside_dns into two simple functions. Currently it's pretty much duplicated in the if and else branch. Besides that: Acked-by: Heiko Hund ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
Re: [Openvpn-devel] [PATCH 14/25] dco: implement dco support for p2mp/server code path
On Freitag, 24. Juni 2022 10:37:58 CEST Antonio Quartulli wrote: > +uint8_t *ptr = BPTR(>dco_packet_in); > +uint8_t op = ptr[0] >> P_OPCODE_SHIFT; > +if (op == P_DATA_V2 || op == P_DATA_V2) This looks odd. Seems you wanted to check for a second opcode, or is it obsolete? > +const char *reason = "(unknown reason by ovpn-dco)"; Wouldn't "ovpn-dco: unknown reason" be better? ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
Re: [Openvpn-devel] [PATCH 20/25] dco-win: add platform dependant check on incompatible options
On Freitag, 24. Juni 2022 10:38:04 CEST Antonio Quartulli wrote: > Some platforms may have different constraints in terms of incompatible > opions, therefore we add a function that explicitly checks those. > > Also, add generic option check for when ovpn-dco-win is in use. > > Signed-off-by: Antonio Quartulli > Signed-off-by: Lev Stipakov Acked-by: Heiko Hund ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
Re: [Openvpn-devel] [PATCH 22/25] dco-win: introduce low-level code for handling ovpn-dco-win in Windows
On Freitag, 24. Juni 2022 10:38:06 CEST Antonio Quartulli wrote:
> +int
> +dco_del_key(dco_context_t *dco, unsigned int peerid, dco_key_slot_t slot)
> +{
> +msg(D_DCO, "%s: peer-id %d, slot %d called but ignored", __func__,
> peerid, +slot);
> +/* FIXME: Implement in driver first */
Suppose this is not critical and the driver gets rid of keys itself?
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel
Re: [Openvpn-devel] [PATCH 15/25] dco: add documentation for ovpn-dco-linux
On Freitag, 24. Juni 2022 10:37:59 CEST Antonio Quartulli wrote: > +application. Note that DCO will use DATA_V2 packets > in P2P mode, therefore, > +this implies that peers must be running 2.6.0+ > in order to have P2P-NCP > +which brings DATA_V2 packet support. [...] > +- OpenVPN 2.4.0 is the minimum peer version. Now I'm not sure what the minimum version is. Maybe clarify. ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
Re: [Openvpn-devel] [PATCH 18/25] dco: turn supported ciphers list into a function
On Freitag, 24. Juni 2022 10:38:02 CEST Antonio Quartulli wrote: > Other platforms may need more complex logic to decide whether a cipher > is supported or not, therefore turn hardcoded list into a function that > can be implemented by each platform independently. > > Signed-off-by: Lev Stipakov > Signed-off-by: Antonio Quartulli Trivial and compile-checked. Acked-by: Heiko Hund ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
Re: [Openvpn-devel] [PATCH 13/25] dco: implement dco support for p2p/client code path
On Freitag, 24. Juni 2022 10:37:57 CEST Antonio Quartulli wrote:
> +/* These inet_pton conversion are fatal since options.c already
> implements
> + * checks to have only valid addresses when setting the
> options */
> +if (c->options.ifconfig_ipv6_remote)
> +{
> +if (inet_pton(AF_INET6, c->options.ifconfig_ipv6_remote,
> _ip6) != 1) +{
> +msg(M_FATAL,
> +"DCO peer init: problem converting IPv6 ifconfig remote
> address %s to binary", +c->options.ifconfig_ipv6_remote);
> +}
> +remote_addr6 = _ip6;
> +}
I'm undecided if these fatal errors are justified with respect to defensive
programming or overly paranoid, because they will never appear.
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel
Re: [Openvpn-devel] [PATCH v4] dco: let open_tun_generic handle the DCO case
On Mittwoch, 29. Juni 2022 14:49:45 CEST Antonio Quartulli wrote: > name when not specified b the user. For this reason the DCO case can nit: b -> by > +strncpynt(tunname, dynamic_name, > + sizeof(dynamic_name)); This need to be sizeof(tunname). ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
Re: [Openvpn-devel] [PATCH applied] Re: dco: introduce low-level code for handling ovpn-dco in the Linux kernel
On Dienstag, 28. Juni 2022 17:07:14 CEST Gert Doering wrote: > Uncrustify has complained at me when I merged the patch (because in that > moment, ovpn_dco_linux.h was "newly modified" and the exclusion rule > does not match on the pre-commit-hook) - but as discussed, this is a bit > complicated due to "kernel style" vs "openvpn style", so I've left it > alone for the moment. That's something which needs to be improved. Who is dev-tools/special-files.lst intended for? Can uncrustify read it? Heiko ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
Re: [Openvpn-devel] [PATCH v2] do not push route-ipv6 entries that are also in the iroute-ipv6 list
On Dienstag, 28. Juni 2022 10:20:24 CEST Antonio Quartulli wrote: > A server should push a route to a client only if there is no matching > iroute for the same client. > > While this logic works fine for IPv4, there is no IPv6 counterpart. > > Implement the same check for IPv6 routes and discard matching ones > from the push list. > > Trac: #354 > Cc: Gert Doering > Signed-off-by: Antonio Quartulli ACK ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
Re: [Openvpn-devel] do not push route-ipv6 entries that are also in the iroute-ipv6 list
On Mittwoch, 23. Mai 2018 21:28:02 CEST Antonio Quartulli wrote: > -if (o && o->push_list.head && o->iroutes) > +if (o && o->push_list.head && (o->iroutes || o->iroutes_ipv6)) [...] > +else if (p[0] && !strcmp(p[0], "route-ipv6") && !p[2]) I think it would make sense to check for o->iroutes_ipv6 here again, so that the address string parsing only happens if there's v6 iroute(s). And then also add a check for o->iroutes to the "route" if statement above. That might save a few cycles, but no big issue. Besides that: ACK Heiko ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
Re: [Openvpn-devel] [PATCH] include unistd.h for _exit(2) declaration
Hi Gert On Freitag, 27. Mai 2022 13:40:57 CEST Gert Doering wrote: > is included by "syshead.h" already today, with a nice > HAVE_UNISTD_H wraper (thus, not depending on a "if it's not WIN32, > it surely must have unistd.h" assumption). > > So I wonder what issue this fixes? I see no compile time warnings about > _exit() either. I get "Implicit declaration of function '_exit' is invalid in C99 [-Wimplicit- function-declaration]" complaints from my semantic analyzer, but it's because it didn't pick up the -DHAVE_CONFIG_H. So I think that can be ignored indeed. Apologies for the noise. Heiko ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
[Openvpn-devel] [PATCH] include unistd.h for _exit(2) declaration
Signed-off-by: Heiko Hund --- src/openvpn/error.h | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/openvpn/error.h b/src/openvpn/error.h index 972619fe..76308560 100644 --- a/src/openvpn/error.h +++ b/src/openvpn/error.h @@ -33,6 +33,8 @@ #if _WIN32 #include +#else +#include #endif /* #define ABORT_ON_ERROR */ -- 2.32.0 ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
[Openvpn-devel] [PATCH 3/4] rename foreign_option() and move it up
Add setenv_ prefix to foreign_option funtion so it is more obvious what
it does. Move it further up within options.c, so it is defined before
all future callers. Also declare all argv strings const.
Signed-off-by: Heiko Hund
---
src/openvpn/options.c | 78 +--
1 file changed, 39 insertions(+), 39 deletions(-)
diff --git a/src/openvpn/options.c b/src/openvpn/options.c
index bc3fd6a2..9a0634a5 100644
--- a/src/openvpn/options.c
+++ b/src/openvpn/options.c
@@ -1013,6 +1013,44 @@ setenv_settings(struct env_set *es, const struct options
*o)
}
}
+static void
+setenv_foreign_option(struct options *o, const char *argv[], int len, struct
env_set *es)
+{
+if (len > 0)
+{
+struct gc_arena gc = gc_new();
+struct buffer name = alloc_buf_gc(OPTION_PARM_SIZE, );
+struct buffer value = alloc_buf_gc(OPTION_PARM_SIZE, );
+int i;
+bool first = true;
+bool good = true;
+
+good &= buf_printf(, "foreign_option_%d", o->foreign_option_index
+ 1);
+++o->foreign_option_index;
+for (i = 0; i < len; ++i)
+{
+if (argv[i])
+{
+if (!first)
+{
+good &= buf_printf(, " ");
+}
+good &= buf_printf(, "%s", argv[i]);
+first = false;
+}
+}
+if (good)
+{
+setenv_str(es, BSTR(), BSTR());
+}
+else
+{
+msg(M_WARN, "foreign_option: name/value overflow");
+}
+gc_free();
+}
+}
+
static in_addr_t
get_ip_addr(const char *ip_string, int msglevel, bool *error)
{
@@ -4378,44 +4416,6 @@ options_string_extract_option(const char
*options_string, const char *opt_name,
return ret;
}
-static void
-foreign_option(struct options *o, char *argv[], int len, struct env_set *es)
-{
-if (len > 0)
-{
-struct gc_arena gc = gc_new();
-struct buffer name = alloc_buf_gc(OPTION_PARM_SIZE, );
-struct buffer value = alloc_buf_gc(OPTION_PARM_SIZE, );
-int i;
-bool first = true;
-bool good = true;
-
-good &= buf_printf(, "foreign_option_%d", o->foreign_option_index
+ 1);
-++o->foreign_option_index;
-for (i = 0; i < len; ++i)
-{
-if (argv[i])
-{
-if (!first)
-{
-good &= buf_printf(, " ");
-}
-good &= buf_printf(, "%s", argv[i]);
-first = false;
-}
-}
-if (good)
-{
-setenv_str(es, BSTR(), BSTR());
-}
-else
-{
-msg(M_WARN, "foreign_option: name/value overflow");
-}
-gc_free();
-}
-}
-
#ifdef _WIN32
/**
* Parses --windows-driver config option
@@ -8014,7 +8014,7 @@ add_option(struct options *options,
else if (streq(p[0], "dhcp-option") && p[1] && !p[3])
{
VERIFY_PERMISSION(OPT_P_IPWIN32);
-foreign_option(options, p, 3, es);
+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 */
{
--
2.32.0
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel
[Openvpn-devel] put --dns options into env as foreign_options as well
As discussed in this week's community meeting, here is the patchset to implement foreign_option emulation for --dns options. For more info please consult the individual commit messages. ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
[Openvpn-devel] [PATCH 1/4] remove foreign_option() call for IPv6 DNS servers
The call survived since the initial commit 94bfc256d, where it was added
as a fallback, since no IPv6 DNS server handling was implemented at the
time. Now there's dhcp_option_dns6_parse() which adds the servers to the
tuntap options, just like how it is done with the v4 servers.
Signed-off-by: Heiko Hund
---
src/openvpn/options.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/src/openvpn/options.c b/src/openvpn/options.c
index 20cc849d..65f4d889 100644
--- a/src/openvpn/options.c
+++ b/src/openvpn/options.c
@@ -7825,7 +7825,6 @@ add_option(struct options *options,
if (strstr(p[2], ":"))
{
ipv6dns = true;
-foreign_option(options, p, 3, es);
dhcp_option_dns6_parse(p[2], o->dns6, >dns6_len, msglevel);
}
else
--
2.32.0
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel
[Openvpn-devel] [PATCH 2/4] remove dead foreign-option parsing code
Signed-off-by: Heiko Hund
---
src/openvpn/options.c | 7 ---
1 file changed, 7 deletions(-)
diff --git a/src/openvpn/options.c b/src/openvpn/options.c
index 65f4d889..bc3fd6a2 100644
--- a/src/openvpn/options.c
+++ b/src/openvpn/options.c
@@ -5531,13 +5531,6 @@ add_option(struct options *options,
print_default_gateway(M_INFO, , );
openvpn_exit(OPENVPN_EXIT_STATUS_GOOD); /* exit point */
}
-#endif
-#if 0
-else if (streq(p[0], "foreign-option") && p[1])
-{
-VERIFY_PERMISSION(OPT_P_IPWIN32);
-foreign_option(options, p, 3, es);
-}
#endif
else if (streq(p[0], "echo") || streq(p[0], "parameter"))
{
--
2.32.0
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel
[Openvpn-devel] [PATCH 4/4] dns: also (re)place foreign dhcp options in env
Override DNS related foreign_options with values set by the --dns
option. This is done, so that scripts looking for these options continue
to work if only --dns option were pushed, or the values in the
--dhcp-options differ fron what's pushed in --dns.
Signed-off-by: Heiko Hund
---
src/openvpn/openvpn.c | 2 +-
src/openvpn/options.c | 88 ---
src/openvpn/options.h | 2 +-
3 files changed, 85 insertions(+), 7 deletions(-)
diff --git a/src/openvpn/openvpn.c b/src/openvpn/openvpn.c
index a6389fed..15e21452 100644
--- a/src/openvpn/openvpn.c
+++ b/src/openvpn/openvpn.c
@@ -248,7 +248,7 @@ openvpn_main(int argc, char *argv[])
}
/* sanity check on options */
-options_postprocess();
+options_postprocess(, c.es);
/* show all option settings */
show_settings();
diff --git a/src/openvpn/options.c b/src/openvpn/options.c
index 9a0634a5..750444fe 100644
--- a/src/openvpn/options.c
+++ b/src/openvpn/options.c
@@ -1381,6 +1381,80 @@ tuntap_options_copy_dns(struct options *o)
}
}
}
+#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];
+openvpn_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, , 1, es);
+}
+}
+
+struct gc_arena gc = gc_new();
+
+while (server)
+{
+if (server->addr4_defined)
+{
+const char *argv[] = {
+"dhcp-option",
+"DNS",
+print_in_addr_t(server->addr4.s_addr, 0, )
+};
+setenv_foreign_option(o, argv, 3, es);
+}
+if (server->addr6_defined)
+{
+const char *argv[] = {
+"dhcp-option",
+"DNS6",
+print_in6_addr(server->addr6, 0, )
+};
+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();
+
+/* remove old leftover entries */
+while (o->foreign_option_index < opt_max)
+{
+char name[32];
+openvpn_snprintf(name, sizeof(name), "foreign_option_%d", opt_max--);
+setenv_del(es, name);
+}
+}
#endif /* if defined(_WIN32) || defined(TARGET_ANDROID) */
#ifndef ENABLE_SMALL
@@ -3368,7 +3442,7 @@ options_set_backwards_compatible_options(struct options
*o)
}
static void
-options_postprocess_mutate(struct options *o)
+options_postprocess_mutate(struct options *o, struct env_set *es)
{
int i;
/*
@@ -3462,12 +3536,14 @@ options_postprocess_mutate(struct options *o)
{
dns_options_preprocess_pull(>dns_options);
}
-#if defined(_WIN32) || defined(TARGET_ANDROID)
else
{
+#if defined(_WIN32) || defined(TARGET_ANDROID)
tuntap_options_copy_dns(o);
-}
+#else
+foreign_options_copy_dns(o, es);
#endif
+}
pre_connect_save(o);
}
@@ -3803,9 +3879,9 @@ options_postprocess_filechecks(struct options *options)
* options.
*/
void
-options_postprocess(struct options *options)
+options_postprocess(struct options *options, struct env_set *es)
{
-options_postprocess_mutate(options);
+options_postprocess_mutate(options, es);
options_postprocess_verify(options);
#ifndef ENABLE_SMALL
options_postprocess_filechecks(options);
@@ -3826,6 +3902,8 @@ options_postprocess_pull(struct options *o, struct
env_set *es)
setenv_dns_options(>dns_options, es);
#if defined(_WIN32) || defined(TARGET_ANDROID)
tuntap_options_copy_dns(o);
+#else
+foreign_options_copy_dns(o, es);
#endif
}
return success;
diff --git a/src/openvpn/options.h b/src/openvpn/options.h
index c2937dc3..0e50c19e 100644
--- a/src/openvpn/options.h
+++ b/src/openvpn/o
Re: [Openvpn-devel] [PATCH] pre-commit: uncrustify based on staged changes
Hi On Mittwoch, 18. Mai 2022 15:35:58 CEST Antonio Quartulli wrote: > > -# does not match any of the extensions specified in $FILE_EXTS > > +# does not match the extensions .c or .h > > is this unrelated? Yes, it is. The original script (collection) has a config file where $FILE_EXTS is defined. However, we hardcode this to C and header files only. Simply and oversight from the initial version drive-by fix. > > -# escape special characters in the source filename: > > -# - '\': backslash needs to be escaped > > -# - '*': used as matching string => '*' would mean expansion > > -#(curiously, '?' must not be escaped) > > -# - '[': used as matching string => '[' would mean start of set > > -# - '|': used as sed split char instead of '/', so it needs to be > > escaped -#in the filename > > -# printf %s particularly important if the filename contains the % > > character -file_escaped_source=$(printf "%s" "$file" | sed -e > > 's/[\*[|]/\\&/g') - > > this seems unrelated too? See below. > > -#--- $file timestamp > > -#+++ - timestamp > > +#--- - timestamp > > +#+++ $tmpout timestamp > > same? See below. > > +git show ":$file" | diff -u -- - "$tmpout" | \ > > +sed -e "1s|--- -|--- \"b/$file_escaped_target\"|" -e "2s|+++ > > $tmpout|+++ \"a/$file_escaped_target\"|" >> "$patch"> Since the patch is now done differently $file_escaped_source is no longer needed. Instead we use $tmpout (the uncrustify output), which has a well defined (no need to being escaped) filename. The file contents we diff from stdin has changed as well. It is no longer the uncrustify output, but instead the file contents from the index (staged), thus the --- and +++ lines, we need to fix, switched as well. Hope that explains it. Heiko ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
[Openvpn-devel] [PATCH] pre-commit: uncrustify based on staged changes
Previously the generated patch was based on the file(s) in the working directory. This is a problem if you have not to be commited changes there and these changes fix formatting issues that exist in the staging area. This effectively circumventes the script from rejecting the commit. An example: git add file.c git commit ... pre-commit hooks complains about formatting ... ... you fix the file manually, forget to git add ... git commit ... succeeds, even though the commit still has issues ... Signed-off-by: Heiko Hund --- dev-tools/git-pre-commit-uncrustify.sh | 25 + 1 file changed, 9 insertions(+), 16 deletions(-) diff --git a/dev-tools/git-pre-commit-uncrustify.sh b/dev-tools/git-pre-commit-uncrustify.sh index a5cf3d41..9851c212 100755 --- a/dev-tools/git-pre-commit-uncrustify.sh +++ b/dev-tools/git-pre-commit-uncrustify.sh @@ -97,6 +97,7 @@ fi # create a filename to store our generated patch patch=$(mktemp /tmp/ovpn-fmt-XX) +tmpout=$(mktemp /tmp/uncrustify-XX) # create one patch containing all changes to the files # sed to remove quotes around the filename, if inserted by the system @@ -106,21 +107,11 @@ sed -e 's/^"\(.*\)"$/\1/' | \ while read file do # ignore file if we do check for file extensions and the file -# does not match any of the extensions specified in $FILE_EXTS +# does not match the extensions .c or .h if ! matches_extension "$file"; then continue; fi -# escape special characters in the source filename: -# - '\': backslash needs to be escaped -# - '*': used as matching string => '*' would mean expansion -#(curiously, '?' must not be escaped) -# - '[': used as matching string => '[' would mean start of set -# - '|': used as sed split char instead of '/', so it needs to be escaped -#in the filename -# printf %s particularly important if the filename contains the % character -file_escaped_source=$(printf "%s" "$file" | sed -e 's/[\*[|]/\\&/g') - # escape special characters in the target filename: # phase 1 (characters escaped in the output diff): # - '\': backslash needs to be escaped in the output diff @@ -136,15 +127,17 @@ do # uncrustify our sourcefile, create a patch with diff and append it to our $patch # The sed call is necessary to transform the patch from -#--- $file timestamp -#+++ - timestamp +#--- - timestamp +#+++ $tmpout timestamp # to both lines working on the same file and having a a/ and b/ prefix. # Else it can not be applied with 'git apply'. -"$UNCRUSTIFY" -q -c "$UNCRUST_CONFIG" -f "$file" | \ -diff -u -- "$file" - | \ -sed -e "1s|--- $file_escaped_source|--- \"a/$file_escaped_target\"|" -e "2s|+++ -|+++ \"b/$file_escaped_target\"|" >> "$patch" +git show ":$file" | "$UNCRUSTIFY" -q -l C -c "$UNCRUST_CONFIG" -o "$tmpout" +git show ":$file" | diff -u -- - "$tmpout" | \ +sed -e "1s|--- -|--- \"b/$file_escaped_target\"|" -e "2s|+++ $tmpout|+++ \"a/$file_escaped_target\"|" >> "$patch" done +rm -f "$tmpout" + # if no patch has been generated all is ok, clean up the file stub and exit if [ ! -s "$patch" ] ; then rm -f "$patch" -- 2.32.0 ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
Re: [Openvpn-devel] [PATCH] doc: fix literal block in tls-options.rst
On Freitag, 13. Mai 2022 10:55:32 CEST Arne Schwabe wrote: > Am 11.05.22 um 14:10 schrieb Heiko Hund: > > + Valid syntax:: > > I don't understand this to be honest. I don't have a good about rst but > all other instances of Valid syntax use the form like what was before > the patch, e.g. > >Valid syntax: > > peer-fingerprint AD:B0:95:D8:09:... > > So why exactly does this one need to be different? It doesn't need to. :: on an end of line are replaced by a single : and serve the same formatting purpose as :: on a separate line followed by a blank line: https://docutils.sourceforge.io/docs/user/rst/quickref.html#literal-blocks Thought it is easier on the eye here, because it is possible. The actual problem is that a blank line after the :: is missing. Let me know if I should send a version with :: separate. Cheers Heiko ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
[Openvpn-devel] [PATCH v2] signal --dns support in peer info
Have clients set a bit in IV_PROTO, so that servers can make an informed decision on whether to push --dns to the client. While unknown options are ignored by clients when pushed, they generate a warning in the log. That can be circumvented by server backends by checking if bit 7 is set. Signed-off-by: Heiko Hund --- src/openvpn/ssl.c | 3 +++ src/openvpn/ssl.h | 3 +++ 2 files changed, 6 insertions(+) diff --git a/src/openvpn/ssl.c b/src/openvpn/ssl.c index 61dea996..24d7f3f4 100644 --- a/src/openvpn/ssl.c +++ b/src/openvpn/ssl.c @@ -1940,6 +1940,9 @@ push_peer_info(struct buffer *buf, struct tls_session *session) /* support for P_DATA_V2 */ int iv_proto = IV_PROTO_DATA_V2; +/* support for the --dns option */ +iv_proto |= IV_PROTO_DNS_OPTION; + /* support for receiving push_reply before sending * push request, also signal that the client wants * to get push-reply messages without without requiring a round diff --git a/src/openvpn/ssl.h b/src/openvpn/ssl.h index 0ba86d3e..c8802707 100644 --- a/src/openvpn/ssl.h +++ b/src/openvpn/ssl.h @@ -93,6 +93,9 @@ * result. */ #define IV_PROTO_NCP_P2P (1<<5) +/** Supports the --dns option introduced in version 2.6 */ +#define IV_PROTO_DNS_OPTION (1<<6) + /* Default field in X509 to be username */ #define X509_USERNAME_FIELD_DEFAULT "CN" -- 2.32.0 ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
Re: [Openvpn-devel] [PATCH] signal --dns support in peer info
On Freitag, 13. Mai 2022 09:17:49 CEST Arne Schwabe wrote: > Am 13.05.2022 um 09:14 schrieb Arne Schwabe: > > Am 13.05.2022 um 01:11 schrieb Heiko Hund: > >> Have clients set a bit in IV_PROTO, so that servers can make an informed > >> decision on whether to push --dns to the client. While unknown options > >> are ignored by clients when pushed, they generate a warning in the log. > >> That can be circumvented by server backends by checking if bit 7 is set. > > > > Acked-By: Arne Schwabe > > I retract my ACK. David noticed that the & in the patch should be an | > and I didn't look closely enough. Quite stupid, v2 incoming. ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
[Openvpn-devel] [PATCH] make %x destination unsigned
The %x specifier requires for the argument to be an unsigned int.
Signed-off-by: Heiko Hund
---
src/openvpn/options.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/openvpn/options.c b/src/openvpn/options.c
index 9ff384d0..3dbd3fab 100644
--- a/src/openvpn/options.c
+++ b/src/openvpn/options.c
@@ -1104,7 +1104,7 @@ parse_hash_fingerprint(const char *str, int nbytes, int
msglevel, struct gc_aren
ALLOC_OBJ_CLEAR_GC(ret, struct verify_hash_list, gc);
char term = 0;
-int byte;
+unsigned int byte;
while (*cp && i < nbytes)
{
--
2.32.0
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel
[Openvpn-devel] [PATCH] signal --dns support in peer info
Have clients set a bit in IV_PROTO, so that servers can make an informed
decision on whether to push --dns to the client. While unknown options
are ignored by clients when pushed, they generate a warning in the log.
That can be circumvented by server backends by checking if bit 7 is set.
Signed-off-by: Heiko Hund
---
src/openvpn/ssl.c | 4 ++--
src/openvpn/ssl.h | 3 +++
2 files changed, 5 insertions(+), 2 deletions(-)
diff --git a/src/openvpn/ssl.c b/src/openvpn/ssl.c
index 61dea996..12f51150 100644
--- a/src/openvpn/ssl.c
+++ b/src/openvpn/ssl.c
@@ -1937,8 +1937,8 @@ push_peer_info(struct buffer *buf, struct tls_session
*session)
/* These are the IV variable that are sent to peers in p2p mode */
if (session->opt->push_peer_info_detail > 0)
{
-/* support for P_DATA_V2 */
-int iv_proto = IV_PROTO_DATA_V2;
+/* support for P_DATA_V2 and the --dns option */
+int iv_proto = IV_PROTO_DATA_V2 & IV_PROTO_DNS_OPTION;
/* support for receiving push_reply before sending
* push request, also signal that the client wants
diff --git a/src/openvpn/ssl.h b/src/openvpn/ssl.h
index 0ba86d3e..c8802707 100644
--- a/src/openvpn/ssl.h
+++ b/src/openvpn/ssl.h
@@ -93,6 +93,9 @@
* result. */
#define IV_PROTO_NCP_P2P (1<<5)
+/** Supports the --dns option introduced in version 2.6 */
+#define IV_PROTO_DNS_OPTION (1<<6)
+
/* Default field in X509 to be username */
#define X509_USERNAME_FIELD_DEFAULT "CN"
--
2.32.0
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel
[Openvpn-devel] [PATCH] doc: fix literal block in tls-options.rst
Signed-off-by: Heiko Hund --- doc/man-sections/tls-options.rst | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/doc/man-sections/tls-options.rst b/doc/man-sections/tls-options.rst index c06ee335..d51aff77 100644 --- a/doc/man-sections/tls-options.rst +++ b/doc/man-sections/tls-options.rst @@ -487,8 +487,8 @@ certificates and keys: https://github.com/OpenVPN/easy-rsa --tls-crypt-v2 keyfile - Valid syntax: - :: + Valid syntax:: + tls-crypt-v2 keyfile tls-crypt-v2 keyfile force-cookie tls-crypt-v2 keyfile allow-noncookie -- 2.32.0 ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
[Openvpn-devel] [PATCH v3] Add git pre-commit hook script to uncrustify
The script is self installing if you call it with "install" as the first
parameter. Once installed as the pre-commit hook it will check files to
be committed according to the rules in uncrustify.conf and abort the
commit if there's formatting issues. The script produces a patch in /tmp
which can be git apply'ed to fix all issues found.
The script was originally authored by David Martin [1] and slightly
modified to fit our needs. At the time it had a 2-clause BSD license.
[1] https://github.com/avidmartin/Pre-commit-hooks
Signed-off-by: Heiko Hund
---
CONTRIBUTING.rst | 8 ++
dev-tools/git-pre-commit-uncrustify.sh | 168 +
2 files changed, 176 insertions(+)
create mode 100755 dev-tools/git-pre-commit-uncrustify.sh
diff --git a/CONTRIBUTING.rst b/CONTRIBUTING.rst
index db47f397..a848f899 100644
--- a/CONTRIBUTING.rst
+++ b/CONTRIBUTING.rst
@@ -18,6 +18,14 @@ When sending patches to "openvpn-devel" the subject line
should be prefixed with
git-format-patch or sent using git-send-email. Try to split large patches into
small, atomic pieces to make reviews easier.
+Please make sure that the source code formatting follows the guidelines at
+https://community.openvpn.net/openvpn/wiki/CodeStyle. Automated checking can be
+done with uncrustify (http://uncrustify.sourceforge.net/) and the configuration
+file which can be found in the git repository at dev-tools/uncrustify.conf.
+There is also a git pre-commit hook script, which runs uncrustify automatically
+each time you commit and lets you format your code conveniently, if needed.
+To install the hook simply run: dev-tools/git-pre-commit-uncrustify.sh install
+
If you want quick feedback on a patch before sending it to openvpn-devel
mailing
list, you can visit the #openvpn-devel channel on irc.libera.chat. Note that
you need to be logged in to Libera to join the channel:
diff --git a/dev-tools/git-pre-commit-uncrustify.sh
b/dev-tools/git-pre-commit-uncrustify.sh
new file mode 100755
index ..a5cf3d41
--- /dev/null
+++ b/dev-tools/git-pre-commit-uncrustify.sh
@@ -0,0 +1,168 @@
+#!/bin/sh
+
+# Copyright (c) 2015, David Martin
+# 2022, Heiko Hund
+# All rights reserved.
+#
+# Redistribution and use in source and binary forms, with or without
+# modification, are permitted provided that the following conditions are met:
+#
+# * Redistributions of source code must retain the above copyright notice, this
+# list of conditions and the following disclaimer.
+#
+# * Redistributions in binary form must reproduce the above copyright notice,
+# this list of conditions and the following disclaimer in the documentation
+# and/or other materials provided with the distribution.
+#
+# THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS"
+# AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
+# IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
ARE
+# DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDER OR CONTRIBUTORS BE LIABLE
+# FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL
+# DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR
+# SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER
+# CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY,
+# OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+# OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+
+
+# git pre-commit hook that runs an Uncrustify stylecheck.
+# Features:
+# - abort commit when commit does not comply with the style guidelines
+# - create a patch of the proposed style changes
+#
+# More info on Uncrustify: http://uncrustify.sourceforge.net/
+
+# This file was taken from a set of unofficial pre-commit hooks available
+# at https://github.com/avidmartin/Pre-commit-hooks and modified to
+# fit the openvpn project's needs
+
+# exit on error
+set -e
+
+
+# If called so, install this script as pre-commit hook
+if [ "$1" = "install" ] ; then
+TARGET="$(git rev-parse --git-path hooks)/pre-commit"
+
+if [ -e "$TARGET" ] ; then
+printf "$TARGET file exists. Won't overwrite.\n"
+printf "Aborting installation.\n"
+exit 1
+fi
+
+read -p "Install as $TARGET? [y/N] " INPUT
+[ "$INPUT" = "y" ] || exit 0
+cp "$0" "$TARGET"
+chmod +x $TARGET
+exit 0
+fi
+
+# check whether the given file matches any of the set extensions
+matches_extension() {
+local filename="$(basename -- "$1")"
+local extension=".${filename##*.}"
+local ext
+
+for ext in .c .h ; do [ "$ext" = "$extension" ] && return 0; done
+
+return 1
+}
+
+# necessary check for initial commit
+if git r
Re: [Openvpn-devel] [PATCH v2] Add git pre-commit hook script to uncrustify
On Donnerstag, 21. April 2022 16:20:14 CEST Frank Lichtenheld wrote: > > Heiko Hund hat am 21.04.2022 15:58 geschrieben: > [...] > > > +ROOTDIR=$(git rev-parse --show-toplevel) > > + > > +# If called so, install this script as pre-commit hook > > +if [ "$1" = "install" ] ; then > > +HOOKSDIR="$ROOTDIR/$(git rev-parse --git-path hooks)" > > Hmm, no. While in a default install $(git rev-parse --git-path hooks) will > be a relative path, there is not guarantee that it is (e.g. if > core.hooksPath is an absolute path). I see not reason why you need an > absolute path here. A relative path should be completely fine? I thought it spits out a path relative to the git root, but it turns out to be relative to the current dir, so you're right. Will post a v3, but will wait for more feedback before I send it. Heiko ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
[Openvpn-devel] [PATCH v2] Add git pre-commit hook script to uncrustify
The script is self installing if you call it with "install" as the first
parameter. Once installed as the pre-commit hook it will check files to
be committed according to the rules in uncrustify.conf and abort the
commit if there's formatting issues. The script produces a patch in /tmp
which can be git apply'ed to fix all issues found.
The script was originally authored by David Martin [1] and slightly
modified to fit our needs. At the time it had a 2-clause BSD license.
[1] https://github.com/avidmartin/Pre-commit-hooks
Signed-off-by: Heiko Hund
---
CONTRIBUTING.rst | 7 +
dev-tools/git-pre-commit-uncrustify.sh | 171 +
2 files changed, 178 insertions(+)
create mode 100755 dev-tools/git-pre-commit-uncrustify.sh
diff --git a/CONTRIBUTING.rst b/CONTRIBUTING.rst
index db47f397..707fa539 100644
--- a/CONTRIBUTING.rst
+++ b/CONTRIBUTING.rst
@@ -18,6 +18,13 @@ When sending patches to "openvpn-devel" the subject line
should be prefixed with
git-format-patch or sent using git-send-email. Try to split large patches into
small, atomic pieces to make reviews easier.
+Please make sure that the source code formatting follows the guidelines. While
+there are no human readable guidelines, a configuration file for uncrustify
+(http://uncrustify.sourceforge.net/) can be found at dev-tools/uncrustify.conf.
+There is also a git pre-commit hook script, which runs uncrustify automatically
+and lets you reformat your code conveniently, when needed.
+To install the hook simply run: dev-tools/git-pre-commit-uncrustify.sh install
+
If you want quick feedback on a patch before sending it to openvpn-devel
mailing
list, you can visit the #openvpn-devel channel on irc.libera.chat. Note that
you need to be logged in to Libera to join the channel:
diff --git a/dev-tools/git-pre-commit-uncrustify.sh
b/dev-tools/git-pre-commit-uncrustify.sh
new file mode 100755
index ..e5b274e6
--- /dev/null
+++ b/dev-tools/git-pre-commit-uncrustify.sh
@@ -0,0 +1,171 @@
+#!/bin/sh
+
+# Copyright (c) 2015, David Martin
+# 2022, Heiko Hund
+# All rights reserved.
+#
+# Redistribution and use in source and binary forms, with or without
+# modification, are permitted provided that the following conditions are met:
+#
+# * Redistributions of source code must retain the above copyright notice, this
+# list of conditions and the following disclaimer.
+#
+# * Redistributions in binary form must reproduce the above copyright notice,
+# this list of conditions and the following disclaimer in the documentation
+# and/or other materials provided with the distribution.
+#
+# THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS"
+# AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
+# IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
ARE
+# DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDER OR CONTRIBUTORS BE LIABLE
+# FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL
+# DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR
+# SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER
+# CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY,
+# OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+# OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+
+
+# git pre-commit hook that runs an Uncrustify stylecheck.
+# Features:
+# - abort commit when commit does not comply with the style guidelines
+# - create a patch of the proposed style changes
+#
+# More info on Uncrustify: http://uncrustify.sourceforge.net/
+
+# This file was taken from a set of unofficial pre-commit hooks available
+# at https://github.com/avidmartin/Pre-commit-hooks and modified to
+# fit the openvpn project's needs
+
+# exit on error
+set -e
+
+
+ROOTDIR=$(git rev-parse --show-toplevel)
+
+# If called so, install this script as pre-commit hook
+if [ "$1" = "install" ] ; then
+HOOKSDIR="$ROOTDIR/$(git rev-parse --git-path hooks)"
+TARGET="$HOOKSDIR/pre-commit"
+
+if [ -e "$TARGET" ] ; then
+printf "$TARGET file exists. Won't overwrite.\n"
+printf "Aborting installation.\n"
+exit 1
+fi
+
+read -p "Install as $TARGET? [y/N] " INPUT
+[ "$INPUT" = "y" ] || exit 0
+cp "$0" "$TARGET"
+chmod +x $TARGET
+exit 0
+fi
+
+# check whether the given file matches any of the set extensions
+matches_extension() {
+local filename="$(basename -- "$1")"
+local extension=".${filename##*.}"
+local ext
+
+for ext in .c .h ; do [ "$ext" = "$extension" ] && return 0; done
+
+return 1
+}
+
+# necessary check for initial commit
+if git rev-
Re: [Openvpn-devel] [PATCH] Add git pre-commit hook script to uncrustify
Hi Frank On Donnerstag, 21. April 2022 10:01:29 CEST Frank Lichtenheld wrote: > > +# If called so, install this script as pre-commit hook > > +if [ "$1" = "install" ] ; then > > +ROOTDIR=$(git rev-parse --show-toplevel) > > +HOOKSDIR="$ROOTDIR/.git/hooks" > > Actually, the correct way to find the actual hooks directory is > git rev-parse --git-path hooks > That takes all configuration like $GIT_DIR and core.hooksDir into account. Nice one, will add in v2. > > +TARGET="$HOOKSDIR/pre-commit" > > + > > +if [ -e "$TARGET" ] ; then > > +printf "$TARGET file exists. Won't overwrite.\n" > > Any reason we're using printf here instead of echo? The latter > would avoid the ugly line endings. Consistency. The script uses printf everywhere. Heiko ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
Re: [Openvpn-devel] [PATCH] Add git pre-commit hook script to uncrustify
Hi David On Donnerstag, 21. April 2022 13:41:58 CEST David Schneider wrote: > Did you consider to use the pre-commit framework [1] written in > Python? There is a maintained hook for uncrustify [2]. > This would allow it to easily integrate other linters/checks. See the > list of supported hooks [3]. I stumbled across this, but it seemed too big for the purpose. Was looking for something small that could integrate into the dev-tools. That said, I don't think it holds anyone back from using the pre-commit stuff with the repo's uncrustify.conf. So, if you already do use it please feel free to add some short howto to the developer documentation (something I missed doing, but will add in v2). Heiko ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
Re: [Openvpn-devel] [PATCH] Add git pre-commit hook script to uncrustify
Hi On Donnerstag, 21. April 2022 10:16:03 CEST Gert Doering wrote: > On Thu, Apr 21, 2022 at 10:01:29AM +0200, Frank Lichtenheld wrote: > > Definitive NACK due to licensing concern mentioned below. > > Streams crossed here, but I saw your NAK before pushing. > So, yes, all valid concerns and I withdraw my ACK. Will post a v2 with the license inline . > > > +# create a filename to store our generated patch > > > +prefix="ovpn-fmt" > > > +suffix="$(date +%C%y%m%d%H%M%S)" > > > +patch="/tmp/$prefix-$suffix.patch" > > > > Are insecure tmp files still a concern? Because this is definitely an > > insecure tmp file... > Especially this one is something I only noticed after I sent my mail - > so this definitely needs to to mktemp-style temp files, or a private > tmp directory (in the local repo?) I don't really see the need for secure tmp files here, as there's no chance to exploit anything by racing for the file(name). Or am I just not creative enough? Cheers, Heiko ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
[Openvpn-devel] [PATCH] Add git pre-commit hook script to uncrustify
The script is self installing if you call it with "install" as the first
parameter. Once installed as the pre-commit hook it will check files to
be committed according to the rules in uncrustify.conf and abort the
commit if there's formatting issues. The script produces a patch in /tmp
which can be git apply'ed to fix all issues found.
The script was originally authored by David Martin [1] and slightly
modified to fit our needs. At the time it had a 2-clause BSD license.
[1] https://github.com/avidmartin/Pre-commit-hooks
Signed-off-by: Heiko Hund
---
dev-tools/git-pre-commit-uncrustify.sh | 146 +
1 file changed, 146 insertions(+)
create mode 100644 dev-tools/git-pre-commit-uncrustify.sh
diff --git a/dev-tools/git-pre-commit-uncrustify.sh
b/dev-tools/git-pre-commit-uncrustify.sh
new file mode 100644
index ..64e5e396
--- /dev/null
+++ b/dev-tools/git-pre-commit-uncrustify.sh
@@ -0,0 +1,146 @@
+#!/bin/sh
+
+# git pre-commit hook that runs an Uncrustify stylecheck.
+# Features:
+# - abort commit when commit does not comply with the style guidelines
+# - create a patch of the proposed style changes
+#
+# More info on Uncrustify: http://uncrustify.sourceforge.net/
+
+# This file was taken from a set of unofficial pre-commit hooks available
+# at https://github.com/avidmartin/Pre-commit-hooks and modified to
+# fit the openvpn project's needs
+
+# exit on error
+set -e
+
+
+# If called so, install this script as pre-commit hook
+if [ "$1" = "install" ] ; then
+ROOTDIR=$(git rev-parse --show-toplevel)
+HOOKSDIR="$ROOTDIR/.git/hooks"
+TARGET="$HOOKSDIR/pre-commit"
+
+if [ -e "$TARGET" ] ; then
+printf "$TARGET file exists. Won't overwrite.\n"
+printf "Aborting installation.\n"
+exit 1
+fi
+
+read -p "Install as $TARGET? [y/N] " INPUT
+[ "$INPUT" = "y" ] || exit 0
+cp "$0" "$TARGET"
+chmod +x $TARGET
+exit 0
+fi
+
+# check whether the given file matches any of the set extensions
+matches_extension() {
+local filename="$(basename -- "$1")"
+local extension=".${filename##*.}"
+local ext
+
+for ext in .c .h ; do [ "$ext" = "$extension" ] && return 0; done
+
+return 1
+}
+
+# necessary check for initial commit
+if git rev-parse --verify HEAD >/dev/null 2>&1 ; then
+against=HEAD
+else
+# Initial commit: diff against an empty tree object
+against=4b825dc642cb6eb9a060e54bf8d69288fbee4904
+fi
+
+UNCRUSTIFY=$(command -v uncrustify)
+UNCRUST_CONFIG="$PWD/dev-tools/uncrustify.conf"
+
+# make sure the config file and executable are correctly set
+if [ ! -f "$UNCRUST_CONFIG" ] ; then
+printf "Error: uncrustify config file not found.\n"
+printf "Expected to find it at $UNCRUST_CONFIG.\n"
+printf "Aborting commit.\n"
+exit 1
+fi
+
+if [ -z "$UNCRUSTIFY" ] ; then
+printf "Error: uncrustify executable not found.\n"
+printf "Is it installed and in your \$PATH?\n"
+printf "Aborting commit.\n"
+exit 1
+fi
+
+# create a filename to store our generated patch
+prefix="ovpn-fmt"
+suffix="$(date +%C%y%m%d%H%M%S)"
+patch="/tmp/$prefix-$suffix.patch"
+
+# create one patch containing all changes to the files
+# sed to remove quotes around the filename, if inserted by the system
+# (done sometimes, if the filename contains special characters, like the quote
itself)
+git diff-index --cached --diff-filter=ACMR --name-only $against -- | \
+sed -e 's/^"\(.*\)"$/\1/' | \
+while read file
+do
+# ignore file if we do check for file extensions and the file
+# does not match any of the extensions specified in $FILE_EXTS
+if ! matches_extension "$file"; then
+continue;
+fi
+
+# escape special characters in the source filename:
+# - '\': backslash needs to be escaped
+# - '*': used as matching string => '*' would mean expansion
+#(curiously, '?' must not be escaped)
+# - '[': used as matching string => '[' would mean start of set
+# - '|': used as sed split char instead of '/', so it needs to be escaped
+#in the filename
+# printf %s particularly important if the filename contains the % character
+file_escaped_source=$(printf "%s" "$file" | sed -e 's/[\*[|]/\\&/g')
+
+# escape special characters in the target filename:
+# phase 1 (characters escaped in the output diff):
+# - '\': backslash needs to be escaped in the output diff
+# - '"': quote needs to be escaped in the output diff if present inside
+#of the filename, as it used to bracket the entire filename
part
+# phase 2 (characters escap
Re: [Openvpn-devel] [PATCH v3] add support for --dns option
On Mittwoch, 23. März 2022 15:34:52 CET Heiko Hund wrote:
> +static void
> +setenv_dns_option(struct env_set *es,
> + const char *format, int i, int j,
> + const char *value)
> +{
> +char name[64];
> +bool name_ok = false;
> +
> +if (j < 0)
> +{
> +name_ok = openvpn_snprintf(name, sizeof(name), format, i);
> +}
> +else
> +{
> +name_ok = openvpn_snprintf(name, sizeof(name), format, i, j);
> +}
> +
> +if (!name_ok)
> +{
> +msg(M_WARN, "WARNING: dns option setenv name buffer overflow");
> +}
> +
> +setenv_str(es, name, value);
> +}
Here's the helper function Gert was asking for. It's somewhat special in how
the 'j' parameter is handled, but since it's local and very specialized, I can
live with that.
Regards, Heiko
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel
[Openvpn-devel] [PATCH v3] add support for --dns option
As a first step towards DNS configuration in openvpn and a unified way to push DNS related settings to clients in v2 and v3, this commit adds support for parsing the new --dns option. Later commits will add support for setting up DNS on different platforms. For now, --dns and DNS related --dhcp-option can be used together for smoother transition. Settings from --dns will override ones --dhcp-option where applicable. For detailed information about the option consult the documentation in this commit. Signed-off-by: Heiko Hund --- doc/man-sections/client-options.rst | 59 doc/man-sections/script-options.rst | 19 ++ doc/man-sections/server-options.rst | 2 +- src/openvpn/Makefile.am | 1 + src/openvpn/dns.c | 510 src/openvpn/dns.h | 164 + src/openvpn/openvpn.vcxproj | 4 +- src/openvpn/openvpn.vcxproj.filters | 8 +- src/openvpn/options.c | 221 src/openvpn/options.h | 7 + src/openvpn/push.c | 4 + src/openvpn/socket.c| 11 + src/openvpn/socket.h| 2 + 13 files changed, 1009 insertions(+), 3 deletions(-) create mode 100644 src/openvpn/dns.c create mode 100644 src/openvpn/dns.h diff --git a/doc/man-sections/client-options.rst b/doc/man-sections/client-options.rst index e53b5262..8e0e4f18 100644 --- a/doc/man-sections/client-options.rst +++ b/doc/man-sections/client-options.rst @@ -154,6 +154,65 @@ configuration. --connect-timeout n See ``--server-poll-timeout``. +--dns args + Client DNS configuration to be used with the connection. + + Valid syntaxes: + :: + + dns search-domains domain [domain ...] + dns server n address addr[:port] [addr[:port]] + dns server n resolve-domains|exclude-domains domain [domain ...] + dns server n dnssec yes|optional|no + dns server n transport DoH|DoT|plain + dns server n sni server-name + + The ``--dns search-domains`` directive takes one or more domain names + to be added as DNS domain suffixes. If it is repeated multiple times within + a configuration the domains are appended, thus e.g. domain names pushed by + a server will amend locally defined ones. + + The ``--dns server`` directive is used to configure DNS server ``n``. + The server id ``n`` must be a value between -128 and 127. For pushed + DNS server options it must be between 0 and 127. The server id is used + to group options and also for ordering the list of configured DNS servers; + lower numbers come first. DNS servers being pushed to a client replace + already configured DNS servers with the same server id. + + The ``address`` option configures the IPv4 and / or IPv6 address of + the DNS server. Optionally a port can be appended after a colon. IPv6 + addresses need to be enclosed in brackets if a port is appended. + + The ``resolve-domains`` and ``exclude-domains`` options take one or + more DNS domains which are explicitly resolved or explicitly not resolved + by a server. Only one of the options can be configured for a server. + ``resolve-domains`` is used to define a split-dns setup, where only + given domains are resolved by a server. ``exclude-domains`` is used to + define domains which will never be resolved by a server (e.g. domains + which can only be resolved locally). Systems which do not support fine + grained DNS domain configuration, will ignore these settings. + + The ``dnssec`` option is used to configure validation of DNSSEC records. + While the exact semantics may differ for resolvers on different systems, + ``yes`` likely makes validation mandatory, ``no`` disables it, and ``optional`` + uses it opportunistically. + + The ``transport`` option enables DNS-over-HTTPS (``DoH``) or DNS-over-TLS (``DoT``) + for a DNS server. The ``sni`` option can be used with them to specify the + ``server-name`` for TLS server name indication. + + Each server has to have at least one address configured for a configuration + to be valid. All the other options can be omitted. + + Note that not all options may be supported on all platforms. As soon support + for different systems is implemented, information will be added here how + unsupported options are treated. + + The ``--dns`` option will eventually obsolete the ``--dhcp-option`` directive. + Until then it will replace configuration at the places ``--dhcp-option`` puts it, + so that ``--dns`` overrides ``--dhcp-option``. Thus, ``--dns`` can be used today + to migrate from ``--dhcp-option``. + --explicit-exit-notify n In UDP client mode or point-to-point mode, send server/peer an exit notification if tunnel is restarted or OpenVPN process is exited. In diff --git a/doc/man-sections/script-options.rst b/doc/man-sections/script-options.rst index 77877a5d..6be0686d 100644 --- a/doc/man-sections/script-options.rst +++ b/doc/man-sections/script-options.rst @@ -588,6 +588,25
Re: [Openvpn-devel] [PATCH] add support for --dns option
Hi Gert
On Donnerstag, 17. März 2022 11:41:22 CET Gert Doering wrote:
> I wonder why we bother to actually *do* this? As in "we already know
> that this can never overflow here" (because all strings involved are
> known, and the max width of %d is known, too), but *if* it ever did,
> calling the subsequent setenv_str() wouldn't be a good idea to do - no?
It's in the spirit of defensive programming. It cannot overflow now, but will
likely overflow some time. However I thought that getting the value into env
might still be worthwhile in some scenarios, even if the name is not complete,
e.g. if the env is logged somewhere. The error message in the openvpn log, will
lead to cause then.
> Can we make transform these into a miniwrapper that does
> like
>
>for (i = 1, d = o->search_domains; d != NULL; i++, d = d->next)
>{
> _do_dns_env_thing( "dns_search_domain_%d", i, d->name );
>}
I actually had such a wrapper in place, but dumped it because I didn't like the
fact that the value would have to come before the name in the argument list,
because of the var-args. Since we have a format string with two %d (and there
might be more / other formatting specifiers in the future), the wrapper would
have to deal with ..., or we'd end up with two wrappers (or more if this is
extended in the future). No matter what:
>const char* format = s->domain_type == DNS_RESOLVE_DOMAINS ?
>"dns_server_%d_resolve_domain_%d" :
> "dns_server_%d_exclude_domain_%d";
requires it to be:
_do_dns_env_thing(d->name, "dns_server_%d_resolve__domain_%d", i, j);
right now already. And that lead me to not making things wrapped.
However I could be talked into removing the checks for openvpn_snprintf()'s
return value if there's a consensus, as it wouldn't introduce an actual issue.
Just maybe a little harder to debug issues in future code possibly. Also not
setting the env if sprintf fails sounds good, would do this rather:
if (openvpn_snprinf())
setenv_str();
else
name_ok = false;
Iff the value is really not useful in the env, when the name is incomplete.
Bonus: no &=.
Heiko
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel
Re: [Openvpn-devel] [PATCH] add support for --dns option
On Samstag, 12. März 2022 14:58:10 CET Heiko Hund wrote: > +name_ok = openvpn_snprintf(env_name, sizeof(env_name), > "dns_search_domain_%d", i) && name_ok; With some distance, I still like the &= version better from a readability standpoint. Even though it's a bit unclean. Heiko ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
Re: [Openvpn-devel] [PATCH] add support for --dns option
Sorry, this is [PATCH v2]. Somehow I managed for send-email to override my --subject. ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
[Openvpn-devel] [PATCH] add support for --dns option
As a first step towards DNS configuration in openvpn and a unified way to push DNS related settings to clients in v2 and v3, this commit adds support for parsing the new --dns option. Later commits will add support for setting up DNS on different platforms. For now, --dns and DNS related --dhcp-option can be used together for smoother transition. Settings from --dns will override ones --dhcp-option where applicable. For detailed information about the option consult the documentation in this commit. Signed-off-by: Heiko Hund --- doc/man-sections/client-options.rst | 59 doc/man-sections/script-options.rst | 19 ++ doc/man-sections/server-options.rst | 2 +- src/openvpn/Makefile.am | 1 + src/openvpn/dns.c | 495 src/openvpn/dns.h | 164 + src/openvpn/openvpn.vcxproj | 4 +- src/openvpn/openvpn.vcxproj.filters | 8 +- src/openvpn/options.c | 221 + src/openvpn/options.h | 7 + src/openvpn/push.c | 4 + src/openvpn/socket.c| 11 + src/openvpn/socket.h| 2 + 13 files changed, 994 insertions(+), 3 deletions(-) create mode 100644 src/openvpn/dns.c create mode 100644 src/openvpn/dns.h diff --git a/doc/man-sections/client-options.rst b/doc/man-sections/client-options.rst index 92a02e28..2d7293c6 100644 --- a/doc/man-sections/client-options.rst +++ b/doc/man-sections/client-options.rst @@ -154,6 +154,65 @@ configuration. --connect-timeout n See ``--server-poll-timeout``. +--dns args + Client DNS configuration to be used with the connection. + + Valid syntaxes: + :: + + dns search-domains domain [domain ...] + dns server n address addr[:port] [addr[:port]] + dns server n resolve-domains|exclude-domains domain [domain ...] + dns server n dnssec yes|optional|no + dns server n transport DoH|DoT|plain + dns server n sni server-name + + The ``--dns search-domains`` directive takes one or more domain names + to be added as DNS domain suffixes. If it is repeated multiple times within + a configuration the domains are appended, thus e.g. domain names pushed by + a server will amend locally defined ones. + + The ``--dns server`` directive is used to configure DNS server ``n``. + The server id ``n`` must be a value between -128 and 127. For pushed + DNS server options it must be between 0 and 127. The server id is used + to group options and also for ordering the list of configured DNS servers; + lower numbers come first. DNS servers being pushed to a client replace + already configured DNS servers with the same server id. + + The ``address`` option configures the IPv4 and / or IPv6 address of + the DNS server. Optionally a port can be appended after a colon. IPv6 + addresses need to be enclosed in brackets if a port is appended. + + The ``resolve-domains`` and ``exclude-domains`` options take one or + more DNS domains which are explicitly resolved or explicitly not resolved + by a server. Only one of the options can be configured for a server. + ``resolve-domains`` is used to define a split-dns setup, where only + given domains are resolved by a server. ``exclude-domains`` is used to + define domains which will never be resolved by a server (e.g. domains + which can only be resolved locally). Systems which do not support fine + grained DNS domain configuration, will ignore these settings. + + The ``dnssec`` option is used to configure validation of DNSSEC records. + While the exact semantics may differ for resolvers on different systems, + ``yes`` likely makes validation mandatory, ``no`` disables it, and ``optional`` + uses it opportunistically. + + The ``transport`` option enables DNS-over-HTTPS (``DoH``) or DNS-over-TLS (``DoT``) + for a DNS server. The ``sni`` option can be used with them to specify the + ``server-name`` for TLS server name indication. + + Each server has to have at least one address configured for a configuration + to be valid. All the other options can be omitted. + + Note that not all options may be supported on all platforms. As soon support + for different systems is implemented, information will be added here how + unsupported options are treated. + + The ``--dns`` option will eventually obsolete the ``--dhcp-option`` directive. + Until then it will replace configuration at the places ``--dhcp-option`` puts it, + so that ``--dns`` overrides ``--dhcp-option``. Thus, ``--dns`` can be used today + to migrate from ``--dhcp-option``. + --explicit-exit-notify n In UDP client mode or point-to-point mode, send server/peer an exit notification if tunnel is restarted or OpenVPN process is exited. In diff --git a/doc/man-sections/script-options.rst b/doc/man-sections/script-options.rst index 22990f4f..c81efe9e 100644 --- a/doc/man-sections/script-options.rst +++ b/doc/man-sections/script-options.rst @@ -586,6 +586,25
Re: [Openvpn-devel] [PATCH] add support for --dns option
On Mittwoch, 9. März 2022 13:40:32 CET Arne Schwabe wrote: > Am 09.03.22 um 00:06 schrieb Heiko Hund: > > +bool dns_server_priority_parse(long *priority, const char *str, bool > > +[...] > > +void show_dns_options(const struct dns_options *o); > > These new functions are missing doxygen comments. We generally have the > doxygen comments in the header files rather than in the c files. Okay, I'll move the comments into the header file. > > +--dns args > > + Client DNS configuration to be used with the connection. > > This man page section sounds like all options are supported on all > platforms while currently that is not the case. An explicit warning etc > that says that some options like port, dnssec, doh etc are not supported > on certain platforms should be included and also what happens if they > are not. Noted. What happens will likely be determined when we have the implementation(s) that actually take this option and try to apply it. Think it is too early to add this info to the manual, as it may change. Some ideas exist, but they have to prove themselves and survive public discussion. So far I have the idea that options will be ignored if they are not supported, and split DNS is turned into all DNS redirected to the pushed server(s) if they are unsupported. Again, this is very theoretical at this point and may change once facing reality. > > The ``--dns`` option will eventually obsolete the ``--dhcp-option`` > > directive.> > > + Until then it will add configuration at the places ``--dhcp-option`` > > puts it + and will override it if both are given. > > Is dhcp-option ignored when dns is present by newer client or are they > mixed? This is a bit vague. Will clarify. > > + long priority; > > why a type that is 32 bit on some platforms and 64 bit on others? Either > go with just int if 32 bit is enough or use long long or int64_t if we > need 64 but long is a weird type. int8_t would be enough, just need an extra few bits for the conversation from text. It is a long because strtol(3) returns a long, and I went with that. Not sure if casting after checks will gain much. If we decide to change this I'd rather use int8_t however, or a future somebody will ponder why the extra bits there, when the priority is really a 8 bit value. IMHO it is not worth the extra code, that's why I went with long. Compilers might align an 8 bit value anyways. > > +name_ok &= openvpn_snprintf(env_name, sizeof(env_name), > > "dns_server_%d_address4", i); > bitwise and with a bool feels wrong. We should use && instead of & for > boolean logic. Since there is no &&= operator, I thought it's more readable with &=. The result is the same because of the way bool is defined. I'll convert it to logical operators for the v2 and then we'll see if it looks and feels better. > > + enum dns_security dnssec; > > + enum dns_server_transport transport; > > + char *sni; > > +}; > > That should be probably be a const char* instead of a modifable char* Right. > > gc_free(>gc); > > > > +gc_free(>dns_options.gc); > > having an extra gc in this way feels a bit strange. Since this is the > only time it is initialised you can just as well just use o->gc instead. Actually no. The --dns option is a bit special as it can partially replace local setting with pushed ones (i.e. override single dns servers and others not). The contained gc is used to allocate (and free) pulled options. If we'd use o->gc as well, the pulled options wouldn't be freed for a potentially long time, increasing the memory footprint over time. > > +struct gc_arena dns_gc = gc_new();; > > extra ; Eagle eyes! =) > > +/* Free DNS options and reset them to pre-pull state */ > > +gc_free(>dns_options.gc); > > There is an additional free for the gc here that has no matching > gc_init. If your goal is to have a gc for the life time of prepull etc, > then it is better to put it there as that is easier to see its lifetime > than to have it only for DNS where the lifetime/ownership is not so clear. The accompanying gc_init() is with the one for o->gc. Don't see how moving the gc into struct options_pre_connect would change anything substantially. The gc_init would still be done in init_options() then. > I see also that this code makes no attempt to remove/refactor the old > DHCP code. The only thing I see is the tuntap_options_copy_dns code. > What the supposed way forward here? That also raises the question about > platform specific code. Does that code now need two implementations? One > for installing options from the dhcp structs and one from the DNS struct? > > Some idea
[Openvpn-devel] [PATCH] add support for --dns option
As a first step towards DNS configuration in openvpn and a unified way to push DNS related settings to clients in v2 and v3, this commit adds support for parsing the new --dns option. Later commits will add support for setting up DNS on different platforms. For now, --dns and DNS related --dhcp-option can be used together for smoother transition. Settings from --dns will override ones --dhcp-option where applicable. For detailed information about the option consult the documentation in this commit. Signed-off-by: Heiko Hund --- doc/man-sections/client-options.rst | 55 +++ doc/man-sections/script-options.rst | 19 + doc/man-sections/server-options.rst | 2 +- src/openvpn/Makefile.am | 1 + src/openvpn/dns.c | 561 src/openvpn/dns.h | 89 + src/openvpn/openvpn.vcxproj | 4 +- src/openvpn/openvpn.vcxproj.filters | 8 +- src/openvpn/options.c | 221 +++ src/openvpn/options.h | 7 + src/openvpn/push.c | 4 + src/openvpn/socket.c| 11 + src/openvpn/socket.h| 2 + 13 files changed, 981 insertions(+), 3 deletions(-) create mode 100644 src/openvpn/dns.c create mode 100644 src/openvpn/dns.h diff --git a/doc/man-sections/client-options.rst b/doc/man-sections/client-options.rst index 92a02e28..bdfe6aac 100644 --- a/doc/man-sections/client-options.rst +++ b/doc/man-sections/client-options.rst @@ -154,6 +154,61 @@ configuration. --connect-timeout n See ``--server-poll-timeout``. +--dns args + Client DNS configuration to be used with the connection. + + Valid syntaxes: + :: + + dns search-domains domain [domain ...] + dns server n address addr[:port] [addr[:port]] + dns server n resolve-domains|exclude-domains domain [domain ...] + dns server n dnssec yes|optional|no + dns server n transport DoH|DoT|plain + dns server n sni server-name + + The ``--dns search-domains`` directive takes one or more domain names + to be added as DNS domain suffixes. If it is repeated multiple times within + a configuration the domains are appended, thus e.g. domain names pushed by + a server will amend locally defined ones. + + The ``--dns server`` directive is used to configure DNS server ``n``. + The server id ``n`` must be a value between -128 and 127. For pushed + DNS server options it must be between 0 and 127. The server id is used + to group options and also for ordering the list of configured DNS servers; + lower numbers come first. DNS servers being pushed to a client replace + already configured DNS servers with the same server id. + + The ``address`` option configures the IPv4 and / or IPv6 address of + the DNS server. Optionally a port can be appended after a colon. IPv6 + addresses need to be enclosed in brackets if a port is appended. + + The ``resolve-domains`` and ``exclude-domains`` options take one or + more DNS domains which are explicitly resolved or explicitly not resolved + by a server. Only one of the options can be configured for a server. + ``resolve-domains`` is used to define a split-dns setup, where only + given domains are resolved by a server. ``exclude-domains`` is used to + define domains which will never be resolved by a server (e.g. domains + which can only be resolved locally). Systems which do not support fine + grained DNS domain configuration, will ignore these settings. + + The ``dnssec`` option is used to configure validation of DNSSEC records. + While the exact semantics may differ for resolvers on different systems, + ``yes`` likely makes validation mandatory, ``no`` disables it, and ``optional`` + uses it opportunistically. + + The ``transport`` option enables DNS-over-HTTPS (``DoH``) or DNS-over-TLS (``DoT``) + for a DNS server. The ``sni`` option can be used with them to specify the + ``server-name`` for TLS server name indication. + + Each server has to have at least one address configured for a configuration + to be valid. All the other options can be omitted. + + The ``--dns`` option will eventually obsolete the ``--dhcp-option`` directive. + Until then it will add configuration at the places ``--dhcp-option`` puts it + and will override it if both are given. So, ``--dns`` can be used today to + migrate from ``--dhcp-option``. + --explicit-exit-notify n In UDP client mode or point-to-point mode, send server/peer an exit notification if tunnel is restarted or OpenVPN process is exited. In diff --git a/doc/man-sections/script-options.rst b/doc/man-sections/script-options.rst index 22990f4f..c81efe9e 100644 --- a/doc/man-sections/script-options.rst +++ b/doc/man-sections/script-options.rst @@ -586,6 +586,25 @@ instances. netsh.exe calls which sometimes just do not work right with interface names). Set prior to ``--up`` or ``--down`` script execution. +:code:`dns_*` +The ``--dns`` configuration options
[Openvpn-devel] [PATCHv2 7/7] Add gc_arena to struct argv to save allocations
With the private gc_arena we do not have to allocate the strings
found during parsing again, since we know the arena they are
allocated in is valid as long as the argv vector is.
Signed-off-by: Heiko Hund <heiko.h...@sophos.com>
---
src/openvpn/argv.c | 44
src/openvpn/argv.h | 1 +
tests/unit_tests/openvpn/test_argv.c | 23 +++
3 files changed, 43 insertions(+), 25 deletions(-)
diff --git a/src/openvpn/argv.c b/src/openvpn/argv.c
index 419b1dc6..51dd1b22 100644
--- a/src/openvpn/argv.c
+++ b/src/openvpn/argv.c
@@ -46,12 +46,11 @@ argv_extend(struct argv *a, const size_t newcap)
{
char **newargv;
size_t i;
-ALLOC_ARRAY_CLEAR(newargv, char *, newcap);
+ALLOC_ARRAY_CLEAR_GC(newargv, char *, newcap, >gc);
for (i = 0; i < a->argc; ++i)
{
newargv[i] = a->argv[i];
}
-free(a->argv);
a->argv = newargv;
a->capacity = newcap;
}
@@ -63,6 +62,7 @@ argv_init(struct argv *a)
a->capacity = 0;
a->argc = 0;
a->argv = NULL;
+a->gc = gc_new();
argv_extend(a, 8);
}
@@ -77,24 +77,21 @@ argv_new(void)
void
argv_free(struct argv *a)
{
-size_t i;
-for (i = 0; i < a->argc; ++i)
-{
-free(a->argv[i]);
-}
-free(a->argv);
+gc_free(>gc);
}
static void
argv_reset(struct argv *a)
{
-size_t i;
-for (i = 0; i < a->argc; ++i)
+if (a->argc)
{
-free(a->argv[i]);
-a->argv[i] = NULL;
+size_t i;
+for (i = 0; i < a->argc; ++i)
+{
+a->argv[i] = NULL;
+}
+a->argc = 0;
}
-a->argc = 0;
}
static void
@@ -106,7 +103,7 @@ argv_grow(struct argv *a, const size_t add)
}
static void
-argv_append(struct argv *a, char *str) /* str must have been malloced or be
NULL */
+argv_append(struct argv *a, char *str) /* str must have been gc_malloced or
be NULL */
{
argv_grow(a, 1);
a->argv[a->argc++] = str;
@@ -127,7 +124,7 @@ argv_clone(const struct argv *a, const size_t headroom)
{
for (i = 0; i < a->argc; ++i)
{
-argv_append(, string_alloc(a->argv[i], NULL));
+argv_append(, string_alloc(a->argv[i], ));
}
}
return r;
@@ -138,7 +135,7 @@ argv_insert_head(const struct argv *a, const char *head)
{
struct argv r;
r = argv_clone(a, 1);
-r.argv[0] = string_alloc(head, NULL);
+r.argv[0] = string_alloc(head, );
return r;
}
@@ -243,7 +240,7 @@ argv_printf_arglist(struct argv *a, const char *format,
va_list arglist)
}
size = adjust_power_of_2(len + 1);
-buf = gc_malloc(size, false, );
+buf = gc_malloc(size, false, >gc);
len = vsnprintf(buf, size, f, arglist);
if (len < 0 || len >= size)
{
@@ -255,11 +252,11 @@ argv_printf_arglist(struct argv *a, const char *format,
va_list arglist)
while (end)
{
*end = '\0';
-argv_append(a, string_alloc(buf, NULL));
+argv_append(a, buf);
buf = end + 1;
end = strchr(buf, delim);
}
-argv_append(a, string_alloc(buf, NULL));
+argv_append(a, buf);
if (a->argc != argc)
{
@@ -303,23 +300,20 @@ argv_parse_cmd(struct argv *a, const char *s)
{
int nparms;
char *parms[MAX_PARMS + 1];
-struct gc_arena gc = gc_new();
argv_reset(a);
-nparms = parse_line(s, parms, MAX_PARMS, "SCRIPT-ARGV", 0,
D_ARGV_PARSE_CMD, );
+nparms = parse_line(s, parms, MAX_PARMS, "SCRIPT-ARGV", 0,
D_ARGV_PARSE_CMD, >gc);
if (nparms)
{
int i;
for (i = 0; i < nparms; ++i)
{
-argv_append(a, string_alloc(parms[i], NULL));
+argv_append(a, parms[i]);
}
}
else
{
-argv_append(a, string_alloc(s, NULL));
+argv_append(a, string_alloc(s, >gc));
}
-
-gc_free();
}
diff --git a/src/openvpn/argv.h b/src/openvpn/argv.h
index 2a1945e3..a1aa7ebb 100644
--- a/src/openvpn/argv.h
+++ b/src/openvpn/argv.h
@@ -33,6 +33,7 @@
#include "buffer.h"
struct argv {
+struct gc_arena gc;
size_t capacity;
size_t argc;
char **argv;
diff --git a/tests/unit_tests/openvpn/test_argv.c
b/tests/unit_tests/openvpn/test_argv.c
index e15e2fe5..5bfef8af 100644
--- a/tests/unit_tests/openvpn/test_argv.c
+++ b/tests/unit_tests/openvpn/test_argv.c
@@ -118,6 +118,28 @@ argv_printf__empty_parameter__argc_correct(void **state)
}
static void
+argv_printf__long_args__data_correct(void **state)
+{
+int i;
+struct argv a = argv_new();
+const char *args[] = {
+"good_tools_have_good_names_even_though_it_might_impair_typing",
+"--long-opt=long
[Openvpn-devel] [PATCHv2 7/7] Add gc_arena to struct argv to save allocations
With the private gc_arena we do not have to allocate the strings
found during parsing again, since we know the arena they are
allocated in is valid as long as the argv vector is.
Signed-off-by: Heiko Hund <heiko.h...@sophos.com>
---
src/openvpn/argv.c | 44
src/openvpn/argv.h | 1 +
tests/unit_tests/openvpn/test_argv.c | 23 +++
3 files changed, 43 insertions(+), 25 deletions(-)
diff --git a/src/openvpn/argv.c b/src/openvpn/argv.c
index 419b1dc6..51dd1b22 100644
--- a/src/openvpn/argv.c
+++ b/src/openvpn/argv.c
@@ -46,12 +46,11 @@ argv_extend(struct argv *a, const size_t newcap)
{
char **newargv;
size_t i;
-ALLOC_ARRAY_CLEAR(newargv, char *, newcap);
+ALLOC_ARRAY_CLEAR_GC(newargv, char *, newcap, >gc);
for (i = 0; i < a->argc; ++i)
{
newargv[i] = a->argv[i];
}
-free(a->argv);
a->argv = newargv;
a->capacity = newcap;
}
@@ -63,6 +62,7 @@ argv_init(struct argv *a)
a->capacity = 0;
a->argc = 0;
a->argv = NULL;
+a->gc = gc_new();
argv_extend(a, 8);
}
@@ -77,24 +77,21 @@ argv_new(void)
void
argv_free(struct argv *a)
{
-size_t i;
-for (i = 0; i < a->argc; ++i)
-{
-free(a->argv[i]);
-}
-free(a->argv);
+gc_free(>gc);
}
static void
argv_reset(struct argv *a)
{
-size_t i;
-for (i = 0; i < a->argc; ++i)
+if (a->argc)
{
-free(a->argv[i]);
-a->argv[i] = NULL;
+size_t i;
+for (i = 0; i < a->argc; ++i)
+{
+a->argv[i] = NULL;
+}
+a->argc = 0;
}
-a->argc = 0;
}
static void
@@ -106,7 +103,7 @@ argv_grow(struct argv *a, const size_t add)
}
static void
-argv_append(struct argv *a, char *str) /* str must have been malloced or be
NULL */
+argv_append(struct argv *a, char *str) /* str must have been gc_malloced or
be NULL */
{
argv_grow(a, 1);
a->argv[a->argc++] = str;
@@ -127,7 +124,7 @@ argv_clone(const struct argv *a, const size_t headroom)
{
for (i = 0; i < a->argc; ++i)
{
-argv_append(, string_alloc(a->argv[i], NULL));
+argv_append(, string_alloc(a->argv[i], ));
}
}
return r;
@@ -138,7 +135,7 @@ argv_insert_head(const struct argv *a, const char *head)
{
struct argv r;
r = argv_clone(a, 1);
-r.argv[0] = string_alloc(head, NULL);
+r.argv[0] = string_alloc(head, );
return r;
}
@@ -243,7 +240,7 @@ argv_printf_arglist(struct argv *a, const char *format,
va_list arglist)
}
size = adjust_power_of_2(len + 1);
-buf = gc_malloc(size, false, );
+buf = gc_malloc(size, false, >gc);
len = vsnprintf(buf, size, f, arglist);
if (len < 0 || len >= size)
{
@@ -255,11 +252,11 @@ argv_printf_arglist(struct argv *a, const char *format,
va_list arglist)
while (end)
{
*end = '\0';
-argv_append(a, string_alloc(buf, NULL));
+argv_append(a, buf);
buf = end + 1;
end = strchr(buf, delim);
}
-argv_append(a, string_alloc(buf, NULL));
+argv_append(a, buf);
if (a->argc != argc)
{
@@ -303,23 +300,20 @@ argv_parse_cmd(struct argv *a, const char *s)
{
int nparms;
char *parms[MAX_PARMS + 1];
-struct gc_arena gc = gc_new();
argv_reset(a);
-nparms = parse_line(s, parms, MAX_PARMS, "SCRIPT-ARGV", 0,
D_ARGV_PARSE_CMD, );
+nparms = parse_line(s, parms, MAX_PARMS, "SCRIPT-ARGV", 0,
D_ARGV_PARSE_CMD, >gc);
if (nparms)
{
int i;
for (i = 0; i < nparms; ++i)
{
-argv_append(a, string_alloc(parms[i], NULL));
+argv_append(a, parms[i]);
}
}
else
{
-argv_append(a, string_alloc(s, NULL));
+argv_append(a, string_alloc(s, >gc));
}
-
-gc_free();
}
diff --git a/src/openvpn/argv.h b/src/openvpn/argv.h
index 2a1945e3..a1aa7ebb 100644
--- a/src/openvpn/argv.h
+++ b/src/openvpn/argv.h
@@ -33,6 +33,7 @@
#include "buffer.h"
struct argv {
+struct gc_arena gc;
size_t capacity;
size_t argc;
char **argv;
diff --git a/tests/unit_tests/openvpn/test_argv.c
b/tests/unit_tests/openvpn/test_argv.c
index e15e2fe5..5bfef8af 100644
--- a/tests/unit_tests/openvpn/test_argv.c
+++ b/tests/unit_tests/openvpn/test_argv.c
@@ -118,6 +118,28 @@ argv_printf__empty_parameter__argc_correct(void **state)
}
static void
+argv_printf__long_args__data_correct(void **state)
+{
+int i;
+struct argv a = argv_new();
+const char *args[] = {
+"good_tools_have_good_names_even_though_it_might_impair_typing",
+"--long-opt=long
[Openvpn-devel] [PATCHv2 6/7] argv: do fewer memory re-allocations
Prevent the re-allocations of memory when the internal argv grows
beyond 2 and 4 arguments by initially allocating argv to hold up to
7 (+ trailing NULL) pointers.
While at it rename argv_reset to argv_free to actually express
what's going on. Redo the argv_reset functionality so that it can
be used to actually reset the argv without re-allocation.
Signed-off-by: Heiko Hund <heiko.h...@sophos.com>
---
src/openvpn/argv.c | 53 ++--
src/openvpn/argv.h | 2 +-
src/openvpn/console_systemd.c| 2 +-
src/openvpn/init.c | 15 ++
src/openvpn/lladdr.c | 2 +-
src/openvpn/multi.c | 10 +++
src/openvpn/options.c| 2 +-
src/openvpn/plugin.c | 2 +-
src/openvpn/route.c | 8 +++---
src/openvpn/socket.c | 4 +--
src/openvpn/ssl_verify.c | 6 ++--
src/openvpn/tun.c| 32 --
tests/unit_tests/openvpn/test_argv.c | 41 +++-
13 files changed, 94 insertions(+), 85 deletions(-)
diff --git a/src/openvpn/argv.c b/src/openvpn/argv.c
index afe8efff..419b1dc6 100644
--- a/src/openvpn/argv.c
+++ b/src/openvpn/argv.c
@@ -40,11 +40,30 @@
#include "options.h"
static void
+argv_extend(struct argv *a, const size_t newcap)
+{
+if (newcap > a->capacity)
+{
+char **newargv;
+size_t i;
+ALLOC_ARRAY_CLEAR(newargv, char *, newcap);
+for (i = 0; i < a->argc; ++i)
+{
+newargv[i] = a->argv[i];
+}
+free(a->argv);
+a->argv = newargv;
+a->capacity = newcap;
+}
+}
+
+static void
argv_init(struct argv *a)
{
a->capacity = 0;
a->argc = 0;
a->argv = NULL;
+argv_extend(a, 8);
}
struct argv
@@ -56,7 +75,7 @@ argv_new(void)
}
void
-argv_reset(struct argv *a)
+argv_free(struct argv *a)
{
size_t i;
for (i = 0; i < a->argc; ++i)
@@ -64,25 +83,18 @@ argv_reset(struct argv *a)
free(a->argv[i]);
}
free(a->argv);
-argv_init(a);
}
static void
-argv_extend(struct argv *a, const size_t newcap)
+argv_reset(struct argv *a)
{
-if (newcap > a->capacity)
+size_t i;
+for (i = 0; i < a->argc; ++i)
{
-char **newargv;
-size_t i;
-ALLOC_ARRAY_CLEAR(newargv, char *, newcap);
-for (i = 0; i < a->argc; ++i)
-{
-newargv[i] = a->argv[i];
-}
-free(a->argv);
-a->argv = newargv;
-a->capacity = newcap;
+free(a->argv[i]);
+a->argv[i] = NULL;
}
+a->argc = 0;
}
static void
@@ -133,14 +145,7 @@ argv_insert_head(const struct argv *a, const char *head)
const char *
argv_str(const struct argv *a, struct gc_arena *gc, const unsigned int flags)
{
-if (a->argv)
-{
-return print_argv((const char **)a->argv, gc, flags);
-}
-else
-{
-return "";
-}
+return print_argv((const char **)a->argv, gc, flags);
}
void
@@ -221,8 +226,6 @@ argv_printf_arglist(struct argv *a, const char *format,
va_list arglist)
va_list tmplist;
int len;
-argv_extend(a, 1); /* ensure trailing NULL */
-
argc = a->argc;
f = argv_prep_format(format, delim, , );
if (f == NULL)
@@ -262,7 +265,6 @@ argv_printf_arglist(struct argv *a, const char *format,
va_list arglist)
{
/* Someone snuck in a \035, fail gracefully */
argv_reset(a);
-argv_extend(a, 1); /* ensure trailing NULL */
goto out;
}
@@ -304,7 +306,6 @@ argv_parse_cmd(struct argv *a, const char *s)
struct gc_arena gc = gc_new();
argv_reset(a);
-argv_extend(a, 1); /* ensure trailing NULL */
nparms = parse_line(s, parms, MAX_PARMS, "SCRIPT-ARGV", 0,
D_ARGV_PARSE_CMD, );
if (nparms)
diff --git a/src/openvpn/argv.h b/src/openvpn/argv.h
index a24ba98f..2a1945e3 100644
--- a/src/openvpn/argv.h
+++ b/src/openvpn/argv.h
@@ -40,7 +40,7 @@ struct argv {
struct argv argv_new(void);
-void argv_reset(struct argv *a);
+void argv_free(struct argv *a);
const char *argv_str(const struct argv *a, struct gc_arena *gc, const unsigned
int flags);
diff --git a/src/openvpn/console_systemd.c b/src/openvpn/console_systemd.c
index e7a72ae3..5b09dee4 100644
--- a/src/openvpn/console_systemd.c
+++ b/src/openvpn/console_systemd.c
@@ -84,7 +84,7 @@ get_console_input_systemd(const char *prompt, const bool
echo, char *input, cons
}
close(std_out);
-argv_reset();
+argv_free();
return ret;
}
diff --git a/src/openvpn/init.c b/src/openvpn/init.c
index 1ed2c55e..25962958 100644
--- a/src/openvpn/init.c
+++ b/src/openvpn/init.c
@@ -162,7 +162,7 @@ run_up_down(const char *command,
msg(M_FATAL,
[Openvpn-devel] [PATCHv2 5/7] re-implement argv_printf_*()
The previous implementation had the problem that it was not fully
compatible with printf() and could only detect % format directives
following a space character (0x20).
It modifies the format string and inserts marks to separate groups
before passing it to the regular printf in libc. The marks are
later used to separate the output string into individual command
line arguments.
The choice of 035 as the argument delimiter is based on the
assumption that no "regular" string passed to argv_printf_*() will
ever have to contain that byte (and the fact that it actually is
the ASCII "group separator" control character, which fits its
purpose).
Signed-off-by: Heiko Hund <heiko.h...@sophos.com>
---
src/openvpn/argv.c | 253 ---
src/openvpn/argv.h | 4 +-
src/openvpn/route.c | 8 +-
src/openvpn/tun.c| 24 ++--
tests/unit_tests/openvpn/test_argv.c | 58 +++-
5 files changed, 184 insertions(+), 163 deletions(-)
diff --git a/src/openvpn/argv.c b/src/openvpn/argv.c
index 95bdfeac..afe8efff 100644
--- a/src/openvpn/argv.c
+++ b/src/openvpn/argv.c
@@ -130,64 +130,6 @@ argv_insert_head(const struct argv *a, const char *head)
return r;
}
-static char *
-argv_term(const char **f)
-{
-const char *p = *f;
-const char *term = NULL;
-size_t termlen = 0;
-
-if (*p == '\0')
-{
-return NULL;
-}
-
-while (true)
-{
-const int c = *p;
-if (c == '\0')
-{
-break;
-}
-if (term)
-{
-if (!isspace(c))
-{
-++termlen;
-}
-else
-{
-break;
-}
-}
-else
-{
-if (!isspace(c))
-{
-term = p;
-termlen = 1;
-}
-}
-++p;
-}
-*f = p;
-
-if (term)
-{
-char *ret;
-ASSERT(termlen > 0);
-ret = malloc(termlen + 1);
-check_malloc_return(ret);
-memcpy(ret, term, termlen);
-ret[termlen] = '\0';
-return ret;
-}
-else
-{
-return NULL;
-}
-}
-
const char *
argv_str(const struct argv *a, struct gc_arena *gc, const unsigned int flags)
{
@@ -217,112 +159,141 @@ argv_msg_prefix(const int msglev, const struct argv *a,
const char *prefix)
gc_free();
}
-static void
-argv_printf_arglist(struct argv *a, const char *format, va_list arglist)
+/*
+ * argv_prep_format - prepare argv format string for further processing
+ *
+ * Individual argument must be separated by space. Ignores leading and
trailing spaces.
+ * Consecutive spaces count as one. Returns prepared format string, with space
replaced
+ * by delim and adds the number of arguments to the count parameter.
+ */
+static char *
+argv_prep_format(const char *format, const char delim, size_t *count, struct
gc_arena *gc)
{
-char *term;
-const char *f = format;
+int i, j;
+char *f;
+bool in_token = false;
-argv_extend(a, 1); /* ensure trailing NULL */
+if (format == NULL)
+{
+return NULL;
+}
-while ((term = argv_term()) != NULL)
+f = gc_malloc(strlen(format) + 1, true, gc);
+for (i = 0, j = 0; i < strlen(format); i++)
{
-if (term[0] == '%')
+if (format[i] == ' ')
{
-if (!strcmp(term, "%s"))
-{
-char *s = va_arg(arglist, char *);
-if (!s)
-{
-s = "";
-}
-argv_append(a, string_alloc(s, NULL));
-}
-else if (!strcmp(term, "%d"))
-{
-char numstr[64];
-openvpn_snprintf(numstr, sizeof(numstr), "%d", va_arg(arglist,
int));
-argv_append(a, string_alloc(numstr, NULL));
-}
-else if (!strcmp(term, "%u"))
-{
-char numstr[64];
-openvpn_snprintf(numstr, sizeof(numstr), "%u", va_arg(arglist,
unsigned int));
-argv_append(a, string_alloc(numstr, NULL));
-}
-else if (!strcmp(term, "%s/%d"))
-{
-char numstr[64];
-char *s = va_arg(arglist, char *);
-
-if (!s)
-{
-s = "";
-}
-
-openvpn_snprintf(numstr, sizeof(numstr), "%d", va_arg(arglist,
int));
-
-{
-const size_t len = strlen(s) + strlen(numstr) + 2;
-char *combined = (char *) malloc(len);
-check_malloc_return(combined);
-
-strcpy(combined, s);
-strcat(combined, "/");
-
Re: [Openvpn-devel] [PATCH 7/7] Add gc_arena to struct argv to save allocations
On Thursday, November 10, 2016 4:01:20 PM CET David Sommerseth wrote:
> On 28/10/16 18:42, Heiko Hund wrote:
> >
> > + a->gc = gc_new ();
> >argv_extend (a, 8);
> > }
>
> Any specific reason we want to keep our own gc_arena on argv? Why not
> pass an existing gc_arena pointer to the argv_new()/argv_init()
> functions? I believe the majority of places argv functions are called
> already have a gc_arena prepared. But I do see it may complicate the
> argv_reset() function.
The idea was that struct argv is usually very short lived. Some callers do not
have a gc and would need to create one just for using struct argv, others have
one and it is rather long lived, so it would hang on to the memory much
longer than needed.
> When looking over the final argv.c file, I also wonder about this:
>
> const char *
> argv_str (const struct argv *a, struct gc_arena *gc, const unsigned int
> flags)
> {
> return print_argv ((const char **)a->argv, gc, flags);
> }
>
> Why not use our own internal gc_arena? Especially when considering the
> current callers:
>
> void
> argv_msg (const int msglev, const struct argv *a)
> {
> struct gc_arena gc = gc_new ();
> msg (msglev, "%s", argv_str (a, , 0));
> gc_free ();
> }
>
> The situation is identical for argv_msg_prefix().
The difference between argv_str and argv_msg is that the latter one doesn't
pass the gc_alloced string to the calling scope. In that case the caller shoul
take care of when the string is freed.
> The overall impression I have is that this patch-set is truly valuable
> and does important clean-ups and fixes a lot of ambiguity and odd
> behaviours. And getting proper unit tests on top of it all is truly
> great! But there's still some improvements needed in the last 3 patches
> (patch 5, 6 and 7).
Thanks for taking the time to review this. Let's do another round for 5-7.
Cheers
Heiko
--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel
Re: [Openvpn-devel] [PATCH 6/7] argv: do fewer memory re-allocations
On Wednesday, November 9, 2016 11:58:21 PM CET David Sommerseth wrote:
> > argv_init (struct argv *a)
> > {
> >
> >a->capacity = 0;
> >a->argc = 0;
> >a->argv = NULL;
> >
> > + argv_extend (a, 8);
>
> Why 8? Done any performance and/or memory utilization tests? Does the
> overall memory consumption increase with this?
I figured it is a fine initial length for the argv vector. No science behind it
whatsoever. If 8 doesn't suffice the argv is reallocated.
> > @@ -126,10 +138,7 @@ argv_insert_head (const struct argv *a, const char
> > *head)>
> > const char *
> > argv_str (const struct argv *a, struct gc_arena *gc, const unsigned int
> > flags) {
> >
> > - if (a->argv)
> > -return print_argv ((const char **)a->argv, gc, flags);
> > - else
> > -return "";
> > + return print_argv ((const char **)a->argv, gc, flags);
>
> There are very few callers of print_argv() (defined in buffer.c).
> Should we migrate print_argv() into this function and move all callers
> over to argv_str() instead?
print_argv actually prints a char** and is not affiliated with struct argv,
even
though its name suggests it. So, if we want to do it it should be an add-on
done after this patchset.
> > diff --git a/src/openvpn/tun.c b/src/openvpn/tun.c
> > --- a/src/openvpn/tun.c
> > +++ b/src/openvpn/tun.c
>
> tun.c:2360 close_tun() seems to be missing a argv_free(), or?
> The same goes for close_tun() in tun.c:2482 and tun.c:2956
> (tun.c:2821 does have it though).
>
> Similarly, in open_tun() [tun.c:2926], argv_new() is called but no
> argv_free() calls.
Nice catch, they were missing back in the argv_reset days as well. While at
it, also removed a couple of unused struct gc_arena.
> On a not so related note. I noticed that init.c have a
> #ifdef ARGV_TEST block. That should probably also be killed; no need
> for that as we have unit tests - and the argv_test() function it calls
> no longer exists.
Aaand it's gone.
Cheers
Heiko
--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel
Re: [Openvpn-devel] [PATCH 5/7] re-implement argv_printf_*()
Hi David,
On Wednesday, November 9, 2016 9:41:21 PM CET David Sommerseth wrote:
> In the new argv_prep_format() function:
>
> + if (!in_token)
> +{
> + ++*count;
> + if (f[0])
> +f[j++] = delim;
> +}
>
> What is the purpose of the f[0] check? At first glance it looks like a
> NOOP, as you'd expect f[0] to always have a value > 0. But that is just
> until you've parsed something which have been put into f, so I presume
> it is some kind of "lock" to not write a delimiter as the first byte in
> a string.
That check is there to not add a delimiter after leading spaces, I've added a
comment in v2 of the patch.
> Can this whole function be made a bit simpler to understand, as I'm
> struggling to fully follow what happens and why? If I spend enough time
> digging into this, I will understand it ... but I prefer code which is
> easier to understand at first glance, unless the task can't be resolved
> with less complexity.
Added comments to explain what's happening in argv_prep_format() and
argv_printf_arglist(). Especially the first one does more than just copying the
format string buffer, so its complexity is justified.
> So to argv_printf_arglist():
>
> + va_copy (tmplist, arglist);
> + len = vsnprintf (NULL, 0, f, tmplist);
> + va_end (tmplist);
> + if (len < 0)
> +goto out;
>
> This is reasonably okay, but would benefit from a comment simply
> explaining what you do here. I presume it is a kind of estimated
> strlen() operation of the final formatted string.
Done.
> Wed Nov 9 21:37:26 2016
> Wed Nov 9 21:37:26 2016 openvpn_execve: called with empty argv
> Wed Nov 9 21:37:26 2016 Exiting due to fatal error
>
> Reverting this patch makes t_cltsrv.sh succeed, so pretty sure this
> patch is the one to blame for this error.
Nice catch! This was triggered by the use of strtok(), which only returns
nonempty tokens. During shutdown openvpn passes empty strings to the down
script, which triggered this. Not using strtok() anymore and also added a unit
test for empty string parameters.
Cheers
Heiko
--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel
[Openvpn-devel] [PATCH] struct argv overhaul
Hi This series is a direct outcome of the incident with my previous patch. I've taken on struct argv and related functions and made them like I please. For a more detailed description what's happening see the comments in the individual patches: [PATCH 1/7] put argv_* functions into own file, add unit tests [PATCH 2/7] Remove unused and unecessary argv interfaces [PATCH 3/7] remove unused system_str from struct argv [PATCH 4/7] Factor out %sc handling from argv_printf() [PATCH 5/7] re-implement argv_printf_*() [PATCH 6/7] argv: do fewer memory re-allocations [PATCH 7/7] Add gc_arena to struct argv to save allocations Code hase only been unit tested, so I'd really also like feedback from someone actually running the code. And if it be buildbot... Cheers Heiko -- The Command Line: Reinvented for Modern Developers Did the resurgence of CLI tooling catch you by surprise? Reconnect with the command line and become more productive. Learn the new .NET and ASP.NET CLI. Get your free copy! http://sdm.link/telerik ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
[Openvpn-devel] [PATCH 7/7] Add gc_arena to struct argv to save allocations
With the private gc_arena we do not have to allocate the strings
found during parsing again, since we know the arena they are
allocated in is valid as long as the argv vector is.
Signed-off-by: Heiko Hund <heiko.h...@sophos.com>
---
src/openvpn/argv.c | 37 ++--
src/openvpn/argv.h | 1 +
tests/unit_tests/openvpn/test_argv.c | 21
3 files changed, 40 insertions(+), 19 deletions(-)
diff --git a/src/openvpn/argv.c b/src/openvpn/argv.c
index 664785b..9f139bb 100644
--- a/src/openvpn/argv.c
+++ b/src/openvpn/argv.c
@@ -62,6 +62,7 @@ argv_init (struct argv *a)
a->capacity = 0;
a->argc = 0;
a->argv = NULL;
+ a->gc = gc_new ();
argv_extend (a, 8);
}
@@ -76,22 +77,23 @@ argv_new (void)
void
argv_free (struct argv *a)
{
- size_t i;
- for (i = 0; i < a->argc; ++i)
-free (a->argv[i]);
+ gc_free (>gc);
free (a->argv);
}
static void
argv_reset (struct argv *a)
{
- size_t i;
- for (i = 0; i < a->argc; ++i)
+ if (a->argc)
{
- free (a->argv[i]);
- a->argv[i] = NULL;
+ size_t i;
+ for (i = 0; i < a->argc; ++i)
+a->argv[i] = NULL;
+ a->argc = 0;
+
+ gc_free (>gc);
+ a->gc = gc_new ();
}
- a->argc = 0;
}
static void
@@ -103,7 +105,7 @@ argv_grow (struct argv *a, const size_t add)
}
static void
-argv_append (struct argv *a, char *str) /* str must have been malloced or be
NULL */
+argv_append (struct argv *a, char *str) /* str must have been gc_malloced or
be NULL */
{
argv_grow (a, 1);
a->argv[a->argc++] = str;
@@ -121,7 +123,7 @@ argv_clone (const struct argv *a, const size_t headroom)
if (a)
{
for (i = 0; i < a->argc; ++i)
-argv_append (, string_alloc (a->argv[i], NULL));
+argv_append (, string_alloc (a->argv[i], ));
}
return r;
}
@@ -131,7 +133,7 @@ argv_insert_head (const struct argv *a, const char *head)
{
struct argv r;
r = argv_clone (a, 1);
- r.argv[0] = string_alloc (head, NULL);
+ r.argv[0] = string_alloc (head, );
return r;
}
@@ -212,7 +214,7 @@ argv_printf_arglist (struct argv *a, const char *format,
va_list arglist)
goto out;
size = adjust_power_of_2 (len + 1);
- buf = gc_malloc (size, false, );
+ buf = gc_malloc (size, false, >gc);
len = vsnprintf (buf, size, f, arglist);
if (len < 0 || len >= size)
goto out;
@@ -220,7 +222,7 @@ argv_printf_arglist (struct argv *a, const char *format,
va_list arglist)
token = strtok (buf, delim);
while (token)
{
- argv_append (a, string_alloc (token, NULL));
+ argv_append (a, token);
token = strtok (NULL, delim);
}
@@ -266,19 +268,16 @@ argv_parse_cmd (struct argv *a, const char *s)
{
int nparms;
char *parms[MAX_PARMS + 1];
- struct gc_arena gc = gc_new ();
argv_reset (a);
- nparms = parse_line (s, parms, MAX_PARMS, "SCRIPT-ARGV", 0,
D_ARGV_PARSE_CMD, );
+ nparms = parse_line (s, parms, MAX_PARMS, "SCRIPT-ARGV", 0,
D_ARGV_PARSE_CMD, >gc);
if (nparms)
{
int i;
for (i = 0; i < nparms; ++i)
-argv_append (a, string_alloc (parms[i], NULL));
+argv_append (a, parms[i]);
}
else
-argv_append (a, string_alloc (s, NULL));
-
- gc_free ();
+argv_append (a, string_alloc (s, >gc));
}
diff --git a/src/openvpn/argv.h b/src/openvpn/argv.h
index 99fd7ad..7169f1d 100644
--- a/src/openvpn/argv.h
+++ b/src/openvpn/argv.h
@@ -34,6 +34,7 @@
#include "buffer.h"
struct argv {
+ struct gc_arena gc;
size_t capacity;
size_t argc;
char **argv;
diff --git a/tests/unit_tests/openvpn/test_argv.c
b/tests/unit_tests/openvpn/test_argv.c
index d2cd96d..3de1897 100644
--- a/tests/unit_tests/openvpn/test_argv.c
+++ b/tests/unit_tests/openvpn/test_argv.c
@@ -123,6 +123,27 @@ argv_printf__combined_path_with_spaces__argc_correct (void
**state)
}
static void
+argv_printf__long_args__data_correct (void **state)
+{
+ int i;
+ struct argv a = argv_new ();
+ const char *args[] = {
+"good_tools_have_good_names_even_though_it_might_impair_typing",
+"--long-opt=long",
+"--long-cat=lnger",
+"file_with_very_descriptive_filename_that_leaves_no_questions_open.jpg.exe"
+ };
+
+ argv_printf (, "%s %s %s %s", args[0], args[1], args[2], args[3]);
+ assert_int_equal (a.argc, 4);
+ for (i = 0; i < a.argc; i++) {
+assert_string_equal (a.argv[i], args[i]);
+ }
+
+ argv_free ();
+}
+
+static void
argv_parse_cmd__command_string__argc_correct (void **state)
{
struct argv a = argv_new ();
--
2.7.4
--
The Command Line: Reinvented for Modern D
[Openvpn-devel] [PATCH 4/7] Factor out %sc handling from argv_printf()
Move functionality to parse command strings into argv_parse_cmd().
That is a preparation for the upcoming refactoring of argv_printf().
Signed-off-by: Heiko Hund <heiko.h...@sophos.com>
---
src/openvpn/argv.c | 47 +---
src/openvpn/argv.h | 1 +
src/openvpn/errlevel.h | 2 +-
src/openvpn/init.c | 2 +-
src/openvpn/misc.c | 10 +++-
src/openvpn/multi.c | 13 --
src/openvpn/options.c| 2 +-
src/openvpn/socket.c | 7 +++---
src/openvpn/ssl_verify.c | 6 +++--
tests/unit_tests/openvpn/test_argv.c | 18 +++---
10 files changed, 56 insertions(+), 52 deletions(-)
diff --git a/src/openvpn/argv.c b/src/openvpn/argv.c
index a279a40..f8287b7 100644
--- a/src/openvpn/argv.c
+++ b/src/openvpn/argv.c
@@ -198,7 +198,6 @@ argv_msg_prefix (const int msglev, const struct argv *a,
const char *prefix)
static void
argv_printf_arglist (struct argv *a, const char *format, va_list arglist)
{
- struct gc_arena gc = gc_new ();
char *term;
const char *f = format;
@@ -215,29 +214,6 @@ argv_printf_arglist (struct argv *a, const char *format,
va_list arglist)
s = "";
argv_append (a, string_alloc (s, NULL));
}
- else if (!strcmp (term, "%sc"))
-{
- char *s = va_arg (arglist, char *);
- if (s)
-{
- int nparms;
- char *parms[MAX_PARMS+1];
- int i;
-
- nparms = parse_line (s, parms, MAX_PARMS, "SCRIPT-ARGV", 0,
D_ARGV_PARSE_CMD, );
- if (nparms)
-{
- for (i = 0; i < nparms; ++i)
-argv_append (a, string_alloc (parms[i], NULL));
-}
- else
-argv_append (a, string_alloc (s, NULL));
-}
- else
-{
- argv_append (a, string_alloc ("", NULL));
-}
-}
else if (!strcmp (term, "%d"))
{
char numstr[64];
@@ -295,7 +271,6 @@ argv_printf_arglist (struct argv *a, const char *format,
va_list arglist)
argv_append (a, term);
}
}
- gc_free ();
}
void
@@ -317,3 +292,25 @@ argv_printf_cat (struct argv *a, const char *format, ...)
va_end (arglist);
}
+void
+argv_parse_cmd (struct argv *a, const char *s)
+{
+ int nparms;
+ char *parms[MAX_PARMS + 1];
+ struct gc_arena gc = gc_new ();
+
+ argv_reset (a);
+ argv_extend (a, 1); /* ensure trailing NULL */
+
+ nparms = parse_line (s, parms, MAX_PARMS, "SCRIPT-ARGV", 0,
D_ARGV_PARSE_CMD, );
+ if (nparms)
+{
+ int i;
+ for (i = 0; i < nparms; ++i)
+argv_append (a, string_alloc (parms[i], NULL));
+}
+ else
+argv_append (a, string_alloc (s, NULL));
+
+ gc_free ();
+}
diff --git a/src/openvpn/argv.h b/src/openvpn/argv.h
index c45bec8..9aee641 100644
--- a/src/openvpn/argv.h
+++ b/src/openvpn/argv.h
@@ -45,6 +45,7 @@ const char *argv_str (const struct argv *a, struct gc_arena
*gc, const unsigned
struct argv argv_insert_head (const struct argv *a, const char *head);
void argv_msg (const int msglev, const struct argv *a);
void argv_msg_prefix (const int msglev, const struct argv *a, const char
*prefix);
+void argv_parse_cmd (struct argv *a, const char *s);
void argv_printf (struct argv *a, const char *format, ...)
#ifdef __GNUC__
diff --git a/src/openvpn/errlevel.h b/src/openvpn/errlevel.h
index ae1f8f4..9d56eb4 100644
--- a/src/openvpn/errlevel.h
+++ b/src/openvpn/errlevel.h
@@ -142,7 +142,7 @@
#define D_PS_PROXY_DEBUG LOGLEV(7, 70, M_DEBUG) /* port share proxy debug
*/
#define D_AUTO_USERIDLOGLEV(7, 70, M_DEBUG) /* AUTO_USERID debugging
*/
#define D_TLS_KEYSELECT LOGLEV(7, 70, M_DEBUG) /* show information on
key selection for data channel */
-#define D_ARGV_PARSE_CMD LOGLEV(7, 70, M_DEBUG) /* show parse_line()
errors in argv_printf %sc */
+#define D_ARGV_PARSE_CMD LOGLEV(7, 70, M_DEBUG) /* show parse_line()
errors in argv_parse_cmd */
#define D_CRYPTO_DEBUG LOGLEV(7, 70, M_DEBUG) /* show detailed info
from crypto.c routines */
#define D_PID_DEBUG LOGLEV(7, 70, M_DEBUG) /* show packet-id
debugging info */
#define D_PF_DROPPED_BCAST LOGLEV(7, 71, M_DEBUG) /* packet filter dropped
a broadcast packet */
diff --git a/src/openvpn/init.c b/src/openvpn/init.c
index 73f8c6d..56140b3 100644
--- a/src/openvpn/init.c
+++ b/src/openvpn/init.c
@@ -1362,7 +1362,7 @@ do_route (const struct options *options,
{
struct argv argv = argv_new ();
setenv_str (es, "script_type", "route-up");
- argv_printf (, "%sc", optio
[Openvpn-devel] [PATCH 5/7] re-implement argv_printf_*()
The previous implementation had the problem that it was not fully
compatible with printf() and could only detect % format directives
following a space character (0x20).
It modifies the format string and inserts marks to separate groups
before passing it to the regular printf in libc. The marks are
later used to separate the output string into individual command
line arguments.
The choice of '\035' as the argument delimiter is based on the
assumption that no "regular" string passed to argv_printf_*() will
ever have to contain that byte (and the fact that it actually is
the ASCII "group separator" control character, which fits its
purpose).
Signed-off-by: Heiko Hund <heiko.h...@sophos.com>
---
src/openvpn/argv.c | 203 ++-
src/openvpn/argv.h | 4 +-
src/openvpn/route.c | 8 +-
src/openvpn/tun.c| 22 ++--
tests/unit_tests/openvpn/test_argv.c | 34 +-
5 files changed, 130 insertions(+), 141 deletions(-)
diff --git a/src/openvpn/argv.c b/src/openvpn/argv.c
index f8287b7..dcfbd76 100644
--- a/src/openvpn/argv.c
+++ b/src/openvpn/argv.c
@@ -6,6 +6,7 @@
* packet compression.
*
* Copyright (C) 2002-2010 OpenVPN Technologies, Inc. <sa...@openvpn.net>
+ *2016 Heiko Hund <heiko.h...@sophos.com>
*
* This program is free software; you can redistribute it and/or modify
* it under the terms of the GNU General Public License version 2
@@ -122,54 +123,6 @@ argv_insert_head (const struct argv *a, const char *head)
return r;
}
-static char *
-argv_term (const char **f)
-{
- const char *p = *f;
- const char *term = NULL;
- size_t termlen = 0;
-
- if (*p == '\0')
-return NULL;
-
- while (true)
-{
- const int c = *p;
- if (c == '\0')
-break;
- if (term)
-{
- if (!isspace (c))
-++termlen;
- else
-break;
-}
- else
-{
- if (!isspace (c))
-{
- term = p;
- termlen = 1;
-}
-}
- ++p;
-}
- *f = p;
-
- if (term)
-{
- char *ret;
- ASSERT (termlen > 0);
- ret = malloc (termlen + 1);
- check_malloc_return (ret);
- memcpy (ret, term, termlen);
- ret[termlen] = '\0';
- return ret;
-}
- else
-return NULL;
-}
-
const char *
argv_str (const struct argv *a, struct gc_arena *gc, const unsigned int flags)
{
@@ -195,101 +148,111 @@ argv_msg_prefix (const int msglev, const struct argv
*a, const char *prefix)
gc_free ();
}
-static void
-argv_printf_arglist (struct argv *a, const char *format, va_list arglist)
+static char *
+argv_prep_format (const char *format, const char delim, size_t *count, struct
gc_arena *gc)
{
- char *term;
- const char *f = format;
+ int i, j;
+ char *f;
+ bool in_token = false;
- argv_extend (a, 1); /* ensure trailing NULL */
+ if (format == NULL)
+return NULL;
- while ((term = argv_term ()) != NULL)
+ f = gc_malloc (strlen (format) + 1, true, gc);
+ for (i = 0, j = 0; i < strlen (format); i++)
{
- if (term[0] == '%')
+ if (format[i] == ' ')
{
- if (!strcmp (term, "%s"))
-{
- char *s = va_arg (arglist, char *);
- if (!s)
-s = "";
- argv_append (a, string_alloc (s, NULL));
-}
- else if (!strcmp (term, "%d"))
-{
- char numstr[64];
- openvpn_snprintf (numstr, sizeof (numstr), "%d", va_arg
(arglist, int));
- argv_append (a, string_alloc (numstr, NULL));
-}
- else if (!strcmp (term, "%u"))
-{
- char numstr[64];
- openvpn_snprintf (numstr, sizeof (numstr), "%u", va_arg
(arglist, unsigned int));
- argv_append (a, string_alloc (numstr, NULL));
-}
- else if (!strcmp (term, "%s/%d"))
-{
- char numstr[64];
- char *s = va_arg (arglist, char *);
-
- if (!s)
-s = "";
-
- openvpn_snprintf (numstr, sizeof (numstr), "%d", va_arg
(arglist, int));
-
- {
-const size_t len = strlen(s) + strlen(numstr) + 2;
-char *combined = (char *) malloc (len);
-check_malloc_return (combined);
-
-strcpy (combined, s);
-strcat (combined, "/");
-strcat (combined, numstr);
-argv_append (a, combined);
- }
-}
- else if (!strcmp (term, "%s%sc"))
-{
- char *s1 = va_arg (arglist, char *);
- char *s2 = va_arg (arglist, char *);
- char *comb
[Openvpn-devel] [PATCH 3/7] remove unused system_str from struct argv
Signed-off-by: Heiko Hund <heiko.h...@sophos.com>
---
src/openvpn/argv.c | 86 --
src/openvpn/argv.h | 1 -
2 files changed, 87 deletions(-)
diff --git a/src/openvpn/argv.c b/src/openvpn/argv.c
index 89c9b14..a279a40 100644
--- a/src/openvpn/argv.c
+++ b/src/openvpn/argv.c
@@ -45,7 +45,6 @@ argv_init (struct argv *a)
a->capacity = 0;
a->argc = 0;
a->argv = NULL;
- a->system_str = NULL;
}
struct argv
@@ -63,7 +62,6 @@ argv_reset (struct argv *a)
for (i = 0; i < a->argc; ++i)
free (a->argv[i]);
free (a->argv);
- free (a->system_str);
argv_init (a);
}
@@ -98,65 +96,6 @@ argv_append (struct argv *a, char *str) /* str must have
been malloced or be NUL
a->argv[a->argc++] = str;
}
-static void
-argv_system_str_append (struct argv *a, const char *str, const bool enquote)
-{
- if (str)
-{
- char *newstr;
-
- /* compute length of new system_str */
- size_t l = strlen (str) + 1; /* space for new string plus trailing '\0'
*/
- if (a->system_str)
-l += strlen (a->system_str) + 1; /* space for existing string + space
(" ") separator */
- if (enquote)
-l += 2; /* space for two quotes */
-
- /* build new system_str */
- newstr = (char *) malloc (l);
- newstr[0] = '\0';
- check_malloc_return (newstr);
- if (a->system_str)
-{
- strcpy (newstr, a->system_str);
- strcat (newstr, " ");
-}
- if (enquote)
-strcat (newstr, "\"");
- strcat (newstr, str);
- if (enquote)
-strcat (newstr, "\"");
- free (a->system_str);
- a->system_str = newstr;
-}
-}
-
-static char *
-argv_extract_cmd_name (const char *path)
-{
- char *ret = NULL;
- if (path)
-{
- char *path_cp = string_alloc(path, NULL); /* POSIX basename()
implementaions may modify its arguments */
- const char *bn = basename (path_cp);
- if (bn)
-{
- char *dot = NULL;
- ret = string_alloc (bn, NULL);
- dot = strrchr (ret, '.');
- if (dot)
-*dot = '\0';
- free(path_cp);
- if (ret[0] == '\0')
-{
- free(ret);
- ret = NULL;
-}
-}
-}
- return ret;
-}
-
static struct argv
argv_clone (const struct argv *a, const size_t headroom)
{
@@ -170,7 +109,6 @@ argv_clone (const struct argv *a, const size_t headroom)
{
for (i = 0; i < a->argc; ++i)
argv_append (, string_alloc (a->argv[i], NULL));
- r.system_str = string_alloc (a->system_str, NULL);
}
return r;
}
@@ -179,17 +117,8 @@ struct argv
argv_insert_head (const struct argv *a, const char *head)
{
struct argv r;
- char *s;
-
r = argv_clone (a, 1);
r.argv[0] = string_alloc (head, NULL);
- s = r.system_str;
- r.system_str = string_alloc (head, NULL);
- if (s)
-{
- argv_system_str_append (, s, false);
- free (s);
-}
return r;
}
@@ -285,7 +214,6 @@ argv_printf_arglist (struct argv *a, const char *format,
va_list arglist)
if (!s)
s = "";
argv_append (a, string_alloc (s, NULL));
- argv_system_str_append (a, s, true);
}
else if (!strcmp (term, "%sc"))
{
@@ -304,13 +232,10 @@ argv_printf_arglist (struct argv *a, const char *format,
va_list arglist)
}
else
argv_append (a, string_alloc (s, NULL));
-
- argv_system_str_append (a, s, false);
}
else
{
argv_append (a, string_alloc ("", NULL));
- argv_system_str_append (a, "echo", false);
}
}
else if (!strcmp (term, "%d"))
@@ -318,14 +243,12 @@ argv_printf_arglist (struct argv *a, const char *format,
va_list arglist)
char numstr[64];
openvpn_snprintf (numstr, sizeof (numstr), "%d", va_arg
(arglist, int));
argv_append (a, string_alloc (numstr, NULL));
- argv_system_str_append (a, numstr, false);
}
else if (!strcmp (term, "%u"))
{
char numstr[64];
openvpn_snprintf (numstr, sizeof (numstr), "%u", va_arg
(arglist, unsigned int));
argv_append (a, string_alloc (numstr, NULL));
- argv_system_str_append (a, numstr, false);
}
else if (!strcmp (term, "%s/%d"))
{
@@ -346,7 +269,6 @@ argv_printf_arglist (struct argv *a, const char *format,
va_list arglist)
strcat (combined, "/");
strcat (combined, numstr);
[Openvpn-devel] [PATCH 6/7] argv: do fewer memory re-allocations
Prevent the re-allocations of memory when the internal argv grows
beyond 2 and 4 arguments by initially allocating argv to hold up to
7 (+ trailing NULL) pointers.
While at it rename argv_reset to argv_free to actually express
what's going on. Redo the argv_reset functionality so that it can
be used to actually reset the argv without re-allocation.
Signed-off-by: Heiko Hund <heiko.h...@sophos.com>
---
src/openvpn/argv.c | 45
src/openvpn/argv.h | 2 +-
src/openvpn/console_systemd.c| 2 +-
src/openvpn/init.c | 2 +-
src/openvpn/lladdr.c | 2 +-
src/openvpn/misc.c | 4 ++--
src/openvpn/multi.c | 10
src/openvpn/options.c| 2 +-
src/openvpn/plugin.c | 2 +-
src/openvpn/route.c | 8 +++
src/openvpn/socket.c | 4 ++--
src/openvpn/ssl_verify.c | 6 ++---
src/openvpn/tun.c| 23 --
tests/unit_tests/openvpn/test_argv.c | 41 +---
14 files changed, 85 insertions(+), 68 deletions(-)
diff --git a/src/openvpn/argv.c b/src/openvpn/argv.c
index dcfbd76..664785b 100644
--- a/src/openvpn/argv.c
+++ b/src/openvpn/argv.c
@@ -41,11 +41,28 @@
#include "options.h"
static void
+argv_extend (struct argv *a, const size_t newcap)
+{
+ if (newcap > a->capacity)
+{
+ char **newargv;
+ size_t i;
+ ALLOC_ARRAY_CLEAR (newargv, char *, newcap);
+ for (i = 0; i < a->argc; ++i)
+newargv[i] = a->argv[i];
+ free (a->argv);
+ a->argv = newargv;
+ a->capacity = newcap;
+}
+}
+
+static void
argv_init (struct argv *a)
{
a->capacity = 0;
a->argc = 0;
a->argv = NULL;
+ argv_extend (a, 8);
}
struct argv
@@ -57,29 +74,24 @@ argv_new (void)
}
void
-argv_reset (struct argv *a)
+argv_free (struct argv *a)
{
size_t i;
for (i = 0; i < a->argc; ++i)
free (a->argv[i]);
free (a->argv);
- argv_init (a);
}
static void
-argv_extend (struct argv *a, const size_t newcap)
+argv_reset (struct argv *a)
{
- if (newcap > a->capacity)
+ size_t i;
+ for (i = 0; i < a->argc; ++i)
{
- char **newargv;
- size_t i;
- ALLOC_ARRAY_CLEAR (newargv, char *, newcap);
- for (i = 0; i < a->argc; ++i)
-newargv[i] = a->argv[i];
- free (a->argv);
- a->argv = newargv;
- a->capacity = newcap;
+ free (a->argv[i]);
+ a->argv[i] = NULL;
}
+ a->argc = 0;
}
static void
@@ -126,10 +138,7 @@ argv_insert_head (const struct argv *a, const char *head)
const char *
argv_str (const struct argv *a, struct gc_arena *gc, const unsigned int flags)
{
- if (a->argv)
-return print_argv ((const char **)a->argv, gc, flags);
- else
-return "";
+ return print_argv ((const char **)a->argv, gc, flags);
}
void
@@ -191,8 +200,6 @@ argv_printf_arglist (struct argv *a, const char *format,
va_list arglist)
va_list tmplist;
int len;
- argv_extend (a, 1); /* ensure trailing NULL */
-
argc = a->argc;
f = argv_prep_format (format, delim[0], , );
if (f == NULL)
@@ -221,7 +228,6 @@ argv_printf_arglist (struct argv *a, const char *format,
va_list arglist)
{
/* Someone snuck in a \035, fail gracefully */
argv_reset (a);
- argv_extend (a, 1); /* ensure trailing NULL */
goto out;
}
@@ -263,7 +269,6 @@ argv_parse_cmd (struct argv *a, const char *s)
struct gc_arena gc = gc_new ();
argv_reset (a);
- argv_extend (a, 1); /* ensure trailing NULL */
nparms = parse_line (s, parms, MAX_PARMS, "SCRIPT-ARGV", 0,
D_ARGV_PARSE_CMD, );
if (nparms)
diff --git a/src/openvpn/argv.h b/src/openvpn/argv.h
index ff3933e..99fd7ad 100644
--- a/src/openvpn/argv.h
+++ b/src/openvpn/argv.h
@@ -40,7 +40,7 @@ struct argv {
};
struct argv argv_new (void);
-void argv_reset (struct argv *a);
+void argv_free (struct argv *a);
const char *argv_str (const struct argv *a, struct gc_arena *gc, const
unsigned int flags);
struct argv argv_insert_head (const struct argv *a, const char *head);
void argv_msg (const int msglev, const struct argv *a);
diff --git a/src/openvpn/console_systemd.c b/src/openvpn/console_systemd.c
index 8a953c9..c8edf67 100644
--- a/src/openvpn/console_systemd.c
+++ b/src/openvpn/console_systemd.c
@@ -84,7 +84,7 @@ get_console_input_systemd (const char *prompt, const bool
echo, char *input, con
}
close (std_out);
-argv_reset ();
+argv_free ();
return ret;
}
diff --git a/src/openvpn/init.c b/src/openvpn/init.c
index 56140b3..ab49262 100644
--- a/src/openvpn/init.c
+++ b/src/openvpn/init.c
@@ -1364,7 +1364,7 @@ do_route (const struct options *options,
setenv_str (es, "script_
[Openvpn-devel] [PATCH 2/7] Remove unused and unecessary argv interfaces
Signed-off-by: Heiko Hund <heiko.h...@sophos.com>
---
src/openvpn/argv.c| 53 +++
src/openvpn/argv.h| 6 -
src/openvpn/console_systemd.c | 3 +--
src/openvpn/route.c | 12 --
src/openvpn/tun.c | 42 +++---
5 files changed, 41 insertions(+), 75 deletions(-)
diff --git a/src/openvpn/argv.c b/src/openvpn/argv.c
index 60534ac..89c9b14 100644
--- a/src/openvpn/argv.c
+++ b/src/openvpn/argv.c
@@ -39,7 +39,7 @@
#include "argv.h"
#include "options.h"
-void
+static void
argv_init (struct argv *a)
{
a->capacity = 0;
@@ -157,12 +157,6 @@ argv_extract_cmd_name (const char *path)
return ret;
}
-const char *
-argv_system_str (const struct argv *a)
-{
- return a->system_str;
-}
-
static struct argv
argv_clone (const struct argv *a, const size_t headroom)
{
@@ -199,7 +193,7 @@ argv_insert_head (const struct argv *a, const char *head)
return r;
}
-char *
+static char *
argv_term (const char **f)
{
const char *p = *f;
@@ -272,33 +266,13 @@ argv_msg_prefix (const int msglev, const struct argv *a,
const char *prefix)
gc_free ();
}
-void
-argv_printf (struct argv *a, const char *format, ...)
-{
- va_list arglist;
- va_start (arglist, format);
- argv_printf_arglist (a, format, 0, arglist);
- va_end (arglist);
- }
-
-void
-argv_printf_cat (struct argv *a, const char *format, ...)
-{
- va_list arglist;
- va_start (arglist, format);
- argv_printf_arglist (a, format, APA_CAT, arglist);
- va_end (arglist);
-}
-
-void
-argv_printf_arglist (struct argv *a, const char *format, const unsigned int
flags, va_list arglist)
+static void
+argv_printf_arglist (struct argv *a, const char *format, va_list arglist)
{
struct gc_arena gc = gc_new ();
char *term;
const char *f = format;
- if (!(flags & APA_CAT))
-argv_reset (a);
argv_extend (a, 1); /* ensure trailing NULL */
while ((term = argv_term ()) != NULL)
@@ -410,3 +384,22 @@ argv_printf_arglist (struct argv *a, const char *format,
const unsigned int flag
gc_free ();
}
+void
+argv_printf (struct argv *a, const char *format, ...)
+{
+ va_list arglist;
+ argv_reset (a);
+ va_start (arglist, format);
+ argv_printf_arglist (a, format, arglist);
+ va_end (arglist);
+ }
+
+void
+argv_printf_cat (struct argv *a, const char *format, ...)
+{
+ va_list arglist;
+ va_start (arglist, format);
+ argv_printf_arglist (a, format, arglist);
+ va_end (arglist);
+}
+
diff --git a/src/openvpn/argv.h b/src/openvpn/argv.h
index df658a0..5f8e21c 100644
--- a/src/openvpn/argv.h
+++ b/src/openvpn/argv.h
@@ -40,18 +40,12 @@ struct argv {
char *system_str;
};
-void argv_init (struct argv *a);
struct argv argv_new (void);
void argv_reset (struct argv *a);
-char *argv_term (const char **f);
const char *argv_str (const struct argv *a, struct gc_arena *gc, const
unsigned int flags);
struct argv argv_insert_head (const struct argv *a, const char *head);
void argv_msg (const int msglev, const struct argv *a);
void argv_msg_prefix (const int msglev, const struct argv *a, const char
*prefix);
-const char *argv_system_str (const struct argv *a);
-
-#define APA_CAT (1<<0) /* concatentate onto existing struct argv list */
-void argv_printf_arglist (struct argv *a, const char *format, const unsigned
int flags, va_list arglist);
void argv_printf (struct argv *a, const char *format, ...)
#ifdef __GNUC__
diff --git a/src/openvpn/console_systemd.c b/src/openvpn/console_systemd.c
index 9cd7575..8a953c9 100644
--- a/src/openvpn/console_systemd.c
+++ b/src/openvpn/console_systemd.c
@@ -60,9 +60,8 @@ get_console_input_systemd (const char *prompt, const bool
echo, char *input, con
{
int std_out;
bool ret = false;
-struct argv argv;
+struct argv argv = argv_new ();
-argv_init ();
argv_printf (, SYSTEMD_ASK_PASSWORD_PATH);
#ifdef SYSTEMD_NEWER_THAN_216
/* the --echo support arrived in upstream systemd 217 */
diff --git a/src/openvpn/route.c b/src/openvpn/route.c
index 96d4e11..98bb228 100644
--- a/src/openvpn/route.c
+++ b/src/openvpn/route.c
@@ -1395,7 +1395,7 @@ add_route (struct route_ipv4 *r,
const struct env_set *es)
{
struct gc_arena gc;
- struct argv argv;
+ struct argv argv = argv_new ();
const char *network;
const char *netmask;
const char *gateway;
@@ -1406,7 +1406,6 @@ add_route (struct route_ipv4 *r,
return;
gc_init ();
- argv_init ();
network = print_in_addr_t (r->network, 0, );
netmask = print_in_addr_t (r->netmask, 0, );
@@ -1671,7 +1670,7 @@ void
add_route_ipv6 (struct route_ipv6 *r6, const struct tuntap *tt, unsigned int
flags, const struct env_set *es)
{
struct gc_arena gc;
- struct argv argv;
+ struct argv argv = argv_new ();
const char *network;
const char *gateway;
@@ -1693,7 +1692,6 @@ add_route_ipv6 (struct route_ipv6 *r6, const struct
t
[Openvpn-devel] [PATCH 1/7] put argv_* functions into own file, add unit tests
misc.c is too crowded with different things to perform any
sane unit testing due to its dependencies. So, in order to re-write
the #ifdef'ed tests for the argv_* family of functions into unit
tests I moved them into a dedicated file.
Signed-off-by: Heiko Hund <heiko.h...@sophos.com>
---
configure.ac | 1 +
src/openvpn/Makefile.am | 1 +
src/openvpn/argv.c | 412 +++
src/openvpn/argv.h | 76 ++
src/openvpn/misc.c | 459 ---
src/openvpn/misc.h | 48 +---
tests/unit_tests/Makefile.am | 2 +-
tests/unit_tests/openvpn/Makefile.am | 15 ++
tests/unit_tests/openvpn/test_argv.c | 194 +++
9 files changed, 701 insertions(+), 507 deletions(-)
create mode 100644 src/openvpn/argv.c
create mode 100644 src/openvpn/argv.h
create mode 100644 tests/unit_tests/openvpn/Makefile.am
create mode 100644 tests/unit_tests/openvpn/test_argv.c
diff --git a/configure.ac b/configure.ac
index 357ba29..a42257a 100644
--- a/configure.ac
+++ b/configure.ac
@@ -1273,6 +1273,7 @@ AC_CONFIG_FILES([
tests/unit_tests/plugins/Makefile
tests/unit_tests/plugins/auth-pam/Makefile
tests/unit_tests/example_test/Makefile
+tests/unit_tests/openvpn/Makefile
vendor/Makefile
sample/Makefile
doc/Makefile
diff --git a/src/openvpn/Makefile.am b/src/openvpn/Makefile.am
index a306726..12b9ebf 100644
--- a/src/openvpn/Makefile.am
+++ b/src/openvpn/Makefile.am
@@ -36,6 +36,7 @@ endif
sbin_PROGRAMS = openvpn
openvpn_SOURCES = \
+ argv.c argv.h \
base64.c base64.h \
basic.h \
buffer.c buffer.h \
diff --git a/src/openvpn/argv.c b/src/openvpn/argv.c
new file mode 100644
index 000..60534ac
--- /dev/null
+++ b/src/openvpn/argv.c
@@ -0,0 +1,412 @@
+/*
+ * OpenVPN -- An application to securely tunnel IP networks
+ * over a single TCP/UDP port, with support for SSL/TLS-based
+ * session authentication and key exchange,
+ * packet encryption, packet authentication, and
+ * packet compression.
+ *
+ * Copyright (C) 2002-2010 OpenVPN Technologies, Inc. <sa...@openvpn.net>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2
+ * as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program (see the file COPYING included with this
+ * distribution); if not, write to the Free Software Foundation, Inc.,
+ * 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
+ *
+ *
+ * A printf-like function (that only recognizes a subset of standard printf
+ * format operators) that prints arguments to an argv list instead
+ * of a standard string. This is used to build up argv arrays for passing
+ * to execve.
+ */
+
+#ifdef HAVE_CONFIG_H
+#include "config.h"
+#elif defined(_MSC_VER)
+#include "config-msvc.h"
+#endif
+
+#include "syshead.h"
+
+#include "argv.h"
+#include "options.h"
+
+void
+argv_init (struct argv *a)
+{
+ a->capacity = 0;
+ a->argc = 0;
+ a->argv = NULL;
+ a->system_str = NULL;
+}
+
+struct argv
+argv_new (void)
+{
+ struct argv ret;
+ argv_init ();
+ return ret;
+}
+
+void
+argv_reset (struct argv *a)
+{
+ size_t i;
+ for (i = 0; i < a->argc; ++i)
+free (a->argv[i]);
+ free (a->argv);
+ free (a->system_str);
+ argv_init (a);
+}
+
+static void
+argv_extend (struct argv *a, const size_t newcap)
+{
+ if (newcap > a->capacity)
+{
+ char **newargv;
+ size_t i;
+ ALLOC_ARRAY_CLEAR (newargv, char *, newcap);
+ for (i = 0; i < a->argc; ++i)
+newargv[i] = a->argv[i];
+ free (a->argv);
+ a->argv = newargv;
+ a->capacity = newcap;
+}
+}
+
+static void
+argv_grow (struct argv *a, const size_t add)
+{
+ const size_t newargc = a->argc + add + 1;
+ ASSERT (newargc > a->argc);
+ argv_extend (a, adjust_power_of_2 (newargc));
+}
+
+static void
+argv_append (struct argv *a, char *str) /* str must have been malloced or be
NULL */
+{
+ argv_grow (a, 1);
+ a->argv[a->argc++] = str;
+}
+
+static void
+argv_system_str_append (struct argv *a, const char *str, const bool enquote)
+{
+ if (str)
+{
+ char *newstr;
+
+ /* compute length of new system_str */
+ size_t l = strlen (str) + 1; /* space for new string plus trailing '\0'
*/
+ if (a->system_str)
+l += strlen
Re: [Openvpn-devel] [PATCH] Windows: do_ifconfig() after open_tun()
On Sonntag, 9. Oktober 2016 17:25:50 CEST Gert Doering wrote:
> diff --git a/src/openvpn/tun.c b/src/openvpn/tun.c
> index 4a11d10..1250547 100644
> --- a/src/openvpn/tun.c
> +++ b/src/openvpn/tun.c
> @@ -1373,11 +1373,13 @@ do_ifconfig (struct tuntap *tt,
> else
> {
> /* example: netsh interface ipv6 set address interface=42
> 2001:608:8003::d store=active */ +char iface[64];
> + openvpn_snprintf(iface, sizeof(iface), "interface=%lu",
> tt->adapter_index ); argv_printf (,
> - "%s%sc interface ipv6 set address interface=%lu %s
> store=active",
> + "%s%sc interface ipv6 set address %s %s store=active",
>get_win_sys_path(),
>NETSH_PATH_SUFFIX,
> - tt->adapter_index,
> + iface,
>ifconfig_ipv6_local );
> netsh_command (, 4, M_FATAL);
> }
>
> I've done a "v2" of Heiko's patch with that change included, which I'll
> append below. Samuli, could you build us a windows installer with master
> plus that patch, for "windows early alpha testing"? If that change
> works for people, we are very close to 2.4_alpha1...
ACK for the partial rollback.
Shakes fist at argv_printf(). =)
Thanks Gert
--
Check out the vibrant tech community on one of the world's most
engaging tech sites, SlashDot.org! http://sdm.link/slashdot
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel
Re: [Openvpn-devel] [PATCH] Windows: do_ifconfig() after open_tun()
On Freitag, 24. Juni 2016 19:37:13 CEST Gert Doering wrote: > This patch changes the order to IFCONFIG_AFTER_TUN_OPEN, moves some > initialization code to init_tun, where it belongs, and removes duplicate > code that is now no longer needed. Yeah, I was skeptic at first, too. But it seems to work. That said, I've only tested with the interactive service in place. It's the only option how to run Openvpn in a sane manner on Windows anyways. =) Cheers Heiko
[Openvpn-devel] [PATCH] Windows: do_ifconfig() after open_tun()
When you had multiple TAP adapters and IPv6 configured you got an error
message about "you must also specify --dev-node" and openvpn exited.
Very inconvenient especially since this is only due to the fact that
Windows tries to set the adapter address before it is opened; for no
good reason.
This patch changes the order to IFCONFIG_AFTER_TUN_OPEN, moves some
initialization code to init_tun, where it belongs, and removes duplicate
code that is now no longer needed.
Signed-off-by: Heiko Hund <heiko.h...@sophos.com>
---
src/openvpn/tun.c | 56 ++-
src/openvpn/tun.h | 2 +-
2 files changed, 19 insertions(+), 39 deletions(-)
diff --git a/src/openvpn/tun.c b/src/openvpn/tun.c
index b7a29f7..4a11d10 100644
--- a/src/openvpn/tun.c
+++ b/src/openvpn/tun.c
@@ -611,6 +611,21 @@ init_tun (const char *dev, /* --dev option */
tt->broadcast = generate_ifconfig_broadcast_addr (tt->local,
tt->remote_netmask);
}
+#ifdef WIN32
+ /*
+ * Make sure that both ifconfig addresses are part of the
+ * same .252 subnet.
+ */
+ if (tun)
+{
+ verify_255_255_255_252 (tt->local, tt->remote_netmask);
+ tt->adapter_netmask = ~3;
+}
+ else
+{
+ tt->adapter_netmask = tt->remote_netmask;
+}
+#endif
tt->did_ifconfig_setup = true;
}
@@ -1327,19 +1342,7 @@ do_ifconfig (struct tuntap *tt,
}
#elif defined (WIN32)
{
- /*
-* Make sure that both ifconfig addresses are part of the
-* same .252 subnet.
-*/
- if (tun)
- {
- verify_255_255_255_252 (tt->local, tt->remote_netmask);
- tt->adapter_netmask = ~3;
- }
- else
- {
- tt->adapter_netmask = tt->remote_netmask;
- }
+ASSERT (actual != NULL);
switch (tt->options.ip_win32_type)
{
@@ -1350,9 +1353,6 @@ do_ifconfig (struct tuntap *tt,
print_in_addr_t (tt->adapter_netmask, 0, ));
break;
case IPW32_SET_NETSH:
- if (!strcmp (actual, "NULL"))
- msg (M_FATAL, "Error: When using --ip-win32 netsh, if you have
more than one TAP-Windows adapter, you must also specify --dev-node");
-
netsh_ifconfig (>options,
actual,
tt->local,
@@ -1366,25 +1366,6 @@ do_ifconfig (struct tuntap *tt,
if ( do_ipv6 )
{
- char * saved_actual;
- char iface[64];
- DWORD idx;
-
- if (!strcmp (actual, "NULL"))
- msg (M_FATAL, "Error: When using --tun-ipv6, if you have more than
one TAP-Windows adapter, you must also specify --dev-node");
-
- idx = get_adapter_index_flexible(actual);
- openvpn_snprintf(iface, sizeof(iface), "interface=%lu", idx);
-
- /* on windows, OpenVPN does ifconfig first, open_tun later, so
-* tt->actual_name might not yet be initialized, but routing code
-* needs to know interface name - point to "actual", restore later
-*/
- saved_actual = tt->actual_name;
- tt->actual_name = (char*) actual;
- /* we use adapter_index in add_route_ipv6 */
- tt->adapter_index = idx;
-
if (tt->options.msg_channel)
{
do_address_service (true, AF_INET6, tt);
@@ -1393,17 +1374,16 @@ do_ifconfig (struct tuntap *tt,
{
/* example: netsh interface ipv6 set address interface=42
2001:608:8003::d store=active */
argv_printf (,
-"%s%sc interface ipv6 set address %s %s store=active",
+"%s%sc interface ipv6 set address interface=%lu %s
store=active",
get_win_sys_path(),
NETSH_PATH_SUFFIX,
-iface,
+tt->adapter_index,
ifconfig_ipv6_local );
netsh_command (, 4, M_FATAL);
}
/* explicit route needed */
add_route_connected_v6_net(tt, es);
- tt->actual_name = saved_actual;
}
#else
msg (M_FATAL, "Sorry, but I don't know how to do 'ifconfig' commands on
this operating system. You should ifconfig your TUN/TAP device manually or use
an --up script.");
diff --git a/src/openvpn/tun.h b/src/openvpn/tun.h
index 4e93a3f..1ccf378 100644
--- a/src/openvpn/tun.h
+++ b/src/openvpn/tun.h
@@ -301,7 +301,7 @@ ifconfig_order(void)
#elif defined(TARGET_NETBSD)
return IFCONFIG_AFTER_TUN_OPEN;
#elif defined(WIN32)
- return IFCONFIG_BEFORE_TUN_OPEN;
+ return IFCONFIG_AFTER_TUN_OPEN;
#elif defined(TARGET_ANDROID)
return IFCONFIG_BEFORE_TUN_OPEN;
#else
--
2.7.4
Re: [Openvpn-devel] [PATCH v3] interactive service v3
On Monday 1 February 2016 11:34:47 Selva Nair wrote: > (1) the config file should reside in some pre-defined location(s) > controlled by, say, a registry key that only an admin user can change > (2) only a limited set of "safe" options may be allowed on the command line 1) can be implemented by only accepting relative filenames for the --config parameter and appending it to the config_dir from the registry. 2) has been talked about but we decided that we go with what we have and implement improvements when the code has been integarted. Currently the goal of the service is not so much to prevent a sophisticated person from setting routes. It's rather to enable ppl. to use openvpn without hacks, while providing a long term safe path of doing so. > The msg_channel handle is passed on to openvpn.exe as a command line > option, so it cannot trust it (could be that of a named pipe opened by a > rogue process). This looks insecure such a pipe server could > impersonate the user. Don't get it. Openvpn doesn't send or receive anything critical to/from the channel. It is just used to send requests for a set of predefined operations that need elevated privileges. If the passer of the channel doesn't have elevated rights as well it could fool openvpn to believe that the operation was successful while it wasn't. I don't think that's critical. > A more serious problem is related to the the service requiring connections > from UI with impersonation allowed. Again, an unprivileged process > pretending to be the service could escalate privileges if an OpenVPNGUI > instance running as admin connects to it. This looks easy to exploit. > > Possible mitigation: > (i) The GUI should not connect to the interactive service if running with > elevated privileges > (ii) openvpn.exe should not honor the --msg-channel option if running with > elevated privileges Yeah. The point is no to run the GUI as admin anymore in the first place. So we could just do (ii) and be sure that this is circumvented. This is only exploitable when a user can run the GUI as admin anyways, so there's no real need for the detour through service and openvpn. So, it's basically a safeguard for legacy installations who worked around the route issue by granting the GUI higher privileges. Good catch. > Other comments I have are more specific to code snippets and of minor > consequence. Will send them after compile/test runs. Yeah, I'll be in the meeting but probably (too) late. Cheers Heiko
Re: [Openvpn-devel] [PATCH v3] interactive service v3
Hi Sorry was too busy so far to catch up reading with this thread. On Tuesday 26 January 2016 22:07:18 Samuli Seppänen wrote: > One question, primarily to Heiko... does the interactive service solve > the use-case where the administrator/user wants to have persistent > connections that come up on boot and are not closed or managed in any > way in the meantime? In this use-case no user interaction is wanted, > needed or expected. As Gert put it correctly, the interactive service is just an addon service for the interactive use case (currently a GUI). However if I understood you guys correctly the main feature of the other new service (v2) is that it works with Windows 10 and that you can select which connections you start/stop. Well that same thing could be achieved with the interactive service. All you'd need is a little helper program that starts/stops a connection talking to the interactive service. Mainly the parts of the GUI that tell the iservice to start a connection. The service itself is spawning a thread for every openvpn tunnel it is signaled to start. I said it could be achieved, because the iservice currently doesn't handle restarts of openvpn. Instead it trusts the GUI to do so if it expects the connection to be up when a openvpn process disappears. One benefit would obviously be the shared code base and that openvpn could be run under an unprivileged account as well. If it makes sense to extend the iservice to handle the automatic use case is a different thing, who will do it yet another one on top. Someone made the point that the automatic service is a corner use case, most ppl will not use it. I agree. Ppl. used it to overcome the routing issue since Vista, but it was never there for this use case anyway and the interactive service solves this problem. So, having the "automatic"/"v2" service as a completely separate service or even project is fine with me. It just has some consequences that have been mentioned already. If you're all fine with these let's do it. Heiko
Re: [Openvpn-devel] [PATCH] extend management interface command "state"
On Wednesday 25 November 2015 16:14:49 Arne Schwabe wrote: > Am 11.11.15 um 16:09 schrieb Heiko Hund: > >and EXITING to show the reason for the disconnect), > > > > - (d) optional TUN/TAP local IP address (shown for ASSIGN_IP > > - and CONNECTED), and > > - (e) optional address of remote server (OpenVPN 2.1 or higher). > > + (d) optional TUN/TAP local IPv4 address > > + (e) optional address of remote server, > > + (f) optional port of remote server, > > + (g) optional local address, > > + (h) optional local port, and > > + (i) optional TUN/TAP local IPv6 address. > > + > > +Fields (e)-(h) are shown for CONNECTED state, > > +(d) and (i) are shown for ASSIGN_IP and CONNECTED states. > > + > > +(e) is available starting from OpenVPN 2.1 > > +(f)-(i) are available starting from OpenVPN 2.4 > > Should we add a note that e,f are the connect time and changed because > of --float? Otherwise ACK from me. I don't know, anyone else? Another option could be to "connect" again after floating if this is not done at the moment. But then I think think that would be outside of the scope of this patch. Heiko Sophos Technology GmbH, Amalienbadstraẞe 41/Bau 52, D-76227 Karlsruhe, Deutschland Tel +49 (0)721 25516 0 Fax +49 (0)721 25516 200 E-Mail i...@sophos.de www.sophos.de Sitz der Gesellschaft: Karlsruhe, Amtsgericht Mannheim HRB 712658 Geschäftsführer: Nicholas Bray, Pino von Kienlin, Wolfgang Hilpert, Jennifer Onslow. Sophos GmbH, Gustav-Stresemann-Ring 1, 65189 Wiesbaden, Deutschland Tel +49 (0) 611 5858-0 Fax +49 (0) 611 5858-1042 E-Mail i...@sophos.de www.sophos.de Sitz der Gesellschaft: Wiesbaden, Amtsgericht Wiesbaden HRB 25915 Geschäftsführer: Nicholas Bray, Wolfgang Hilpert, Pino von Kienlin, Jennifer Onslow
