| Message ID | 1577961050-8756-1-git-send-email-bilalhp@gmail.com |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | [v4] Add application extension data to WPS IE | expand |
On Thu, Jan 02, 2020 at 01:30:50PM +0300, bilalhp@gmail.com wrote: > Application Extension attribute is defined in WPS Spec v2.06 page 105. > This patch makes hostapd add this application extension data to WPS IE > if configured. The implementation is very similar to vendor extension. > > Added a new optional entry called "wps_application_ext[]" to hostapd config > file to configure this. That looks overly complex. Why would there be need to assign specific application extension indexes? Couldn't the next index value be assigned automatically so that first wps_application_ext=<hex> entry would get index 0 and the next such line would get index 1? Or alternatively, support only one Application Extension attribute if there is no existing use case that needs more of those.. It is not likely WSC should really be used for anything new and instead, DPP should be considered to be the current standard for doing device provisioning. It should also be noted that the WSC technical specification does not say anything about including Application Extension attributes in Beacon or Probe Response frames; the only example use case for these attributes is within the encrypted section of M7 and M8. This patch seems to be doing something very different. Is that use defined in some public specification? > --- > hostapd/config_file.c | 12 ++++++++++++ > src/ap/ap_config.c | 2 ++ > src/ap/ap_config.h | 1 + > src/ap/wps_hostapd.c | 29 +++++++++++++++++++++++++++++ > src/wps/wps.h | 5 +++++ > src/wps/wps_dev_attr.c | 19 +++++++++++++++++++ > src/wps/wps_dev_attr.h | 1 + > src/wps/wps_registrar.c | 6 ++++-- > 8 files changed, 73 insertions(+), 2 deletions(-) hostapd/hostapd.conf should also be updated to document the new configuration parameter, including the format it uses and requirements for the contents (UUID in the beginning).
On Thu, 2 Jan 2020 at 17:08, Jouni Malinen <j@w1.fi> wrote: > > On Thu, Jan 02, 2020 at 01:30:50PM +0300, bilalhp@gmail.com wrote: > > Application Extension attribute is defined in WPS Spec v2.06 page 105. > > This patch makes hostapd add this application extension data to WPS IE > > if configured. The implementation is very similar to vendor extension. > > > > Added a new optional entry called "wps_application_ext[]" to hostapd config > > file to configure this. > > That looks overly complex. Why would there be need to assign specific > application extension indexes? Couldn't the next index value be assigned > automatically so that first wps_application_ext=<hex> entry would get > index 0 and the next such line would get index 1? Or alternatively, > support only one Application Extension attribute if there is no existing > use case that needs more of those.. It is not likely WSC should really > be used for anything new and instead, DPP should be considered to be the > current standard for doing device provisioning. OK. I will make it single Application Extension attribute in v5. > > It should also be noted that the WSC technical specification does not > say anything about including Application Extension attributes in Beacon > or Probe Response frames; the only example use case for these attributes > is within the encrypted section of M7 and M8. This patch seems to be > doing something very different. Is that use defined in some public > specification? Actually, I have not seen any existing usage in any code base. My understanding is that this has not been used ever in any of the WSC implementations so far. As you already stated, there is an example use defined in the spec, which has never leveraged by anyone yet, and I don't think it would be the case in the future ever as you stated. Our understanding from WSC spec is that this is a standard TLV definition and it can be used optionally in any of the frames, including Beacon and Probe Responses. This is what we understand from: <other…> O Multiple attributes are permitted. row in tables inside sections 8.2 and 8.3. Regards. > > > --- > > hostapd/config_file.c | 12 ++++++++++++ > > src/ap/ap_config.c | 2 ++ > > src/ap/ap_config.h | 1 + > > src/ap/wps_hostapd.c | 29 +++++++++++++++++++++++++++++ > > src/wps/wps.h | 5 +++++ > > src/wps/wps_dev_attr.c | 19 +++++++++++++++++++ > > src/wps/wps_dev_attr.h | 1 + > > src/wps/wps_registrar.c | 6 ++++-- > > 8 files changed, 73 insertions(+), 2 deletions(-) > > hostapd/hostapd.conf should also be updated to document the new > configuration parameter, including the format it uses and requirements > for the contents (UUID in the beginning). > > -- > Jouni Malinen PGP id EFC895FA
diff --git a/hostapd/config_file.c b/hostapd/config_file.c index 21c9ab2..cb03440 100644 --- a/hostapd/config_file.c +++ b/hostapd/config_file.c @@ -3616,6 +3616,18 @@ static int hostapd_config_fill(struct hostapd_config *conf, } os_free(bss->device_name); bss->device_name = os_strdup(pos); + } else if (os_strncmp(buf, "wps_application_ext[", 20) == 0) { + char *endp = NULL; + long int i = strtol(buf + 20, &endp, 10); + + if ((*endp != ']') || + (i < 0) || + (i >= MAX_WPS_APPLICATION_EXTENSIONS)) { + wpa_printf(MSG_ERROR, "Line %d: Invalid index", line); + return 1; + } + wpabuf_free(bss->wps_application_ext[i]); + bss->wps_application_ext[i] = wpabuf_parse_bin(pos); } else if (os_strcmp(buf, "manufacturer") == 0) { if (os_strlen(pos) > 64) { wpa_printf(MSG_ERROR, "Line %d: Too long manufacturer", diff --git a/src/ap/ap_config.c b/src/ap/ap_config.c index 68af3c1..684450f 100644 --- a/src/ap/ap_config.c +++ b/src/ap/ap_config.c @@ -813,6 +813,8 @@ void hostapd_config_free_bss(struct hostapd_bss_config *conf) os_free(conf->upc); for (i = 0; i < MAX_WPS_VENDOR_EXTENSIONS; i++) wpabuf_free(conf->wps_vendor_ext[i]); + for (i = 0; i < MAX_WPS_APPLICATION_EXTENSIONS; i++) + wpabuf_free(conf->wps_application_ext[i]); wpabuf_free(conf->wps_nfc_dh_pubkey); wpabuf_free(conf->wps_nfc_dh_privkey); wpabuf_free(conf->wps_nfc_dev_pw); diff --git a/src/ap/ap_config.h b/src/ap/ap_config.h index 7e4b926..e8f033a 100644 --- a/src/ap/ap_config.h +++ b/src/ap/ap_config.h @@ -498,6 +498,7 @@ struct hostapd_bss_config { char *model_url; char *upc; struct wpabuf *wps_vendor_ext[MAX_WPS_VENDOR_EXTENSIONS]; + struct wpabuf *wps_application_ext[MAX_WPS_APPLICATION_EXTENSIONS]; int wps_nfc_pw_from_config; int wps_nfc_dev_pw_id; struct wpabuf *wps_nfc_dh_pubkey; diff --git a/src/ap/wps_hostapd.c b/src/ap/wps_hostapd.c index 175b9fc..dc4e0b5 100644 --- a/src/ap/wps_hostapd.c +++ b/src/ap/wps_hostapd.c @@ -985,6 +985,31 @@ static int hostapd_wps_set_vendor_ext(struct hostapd_data *hapd, } +static int hostapd_wps_set_application_ext(struct hostapd_data *hapd, + struct wps_context *wps) +{ + int i; + + for (i = 0; i < MAX_WPS_APPLICATION_EXTENSIONS; i++) { + wpabuf_free(wps->dev.application_ext[i]); + wps->dev.application_ext[i] = NULL; + + if (hapd->conf->wps_application_ext[i] == NULL) + continue; + + wps->dev.application_ext[i] = + wpabuf_dup(hapd->conf->wps_application_ext[i]); + if (wps->dev.application_ext[i] == NULL) { + while (--i >= 0) + wpabuf_free(wps->dev.application_ext[i]); + return -1; + } + } + + return 0; +} + + static void hostapd_free_wps(struct wps_context *wps) { int i; @@ -1077,6 +1102,9 @@ int hostapd_init_wps(struct hostapd_data *hapd, if (hostapd_wps_set_vendor_ext(hapd, wps) < 0) goto fail; + if (hostapd_wps_set_application_ext(hapd, wps) < 0) + goto fail; + wps->dev.os_version = WPA_GET_BE32(hapd->conf->os_version); if (conf->wps_rf_bands) { @@ -1311,6 +1339,7 @@ void hostapd_update_wps(struct hostapd_data *hapd) #endif /* CONFIG_WPS_UPNP */ hostapd_wps_set_vendor_ext(hapd, hapd->wps); + hostapd_wps_set_application_ext(hapd, hapd->wps); if (hapd->conf->wps_state) wps_registrar_update_ie(hapd->wps->registrar); diff --git a/src/wps/wps.h b/src/wps/wps.h index 9963c46..8605490 100644 --- a/src/wps/wps.h +++ b/src/wps/wps.h @@ -66,6 +66,10 @@ struct wps_credential { #define WPS_MAX_VENDOR_EXT_LEN 1024 /* maximum number of parsed WPS vendor extension attributes */ #define MAX_WPS_PARSE_VENDOR_EXT 10 +/* maximum number of advertised WPS application extension attributes */ +#define MAX_WPS_APPLICATION_EXTENSIONS 10 +/* maximum size of WPS application extension attribute */ +#define WPS_MAX_APPLICATION_EXT_LEN 1024 /** * struct wps_device_data - WPS Device Data @@ -98,6 +102,7 @@ struct wps_device_data { u16 config_methods; struct wpabuf *vendor_ext_m1; struct wpabuf *vendor_ext[MAX_WPS_VENDOR_EXTENSIONS]; + struct wpabuf *application_ext[MAX_WPS_APPLICATION_EXTENSIONS]; int p2p; u8 multi_ap_ext; diff --git a/src/wps/wps_dev_attr.c b/src/wps/wps_dev_attr.c index b209fea..0cc312e 100644 --- a/src/wps/wps_dev_attr.c +++ b/src/wps/wps_dev_attr.c @@ -242,6 +242,25 @@ int wps_build_vendor_ext(struct wps_device_data *dev, struct wpabuf *msg) } +int wps_build_application_ext(struct wps_device_data *dev, struct wpabuf *msg) +{ + int i; + + for (i = 0; i < MAX_WPS_APPLICATION_EXTENSIONS; i++) { + if (dev->application_ext[i] == NULL) + continue; + wpa_hexdump(MSG_DEBUG, "WPS: * Application Extension", + wpabuf_head_u8(dev->application_ext[i]), + wpabuf_len(dev->application_ext[i])); + wpabuf_put_be16(msg, ATTR_APPLICATION_EXT); + wpabuf_put_be16(msg, wpabuf_len(dev->application_ext[i])); + wpabuf_put_buf(msg, dev->application_ext[i]); + } + + return 0; +} + + static int wps_process_manufacturer(struct wps_device_data *dev, const u8 *str, size_t str_len) { diff --git a/src/wps/wps_dev_attr.h b/src/wps/wps_dev_attr.h index a4b4173..81fdd5f 100644 --- a/src/wps/wps_dev_attr.h +++ b/src/wps/wps_dev_attr.h @@ -33,6 +33,7 @@ void wps_process_vendor_ext_m1(struct wps_device_data *dev, const u8 ext); int wps_process_rf_bands(struct wps_device_data *dev, const u8 *bands); void wps_device_data_free(struct wps_device_data *dev); int wps_build_vendor_ext(struct wps_device_data *dev, struct wpabuf *msg); +int wps_build_application_ext(struct wps_device_data *dev, struct wpabuf *msg); int wps_build_req_dev_type(struct wps_device_data *dev, struct wpabuf *msg, unsigned int num_req_dev_types, const u8 *req_dev_types); diff --git a/src/wps/wps_registrar.c b/src/wps/wps_registrar.c index 671f5fe..da81d1a 100644 --- a/src/wps/wps_registrar.c +++ b/src/wps/wps_registrar.c @@ -1331,7 +1331,8 @@ static int wps_set_ie(struct wps_registrar *reg) wps_build_sel_pbc_reg_uuid_e(reg, beacon) || (reg->dualband && wps_build_rf_bands(®->wps->dev, beacon, 0)) || wps_build_wfa_ext(beacon, 0, auth_macs, count, 0) || - wps_build_vendor_ext(®->wps->dev, beacon)) { + wps_build_vendor_ext(®->wps->dev, beacon) || + wps_build_application_ext(®->wps->dev, beacon)) { wpabuf_free(beacon); wpabuf_free(probe); return -1; @@ -1361,7 +1362,8 @@ static int wps_set_ie(struct wps_registrar *reg) wps_build_probe_config_methods(reg, probe) || (reg->dualband && wps_build_rf_bands(®->wps->dev, probe, 0)) || wps_build_wfa_ext(probe, 0, auth_macs, count, 0) || - wps_build_vendor_ext(®->wps->dev, probe)) { + wps_build_vendor_ext(®->wps->dev, probe) || + wps_build_application_ext(®->wps->dev, probe)) { wpabuf_free(beacon); wpabuf_free(probe); return -1;