ARMmbed / mbed-os

Arm Mbed OS is a platform operating system designed for the internet of things
https://mbed.com
Other
4.66k stars 2.97k forks source link

UBLOX_EVK_ODIN_W2 - connect fails unless security is specified #3431

Closed JanneKiiskila closed 7 years ago

JanneKiiskila commented 7 years ago

Note: This is just a template, so feel free to use/remove the unnecessary things

Description


Bug

Target UBLOX_EVK_ODIN_W2

Toolchain: GCC_ARM|ARM|IAR

Toolchain version:

mbed-cli version: (mbed --version) 0.9.10

meed-os sha: (git log -n1 --oneline)

ce9d252 (HEAD -> mbed-os-5.3, origin/master, origin/HEAD) Merge pull request #3409 from jeromecoutant/PR_ST_L1_ASSERT

DAPLink version:

Expected behavior

WiFi should be able to connect with wifi.connect(ssid, passwd);

Actual behavior

It doesn't connect, unless you specify the security mode as well.

Steps to reproduce

Build the mbed-os-example-client and try that.


3 – seems ESP8266 and ODIN behave differently on the connect. The WiFi example has DIFFERENT connect parameters than mbed client.

Client has this.

if MBED_CONF_APP_NETWORK_INTERFACE == WIFI

connect_success = wifi.connect(MBED_CONF_APP_WIFI_SSID, MBED_CONF_APP_WIFI_PASSWORD);
network_interface = &wifi;

while, Wifi example has this:

output.printf("\r\nConnecting...\r\n");
 int ret = wifi.connect(MBED_CONF_APP_WIFI_SSID, MBED_CONF_APP_WIFI_PASSWORD, NSAPI_SECURITY_WPA_WPA2);

I think the WiFi driver should be able to negotiate the security mode by itself.

JanneKiiskila commented 7 years ago

@MarceloSalazar - I think this issue explains your OOB testing finding.

bulislaw commented 7 years ago

@andreaslarssonublox Could you have a look at that from the Odin side please.

bulislaw commented 7 years ago

It's not a bug, that's the way WiFi API is defined -> the security parameter defaults to no encryption, ESP is quite dummy (or smart depending on direction you look at it) and doesn't care about the encryption it'll try in any case. But hopefully we can make Odin behave similarly.

geky commented 7 years ago

This does point out that the esp8266 does not do what the default parameter implies. Currently WiFiInterface::connect defaults to NSAPI_SECURITY_NONE, but the esp8266 automatically determines the security type.

We could change the default parameter to something like NSAPI_SECURITY_AUTO, or we could just remove the default parameter to avoid the confusion. Thoughts?

JanneKiiskila commented 7 years ago

My vote is on the auto, that would make most sense. If we fix it to something, then apps will have try to do some "try this and if that fails, try that".. complicated, fussy and not developer friendly.

bulislaw commented 7 years ago

Let's wait for @andreaslarssonublox to hear if it's feasible to implement auto.

andreaslarssonublox commented 7 years ago

I think it's possible to use the auto security mode for Open, WEP and WPA/WPA2. The drawback is that we need to do a scan before to retrieve the security mode of the access point. Another drawback might be that the application is not aware of the chosen security mode if NSAPI_SECURITY_AUTO is used. It will also not work when we add enterprise security later on since it requires certificates and some other stuff. Most likely we will need another connect API for that or modify the existing one.

We can try to align the behavior of ODIN with ESP8266 and change the default to NSAPI_SECURITY_AUTO in the next release of the binary(after 5.3.0 I guess).

JanneKiiskila commented 7 years ago

Which one is faster - scan the network for it to tell what the security is, OR just try looping around the different security modes one by one and then return error if all of them fails? Enterprise is a different issue, agree on that.

tommikas commented 7 years ago

I've just been playing around with this. It seems that not only it doesn't connect, the connect call never returns. I'd expect it to timeout and return some error code.

This seems to happen if any of the parameters SSID, pass phrase, or security is wrong for the ap being connected to.

JanneKiiskila commented 7 years ago

Sounds like we would need a test case for something like that. That's definitely a bug.

bulislaw commented 7 years ago

Yes, we should add some 'negative' tests as well. I'll take an action to add some in the near future.

andreaslarssonublox commented 7 years ago

@tommikas The default behavior is that the driver will wait infinitely to connect to the AP. You should be able to set a finite timeout with set_timeout in OdinWiFiInterface.h.

JanneKiiskila commented 7 years ago

@andreaslarssonublox - we can't have a thinking in place that each network stack works the same way. The life of clients/apps gets way too complicated, if we cant' rely on the network stacks working the same way.

bulislaw commented 7 years ago

I'm not sure why we discuss two separate issues in one ticket. As to the timeout @andreaslarssonublox is correct, by default the timeout is set to 'wait forever' by the API. At the same time that shouldn't suppress wrong credentials errors in my opinion.

JanneKiiskila commented 7 years ago

Yes, exactly. The base class API clearly defines it should return an error.

https://docs.mbed.com/docs/mbed-os-api/en/mbed-os-5.3/api/classNetworkInterface.html#aa7ef54ecbd066f2083e6031bd1f3cb00

virtual nsapi_error_t NetworkInterface::connect ( )
pure virtual Start the interface

Returns 0 on success, negative error code on failure Implemented in WiFiInterface, EthernetInterface, and CellularInterface.

tommikas commented 7 years ago

At the same time that shouldn't suppress wrong credentials errors in my opinion.

Yep, that's the main thing that seems off to me. If the credentials are wrong the connection isn't going to happen no matter how long the timeout is.

by default the timeout is set to 'wait forever' by the API.

To me personally it'd make more sense to have some reasonable default timeout and give the option of setting a different (e.g. infinite) timeout if necessary, which would explicitly leave the choice of exactly how to handle the "no connection" situation to the developer, but that's a different matter.

bulislaw commented 7 years ago

@tommikas yes, that's something we definitely can do. @andreaslarssonublox I'm guessing you don't report errors back is because there's no clear way of figuring it out without timeout, what would be sane timeout value in this case? 10s?

andreaslarssonublox commented 7 years ago

@tommikas Yes, I agree that it would be better to have a default timeout of something like 10s instead of infinite. I'm not sure why it was done the way it is today but if all other network stacks have a finite timeout on the connect the ODIN WiFi should behave the same. We can modify it in the next release of the driver.

JanneKiiskila commented 7 years ago

This is fixed, therefore closing.