On Thu, 2019-05-16 at 17:19 -0700, Eric Caruso wrote: > Add D-Bus property: > * MACAddressRandomizationMask: a{say} > > which configure random MAC address functionality in the Wi-Fi > driver via netlink. This also fixes weird pointer ownership > that was causing memory issues. > > Change-Id: I6b712e371de2e6247249627058014f51a06cc625 > Signed-off-by: Eric Caruso <ejcaruso@xxxxxxxxxxxx> > --- > doc/dbus.doxygen | 5 + > wpa_supplicant/ctrl_iface.c | 55 +-------- > wpa_supplicant/dbus/dbus_new.c | 5 + > wpa_supplicant/dbus/dbus_new_handlers.c | 153 ++++++++++++++++++++++++ > wpa_supplicant/dbus/dbus_new_handlers.h | 2 + > wpa_supplicant/scan.c | 91 ++++++++------ > wpa_supplicant/scan.h | 2 + > wpa_supplicant/wpa_supplicant.c | 56 +++++++++ > wpa_supplicant/wpa_supplicant_i.h | 4 + > 9 files changed, 287 insertions(+), 86 deletions(-) > > diff --git a/doc/dbus.doxygen b/doc/dbus.doxygen > index 072ed3486..9f6af52b1 100644 > --- a/doc/dbus.doxygen > +++ b/doc/dbus.doxygen > @@ -1043,6 +1043,11 @@ fi.w1.wpa_supplicant1.CreateInterface. > <h3>WpsPriority - s - (read/write)</h3> > <p>Priority for the networks added through WPS</p> > </li> > + > + <li> > + <h3>MACAddressRandomizationMask - a{say} - (read/write)</h3> > + <p>Masks to show which bits not to randomize with MAC address randomization. Possible keys are "scan", "sched_scan", and "pno". Values must be an array of 6 bytes.</p> > + </li> > </ul> > > \subsection dbus_interface_signals Signals > diff --git a/wpa_supplicant/ctrl_iface.c b/wpa_supplicant/ctrl_iface.c > index c1664d043..cea5a4e9b 100644 > --- a/wpa_supplicant/ctrl_iface.c > +++ b/wpa_supplicant/ctrl_iface.c > @@ -9583,59 +9583,10 @@ static int wpas_ctrl_iface_mac_rand_scan(struct wpa_supplicant *wpa_s, > return -1; > } > > - if (!enable) { > - wpas_mac_addr_rand_scan_clear(wpa_s, type); > - if (wpa_s->pno) { > - if (type & MAC_ADDR_RAND_PNO) { > - wpas_stop_pno(wpa_s); > - wpas_start_pno(wpa_s); > - } > - } else if (wpa_s->sched_scanning && > - (type & MAC_ADDR_RAND_SCHED_SCAN)) { > - wpas_scan_restart_sched_scan(wpa_s); > - } > - return 0; > - } > + if (!enable) > + return wpas_disable_mac_addr_randomization(wpa_s, type); > > - if ((addr && !mask) || (!addr && mask)) { > - wpa_printf(MSG_INFO, > - "CTRL: MAC_RAND_SCAN invalid addr/mask combination"); > - return -1; > - } > - > - if (addr && mask && (!(mask[0] & 0x01) || (addr[0] & 0x01))) { > - wpa_printf(MSG_INFO, > - "CTRL: MAC_RAND_SCAN cannot allow multicast address"); > - return -1; > - } > - > - if (type & MAC_ADDR_RAND_SCAN) { > - if (wpas_mac_addr_rand_scan_set(wpa_s, MAC_ADDR_RAND_SCAN, > - addr, mask)) > - return -1; > - } > - > - if (type & MAC_ADDR_RAND_SCHED_SCAN) { > - if (wpas_mac_addr_rand_scan_set(wpa_s, MAC_ADDR_RAND_SCHED_SCAN, > - addr, mask)) > - return -1; > - > - if (wpa_s->sched_scanning && !wpa_s->pno) > - wpas_scan_restart_sched_scan(wpa_s); > - } > - > - if (type & MAC_ADDR_RAND_PNO) { > - if (wpas_mac_addr_rand_scan_set(wpa_s, MAC_ADDR_RAND_PNO, > - addr, mask)) > - return -1; > - > - if (wpa_s->pno) { > - wpas_stop_pno(wpa_s); > - wpas_start_pno(wpa_s); > - } > - } > - > - return 0; > + return wpas_enable_mac_addr_randomization(wpa_s, type, addr, mask); > } > > > diff --git a/wpa_supplicant/dbus/dbus_new.c b/wpa_supplicant/dbus/dbus_new.c > index fc2fc2ef1..c5d3b8a02 100644 > --- a/wpa_supplicant/dbus/dbus_new.c > +++ b/wpa_supplicant/dbus/dbus_new.c > @@ -3803,6 +3803,11 @@ static const struct wpa_dbus_property_desc wpas_dbus_interface_properties[] = { > NULL, > NULL > }, > + { "MACAddressRandomizationMask", WPAS_DBUS_NEW_IFACE_INTERFACE, "a{say}", > + wpas_dbus_getter_mac_address_randomization_mask, > + wpas_dbus_setter_mac_address_randomization_mask, > + NULL > + }, > { NULL, NULL, NULL, NULL, NULL, NULL } > }; > > diff --git a/wpa_supplicant/dbus/dbus_new_handlers.c b/wpa_supplicant/dbus/dbus_new_handlers.c > index 6c36d91a0..f528edc7c 100644 > --- a/wpa_supplicant/dbus/dbus_new_handlers.c > +++ b/wpa_supplicant/dbus/dbus_new_handlers.c > @@ -3989,6 +3989,159 @@ out: > } > > > +/** > + * wpas_dbus_setter_mac_address_randomization_mask - Set masks used for > + * MAC address randomization > + * @iter: Pointer to incoming dbus message iter > + * @error: Location to store error on failure > + * @user_data: Function specific data > + * Returns: TRUE on success, FALSE on failure > + * > + * Setter for "MACAddressRandomizationMask" property. > + */ > +dbus_bool_t wpas_dbus_setter_mac_address_randomization_mask( > + const struct wpa_dbus_property_desc *property_desc, > + DBusMessageIter *iter, DBusError *error, void *user_data) > +{ > + struct wpa_supplicant *wpa_s = user_data; > + DBusMessageIter variant_iter, dict_iter, entry_iter, array_iter; > + char *key; Can we make this 'const char *key'? I know other code that calls dbus_message_iter_get_basic() for strings doesn't do that, but I think it should. And we shouldn't be modifying the value anyway. > + uint rand_type = 0; > + u8 *mask; Same here, let's make this 'const u8 *mask'. > + int mask_len; > + uint rand_types_to_disable = MAC_ADDR_RAND_ALL; I think wpa_supplicant preference is "unsigned int" rather than "uint", but I could be wrong. > + dbus_message_iter_recurse(iter, &variant_iter); > + dbus_message_iter_recurse(&variant_iter, &dict_iter); > + while (dbus_message_iter_get_arg_type(&dict_iter) == > + DBUS_TYPE_DICT_ENTRY) { > + dbus_message_iter_recurse(&dict_iter, &entry_iter); > + if (dbus_message_iter_get_arg_type(&entry_iter) != DBUS_TYPE_STRING) { > + dbus_set_error(error, DBUS_ERROR_FAILED, > + "%s: key not a string", __func__); > + return FALSE; > + } > + dbus_message_iter_get_basic(&entry_iter, &key); > + dbus_message_iter_next(&entry_iter); > + if (dbus_message_iter_get_arg_type(&entry_iter) != DBUS_TYPE_ARRAY || > + dbus_message_iter_get_element_type(&entry_iter) != DBUS_TYPE_BYTE) { > + dbus_set_error(error, DBUS_ERROR_FAILED, > + "%s: mask was not a byte array", __func__); > + return FALSE; > + } > + dbus_message_iter_recurse(&entry_iter, &array_iter); > + dbus_message_iter_get_fixed_array(&array_iter, &mask, &mask_len); > + > + if (os_strcmp(key, "scan") == 0) { > + rand_type = MAC_ADDR_RAND_SCAN; > + } else if (os_strcmp(key, "sched_scan") == 0) { > + rand_type = MAC_ADDR_RAND_SCHED_SCAN; > + } else if (os_strcmp(key, "pno") == 0) { > + rand_type = MAC_ADDR_RAND_PNO; > + } else { > + dbus_set_error(error, DBUS_ERROR_FAILED, > + "%s: bad scan type \"%s\"", __func__, key); > + return FALSE; > + } > + > + if (mask_len != ETH_ALEN) { > + dbus_set_error(error, DBUS_ERROR_FAILED, > + "%s: malformed MAC mask given", __func__); > + return FALSE; > + } > + > + if (wpas_enable_mac_addr_randomization( > + wpa_s, rand_type, wpa_s->perm_addr, mask)) { > + dbus_set_error(error, DBUS_ERROR_FAILED, > + "%s: failed to set up MAC address randomization for %s", > + __func__, key); > + return FALSE; > + } > + > + wpa_printf(MSG_DEBUG, "%s: Enabled MAC address randomization for %s with mask: " > + MACSTR, wpa_s->ifname, key, MAC2STR(mask)); > + rand_types_to_disable &= ~rand_type; > + dbus_message_iter_next(&dict_iter); > + } > + > + if (rand_types_to_disable && > + wpas_disable_mac_addr_randomization(wpa_s, rand_types_to_disable)) { > + dbus_set_error(error, DBUS_ERROR_FAILED, > + "%s: failed to disable MAC address randomization", > + __func__); > + return FALSE; > + } > + > + return TRUE; > +} > + > + > +dbus_bool_t wpas_dbus_getter_mac_address_randomization_mask( > + const struct wpa_dbus_property_desc *property_desc, > + DBusMessageIter *iter, DBusError *error, void *user_data) > +{ > + struct wpa_supplicant *wpa_s = user_data; > + DBusMessageIter variant_iter, dict_iter, entry_iter, array_iter; > + int i; > + u8 mask_buf[ETH_ALEN]; > + /* Read docs on dbus_message_iter_append_fixed_array for why this > + * is necessary... */ > + u8* mask = mask_buf; > + static const struct { > + const char *key; > + unsigned int type; > + } types[] = { > + { "scan", MAC_ADDR_RAND_SCAN }, > + { "sched_scan", MAC_ADDR_RAND_SCHED_SCAN }, > + { "pno", MAC_ADDR_RAND_PNO } > + }; > + > + if (!dbus_message_iter_open_container(iter, DBUS_TYPE_VARIANT, > + "a{say}", &variant_iter) || > + !dbus_message_iter_open_container(&variant_iter, DBUS_TYPE_ARRAY, > + "{say}", &dict_iter)) { > + dbus_set_error_const(error, DBUS_ERROR_NO_MEMORY, "no memory"); > + return FALSE; > + } > + > + for (i = 0; i < sizeof(types)/sizeof(types[0]); i++) { > + if (wpas_mac_addr_rand_scan_get_mask(wpa_s, types[i].type, mask)) > + continue; > + > + if (!dbus_message_iter_open_container(&dict_iter, > + DBUS_TYPE_DICT_ENTRY, > + NULL, &entry_iter) || > + !dbus_message_iter_append_basic(&entry_iter, > + DBUS_TYPE_STRING, > + &types[i].key) || > + !dbus_message_iter_open_container(&entry_iter, > + DBUS_TYPE_ARRAY, > + DBUS_TYPE_BYTE_AS_STRING, > + &array_iter) || > + !dbus_message_iter_append_fixed_array(&array_iter, > + DBUS_TYPE_BYTE, > + &mask, > + ETH_ALEN) || > + !dbus_message_iter_close_container(&entry_iter, > + &array_iter) || > + !dbus_message_iter_close_container(&dict_iter, > + &entry_iter)) { > + dbus_set_error_const(error, DBUS_ERROR_NO_MEMORY, > + "no memory"); > + return FALSE; > + } > + } > + > + if (!dbus_message_iter_close_container(&variant_iter, &dict_iter) || > + !dbus_message_iter_close_container(iter, &variant_iter)) { > + dbus_set_error_const(error, DBUS_ERROR_NO_MEMORY, "no memory"); > + return FALSE; > + } > + > + return TRUE; > +} > + > + > /** > * wpas_dbus_getter_sta_address - Return the address of a connected station > * @iter: Pointer to incoming dbus message iter > diff --git a/wpa_supplicant/dbus/dbus_new_handlers.h b/wpa_supplicant/dbus/dbus_new_handlers.h > index d922ce1b4..afa26efed 100644 > --- a/wpa_supplicant/dbus/dbus_new_handlers.h > +++ b/wpa_supplicant/dbus/dbus_new_handlers.h > @@ -177,6 +177,8 @@ DECLARE_ACCESSOR(wpas_dbus_getter_pkcs11_engine_path); > DECLARE_ACCESSOR(wpas_dbus_getter_pkcs11_module_path); > DECLARE_ACCESSOR(wpas_dbus_getter_blobs); > DECLARE_ACCESSOR(wpas_dbus_getter_stas); > +DECLARE_ACCESSOR(wpas_dbus_getter_mac_address_randomization_mask); > +DECLARE_ACCESSOR(wpas_dbus_setter_mac_address_randomization_mask); > DECLARE_ACCESSOR(wpas_dbus_getter_sta_address); > DECLARE_ACCESSOR(wpas_dbus_getter_sta_aid); > DECLARE_ACCESSOR(wpas_dbus_getter_sta_caps); > diff --git a/wpa_supplicant/scan.c b/wpa_supplicant/scan.c > index 7abb028dd..c61f5553e 100644 > --- a/wpa_supplicant/scan.c > +++ b/wpa_supplicant/scan.c > @@ -79,6 +79,27 @@ static int wpas_wps_in_use(struct wpa_supplicant *wpa_s, > #endif /* CONFIG_WPS */ > > > +static int wpa_setup_mac_addr_rand_params(struct wpa_driver_scan_params *params, > + const u8 *mac_addr) > +{ > + u8 *tmp; > + > + if (!mac_addr) > + return 0; > + > + params->mac_addr_rand = 1; > + > + tmp = os_malloc(2 * ETH_ALEN); > + if (!tmp) > + return -1; > + > + os_memcpy(tmp, mac_addr, 2 * ETH_ALEN); > + params->mac_addr = tmp; > + params->mac_addr_mask = tmp + ETH_ALEN; > + return 0; All the wpa_setup_mac_addr_rand_params() stuff could be a separate patch in the series and might be easier to review/bisect later if needed. I'll leave that up to Jouni though. > +} > + > + > /** > * wpa_supplicant_enabled_networks - Check whether there are enabled networks > * @wpa_s: Pointer to wpa_supplicant data > @@ -169,7 +190,9 @@ static void wpas_trigger_scan_cb(struct wpa_radio_work *work, int deinit) > return; > } > > - if (wpas_update_random_addr_disassoc(wpa_s) < 0) { > + if (wpa_s->mac_addr_rand_enable & MAC_ADDR_RAND_SCAN) > + wpa_setup_mac_addr_rand_params(params, wpa_s->mac_addr_scan); > + else if (wpas_update_random_addr_disassoc(wpa_s) < 0) { > wpa_msg(wpa_s, MSG_INFO, > "Failed to assign random MAC address for a scan"); > wpa_scan_free_params(params); > @@ -185,6 +208,7 @@ static void wpas_trigger_scan_cb(struct wpa_radio_work *work, int deinit) > "Request driver to clear scan cache due to local BSS flush"); > params->only_new_results = 1; > } > + Spurious newline? > ret = wpa_drv_scan(wpa_s, params); > /* > * Store the obtained vendor scan cookie (if any) in wpa_s context. > @@ -1212,11 +1236,7 @@ ssid_list_set: > > if ((wpa_s->mac_addr_rand_enable & MAC_ADDR_RAND_SCAN) && > wpa_s->wpa_state <= WPA_SCANNING) { > - params.mac_addr_rand = 1; > - if (wpa_s->mac_addr_scan) { > - params.mac_addr = wpa_s->mac_addr_scan; > - params.mac_addr_mask = wpa_s->mac_addr_scan + ETH_ALEN; > - } > + wpa_setup_mac_addr_rand_params(¶ms, wpa_s->mac_addr_scan); > } > > if (!is_zero_ether_addr(wpa_s->next_scan_bssid)) { > @@ -1665,12 +1685,7 @@ scan: > > if ((wpa_s->mac_addr_rand_enable & MAC_ADDR_RAND_SCHED_SCAN) && > wpa_s->wpa_state <= WPA_SCANNING) { > - params.mac_addr_rand = 1; > - if (wpa_s->mac_addr_sched_scan) { > - params.mac_addr = wpa_s->mac_addr_sched_scan; > - params.mac_addr_mask = wpa_s->mac_addr_sched_scan + > - ETH_ALEN; > - } > + wpa_setup_mac_addr_rand_params(¶ms, wpa_s->mac_addr_sched_scan); > } > > wpa_scan_set_relative_rssi_params(wpa_s, scan_params); > @@ -2535,23 +2550,9 @@ wpa_scan_clone_params(const struct wpa_driver_scan_params *src) > params->sched_scan_plans_num = src->sched_scan_plans_num; > } > > - if (src->mac_addr_rand) { > - params->mac_addr_rand = src->mac_addr_rand; > - > - if (src->mac_addr && src->mac_addr_mask) { > - u8 *mac_addr; > - > - mac_addr = os_malloc(2 * ETH_ALEN); > - if (!mac_addr) > - goto failed; > - > - os_memcpy(mac_addr, src->mac_addr, ETH_ALEN); > - os_memcpy(mac_addr + ETH_ALEN, src->mac_addr_mask, > - ETH_ALEN); > - params->mac_addr = mac_addr; > - params->mac_addr_mask = mac_addr + ETH_ALEN; > - } > - } > + if (src->mac_addr_rand && > + wpa_setup_mac_addr_rand_params(params, (const u8 *)src->mac_addr)) > + goto failed; > > if (src->bssid) { > u8 *bssid; > @@ -2739,11 +2740,7 @@ int wpas_start_pno(struct wpa_supplicant *wpa_s) > > if ((wpa_s->mac_addr_rand_enable & MAC_ADDR_RAND_PNO) && > wpa_s->wpa_state <= WPA_SCANNING) { > - params.mac_addr_rand = 1; > - if (wpa_s->mac_addr_pno) { > - params.mac_addr = wpa_s->mac_addr_pno; > - params.mac_addr_mask = wpa_s->mac_addr_pno + ETH_ALEN; > - } > + wpa_setup_mac_addr_rand_params(¶ms, wpa_s->mac_addr_pno); > } > > wpa_scan_set_relative_rssi_params(wpa_s, ¶ms); > @@ -2843,6 +2840,32 @@ int wpas_mac_addr_rand_scan_set(struct wpa_supplicant *wpa_s, > } > > > +int wpas_mac_addr_rand_scan_get_mask(struct wpa_supplicant *wpa_s, > + unsigned int type, u8 *mask) > +{ > + u8 *to_copy = NULL; > + > + if ((wpa_s->mac_addr_rand_enable & type) != type) > + return -1; > + > + if (type == MAC_ADDR_RAND_SCAN) { > + to_copy = wpa_s->mac_addr_scan; > + } else if (type == MAC_ADDR_RAND_SCHED_SCAN) { > + to_copy = wpa_s->mac_addr_sched_scan; > + } else if (type == MAC_ADDR_RAND_PNO) { > + to_copy = wpa_s->mac_addr_pno; Ugh; future cleanup here could be to make wpa_s->mac_addrs[3][ETH_ALEN] or something and have MAC_ADDR_RAND_* be an index into that table. Overall LGTM after the variable type changes. Thanks, Dan > + } else { > + wpa_printf(MSG_DEBUG, > + "scan: Invalid MAC randomization type=0x%x", > + type); > + return -1; > + } > + > + os_memcpy(mask, to_copy + ETH_ALEN, ETH_ALEN); > + return 0; > +} > + > + > int wpas_abort_ongoing_scan(struct wpa_supplicant *wpa_s) > { > struct wpa_radio_work *work; > diff --git a/wpa_supplicant/scan.h b/wpa_supplicant/scan.h > index 2aa0a8be0..58caa7818 100644 > --- a/wpa_supplicant/scan.h > +++ b/wpa_supplicant/scan.h > @@ -52,6 +52,8 @@ void wpas_mac_addr_rand_scan_clear(struct wpa_supplicant *wpa_s, > int wpas_mac_addr_rand_scan_set(struct wpa_supplicant *wpa_s, > unsigned int type, const u8 *addr, > const u8 *mask); > +int wpas_mac_addr_rand_scan_get_mask(struct wpa_supplicant *wpa_s, > + unsigned int type, u8 *mask); > int wpas_abort_ongoing_scan(struct wpa_supplicant *wpa_s); > void filter_scan_res(struct wpa_supplicant *wpa_s, > struct wpa_scan_results *res); > diff --git a/wpa_supplicant/wpa_supplicant.c b/wpa_supplicant/wpa_supplicant.c > index 799e215a7..a5c3f6f29 100644 > --- a/wpa_supplicant/wpa_supplicant.c > +++ b/wpa_supplicant/wpa_supplicant.c > @@ -7477,3 +7477,59 @@ int wpa_is_bss_tmp_disallowed(struct wpa_supplicant *wpa_s, > > return 1; > } > + > +int wpas_enable_mac_addr_randomization(struct wpa_supplicant *wpa_s, > + int type, u8 *addr, u8 *mask) > +{ > + if ((addr && !mask) || (!addr && mask)) { > + wpa_printf(MSG_INFO, > + "MAC_ADDR_RAND_SCAN invalid addr/mask combination"); > + return -1; > + } > + > + if (addr && mask && (!(mask[0] & 0x01) || (addr[0] & 0x01))) { > + wpa_printf(MSG_INFO, > + "MAC_ADDR_RAND_SCAN cannot allow multicast address"); > + return -1; > + } > + > + if (type & MAC_ADDR_RAND_SCAN) { > + wpas_mac_addr_rand_scan_set(wpa_s, MAC_ADDR_RAND_SCAN, > + addr, mask); > + } > + > + if (type & MAC_ADDR_RAND_SCHED_SCAN) { > + wpas_mac_addr_rand_scan_set(wpa_s, MAC_ADDR_RAND_SCHED_SCAN, > + addr, mask); > + > + if (wpa_s->sched_scanning && !wpa_s->pno) > + wpas_scan_restart_sched_scan(wpa_s); > + } > + > + if (type & MAC_ADDR_RAND_PNO) { > + wpas_mac_addr_rand_scan_set(wpa_s, MAC_ADDR_RAND_PNO, > + addr, mask); > + if (wpa_s->pno) { > + wpas_stop_pno(wpa_s); > + wpas_start_pno(wpa_s); > + } > + } > + > + return 0; > +} > + > + > +int wpas_disable_mac_addr_randomization(struct wpa_supplicant *wpa_s, int type) > +{ > + wpas_mac_addr_rand_scan_clear(wpa_s, type); > + if (wpa_s->pno) { > + if (type & MAC_ADDR_RAND_PNO) { > + wpas_stop_pno(wpa_s); > + wpas_start_pno(wpa_s); > + } > + } else if (wpa_s->sched_scanning && (type & MAC_ADDR_RAND_SCHED_SCAN)) { > + wpas_scan_restart_sched_scan(wpa_s); > + } > + > + return 0; > +} > diff --git a/wpa_supplicant/wpa_supplicant_i.h b/wpa_supplicant/wpa_supplicant_i.h > index b51390ba3..d2177369d 100644 > --- a/wpa_supplicant/wpa_supplicant_i.h > +++ b/wpa_supplicant/wpa_supplicant_i.h > @@ -1417,6 +1417,10 @@ size_t wpas_supp_op_class_ie(struct wpa_supplicant *wpa_s, > struct wpa_ssid *ssid, > int freq, u8 *pos, size_t len); > > +int wpas_enable_mac_addr_randomization(struct wpa_supplicant *wpa_s, > + int type, u8 *addr, u8 *mask); > +int wpas_disable_mac_addr_randomization(struct wpa_supplicant *wpa_s, int type); > + > /** > * wpa_supplicant_ctrl_iface_ctrl_rsp_handle - Handle a control response > * @wpa_s: Pointer to wpa_supplicant data _______________________________________________ Hostap mailing list Hostap@xxxxxxxxxxxxxxxxxxx http://lists.infradead.org/mailman/listinfo/hostap