Skip to content

Commit

Permalink
Fix SSL issues (#2788)
Browse files Browse the repository at this point in the history
This PR aims to address some issues encountered whilst trying to use SSL for a basic `HttpClient` download session.

**Fix malloc_count link error 'undefined reference to __wrap_strdup`**

Implementation also needs to call mc_malloc, not malloc.

**AxCertificate destructor accesses ssl after it's been destroyed**

Picked up by valgrind.

**Provide time implementations in RTC.cpp, add test**

Library code requires libc implementations for `gettimeofday` and `time_t`.
On Esp8266 typically get `please start sntp first !` message.
This should be synced with `SystemClock` so removed the `time replacement` code from AXTLS and use that.

Test added to HostTests to ensure SystemClock and `time()` are synced. Checked on esp8266, rp2040, esp32s2, host.


**Replace automatic SSL certificate generation with `generate-cert` build target**

These don't need to be auto-generated as they're not always required.
There are also multiple ways to get this information into an application.
Several samples don't make use of these files, so removed.

NOTE: The `make_certs.sh` script no longer appears to work, at least with openssl 3.2.1 (Jan 2024).
The headers are generated but Axtls fails to load the certificate with -269 (SSL_ERROR_INVALID_KEY).

**Put generated SSL certificate information into PROGMEM**

Bit wasteful of RAM.

**Enforce consistent 'verifyLater' behaviour with Bearssl**

When attempting to fetch an https resource (using HttpClient) *without* setting request `onSslInit` we get this behaviour:

- Axtls: Fails with `X509_VFY_ERROR_NO_TRUSTED_CERT`
- Bearssl: No problem, goes right ahead.

This behaviour with Bearssl is not desirable as it could inadvertently compromise security.
Add a check on `verifyLater` and fail with `X509_NOT_TRUSTED` as appropriate.

**Notes**

- Add `setSslInitHandler` method to HttpClient?  NO ! `Use request->onSslInit`
- Does this work with lwip2 on esp8266? Doesn't appear to make things worse, but lwip2 looks to be kinda broken. Needs major update.
- Certificate generation throws errors with openssl 3, this needs addressing separately as it's only really appropriate for basic testing anyway.
  • Loading branch information
mikee47 authored Jun 7, 2024
1 parent 7e11439 commit 8a2dcc7
Show file tree
Hide file tree
Showing 43 changed files with 268 additions and 712 deletions.
6 changes: 6 additions & 0 deletions Sming/Arch/Esp32/Platform/RTC.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
****/

#include <Platform/RTC.h>
#include <sys/time.h>

// #include <ESP_VARIANT/clk.h>
extern "C" uint64_t esp_clk_rtc_time(void);
Expand Down Expand Up @@ -45,5 +46,10 @@ bool RtcClass::setRtcNanoseconds(uint64_t nanoseconds)

bool RtcClass::setRtcSeconds(uint32_t seconds)
{
struct timeval tv {
seconds
};
settimeofday(&tv, nullptr);

return setRtcNanoseconds(uint64_t(seconds) * NS_PER_SECOND);
}
23 changes: 22 additions & 1 deletion Sming/Arch/Esp8266/Platform/RTC.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@

#include <Platform/RTC.h>
#include <esp_systemapi.h>
#include <sys/time.h>

RtcClass RTC;

Expand Down Expand Up @@ -97,9 +98,29 @@ void loadTime(RtcData& data)

// Initialise the time struct
if(data.magic != RTC_MAGIC) {
debugf("rtc time init...");
debug_d("rtc time init...");
data.magic = RTC_MAGIC;
data.time = 0;
data.cycles = 0;
}
}

extern "C" int _gettimeofday_r(struct _reent*, struct timeval* tp, void*)
{
if(tp) {
// ensureBootTimeIsSet();
uint32_t micros = RTC.getRtcNanoseconds() / 1000LL;
tp->tv_sec = micros / 1000;
tp->tv_usec = micros % 1000;
}
return 0;
}

extern "C" time_t time(time_t* t)
{
time_t seconds = RTC.getRtcSeconds();
if(t) {
*t = seconds;
}
return seconds;
}
8 changes: 8 additions & 0 deletions Sming/Arch/Rp2040/Platform/RTC.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,9 @@
#include <Platform/RTC.h>
#include <DateTime.h>
#include <hardware/rtc.h>
#include <sys/time.h>

extern "C" int settimeofday(const struct timeval*, const struct timezone*);

RtcClass RTC;

Expand Down Expand Up @@ -52,6 +55,11 @@ bool RtcClass::setRtcNanoseconds(uint64_t nanoseconds)

bool RtcClass::setRtcSeconds(uint32_t seconds)
{
struct timeval tv {
seconds
};
settimeofday(&tv, nullptr);

DateTime dt{seconds};

datetime_t t = {
Expand Down
130 changes: 7 additions & 123 deletions Sming/Components/axtls-8266/axtls-8266.patch
Original file line number Diff line number Diff line change
Expand Up @@ -220,140 +220,24 @@ index 53509d0..25c568d 100644
#endif

diff --git a/replacements/time.c b/replacements/time.c
index 4972119..d39481c 100644
index 4972119..1447711 100644
--- a/replacements/time.c
+++ b/replacements/time.c
@@ -16,29 +16,25 @@
@@ -16,6 +16,8 @@
*
*/

+#define _C_TYPES_H_
+#include <c_types.h>
+#if 0
+
#include <time.h>
-#include <sntp.h>
+#include <lwip/sntp.h>
#include <sntp.h>

extern uint32_t system_get_time(void);
extern uint64_t system_mktime(uint32_t year, uint32_t mon, uint32_t day, uint32_t hour, uint32_t min, uint32_t sec);

static int errno_var = 0;

-int* __errno(void) {
+// These functions are implemented in Espressif SDK versions 2 and later (libmain.a) so we weaken them to avoid linker problems
+#define WEAK_ATTR __attribute__((weak))
+
+int* WEAK_ATTR __errno(void) {
// DEBUGV("__errno is called last error: %d (not current)\n", errno_var);
return &errno_var;
}

-unsigned long millis(void)
-{
- return system_get_time() / 1000UL;
-}
-
-unsigned long micros(void)
-{
- return system_get_time();
-}
-
#ifndef _TIMEVAL_DEFINED
#define _TIMEVAL_DEFINED
struct timeval {
@@ -60,12 +56,12 @@ static time_t s_bootTime = 0;
// calculate offset used in gettimeofday
static void ensureBootTimeIsSet()
{
- if (!s_bootTime)
+ if (s_bootTime == 0)
{
time_t now = sntp_get_current_timestamp();
- if (now)
+ if (now != 0)
{
- s_bootTime = now - millis() / 1000;
+ s_bootTime = now - (system_get_time() / 1000000);
}
}
}
@@ -79,7 +75,7 @@ static void setServer(int id, const char* name_or_ip)
}
}

-void configTime(int timezone, int daylightOffset_sec, const char* server1, const char* server2, const char* server3)
+void WEAK_ATTR configTime(int timezone, int daylightOffset_sec, const char* server1, const char* server2, const char* server3)
{
sntp_stop();

@@ -93,22 +89,16 @@ void configTime(int timezone, int daylightOffset_sec, const char* server1, const
sntp_init();
}

-int clock_gettime(clockid_t unused, struct timespec *tp)
+int WEAK_ATTR clock_gettime(clockid_t unused, struct timespec *tp)
{
- tp->tv_sec = millis() / 1000;
- tp->tv_nsec = micros() * 1000;
+ unsigned long us = system_get_time();
+ tp->tv_sec = us / 1000000UL;
+ us %= 1000000UL;
+ tp->tv_nsec = us * 1000;
return 0;
}

-// seconds since 1970
-time_t mktime(struct tm *t)
-{
- // system_mktime expects month in range 1..12
- #define START_MONTH 1
- return DIFF1900TO1970 + system_mktime(t->tm_year, t->tm_mon + START_MONTH, t->tm_mday, t->tm_hour, t->tm_min, t->tm_sec);
-}
-
-time_t time(time_t * t)
+time_t WEAK_ATTR time(time_t * t)
{
time_t seconds = sntp_get_current_timestamp();
if (t)
@@ -118,30 +108,32 @@ time_t time(time_t * t)
return seconds;
}

-char* asctime(const struct tm *t)
+char* WEAK_ATTR asctime(const struct tm *t)
{
return sntp_asctime(t);
}

-struct tm* localtime(const time_t *clock)
+struct tm* WEAK_ATTR localtime(const time_t *clock)
{
return sntp_localtime(clock);
}

-char* ctime(const time_t *t)
+char* WEAK_ATTR ctime(const time_t *t)
{
struct tm* p_tm = localtime(t);
char* result = asctime(p_tm);
return result;
}

-int gettimeofday(struct timeval *tp, void *tzp)
+int WEAK_ATTR gettimeofday(struct timeval *tp, void *tzp)
{
if (tp)
{
ensureBootTimeIsSet();
- tp->tv_sec = (s_bootTime + millis()) / 1000;
- tp->tv_usec = micros() * 1000;
+ unsigned long us = system_get_time();
+ tp->tv_sec = s_bootTime + (us / 1000000UL);
+ us %= 1000000UL;
+ tp->tv_usec = us * 1000UL;
@@ -145,3 +147,5 @@ int gettimeofday(struct timeval *tp, void *tzp)
}
return 0;
}
+
+#endif
diff --git a/ssl/asn1.c b/ssl/asn1.c
index a08a618..3c64064 100644
--- a/ssl/asn1.c
Expand Down
53 changes: 31 additions & 22 deletions Sming/Components/malloc_count/malloc_count.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -82,8 +82,17 @@ constexpr size_t alignment{16}; /* bytes (>= 2*sizeof(size_t)) */
/* a sentinel value prefixed to each allocation */
constexpr size_t sentinel{0xDEADC0DE};

/* Macro to get pointer to sentinel */
#define GET_SENTINEL(ptr) (size_t*)((char*)ptr - sizeof(size_t))
/* Add an offset to a pointer return that as a pointer cast to required type */
template <typename T = void*> T offsetPointer(void* ptr, intptr_t offset)
{
return reinterpret_cast<T>(reinterpret_cast<intptr_t>(ptr) + offset);
}

/* Get pointer to sentinel */
size_t* getSentinel(void* ptr)
{
return offsetPointer<size_t*>(ptr, -sizeof(size_t));
}

/* output */
#define PPREFIX "MC## "
Expand Down Expand Up @@ -224,9 +233,9 @@ extern "C" void* mc_malloc(size_t size)
}

/* prepend allocation size and check sentinel */
*(size_t*)ret = size;
ret = (char*)ret + alignment;
*GET_SENTINEL(ret) = sentinel;
*static_cast<size_t*>(ret) = size;
ret = offsetPointer(ret, alignment);
*getSentinel(ret) = sentinel;

inc_count(size);
if(size >= logThreshold) {
Expand All @@ -252,19 +261,19 @@ extern "C" void mc_free(void* ptr)
return;
}

size_t* p_sentinel = GET_SENTINEL(ptr);
size_t* p_sentinel = getSentinel(ptr);
if(*p_sentinel != sentinel) {
log("free(%p) has no sentinel !!! memory corruption?", ptr);
// ... or memory not allocated by our malloc()
} else {
*p_sentinel = 0; // Clear sentinel to avoid false-positives
ptr = (char*)ptr - alignment;
ptr = offsetPointer(ptr, -alignment);

size_t size = *(size_t*)ptr;
size_t size = *static_cast<size_t*>(ptr);
dec_count(size);

if(size >= logThreshold) {
log("free(%p) -> %u (cur %u)", (char*)ptr + alignment, size, stats.current);
log("free(%p) -> %u (cur %u)", offsetPointer(ptr, alignment), size, stats.current);
}
}

Expand All @@ -289,7 +298,7 @@ extern "C" void* mc_realloc(void* ptr, size_t size)
return mc_malloc(size);
}

if(*GET_SENTINEL(ptr) != sentinel) {
if(*getSentinel(ptr) != sentinel) {
log("free(%p) has no sentinel !!! memory corruption?", ptr);
// ... or memory not allocated by our malloc()
return REAL(F_REALLOC)(ptr, size);
Expand All @@ -301,9 +310,9 @@ extern "C" void* mc_realloc(void* ptr, size_t size)
return nullptr;
}

ptr = (char*)ptr - alignment;
ptr = offsetPointer(ptr, -alignment);

size_t oldsize = *(size_t*)ptr;
size_t oldsize = *static_cast<size_t*>(ptr);

void* newptr = REAL(F_REALLOC)(ptr, alignment + size);

Expand All @@ -317,16 +326,16 @@ extern "C" void* mc_realloc(void* ptr, size_t size)

if(size >= logThreshold) {
if(newptr == ptr) {
log("realloc(%u -> %u) = %p (cur %u)", oldsize, size, (char*)newptr + alignment, stats.current);
log("realloc(%u -> %u) = %p (cur %u)", oldsize, size, offsetPointer(newptr, alignment), stats.current);
} else {
log("realloc(%u -> %u) = %p -> %p (cur %u)", oldsize, size, (char*)ptr + alignment,
(char*)newptr + alignment, stats.current);
log("realloc(%u -> %u) = %p -> %p (cur %u)", oldsize, size, offsetPointer(ptr, alignment),
offsetPointer(newptr, alignment), stats.current);
}
}

*(size_t*)newptr = size;
*static_cast<size_t*>(newptr) = size;

return (char*)newptr + alignment;
return offsetPointer(newptr, alignment);
}

static __attribute__((destructor)) void finish()
Expand All @@ -349,6 +358,8 @@ extern "C" void* WRAP(calloc)(size_t, size_t) __attribute__((alias("mc_calloc"))
extern "C" void* WRAP(realloc)(void*, size_t) __attribute__((alias("mc_realloc")));
extern "C" void WRAP(free)(void*) __attribute__((alias("mc_free")));

using namespace MallocCount;

#ifdef ARCH_ESP8266

extern "C" void* WRAP(pvPortMalloc)(size_t) __attribute__((alias("mc_malloc")));
Expand All @@ -360,8 +371,6 @@ extern "C" void WRAP(vPortFree)(void*) __attribute__((alias("mc_free")));

#else

using namespace MallocCount;

void* operator new(size_t size)
{
return mc_malloc(size);
Expand Down Expand Up @@ -406,14 +415,14 @@ void operator delete[](void* ptr, size_t)

#endif

#endif // ESP8266

extern "C" char* WRAP(strdup)(const char* s)
{
auto len = strlen(s) + 1;
auto dup = (char*)malloc(len);
auto dup = static_cast<char*>(mc_malloc(len));
memcpy(dup, s, len);
return dup;
}

#endif

#endif // ENABLE_MALLOC_COUNT
6 changes: 3 additions & 3 deletions Sming/Components/ssl/Axtls/AxConnection.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -34,14 +34,14 @@ int AxConnection::write(const uint8_t* data, size_t length)

int available = tcp_sndbuf(tcp);
if(available < required) {
debug_i("SSL: Required: %d, Available: %d", required, available);
debug_i("[SSL] Required: %d, Available: %d", required, available);
return SSL_NOT_OK;
}

int written = ssl_write(ssl, data, length);
debug_d("SSL: Write len: %d, Written: %d", length, written);
debug_d("[SSL] Write len: %d, Written: %d", length, written);
if(written < 0) {
debug_e("SSL: Write Error: %d", written);
debug_e("[SSL] Write Error: %d", written);
}

return written;
Expand Down
1 change: 1 addition & 0 deletions Sming/Components/ssl/Axtls/AxConnection.h
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ class AxConnection : public Connection

~AxConnection()
{
certificate.reset();
// Typically sends out closing message
ssl_free(ssl);
}
Expand Down
Loading

0 comments on commit 8a2dcc7

Please sign in to comment.