Re: [PATCH 09/10] openvpn: Rewrite plugin to support VPN agent and
encrypted private keys
by Jussi Laakkonen
Hi Daniel,
> Hi Jussi,
>
> On Mon, Nov 11, 2019 at 04:01:53PM +0200, Jussi Laakkonen wrote:
>
> This link will go stale. Would the plugin still work if the OpenVPN
> client doesn't have this patch? Or could it made at least not hard
> depending on it?
>
Plugin does work without the patch but it does not receive the error:
> >PASSWORD:Verification Failed: 'Private Key'
and just keeps trying with the invalid password. Maybe I just change the
link to
https://git.sailfishos.org/mer-core/openvpn/blob/4f4b4af116292a207416c8a9...
that should be more permanent.
> And now some nitpicks :)
>
Well, it can be also called a review. Thanks for commenting :)
>
> IIRC, newer gcc will warn about the 'return' here.
>
Will remove in v2.
>
> One empty line too much.
>
Will remove in v2.
>
> Just use plain normal bools, not the ones from Glib if the we don't
have to.
>
Sure, I'll change in v2.
>
> Hmm, I would prefer to have the g_io_channel_read_line() not part of
> the if condition. Couldn't this function be organized slightly
> different? In many places we use a 'goto err' to untangle this kind of
> code. Just an idea.
>
I'll restructure this a bit.
>
> This information will go stale. Maybe add this to our TODO file?
>
A permalink to the patch could suffice? I'm not sure if this is ConnMan
TODO as it does not involve ConnMan changes, but if you think it is I
can add that. Would README be a better place as there is a section for
OpenVPN (the long URL is a drag, though..)
>
> data->failed_attempts_privatekey++;
>
Yeah.. in v2.
>
> Brackets not really needed.
>
All parts aren't really looked that closely, will fix in v2.
>
> data->connect_attemps++;
Yeah.. in v2.
>
>
> I don't think we should hardcode the /tmp path.
>
Oh, that one. We haven't paid attention to that small thing. Yeah, it
should be defined via Makefile -DTMPDIR as the others are. Will fix in v2.
>
>
> Overall, this is looks pretty good. I know understand your comment
> about to much infrastructure code in the plugin. Indeed moving some of
> this code into a library so other plugins can reuse it makes absolutely
> sense.
>
Thanks. Yes, moving all that code from plugins into one library is a
task in the future (I hope not so distant one).
Cheers,
Jussi
11 months, 2 weeks
Re: [PATCH 03/10] doc: Document VPN agent credential
storage/retrieval options
by Jussi Laakkonen
Hi Daniel,
> Hi,
>
> Maybe add some additiional information on the use case it in
> vpn-overview.txt? I am sure there will be questions on how to use it,
> or what the expected behavior of the UI should be.
>
Sure, I'll write an example case there and add it into v2 of this patch.
I haven't actually even viewed that file before :)
>
> Just a nitpick, indent the "Value" to the same level as
> "Requirement". Initially, I read it as nested value.
>
Will do.
Cheers,
Jussi
11 months, 2 weeks
[PATCH v3 0/4] Remove GDateTime API usage
by Daniel Wagner
As it turns out, the GDateTime was introduced in 2.26 but not the
iso8601 function.
changes since v2:
- moved new code into util helper file
- updated wifi plugin as well
changes since v1:
- added error handling
- use gettimeofday() like Glib does
- backwards compatibility added to string parser
Daniel Wagner (4):
util: Add timestamp helpers
service: Remove GDateTime API usage
wifi: Replace GDateVal API usage
tools: Remove GDateTime API usage
plugins/wifi.c | 12 +++++++-----
src/service.c | 45 ++++++++++-----------------------------------
src/shared/util.c | 44 ++++++++++++++++++++++++++++++++++++++++++++
src/shared/util.h | 6 ++++++
tools/stats-tool.c | 13 +++----------
5 files changed, 70 insertions(+), 50 deletions(-)
--
2.23.0
11 months, 2 weeks
Re: [RFC 1/2] Storage based service retrieval
by Ryll, Jan (GED-SDD2)
Hi,
this RFC seems to fit to my question "what if a wifi network service is not available anymore and I need to remove/clear the credential / Passphrase? How could I achieve this?".
So I read the comments and agree with Patrik. The GetKnownServices() method should return also the currently unavailable/absent networks for which a service folder exists.
In my case there are many service folders like
/var/lib/connman/wifi_38b4d3ffe973_4c4544455f48435f32383434_managed_psk
/var/lib/connman/wifi_38b4d3ffe973_4c45765455f484366f323758_managed_psk
...
This folders should be also removable after the change with the service remove functionality.
Could this be a solution?
Best regards
Jan
11 months, 2 weeks
[PATCH v2 0/2] Remove GDateTime API usage
by Daniel Wagner
As it turns out, the GDateTime was introduced in 2.26 but not the
iso8601 function.
changes since v1:
- added error handling
- use gettimeofday() like Glib does
- backwards compatibility added to string parser
Daniel Wagner (2):
service: Remove GDateTime API usage
tools: Remove GDateTime API usage
src/service.c | 59 +++++++++++++++++++++++++++++++---------------
tools/stats-tool.c | 13 +++-------
2 files changed, 43 insertions(+), 29 deletions(-)
--
2.23.0
11 months, 2 weeks
[PATCH] l2tp: Unset password as hidden value to avoid property changed signal
by Jussi Laakkonen
If a password is unset with vpn_provider_set_string() a property change
signal will be sent (see vpn/vpn-provider.c:set_string()). This should
be avoided because a VPN agent listening for changes may interpret this
as changing the password into empty string.
This would be the case when authentication fails or when disconnecting.
At the next connection attempt the password retrieved via VPN agent
would be an empty string. Added empty string checks to make sure this
never happens and password is requested from VPN agent when it is also
an empty string.
---
vpn/plugins/l2tp.c | 11 ++++++-----
1 file changed, 6 insertions(+), 5 deletions(-)
diff --git a/vpn/plugins/l2tp.c b/vpn/plugins/l2tp.c
index efe80863..48894aa5 100644
--- a/vpn/plugins/l2tp.c
+++ b/vpn/plugins/l2tp.c
@@ -179,7 +179,8 @@ static int l2tp_notify(DBusMessage *msg, struct vpn_provider *provider)
DBG("authentication failure");
vpn_provider_set_string(provider, "L2TP.User", NULL);
- vpn_provider_set_string(provider, "L2TP.Password", NULL);
+ vpn_provider_set_string_hide_value(provider, "L2TP.Password",
+ NULL);
return VPN_STATE_AUTH_FAILURE;
}
@@ -652,7 +653,7 @@ static int run_connect(struct vpn_provider *provider,
int l2tp_fd, pppd_fd;
int err;
- if (!username || !password) {
+ if (!username || !*username || !password || !*password) {
DBG("Cannot connect username %s password %p",
username, password);
err = -EINVAL;
@@ -723,7 +724,7 @@ static void request_input_cb(struct vpn_provider *provider,
{
struct l2tp_private_data *data = user_data;
- if (!username || !password)
+ if (!username || !*username || !password || !*password)
DBG("Requesting username %s or password failed, error %s",
username, error);
else if (error)
@@ -758,7 +759,7 @@ static int l2tp_connect(struct vpn_provider *provider,
DBG("user %s password %p", username, password);
- if (!username || !password) {
+ if (!username || !*username || !password || !*password) {
struct l2tp_private_data *data;
data = g_try_new0(struct l2tp_private_data, 1);
@@ -805,7 +806,7 @@ static void l2tp_disconnect(struct vpn_provider *provider)
if (!provider)
return;
- vpn_provider_set_string(provider, "L2TP.Password", NULL);
+ vpn_provider_set_string_hide_value(provider, "L2TP.Password", NULL);
connman_agent_cancel(provider);
}
--
2.20.1
11 months, 3 weeks
[PATCH] l2tp: Remove use of lock option for pppd
by Jussi Laakkonen
Connection with L2TP plugin cannot be established if the "lock" option
is used. When running xl2tpd in terminal with the file content created
by the plugin following is reported and connection fails:
xl2tpd -D -C control -c connman-xl2tpd.conf
xl2tpd[17773]: setsockopt recvref[30]: Protocol not available
xl2tpd[17773]: Using l2tp kernel support.
xl2tpd[17773]: xl2tpd version xl2tpd-1.3.8 started on Sailfish PID:17773
xl2tpd[17773]: Written by Mark Spencer, Copyright (C) 1998, Adtran, Inc.
xl2tpd[17773]: Forked by Scott Balmos and David Stipp, (C) 2001
xl2tpd[17773]: Inherited by Jeff McAdams, (C) 2002
xl2tpd[17773]: Forked again by Xelerance (www.xelerance.com) (C) 2006-2016
xl2tpd[17773]: Listening on IP address a.b.c.d, port 32000
xl2tpd[17773]: Connecting to host w.x.y.z, port 1701
xl2tpd[17773]: Connection established to w.x.y.z, 1701. Local: 18554, Remote: 20429 (ref=0/0).
xl2tpd[17773]: Calling on tunnel 18554
xl2tpd[17773]: Call established with w.x.y.z, Local: 42029, Remote: 24137, Serial: 1 (ref=0/0)
xl2tpd[17773]: start_pppd: I'm running:
xl2tpd[17773]: "/usr/sbin/pppd"
xl2tpd[17773]: "plugin"
xl2tpd[17773]: "pppol2tp.so"
xl2tpd[17773]: "pppol2tp"
xl2tpd[17773]: "7"
xl2tpd[17773]: "passive"
xl2tpd[17773]: "nodetach"
xl2tpd[17773]: ":" §
xl2tpd[17773]: "name"
xl2tpd[17773]: "user"
xl2tpd[17773]: "file"
xl2tpd[17773]: "/path/to/l2tp/connman-ppp-option.conf"
/usr/sbin/pppd: In file /path/to/l2tp/connman-ppp-option.conf: unrecognized option 'lock'
xl2tpd[17773]: child_handler : pppd exited for call 24137 with code 2
xl2tpd[17773]: call_close: Call 42029 to w.x.y.z disconnected
xl2tpd[17773]: write_packet: tty is not open yet.
xl2tpd[17773]: Terminating pppd: sending TERM signal to pid 17774
xl2tpd[17773]: get_call: can't find call 42029 in tunnel 18554
(ref=0/0)xl2tpd[17773]: get_call: can't find call 42029 in tunnel 18554
Tested with PPP built from commit
8e77984ac5d7acbe68b2b2f590abd17564c9730d.
---
vpn/plugins/l2tp.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/vpn/plugins/l2tp.c b/vpn/plugins/l2tp.c
index 2255daaa..705c87af 100644
--- a/vpn/plugins/l2tp.c
+++ b/vpn/plugins/l2tp.c
@@ -381,7 +381,6 @@ static int write_pppd_option(struct vpn_provider *provider, int fd)
const char *opt_s;
l2tp_write_option(fd, "nodetach", NULL);
- l2tp_write_option(fd, "lock", NULL);
l2tp_write_option(fd, "logfd", "2");
l2tp_write_option(fd, "usepeerdns", NULL);
l2tp_write_option(fd, "noipdefault", NULL);
--
2.20.1
11 months, 3 weeks
[PATCH 0/2] Remove GDateTime API usage
by Daniel Wagner
As it turns out, the GDateTime was introduced in 2.26 but not the
iso8601 function.
Daniel Wagner (2):
service: Remove GDateTime API usage
tools: Remove GDateTime API usage
src/service.c | 31 +++++++++++++------------------
tools/stats-tool.c | 13 +++----------
2 files changed, 16 insertions(+), 28 deletions(-)
--
2.23.0
11 months, 3 weeks
[PATCH v1 00/11] WireGuard support
by Daniel Wagner
Add WireGuard support to the VPN daemon. This version isn't too bad,
so if no one is speaking up I am going to merge it. We can still clean
up stuff later. The only thing which should be in place from day one
are the config options. Please have a look at them and speak up if you
think these are wrongly named etc.
One thing which needs to be changed after this goes in, are how we
handle the secret data (passwords, etc). Currently, all settings are
showing up in the logs unless the hackish 'hide_value' API is
used. But this is for latter.
changes v1:
- error handling improved
- rewrite of host/ip parsing code
- configuration options overhal
Daniel Wagner (11):
include: Remove unused definition
shared: Remove netlink code
vpn: Remove Host check in plugins
vpn: Add provider only on success to hash table
build: Test for libmnl independent of nftables
vpn: Add WireGuard plugin placeholder
shared: Add Generic Netlink helpers for libmnl
vpn: Add embeddable WireGuard library
vpn: Introduce VPN_FLAG_NO_DAEMON
doc: Document WireGuard VPN plugin
vpn: Add WireGuard support
Makefile.am | 13 +-
Makefile.plugins | 25 +
configure.ac | 23 +-
doc/vpn-config-format.txt | 22 +
include/provider.h | 2 -
src/shared/mnlg.c | 325 ++++++++++++
src/shared/mnlg.h | 27 +
src/shared/netlink.c | 666 -------------------------
src/shared/netlink.h | 53 --
tools/netlink-test.c | 123 -----
vpn/plugins/libwireguard.c | 998 +++++++++++++++++++++++++++++++++++++
vpn/plugins/openconnect.c | 7 -
vpn/plugins/openvpn.c | 6 -
vpn/plugins/pptp.c | 8 +-
vpn/plugins/vpn.c | 51 +-
vpn/plugins/vpn.h | 3 +-
vpn/plugins/vpnc.c | 6 -
vpn/plugins/wireguard.c | 373 ++++++++++++++
vpn/plugins/wireguard.h | 103 ++++
vpn/vpn-config.c | 7 +-
20 files changed, 1951 insertions(+), 890 deletions(-)
create mode 100644 src/shared/mnlg.c
create mode 100644 src/shared/mnlg.h
delete mode 100644 src/shared/netlink.c
delete mode 100644 src/shared/netlink.h
delete mode 100644 tools/netlink-test.c
create mode 100644 vpn/plugins/libwireguard.c
create mode 100644 vpn/plugins/wireguard.c
create mode 100644 vpn/plugins/wireguard.h
--
2.23.0
11 months, 3 weeks
