tls vs (sntp vs sntp)

[nwf]

While attempting to debug #1707, I was getting mysterious "please start SNTP first" errors. This was confusing, because I am starting SNTP, via sntp.sync(), and even waiting for the all-clear from the callback. Imagine my surprise to discover that the "please start SNTP first" comes from an entirely duplicate implementation of SNTP inside LwIP which, through some closed-source that I cannot trace in libmain.a, is backing mbedtls's notion of the current time (via the time() function), rather than the RTC, yielding "certificate in the future" errors.

Expected behavior

sntp.sync() should set every part of the system's idea of the time.

Actual behavior

There are at least two confused brains inside this thing.

As a hackish workaround, I defined LUA_USE_MODULES_RTCTIME and redefined MBEDTLS_PLATFORM_TIME_MACRO to a function which knows how to get the time from the RTC and removed app/include/lwip/sntp.h, app/include/lwip/app/time.h, and app/lwip/core/sntp.c. This is a link error waiting to happen, but as it happens, isn't right now. ;)

This is a little tricky to get right, because in a sense it means that LUA_USE_MODULES_RTCTIME gates MBEDTLS_HAVE_TIME and such, but the right files aren't included in the right place to do that at the moment.

Possibly the better option is to eliminate app/modules/sntp.c in favor of the LwIP implementation, or to create a shim replacement for the LwIP version that uses the sntp module's internals. Note that the latter option will require a timer to tick every second and keep a software clock up to date (like the LwIP implementation) or the use of the RTC.

Alternatively, we could disable that MBEDTLS_HAVE_TIME and MBEDTLS_HAVE_TIME_DATE so that the TLS implementation does not try to check lifetimes.

[marcelstoer]

the "please start SNTP first" comes from an entirely duplicate implementation of SNTP inside LwIP

Yep, stumbled upon this the other day as well. Thanks for the analysis.

@pjsg and/or @jmattsson could you address this?

[pjsg]

I'm not available to look at this for about a month (due to travel).... I'm (sort of) reluctant to switch over to using the lwip flavor of sntp as I did a bunch of work to improve timekeeping and clock tuning to get a reasonably accurate clock (when not in deep sleep).

[nwf]

@pjsg I think my preference would be to keep the lua SNTP module as well. Could we edit libmain.a to remove everything that (transitively) depends on the lwip SNTP module, just so there's no risk of surprising link failure later?

I think I want it to be the case that LUA_USE_MODULES_RTCTIME implies and is implied by MBEDTLS_HAVE_TIME{,_DATE}. Does that seem reasonable?

[nwf]

It occurs to me to explicitly reference #1707 from this report, just to keep things tidy. Absent a fix to the mbedtls time source, we don't stand a chance at validating certificate lifetimes.

[marcelstoer]

@nwf I agree but that's still catch 22, isn't it? If we can't establish a TLS connection we don't stand a chance at validating its certificate lifetime 😜

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[nwf]

That's fair, stalebot. This is still a problem, tho', so far as I know.