blynkkk / blynk-library

Blynk library for IoT boards. Works with Arduino, ESP32, ESP8266, Raspberry Pi, Particle, ARM Mbed, etc.
https://blynk.io
MIT License
3.81k stars 1.38k forks source link

Blynk.run() blocks if NTP server is unreachable #509

Open tomikaa87 opened 3 years ago

tomikaa87 commented 3 years ago

Blynk library version: master branch at 144a90f3bdb66c3b6c03cce0335c1f1e304d91d6 IDE: VS Code + PlatformIO IDE version: 1.52.1 Board type: ESP8266 Additional modules: Arduino ESP8266

Scenario, steps to reproduce

Blynk.run() blocks if there is no internet connection on the network and connecting to a local server via SSL. According to my investigation, BlynkArduinoClientSecure::connect() enters into an infinite loop if it can't obtain the current time via NTP:

        if (now < 100000) {
            // Synchronize time useing SNTP. This is necessary to verify that
            // the TLS certificates offered by the server are currently valid.
            configTime(0, 0, "pool.ntp.org", "time.nist.gov");

            while (now < 100000) {
                delay(100);
                now = time(nullptr);
            }
        }

If there is no answer for the NTP request, the inner while loop doesn't finish. Since Blynk is not the only application that must be executed in the Arduino loop(), this bug renders my device unusable.

Place of call in my application: https://github.com/tomikaa87/esp-iot-base/blob/bc40f1b2310e56407f1013bfcc85dd5c5c7118cb/src/CoreApplication.cpp#L163 (BlynkHandler::task() calls Blynk.run() directly)

Expected Result

Blynk.run() doesn't block if the Internet is not reachable, leaving the SSL socket disconnected until the time is properly set and the verification can be done.

Actual Result

Blynk.run() doesn't return if the NTP server baked into BlynkArduinoClientSecure is not reachable.

Phantomouse commented 3 years ago

There is no point in using TLS if you wouldn't check current validity of certificate. I think that would be wrong to add any bypass hidden from end user to BlynkArduinoClientSecure::connect() because we'll get the potential security breach. It may be right to make possibility to register your own local SNTP server address for case if you encountering the internet connection issues.

Instantiate your own SNTP server in your local net and then just call configTime(0, 0, "YOUR_SERVER_ADDRESS", "time.nist.gov"); before the first call of Blynk.run()

tomikaa87 commented 3 years ago

@Phantomouse I'm not talking about skipping the certificate check. What I meant that if there is no way to verify the certificate, run() must not block the thread, but return and leave Blynk disconnected and next time run() is called again it can check if all the necessary conditions are met to do the verification. IMHO most of the applications can happily do something else until the connection is restored and Blynk can connect again. Until this issue is fixed, one have to write something like this in the code:

if (time(nullptr) > 100000) {
    Blynk.run();
}

This is totally unnecessary and since it's not documented, it's a pretty bad surprise when the application just stops out of the blue without any logical reason.

Edit: probably it was not too obvious what I meant in the original post so I've clarified the Expected Result.