[TASK] Revive bml_set_wifi_credentials

tomereli commented 4 years ago

Description

bml_set_wifi_credentials is an old implementation for a CLI command for changing credentials for a specific radio. It needs to be revived to allow setting credentials for any agent in the network.

Currently, the CLI command is defined to accept the following parameters:

int bml_set_wifi_credentials(BML_CTX ctx, const char *ssid, const char *pass, int sec, int vap_id, int force)

The way to set credentials in EasyMesh is using autoconfig where the controller is configured per alid. Supported parameters are ssid, password (network key), encryption type, authentication type, operating class and whether to configure as bAP or fAP. For more information, refer to the WSC M2 message defined in the WSC specification.

Therefore, the bml_set_wifi_credentials command should be updated to support all the new parameters, and delete the unused ones (vap_id and force). In addition, it should provide another option which is whether to send auto-config renew to all known agents so the new credentials are picked up.

The BML API should be refactored to allow setting credentials for a specific alid, with multiple operating classes.

Detailed description

The BML cli should support the following operations:

set credentials for alid
clear credentials
update credentials

Examples

Set fronthaul 2.4G VAP

bml_set_wifi_credentials 11:22:33:44:55:66 multiap-24g maprocks1 8x

Set backhaul 5G VAP

bml_set_wifi_credentials 11:22:33:44:55:66 multiap-5g maprocks1 12x backhaul

Clear credentials for alid 11:22:33:44:55:66

bml_clear_wifi_credentials 11:22:33:44:55:66

Clear all credentials:

bml_clear_wifi_credentials

Trigger autoconfig renew to all alids:

bml_update_wifi_credentials

Trigger autoconfig renew to specific alid:

bml_update_wifi_credentials 11:22:33:44:55:66

Proposed low-level API in son_master_thread

struct sWifiCredentials {
    std::string ssid;
    std::string network_key;
    int opclass;
    int auth_type;
    int enc_type;
    bool backhaul;
}

/**
 * @brief set wifi credentials for specific Alid
 * 
 * Sets wifi credentials for specific alid and sends autoconfig renew if requested.
 *
 * @param alid alid of the agent to configure
 * @param credentials credentials vector, can be empty for teardown.
 * @return true if successful, false otherwise 
 */
bool set_wifi_credentials(const sMacAddr &alid,
                                       std::vector<sWifiCredentials> &credentials);

/**
 * @brief trigger wifi credentials update
 * 
 * Triggers wifi credentials update using the controller's current configuration.
 * If alids is empty, autoconfig renew for all known alids is sent.
 * 
 * @param alids list of alids for which to trigger credentials update
 * @return true if successful, false otherwise 
 */
bool update_wifi_credentials(const std::vector<sMacAddr> &alids);

Exit Criteria

Test all new BML commands on actual RAX40 device with a NUC controller.

vitalybu commented 4 years ago

Few questions and a comment:

What's the purpose of the "clear credentials" operation?
What's the difference between "set credentials for alid" and "update credentials"?
In what scenario we'll need to clear the credentials of a specific alid? Or clear the credentials at all?
Calling bml_clear_wifi_credentials without any arguments to clear the entire network is the best idea... It'll happen all the time.

tomereli commented 4 years ago

Few questions and a comment:

What's the purpose of the "clear credentials" operation?

The purpose is to allow tearing down all radios in an agent.

What's the difference between "set credentials for alid" and "update credentials"?

set credentials update the controller bss_info_database:

std::unordered_map<sMacAddr, std::list<wireless_utils::sBssInfoConf>> bss_infos; // key=al_mac

which is used by the controller to construct M2 encrypted settings when an agent joins the controller by sending M1, whereas update credentials is used to trigger autoconfig renew for an existing agent[s]. So if you bring up the controller before you bring up the agents, update credentials is not needed. If you bring up a gateway only for example and want to change credentials, you need both since the local agent will already join the controller by the time you update its database using set credentials.

In what scenario we'll need to clear the credentials of a specific alid? Or clear the credentials at all?

As I said, whenever tear down is needed we will need to use the clear credentials API. It's just another feature in EasyMesh so we should provide some way to set it in our controller.

Calling bml_clear_wifi_credentials without any arguments to clear the entire network is the best idea... It'll happen all the time.

Why would it happen all the time? It will only happen if someone actively runs the command :)

vitalybu commented 4 years ago

So "set credentials" doesn't actually update the agents? Only stores the new credentials in the controller's DB?

Why would it happen all the time? It will only happen if someone actively runs the command :)

And that's exactly what will happen once the command will be added to the CLI :) Maybe bml_clear_wifi_credentials all is a slightly safer version. Same goes for the update command: bml_update_wifi_credentials all

arnout commented 4 years ago

Point by point feedback (which also responds to some of @vitalybu's comments):

The BML cli should support the following operations:
set credentials for alid

I think the usual case is that you set credentials globally. So I think the "for alid" bit should be optional. However, for our immediate need (UTAF testing) we don't really require it because we anyway know the alid.

update credentials

I thought this meant "modify a specific set of credentials". To avoid the confusion, maybe "renew" is clearer than "update"? That said, in the actual implementation it's pretty clear since it takes no arguments.

Set fronthaul 2.4G VAP

bml_set_wifi_credentials 11:22:33:44:55:66 multiap-24g maprocks1 8x

The 8x comes straight from the test plan, I don't think it's very convenient. Instead there should be an optional argument 24g/5g (if it's missing, it's for both).

Also, since the almac should be optional, I would move it last.

Clear credentials for alid 11:22:33:44:55:66

I wouldn't bother with that. I think we'll rarely work with specific almac.

Trigger autoconfig renew to all alids:

bml_update_wifi_credentials

:+1:

Trigger autoconfig renew to specific alid:

Again, not necessary IMO.

Proposed low-level API in son_master_thread
struct sWifiCredentials {
    std::string ssid;
    std::string network_key;
    int opclass;

IMO just band is enough.

int auth_type;
int enc_type;

IMO eWifiSec covers it nicely.

bool backhaul;

It can be fronthaul, backhaul, or both, so we need an enum here. eBssType fits the bill.


}

/**
 * @brief set wifi credentials for specific Alid

Please avoid alid, the correct term is AL-MAC address, or AL-MAC for short.

Sets wifi credentials for specific alid and sends autoconfig renew if requested.

@param alid alid of the agent to configure

@param credentials credentials vector, can be empty for teardown.

@return true if successful, false otherwise */ bool set_wifi_credentials(const sMacAddr &alid, std::vector &credentials);

Hm. When called from the command line, the vector will always have just a single element. When not called from the command line, we actually don't need an explicit clear and update - we'll typically just set all credentials (for all agents) with a single set_wifi_credentials. So maybe we want:

set_wifi_credentials(const sWifiCredentials &credentials, const sMacAddr &alid = network_utils::WILD_MAC): add a single credential
set_all_wifi_credentials(const std::vector<sWifiCredentials> &credentials): combination of clear, adding credentials, and sending update - but not possible to do this for a specific AL-MAC.

We can't use the second version on the command line, but it's still useful in the C++ interface.

If alids is empty, autoconfig renew for all known alids is sent.

I'm not a fan... If we want something like this, I think it would be better to keep track of which almacs are "dirty" and only renew those.

Test all new BML commands on actual RAX40 device with a NUC controller.

And in test_flows!

prplfoundation / prplMesh