Skip to content
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

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

Open
tomikaa87 opened this issue Dec 30, 2020 · 2 comments
Open

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

tomikaa87 opened this issue Dec 30, 2020 · 2 comments
Assignees
Labels
good first issue Good for newcomers

Comments

@tomikaa87
Copy link

tomikaa87 commented Dec 30, 2020

Blynk library version: master branch at 144a90f
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.

@tomikaa87 tomikaa87 changed the title Blynk::run() blocks if NTP server is unreachable Blynk.run() blocks if NTP server is unreachable Dec 30, 2020
@Phantomouse
Copy link

Phantomouse commented Feb 25, 2021

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
Copy link
Author

tomikaa87 commented Feb 25, 2021

@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.

@vshymanskyy vshymanskyy added the good first issue Good for newcomers label Mar 30, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers
Projects
None yet
Development

No branches or pull requests

3 participants