diff mbox series

A possible bug and a fix (patch attached)

Message ID CAHiu4JMf1BdK4oLj_AVNDe3JEunwQXSVzW9XyMiRz4+Y-n2fHw@mail.gmail.com
State Rejected
Headers show
Series A possible bug and a fix (patch attached) | expand

Commit Message

M. Ranganathan Oct. 19, 2019, 10:52 p.m. UTC
Hello,

I ran into an issue in config_file.c

The function is

static void wpa_config_write_network(FILE *f, struct wpa_ssid *ssid)

When writing out the ssid, the quote is omitted. I had to make the
following changes:

#define QUOTED_STR(t) write_quoted_str(f, #t, ssid)

    //STR(ssid);
    QUOTED_STR(ssid);

And I had to add the function :

static void write_quoted_str(FILE *f, const char *field, struct wpa_ssid *ssid)
{
    char *value = wpa_config_get(ssid, field);
    if (value == NULL)
        return;
    fprintf(f, "\t%s=\"%s\"\n", field, value);
    str_clear_free(value);
}

Also I've run into a bug with dpp which can't handle SSID's larger
than 4 characters it appears.

Comments

Jouni Malinen Oct. 21, 2019, 3:35 p.m. UTC | #1
On Sat, Oct 19, 2019 at 06:52:26PM -0400, M. Ranganathan wrote:
> I ran into an issue in config_file.c
> 
> The function is
> 
> static void wpa_config_write_network(FILE *f, struct wpa_ssid *ssid)
> 
> When writing out the ssid, the quote is omitted. I had to make the
> following changes:
> 
> #define QUOTED_STR(t) write_quoted_str(f, #t, ssid)
> 
>     //STR(ssid);
>     QUOTED_STR(ssid);
> 
> And I had to add the function :
> 
> static void write_quoted_str(FILE *f, const char *field, struct wpa_ssid *ssid)
> {
>     char *value = wpa_config_get(ssid, field);
>     if (value == NULL)
>         return;
>     fprintf(f, "\t%s=\"%s\"\n", field, value);
>     str_clear_free(value);
> }

This would break wpa_supplicant configuration writing for me..
wpa_config_get() returns a string with the needed quotation marks. Could
you please provide the exact parameter value in the ssid field that does
not work for you?

> Also I've run into a bug with dpp which can't handle SSID's larger
> than 4 characters it appears.

I have not seen this. Could you please provide more details and a debug
log showing this?
M. Ranganathan Oct. 22, 2019, 1:33 p.m. UTC | #2
Debug logs mailed to Jouni directly (too big for the list).

---------- Forwarded message ---------
From: M. Ranganathan <mranga@gmail.com>
Date: Mon, Oct 21, 2019 at 3:22 PM
Subject: Re: A possible bug and a fix (patch attached)
To: Jouni Malinen <j@w1.fi>


Thank you for responding. I am inexperienced with this code base so
I'll describe the scenario best I can.

I am experimenting with Device Provisioning Protocol (DPP).

Scenario 1:

In my scenario, I use dpp to send the configuration from the
configurator to the enrollee. When the enrollee receives the
configuration, I save it using save_config. The ssid is saved without
the quotation marks.

Here is the scenario:

The access point is using psk. Here is its configuration (note that I
have limited ssid to 4 characters):

nterface=ap1-wlan1
driver=nl80211
ssid=012a
wds_sta=1
hw_mode=g
channel=1
auth_algs=1
wpa=2
wpa_key_mgmt=WPA-PSK
wpa_pairwise=CCMP
wpa_passphrase=12345678
ctrl_interface=/var/run/hostapd
ctrl_interface_group=0




sta1 is the configurator:

Its configuration is as follows:
ctrl_interface=/var/run/wpa_supplicant1
ctrl_interface_group=0
network={
   ssid="012a"
   psk="12345678"
   proto=WPA2
   pairwise=CCMP
   key_mgmt=WPA-PSK
}

sta2 is the enrollee:

It's initial configuration (prior to enrollment ) is as follows:

ctrl_interface=/var/run/wpa_supplicant2
ctrl_interface_group=0
update_config=1
pmf=1
dpp_config_processing=2

==============================================================================

Test scenario:

1. get the psk from the ssid using the wpa_passphrase command

2. Configurator: add a configurator object
wpa_cli -p /var/run/wpa_supplicant1  dpp_configurator_add

3. Configurator: get the configurator key using the returned ID
wpa_cli -p /var/run/wpa_supplicant1  dpp_configurator_get_key 1

4. Configurator: self sign the configurator
wpa_cli -p /var/run/wpa_supplicant1  dpp_configurator_sign
conf=sta-psk psk=29153c1e60c0e50afa47530eb7b6db1193b0131616c139e9f1785d174861cca7
ssid=012a configurator=1'

5. Enrollee: generate QR code for device
wpa_cli -p /var/run/wpa_supplicant2 dpp_bootstrap_gen  type=qrcode
mac=00:00:00:00:00:82 key=.....

6. Enrollee: get the qr code using the returned bootstrap_info_id
wpa_cli -p /var/run/wpa_supplicant2 dpp_bootstrap_get_uri 1

7. Enrollee: listen for dpp provisioning request
wpa_cli -p /var/run/wpa_supplicant2 dpp_listen 2412

8. Configurator:  Enter the sta QR Code in the Configurator.
wpa_cli -p /var/run/wpa_supplicant1  dpp_qr_code 'DPP:.....'

9. Configurator: Send provisioning request to enrollee.
wpa_cli -p /var/run/wpa_supplicant1  dpp_auth_init peer=1 conf=sta-psk
ssid=012a psk=.... configurator=1

10. Enrollee: save the config file
wpa_cli -p /var/run/wpa_supplicant2 save_config

11. Enrollee:  reload the config file
wpa_cli -p /var/run/wpa_supplicant2 reconfigure

Observations
===========

1. After making the suggested fix to wpa_supplicant/config_file.c, the
configuration is written out correctly i.e. for the enrollee:

ctrl_interface=/var/run/wpa_supplicant2
ctrl_interface_group=0
update_config=1
pmf=1
dpp_config_processing=2
network={
    ssid="012a"
    psk=29153c1e60c0e50afa47530eb7b6db1193b0131616c139e9f1785d174861cca7
    key_mgmt=WPA-PSK FT-PSK WPA-PSK-SHA256
    ieee80211w=1
}

And the enrollee is correctly onboarded.

2.  If I change my ssid to something long,  012abcdef, I observe the
following at the rnrollee:

ctrl_interface=/var/run/wpa_supplicant2
ctrl_interface_group=0
update_config=1
pmf=1
dpp_config_processing=2

network={
    ssid="012affff66666263ffff66666465"
    psk=187d02efd4a180287f01af32f91e3c4b7551dce96175c3e97bab69a899d2c0fb
    key_mgmt=WPA-PSK FT-PSK WPA-PSK-SHA256
    ieee80211w=1
}

And sometimes I see the following:

('wpa_cli -p /var/run/wpa_supplicant1  dpp_configurator_sign
conf=sta-psk psk=38d423e9e635b3544ff495ef2fa73a6a89b62b75ec3f98d8b2d864769b23eae6
ssid=012abcdefg configurator=1',)
Selected interface 'sta1-wlan0'
FAIL


I've attached debug logs for the failing scenario (i.e. with the
longer ssid with the corrupted ssid being configured at the enrollee).

I am sorry about the length of this email but I thought I should be
thurough in my report.

Sincerely,

Ranga





On Mon, Oct 21, 2019 at 11:35 AM Jouni Malinen <j@w1.fi> wrote:
>
> On Sat, Oct 19, 2019 at 06:52:26PM -0400, M. Ranganathan wrote:
> > I ran into an issue in config_file.c
> >
> > The function is
> >
> > static void wpa_config_write_network(FILE *f, struct wpa_ssid *ssid)
> >
> > When writing out the ssid, the quote is omitted. I had to make the
> > following changes:
> >
> > #define QUOTED_STR(t) write_quoted_str(f, #t, ssid)
> >
> >     //STR(ssid);
> >     QUOTED_STR(ssid);
> >
> > And I had to add the function :
> >
> > static void write_quoted_str(FILE *f, const char *field, struct wpa_ssid *ssid)
> > {
> >     char *value = wpa_config_get(ssid, field);
> >     if (value == NULL)
> >         return;
> >     fprintf(f, "\t%s=\"%s\"\n", field, value);
> >     str_clear_free(value);
> > }
>
> This would break wpa_supplicant configuration writing for me..
> wpa_config_get() returns a string with the needed quotation marks. Could
> you please provide the exact parameter value in the ssid field that does
> not work for you?
>
> > Also I've run into a bug with dpp which can't handle SSID's larger
> > than 4 characters it appears.
>
> I have not seen this. Could you please provide more details and a debug
> log showing this?
>
> --
> Jouni Malinen                                            PGP id EFC895FA



--
M. Ranganathan
Jouni Malinen Oct. 27, 2019, 3:18 p.m. UTC | #3
On Tue, Oct 22, 2019 at 09:33:25AM -0400, M. Ranganathan wrote:
> The access point is using psk. Here is its configuration (note that I
> have limited ssid to 4 characters):

> sta1 is the configurator:
> 
> Its configuration is as follows:
> ctrl_interface=/var/run/wpa_supplicant1
> ctrl_interface_group=0
> network={
>    ssid="012a"
>    psk="12345678"

> 4. Configurator: self sign the configurator
> wpa_cli -p /var/run/wpa_supplicant1  dpp_configurator_sign
> conf=sta-psk psk=29153c1e60c0e50afa47530eb7b6db1193b0131616c139e9f1785d174861cca7
> ssid=012a configurator=1'

Please note that the psk and ssid parameter here take in a hexdump value
of the passphrase, i.e., that ssid=012a encodes a two octet, not four
octet, SSID.. This should have been using ssid=30313261 if you want the
SSID to be a four octet string "012a".

> 9. Configurator: Send provisioning request to enrollee.
> wpa_cli -p /var/run/wpa_supplicant1  dpp_auth_init peer=1 conf=sta-psk
> ssid=012a psk=.... configurator=1

Same here.

> 1. After making the suggested fix to wpa_supplicant/config_file.c, the
> configuration is written out correctly i.e. for the enrollee:

No it is not.. In this case, the DPP Configurator was asked to set the
SSID to a two octet string 0x01 0x2a which is encoded as ssid=012a in
wpa_supplicant configuration.

>     ssid="012a"

In other words, the patch broke this.. ssid=012a would have been
correct. ssid="012a" would show here had the Configurator been set up
correctly using ssid=30313261 as described above.

> 2.  If I change my ssid to something long,  012abcdef, I observe the
> following at the rnrollee:

>     ssid="012affff66666263ffff66666465"

This does actually show up a real bug, but in the JSON string escaping
mechanism.. I'll fix that. Anyway, your example should have used
ssid=303132616263646566 to get the SSID value that I think you were
trying to use (and that one would not trigger the JSON bug either since
there are no non-ASCII characters in the string encoded here in
hex format).
diff mbox series

Patch

diff --git a/wpa_supplicant/config_file.c b/wpa_supplicant/config_file.c
index cf4b7bc..864b61c 100644
--- a/wpa_supplicant/config_file.c
+++ b/wpa_supplicant/config_file.c
@@ -496,6 +496,14 @@  static void write_str(FILE *f, const char *field, struct wpa_ssid *ssid)
 	str_clear_free(value);
 }
 
+static void write_quoted_str(FILE *f, const char *field, struct wpa_ssid *ssid)
+{
+	char *value = wpa_config_get(ssid, field);
+	if (value == NULL)
+		return;
+	fprintf(f, "\t%s=\"%s\"\n", field, value);
+	str_clear_free(value);
+}
 
 static void write_int(FILE *f, const char *field, int value, int def)
 {
@@ -748,8 +756,10 @@  static void wpa_config_write_network(FILE *f, struct wpa_ssid *ssid)
 #define INTe(t, m) write_int(f, #t, ssid->eap.m, 0)
 #define INT_DEF(t, def) write_int(f, #t, ssid->t, def)
 #define INT_DEFe(t, m, def) write_int(f, #t, ssid->eap.m, def)
+#define QUOTED_STR(t) write_quoted_str(f, #t, ssid)
 
-	STR(ssid);
+	//STR(ssid);
+	QUOTED_STR(ssid);
 	INT(scan_ssid);
 	write_bssid(f, ssid);
 	write_bssid_hint(f, ssid);
@@ -1407,9 +1417,6 @@  static void wpa_config_write_global(FILE *f, struct wpa_config *config)
 		fprintf(f, "\n");
 	}
 
-	if (config->sae_pwe)
-		fprintf(f, "sae_pwe=%d\n", config->sae_pwe);
-
 	if (config->sae_pmkid_in_assoc)
 		fprintf(f, "sae_pmkid_in_assoc=%d\n",
 			config->sae_pmkid_in_assoc);