Send connman mailing list submissions to
[email protected]
To subscribe or unsubscribe via the World Wide Web, visit
https://lists.01.org/mailman/listinfo/connman
or, via email, send a message with subject or body 'help' to
[email protected]
You can reach the person managing the list at
[email protected]
When replying, please edit your Subject line so it is more specific
than "Re: Contents of connman digest..."
Today's Topics:
1. [PATCH] gsupplicant: Mem leak in wpa_s because
"RemoveNetwork" not called (Naveen Singh)
2. Re: [RFC] vpn: Restrict connman-vpnd capabilities (Andrew Bibb)
3. Re: [RFC] vpn: Restrict connman-vpnd capabilities (Andrew Bibb)
----------------------------------------------------------------------
Message: 1
Date: Sun, 7 Feb 2016 00:06:49 -0800
From: Naveen Singh <[email protected]>
To: [email protected]
Subject: [PATCH] gsupplicant: Mem leak in wpa_s because
"RemoveNetwork" not called
Message-ID:
<[email protected]>
From: nasingh <[email protected]>
Connman did not call netwok_remove in case AP deauthenticated client causing
wpa_s to re-allocate the ssid pointer even if the next connection attempt
is for the same SSID. This change ensures that at the time of connection
(DBUS Method call AddNetwork) if the network is found not removed, it calls
the dbus API to remove the network and once network is removed, proceed with
the connection.
Tested by running a deauth loop script at the AP end and ensure that wpa_s does
not allocate memory for struct wpa_ssid for all the subsequent
connection attempts. During the test memory usage of wpa_s is monitored.
---
gsupplicant/supplicant.c | 139 ++++++++++++++++++++++++++++++-----------------
1 file changed, 89 insertions(+), 50 deletions(-)
diff --git a/gsupplicant/supplicant.c b/gsupplicant/supplicant.c
index 342cb01..b954e91 100644
--- a/gsupplicant/supplicant.c
+++ b/gsupplicant/supplicant.c
@@ -249,6 +249,51 @@ struct _GSupplicantGroup {
GSList *members;
};
+struct interface_data {
+ GSupplicantInterface *interface;
+ char *path; /* Interface path cannot be taken from interface (above) as
+ * it might have been freed already.
+ */
+ GSupplicantInterfaceCallback callback;
+ void *user_data;
+ bool network_remove_in_progress;
+};
+
+struct interface_create_data {
+ char *ifname;
+ char *driver;
+ char *bridge;
+ GSupplicantInterface *interface;
+ GSupplicantInterfaceCallback callback;
+ void *user_data;
+};
+
+struct interface_connect_data {
+ GSupplicantInterface *interface;
+ char *path;
+ GSupplicantInterfaceCallback callback;
+ void *user_data;
+ bool network_remove_in_progress;
+ union {
+ GSupplicantSSID *ssid;
+ GSupplicantPeerParams *peer;
+ };
+};
+
+struct interface_scan_data {
+ GSupplicantInterface *interface;
+ char *path;
+ GSupplicantInterfaceCallback callback;
+ GSupplicantScanParams *scan_params;
+ void *user_data;
+};
+
+
+static int network_remove(struct interface_data *data);
+static void network_remove_params(DBusMessageIter *iter, void *user_data);
+static void network_remove_result(const char *error,
+ DBusMessageIter *iter, void *user_data);
+
static inline void debug(const char *format, ...)
{
char str[256];
@@ -3476,43 +3521,6 @@ GSupplicantPeer
*g_supplicant_interface_peer_lookup(GSupplicantInterface *interf
return peer;
}
-struct interface_data {
- GSupplicantInterface *interface;
- char *path; /* Interface path cannot be taken from interface (above) as
- * it might have been freed already.
- */
- GSupplicantInterfaceCallback callback;
- void *user_data;
-};
-
-struct interface_create_data {
- char *ifname;
- char *driver;
- char *bridge;
- GSupplicantInterface *interface;
- GSupplicantInterfaceCallback callback;
- void *user_data;
-};
-
-struct interface_connect_data {
- GSupplicantInterface *interface;
- char *path;
- GSupplicantInterfaceCallback callback;
- union {
- GSupplicantSSID *ssid;
- GSupplicantPeerParams *peer;
- };
- void *user_data;
-};
-
-struct interface_scan_data {
- GSupplicantInterface *interface;
- char *path;
- GSupplicantInterfaceCallback callback;
- GSupplicantScanParams *scan_params;
- void *user_data;
-};
-
static void interface_create_data_free(struct interface_create_data *data)
{
g_free(data->ifname);
@@ -4105,7 +4113,6 @@ static void interface_add_network_result(const char
*error,
SUPPLICANT_DBG("PATH: %s", path);
- g_free(interface->network_path);
interface->network_path = g_strdup(path);
supplicant_dbus_method_call(data->interface->path,
@@ -4656,7 +4663,7 @@ int g_supplicant_interface_connect(GSupplicantInterface
*interface,
void *user_data)
{
struct interface_connect_data *data;
- int ret;
+ int ret = 0;
if (!interface)
return -EINVAL;
@@ -4685,12 +4692,33 @@ int g_supplicant_interface_connect(GSupplicantInterface
*interface,
SUPPLICANT_INTERFACE ".Interface.WPS",
"ProcessCredentials", DBUS_TYPE_BOOLEAN_AS_STRING,
wps_process_credentials, wps_start, data, interface);
- } else
- ret = supplicant_dbus_method_call(interface->path,
- SUPPLICANT_INTERFACE ".Interface", "AddNetwork",
- interface_add_network_params,
- interface_add_network_result, data,
- interface);
+ } else {
+ /* By the time there is a request for connect and the network
+ * path is not NULL it means that connman has not removed the
+ * previous network pointer. This can happen in the case AP
+ * deauthenticated client and connman does not remove the
+ * previously connected network pointer. This causes supplicant
+ * to reallocate the memory for struct wpa_ssid again even if it
+ * is the same SSID. This causes memory usage of wpa_supplicnat
+ * to go high. The idea here is that if the previously connected
+ * network is not removed at the time of next connection attempt
+ * check if the network path is not NULL. In case it is non-NULL
+ * first remove the network and then once removal is
successful, add
+ * the network.
+ */
+
+ if (interface->network_path != NULL) {
+ data->network_remove_in_progress = TRUE;
+ network_remove((struct interface_data *)data);
+ }
+ else {
+ ret = supplicant_dbus_method_call(interface->path,
+ SUPPLICANT_INTERFACE ".Interface",
"AddNetwork",
+ interface_add_network_params,
+ interface_add_network_result, data,
+ interface);
+ }
+ }
if (ret < 0) {
g_free(data->path);
@@ -4716,12 +4744,23 @@ static void network_remove_result(const char *error,
result = -ECONNABORTED;
}
- g_free(data->path);
+ g_free(data->interface->network_path);
+ data->interface->network_path = NULL;
- if (data->callback)
- data->callback(result, data->interface, data->user_data);
-
- dbus_free(data);
+ if (data->network_remove_in_progress == TRUE) {
+ data->network_remove_in_progress = FALSE;
+ supplicant_dbus_method_call(data->interface->path,
+ SUPPLICANT_INTERFACE ".Interface", "AddNetwork",
+ interface_add_network_params,
+ interface_add_network_result, data,
+ data->interface);
+ }
+ else {
+ g_free(data->path);
+ if (data->callback)
+ data->callback(result, data->interface,
data->user_data);
+ dbus_free(data);
+ }
}
static void network_remove_params(DBusMessageIter *iter, void *user_data)
--
2.7.0.rc3.207.g0ac5344
------------------------------
Message: 2
Date: Sun, 7 Feb 2016 10:51:38 -0500
From: Andrew Bibb <[email protected]>
To: [email protected]
Subject: Re: [RFC] vpn: Restrict connman-vpnd capabilities
Message-ID: <[email protected]>
Content-Type: text/plain; charset="windows-1252"; format=flowed
I'm sorry about being late to the party, but now that 1.31 is out I've
discovered that this change seems to make OpenVPN not work. I had an
OpenVPN config file that works fine in 1.30 but not 1.31. Removing the
ProtectHome and ProtectSystem lines from the .service file allows
OpenVPN to work in 1.31.
I also have a functional PPTP config file and that works in both 1.30
and 1.31 without modification.
Thanks
Andrew
On 12/1/2015 8:32 AM, Patrik Flykt wrote:
> Have systemd set /home and /run/users read only as VPN certificates can
> be stored also in these directories. Protect other directories in the
> system by making also them read only. The directory options affect also
> all VPN applications started by connman-vpnd.
>
> Restrict capabilities to a subset necessary for normal operations.
> ---
>
> ProtectSystem=full means the VPN applications cannot write anything to
> /usr or /etc. Let's hope this works out for all VPN daemons.
>
> Please test,
>
> Patrik
>
>
> vpn/connman-vpn.service.in | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/vpn/connman-vpn.service.in b/vpn/connman-vpn.service.in
> index 120245e..e98fb71 100644
> --- a/vpn/connman-vpn.service.in
> +++ b/vpn/connman-vpn.service.in
> @@ -6,6 +6,9 @@ Type=dbus
> BusName=net.connman.vpn
> ExecStart=@sbindir@/connman-vpnd -n
> StandardOutput=null
> +CapabilityBoundingSet=CAP_KILL CAP_NET_ADMIN CAP_NET_BIND_SERVICE CAP_NET_RAW
> +ProtectHome=read-only
> +ProtectSystem=full
>
> [Install]
> WantedBy=multi-user.target
------------------------------
Message: 3
Date: Sun, 7 Feb 2016 13:26:03 -0500
From: Andrew Bibb <[email protected]>
To: [email protected]
Subject: Re: [RFC] vpn: Restrict connman-vpnd capabilities
Message-ID: <[email protected]>
Content-Type: text/plain; charset="windows-1252"; format=flowed
Actually need to remove all three lines, not just the two I mentioned.
I reverted the file back to the way it was in 1.30, and I initially
thought it was only the two lines that were different.
Sorry about the initial misinformation.
-
Andrew
On 2/7/2016 10:51 AM, Andrew Bibb wrote:
> I'm sorry about being late to the party, but now that 1.31 is out I've
> discovered that this change seems to make OpenVPN not work. I had an
> OpenVPN config file that works fine in 1.30 but not 1.31. Removing the
> ProtectHome and ProtectSystem lines from the .service file allows
> OpenVPN to work in 1.31.
>
> I also have a functional PPTP config file and that works in both 1.30
> and 1.31 without modification.
>
> Thanks
> Andrew
>
> On 12/1/2015 8:32 AM, Patrik Flykt wrote:
>> Have systemd set /home and /run/users read only as VPN certificates can
>> be stored also in these directories. Protect other directories in the
>> system by making also them read only. The directory options affect also
>> all VPN applications started by connman-vpnd.
>>
>> Restrict capabilities to a subset necessary for normal operations.
>> ---
>>
>> ProtectSystem=full means the VPN applications cannot write anything to
>> /usr or /etc. Let's hope this works out for all VPN daemons.
>>
>> Please test,
>>
>> Patrik
>>
>>
>> vpn/connman-vpn.service.in | 3 +++
>> 1 file changed, 3 insertions(+)
>>
>> diff --git a/vpn/connman-vpn.service.in b/vpn/connman-vpn.service.in
>> index 120245e..e98fb71 100644
>> --- a/vpn/connman-vpn.service.in
>> +++ b/vpn/connman-vpn.service.in
>> @@ -6,6 +6,9 @@ Type=dbus
>> BusName=net.connman.vpn
>> ExecStart=@sbindir@/connman-vpnd -n
>> StandardOutput=null
>> +CapabilityBoundingSet=CAP_KILL CAP_NET_ADMIN CAP_NET_BIND_SERVICE
>> CAP_NET_RAW
>> +ProtectHome=read-only
>> +ProtectSystem=full
>> [Install]
>> WantedBy=multi-user.target
>
> _______________________________________________
> connman mailing list
> [email protected]
> https://lists.01.org/mailman/listinfo/connman
>
>
------------------------------
Subject: Digest Footer
_______________________________________________
connman mailing list
[email protected]
https://lists.01.org/mailman/listinfo/connman
------------------------------
End of connman Digest, Vol 4, Issue 8
*************************************
