Message ID | CAEK8rrqXVmq9n9KnkF0gRUzX255EC92=AmRiWz=d5Rd5AxbOCA@mail.gmail.com |
---|---|
State | Changes Requested |
Headers | show |
Series | wpa_supplicant: If empty bytearray sent via DBUS use default value | expand |
On Thu, 2019-08-22 at 21:43 +0200, István Bodnár wrote: > In case wpa_supplicant receive empty byte array via dbus > (e.g.: from Network Manager which sends empty private key password > field) > it will use the "NULL" default value. Any idea which NM versions send this? NM should get fixed to not do this if it's a problem. Dan > Signed-off-by: Istvan Bodnar <mail.bodnaristvan@gmail.com> > --- > wpa_supplicant/dbus/dbus_new_handlers.c | 18 +++++++++++++----- > 1 file changed, 13 insertions(+), 5 deletions(-) > > diff --git a/wpa_supplicant/dbus/dbus_new_handlers.c > b/wpa_supplicant/dbus/dbus_new_handlers.c > index 6c36d91a0..45c16c2e7 100644 > --- a/wpa_supplicant/dbus/dbus_new_handlers.c > +++ b/wpa_supplicant/dbus/dbus_new_handlers.c > @@ -206,17 +206,25 @@ dbus_bool_t set_network_properties(struct > wpa_supplicant *wpa_s, > value = NULL; > if (entry.type == DBUS_TYPE_ARRAY && > entry.array_type == DBUS_TYPE_BYTE) { > - if (entry.array_len <= 0) > + if (entry.array_len < 0) > goto error; > + if (entry.array_len == 0) > + size = strlen("NULL") + 3; > + else > + size = entry.array_len * 2 + 1; > > - size = entry.array_len * 2 + 1; > value = os_zalloc(size); > if (value == NULL) > goto error; > > - ret = wpa_snprintf_hex(value, size, > - (u8 *) > entry.bytearray_value, > - entry.array_len); > + if (entry.array_len == 0) > + ret = os_snprintf(value, size, > "\"%s\"", > + "NULL"); > + else > + ret = wpa_snprintf_hex(value, size, > + (u8 *) > entry.bytearray_value, > + entry.array_le > n); > + > if (ret <= 0) > goto error; > } else if (entry.type == DBUS_TYPE_STRING) { > _______________________________________________ > Hostap mailing list > Hostap@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/hostap
The NM version which causes the problem is 1.10.6. I didn't check other versions. I think that wpa_supplicant should do the same in both cases, when it is configured via config file and when it is configured via dbus communication. I mean that it shouldn't work differently if the only difference is the interface which is used to configure wpa_supplicant, now this is not fulfilled. This commit can be abandoned, but what is the proposed way to solve this inconsistency? I'm open to implement a better solution for this. Istvan Dan Williams <dcbw@redhat.com> ezt írta (időpont: 2019. aug. 22., Cs, 22:48): > > On Thu, 2019-08-22 at 21:43 +0200, István Bodnár wrote: > > In case wpa_supplicant receive empty byte array via dbus > > (e.g.: from Network Manager which sends empty private key password > > field) > > it will use the "NULL" default value. > > Any idea which NM versions send this? NM should get fixed to not do > this if it's a problem. > > Dan > > > Signed-off-by: Istvan Bodnar <mail.bodnaristvan@gmail.com> > > --- > > wpa_supplicant/dbus/dbus_new_handlers.c | 18 +++++++++++++----- > > 1 file changed, 13 insertions(+), 5 deletions(-) > > > > diff --git a/wpa_supplicant/dbus/dbus_new_handlers.c > > b/wpa_supplicant/dbus/dbus_new_handlers.c > > index 6c36d91a0..45c16c2e7 100644 > > --- a/wpa_supplicant/dbus/dbus_new_handlers.c > > +++ b/wpa_supplicant/dbus/dbus_new_handlers.c > > @@ -206,17 +206,25 @@ dbus_bool_t set_network_properties(struct > > wpa_supplicant *wpa_s, > > value = NULL; > > if (entry.type == DBUS_TYPE_ARRAY && > > entry.array_type == DBUS_TYPE_BYTE) { > > - if (entry.array_len <= 0) > > + if (entry.array_len < 0) > > goto error; > > + if (entry.array_len == 0) > > + size = strlen("NULL") + 3; > > + else > > + size = entry.array_len * 2 + 1; > > > > - size = entry.array_len * 2 + 1; > > value = os_zalloc(size); > > if (value == NULL) > > goto error; > > > > - ret = wpa_snprintf_hex(value, size, > > - (u8 *) > > entry.bytearray_value, > > - entry.array_len); > > + if (entry.array_len == 0) > > + ret = os_snprintf(value, size, > > "\"%s\"", > > + "NULL"); > > + else > > + ret = wpa_snprintf_hex(value, size, > > + (u8 *) > > entry.bytearray_value, > > + entry.array_le > > n); > > + > > if (ret <= 0) > > goto error; > > } else if (entry.type == DBUS_TYPE_STRING) { > > _______________________________________________ > > Hostap mailing list > > Hostap@lists.infradead.org > > http://lists.infradead.org/mailman/listinfo/hostap >
Hi! Can someone help me with this issue? I would like to resolve this inconsistency somehow. Thank you, Istvan István Bodnár <mail.bodnaristvan@gmail.com> ezt írta (időpont: 2019. aug. 22., Cs, 23:10): > > The NM version which causes the problem is 1.10.6. I didn't check > other versions. > I think that wpa_supplicant should do the same in both cases, when it > is configured via config file and when it is configured via dbus > communication. > I mean that it shouldn't work differently if the only difference is > the interface which is used to configure wpa_supplicant, now this is > not fulfilled. > This commit can be abandoned, but what is the proposed way to solve > this inconsistency? I'm open to implement a better solution for this. > > Istvan > > > Dan Williams <dcbw@redhat.com> ezt írta (időpont: 2019. aug. 22., Cs, 22:48): > > > > On Thu, 2019-08-22 at 21:43 +0200, István Bodnár wrote: > > > In case wpa_supplicant receive empty byte array via dbus > > > (e.g.: from Network Manager which sends empty private key password > > > field) > > > it will use the "NULL" default value. > > > > Any idea which NM versions send this? NM should get fixed to not do > > this if it's a problem. > > > > Dan > > > > > Signed-off-by: Istvan Bodnar <mail.bodnaristvan@gmail.com> > > > --- > > > wpa_supplicant/dbus/dbus_new_handlers.c | 18 +++++++++++++----- > > > 1 file changed, 13 insertions(+), 5 deletions(-) > > > > > > diff --git a/wpa_supplicant/dbus/dbus_new_handlers.c > > > b/wpa_supplicant/dbus/dbus_new_handlers.c > > > index 6c36d91a0..45c16c2e7 100644 > > > --- a/wpa_supplicant/dbus/dbus_new_handlers.c > > > +++ b/wpa_supplicant/dbus/dbus_new_handlers.c > > > @@ -206,17 +206,25 @@ dbus_bool_t set_network_properties(struct > > > wpa_supplicant *wpa_s, > > > value = NULL; > > > if (entry.type == DBUS_TYPE_ARRAY && > > > entry.array_type == DBUS_TYPE_BYTE) { > > > - if (entry.array_len <= 0) > > > + if (entry.array_len < 0) > > > goto error; > > > + if (entry.array_len == 0) > > > + size = strlen("NULL") + 3; > > > + else > > > + size = entry.array_len * 2 + 1; > > > > > > - size = entry.array_len * 2 + 1; > > > value = os_zalloc(size); > > > if (value == NULL) > > > goto error; > > > > > > - ret = wpa_snprintf_hex(value, size, > > > - (u8 *) > > > entry.bytearray_value, > > > - entry.array_len); > > > + if (entry.array_len == 0) > > > + ret = os_snprintf(value, size, > > > "\"%s\"", > > > + "NULL"); > > > + else > > > + ret = wpa_snprintf_hex(value, size, > > > + (u8 *) > > > entry.bytearray_value, > > > + entry.array_le > > > n); > > > + > > > if (ret <= 0) > > > goto error; > > > } else if (entry.type == DBUS_TYPE_STRING) { > > > _______________________________________________ > > > Hostap mailing list > > > Hostap@lists.infradead.org > > > http://lists.infradead.org/mailman/listinfo/hostap > >
On Thu, Aug 22, 2019 at 09:43:34PM +0200, István Bodnár wrote: > In case wpa_supplicant receive empty byte array via dbus > (e.g.: from Network Manager which sends empty private key password field) > it will use the "NULL" default value. Why would a string parameter like private_key_passwd be configured as DBUS_TYPE_ARRAY(DBUS_TYPE_BYTE) instead of DBUS_TYPE_STRING? It should also be noted that there is a difference between a zero-length string value ("") and no value set (NULL). If we were to convert zero-length byte array into NULL, there would be no way of setting an empty string (""). When setting parameter values over the text-based control interface, those two cases are separate: SET_NETWORK 0 field NULL SET_NETWORK 0 field "" Based on the actual changes, there may also be some misunderstanding here on that "default value" part.. Setting a string to NULL (i.e., without quotation marks) clears the parameter (sets the C pointer to NULL) while setting a string to "NULL" (i.e., with quotation marks) sets the parameter to a string "NULL" (i.e., allocated memory with NULL\0 in it). > diff --git a/wpa_supplicant/dbus/dbus_new_handlers.c > @@ -206,17 +206,25 @@ dbus_bool_t set_network_properties(struct > wpa_supplicant *wpa_s, > value = NULL; > if (entry.type == DBUS_TYPE_ARRAY && > entry.array_type == DBUS_TYPE_BYTE) { > - if (entry.array_len <= 0) > + if (entry.array_len < 0) > goto error; This part might make sense to allow the zero-length value to be set (and something similar for the DBUS_TYPE_STRING case for should_quote_opt() cases). > + if (entry.array_len == 0) > + ret = os_snprintf(value, size, "\"%s\"", > + "NULL"); This looks quite strange.. This would set the string to "NULL" instead of removing a previously configured parameter. Was this supposed to be simply os_snprintf(value, size, "NULL") to get the remove-a-parameter behavior? > } else if (entry.type == DBUS_TYPE_STRING) { ... this is the case where zero-length string is also rejected. That should likely remain consistent with the DBUS_TYPE_ARRAY(DBUS_TYPE_BYTE) case.
From 230b2f6857b136858d6dd6b815e5b86906cc13fc Mon Sep 17 00:00:00 2001 From: Istvan Bodnar <mail.bodnaristvan@gmail.com> Date: Thu, 22 Aug 2019 21:26:43 +0200 Subject: [PATCH] wpa_supplicant: If empty bytearray sent via DBUS use default value In case wpa_supplicant receive empty byte array via dbus (e.g.: from Network Manager which sends empty private key password field) it will use the "NULL" default value. Signed-off-by: Istvan Bodnar <mail.bodnaristvan@gmail.com> --- wpa_supplicant/dbus/dbus_new_handlers.c | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) diff --git a/wpa_supplicant/dbus/dbus_new_handlers.c b/wpa_supplicant/dbus/dbus_new_handlers.c index 6c36d91a0..45c16c2e7 100644 --- a/wpa_supplicant/dbus/dbus_new_handlers.c +++ b/wpa_supplicant/dbus/dbus_new_handlers.c @@ -206,17 +206,25 @@ dbus_bool_t set_network_properties(struct wpa_supplicant *wpa_s, value = NULL; if (entry.type == DBUS_TYPE_ARRAY && entry.array_type == DBUS_TYPE_BYTE) { - if (entry.array_len <= 0) + if (entry.array_len < 0) goto error; + if (entry.array_len == 0) + size = strlen("NULL") + 3; + else + size = entry.array_len * 2 + 1; - size = entry.array_len * 2 + 1; value = os_zalloc(size); if (value == NULL) goto error; - ret = wpa_snprintf_hex(value, size, - (u8 *) entry.bytearray_value, - entry.array_len); + if (entry.array_len == 0) + ret = os_snprintf(value, size, "\"%s\"", + "NULL"); + else + ret = wpa_snprintf_hex(value, size, + (u8 *) entry.bytearray_value, + entry.array_len); + if (ret <= 0) goto error; } else if (entry.type == DBUS_TYPE_STRING) { -- 2.17.1
In case wpa_supplicant receive empty byte array via dbus (e.g.: from Network Manager which sends empty private key password field) it will use the "NULL" default value. Signed-off-by: Istvan Bodnar <mail.bodnaristvan@gmail.com> --- wpa_supplicant/dbus/dbus_new_handlers.c | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) goto error; } else if (entry.type == DBUS_TYPE_STRING) {