Skip to contentSkip to conversation

Added "remote-override" config directive.

PATCH In review James Yonan 7 participants 35 messages Mar 3, 2016 – Aug 25, 2016 1 message not in the archive

lore
Share & tools
lore
#0 lore raw

PATCH 1/10 Added "remote-override" config directive.

remote-override <alt-remote> -- replace the hostname in all remote directives with alt-remote.

Merged from OpenVPN 2.1

---
 doc/openvpn.8         | 5 +++++
 src/openvpn/options.c | 8 +++++++-
 src/openvpn/options.h | 2 ++
 3 files changed, 14 insertions(+), 1 deletion(-)
doc/openvpn.8 +5 -0
index 628d877..249100d 100644
@@ -281,6 +281,11 @@ DNS caching.  For example, "foo.bar.gov" would be modified to
 "<random-chars>.foo.bar.gov".
 .\"*********************************************************
 .TP
+.B \-\-remote-override [alt-remote]
+Replace the hostname in all remote directives with
+.B alt-remote.
+.\"*********************************************************
+.TP
 .B <connection>
 Define a client connection
 profile.  Client connection profiles are groups of OpenVPN options that
src/openvpn/options.c +7 -1
index 02def3a..0942a4d 100644
@@ -118,6 +118,7 @@ static const char usage_message[] =
   "--remote host [port] : Remote host name or ip address.\n"
   "--remote-random : If multiple --remote options specified, choose one randomly.\n"
   "--remote-random-hostname : Add a random string to remote DNS name.\n"
+  "--remote-override a : Replace the hostname in all remote directives with a.\n"
   "--mode m        : Major mode, m = 'p2p' (default, point-to-point) or 'server'.\n"
   "--proto p       : Use protocol p for communicating with peer.\n"
   "                  p = udp (default), tcp-server, or tcp-client\n"
@@ -4565,6 +4566,11 @@ add_option (struct options *options,
 	goto err;
     }
 #endif
+  else if (streq (p[0], "remote-override") && p[1])
+    {
+      VERIFY_PERMISSION (OPT_P_GENERAL);
+      options->remote_override = p[1];
+    }
   else if (streq (p[0], "remote") && p[1] && !p[4])
     {
       struct remote_entry re;
@@ -4573,7 +4579,7 @@ add_option (struct options *options,
       re.af=0;
 
       VERIFY_PERMISSION (OPT_P_GENERAL|OPT_P_CONNECTION);
-      re.remote = p[1];
+      re.remote = options->remote_override ? options->remote_override : p[1];
       if (p[2])
 	{
 	  re.remote_port = p[2];
src/openvpn/options.h +2 -0
index 23d3992..e281069 100644
@@ -213,6 +213,8 @@ struct options
 
   struct remote_host_store *rh_store;
 
+  const char *remote_override;
+
   bool remote_random;
   const char *ipchange;
   const char *dev;
Signature · 1 line
1.9.1
#1 lore raw

PATCH 2/10 Added PIP_OPT_MASK for process_ip_header fast exit path.

Define PIP_OPT_MASK to represent all flags of interest to process_ip_header, so that it can have a fast exit path if no flags are set.

Merged from OpenVPN 2.1

---
 src/openvpn/forward.c | 8 +-------
 src/openvpn/forward.h | 4 +++-
 2 files changed, 4 insertions(+), 8 deletions(-)
src/openvpn/forward.c +1 -7
index 4a91f92..ef554fc 100644
@@ -1054,13 +1054,7 @@ process_ip_header (struct context *c, unsigned int flags, struct buffer *buf)
        * The --passtos and --mssfix options require
        * us to examine the IPv4 header.
        */
-
-      if (flags & (PIP_MSSFIX
-#if PASSTOS_CAPABILITY
-	  | PIPV4_PASSTOS
-#endif
-	  | PIPV4_CLIENT_NAT
-	  ))
+      if (flags & PIP_OPT_MASK)
 	{
 	  struct buffer ipbuf = *buf;
 	  if (is_ipv4 (TUNNEL_TYPE (c->c1.tuntap), &ipbuf))
src/openvpn/forward.h +3 -1
index af3b0a6..7debcb1 100644
@@ -249,9 +249,11 @@ bool send_control_channel_string (struct context *c, const char *str, int msglev
 
 #define PIPV4_PASSTOS         (1<<0)
 #define PIP_MSSFIX            (1<<1)         /* v4 and v6 */
-#define PIPV4_OUTGOING        (1<<2)
 #define PIPV4_EXTRACT_DHCP_ROUTER (1<<3)
 #define PIPV4_CLIENT_NAT      (1<<4)
+#define PIP_OPT_MASK          0xFFFF         /* all possible options for */
+                                             /* process_ip_header() */
+#define PIPV4_OUTGOING        (1<<16)
 
 void process_ip_header (struct context *c, unsigned int flags, struct buffer *buf);
 
Signature · 1 line
1.9.1
#2 lore raw

Re: [Openvpn-devel] [PATCH 02/10] Added PIP_OPT_MASK for process_ip_header fast exit path.

Am 03.03.16 um 09:18 schrieb James Yonan:

Quoted 3 lines

Define PIP_OPT_MASK to represent all flags of interest to process_ip_header, so that it can have a fast exit path if no flags are set.

Basically what this patch does is to change the condition to

if (flags)

and if for example PASSTOS_CAPABILITY is not 1, the following path will always be taken:

process_ip_header (c, PIPV4_PASSTOS|PIP_MSSFIX|PIPV4_CLIENT_NAT,
&c->c2.buf);

flags mean that possible passtos, mssfix and client_nat should be applied here.

#if PASSTOS_CAPABILITY
if (!c->options.passtos)

    flags &= ~PIPV4_PASSTOS;

#endif

is not compiled in. So flags is at least PIPV4_PASSTOS

So if (flags & 0xffff) is still true.

So NACK from me butthe code is very confusing...

Arne

Quoted 46 lines

Merged from OpenVPN 2.1

---
 src/openvpn/forward.c | 8 +-------
 src/openvpn/forward.h | 4 +++-
 2 files changed, 4 insertions(+), 8 deletions(-)
src/openvpn/forward.c +1 -7
index 4a91f92..ef554fc 100644
@@ -1054,13 +1054,7 @@ process_ip_header (struct context *c, unsigned int flags, struct buffer *buf)
        * The --passtos and --mssfix options require
        * us to examine the IPv4 header.
        */
-
-      if (flags & (PIP_MSSFIX
-#if PASSTOS_CAPABILITY
-	  | PIPV4_PASSTOS
-#endif
-	  | PIPV4_CLIENT_NAT
-	  ))
+      if (flags & PIP_OPT_MASK)
 	{
 	  struct buffer ipbuf = *buf;
 	  if (is_ipv4 (TUNNEL_TYPE (c->c1.tuntap), &ipbuf))
src/openvpn/forward.h +3 -1
index af3b0a6..7debcb1 100644
@@ -249,9 +249,11 @@ bool send_control_channel_string (struct context *c, const char *str, int msglev
 
 #define PIPV4_PASSTOS         (1<<0)
 #define PIP_MSSFIX            (1<<1)         /* v4 and v6 */
-#define PIPV4_OUTGOING        (1<<2)
 #define PIPV4_EXTRACT_DHCP_ROUTER (1<<3)
 #define PIPV4_CLIENT_NAT      (1<<4)
+#define PIP_OPT_MASK          0xFFFF         /* all possible options for */
+                                             /* process_ip_header() */
+#define PIPV4_OUTGOING        (1<<16)
 
 void process_ip_header (struct context *c, unsigned int flags, struct buffer *buf);
 
#3 lore raw

Re: [Openvpn-devel] [PATCH 02/10] Added PIP_OPT_MASK for process_ip_header fast exit path.

Quoted 22 lines · Arne Schwabe

Am 03.03.16 um 09:18 schrieb James Yonan:

Quoted 3 lines

Define PIP_OPT_MASK to represent all flags of interest to process_ip_header, so that it can have a fast exit path if no flags are set.

Basically what this patch does is to change the condition to

if (flags)

and if for example PASSTOS_CAPABILITY is not 1, the following path will always be taken:

process_ip_header (c, PIPV4_PASSTOS|PIP_MSSFIX|PIPV4_CLIENT_NAT,
&c->c2.buf);

flags mean that possible passtos, mssfix and client_nat should be applied here.

#if PASSTOS_CAPABILITY
   if (!c->options.passtos)
     flags &= ~PIPV4_PASSTOS;

#endif

Quoted 7 lines

is not compiled in. So flags is at least PIPV4_PASSTOS

So if (flags & 0xffff) is still true.

So NACK from me butthe code is very confusing...

Arne

I think what makes this patch confusing is that it's really a patch that facilitates another patch that we've used in the past at OpenVPN Tech. for some custom NAT algs. This patch reduces the footprint of the second patch, making it easier to maintain.

James

Quoted 47 lines
Quoted 47 lines

Merged from OpenVPN 2.1

---
  src/openvpn/forward.c | 8 +-------
  src/openvpn/forward.h | 4 +++-
  2 files changed, 4 insertions(+), 8 deletions(-)
src/openvpn/forward.c +1 -7
index 4a91f92..ef554fc 100644
@@ -1054,13 +1054,7 @@ process_ip_header (struct context *c, unsigned int flags, struct buffer *buf)
         * The --passtos and --mssfix options require
         * us to examine the IPv4 header.
         */
-
-      if (flags & (PIP_MSSFIX
-#if PASSTOS_CAPABILITY
-	  | PIPV4_PASSTOS
-#endif
-	  | PIPV4_CLIENT_NAT
-	  ))
+      if (flags & PIP_OPT_MASK)
  	{
  	  struct buffer ipbuf = *buf;
  	  if (is_ipv4 (TUNNEL_TYPE (c->c1.tuntap), &ipbuf))
src/openvpn/forward.h +3 -1
index af3b0a6..7debcb1 100644
@@ -249,9 +249,11 @@ bool send_control_channel_string (struct context *c, const char *str, int msglev
 
  #define PIPV4_PASSTOS         (1<<0)
  #define PIP_MSSFIX            (1<<1)         /* v4 and v6 */
-#define PIPV4_OUTGOING        (1<<2)
  #define PIPV4_EXTRACT_DHCP_ROUTER (1<<3)
  #define PIPV4_CLIENT_NAT      (1<<4)
+#define PIP_OPT_MASK          0xFFFF         /* all possible options for */
+                                             /* process_ip_header() */
+#define PIPV4_OUTGOING        (1<<16)
 
  void process_ip_header (struct context *c, unsigned int flags, struct buffer *buf);
 
#4 lore raw

Re: [Openvpn-devel] [PATCH 02/10] Added PIP_OPT_MASK for process_ip_header fast exit path.

Am 04.03.16 um 08:29 schrieb James Yonan:

Quoted 27 lines · Arne Schwabe
Quoted 26 lines

Am 03.03.16 um 09:18 schrieb James Yonan:

Quoted 3 lines

Define PIP_OPT_MASK to represent all flags of interest to process_ip_header, so that it can have a fast exit path if no flags are set.

Basically what this patch does is to change the condition to

if (flags)

and if for example PASSTOS_CAPABILITY is not 1, the following path will always be taken:

process_ip_header (c, PIPV4_PASSTOS|PIP_MSSFIX|PIPV4_CLIENT_NAT,
&c->c2.buf);

flags mean that possible passtos, mssfix and client_nat should be applied here.

#if PASSTOS_CAPABILITY
   if (!c->options.passtos)
     flags &= ~PIPV4_PASSTOS;

#endif

is not compiled in. So flags is at least PIPV4_PASSTOS

So if (flags & 0xffff) is still true.

Quoted 9 lines
Quoted 3 lines

So NACK from me butthe code is very confusing...

Arne

I think what makes this patch confusing is that it's really a patch that facilitates another patch that we've used in the past at OpenVPN Tech. for some custom NAT algs. This patch reduces the footprint of the second patch, making it easier to maintain.

Yes. I think I get the intention of the patch. But if this patch should work in OpenVPN now we need to replace

#if PASSTOS_CAPABILITY
if (!c->options.passtos)

    flags &= ~PIPV4_PASSTOS;

#endif

with

#if PASSTOS_CAPABILITY
if (!c->options.passtos)
#endif
flags &= ~PIPV4_PASSTOS;

to have the correct behaviour if PASSTOS_CAPABILITY is missing.

Arne

#5 lore raw

Re: [Openvpn-devel] [PATCH 02/10] Added PIP_OPT_MASK for process_ip_header fast exit path.

Quoted 45 lines · Arne Schwabe

Am 04.03.16 um 08:29 schrieb James Yonan:

Quoted 27 lines · Arne Schwabe
Quoted 26 lines

Am 03.03.16 um 09:18 schrieb James Yonan:

Quoted 3 lines

Define PIP_OPT_MASK to represent all flags of interest to process_ip_header, so that it can have a fast exit path if no flags are set.

Basically what this patch does is to change the condition to

if (flags)

and if for example PASSTOS_CAPABILITY is not 1, the following path will always be taken:

process_ip_header (c, PIPV4_PASSTOS|PIP_MSSFIX|PIPV4_CLIENT_NAT,
&c->c2.buf);

flags mean that possible passtos, mssfix and client_nat should be applied here.

#if PASSTOS_CAPABILITY
    if (!c->options.passtos)
      flags &= ~PIPV4_PASSTOS;

#endif

is not compiled in. So flags is at least PIPV4_PASSTOS

So if (flags & 0xffff) is still true.

Quoted 8 lines
Quoted 3 lines

So NACK from me butthe code is very confusing...

Arne

I think what makes this patch confusing is that it's really a patch that facilitates another patch that we've used in the past at OpenVPN Tech. for some custom NAT algs. This patch reduces the footprint of the second patch, making it easier to maintain.

Yes. I think I get the intention of the patch. But if this patch should work in OpenVPN now we need to replace

#if PASSTOS_CAPABILITY
   if (!c->options.passtos)
     flags &= ~PIPV4_PASSTOS;

#endif

Quoted 8 lines

with

#if PASSTOS_CAPABILITY
   if (!c->options.passtos)

#endif
flags &= ~PIPV4_PASSTOS;

to have the correct behaviour if PASSTOS_CAPABILITY is missing.

Actually, I'm fine to withdraw this patch for now -- it's probably more trouble than it's worth.

James

#6 lore raw

PATCH 3/10 tls_serial_{n} value should be distinguishable as hex or decimal.

In reply to James Yonan

To accomplish this, prepend 'x' before hex serial numbers, so they can be distinguished from decimal serial numbers. For example:

tls_serial_1 = "x4D:9B:7C:94"

is equivalent to:

tls_serial_1 = "1302035604"

Currently, only PolarSSL generates hex serial numbers while OpenSSL returns decimal serial numbers.

RFC 5280, published in 2008, decrees that serial numbers can be up to 20 bytes long, hence it is necessary to support SSL libraries that return the serial number as a hex string.

---
 src/openvpn/ssl_verify_polarssl.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)
src/openvpn/ssl_verify_polarssl.c +4 -3
index a2e6a8e..e87d2e2 100644
@@ -161,11 +161,12 @@ char *
 backend_x509_get_serial_hex (openvpn_x509_cert_t *cert, struct gc_arena *gc)
 {
   char *buf = NULL;
-  size_t len = cert->serial.len * 3 + 1;
+  size_t len = cert->serial.len * 3;
 
-  buf = gc_malloc(len, true, gc);
+  buf = gc_malloc(len+1, true, gc);
 
-  if(x509_serial_gets(buf, len-1, &cert->serial) < 0)
+  buf[0] = 'x';
+  if(x509_serial_gets(buf+1, len, &cert->serial) < 0)
     buf = NULL;
 
   return buf;
Signature · 1 line
1.9.1
#7 lore raw

Re: [Openvpn-devel] [PATCH 03/10] tls_serial_{n} value should be distinguishable as hex or decimal.

Hi,

Quoted 11 lines · James Yonan

To accomplish this, prepend 'x' before hex serial numbers, so they can be distinguished from decimal serial numbers. For example:

tls_serial_1 = "x4D:9B:7C:94"

is equivalent to:

tls_serial_1 = "1302035604"

Currently, only PolarSSL generates hex serial numbers while OpenSSL returns decimal serial numbers.

we have this one in the community tree today:

commit f80a52b09eed8e5e0cad990c56ec99256d6cc2d0 (master) commit 142d4dd2e98317a03ca9827f03fc4643fe922834 (release/2.3)

Author: Steffan Karger <[email protected]...>
Date:   Sun Apr 27 10:49:20 2014 +0200

Make serial env exporting consistent amongst OpenSSL and PolarSSL builds.

This changes the representation of the tls_serial_{n} environment variable from hex to decimal for PolarSSL builds, to match OpenSSL build behaviour.

Because hex representation for serials makes sense too, and to ease transition for PolarSSL users, added tls_serial_hex_{n} that exports the serial in hex represenation for both crypto library backends.

so I think the intention of your patch has been achieved already.

gert

Signature · 4 lines
USENET is *not* the non-clickable part of WWW!
                                                           //www.muc.de/~gert/
Gert Doering - Munich, Germany                             [email protected]...
fax: +49-89-35655025                        [email protected]...
#8 lore raw

PATCH 4/10 Added flags parameter to format_hex_ex.

In reply to James Yonan

We add the flags parameter without changing the signature of the function by repurposing the space_break parameter into space_break_flags where the lower 8 bits are used for the previous space_break parameter and the higher bits are used for flag values.

Added new flag FHE_CAPS that formats the generated hex string in upper case.

---
 src/openvpn/buffer.c | 11 +++++++----
 src/openvpn/buffer.h |  4 +++-
 2 files changed, 10 insertions(+), 5 deletions(-)
src/openvpn/buffer.c +7 -4
index bc67d65..52c6ab9 100644
@@ -435,18 +435,21 @@ gc_transfer (struct gc_arena *dest, struct gc_arena *src)
 
 char *
 format_hex_ex (const uint8_t *data, int size, int maxoutput,
-	       int space_break, const char* separator,
+	       unsigned int space_break_flags, const char* separator,
 	       struct gc_arena *gc)
 {
   struct buffer out = alloc_buf_gc (maxoutput ? maxoutput :
-				    ((size * 2) + (size / space_break) * (int) strlen (separator) + 2),
+				    ((size * 2) + (size / (space_break_flags & FHE_SPACE_BREAK_MASK)) * (int) strlen (separator) + 2),
 				    gc);
   int i;
   for (i = 0; i < size; ++i)
     {
-      if (separator && i && !(i % space_break))
+      if (separator && i && !(i % (space_break_flags & FHE_SPACE_BREAK_MASK)))
 	buf_printf (&out, "%s", separator);
-      buf_printf (&out, "%02x", data[i]);
+      if (space_break_flags & FHE_CAPS)
+	buf_printf (&out, "%02X", data[i]);
+      else
+	buf_printf (&out, "%02x", data[i]);
     }
   buf_catrunc (&out, "[more...]");
   return (char *)out.data;
src/openvpn/buffer.h +3 -1
index 24f52aa..8070439 100644
@@ -403,9 +403,11 @@ bool buf_parse (struct buffer *buf, const int delim, char *line, const int size)
 /*
  * Hex dump -- Output a binary buffer to a hex string and return it.
  */
+#define FHE_SPACE_BREAK_MASK 0xFF /* space_break parameter in lower 8 bits */
+#define FHE_CAPS 0x100            /* output hex in caps */
 char *
 format_hex_ex (const uint8_t *data, int size, int maxoutput,
-	       int space_break, const char* separator,
+	       unsigned int space_break_flags, const char* separator,
 	       struct gc_arena *gc);
 
 static inline char *
Signature · 1 line
1.9.1
#9 lore raw

Re: [Openvpn-devel] [PATCH 04/10] Added flags parameter to format_hex_ex.

Quoted 52 lines · James Yonan

We add the flags parameter without changing the signature of the function by repurposing the space_break parameter into space_break_flags where the lower 8 bits are used for the previous space_break parameter and the higher bits are used for flag values.

Added new flag FHE_CAPS that formats the generated hex string in upper case.

---
 src/openvpn/buffer.c | 11 +++++++----
 src/openvpn/buffer.h |  4 +++-
 2 files changed, 10 insertions(+), 5 deletions(-)
src/openvpn/buffer.c +7 -4
index bc67d65..52c6ab9 100644
@@ -435,18 +435,21 @@ gc_transfer (struct gc_arena *dest, struct gc_arena *src)
 
 char *
 format_hex_ex (const uint8_t *data, int size, int maxoutput,
-              int space_break, const char* separator,
+              unsigned int space_break_flags, const char* separator,
               struct gc_arena *gc)
 {
   struct buffer out = alloc_buf_gc (maxoutput ? maxoutput :
-                                   ((size * 2) + (size / space_break) * (int) strlen (separator) + 2),
+                                   ((size * 2) + (size / (space_break_flags & FHE_SPACE_BREAK_MASK)) * (int) strlen (separator) + 2),
                                    gc);
   int i;
   for (i = 0; i < size; ++i)
     {
-      if (separator && i && !(i % space_break))
+      if (separator && i && !(i % (space_break_flags & FHE_SPACE_BREAK_MASK)))
        buf_printf (&out, "%s", separator);
-      buf_printf (&out, "%02x", data[i]);
+      if (space_break_flags & FHE_CAPS)
+       buf_printf (&out, "%02X", data[i]);
+      else
+       buf_printf (&out, "%02x", data[i]);
     }
   buf_catrunc (&out, "[more...]");
   return (char *)out.data;
src/openvpn/buffer.h +0 -0
index 24f52aa..8070439 100644
@@ -403,9 +403,11 @@ bool buf_parse (struct buffer *buf, const int delim, char *line, const int size)
 /*
  * Hex dump -- Output a binary buffer to a hex string and return it.

src/openvpn/buffer.h bool buf_parse (struct buffer *buf, const int delim, char *line, const int size) · line 413

Quoted 10 lines
src/openvpn/buffer.h +3 -1
  */
+#define FHE_SPACE_BREAK_MASK 0xFF /* space_break parameter in lower 8 bits */
+#define FHE_CAPS 0x100            /* output hex in caps */
 char *
 format_hex_ex (const uint8_t *data, int size, int maxoutput,
-              int space_break, const char* separator,
+              unsigned int space_break_flags, const char* separator,
               struct gc_arena *gc);
 
 static inline char *

ACK

-Steffan

#10 lore raw

[Openvpn-devel] [PATCH applied] Re: Added flags parameter to format_hex_ex.

In reply to James Yonan

Your patch has been applied to the master branch.

commit 0a6a80156ee60705f7856100da7a2a61f018e2a0

Author: James Yonan
Date:   Thu Mar 3 01:19:00 2016 -0700
     Added flags parameter to format_hex_ex.

     Signed-off-by: James Yonan <[email protected]...>
     Acked-by: Steffan Karger <[email protected]...>
     Message-Id: <[email protected]...>
     URL: http://article.gmane.org/gmane.network.openvpn.devel/11275
     Signed-off-by: Gert Doering <[email protected]...>

--
kind regards,

Gert Doering

#11 lore raw

PATCH 5/10 Extended x509-track for OpenSSL to report SHA1 fingerprint.

In reply to James Yonan

For example:

x509-track "+SHA1"

will extract the SHA1 fingerprints for all certs in the peer chain.

This patch is ported from OpenVPN 2.1.

---
 src/openvpn/ssl_verify_openssl.c | 114 +++++++++++++++++++++++++--------------
 1 file changed, 74 insertions(+), 40 deletions(-)
src/openvpn/ssl_verify_openssl.c +74 -40
index d014f9d..cde884b 100644
@@ -305,6 +305,27 @@ err:
 
 #ifdef ENABLE_X509_TRACK
 
+/*
+ * x509-track implementation -- save X509 fields to environment,
+ * using the naming convention:
+ *
+ *  X509_{cert_depth}_{name}={value}
+ *
+ * This function differs from x509_setenv below in the following ways:
+ *
+ * (1) Only explicitly named attributes in xt are saved, per usage
+ *     of "x509-track" program options.
+ * (2) Only the level 0 cert info is saved unless the XT_FULL_CHAIN
+ *     flag is set in xt->flags (corresponds with prepending a '+'
+ *     to the name when specified by "x509-track" program option).
+ * (3) This function supports both X509 subject name fields as
+ *     well as X509 V3 extensions.
+ * (4) This function can return the SHA1 fingerprint of a cert, e.g.
+ *       x509-track "+SHA1"
+ *     will return the SHA1 fingerprint for each certificate in the
+ *     peer chain.
+ */
+
 void
 x509_track_add (const struct x509_track **ll_head, const char *name, int msglevel, struct gc_arena *gc)
 {
@@ -346,58 +367,71 @@ do_setenv_x509 (struct env_set *es, const char *name, char *value, int depth)
 void
 x509_setenv_track (const struct x509_track *xt, struct env_set *es, const int depth, X509 *x509)
 {
+  struct gc_arena gc = gc_new();
   X509_NAME *x509_name = X509_get_subject_name (x509);
   const char nullc = '\0';
-  int i;
 
   while (xt)
     {
       if (depth == 0 || (xt->flags & XT_FULL_CHAIN))
 	{
-	  i = X509_NAME_get_index_by_NID(x509_name, xt->nid, -1);
-	  if (i >= 0)
+	  switch (xt->nid)
 	    {
-	      X509_NAME_ENTRY *ent = X509_NAME_get_entry(x509_name, i);
-	      if (ent)
-		{
-		  ASN1_STRING *val = X509_NAME_ENTRY_get_data (ent);
-		  unsigned char *buf;
-		  buf = (unsigned char *)1; /* bug in OpenSSL 0.9.6b ASN1_STRING_to_UTF8 requires this workaround */
-		  if (ASN1_STRING_to_UTF8 (&buf, val) > 0)
-		    {
-		      do_setenv_x509(es, xt->name, (char *)buf, depth);
-		      OPENSSL_free (buf);
-		    }
-		}
-	    }
-	  else
-	    {
-	      i = X509_get_ext_by_NID(x509, xt->nid, -1);
-	      if (i >= 0)
-		{
-		  X509_EXTENSION *ext = X509_get_ext(x509, i);
-		  if (ext)
-		    {
-		      BIO *bio = BIO_new(BIO_s_mem());
-		      if (bio)
-			{
-			  if (X509V3_EXT_print(bio, ext, 0, 0))
-			    {
-			      if (BIO_write(bio, &nullc, 1) == 1)
-				{
-				  char *str;
-				  BIO_get_mem_data(bio, &str);
-				  do_setenv_x509(es, xt->name, str, depth);
-				}
-			    }
-			  BIO_free(bio);
-			}
-		    }
-		}
+	    case NID_sha1:
+	      {
+		char *sha1_fingerprint = format_hex_ex(x509->sha1_hash, SHA_DIGEST_LENGTH, 0, 1 | FHE_CAPS, ":", &gc);
+		do_setenv_x509(es, xt->name, sha1_fingerprint, depth);
+	      }
+	      break;
+	    default:
+	      {
+		int i = X509_NAME_get_index_by_NID(x509_name, xt->nid, -1);
+		if (i >= 0)
+		  {
+		    X509_NAME_ENTRY *ent = X509_NAME_get_entry(x509_name, i);
+		    if (ent)
+		      {
+			ASN1_STRING *val = X509_NAME_ENTRY_get_data (ent);
+			unsigned char *buf;
+			buf = (unsigned char *)1; /* bug in OpenSSL 0.9.6b ASN1_STRING_to_UTF8 requires this workaround */
+			if (ASN1_STRING_to_UTF8 (&buf, val) > 0)
+			  {
+			    do_setenv_x509(es, xt->name, (char *)buf, depth);
+			    OPENSSL_free (buf);
+			  }
+		      }
+		  }
+		else
+		  {
+		    i = X509_get_ext_by_NID(x509, xt->nid, -1);
+		    if (i >= 0)
+		      {
+			X509_EXTENSION *ext = X509_get_ext(x509, i);
+			if (ext)
+			  {
+			    BIO *bio = BIO_new(BIO_s_mem());
+			    if (bio)
+			      {
+				if (X509V3_EXT_print(bio, ext, 0, 0))
+				  {
+				    if (BIO_write(bio, &nullc, 1) == 1)
+				      {
+					char *str;
+					BIO_get_mem_data(bio, &str);
+					do_setenv_x509(es, xt->name, str, depth);
+				      }
+				  }
+				BIO_free(bio);
+			      }
+			  }
+		      }
+		  }
+	      }
 	    }
 	}
       xt = xt->next;
     }
+  gc_free(&gc);
 }
 #endif
 
Signature · 1 line
1.9.1
#12 lore raw

Re: [Openvpn-devel] [PATCH 05/10] Extended x509-track for OpenSSL to report SHA1 fingerprint.

Hi,

Quoted 1 line · James Yonan

+ char *sha1_fingerprint = format_hex_ex(x509->sha1_hash, SHA_DIGEST_LENGTH, 0, 1 | FHE_CAPS, ":", &gc);

This line could use some wrapping. Perhaps Gert can fix this when applying?

Otherwise, ACK.

-Steffan

#13 lore raw

[Openvpn-devel] [PATCH applied] Re: Extended x509-track for OpenSSL to report SHA1 fingerprint.

In reply to James Yonan

Your patch has been applied to the master branch.

As Steffan has requested, I have wrapped the long "sha1_fingerprint = ..." line to keep line lengths below 80 characters. Purely whitespace change.

commit f6608a15efab027e25553da3026c172dbc3aa73e (master)

Author: James Yonan
Date:   Thu Mar 3 01:19:01 2016 -0700
     Extended x509-track for OpenSSL to report SHA1 fingerprint.

     Signed-off-by: James Yonan <[email protected]...>
     Acked-by: Steffan Karger <[email protected]...>
     Message-Id: <[email protected]...>
     URL: http://article.gmane.org/gmane.network.openvpn.devel/11281
     Signed-off-by: Gert Doering <[email protected]...>

--
kind regards,

Gert Doering

#14 lore raw

PATCH 6/10 PolarSSL x509_get_sha1_hash now returns correct SHA1 fingerprint.

In reply to James Yonan
---
 src/openvpn/ssl_verify_polarssl.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
src/openvpn/ssl_verify_polarssl.c +1 -1
index e87d2e2..9d0d086 100644
@@ -176,7 +176,7 @@ unsigned char *
 x509_get_sha1_hash (x509_crt *cert, struct gc_arena *gc)
 {
   unsigned char *sha1_hash = gc_malloc(SHA_DIGEST_LENGTH, false, gc);
-  sha1(cert->tbs.p, cert->tbs.len, sha1_hash);
+  sha1(cert->raw.p, cert->raw.len, sha1_hash);
   return sha1_hash;
 }
 
Signature · 1 line
1.9.1

A message that is not in the archive (sent off-list or not yet received): [email protected]...

#16 lore raw

Re: [Openvpn-devel] [PATCH 06/10] PolarSSL x509_get_sha1_hash now returns correct SHA1 fingerprint.

Hi list,

I just noticed I sent my previous reply only to James, instead of to the list. See below.

Quoted 36 lines · Steffan Karger

Hi James,

Quoted 10 lines · James Yonan
---
 src/openvpn/ssl_verify_polarssl.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
src/openvpn/ssl_verify_polarssl.c +0 -0
index e87d2e2..9d0d086 100644

src/openvpn/ssl_verify_polarssl.c unsigned char * · line 181

Quoted 7 lines
src/openvpn/ssl_verify_polarssl.c +1 -1
@@ -176,7 +176,7 @@ unsigned char *
 x509_get_sha1_hash (x509_crt *cert, struct gc_arena *gc)
 {
   unsigned char *sha1_hash = gc_malloc(SHA_DIGEST_LENGTH, false, gc);
-  sha1(cert->tbs.p, cert->tbs.len, sha1_hash);
+  sha1(cert->raw.p, cert->raw.len, sha1_hash);
   return sha1_hash;
 }

I agree with the change, but this needs a bit more explanation in the commit msg and a note in Changes.rst, because it changes externally-visible behaviour for PolarSSL/mbedTLS builds (at least for --verify-hash, maybe more?).

It took me a bit of time to figure out *why* this is the correct, so I'll jot down my findings here.

At first, this didn't not look correct to me. A certificate digest is calculated over the TBSCertificate/cert.tbs (TBS = To Be Signed), not the Certificate/cert.raw field. This must be, because the Certificate field contains the TBSCertificate and the signatureValue (which depends on the hash). Also see section 4.1 of RFC5280
(https://tools.ietf.org/html/rfc5280#page-16).

Quoted 7 lines

However, a certificate *fingerprint* (i.e. the output op openssl x509 -fingerprint) is calculated over the (contents of the) Certificate field. And this is what this function should be doing. To make this more clear I suggest renaming this function to x509_get_sha1_fingerprint(), and updating the doxygen.

-Steffan

So, since Gert is looking into applying these, ACK to the code, and attached an updated patch with suggestions for the commit msg and Changes.rst.

-Steffan

From 6f372be44926f555ec9f860746dda8c5e00640f8 Mon Sep 17 00:00:00 2001
From: Steffan Karger <[email protected]>
Date: Mon, 4 Apr 2016 21:59:38 +0200
Subject: [PATCH] PolarSSL x509_get_sha1_hash now returns correct SHA1
fingerprint.

509_get_sha1_hash() is supposed to return the certificate fingerprint, which is the hash of the entire certificate - including the signature - and not just the 'to be signed' data (cert->tbs in polarssl).

This changes externally visible behavior for polarssl builds: it will change the value of the tls_digest_N values exported to the environment for scripts.

v2 Steffan Karger: added commit message and Changes.rst entry.
Code unchanged from v1 by James.

---
 Changes.rst                       | 6 +++++-
 src/openvpn/ssl_verify_polarssl.c | 2 +-
 2 files changed, 6 insertions(+), 2 deletions(-)
Changes.rst +5 -1
index af70d14..cb900dc 100644
@@ -63,7 +63,6 @@ User-visible Changes
   In --static mode connect-timeout specifies the timeout for TCP and
   proxy connection establishment
 
-
 - connect-retry now specifies the maximum number of unsucessfully
   trying all remote/connection entries before exiting.
 
@@ -86,6 +85,11 @@ User-visible Changes
 - Removed --enable-password-save from configure. This option is now
   always enabled.
 
+- PolarSSL builds: changed the tls_digest_N values exported to the script
+  environment to be equal to the ones exported by OpenSSL builds, namely
+  the certificate fingerprint (was the hash of the 'to be signed' data).
+
+
 Maintainer-visible changes
 --------------------------
 - OpenVPN no longer supports building with crypto support, but without TLS
src/openvpn/ssl_verify_polarssl.c +1 -1
index a2e6a8e..bf1a6ec 100644
@@ -175,7 +175,7 @@ unsigned char *
 x509_get_sha1_hash (x509_crt *cert, struct gc_arena *gc)
 {
   unsigned char *sha1_hash = gc_malloc(SHA_DIGEST_LENGTH, false, gc);
-  sha1(cert->tbs.p, cert->tbs.len, sha1_hash);
+  sha1(cert->raw.p, cert->raw.len, sha1_hash);
   return sha1_hash;
 }
 
Signature · 1 line
2.5.0
#17 lore raw

Re: [Openvpn-devel] [PATCH 06/10] PolarSSL x509_get_sha1_hash now returns correct SHA1 fingerprint.

Am 04.04.16 um 23:01 schrieb Steffan Karger:

Quoted 5 lines

Hi list,

I just noticed I sent my previous reply only to James, instead of to the list. See below.

ACK. Also code identical to James and only docu added.

Arne

#18 lore raw

[Openvpn-devel] [PATCH applied] Re: PolarSSL x509_get_sha1_hash now returns correct SHA1 fingerprint.

In reply to Steffan Karger

Your patch has been applied to the master branch.

(Changes.rst had to be manually applied as it conflicted with an intermediate change)

commit dd2fbc26eb7b32325793ae3f7d215f46e881e68c (master)

Author: Steffan Karger
Date:   Mon Apr 4 21:59:38 2016 +0200
     PolarSSL x509_get_sha1_hash now returns correct SHA1 fingerprint.

     Signed-off-by: James Yonan <[email protected]...>
     Signed-off-by: Steffan Karger <[email protected]...>
     Acked-by: Arne Schwabe <[email protected]...>
     Message-Id: <[email protected]...>
     URL: http://article.gmane.org/gmane.network.openvpn.devel/11396
     Signed-off-by: Gert Doering <[email protected]...>

--
kind regards,

Gert Doering

#19 lore raw

PATCH 7/10 Implemented x509-track for PolarSSL.

In reply to James Yonan
---
 src/openvpn/ssl_verify_polarssl.c | 166 ++++++++++++++++++++++++++++++++++++++
 src/openvpn/syshead.h             |   2 +-
 2 files changed, 167 insertions(+), 1 deletion(-)
src/openvpn/ssl_verify_polarssl.c +166 -0
index 9d0d086..ab693d2 100644
@@ -198,6 +198,172 @@ x509_get_subject(x509_crt *cert, struct gc_arena *gc)
   return subject;
 }
 
+#ifdef ENABLE_X509_TRACK
+
+/* these match NID's in OpenSSL crypto/objects/objects.h */
+#define NID_undef			0
+#define NID_sha1                        64
+#define NID_commonName                  13
+#define NID_countryName                 14
+#define NID_localityName                15
+#define NID_stateOrProvinceName         16
+#define NID_organizationName		17
+#define NID_organizationalUnitName      18
+#define NID_pkcs9_emailAddress          48
+
+struct nid_entry {
+  const char *name;
+  int nid;
+};
+
+static const struct nid_entry nid_list[] = {
+  { "SHA1",         NID_sha1 },
+  { "CN",           NID_commonName },
+  { "C",            NID_countryName },
+  { "L",            NID_localityName },
+  { "ST",           NID_stateOrProvinceName },
+  { "O",            NID_organizationName },
+  { "OU",           NID_organizationalUnitName },
+  { "emailAddress", NID_pkcs9_emailAddress },
+  { NULL, 0 }
+};
+
+static int
+name_to_nid(const char *name)
+{
+  const struct nid_entry *e = nid_list;
+  while (e->name)
+    {
+      if (!strcmp(name, e->name))
+	return e->nid;
+      ++e;
+    }
+  return NID_undef;
+}
+
+static void
+do_setenv_x509 (struct env_set *es, const char *name, char *value, int depth)
+{
+  char *name_expand;
+  size_t name_expand_size;
+
+  string_mod (value, CC_ANY, CC_CRLF, '?');
+  msg (D_X509_ATTR, "X509 ATTRIBUTE name='%s' value='%s' depth=%d", name, value, depth);
+  name_expand_size = 64 + strlen (name);
+  name_expand = (char *) malloc (name_expand_size);
+  check_malloc_return (name_expand);
+  openvpn_snprintf (name_expand, name_expand_size, "X509_%d_%s", depth, name);
+  setenv_str (es, name_expand, value);
+  free (name_expand);
+}
+
+static void
+do_setenv_nid_value(struct env_set *es, const struct x509_track *xt, const x509_name *xn,
+		    int depth, struct gc_arena *gc)
+{
+  size_t i;
+  char *val;
+
+  for (i = 0; i < xn->val.len; ++i)
+    if (xn->val.p[i] == '\0') /* error if embedded null in value */
+      return;
+  val = gc_malloc(xn->val.len+1, false, gc);
+  memcpy(val, xn->val.p, xn->val.len);
+  val[xn->val.len] = '\0';
+  do_setenv_x509(es, xt->name, val, depth);
+}
+
+static void
+do_setenv_nid(struct env_set *es, const struct x509_track *xt, const x509_crt *cert,
+	      int depth, struct gc_arena *gc)
+{
+  const x509_name *xn;
+  for (xn = &cert->subject; xn != NULL; xn = xn->next)
+    {
+      switch (xt->nid)
+	{
+	case NID_commonName:
+	  if (OID_CMP(OID_AT_CN, &xn->oid))
+	    do_setenv_nid_value(es, xt, xn, depth, gc);
+	  break;
+	case NID_countryName:
+	  if (OID_CMP(OID_AT_COUNTRY, &xn->oid))
+	    do_setenv_nid_value(es, xt, xn, depth, gc);
+	  break;
+	case NID_localityName:
+	  if (OID_CMP(OID_AT_LOCALITY, &xn->oid))
+	    do_setenv_nid_value(es, xt, xn, depth, gc);
+	  break;
+	case NID_stateOrProvinceName:
+	  if (OID_CMP(OID_AT_STATE, &xn->oid))
+	    do_setenv_nid_value(es, xt, xn, depth, gc);
+	  break;
+	case NID_organizationName:
+	  if (OID_CMP(OID_AT_ORGANIZATION, &xn->oid))
+	    do_setenv_nid_value(es, xt, xn, depth, gc);
+	  break;
+	case NID_organizationalUnitName:
+	  if (OID_CMP(OID_AT_ORG_UNIT, &xn->oid))
+	    do_setenv_nid_value(es, xt, xn, depth, gc);
+	  break;
+	case NID_pkcs9_emailAddress:
+	  if (OID_CMP(OID_PKCS9_EMAIL, &xn->oid))
+	    do_setenv_nid_value(es, xt, xn, depth, gc);
+	  break;
+	}
+    }
+}
+
+void
+x509_track_add (const struct x509_track **ll_head, const char *name, int msglevel, struct gc_arena *gc)
+{
+  struct x509_track *xt;
+  ALLOC_OBJ_CLEAR_GC (xt, struct x509_track, gc);
+  if (*name == '+')
+    {
+      xt->flags |= XT_FULL_CHAIN;
+      ++name;
+    }
+  xt->name = name;
+  xt->nid = name_to_nid(name);
+  if (xt->nid != NID_undef)
+    {
+      xt->next = *ll_head;
+      *ll_head = xt;
+    }
+  else
+    msg(msglevel, "x509_track: no such attribute '%s'", name);
+}
+
+void
+x509_setenv_track (const struct x509_track *xt, struct env_set *es, const int depth, x509_crt *cert)
+{
+  struct gc_arena gc = gc_new();
+  while (xt)
+    {
+      if (depth == 0 || (xt->flags & XT_FULL_CHAIN))
+	{
+	  switch (xt->nid)
+	    {
+	    case NID_sha1:
+	      {
+		unsigned char *sha1_hash = x509_get_sha1_hash(cert, &gc);
+		char *sha1_fingerprint = format_hex_ex(sha1_hash, SHA_DIGEST_LENGTH, 0, 1 | FHE_CAPS, ":", &gc);
+		do_setenv_x509(es, xt->name, sha1_fingerprint, depth);
+	      }
+	      break;
+	    default:
+	      do_setenv_nid(es, xt, cert, depth, &gc);
+	      break;
+	    }
+	}
+      xt = xt->next;
+    }
+  gc_free(&gc);
+}
+
+#endif
+
 /*
  * Save X509 fields to environment, using the naming convention:
  *
src/openvpn/syshead.h +1 -1
index 7e77b6c..25aa69b 100644
@@ -634,7 +634,7 @@ socket_defined (const socket_descriptor_t sd)
 /*
  * Enable x509-track feature?
  */
-#if defined(ENABLE_CRYPTO) && defined (ENABLE_CRYPTO_OPENSSL)
+#if defined(ENABLE_CRYPTO) && (defined(ENABLE_CRYPTO_OPENSSL) || defined(ENABLE_CRYPTO_POLARSSL))
 #define ENABLE_X509_TRACK
 #endif
 
Signature · 1 line
1.9.1
#20 lore raw

Re: [Openvpn-devel] [PATCH 07/10] Implemented x509-track for PolarSSL.

Hi,

This addition is welcome and the code does the job it promises to do, but after reviewing the code I would like to propose a different implementation. The reasons for this are gives as inline replies below. The alternative patch proposal is attached.

Quoted 16 lines · James Yonan
---
 src/openvpn/ssl_verify_polarssl.c | 166 ++++++++++++++++++++++++++++++++++++++
 src/openvpn/syshead.h             |   2 +-
 2 files changed, 167 insertions(+), 1 deletion(-)
src/openvpn/ssl_verify_polarssl.c +2 -0
index 9d0d086..ab693d2 100644
@@ -198,6 +198,172 @@ x509_get_subject(x509_crt *cert, struct gc_arena *gc)
   return subject;
 }
 
+#ifdef ENABLE_X509_TRACK
+

src/openvpn/ssl_verify_polarssl.c x509_get_subject(x509_crt *cert, struct gc_arena *gc) · line 212

Quoted 10 lines
src/openvpn/ssl_verify_polarssl.c +10 -0
+/* these match NID's in OpenSSL crypto/objects/objects.h */
+#define NID_undef                      0
+#define NID_sha1                        64
+#define NID_commonName                  13
+#define NID_countryName                 14
+#define NID_localityName                15
+#define NID_stateOrProvinceName         16
+#define NID_organizationName           17
+#define NID_organizationalUnitName      18
+#define NID_pkcs9_emailAddress          48

Hmm, I don't really like to maintain lists in the source code. If possible, I would prefer an implementation that fits the polarssl approach, rather than wrapping polarssl with openssl-like code.

src/openvpn/syshead.h socket_defined (const socket_descriptor_t sd) · line 637

Quoted 10 lines
src/openvpn/syshead.h +1 -1
index 7e77b6c..25aa69b 100644
@@ -634,7 +634,7 @@ socket_defined (const socket_descriptor_t sd)
 /*
  * Enable x509-track feature?
  */
-#if defined(ENABLE_CRYPTO) && defined (ENABLE_CRYPTO_OPENSSL)
+#if defined(ENABLE_CRYPTO) && (defined(ENABLE_CRYPTO_OPENSSL) || defined(ENABLE_CRYPTO_POLARSSL))

Since both crypto backends now support --x509-track, let's just get rid of this define all together :)

Finally, while reviewing I noticed that the --x509-track code in options.c lives inside #ifdef MANAGEMENT, but there seems to be no valid reason for this. Let's move it outside of this #ifdef.

The attached patch takes all these remarks into account. The upsides of my alternative are less code, and no lists to maintain. The downside is less error reporting. I'm curious to hear what you think of the alternative implementation.

-Steffan

From 2363205265ebc77c1dab740b56239ca6e78f5d11 Mon Sep 17 00:00:00 2001
From: Steffan Karger <[email protected]>
Date: Sat, 5 Mar 2016 17:08:22 +0100
Subject: [PATCH] Implemented x509-track for PolarSSL.

This patch is a variant of the patch to implement x509-track for PolarSSL that was sent to openvpn-devel@ by James Yonan (<[email protected]>). It still uses some of the original code from James, but proposes a different implementation.

This patch does the following things differently:
* Do not introduce NID_* defines that need to be maintained. Instead, just use the short name of the attribute for identification. This has the advantage that we automatically support everything that PolarSSL supports, it is less code and we do not have maintain the list. But the disadvantage is that this approach will not error out when an unknown attribute name is supplied. PolarSSL (at least 1.3, I didn't check 2.x) does not provide the functions required to do that. Instead of erroring out, this implementation will just silently ignore the unknown --x509-track attribute name.
* Remove the ENABLE_X509_TRACK define completely - it depended just on ENABLE_CRYPTO anyway.
* Move the --x509-track option parsing out of ENABLE_MANAGEMENT, since it does not depend on management functionality.

---
 doc/openvpn.8                     |  1 -
 src/openvpn/init.c                |  2 -
 src/openvpn/options.c             | 14 +++---
 src/openvpn/options.h             |  2 -
 src/openvpn/ssl_common.h          |  2 -
 src/openvpn/ssl_verify.c          | 16 ++-----
 src/openvpn/ssl_verify.h          |  6 ---
 src/openvpn/ssl_verify_backend.h  |  4 --
 src/openvpn/ssl_verify_openssl.c  |  3 --
 src/openvpn/ssl_verify_polarssl.c | 90 +++++++++++++++++++++++++++++++++++++++
 src/openvpn/syshead.h             |  7 ---
 11 files changed, 99 insertions(+), 48 deletions(-)
doc/openvpn.8 +0 -1
index decffc7..76b04f6 100644
@@ -5112,7 +5112,6 @@ to save values from full cert chain.  Values will be encoded
 as X509_<depth>_<attribute>=<value>.  Multiple
 .B \-\-x509\-track
 options can be defined to track multiple attributes.
-Not available with PolarSSL.
 .\"*********************************************************
 .TP
 .B \-\-ns\-cert\-type client|server
src/openvpn/init.c +0 -2
index 84fac07..42baf97 100644
@@ -2355,9 +2355,7 @@ do_init_crypto_tls (struct context *c, const unsigned int flags)
   to.auth_user_pass_file = options->auth_user_pass_file;
 #endif
 
-#ifdef ENABLE_X509_TRACK
   to.x509_track = options->x509_track;
-#endif
 
 #if P2MP
 #ifdef ENABLE_CLIENT_CR
src/openvpn/options.c +5 -9
index 57f3dc5..564e706 100644
@@ -612,10 +612,8 @@ static const char usage_message[] =
   "                  of verification.\n"
   "--ns-cert-type t: Require that peer certificate was signed with an explicit\n"
   "                  nsCertType designation t = 'client' | 'server'.\n"
-#ifdef ENABLE_X509_TRACK
   "--x509-track x  : Save peer X509 attribute x in environment for use by\n"
   "                  plugins and management interface.\n"
-#endif
 #if defined(ENABLE_CRYPTO_OPENSSL) && OPENSSL_VERSION_NUMBER >= 0x10001000
   "--keying-material-exporter label len : Save Exported Keying Material (RFC5705)\n"
   "                  of len bytes (min. 16 bytes) using label in environment for use by plugins.\n"
@@ -4337,13 +4335,6 @@ add_option (struct options *options,
       options->management_flags |= MF_CLIENT_AUTH;
     }
 #endif
-#ifdef ENABLE_X509_TRACK
-  else if (streq (p[0], "x509-track") && p[1] && !p[2])
-    {
-      VERIFY_PERMISSION (OPT_P_GENERAL);
-      x509_track_add (&options->x509_track, p[1], msglevel, &options->gc);
-    }
-#endif
 #ifdef MANAGEMENT_PF
   else if (streq (p[0], "management-client-pf") && !p[1])
     {
@@ -7026,6 +7017,11 @@ add_option (struct options *options,
 	}
       options->key_method = key_method;
     }
+  else if (streq (p[0], "x509-track") && p[1] && !p[2])
+    {
+      VERIFY_PERMISSION (OPT_P_GENERAL);
+      x509_track_add (&options->x509_track, p[1], msglevel, &options->gc);
+    }
 #ifdef ENABLE_X509ALTUSERNAME
   else if (streq (p[0], "x509-username-field") && p[1] && !p[2])
     {
src/openvpn/options.h +0 -2
index 8a26e14..ffbe5e1 100644
@@ -574,9 +574,7 @@ struct options
 
 #endif /* ENABLE_CRYPTO */
 
-#ifdef ENABLE_X509_TRACK
   const struct x509_track *x509_track;
-#endif
 
   /* special state parms */
   int foreign_option_index;
src/openvpn/ssl_common.h +0 -2
index 334ccb0..a0df0ff 100644
@@ -308,9 +308,7 @@ struct tls_options
   struct man_def_auth_context *mda_context;
 #endif
 
-#ifdef ENABLE_X509_TRACK
   const struct x509_track *x509_track;
-#endif
 
 #ifdef ENABLE_CLIENT_CR
   const struct static_challenge_info *sci;
src/openvpn/ssl_verify.c +4 -12
index ea381f8..b373bed 100644
@@ -393,22 +393,17 @@ verify_peer_cert(const struct tls_options *opt, openvpn_x509_cert_t *peer_cert,
  */
 static void
 verify_cert_set_env(struct env_set *es, openvpn_x509_cert_t *peer_cert, int cert_depth,
-    const char *subject, const char *common_name
-#ifdef ENABLE_X509_TRACK
-    , const struct x509_track *x509_track
-#endif
-    )
+    const char *subject, const char *common_name,
+    const struct x509_track *x509_track)
 {
   char envname[64];
   char *serial = NULL;
   struct gc_arena gc = gc_new ();
 
   /* Save X509 fields in environment */
-#ifdef ENABLE_X509_TRACK
   if (x509_track)
     x509_setenv_track (x509_track, es, cert_depth, peer_cert);
   else
-#endif
     x509_setenv (es, cert_depth, peer_cert);
 
   /* export subject name string as environmental variable */
@@ -658,11 +653,8 @@ verify_cert(struct tls_session *session, openvpn_x509_cert_t *cert, int cert_dep
   session->verify_maxlevel = max_int (session->verify_maxlevel, cert_depth);
 
   /* export certificate values to the environment */
-  verify_cert_set_env(opt->es, cert, cert_depth, subject, common_name
-#ifdef ENABLE_X509_TRACK
-      , opt->x509_track
-#endif
-      );
+  verify_cert_set_env(opt->es, cert, cert_depth, subject, common_name,
+      opt->x509_track);
 
   /* export current untrusted IP */
   setenv_untrusted (session);
src/openvpn/ssl_verify.h +0 -6
index d5bf22e..8527415 100644
@@ -201,8 +201,6 @@ void verify_user_pass(struct user_pass *up, struct tls_multi *multi,
  */
 void verify_final_auth_checks(struct tls_multi *multi, struct tls_session *session);
 
-#ifdef ENABLE_X509_TRACK
-
 struct x509_track
 {
   const struct x509_track *next;
@@ -212,10 +210,6 @@ struct x509_track
   int nid;
 };
 
-void x509_track_add (const struct x509_track **ll_head, const char *name, int msglevel, struct gc_arena *gc);
-
-#endif
-
 /*
  * Certificate checking for verify_nsCertType
  */
src/openvpn/ssl_verify_backend.h +0 -4
index 17e88fb..9d2057b 100644
@@ -150,8 +150,6 @@ char *backend_x509_get_serial_hex (openvpn_x509_cert_t *cert,
  */
 void x509_setenv (struct env_set *es, int cert_depth, openvpn_x509_cert_t *cert);
 
-#ifdef ENABLE_X509_TRACK
-
 /*
  * Start tracking the given attribute.
  *
@@ -189,8 +187,6 @@ void x509_track_add (const struct x509_track **ll_head, const char *name,
 void x509_setenv_track (const struct x509_track *xt, struct env_set *es,
     const int depth, openvpn_x509_cert_t *x509);
 
-#endif
-
 /*
  * Check X.509 Netscape certificate type field, if available.
  *
src/openvpn/ssl_verify_openssl.c +0 -3
index 34e7d16..adf68d8 100644
@@ -303,8 +303,6 @@ err:
 }
 
 
-#ifdef ENABLE_X509_TRACK
-
 /*
  * x509-track implementation -- save X509 fields to environment,
  * using the naming convention:
@@ -433,7 +431,6 @@ x509_setenv_track (const struct x509_track *xt, struct env_set *es, const int de
     }
   gc_free(&gc);
 }
-#endif
 
 /*
  * Save X509 fields to environment, using the naming convention:
src/openvpn/ssl_verify_polarssl.c +90 -0

103 lines not on this page · raw message

src/openvpn/syshead.h +0 -7
index 7e77b6c..1c9248f 100644
@@ -632,13 +632,6 @@ socket_defined (const socket_descriptor_t sd)
 #endif
 
 /*
- * Enable x509-track feature?
- */
-#if defined(ENABLE_CRYPTO) && defined (ENABLE_CRYPTO_OPENSSL)
-#define ENABLE_X509_TRACK
-#endif
-
-/*
  * Is poll available on this platform?
  */
 #if defined(HAVE_POLL) && defined(HAVE_SYS_POLL_H)
Signature · 1 line
2.5.0
#21 lore raw

Re: [Openvpn-devel] [PATCH 07/10] Implemented x509-track for PolarSSL.

Am 09.03.16 um 00:10 schrieb Steffan Karger:

Quoted 6 lines

Hi,

This addition is welcome and the code does the job it promises to do, but after reviewing the code I would like to propose a different implementation. The reasons for this are gives as inline replies below. The alternative patch proposal is attached.

Code looks good to me, ACK.

Arne

#22 lore raw

[Openvpn-devel] [PATCH applied] Re: Implemented x509-track for PolarSSL.

In reply to Steffan Karger

Your patch has been applied to the master branch.

commit fab49d17d36053189cf504d57e53a8b0cb907f6f

Author: Steffan Karger
Date:   Sat Mar 5 17:08:22 2016 +0100
     Implemented x509-track for PolarSSL.

     Signed-off-by: Steffan Karger <[email protected]...>
     Acked-by: Arne Schwabe <[email protected]...>
     Message-Id: <[email protected]...>
     URL: http://article.gmane.org/gmane.network.openvpn.devel/11350
     Signed-off-by: Gert Doering <[email protected]...>

--
kind regards,

Gert Doering

#23 lore raw

PATCH 8/10 Added ./configure flag to disable user-defined scripts.

In reply to James Yonan

Added ./configure --disable-scripts build flag to ignore the "script-security" directive in configs. This can be used to prevent configs from raising the script-security level to allow user-defined script execution.

At the C level, script support is enabled by:

  /* Enable user-defined scripting */
  #define ENABLE_SCRIPTS 1

or disabled by ensuring that ENABLE_SCRIPTS is undefined.

If ENABLE_SCRIPTS is undefined, OpenVPN will log "[NOSCRIPTS]" in the initial version info line.

This patch assumes that script_security defaults to SSEC_BUILT_IN or less.

---
 configure.ac          | 8 ++++++++
 src/openvpn/options.c | 7 +++++++
 2 files changed, 15 insertions(+)
configure.ac +8 -0
index b75d51f..c3ff472 100644
@@ -128,6 +128,13 @@ AC_ARG_ENABLE(
 )
 
 AC_ARG_ENABLE(
+	[scripts],
+	[AS_HELP_STRING([--disable-scripts], [disable user-defined scripting support @<:@default=yes@:>@])],
+	,
+	[enable_scripts="yes"]
+)
+
+AC_ARG_ENABLE(
 	[pkcs11],
 	[AS_HELP_STRING([--enable-pkcs11], [enable pkcs11 support @<:@default=no@:>@])],
 	,
@@ -1059,6 +1066,7 @@ test "${ac_cv_header_sys_uio_h}" = "yes" && AC_DEFINE([HAVE_IOVEC], [1], [struct
 test "${enable_multi}" = "yes" && AC_DEFINE([ENABLE_CLIENT_SERVER], [1], [Enable client/server capability])
 test "${enable_server}" = "no" && AC_DEFINE([ENABLE_CLIENT_ONLY], [1], [Enable client capability only])
 test "${enable_management}" = "yes" && AC_DEFINE([ENABLE_MANAGEMENT], [1], [Enable management server capability])
+test "${enable_scripts}" = "yes" && AC_DEFINE([ENABLE_SCRIPTS], [1], [Enable user-defined scripting])
 test "${enable_multihome}" = "yes" && AC_DEFINE([ENABLE_MULTIHOME], [1], [Enable multi-homed UDP server capability])
 test "${enable_debug}" = "yes" && AC_DEFINE([ENABLE_DEBUG], [1], [Enable debugging support])
 test "${enable_small}" = "yes" && AC_DEFINE([ENABLE_SMALL], [1], [Enable smaller executable size])
src/openvpn/options.c +7 -0
index 0942a4d..b53f7ac 100644
@@ -99,6 +99,9 @@ const char title_string[] =
 #if ENABLE_IP_PKTINFO
   " [MH]"
 #endif
+#ifndef ENABLE_SCRIPTS
+  " [NOSCRIPTS]"
+#endif
   " [IPv6]"
   " built on " __DATE__
 ;
@@ -5476,7 +5479,11 @@ add_option (struct options *options,
   else if (streq (p[0], "script-security") && p[1] && !p[2])
     {
       VERIFY_PERMISSION (OPT_P_GENERAL);
+#ifdef ENABLE_SCRIPTS
       script_security = atoi (p[1]);
+#else
+      msg (M_WARN, "NOTE: script-security directive ignored due to build setting");
+#endif
     }
   else if (streq (p[0], "mssfix") && !p[2])
     {
Signature · 1 line
1.9.1
#24 lore raw

PATCH 9/10 Added directive to specify HTTP proxy credentials in config.

In reply to James Yonan

The inline directive http-proxy-user-pass can be used to specify proxy credentials in config, e.g.:

http-proxy proxy.tld 3128 auto-nct
<http-proxy-user-pass>
foo
bar
</http-proxy-user-pass>

This usage is already supported by OpenVPN 3.

---
 src/openvpn/misc.c    |  8 ++++++++
 src/openvpn/misc.h    |  2 ++
 src/openvpn/options.c | 13 +++++++++++++
 src/openvpn/proxy.c   |  2 ++
 src/openvpn/proxy.h   |  1 +
 5 files changed, 26 insertions(+)
src/openvpn/misc.c +8 -0
index 05ed073..4766a17 100644
@@ -1090,6 +1090,14 @@ get_user_pass_cr (struct user_pass *up,
 	  if (!strlen (up->password))
 	    strcpy (up->password, "ok");
 	}
+      else if (flags & GET_USER_PASS_INLINE_CREDS)
+	{
+	  struct buffer buf;
+	  buf_set_read (&buf, (uint8_t*) auth_file, strlen (auth_file) + 1);
+	  if (!(flags & GET_USER_PASS_PASSWORD_ONLY))
+	    buf_parse (&buf, '\n', up->username, USER_PASS_LEN);
+	  buf_parse (&buf, '\n', up->password, USER_PASS_LEN);
+	}
       else if (from_authfile)
         {
           /*
src/openvpn/misc.h +2 -0
index 65a6e55..b694096 100644
@@ -256,6 +256,8 @@ struct static_challenge_info {};
 #define GET_USER_PASS_STATIC_CHALLENGE       (1<<8) /* SCRV1 protocol -- static challenge */
 #define GET_USER_PASS_STATIC_CHALLENGE_ECHO  (1<<9) /* SCRV1 protocol -- echo response */
 
+#define GET_USER_PASS_INLINE_CREDS (1<<10)  /* indicates that auth_file is actually inline creds */
+
 bool get_user_pass_cr (struct user_pass *up,
 		       const char *auth_file,
 		       const char *prefix,
src/openvpn/options.c +13 -0
index b53f7ac..8f560a7 100644
@@ -5121,6 +5121,19 @@ add_option (struct options *options,
 	  ho->auth_method_string = "none";
 	}
     }
+  else if (streq (p[0], "http-proxy-user-pass") && p[1])
+    {
+      struct http_proxy_options *ho;
+      VERIFY_PERMISSION (OPT_P_GENERAL);
+      ho = init_http_proxy_options_once (&options->ce.http_proxy_options, &options->gc);
+      if (streq (p[1], INLINE_FILE_TAG) && p[2])
+	{
+	  ho->auth_file = p[2];
+	  ho->inline_creds = true;
+	}
+      else
+	ho->auth_file = p[1];
+    }
   else if (streq (p[0], "http-proxy-retry") && !p[1])
     {
       struct http_proxy_options *ho;
src/openvpn/proxy.c +2 -0
index 2568e19..865a026 100644
@@ -241,6 +241,8 @@ get_user_pass_http (struct http_proxy_info *p, const bool force)
       unsigned int flags = GET_USER_PASS_MANAGEMENT;
       if (p->queried_creds)
 	flags |= GET_USER_PASS_PREVIOUS_CREDS_FAILED;
+      if (p->options.inline_creds)
+	flags |= GET_USER_PASS_INLINE_CREDS;
       get_user_pass (&static_proxy_user_pass,
 		     p->options.auth_file,
 		     UP_TYPE_PROXY,
src/openvpn/proxy.h +1 -0
index 4715940..864d3b9 100644
@@ -58,6 +58,7 @@ struct http_proxy_options {
   const char *http_version;
   const char *user_agent;
   struct http_custom_header custom_headers[MAX_CUSTOM_HTTP_HEADER];
+  bool inline_creds;
 };
 
 struct http_proxy_options_simple {
Signature · 1 line
1.9.1
#25 lore raw

Re: [Openvpn-devel] [PATCH 09/10] Added directive to specify HTTP proxy credentials in config.

Quoted 12 lines · James Yonan

The inline directive http-proxy-user-pass can be used to specify proxy credentials in config, e.g.:

http-proxy proxy.tld 3128 auto-nct
<http-proxy-user-pass>
foo
bar
</http-proxy-user-pass>

This usage is already supported by OpenVPN 3.

Quoted 8 lines
---
 src/openvpn/misc.c    |  8 ++++++++
 src/openvpn/misc.h    |  2 ++
 src/openvpn/options.c | 13 +++++++++++++
 src/openvpn/proxy.c   |  2 ++
 src/openvpn/proxy.h   |  1 +
 5 files changed, 26 insertions(+)

No man page documentation?

#26 lore raw

Re: [Openvpn-devel] [PATCH 09/10] Added directive to specify HTTP proxy credentials in config.

In reply to James Yonan

Am 03.03.16 um 09:19 schrieb James Yonan:

Quoted 11 lines

The inline directive http-proxy-user-pass can be used to specify proxy credentials in config, e.g.:

http-proxy proxy.tld 3128 auto-nct
<http-proxy-user-pass>
foo
bar
</http-proxy-user-pass>

This usage is already supported by OpenVPN 3.

ACK. But it really should come with manpage (and a short note in Changes.rst)

Arne

#27 lore raw

PATCH 1/1 Add documentation for http-proxy-user-pass option

---
 Changes.rst   | 4 ++++
 doc/openvpn.8 | 7 +++++--
 2 files changed, 9 insertions(+), 2 deletions(-)
Changes.rst +4 -0
index ab322e2..f945ad7 100644
@@ -51,6 +51,10 @@ AEAD (GCM) data channel cipher support
     bytes per packet for AES-128-GCM instead of 36 bytes per packet for
     AES-128-CBC + HMAC-SHA1).
 
+Http proxy password inside config file
+	Http proxy passwords can be specified with the inline file option
+    http-proxy-user-pass
+
 
 User-visible Changes
 --------------------
doc/openvpn.8 +5 -2
index f1a4361..da4ad8b 100644
@@ -497,7 +497,10 @@ and port
 If HTTP Proxy-Authenticate is required,
 .B authfile
 is a file containing a username and password on 2 lines, or
-"stdin" to prompt from console.
+"stdin" to prompt from console. Its content can also be specified
+in the config file with the
+\.B http-proxy-user-pass 
+option. (See section on inline files)
 
 .B auth\-method
 should be one of "none", "basic", or "ntlm".
@@ -6581,7 +6584,7 @@ X509_1_C=KG
 .SH INLINE FILE SUPPORT
 OpenVPN allows including files in the main configuration for the
 .B \-\-ca, \-\-cert, \-\-dh, \-\-extra\-certs, \-\-key, \-\-pkcs12, \-\-secret,
-.B \-\-crl-verify
+.B \-\-crl-verify, http-proxy-user-pass
 and
 .B \-\-tls\-auth
 options.
Signature · 1 line
2.7.4 (Apple Git-66)
#28 lore raw

Re: [Openvpn-devel] [PATCH] Add documentation for http-proxy-user-pass option

Quoted 24 lines · Arne Schwabe
 Changes.rst   | 4 ++++
 doc/openvpn.8 | 7 +++++--
 2 files changed, 9 insertions(+), 2 deletions(-)
Changes.rst +4 -0
index ab322e2..f945ad7 100644
@@ -51,6 +51,10 @@ AEAD (GCM) data channel cipher support
     bytes per packet for AES-128-GCM instead of 36 bytes per packet for
     AES-128-CBC + HMAC-SHA1).
 
+Http proxy password inside config file
+       Http proxy passwords can be specified with the inline file option
+    http-proxy-user-pass
+
 
 User-visible Changes
 --------------------
doc/openvpn.8 +0 -0
index f1a4361..da4ad8b 100644

doc/openvpn.8 and port · line 503

Quoted 8 lines
doc/openvpn.8 +3 -1
@@ -497,7 +497,10 @@ and port
 If HTTP Proxy-Authenticate is required,
 .B authfile
 is a file containing a username and password on 2 lines, or
-"stdin" to prompt from console.
+"stdin" to prompt from console. Its content can also be specified
+in the config file with the
+\.B http-proxy-user-pass
 

man on some systems may treat \.B as .B, but many programs may not parse that as bold. In any case the macro is

.B not \.B

Also see below on hyphens vs dashes..

doc/openvpn.8 X509_1_C=KG · line 6589

Quoted 12 lines
doc/openvpn.8 +1 -0
+option. (See section on inline files)
 
 .B auth\-method

should be one of "none", "basic", or "ntlm".

doc/openvpn.8 +1 -1
@@ -6581,7 +6584,7 @@ X509_1_C=KG
 .SH INLINE FILE SUPPORT
 OpenVPN allows including files in the main configuration for the
 .B \-\-ca, \-\-cert, \-\-dh, \-\-extra\-certs, \-\-key, \-\-pkcs12,
 \-\-secret,
-.B \-\-crl-verify
+.B \-\-crl-verify, http-proxy-user-pass
 

To be consistent, that would be --http-proxy-user-pass (leading double dashes missing).

Also using \- for dashes to distinguish them from hyphens is the usual practice.

Selva

#29 lore raw

PATCH v2 1/1 Add documentation for http-proxy-user-pass option

In reply to Arne Schwabe

Patch V2: fix formatting problems

---
 Changes.rst   | 4 ++++
 doc/openvpn.8 | 7 +++++--
 2 files changed, 9 insertions(+), 2 deletions(-)
Changes.rst +4 -0
index ab322e2..f945ad7 100644
@@ -51,6 +51,10 @@ AEAD (GCM) data channel cipher support
     bytes per packet for AES-128-GCM instead of 36 bytes per packet for
     AES-128-CBC + HMAC-SHA1).
 
+Http proxy password inside config file
+	Http proxy passwords can be specified with the inline file option
+    http-proxy-user-pass
+
 
 User-visible Changes
 --------------------
doc/openvpn.8 +5 -2
index f1a4361..6e8a6c1 100644
@@ -497,7 +497,10 @@ and port
 If HTTP Proxy-Authenticate is required,
 .B authfile
 is a file containing a username and password on 2 lines, or
-"stdin" to prompt from console.
+"stdin" to prompt from console. Its content can also be specified
+in the config file with the
+.B \-\-http\-proxy\-user\-pass 
+option. (See section on inline files)
 
 .B auth\-method
 should be one of "none", "basic", or "ntlm".
@@ -6581,7 +6584,7 @@ X509_1_C=KG
 .SH INLINE FILE SUPPORT
 OpenVPN allows including files in the main configuration for the
 .B \-\-ca, \-\-cert, \-\-dh, \-\-extra\-certs, \-\-key, \-\-pkcs12, \-\-secret,
-.B \-\-crl-verify
+.B \-\-crl-verify,\-\-http\-proxy\-user\-pass
 and
 .B \-\-tls\-auth
 options.
Signature · 1 line
2.7.4 (Apple Git-66)
#30 lore raw

Re: [Openvpn-devel] [PATCH v2] Add documentation for http-proxy-user-pass option

Quoted 38 lines · Arne Schwabe

Patch V2: fix formatting problems

---
 Changes.rst   | 4 ++++
 doc/openvpn.8 | 7 +++++--
 2 files changed, 9 insertions(+), 2 deletions(-)
Changes.rst +4 -0
index ab322e2..f945ad7 100644
@@ -51,6 +51,10 @@ AEAD (GCM) data channel cipher support
     bytes per packet for AES-128-GCM instead of 36 bytes per packet for
     AES-128-CBC + HMAC-SHA1).
 
+Http proxy password inside config file
+       Http proxy passwords can be specified with the inline file option
+    http-proxy-user-pass
+
 
 User-visible Changes
 --------------------
doc/openvpn.8 +4 -1
index f1a4361..6e8a6c1 100644
@@ -497,7 +497,10 @@ and port
 If HTTP Proxy-Authenticate is required,
 .B authfile
 is a file containing a username and password on 2 lines, or
-"stdin" to prompt from console.
+"stdin" to prompt from console. Its content can also be specified
+in the config file with the
+.B \-\-http\-proxy\-user\-pass
+option. (See section on inline files)
 
 .B auth\-method
 should be one of "none", "basic", or "ntlm".

doc/openvpn.8 X509_1_C=KG · line 6589

Quoted 7 lines
doc/openvpn.8 +1 -1
@@ -6581,7 +6584,7 @@ X509_1_C=KG
 .SH INLINE FILE SUPPORT
 OpenVPN allows including files in the main configuration for the
 .B \-\-ca, \-\-cert, \-\-dh, \-\-extra\-certs, \-\-key, \-\-pkcs12,
 \-\-secret,
-.B \-\-crl-verify
+.B \-\-crl-verify,\-\-http\-proxy\-user\-pass
 

At merge time crl-verify on that line may be changed to crl\-verify too. Not a fault of this patch.

ACK.

Selva

#31 lore raw

[Openvpn-devel] [PATCH applied] Re: Add documentation for http-proxy-user-pass option

In reply to Arne Schwabe

Your patch has been applied to the master branch.

(I have taken the liberty to add the suggested "\" to the crl-verify preceding --http-proxy-user-pass, also a blank was missing)

commit ec0c1dcabd1d31cd4bd452db6d099b4df0739778

Author: Arne Schwabe
Date:   Fri Jun 17 14:49:46 2016 +0200
     Add documentation for http-proxy-user-pass option

     Acked-by: Selva Nair <[email protected]...>
     Message-Id: <[email protected]...>
     URL: http://article.gmane.org/gmane.network.openvpn.devel/11933
     Signed-off-by: Gert Doering <[email protected]...>

--
kind regards,

Gert Doering

#32 lore raw

[Openvpn-devel] [PATCH applied] Re: Added directive to specify HTTP proxy credentials in config.

In reply to James Yonan

Another ACK from me - the patch did not apply easily to current master as there was a change in context last December. So did a more in-depth review to ensure that the actual logic of get_user_pass_cr() is still matching the expectations of this change, and it should be fine.

(Somewhere in our patch queue, there's the "permit auth-user-pass as <INLINE>", but since that didn't apply and nobody felt like rebasing it, an eventual rebase will need to take *this* change into account as well)

Your patch has been applied to the master branch.

commit c9a35a20812aafdacc3682a0379f52126bd567ae

Author: James Yonan
Date:   Thu Mar 3 01:19:05 2016 -0700
     Added directive to specify HTTP proxy credentials in config.

     Signed-off-by: James Yonan <[email protected]...>
     Acked-by: Arne Schwabe <[email protected]...>
     Acked-by: Gert Doering <[email protected]...>
     Message-Id: <[email protected]...>
     URL: http://article.gmane.org/gmane.network.openvpn.devel/11283
     Signed-off-by: Gert Doering <[email protected]...>

--
kind regards,

Gert Doering

#33 lore raw

PATCH 10/10 Bind to local socket before dropping privileges

In reply to James Yonan

Bind the local TCP/UDP socket before UID/GID downgrade, otherwise we cannot bind to ports < 1024.

---
 src/openvpn/init.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)
src/openvpn/init.c +4 -4
index cb73a3d..9658895 100644
@@ -3630,16 +3630,16 @@ init_instance (struct context *c, const struct env_set *env, const unsigned int
     open_plugins (c, false, OPENVPN_PLUGIN_INIT_POST_DAEMON);
 #endif
 
+  /* finalize the TCP/UDP socket */
+  if (c->mode == CM_P2P || c->mode == CM_TOP || c->mode == CM_CHILD_TCP)
+    do_init_socket_2 (c);
+
   /*
    * Actually do UID/GID downgrade, and chroot, if requested.
    * May be delayed by --client, --pull, or --up-delay.
    */
   do_uid_gid_chroot (c, c->c2.did_open_tun);
 
-  /* finalize the TCP/UDP socket */
-  if (c->mode == CM_P2P || c->mode == CM_TOP || c->mode == CM_CHILD_TCP)
-    do_init_socket_2 (c);
-
   /* initialize timers */
   if (c->mode == CM_P2P || child)
     do_init_timers (c, false);
Signature · 1 line
1.9.1
#34 lore raw

Re: [Openvpn-devel] [PATCH applied] Bind to local socket before dropping privileges

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

ACK. Tested both master and release/2.3 branches with and without this patch. On master this patch was needed to allow --lport 443 together with --user openvpn --group openvpn. On release/2.3 this patch was not needed.

Your patch has been applied to the master branch

commit b3e975824ea9ebae8dbea5b451c8d02525c83ffe

Author: James Yonan
Date:   Thu Mar 3 01:19:06 2016 -0700
     Bind to local socket before dropping privileges

     Signed-off-by: James Yonan <[email protected]...>
     Acked-by: David Sommerseth <[email protected]...>
     Message-Id: [email protected]...
     URL: http://www.mail-archive.com/search?l=mid&[email protected]...
     Signed-off-by: David Sommerseth <[email protected]...>

- --
kind regards,

David Sommerseth

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.22 (GNU/Linux)

iQIcBAEBAgAGBQJXv0O7AAoJEIbPlEyWcf3yZLAQAMYxU27vByo6r73VlbSPm+aJ ycUd3OHSgLEgofRtmmXVJQv9IV5iAQnv5WYY5yiWTbJ2lEYWW5j6hxs06Lq6gdZM MxsBq4/xrFHJxEwq8mfjjxJt3dEz+XX+UwNr3uz69vSDqK3xq7waomdMnd7lAe0M hdqSMwyEcCoyIr5OQo2erlJuku6H8qSjDANb1MzPIB4RPLzW99UuwSMzS/WjhVvZ m+l4LU3/fYkKiGue2r4DKHDFoc1cFPHBLA4rSCmnf2YGj8X2XHuV3cgZacmbt2VI ygYIDtcnkvmVNXKgCPx1I+R/uyiy58auDXwEIK3fdSWP9ppi5vI9UeXf6yG8En7m XMMUtoOBRSCH8H29fVRwgkq9H84eyMiBIsDNDda6OS9J3q8QMswOTOyXH/TpTwj1 A5cvaX64suA8RnvDRpq0gCzo92TLkU+iL1KSctOWDClUhMW6PF8mwIY4e6CpqWUk Z8v4tBTo8xva4zCarhjLm0Xefd68sAaqppJImTSj32Ae0VzGbLRqeuKZ50uBPgDZ MW7KuNCTV3ji6FbdJnLiynvIQdYkJLDyP5H7vbeaW5RIaC6gDdhSVUBkynSJOqGZ MQW8uaQuPbl5GdPRmvsLcyupZhwnYhsmQ2oxn0pUKC0uFqErhhBQlst4/nBWDHDI 51Xvwwb6b+IyzvgWB+jX
=aaSH
-----END PGP SIGNATURE-----

#35 lore raw

Re: [Openvpn-devel] [PATCH applied] Bind to local socket before dropping privileges

Hi,

Quoted 6 lines · David Sommerseth

ACK. Tested both master and release/2.3 branches with and without this patch. On master this patch was needed to allow --lport 443 together with --user openvpn --group openvpn. On release/2.3 this patch was not needed.

Your patch has been applied to the master branch

Uh. I'm not sure what happened here, but this patch breaks all (most?) TCP client connections for me. Across the board, all buildslaves (BSD and Solaris) have issues connecting via TCP, and my linux client fails as well:

Thu Aug 25 22:07:29 2016 Attempting to establish TCP connection with [AF_INET]199.102.77.82:51194 [nonblock] Thu Aug 25 22:07:29 2016 TCP: connect to [AF_INET]199.102.77.82:51194 failed: Connection timed out Thu Aug 25 22:07:29 2016 SIGUSR1[connection failed(soft),init_instance] received, process restarting Thu Aug 25 22:07:29 2016 Restart pause, 5 second(s)

seems that the patch interferes with the new timeout logic, and a "sudden death" timeout of 0 seconds strikes because something is not initialized yet.

David, please test the stuff you commit a bit more thoroughly, with a full set of t_client tests (udp, tcp, tun+tap) at least. *Especially* those "harmless looking" patches that have lingered around for a bit while other serious code changes have happened.

Please revert.

gert

Signature · 4 lines
USENET is *not* the non-clickable part of WWW!
                                                           //www.muc.de/~gert/
Gert Doering - Munich, Germany                             [email protected]...
fax: +49-89-35655025                        [email protected]...