Openvpn for Android patch set
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)
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
@@ -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)
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
Signed-off-by Arne Schwabe <[email protected]...> ---
src/openvpn/options.c| 1 + 1 files changed, 1 insertions(+), 0
deletions(-)src/openvpn/options.c index renamed from src/openvpn/options.c +0
-008f9cb2..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-----
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
Signed-off-by Arne Schwabe <[email protected]...> ---
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
-008f9cb2..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-----
Re: [Openvpn-devel] [PATCH 1/8] Silence Port is now 1194 IANA warning if remote option with port is used
In reply to David SommersethHI,
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_usedmember completely too. If you do:$ git grep -n port_option_usedin 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]...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
@@ -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)
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
Signed-off-by Arne Schwabe <[email protected]...>--- 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-----
David Sommerseth
AppliedRe: [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 SchwabeQuoted 6 lines · Arne Schwabe
Signed-off-by Arne Schwabe <[email protected]...>--- 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
PATCH 3/8 Only use tmpdir if tmp_dir is really used.
In reply to Arne SchwabeThis 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
@@ -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
@@ -403,7 +403,9 @@ struct optionsstruct plugin_option_list *plugin_list;#endif +#ifdef ENABLE_TMPDIR const char *tmp_dir; +#endif #if P2MP
src/openvpn/syshead.h +1 -0
@@ -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)
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_SERVERhas been enabled too.Signed-off-by Arne Schwabe <[email protected]...> ---
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
-08e5d3b9..71d43b7 100644 ---
a/src/openvpn/options.c+++b/src/openvpn/options.c@@ -862,6 +862,7 @@init_options(struct options *o, const boolinit_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 boolinit_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 +#ifENABLE_TMPDIRerrs |=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 --gita/src/openvpn/options.hb/src/openvpn/options.hindex cc3e47a..50d42fb 100644 ---a/src/openvpn/options.h+++b/src/openvpn/options.h@@ -403,7 +403,9 @@ struct options structplugin_option_list*plugin_list; #endif+#ifdef
ENABLE_TMPDIRconst char*tmp_dir; +#endif#if P2MP
src/openvpn/syshead.h index renamed from src/openvpn/syshead.h +0
-03337764..f5c8774 100644 ---
a/src/openvpn/syshead.h+++b/src/openvpn/syshead.h@@ -506,6 +506,7 @@socket_defined(constsocket_descriptor_tsd)
Quoted 2 lines
#if P2MP && !defined(ENABLE_CLIENT_ONLY) #define
P2MP_SERVER1 +#defineENABLE_TMPDIR1 #else #defineP2MP_SERVER0 #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-----
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.hin v2.4, it might rather make sense to try to avoid extendingsyshead.h. I see the benefit from code clarity point of view. But I'd probably just recommend to check forP2MP_SERVER, with a comment that --tmp-dir is only used in theP2MP_SERVERcode; instead of adding theENABLE_TMPDIRmacro.
+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]...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
@@ -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
@@ -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)
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
Signed-off-by Arne Schwabe <[email protected]...> --- doc/openvpn.8| 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-----
David Sommerseth
AppliedRe: [Openvpn-devel] [PATCH 4/8] Explain that route-nopull also causes the client to ignore dhcp options.
In reply to Arne SchwabeQuoted 6 lines · Arne Schwabe
Signed-off-by Arne Schwabe <[email protected]...>--- 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
PATCH 5/8 Add the name of the context where option is not allowed to the error message.
In reply to Arne SchwabeChanges
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
@@ -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)
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])
Signed-off-by Arne Schwabe <[email protected]...> ---
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-----
David Sommerseth
AppliedRe: [Openvpn-devel] [PATCH 5/8] Add the name of the context where option is not allowed to the error message.
In reply to Arne SchwabeQuoted 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])
Signed-off-by Arne Schwabe <[email protected]...>--- 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
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
@@ -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
@@ -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)
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
Signed-off-by Arne Schwabe <[email protected]...> ---
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
-061ced5d..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) { + /* Ignoreroute_delay, would causeROUTE_BEFORE_TUNto 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 intoption_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 --gita/src/openvpn/tun.hb/src/openvpn/tun.hindex 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_TUN0 +#defineROUTE_AFTER_TUN1 +#defineROUTE_ORDER_DEFAULTROUTE_AFTER_TUN+ +static inline int
+route_order(void) +{ + returnROUTE_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-----
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
Signed-off-by Arne Schwabe <[email protected]...> ---
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
-061ced5d..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){ + /* Ignoreroute_delay, would causeROUTE_BEFORE_TUNto 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 intoption_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 --gita/src/openvpn/tun.hb/src/openvpn/tun.hindex 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_TUN0 +#defineROUTE_AFTER_TUN1 +#defineROUTE_ORDER_DEFAULTROUTE_AFTER_TUN+ +static inline int +route_order(void) +{ + returnROUTE_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_DEFAULTmacro for those platforms who needROUTE_AFTER_TUNand use theROUTE_ORDER_DEFAULTmacro 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-----
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_DEFAULTmacro for those platforms who needROUTE_AFTER_TUNand use theROUTE_ORDER_DEFAULTmacro 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]...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_DEFAULTmacro for those platforms who needROUTE_AFTER_TUNand use theROUTE_ORDER_DEFAULTmacro 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_TUNhas 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 theIFCONFIG_AFTER_TUNstuff on someones TODO list :-) - but we could check that one with James...(JFTR, OpenBSD used to have
IFCONFIG_BEFORE_TUNas 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 toIFCONFIG_AFTER_TUNand 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
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
@@ -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
@@ -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)
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
@@ -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
@@ -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 flaargv_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
@@ -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
@@ -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
@@ -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
@@ -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]...
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
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-----