This change updates netdev_create_from_genl to take an enum which describes what type of MAC address we are using. These types map 1:1 to the valid 'AddressRandomization' configuration values: disabled: use permanent address for adapter once: randomize the MAC address once, on startup The current behavior is maintained but instead of generating a MAC inside manager (for the 'once' case) we generate it inside netdev. This patch is to prepare for address randomization per-network. --- src/manager.c | 33 +++++++++++---------------------- src/netdev.c | 12 +++++++++--- src/netdev.h | 2 +- 3 files changed, 21 insertions(+), 26 deletions(-) diff --git a/src/manager.c b/src/manager.c index a63f35ee..05f61b6f 100644 --- a/src/manager.c +++ b/src/manager.c @@ -46,7 +46,7 @@ static struct l_genl_family *nl80211 = NULL; static char **whitelist_filter; static char **blacklist_filter; -static bool randomize; +static enum netdev_set_mac_type mac_type; static bool use_default; struct wiphy_setup_state { @@ -104,9 +104,6 @@ static void wiphy_setup_state_destroy(struct wiphy_setup_state *state) static bool manager_use_default(struct wiphy_setup_state *state) { - uint8_t addr_buf[6]; - uint8_t *addr = NULL; - l_debug(""); if (!state->default_if_msg) { @@ -116,20 +113,13 @@ static bool manager_use_default(struct wiphy_setup_state *state) return false; } - if (randomize) { - wiphy_generate_random_address(state->wiphy, addr_buf); - addr = addr_buf; - } - - netdev_create_from_genl(state->default_if_msg, addr); + netdev_create_from_genl(state->default_if_msg, mac_type); return true; } static void manager_new_interface_cb(struct l_genl_msg *msg, void *user_data) { struct wiphy_setup_state *state = user_data; - uint8_t addr_buf[6]; - uint8_t *addr = NULL; int error; l_debug(""); @@ -161,13 +151,12 @@ static void manager_new_interface_cb(struct l_genl_msg *msg, void *user_data) return; } - if (randomize && !wiphy_has_feature(state->wiphy, - NL80211_FEATURE_MAC_ON_CREATE)) { - wiphy_generate_random_address(state->wiphy, addr_buf); - addr = addr_buf; - } + /* MAC will already be set, no need for netdev to set it again */ + if (mac_type == NETDEV_SET_MAC_ONCE && wiphy_has_feature(state->wiphy, + NL80211_FEATURE_MAC_ON_CREATE)) + mac_type = NETDEV_SET_MAC_DISABLED; - netdev_create_from_genl(msg, addr); + netdev_create_from_genl(msg, mac_type); } static void manager_new_interface_done(void *user_data) @@ -214,7 +203,7 @@ static void manager_create_interfaces(struct wiphy_setup_state *state) l_genl_msg_append_attr(msg, NL80211_ATTR_4ADDR, 1, "\0"); l_genl_msg_append_attr(msg, NL80211_ATTR_SOCKET_OWNER, 0, ""); - if (randomize && wiphy_has_feature(state->wiphy, + if (mac_type == NETDEV_SET_MAC_ONCE && wiphy_has_feature(state->wiphy, NL80211_FEATURE_MAC_ON_CREATE)) { uint8_t random_addr[6]; @@ -756,9 +745,9 @@ static int manager_init(void) "AddressRandomization"); if (randomize_str) { if (!strcmp(randomize_str, "once")) - randomize = true; + mac_type = NETDEV_SET_MAC_ONCE; else if (!strcmp(randomize_str, "disabled")) - randomize = false; + mac_type = NETDEV_SET_MAC_DISABLED; else l_warn("Invalid [General].AddressRandomization" " value: %s", randomize_str); @@ -796,7 +785,7 @@ static void manager_exit(void) l_genl_family_free(nl80211); nl80211 = NULL; - randomize = false; + mac_type = NETDEV_SET_MAC_DISABLED; } IWD_MODULE(manager, manager_init, manager_exit); diff --git a/src/netdev.c b/src/netdev.c index c11a483f..7aed9da5 100644 --- a/src/netdev.c +++ b/src/netdev.c @@ -4276,7 +4276,7 @@ error: } struct netdev *netdev_create_from_genl(struct l_genl_msg *msg, - const uint8_t *set_mac) + enum netdev_set_mac_type mac_type) { const char *ifname; const uint8_t *ifaddr; @@ -4337,8 +4337,14 @@ struct netdev *netdev_create_from_genl(struct l_genl_msg *msg, netdev->wiphy = wiphy; netdev->pae_over_nl80211 = pae_io == NULL; - if (set_mac) - memcpy(netdev->set_mac_once, set_mac, 6); + switch (mac_type) { + case NETDEV_SET_MAC_ONCE: + wiphy_generate_random_address(netdev->wiphy, + netdev->set_mac_once); + break; + default: + break; + } if (pae_io) { netdev->pae_io = pae_io; diff --git a/src/netdev.h b/src/netdev.h index d468dc46..a553e3d4 100644 --- a/src/netdev.h +++ b/src/netdev.h @@ -197,5 +197,5 @@ uint32_t netdev_station_watch_add(struct netdev *netdev, bool netdev_station_watch_remove(struct netdev *netdev, uint32_t id); struct netdev *netdev_create_from_genl(struct l_genl_msg *msg, - const uint8_t *set_mac); + enum netdev_set_mac_type mac_type); bool netdev_destroy(struct netdev *netdev); -- 2.21.1

For privacy reasons its advantageous to randomize or mask the MAC address when connecting to networks, especially public networks. This patch allows netdev to generate a new MAC address on a per-network basis. The generated MAC will remain the same when connecting to the same network. This allows reauthentications or roaming to work, and not have to fully re-connect (which would be required if the MAC changed on every connection). Changing the MAC requires bringing the interface down. This does lead to potential race conditions with respect to external processes. There are two potential conditions which are explained in a TODO comment in this patch. --- src/netdev.c | 219 ++++++++++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 206 insertions(+), 13 deletions(-) diff --git a/src/netdev.c b/src/netdev.c index 7aed9da5..72bd5e11 100644 --- a/src/netdev.c +++ b/src/netdev.c @@ -109,6 +109,7 @@ struct netdev { uint32_t set_interface_cmd_id; uint32_t rekey_offload_cmd_id; uint32_t qos_map_cmd_id; + uint32_t mac_change_cmd_id; enum netdev_result result; uint16_t last_code; /* reason or status, depending on result */ struct l_timeout *neighbor_report_timeout; @@ -146,6 +147,7 @@ struct netdev { bool expect_connect_failure : 1; bool aborting : 1; bool events_ready : 1; + bool mac_per_ssid : 1; }; struct netdev_preauth_state { @@ -613,6 +615,11 @@ static void netdev_free(void *data) netdev->qos_map_cmd_id = 0; } + if (netdev->mac_change_cmd_id) { + l_netlink_cancel(rtnl, netdev->mac_change_cmd_id); + netdev->mac_change_cmd_id = 0; + } + if (netdev->events_ready) WATCHLIST_NOTIFY(&netdev_watches, netdev_watch_func_t, netdev, NETDEV_WATCH_EVENT_DEL); @@ -2377,13 +2384,15 @@ static struct l_genl_msg *netdev_build_cmd_connect(struct netdev *netdev, return msg; } -static int netdev_connect_common(struct netdev *netdev, - struct l_genl_msg *cmd_connect, - struct scan_bss *bss, - struct handshake_state *hs, - struct eapol_sm *sm, - netdev_event_func_t event_filter, - netdev_connect_cb_t cb, void *user_data) +struct rtnl_data { + struct netdev *netdev; + struct l_genl_msg *cmd_connect; + uint8_t addr[ETH_ALEN]; + int ref; +}; + +static int netdev_begin_connection(struct netdev *netdev, + struct l_genl_msg *cmd_connect) { if (cmd_connect) { netdev->connect_cmd_id = l_genl_family_send(nl80211, @@ -2396,6 +2405,176 @@ static int netdev_connect_common(struct netdev *netdev, } } + handshake_state_set_supplicant_address(netdev->handshake, netdev->addr); + + /* set connected since the auth protocols cannot do so internally */ + if (netdev->ap && auth_proto_start(netdev->ap)) + netdev->connected = true; + + return 0; +} + +static void netdev_mac_change_failed(struct netdev *netdev, + struct rtnl_data *req, int error) +{ + l_error("Error setting mac address on %d: %s", netdev->index, + strerror(-error)); + + l_genl_msg_unref(req->cmd_connect); + netdev->mac_change_cmd_id = 0; + + netdev_connect_failed(netdev, NETDEV_RESULT_SET_MAC_FAILED, + MMPDU_REASON_CODE_UNSPECIFIED); + + /* + * If the interface is down and we failed to up it we need to notify + * any watchers since we have been skipping the notification while + * mac_change_cmd_id was set. + */ + if (!netdev_get_is_up(netdev)) + WATCHLIST_NOTIFY(&netdev_watches, netdev_watch_func_t, + netdev, NETDEV_WATCH_EVENT_DOWN); + +} + +static void netdev_mac_destroy(void *user_data) +{ + struct rtnl_data *req = user_data; + + req->ref--; + + /* still pending requests? */ + if (req->ref) + return; + + l_free(req); +} + +static void netdev_mac_power_up_cb(int error, uint16_t type, + const void *data, uint32_t len, + void *user_data) +{ + struct rtnl_data *req = user_data; + struct netdev *netdev = req->netdev; + + netdev->mac_change_cmd_id = 0; + + if (error) { + l_error("Error taking interface %u up for per-network MAC " + "generation: %s", netdev->index, strerror(-error)); + netdev_mac_change_failed(netdev, req, error); + return; + } + + /* + * Pick up where we left off in netdev_connect_commmon. + */ + if (netdev_begin_connection(netdev, req->cmd_connect) < 0) { + l_error("Failed to connect after changing MAC"); + netdev_connect_failed(netdev, NETDEV_RESULT_ASSOCIATION_FAILED, + MMPDU_STATUS_CODE_UNSPECIFIED); + } +} + +static void netdev_mac_power_down_cb(int error, uint16_t type, + const void *data, uint32_t len, + void *user_data) +{ + struct rtnl_data *req = user_data; + struct netdev *netdev = req->netdev; + + if (error) { + l_error("Error taking interface %u down for per-network MAC " + "generation: %s", netdev->index, strerror(-error)); + netdev_mac_change_failed(netdev, req, error); + return; + } + + req->ref++; + + l_debug("Setting generated address on ifindex: %d to: "MAC, + netdev->index, MAC_STR(req->addr)); + netdev->mac_change_cmd_id = l_rtnl_set_mac(rtnl, netdev->index, + req->addr, true, + netdev_mac_power_up_cb, req, + netdev_mac_destroy); + if (!netdev->mac_change_cmd_id) { + req->ref--; + netdev_mac_change_failed(netdev, req, -EIO); + } +} + +/* + * TODO: There are some potential race conditions that are being ignored. There + * is nothing that IWD itself can do to solve these, they require kernel + * changes: + * + * 1. A perfectly timed ifdown could be ignored. If an external process + * brings down an interface just before calling this function we would only + * get a single newlink event since there is no state change doing a second + * ifdown (nor an error from the kernel). This newlink event would be ignored + * since IWD thinks its from our own doing. This would result in IWD changing + * the MAC and bringing the interface back up which would look very strange + * and unexpected to someone who just tried to ifdown an interface. + * + * 2. A perfectly timed ifup could result in a failed connection. If an external + * process ifup's just after IWD ifdown's but before changing the MAC this + * would cause the MAC change to fail. This failure would result in a failed + * connection. + * + * Returns 0 if a MAC change procedure was started. + * Returns -EALREADY if the requested MAC matched our current MAC + * Returns -EIO if there was an IO error when powering down + */ +static int netdev_start_powered_mac_change(struct netdev *netdev, + struct scan_bss *bss, + struct l_genl_msg *cmd_connect) +{ + struct rtnl_data *req; + uint8_t new_addr[6]; + + wiphy_generate_address_from_ssid(netdev->wiphy, (const char *)bss->ssid, + new_addr); + + /* + * MAC has already been changed previously, no need to again + */ + if (!memcmp(new_addr, netdev->addr, sizeof(new_addr))) + return -EALREADY; + + req = l_new(struct rtnl_data, 1); + req->netdev = netdev; + /* This message will need to be unreffed upon any error */ + req->cmd_connect = cmd_connect; + req->ref++; + memcpy(req->addr, new_addr, sizeof(req->addr)); + + netdev->mac_change_cmd_id = l_rtnl_set_powered(rtnl, netdev->index, + false, netdev_mac_power_down_cb, + req, netdev_mac_destroy); + + if (!netdev->mac_change_cmd_id) { + l_genl_msg_unref(req->cmd_connect); + l_free(req); + /* + * If the user is requiring MAC randomization/hashing we should + * not allow a connection if setting this MAC failed. + */ + netdev_mac_change_failed(netdev, req, -EIO); + return -EIO; + } + + return 0; +} + +static int netdev_connect_common(struct netdev *netdev, + struct l_genl_msg *cmd_connect, + struct scan_bss *bss, + struct handshake_state *hs, + struct eapol_sm *sm, + netdev_event_func_t event_filter, + netdev_connect_cb_t cb, void *user_data) +{ netdev->event_filter = event_filter; netdev->connect_cb = cb; netdev->user_data = user_data; @@ -2407,17 +2586,21 @@ static int netdev_connect_common(struct netdev *netdev, netdev_rssi_level_init(netdev); handshake_state_set_authenticator_address(hs, bss->addr); - handshake_state_set_supplicant_address(hs, netdev->addr); if (!wiphy_has_ext_feature(netdev->wiphy, NL80211_EXT_FEATURE_CAN_REPLACE_PTK0)) handshake_state_set_no_rekey(hs, true); - /* set connected since the auth protocols cannot do so internally */ - if (netdev->ap && auth_proto_start(netdev->ap)) - netdev->connected = true; + if (netdev->mac_per_ssid) { + int ret = netdev_start_powered_mac_change(netdev, bss, + cmd_connect); + if (ret == 0) + return 0; + else if (ret != -EALREADY) + return ret; + } - return 0; + return netdev_begin_connection(netdev, cmd_connect); } int netdev_connect(struct netdev *netdev, struct scan_bss *bss, @@ -4071,7 +4254,14 @@ static void netdev_newlink_notify(const struct ifinfomsg *ifi, int bytes) new_up = netdev_get_is_up(netdev); - if (old_up != new_up) + /* + * If mac_change_cmd_id is set we are in the process of changing the + * MAC address and this event is a result of powering down/up. In this + * case we do not want to emit a netdev DOWN/UP event as this would + * cause other modules to behave as such. We do, however, want to emit + * address changes so other modules get the new MAC address updated. + */ + if (old_up != new_up && !netdev->mac_change_cmd_id) WATCHLIST_NOTIFY(&netdev_watches, netdev_watch_func_t, netdev, new_up ? NETDEV_WATCH_EVENT_UP : NETDEV_WATCH_EVENT_DOWN); @@ -4342,6 +4532,9 @@ struct netdev *netdev_create_from_genl(struct l_genl_msg *msg, wiphy_generate_random_address(netdev->wiphy, netdev->set_mac_once); break; + case NETDEV_SET_MAC_NETWORK: + netdev->mac_per_ssid = true; + break; default: break; } -- 2.21.1

This is to provide a better hint to a user of exactly what went wrong since the error is not related to the actual connection attmempt, and rather some kind of kernel problem. --- src/station.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/station.c b/src/station.c index 6ec7eb4b..cd93b97c 100644 --- a/src/station.c +++ b/src/station.c @@ -2251,6 +2251,10 @@ static void station_connect_cb(struct netdev *netdev, enum netdev_result result, return; break; + case NETDEV_RESULT_SET_MAC_FAILED: + l_error("failed to set MAC address during connection, try " + " turning off AddressRandomization=network option"); + break; default: break; } -- 2.21.1