Skip to contentSkip to conversation

Openvpn for Android patch set

PATCH Applied Arne Schwabe 4 participants 26 messages Jun 2, 2012 – Jun 12, 2012 1 message not in the archive

lore
Share & tools
lore
#0 lore raw

cover Openvpn for Android patch set

These are the patches that currently are applied to the openvpn version distributed in the android version of openvpn. Only the last two patches are really android specific the rest of the patches should also be useful on other platforms.

Arne Schwabe (8):
Silence Port is now 1194 IANA warning if remote option with port is
used
Add the query to the error message. Makes the diagnose what went
wrong from logs easier.
Only use tmpdir if tmp_dir is really used.
Explain that route-nopull also causes the client to ignore dhcp
options.
Add the name of the context where option is not allowed to the error
message.
Allow routes to be set before opening tun, similar to ifconfig before
opening tun
add ability to send/receive file descriptors via management
interface, only used in android so. For now under #ifdef ANDROID
Android platform specific changes.

 doc/openvpn.8         |    3 +-
 src/openvpn/console.c |    2 +-
 src/openvpn/init.c    |   11 +++++-
 src/openvpn/manage.c  |   88 +++++++++++++++++++++++++++++++++++++++++++++++++
 src/openvpn/manage.h  |    4 ++
 src/openvpn/options.c |   26 ++++++++++----
 src/openvpn/options.h |    2 +
 src/openvpn/route.c   |   25 +++++++++++++-
 src/openvpn/socket.c  |   10 +++++
 src/openvpn/ssl.c     |    2 +
 src/openvpn/syshead.h |    3 +-
 src/openvpn/tun.c     |   88 ++++++++++++++++++++++++++++++++++++++++++++++++-
 src/openvpn/tun.h     |   19 ++++++++++-
 13 files changed, 267 insertions(+), 16 deletions(-)
Signature · 1 line
1.7.7.5 (Apple Git-26)
#1 lore raw

PATCH 1/8 Silence Port is now 1194 IANA warning if remote option with port is used

---
 src/openvpn/options.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)
src/openvpn/options.c +1 -0
index 08f9cb2..8e5d3b9 100644
@@ -4506,6 +4506,7 @@ add_option (struct options *options,
 	      goto err;
 	    }
 	  re.remote_port = port;
+      options->ce.port_option_used = true;
 	  if (p[3])
 	    {
 	      const int proto = ascii2proto (p[3]);
Signature · 1 line
1.7.7.5 (Apple Git-26)
#2 lore raw

Re: [Openvpn-devel] [PATCH 1/8] Silence Port is now 1194 IANA warning if remote option with port is used

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

First of all, thank you very much for your work on this patch set! And you have a lot of good things here.

So to this patch ...

Quoted 10 lines · Arne Schwabe

src/openvpn/options.c | 1 + 1 files changed, 1 insertions(+), 0
deletions(-)

src/openvpn/options.c index renamed from src/openvpn/options.c +0 -0

08f9cb2..8e5d3b9 100644 --- a/src/openvpn/options.c +++ b/src/openvpn/options.c @@ -4506,6 +4506,7 @@ add_option (struct options *options, goto err; } re.remote_port = port; + options->ce.port_option_used = true;

Instead of this change, wouldn't it rather make sense to just remove the whole "#if 1 /* JYFIXME -- port warning */" block in init.c:2470 instead? This "new" port number was introduced in November 2004, so I'd say this warning has been laying around long enough.

When doing that, we can also get rid of the port_option_used member completely too. If you do:

   $ git grep -n port_option_used

in the source tree, you see where this is used, and it's not many places at all.

kind regards,

David Sommerseth

Quoted 1 line

if (p[3]) { const int proto = ascii2proto (p[3]);

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.11 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iEYEARECAAYFAk/KPiAACgkQDC186MBRfrouAACgmv1F0CiIM0HyrhNYAFltqpPv szEAnj8O/j/vqx8884KDjZlNrLhLOfyh
=fMoA
-----END PGP SIGNATURE-----

#3 lore raw

Re: [Openvpn-devel] [PATCH 1/8] Silence Port is now 1194 IANA warning if remote option with port is used

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

Am 02.06.12 18:24, schrieb David Sommerseth:

Quoted 12 lines

First of all, thank you very much for your work on this patch set! And you have a lot of good things here.

So to this patch ...

Quoted 3 lines · Arne Schwabe

src/openvpn/options.c | 1 + 1 files changed, 1 insertions(+), 0 deletions(-)

Quoted 10 lines
Quoted 5 lines
src/openvpn/options.c index renamed from src/openvpn/options.c +0 -0

08f9cb2..8e5d3b9 100644 --- a/src/openvpn/options.c +++ b/src/openvpn/options.c @@ -4506,6 +4506,7 @@ add_option (struct options *options, goto err; } re.remote_port = port; + options->ce.port_option_used = true;

Instead of this change, wouldn't it rather make sense to just remove the whole "#if 1 /* JYFIXME -- port warning */" block in init.c:2470 instead? This "new" port number was introduced in November 2004, so I'd say this warning has been laying around long enough.

I did not realise that the warning is around that long. It is probably more confusing than helping now.

Arne
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.11 (Darwin)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iEYEARECAAYFAk/KRAMACgkQe8+cMNS4zRceugCfRbJ/CdSV7/fSCkJXQhFsablz ycYAoJB9dUw8w3Va1Hz3U6WNxjK2XuVP
=dSi/
-----END PGP SIGNATURE-----

#4 lore raw

Re: [Openvpn-devel] [PATCH 1/8] Silence Port is now 1194 IANA warning if remote option with port is used

In reply to David Sommerseth

HI,

Quoted 5 lines · David Sommerseth

Instead of this change, wouldn't it rather make sense to just remove the whole "#if 1 /* JYFIXME -- port warning */" block in init.c:2470 instead? This "new" port number was introduced in November 2004, so I'd say this warning has been laying around long enough.

Quoted 7 lines

When doing that, we can also get rid of the port_option_used member completely too. If you do:

   $ git grep -n port_option_used

in the source tree, you see where this is used, and it's not many places at all.

Sounds good to me. Feature-ACK on removing the warning block and port_option_used.

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]...
#5 lore raw

PATCH 2/8 Add the query to the error message. Makes the diagnose what went wrong from logs easier.

In reply to Arne Schwabe
---
 src/openvpn/console.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)
src/openvpn/console.c +1 -1
index 2464e7e..afda8ca 100644
@@ -232,7 +232,7 @@ get_console_input (const char *prompt, const bool echo, char *input, const int c
 	}
     }
 #else
-  msg (M_FATAL, "Sorry, but I can't get console input on this OS");
+  msg (M_FATAL, "Sorry, but I can't get console input on this OS (%s)", prompt);
 #endif
   return ret;
 }
Signature · 1 line
1.7.7.5 (Apple Git-26)
#6 lore raw

Re: [Openvpn-devel] [PATCH 2/8] Add the query to the error message. Makes the diagnose what went wrong from logs easier.

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

Quoted 6 lines · Arne Schwabe
---
 src/openvpn/console.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

ACK. Will apply this soonish, unless somebody objects during next week.

kind regards,

David Sommerseth

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.11 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iEYEARECAAYFAk/KQcQACgkQDC186MBRfrpPbACfVDp8U2iLQX30eEwqxXJ2+bw5 +7MAn2y9Y9XWkrxz0PdQUp6ds2zrD11O
=SZT1
-----END PGP SIGNATURE-----

#7 lore raw

Re: [Openvpn-devel] [PATCH 2/8] Add the query to the error message. Makes the diagnose what went wrong from logs easier.

In reply to Arne Schwabe
Quoted 6 lines · Arne Schwabe
---
 src/openvpn/console.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

Applied to master.

commit 803613de394a0b25374797d001d4a2cdea8b593e

Author: Arne Schwabe <[email protected]...>
Date:   Sat Jun 2 15:03:37 2012 +0200

Add the query to the error message. Makes the diagnose what went
wrong from logs easier.

    Signed-off-by: Arne Schwabe <[email protected]...>
    Acked-by: David Sommerseth <[email protected]...>
    Message-Id: [email protected]...
    URL: http://article.gmane.org/gmane.network.openvpn.devel/6645
    Signed-off-by: David Sommerseth <[email protected]...>

kind regards,

David Sommerseth

#8 lore raw

PATCH 3/8 Only use tmpdir if tmp_dir is really used.

In reply to Arne Schwabe

This fixes starting openvpn compiled as client only version of systems that have no /tmp (Android). --tmp-dir could only be set if P2MP_SERVER has been enabled too.

---
 src/openvpn/options.c |   12 ++++++++----
 src/openvpn/options.h |    2 ++
 src/openvpn/syshead.h |    1 +
 3 files changed, 11 insertions(+), 4 deletions(-)
src/openvpn/options.c +8 -4
index 8e5d3b9..71d43b7 100644
@@ -862,6 +862,7 @@ init_options (struct options *o, const bool init_gc)
   o->pkcs11_pin_cache_period = -1;
 #endif			/* ENABLE_PKCS11 */
 
+#ifdef ENABLE_TMPDIR
   /* Set default --tmp-dir */
 #ifdef WIN32
   /* On Windows, find temp dir via enviroment variables */
@@ -873,6 +874,7 @@ init_options (struct options *o, const bool init_gc)
           o->tmp_dir = "/tmp";
   }
 #endif /* WIN32 */
+#endif /* ENABLE_TMPDIR */
 }
 
 void
@@ -2759,19 +2761,21 @@ options_postprocess_filechecks (struct options *options)
                              options->management_user_pass, R_OK,
                              "--management user/password file");
 #endif /* ENABLE_MANAGEMENT */
-#if P2MP
+#if ENABLE_TMPDIR
   errs |= check_file_access (CHKACC_FILE|CHKACC_ACPTSTDIN,
                              options->auth_user_pass_file, R_OK,
                              "--auth-user-pass");
-#endif /* P2MP */
+
+  errs |= check_file_access (CHKACC_FILE, options->tmp_dir,
+                             R_OK|W_OK|X_OK, "Temporary directory (--tmp-dir)");
+
+#endif /* ENABLE_TMPDIR */
 
   /* ** System related ** */
   errs |= check_file_access (CHKACC_FILE, options->chroot_dir,
                              R_OK|X_OK, "--chroot directory");
   errs |= check_file_access (CHKACC_DIRPATH|CHKACC_FILEXSTWR, options->writepid,
                              R_OK|W_OK, "--writepid");
-  errs |= check_file_access (CHKACC_FILE, options->tmp_dir,
-                             R_OK|W_OK|X_OK, "Temporary directory (--tmp-dir)");
 
   /* ** Log related ** */
   errs |= check_file_access (CHKACC_DIRPATH|CHKACC_FILEXSTWR, options->status_file,
src/openvpn/options.h +2 -0
index cc3e47a..50d42fb 100644
@@ -403,7 +403,9 @@ struct options
   struct plugin_option_list *plugin_list;
 #endif
 
+#ifdef ENABLE_TMPDIR
   const char *tmp_dir;
+#endif
 
 #if P2MP
 
src/openvpn/syshead.h +1 -0
index 3337764..f5c8774 100644
@@ -506,6 +506,7 @@ socket_defined (const socket_descriptor_t sd)
 
 #if P2MP && !defined(ENABLE_CLIENT_ONLY)
 #define P2MP_SERVER 1
+#define ENABLE_TMPDIR 1
 #else
 #define P2MP_SERVER 0
 #endif
Signature · 1 line
1.7.7.5 (Apple Git-26)
#9 lore raw

Re: [Openvpn-devel] [PATCH 3/8] Only use tmpdir if tmp_dir is really used.

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

See comments below.

Quoted 54 lines · Arne Schwabe

This fixes starting openvpn compiled as client only version of systems that have no /tmp (Android). --tmp-dir could only be set if P2MP_SERVER has been enabled too.

src/openvpn/options.c | 12 ++++++++---- src/openvpn/options.h |
2 ++ src/openvpn/syshead.h | 1 + 3 files changed, 11
insertions(+), 4 deletions(-)

src/openvpn/options.c index renamed from src/openvpn/options.c +0 -0

8e5d3b9..71d43b7 100644 --- a/src/openvpn/options.c +++ b/src/openvpn/options.c @@ -862,6 +862,7 @@ init_options (struct options *o, const bool init_gc) o->pkcs11_pin_cache_period = -1;
#endif /* ENABLE_PKCS11 */

+#ifdef ENABLE_TMPDIR /* Set default --tmp-dir */ #ifdef WIN32 /* On Windows, find temp dir via enviroment variables */ @@ -873,6 +874,7 @@ init_options (struct options *o, const bool init_gc) o->tmp_dir = "/tmp"; } #endif /* WIN32 */ +#endif /* ENABLE_TMPDIR */ }

void @@ -2759,19 +2761,21 @@ options_postprocess_filechecks (struct options *options) options->management_user_pass, R_OK, "--management user/password file"); #endif /* ENABLE_MANAGEMENT */ -#if P2MP +#if ENABLE_TMPDIR errs |= check_file_access (CHKACC_FILE|CHKACC_ACPTSTDIN, options->auth_user_pass_file, R_OK, "--auth-user-pass"); -#endif /* P2MP */ + + errs |= check_file_access (CHKACC_FILE, options->tmp_dir, + R_OK|W_OK|X_OK, "Temporary directory (--tmp-dir)"); + +#endif /* ENABLE_TMPDIR */

/* ** System related ** */ errs |= check_file_access (CHKACC_FILE, options->chroot_dir, R_OK|X_OK, "--chroot directory"); errs |= check_file_access (CHKACC_DIRPATH|CHKACC_FILEXSTWR, options->writepid, R_OK|W_OK, "--writepid"); - errs |= check_file_access (CHKACC_FILE, options->tmp_dir, - R_OK|W_OK|X_OK, "Temporary directory (--tmp-dir)");

/* ** Log related ** */ errs |= check_file_access (CHKACC_DIRPATH|CHKACC_FILEXSTWR, options->status_file, diff --git a/src/openvpn/options.h b/src/openvpn/options.h index cc3e47a..50d42fb 100644 --- a/src/openvpn/options.h +++ b/src/openvpn/options.h @@ -403,7 +403,9 @@ struct options struct plugin_option_list *plugin_list; #endif

+#ifdef ENABLE_TMPDIR const char *tmp_dir; +#endif

#if P2MP

src/openvpn/syshead.h index renamed from src/openvpn/syshead.h +0 -0

3337764..f5c8774 100644 --- a/src/openvpn/syshead.h +++ b/src/openvpn/syshead.h @@ -506,6 +506,7 @@ socket_defined (const socket_descriptor_t sd)

Quoted 2 lines

#if P2MP && !defined(ENABLE_CLIENT_ONLY) #define P2MP_SERVER 1 +#define ENABLE_TMPDIR 1 #else #define P2MP_SERVER 0 #endif

As we're trying to kill off syshead.h in v2.4, it might rather make sense to try to avoid extending syshead.h. I see the benefit from code clarity point of view. But I'd probably just recommend to check for P2MP_SERVER, with a comment that --tmp-dir is only used in the P2MP_SERVER code; instead of adding the ENABLE_TMPDIR macro.

kind regards,

David Sommerseth

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.11 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iEYEARECAAYFAk/KPzUACgkQDC186MBRfroO2QCfbF5BTeUTrjiTuxSrwmhkw7OV qe4AoINCDLN4Vd9CLF3kP3LJ0D+C+HIt
=kC7H
-----END PGP SIGNATURE-----

#10 lore raw

Re: [Openvpn-devel] [PATCH 3/8] Only use tmpdir if tmp_dir is really used.

Hi,

Quoted 5 lines · David Sommerseth

As we're trying to kill off syshead.h in v2.4, it might rather make sense to try to avoid extending syshead.h. I see the benefit from code clarity point of view. But I'd probably just recommend to check for P2MP_SERVER, with a comment that --tmp-dir is only used in the P2MP_SERVER code; instead of adding the ENABLE_TMPDIR macro.

+1 - another feature-ACK :-)

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]...
#11 lore raw

PATCH 4/8 Explain that route-nopull also causes the client to ignore dhcp options.

In reply to Arne Schwabe
---
 doc/openvpn.8         |    3 ++-
 src/openvpn/options.c |    2 +-
 2 files changed, 3 insertions(+), 2 deletions(-)
doc/openvpn.8 +2 -1
index 24b1a2c..f420d58 100644
@@ -1087,7 +1087,8 @@ When used with
 .B \-\-client
 or
 .B \-\-pull,
-accept options pushed by server EXCEPT for routes.
+accept options pushed by server EXCEPT for routes and dhcp options
+like DNS servers.
 
 When used on the client, this option effectively bars the
 server from adding routes to the client's routing table,
src/openvpn/options.c +1 -1
index 71d43b7..1d5a75b 100644
@@ -231,7 +231,7 @@ static const char usage_message[] =
   "--route-noexec  : Don't add routes automatically.  Instead pass routes to\n"
   "                  --route-up script using environmental variables.\n"
   "--route-nopull  : When used with --client or --pull, accept options pushed\n"
-  "                  by server EXCEPT for routes.\n"
+  "                  by server EXCEPT for routes and dhcp options.\n"
   "--allow-pull-fqdn : Allow client to pull DNS names from server for\n"
   "                    --ifconfig, --route, and --route-gateway.\n"
   "--redirect-gateway [flags]: Automatically execute routing\n"
Signature · 1 line
1.7.7.5 (Apple Git-26)
#12 lore raw

Re: [Openvpn-devel] [PATCH 4/8] Explain that route-nopull also causes the client to ignore dhcp options.

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

Quoted 5 lines · Arne Schwabe

| 3 ++- src/openvpn/options.c | 2 +- 2 files changed, 3
insertions(+), 2 deletions(-)

ACK. Will apply this soonish, unless somebody objects during next week.

kind regards,

David Sommerseth

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.11 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iEYEARECAAYFAk/KQjYACgkQDC186MBRfrpXjwCgoVxqsPeeJFD3LFpPwj1qcdUI 2b0AnA1DPg8fuQM517AWYe1wLeaFdDSs
=mPGy
-----END PGP SIGNATURE-----

#13 lore raw

Re: [Openvpn-devel] [PATCH 4/8] Explain that route-nopull also causes the client to ignore dhcp options.

In reply to Arne Schwabe
Quoted 6 lines · Arne Schwabe
---
 doc/openvpn.8         |    3 ++-
 src/openvpn/options.c |    2 +-
 2 files changed, 3 insertions(+), 2 deletions(-)

Applied to master.

commit 97235cc7077630004e11d6d44862c3bb7e49a40e

Author: Arne Schwabe <[email protected]...>
Date:   Sat Jun 2 15:03:39 2012 +0200
    Explain that route-nopull also causes the client to ignore dhcp options.
    
    Signed-off-by: Arne Schwabe <[email protected]...>
    Acked-by: David Sommerseth <[email protected]...>
    Message-Id: [email protected]...
    URL: http://article.gmane.org/gmane.network.openvpn.devel/6649
    Signed-off-by: David Sommerseth <[email protected]...>

kind regards,

David Sommerseth

#14 lore raw

PATCH 5/8 Add the name of the context where option is not allowed to the error message.

In reply to Arne Schwabe

Changes

Options error: option 'dhcp-option' cannot be used in this context

to

P:Options error: option 'dhcp-option' cannot be used in this context ([PUSH-OPTIONS])

---
 src/openvpn/options.c |    5 +++--
 1 files changed, 3 insertions(+), 2 deletions(-)
src/openvpn/options.c +3 -2
index 1d5a75b..9ed4ff5 100644
@@ -4021,10 +4021,11 @@ void options_string_import (struct options *options,
 
 #if P2MP
 
-#define VERIFY_PERMISSION(mask) { if (!verify_permission(p[0], (mask), permission_mask, option_types_found, msglevel)) goto err; }
+#define VERIFY_PERMISSION(mask) { if (!verify_permission(p[0], file, (mask), permission_mask, option_types_found, msglevel)) goto err; }
 
 static bool
 verify_permission (const char *name,
+		   const char* file,
 		   const unsigned int type,
 		   const unsigned int allowed,
 		   unsigned int *found,
@@ -4032,7 +4033,7 @@ verify_permission (const char *name,
 {
   if (!(type & allowed))
     {
-      msg (msglevel, "option '%s' cannot be used in this context", name);
+      msg (msglevel, "option '%s' cannot be used in this context (%s)", name, file);
       return false;
     }
   else
Signature · 1 line
1.7.7.5 (Apple Git-26)
#15 lore raw

Re: [Openvpn-devel] [PATCH 5/8] Add the name of the context where option is not allowed to the error message.

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

Quoted 4 lines · Arne Schwabe

Changes

Options error: option 'dhcp-option' cannot be used in this context

Quoted 9 lines

to

P:Options error: option 'dhcp-option' cannot be used in this context ([PUSH-OPTIONS])

src/openvpn/options.c | 5 +++-- 1 files changed, 3
insertions(+), 2 deletions(-)

ACK. Will apply this soonish, unless somebody objects during next week.

kind regards,

David Sommerseth

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.11 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iEYEARECAAYFAk/KQn0ACgkQDC186MBRfrri5ACaA9G/8yK0yb3t/y7ZIKddKMIg KT8AoKftgquym05zyrXimZk2DZc+9d6J
=nQLp
-----END PGP SIGNATURE-----

#16 lore raw

Re: [Openvpn-devel] [PATCH 5/8] Add the name of the context where option is not allowed to the error message.

In reply to Arne Schwabe
Quoted 4 lines · Arne Schwabe

Changes

Options error: option 'dhcp-option' cannot be used in this context

Quoted 9 lines

to

P:Options error: option 'dhcp-option' cannot be used in this context ([PUSH-OPTIONS])

---
 src/openvpn/options.c |    5 +++--
 1 files changed, 3 insertions(+), 2 deletions(-)

Applied to master.

commit e656b995b44fab0b8290c6c2a4a73079b3f9813b

Author: Arne Schwabe <[email protected]...>
Date:   Sat Jun 2 15:03:40 2012 +0200
    Add the name of the context where option is not allowed to the error message.

    Signed-off-by: Arne Schwabe <[email protected]...>
    Acked-by: David Sommerseth <[email protected]...>
    Message-Id: [email protected]...
    URL: http://article.gmane.org/gmane.network.openvpn.devel/6646
    Signed-off-by: David Sommerseth <[email protected]...>

kind regards,

David Sommerseth

#17 lore raw

PATCH 6/8 Allow routes to be set before opening tun, similar to ifconfig before opening tun

In reply to Arne Schwabe
---
 src/openvpn/init.c |   11 +++++++++--
 src/openvpn/tun.h  |   11 +++++++++++
 2 files changed, 20 insertions(+), 2 deletions(-)
src/openvpn/init.c +9 -2
index 61ced5d..a25de5d 100644
@@ -1478,6 +1478,13 @@ do_open_tun (struct context *c)
 	  do_ifconfig (c->c1.tuntap, guess, TUN_MTU_SIZE (&c->c2.frame), c->c2.es);
 	}
 
+      /* possibly add routes */
+      if(ifconfig_order() == ROUTE_BEFORE_TUN) {
+		  /* Ignore route_delay, would cause ROUTE_BEFORE_TUN to be ignored */
+		  do_route (&c->options, c->c1.route_list, c->c1.route_ipv6_list,
+					c->c1.tuntap, c->plugins, c->c2.es);
+    }
+
       /* open the tun device */
       open_tun (c->options.dev, c->options.dev_type, c->options.dev_node,
 		c->c1.tuntap);
@@ -1509,7 +1516,7 @@ do_open_tun (struct context *c)
 		   c->c2.es);
 
       /* possibly add routes */
-      if (!c->options.route_delay_defined)
+      if ((ifconfig_order() == ROUTE_AFTER_TUN) && (!c->options.route_delay_defined))
 	do_route (&c->options, c->c1.route_list, c->c1.route_ipv6_list,
 		  c->c1.tuntap, c->plugins, c->c2.es);
 
@@ -1717,7 +1724,7 @@ do_up (struct context *c, bool pulled_options, unsigned int option_types_found)
 #endif
 
 	  /* if --route-delay was specified, start timer */
-	  if (c->options.route_delay_defined)
+	  if ((ifconfig_order() == ROUTE_AFTER_TUN) && c->options.route_delay_defined)
 	    {
 	      event_timeout_init (&c->c2.route_wakeup, c->options.route_delay, now);
 	      event_timeout_init (&c->c2.route_wakeup_expire, c->options.route_delay + c->options.route_delay_window, now);
src/openvpn/tun.h +11 -0
index 9bd990f..60d75c8 100644
@@ -297,6 +297,17 @@ ifconfig_order(void)
 #endif
 }
 
+#define ROUTE_BEFORE_TUN 0
+#define ROUTE_AFTER_TUN 1
+#define ROUTE_ORDER_DEFAULT ROUTE_AFTER_TUN
+
+static inline int
+route_order(void)
+{
+    return ROUTE_ORDER_DEFAULT;
+}
+
+
 #ifdef WIN32
 
 #define TUN_PASS_BUFFER
Signature · 1 line
1.7.7.5 (Apple Git-26)
#18 lore raw

Re: [Openvpn-devel] [PATCH 6/8] Allow routes to be set before opening tun, similar to ifconfig before opening tun

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

See comments below.

Quoted 40 lines · Arne Schwabe

src/openvpn/init.c | 11 +++++++++-- src/openvpn/tun.h | 11
+++++++++++ 2 files changed, 20 insertions(+), 2 deletions(-)

src/openvpn/init.c index renamed from src/openvpn/init.c +0 -0

61ced5d..a25de5d 100644 --- a/src/openvpn/init.c +++ b/src/openvpn/init.c @@ -1478,6 +1478,13 @@ do_open_tun (struct context *c) do_ifconfig (c->c1.tuntap, guess, TUN_MTU_SIZE (&c->c2.frame), c->c2.es); }

+ /* possibly add routes */ + if(ifconfig_order() ==
ROUTE_BEFORE_TUN) { + /* Ignore route_delay, would cause
ROUTE_BEFORE_TUN to be ignored */ + do_route (&c->options,
c->c1.route_list, c->c1.route_ipv6_list, + c->c1.tuntap,
c->plugins, c->c2.es); + } + /* open the tun device */ open_tun
(c->options.dev, c->options.dev_type, c->options.dev_node, c->c1.tuntap); @@ -1509,7 +1516,7 @@ do_open_tun (struct context *c) c->c2.es);

/* possibly add routes */ - if
(!c->options.route_delay_defined) + if ((ifconfig_order() ==
ROUTE_AFTER_TUN) && (!c->options.route_delay_defined)) do_route (&c->options, c->c1.route_list, c->c1.route_ipv6_list, c->c1.tuntap, c->plugins, c->c2.es);

src/openvpn/init.c index renamed from src/openvpn/init.c +0 -0
@@ -1717,7 +1724,7 @@ do_up (struct context *c, bool

pulled_options, unsigned int option_types_found) #endif

/* if --route-delay was specified, start timer */ - if
(c->options.route_delay_defined) + if ((ifconfig_order() ==
ROUTE_AFTER_TUN) && c->options.route_delay_defined) { event_timeout_init (&c->c2.route_wakeup, c->options.route_delay, now); event_timeout_init (&c->c2.route_wakeup_expire, c->options.route_delay + c->options.route_delay_window, now); diff --git a/src/openvpn/tun.h b/src/openvpn/tun.h index 9bd990f..60d75c8 100644 --- a/src/openvpn/tun.h +++ b/src/openvpn/tun.h @@ -297,6 +297,17 @@ ifconfig_order(void) #endif }

Quoted 3 lines

+#define ROUTE_BEFORE_TUN 0 +#define ROUTE_AFTER_TUN 1 +#define ROUTE_ORDER_DEFAULT ROUTE_AFTER_TUN + +static inline int
+route_order(void) +{ + return ROUTE_ORDER_DEFAULT; +}

Is there any reason why it is an advantage of calling a function returning a macro value, instead of just redefine the ROUTE_ORDER_DEFAULT macro for those platforms who need ROUTE_AFTER_TUN and use the ROUTE_ORDER_DEFAULT macro directly where needed?

And for type-safety, would it be better to use an typedef enum instead?

kind regards,

David Sommerseth

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.11 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iEYEARECAAYFAk/KQOMACgkQDC186MBRfrr7vgCdGqn+OtLv5A0pI6bghIX+IQSQ 8RcAn0zxo+W13KLq0Y8wkHuTsQUZ8eh6
=nlce
-----END PGP SIGNATURE-----

#19 lore raw

Re: [Openvpn-devel] [PATCH 6/8] Allow routes to be set before opening tun, similar to ifconfig before opening tun

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

Am 02.06.12 18:35, schrieb David Sommerseth:

Quoted 53 lines

See comments below.

Quoted 39 lines · Arne Schwabe

src/openvpn/init.c | 11 +++++++++-- src/openvpn/tun.h | 11 +++++++++++ 2 files changed, 20 insertions(+), 2 deletions(-)

src/openvpn/init.c index renamed from src/openvpn/init.c +0 -0

61ced5d..a25de5d 100644 --- a/src/openvpn/init.c +++ b/src/openvpn/init.c @@ -1478,6 +1478,13 @@ do_open_tun (struct context *c) do_ifconfig (c->c1.tuntap, guess, TUN_MTU_SIZE (&c->c2.frame), c->c2.es); }

+ /* possibly add routes */ + if(ifconfig_order() == ROUTE_BEFORE_TUN) { + /* Ignore route_delay, would cause ROUTE_BEFORE_TUN to be ignored */ + do_route (&c->options, c->c1.route_list, c->c1.route_ipv6_list, + c->c1.tuntap, c->plugins, c->c2.es); + } + /* open the tun device */ open_tun (c->options.dev, c->options.dev_type, c->options.dev_node, c->c1.tuntap); @@ -1509,7 +1516,7 @@ do_open_tun (struct context *c) c->c2.es);

/* possibly add routes */ - if (!c->options.route_delay_defined) + if ((ifconfig_order() == ROUTE_AFTER_TUN) && (!c->options.route_delay_defined)) do_route (&c->options, c->c1.route_list, c->c1.route_ipv6_list, c->c1.tuntap, c->plugins, c->c2.es);

src/openvpn/init.c index renamed from src/openvpn/init.c +0 -0
@@ -1717,7 +1724,7 @@ do_up (struct context *c, bool

pulled_options, unsigned int option_types_found) #endif

/* if --route-delay was specified, start timer */ - if (c->options.route_delay_defined) + if ((ifconfig_order() == ROUTE_AFTER_TUN) && c->options.route_delay_defined) { event_timeout_init (&c->c2.route_wakeup, c->options.route_delay, now); event_timeout_init (&c->c2.route_wakeup_expire, c->options.route_delay + c->options.route_delay_window, now); diff --git a/src/openvpn/tun.h b/src/openvpn/tun.h index 9bd990f..60d75c8 100644 --- a/src/openvpn/tun.h +++ b/src/openvpn/tun.h @@ -297,6 +297,17 @@ ifconfig_order(void) #endif }

Quoted 3 lines

+#define ROUTE_BEFORE_TUN 0 +#define ROUTE_AFTER_TUN 1 +#define ROUTE_ORDER_DEFAULT ROUTE_AFTER_TUN + +static inline int +route_order(void) +{ + return ROUTE_ORDER_DEFAULT; +}

Is there any reason why it is an advantage of calling a function returning a macro value, instead of just redefine the ROUTE_ORDER_DEFAULT macro for those platforms who need ROUTE_AFTER_TUN and use the ROUTE_ORDER_DEFAULT macro directly where needed?

Quoted 1 line

And for type-safety, would it be better to use an typedef enum instead?

I just followed the IFCONFIG_AFTER_TUN/BEFORE_TUN which already is in place. Just using the defines is fine for me too but it should be consistend for both options.

Arne

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.11 (Darwin)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iEYEARECAAYFAk/KQzsACgkQe8+cMNS4zRdNcQCdGJrXIhLQUvLmYb2/2leTqgCg 9+UAoKGiz47XWzyDOPQR2ZfJBUJDeaJ7
=lqyN
-----END PGP SIGNATURE-----

#20 lore raw

Re: [Openvpn-devel] [PATCH 6/8] Allow routes to be set before opening tun, similar to ifconfig before opening tun

Hi,

Quoted 8 lines · Arne Schwabe
Quoted 5 lines

returning a macro value, instead of just redefine the ROUTE_ORDER_DEFAULT macro for those platforms who need ROUTE_AFTER_TUN and use the ROUTE_ORDER_DEFAULT macro directly where needed?

And for type-safety, would it be better to use an typedef enum instead?

I just followed the IFCONFIG_AFTER_TUN/BEFORE_TUN which already is in place. Just using the defines is fine for me too but it should be consistend for both options.

I've always wondered why IFCONFIG_AFTER_TUN has been done that way, as it seemed confusing - it sort of looks like it was done with the option to change it at run-time later, but I don't really see where this would make sense. Today all unix platforms have one variant, Windows has the other one, and reversing order for either isn't going to work...

So I would opt for "no function for ROUTE_ORDER_DEFAULT", and put cleaning up the IFCONFIG_AFTER_TUN stuff on someones TODO list :-) - but we could check that one with James...

(JFTR, OpenBSD used to have IFCONFIG_BEFORE_TUN as the single exception besides Windows - but that looked like "the one who did the port not understanding how tun(4) works on OpenBSD" - I changed it to IFCONFIG_AFTER_TUN and that cleaned up quite a bit of special cases in the code, and it works better than ever before :-) )

What I'm also not really happy in the current implementation of both options is that all order variants get compiled-in, instead of just having what the platform in question needs. But that's more intrustive surgery...

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]...
#21 lore raw

Re: [Openvpn-devel] [PATCH 6/8] Allow routes to be set before opening tun, similar to ifconfig before opening tun

Gert,
You can see the tun branch[1].
I changed this per platform.
Alon.

[1] https://github.com/alonbl/openvpn/commits/tun
[2] https://github.com/alonbl/openvpn/commit/4d2fc1f21e6c5cba163ec481119370235c736672

Quoted 52 lines · Gert Doering

Hi,

Quoted 8 lines · Arne Schwabe
Quoted 5 lines

returning a macro value, instead of just redefine the ROUTE_ORDER_DEFAULT macro for those platforms who need ROUTE_AFTER_TUN and use the ROUTE_ORDER_DEFAULT macro directly where needed?

And for type-safety, would it be better to use an typedef enum instead?

I just followed the IFCONFIG_AFTER_TUN/BEFORE_TUN which already is in place. Just using the defines is fine for me too but it should be consistend for both options.

I've always wondered why IFCONFIG_AFTER_TUN has been done that way, as it seemed confusing - it sort of looks like it was done with the option to change it at run-time later, but I don't really see where this would make sense.  Today all unix platforms have one variant, Windows has the other one, and reversing order for either isn't going to work...

So I would opt for "no function for ROUTE_ORDER_DEFAULT", and put cleaning up the IFCONFIG_AFTER_TUN stuff on someones TODO list :-) - but we could check that one with James...

(JFTR, OpenBSD used to have IFCONFIG_BEFORE_TUN as the single exception besides Windows - but that looked like "the one who did the port not understanding how tun(4) works on OpenBSD" - I changed it to IFCONFIG_AFTER_TUN and that cleaned up quite a bit of special cases in the code, and it works better than ever before :-) )

What I'm also not really happy in the current implementation of both options is that all order variants get compiled-in, instead of just having what the platform in question needs.  But that's more intrustive surgery...

gert
--
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]...

------------------------------------------------------------------------------
Live Security Virtual Conference
Exclusive live event will cover all the ways today's security and threat landscape has changed and how IT managers can respond. Discussions will include endpoint security, mobile security and the latest in malware threats. http://www.accelacomm.com/jaw/sfrnl04242012/114/50122263/
_______________________________________________
Openvpn-devel mailing list [email protected]
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

#22 lore raw

PATCH 7/8 add ability to send/receive file descriptors via management interface, only used in android so. For now under #ifdef ANDROID

In reply to Arne Schwabe
---
 src/openvpn/manage.c |   88 ++++++++++++++++++++++++++++++++++++++++++++++++++
 src/openvpn/manage.h |    4 ++
 2 files changed, 92 insertions(+), 0 deletions(-)
src/openvpn/manage.c +88 -0
index 1dddd41..cc22208 100644
@@ -1805,6 +1805,79 @@ man_io_error (struct management *man, const char *prefix)
     return false;
 }
 
+#ifdef TARGET_ANDROID
+static ssize_t write_fd (int fd, void *ptr, size_t nbytes, int flags, int sendfd)
+{
+    struct msghdr msg;
+    struct iovec iov[1];
+    
+    union {
+        struct cmsghdr cm;
+        char    control[CMSG_SPACE(sizeof(int))];
+    } control_un;
+    struct cmsghdr *cmptr;
+    
+    msg.msg_control = control_un.control;
+    msg.msg_controllen = sizeof(control_un.control);
+    
+    cmptr = CMSG_FIRSTHDR(&msg);
+    cmptr->cmsg_len = CMSG_LEN(sizeof(int));
+    cmptr->cmsg_level = SOL_SOCKET;
+    cmptr->cmsg_type = SCM_RIGHTS;
+    *((int *) CMSG_DATA(cmptr)) = sendfd;
+    
+    msg.msg_name = NULL;
+    msg.msg_namelen = 0;
+    
+    iov[0].iov_base = ptr;
+    iov[0].iov_len = nbytes;
+    msg.msg_iov = iov;
+    msg.msg_iovlen = 1;
+    
+    return (sendmsg(fd, &msg, flags));
+}
+
+static ssize_t read_fd(int fd, void *ptr, size_t nbytes, int flags, int *recvfd)
+{
+    struct msghdr msghdr;
+    struct iovec iov[1];
+    ssize_t n;
+    
+    union {
+        struct cmsghdr cm;
+        char     control[CMSG_SPACE(sizeof (int))];
+    } control_un;
+    struct cmsghdr  *cmptr;
+    
+    msghdr.msg_control  = control_un.control;
+    msghdr.msg_controllen = sizeof(control_un.control);
+    
+    msghdr.msg_name = NULL;
+    msghdr.msg_namelen = 0;
+    
+    iov[0].iov_base = ptr;
+    iov[0].iov_len = nbytes;
+    msghdr.msg_iov = iov;
+    msghdr.msg_iovlen = 1;
+    
+    if ( (n = recvmsg(fd, &msghdr, flags)) <= 0)
+        return (n);
+    
+    if ( (cmptr = CMSG_FIRSTHDR(&msghdr)) != NULL &&
+        cmptr->cmsg_len == CMSG_LEN(sizeof(int))) {
+        if (cmptr->cmsg_level != SOL_SOCKET)
+            msg (M_ERR, "control level != SOL_SOCKET");
+        if (cmptr->cmsg_type != SCM_RIGHTS)
+            msg (M_ERR, "control type != SCM_RIGHTS");
+        *recvfd = *((int *) CMSG_DATA(cmptr));
+    } else
+        *recvfd = -1;           /* descriptor was not passed */
+    
+    return (n);
+}
+#endif
+
+
 static int
 man_read (struct management *man)
 {
@@ -1814,7 +1887,15 @@ man_read (struct management *man)
   unsigned char buf[256];
   int len = 0;
 
+#ifdef TARGET_ANDROID
+  int fd;
+  len = read_fd (man->connection.sd_cli, buf, sizeof (buf), MSG_NOSIGNAL, &fd);
+  if(fd >= 0) 
+     man->connection.lastfdreceived = fd;
+#else
   len = recv (man->connection.sd_cli, buf, sizeof (buf), MSG_NOSIGNAL);
+#endif
+
   if (len == 0)
     {
       man_reset_client_socket (man, false);
@@ -1891,6 +1972,13 @@ man_write (struct management *man)
   if (buf && BLEN (buf))
     {
       const int len = min_int (size_hint, BLEN (buf));
+#ifdef TARGET_ANDROID
+      if (man->connection.fdtosend > 0) 
+        {
+         sent = write_fd (man->connection.sd_cli, BPTR (buf), len, MSG_NOSIGNAL,man->connection.fdtosend);
+            man->connection.fdtosend = -1;
+        } else
+#endif
       sent = send (man->connection.sd_cli, BPTR (buf), len, MSG_NOSIGNAL);
       if (sent >= 0)
 	{
src/openvpn/manage.h +4 -0
index f681f8d..991b489 100644
@@ -303,6 +303,10 @@ struct man_connection {
 #ifdef MANAGMENT_EXTERNAL_KEY
   struct buffer_list *rsa_sig;
 #endif
+#ifdef TARGET_ANDROID
+    int fdtosend;
+    int lastfdreceived;
+#endif
 };
 
 struct management
Signature · 1 line
1.7.7.5 (Apple Git-26)
#23 lore raw

PATCH 8/8 Android platform specific changes.

In reply to Arne Schwabe
---
 src/openvpn/options.c |    6 +++
 src/openvpn/route.c   |   25 +++++++++++++-
 src/openvpn/socket.c  |   10 +++++
 src/openvpn/ssl.c     |    2 +
 src/openvpn/syshead.h |    2 +-
 src/openvpn/tun.c     |   88 ++++++++++++++++++++++++++++++++++++++++++++++++-
 src/openvpn/tun.h     |    8 ++++-
 7 files changed, 136 insertions(+), 5 deletions(-)
src/openvpn/options.c +6 -0
index 9ed4ff5..7b7fc68 100644
@@ -1129,7 +1129,9 @@ show_tuntap_options (const struct tuntap_options *o)
 }
 
 #endif
+#endif
 
+#if defined(WIN32) || defined(TARGET_ANDROID) 
 static void
 dhcp_option_address_parse (const char *name, const char *parm, in_addr_t *array, int *len, int msglevel)
 {
@@ -6060,6 +6062,8 @@ add_option (struct options *options,
       to->ip_win32_type = index;
       to->ip_win32_defined = true; 
     }
+#endif
+#if defined(WIN32) || defined(TARGET_ANDROID)
   else if (streq (p[0], "dhcp-option") && p[1])
     {
       struct tuntap_options *o = &options->tuntap_options;
@@ -6111,6 +6115,8 @@ add_option (struct options *options,
 	}
       o->dhcp_options = true;
     }
+#endif
+#ifdef WIN32
   else if (streq (p[0], "show-adapters"))
     {
       VERIFY_PERMISSION (OPT_P_GENERAL);
src/openvpn/route.c +23 -2
index 7c25c77..7c02d6f 100644
@@ -1344,6 +1344,17 @@ add_route (struct route *r,
   argv_msg (D_ROUTE, &argv);
   status = openvpn_execve_check (&argv, es, 0, "ERROR: Linux route add command failed");
 
+#elif defined (TARGET_ANDROID)
+
+    struct user_pass up;    
+    struct buffer out = alloc_buf_gc (64, &gc);
+
+    buf_printf (&out, "%s %s", network, netmask);
+
+    strcpy(up.username, buf_bptr(&out));
+    management_query_user_pass(management, &up , "ROUTE", GET_USER_PASS_NEED_OK,(void*) 0);
+
+
 #elif defined (WIN32)
   {
     DWORD ai = TUN_ADAPTER_INDEX_INVALID;
@@ -1597,6 +1608,15 @@ add_route_ipv6 (struct route_ipv6 *r6, const struct tuntap *tt, unsigned int fla
   argv_msg (D_ROUTE, &argv);
   status = openvpn_execve_check (&argv, es, 0, "ERROR: Linux route -6/-A inet6 add command failed");
 
+#elif defined (TARGET_ANDROID)
+    struct user_pass up;    
+    struct buffer out = alloc_buf_gc (64, &gc);
+    
+    buf_printf (&out, "%s/%d", network, r6->netbits);
+    
+    strcpy(up.username, buf_bptr(&out));
+    management_query_user_pass(management, &up , "ROUTE6", GET_USER_PASS_NEED_OK,(void*) 0);
+
 #elif defined (WIN32)
 
   /* netsh interface ipv6 add route 2001:db8::/32 MyTunDevice */
@@ -1847,7 +1867,8 @@ delete_route (struct route *r,
 
   argv_msg (D_ROUTE, &argv);
   openvpn_execve_check (&argv, es, 0, "ERROR: OpenBSD/NetBSD route delete command failed");
-
+#elif defined(TARGET_ANDROID)
+  msg (M_NONFATAL, "Sorry, deleting routes on Android is not possible. The VpnService API allows routes to be set on connect only.");
 #else
   msg (M_FATAL, "Sorry, but I don't know how to do 'route' commands on this operating system.  Try putting your routes in a --route-up script");
 #endif
@@ -2371,7 +2392,7 @@ show_routes (int msglev)
   gc_free (&gc);
 }
 
-#elif defined(TARGET_LINUX)
+#elif defined(TARGET_LINUX) || defined(TARGET_ANDROID)
 
 void
 get_default_gateway (struct route_gateway_info *rgi)
src/openvpn/socket.c +10 -0
index 6b1f8d2..d417172 100644
@@ -38,6 +38,7 @@
 #include "ps.h"
 #include "manage.h"
 #include "misc.h"
+#include "manage.h"
 
 #include "memdbg.h"
 
@@ -959,6 +960,15 @@ create_socket (struct link_socket *sock)
     {
       ASSERT (0);
     }
+#ifdef TARGET_ANDROID
+    struct user_pass up;
+    strcpy(up.username ,__func__);
+    management->connection.fdtosend = sock->sd;
+    management_query_user_pass(management, &up , "PROTECTFD", GET_USER_PASS_NEED_OK,(void*) 0);
+
+
+#endif
+    
 }
 
 /*
src/openvpn/ssl.c +2 -0
index 19512c0..9f570b9 100644
@@ -1650,6 +1650,8 @@ push_peer_info(struct buffer *buf, struct tls_session *session)
       buf_printf (&out, "IV_PLAT=netbsd\n");
 #elif defined(TARGET_FREEBSD)
       buf_printf (&out, "IV_PLAT=freebsd\n");
+#elif defined(TARGET_ANDROID)
+      buf_printf(&out, "IV_PLAT=android\n");
 #elif defined(WIN32)
       buf_printf (&out, "IV_PLAT=win\n");
 #endif
src/openvpn/syshead.h +1 -1
index f5c8774..ed00e82 100644
@@ -212,7 +212,7 @@
 #include <net/if_tap.h>
 #endif
 
-#ifdef TARGET_LINUX
+#if defined(TARGET_LINUX) || defined (TARGET_ANDROID)
 
 #if defined(HAVE_NETINET_IF_ETHER_H)
 #include <netinet/if_ether.h>
src/openvpn/tun.c +87 -1

107 lines not on this page · raw message

src/openvpn/tun.h +7 -1
index 60d75c8..0f4f012 100644
@@ -38,7 +38,7 @@
 #include "proto.h"
 #include "misc.h"
 
-#ifdef WIN32
+#if defined(WIN32) || defined(TARGET_ANDROID)
 
 #define TUN_ADAPTER_INDEX_INVALID ((DWORD)-1)
 
@@ -292,6 +292,8 @@ ifconfig_order(void)
   return IFCONFIG_AFTER_TUN_OPEN;
 #elif defined(WIN32)
   return IFCONFIG_BEFORE_TUN_OPEN;
+#elif defined(TARGET_ANDROID)
+  return IFCONFIG_BEFORE_TUN_OPEN;
 #else
   return IFCONFIG_DEFAULT;
 #endif
@@ -304,7 +306,11 @@ ifconfig_order(void)
 static inline int
 route_order(void)
 {
+#if defined(TARGET_ANDROID)
+    return ROUTE_BEFORE_TUN;
+#else
     return ROUTE_ORDER_DEFAULT;
+#endif
 }
 
 
Signature · 1 line
1.7.7.5 (Apple Git-26)

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

#25 lore raw

Re: [Openvpn-devel] [PATCH 8/8] Android platform specific changes.

Am 07.06.12 18:44, schrieb Adriaan de Jong:

Quoted 11 lines

Hi Arne,

Could you please split this patch up a little further? As it is, it performs a number of functions, which, although related to Android would be easier to ack if they were separate.

Am I correct in noting that you use management_query_user_pass() to send queries across the management interface? If so, that begs an open question to the community: is there a better way of communicating across the management interface?

Yes. At the moment almost all information is passed with management_query_user_pass, I have no split up the patch further since we should first discussed the best way to pass information around. (See also the lengthy discussion when I first submitted an Android patch)

Arne

#26 lore raw

Re: [Openvpn-devel] [PATCH 8/8] Android platform specific changes.

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

Quoted 12 lines · Arne Schwabe

Am 07.06.12 18:44, schrieb Adriaan de Jong:

Quoted 11 lines

Hi Arne,

Could you please split this patch up a little further? As it is, it performs a number of functions, which, although related to Android would be easier to ack if they were separate.

Am I correct in noting that you use management_query_user_pass() to send queries across the management interface? If so, that begs an open question to the community: is there a better way of communicating across the management interface?

Quoted 5 lines

Yes. At the moment almost all information is passed with management_query_user_pass, I have no split up the patch further since we should first discussed the best way to pass information around. (See also the lengthy discussion when I first submitted an Android patch)

Your splitting in this round has been very good. But it would be good if patch 8 and maybe some of 7 could be split up a bit more. Small contained changes are far easier to review. And it might be easier to do modifications later on, if needed too.

One thing though, which I don't like so much is that you "abuse" management_query_user_pass() for passing other types of data. That makes it harder to understand what you are doing and why. When reviewing I wondered for a little while why you where doing so much management user/pass stuff on the client - until I saw what you populated the user_pass structs with.

Would it be possible to rework the management_query_* stuff to have a more generic API? Where you could pass over the information you need to pass and then keep the management_query_user_pass() for username/password stuff. This will help understanding the overall code far better. The management_query_user_pass() may surely use a more flexible/generic backend, to avoid having code duplication.

I haven't been able to completely comprehend all you do in your patch yet, so it might be more things too. But this is at least a starter of some feedback.

kind regards,

David Sommerseth
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.11 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iEYEARECAAYFAk/XLQ0ACgkQDC186MBRfrp8fgCgg0mTKsbYpghVPB8Msiqv8Hhr vn0AnA9COKbvR3EGOaL5NRmbEUdVryNP
=rtwU
-----END PGP SIGNATURE-----