wpa_supplicant: If empty bytearray sent via DBUS use default value
diff mbox series

Message ID CAEK8rrqXVmq9n9KnkF0gRUzX255EC92=AmRiWz=d5Rd5AxbOCA@mail.gmail.com
State New
Headers show
Series
  • wpa_supplicant: If empty bytearray sent via DBUS use default value
Related show

Commit Message

István Bodnár Aug. 22, 2019, 7:43 p.m. UTC
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) {

Comments

Dan Williams Aug. 22, 2019, 8:48 p.m. UTC | #1
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
István Bodnár Aug. 22, 2019, 9:10 p.m. UTC | #2
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
>
István Bodnár Sept. 9, 2019, 1:59 p.m. UTC | #3
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
> >

Patch
diff mbox series

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