Skip to content

Add WPA2 Enterprise APIs and example #75

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 5 commits into from
Closed

Conversation

facchinm
Copy link
Contributor

@facchinm facchinm commented Aug 8, 2019

Sibling PR: arduino/nina-fw#33

Since Filesystem APIs are used, it should be applied after merging #74

No more dependant on FileSystem APIs

// WPA2Enterprise data("myidentity", ca_pem);
// Certificates are stored in secret tab to avoid sharing them.

WPA2Enterprise data(EAP_TLS, "myidentity", "username", "password", ca_pem, client_crt, client_key);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@tigoe do you have any suggestions on the API for WPA2 enterprise support?

The way the underlying SDK on the NINA module works is it will accept all the configs (identity, username, password, CA, cert, key), then figure out what to use.

@facchinm has made a WPA2Enterprise class to store it, the full list constructor args is optional.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd make the default interface simple, like the macos login: begin(ssid, userid, password). Use defaults for the other properties. Then I'd add functions to set the rest of the properties.

Here's a typical wpa_supplicant record from linux:

ssid="network_name"
  key_mgmt=WPA-EAP
  eap=PEAP
  proto=WPA2
  phase2="auth=MSCHAPV2"
  anonymous_identity="your_username"
  identity="your_username"
  password=your_password

So if you want to be thorough:
.setKeyMgmt()
.setEAP()
.setProto()
.setPhase2()
.setAnonymousID()
.setID()
.setPassword()
That may be overkill, though, and it might be better just to have a version of .begin() that could take a wpa_supplicant-style record.
I think eduroam uses the same ID for identity and anonymous_identity, so you could say if the latter is not set, use the former for both. And PEAP and Mschapv2 seem to be common on wpa enterprise networks I've seen, so those might be good defaults. Double check those against eduroam though, because I think one of the big values of this is login for schools, and many universities worldwide use that.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi Tom, I'm setting up a test network to test how the ESP firmware performs in the various cases. My concert in that, if username (identity) + password is not covering the 99% of use cases, you'll need to add other information anyway, making the whole process split into 2 (or N) places.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fair enough. I would add the settings for eduroam in the examples then, since it's likely to be a popular use of this feature. Worth putting some notes in the documentation on how to find out these details as well, might want to pass that along to Simone.

//IPAddress server(74,125,232,128); // numeric IP for Google (no DNS)
char server[] = "www.google.com"; // name address for Google (using DNS)

// Initialize the Ethernet client library
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure what the best text would be, but certainly this comment and the two following lines are wrong. Perhaps:

// create a WiFiClient object

Copy link

@svihra svihra left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Build (and thus merge) fails due to usage of wrong method, please change all occurences of sendParamNoLen with sendBuffer and request new merge please.

Comment on lines +1178 to +1179
SpiDrv::sendParamNoLen((uint8_t*)client_crt, strlen(client_crt), NO_LAST_PARAM);
SpiDrv::sendParamNoLen((uint8_t*)client_key, strlen(client_key), LAST_PARAM);
Copy link

@svihra svihra Dec 5, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

replace SpiDrv::sendParamNoLen with SpiDrv::sendBuffer

// Send Command
SpiDrv::sendCmd(WPA2_ENTERPRISE_SET_CA_CERT, PARAM_NUMS_1);
SpiDrv::sendParamLen16(strlen(ca_pem));
SpiDrv::sendParamNoLen((uint8_t*)ca_pem, strlen(ca_pem), LAST_PARAM);
Copy link

@svihra svihra Dec 5, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

replace SpiDrv::sendParamNoLen with SpiDrv::sendBuffer

@sandeepmistry
Copy link
Contributor

Closing in favour of #97.

@per1234 per1234 added conclusion: duplicate Has already been submitted topic: code Related to content of the project itself type: enhancement Proposed improvement labels Mar 2, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
conclusion: duplicate Has already been submitted topic: code Related to content of the project itself type: enhancement Proposed improvement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants