Merge pull request #72 from adafruit/fix-tls-socket
Fix tls socket race condition
This commit is contained in:
commit
4336fbe17a
8 changed files with 112 additions and 32 deletions
2
.github/workflows/build.yml
vendored
2
.github/workflows/build.yml
vendored
|
|
@ -14,7 +14,7 @@ concurrency:
|
|||
cancel-in-progress: true
|
||||
|
||||
env:
|
||||
IDF_VERSION: v5.3.2
|
||||
IDF_VERSION: v5.5
|
||||
|
||||
jobs:
|
||||
build:
|
||||
|
|
|
|||
3
.gitmodules
vendored
3
.gitmodules
vendored
|
|
@ -1,9 +1,6 @@
|
|||
[submodule "certificates"]
|
||||
path = certificates
|
||||
url = https://github.com/adafruit/certificates
|
||||
[submodule "esp-idf"]
|
||||
path = esp-idf
|
||||
url = https://github.com/espressif/esp-idf
|
||||
[submodule "arduino-esp32"]
|
||||
path = components/arduino-esp32
|
||||
url = https://github.com/espressif/arduino-esp32
|
||||
|
|
|
|||
|
|
@ -10,12 +10,6 @@ else ()
|
|||
message(FATAL_ERROR "Unsupported BOARD: ${BOARD}. Supported boards are: fruitjam_c6, esp32")
|
||||
endif ()
|
||||
|
||||
# caused by esp-idf/components/bt/controller/esp32c6/bt.c:253:11: In function 'esp_bt_controller_log_init':
|
||||
# error: 'task_create' may be used uninitialized [-Werror=maybe-uninitialized]
|
||||
if (IDF_TARGET STREQUAL "esp32c6")
|
||||
add_compile_options(-Wno-maybe-uninitialized)
|
||||
endif ()
|
||||
|
||||
set(SDKCONFIG ${CMAKE_BINARY_DIR}/sdkconfig)
|
||||
set(SDKCONFIG_DEFAULTS sdkconfig.defaults ${CMAKE_CURRENT_LIST_DIR}/boards/${BOARD}/sdkconfig)
|
||||
if (CMAKE_BUILD_TYPE STREQUAL "Debug")
|
||||
|
|
@ -34,6 +28,9 @@ add_compile_definitions(
|
|||
)
|
||||
set(EXTRA_COMPONENT_DIRS ${CMAKE_CURRENT_LIST_DIR}/boards)
|
||||
|
||||
# required when upgrading to esp-idf v5.5 and arduino-esp32 v3.3.0
|
||||
add_link_options("-Wl,-u,__wrap_esp_log_writev")
|
||||
|
||||
project(nina-fw)
|
||||
|
||||
# Post build to run combine.py
|
||||
|
|
|
|||
|
|
@ -27,7 +27,7 @@ original Arduino firmware repository.
|
|||
|
||||
1. [Download the ESP32 toolchain](https://docs.espressif.com/projects/esp-idf/en/v3.3.1/get-started/index.html#setup-toolchain)
|
||||
1. Extract it and add it to your `PATH`: `export PATH=$PATH:<path/to/toolchain>/bin`
|
||||
1. Clone **v3.3.1** of the IDF: `git clone --branch v3.3.1 --recursive https://github.com/espressif/esp-idf.git`
|
||||
1. Clone **v5.5** of the IDF: `git clone --branch v5.5 --recursive https://github.com/espressif/esp-idf.git`
|
||||
1. Set the `IDF_PATH` environment variable: `export IDF_PATH=<path/to/idf>`
|
||||
1. `git submodule update --init` to fetch the `certificates` submodule.
|
||||
1. Run `make firmware` to build the firmware (in the directory of this read me)
|
||||
|
|
|
|||
|
|
@ -1 +1 @@
|
|||
Subproject commit 988dbe29731e2a2d09db2ed642c06271afa93705
|
||||
Subproject commit dbaf6a3226317a7c5e452e7b8a15e54c86bc2b6a
|
||||
1
esp-idf
1
esp-idf
|
|
@ -1 +0,0 @@
|
|||
Subproject commit 9d7f2d69f50d1288526d4f1027108e314e8c879f
|
||||
|
|
@ -63,8 +63,8 @@ int errno;
|
|||
|
||||
// Note: following version definition line is parsed by python script. Please don't change its format (space, indent) only update its version number.
|
||||
// ADAFRUIT-CHANGE: not fixed length
|
||||
// The version number obeys semver rules. We suffix with "+adafruit" to distinguish from Arduino NINA-FW.
|
||||
const char FIRMWARE_VERSION[] = "3.0.0";
|
||||
// The version number obeys semver rules. We suffix with "+adafruit" to distinguish from Arduino NINA-FW.
|
||||
const char FIRMWARE_VERSION[] = "3.1.0";
|
||||
|
||||
// ADAFRUIT-CHANGE: user-supplied cert and key
|
||||
// Optional, user-defined X.509 certificate
|
||||
|
|
@ -81,6 +81,7 @@ uint32_t resolvedHostname;
|
|||
#define MAX_SOCKETS CONFIG_LWIP_MAX_SOCKETS
|
||||
|
||||
uint8_t socketTypes[MAX_SOCKETS];
|
||||
SemaphoreHandle_t socketMutex[MAX_SOCKETS];
|
||||
NetworkClient tcpClients[MAX_SOCKETS];
|
||||
NetworkUDP udps[MAX_SOCKETS];
|
||||
NetworkServer tcpServers[MAX_SOCKETS];
|
||||
|
|
@ -668,6 +669,7 @@ int startServerTcp(const uint8_t command[], uint8_t response[])
|
|||
int getStateTcp(const uint8_t command[], uint8_t response[])
|
||||
{
|
||||
uint8_t socket = command[4];
|
||||
xSemaphoreTake(socketMutex[socket], portMAX_DELAY);
|
||||
|
||||
response[2] = 1; // number of parameters
|
||||
response[3] = 1; // parameter 1 length
|
||||
|
|
@ -678,6 +680,8 @@ int getStateTcp(const uint8_t command[], uint8_t response[])
|
|||
response[4] = 0;
|
||||
}
|
||||
|
||||
xSemaphoreGive(socketMutex[socket]);
|
||||
|
||||
return 6;
|
||||
}
|
||||
|
||||
|
|
@ -697,6 +701,8 @@ int availDataTcp(const uint8_t command[], uint8_t response[])
|
|||
uint8_t socket = command[4];
|
||||
uint16_t available = 0;
|
||||
|
||||
xSemaphoreTake(socketMutex[socket], portMAX_DELAY);
|
||||
|
||||
if (socketTypes[socket] == TCP_MODE) {
|
||||
if (tcpServers[socket]) {
|
||||
|
||||
|
|
@ -763,6 +769,8 @@ int availDataTcp(const uint8_t command[], uint8_t response[])
|
|||
|
||||
memcpy(&response[4], &available, sizeof(available));
|
||||
|
||||
xSemaphoreGive(socketMutex[socket]);
|
||||
|
||||
return 7;
|
||||
}
|
||||
|
||||
|
|
@ -771,6 +779,8 @@ int getDataTcp(const uint8_t command[], uint8_t response[])
|
|||
uint8_t socket = command[4];
|
||||
uint8_t peek = command[6];
|
||||
|
||||
xSemaphoreTake(socketMutex[socket], portMAX_DELAY);
|
||||
|
||||
response[2] = 1; // number of parameters
|
||||
response[3] = 1; // parameter 1 length
|
||||
|
||||
|
|
@ -794,6 +804,8 @@ int getDataTcp(const uint8_t command[], uint8_t response[])
|
|||
}
|
||||
}
|
||||
|
||||
xSemaphoreGive(socketMutex[socket]);
|
||||
|
||||
return 6;
|
||||
}
|
||||
|
||||
|
|
@ -823,6 +835,8 @@ int startClientTcp(const uint8_t command[], uint8_t response[])
|
|||
type = command[15 + command[3]];
|
||||
}
|
||||
|
||||
xSemaphoreTake(socketMutex[socket], portMAX_DELAY);
|
||||
|
||||
if (type == TCP_MODE) {
|
||||
int result;
|
||||
|
||||
|
|
@ -832,6 +846,8 @@ int startClientTcp(const uint8_t command[], uint8_t response[])
|
|||
result = tcpClients[socket].connect(ip, port);
|
||||
}
|
||||
|
||||
xSemaphoreGive(socketMutex[socket]);
|
||||
|
||||
if (result) {
|
||||
socketTypes[socket] = TCP_MODE;
|
||||
|
||||
|
|
@ -854,6 +870,8 @@ int startClientTcp(const uint8_t command[], uint8_t response[])
|
|||
result = udps[socket].beginPacket(ip, port);
|
||||
}
|
||||
|
||||
xSemaphoreGive(socketMutex[socket]);
|
||||
|
||||
if (result == 1) {
|
||||
socketTypes[socket] = UDP_MODE;
|
||||
|
||||
|
|
@ -884,6 +902,8 @@ int startClientTcp(const uint8_t command[], uint8_t response[])
|
|||
result = tlsClients[socket].connect(ip, port);
|
||||
}
|
||||
|
||||
xSemaphoreGive(socketMutex[socket]);
|
||||
|
||||
if (result) {
|
||||
socketTypes[socket] = TLS_MODE;
|
||||
|
||||
|
|
@ -907,6 +927,7 @@ int startClientTcp(const uint8_t command[], uint8_t response[])
|
|||
int stopClientTcp(const uint8_t command[], uint8_t response[])
|
||||
{
|
||||
uint8_t socket = command[4];
|
||||
xSemaphoreTake(socketMutex[socket], portMAX_DELAY);
|
||||
|
||||
if (socketTypes[socket] == TCP_MODE) {
|
||||
tcpClients[socket].stop();
|
||||
|
|
@ -917,6 +938,8 @@ int stopClientTcp(const uint8_t command[], uint8_t response[])
|
|||
}
|
||||
socketTypes[socket] = NO_MODE;
|
||||
|
||||
xSemaphoreGive(socketMutex[socket]);
|
||||
|
||||
response[2] = 1; // number of parameters
|
||||
response[3] = 1; // parameter 1 length
|
||||
response[4] = 1;
|
||||
|
|
@ -927,6 +950,7 @@ int stopClientTcp(const uint8_t command[], uint8_t response[])
|
|||
int getClientStateTcp(const uint8_t command[], uint8_t response[])
|
||||
{
|
||||
uint8_t socket = command[4];
|
||||
xSemaphoreTake(socketMutex[socket], portMAX_DELAY);
|
||||
|
||||
response[2] = 1; // number of parameters
|
||||
response[3] = 1; // parameter 1 length
|
||||
|
|
@ -936,10 +960,15 @@ int getClientStateTcp(const uint8_t command[], uint8_t response[])
|
|||
} else if ((socketTypes[socket] == TLS_MODE) && tlsClients[socket].connected()) {
|
||||
response[4] = 4;
|
||||
} else {
|
||||
socketTypes[socket] = NO_MODE;
|
||||
// ADAFRUIT: Original code reset socket type probably to clean up socket that was closed by remote host (once connected)
|
||||
// However if application tries to query socket state while TLS socket is in middle of handshaking (may take a while),
|
||||
// it can reset socket and cause esp32 report as closed/non-available connection.
|
||||
// socketTypes[socket] = NO_MODE;
|
||||
response[4] = 0;
|
||||
}
|
||||
|
||||
xSemaphoreGive(socketMutex[socket]);
|
||||
|
||||
return 6;
|
||||
}
|
||||
|
||||
|
|
@ -1032,6 +1061,7 @@ int getFwVersion(const uint8_t command[], uint8_t response[])
|
|||
int sendUDPdata(const uint8_t command[], uint8_t response[])
|
||||
{
|
||||
uint8_t socket = command[4];
|
||||
xSemaphoreTake(socketMutex[socket], portMAX_DELAY);
|
||||
|
||||
response[2] = 1; // number of parameters
|
||||
response[3] = 1; // parameter 1 length
|
||||
|
|
@ -1042,12 +1072,15 @@ int sendUDPdata(const uint8_t command[], uint8_t response[])
|
|||
response[4] = 0;
|
||||
}
|
||||
|
||||
xSemaphoreGive(socketMutex[socket]);
|
||||
|
||||
return 6;
|
||||
}
|
||||
|
||||
int getRemoteData(const uint8_t command[], uint8_t response[])
|
||||
{
|
||||
uint8_t socket = command[4];
|
||||
xSemaphoreTake(socketMutex[socket], portMAX_DELAY);
|
||||
|
||||
/*IPAddress*/uint32_t ip = /*IPAddress(0, 0, 0, 0)*/0;
|
||||
uint16_t port = 0;
|
||||
|
|
@ -1063,6 +1096,8 @@ int getRemoteData(const uint8_t command[], uint8_t response[])
|
|||
port = tlsClients[socket].remotePort();
|
||||
}
|
||||
|
||||
xSemaphoreGive(socketMutex[socket]);
|
||||
|
||||
response[2] = 2; // number of parameters
|
||||
|
||||
response[3] = 4; // parameter 1 length
|
||||
|
|
@ -2420,15 +2455,50 @@ const CommandHandlerType commandHandlers[] = {
|
|||
};
|
||||
#define NUM_COMMAND_HANDLERS (sizeof(commandHandlers) / sizeof(commandHandlers[0]))
|
||||
|
||||
#if defined(CMAKE_BUILD_TYPE_DEBUG) && 0
|
||||
const char* commandStrings[] = {
|
||||
// 0x00 -> 0x0f
|
||||
NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL,
|
||||
// 0x10 -> 0x1f
|
||||
"setNet", "setPassPhrase", "setKey", NULL, "setIPconfig", "setDNSconfig", "setHostname", "setPowerMode", "setApNet", "setApPassPhrase", "setDebug", "getTemperature", NULL, NULL, "getDNSconfig", "getReasonCode",
|
||||
// 0x20 -> 0x2f
|
||||
"getConnStatus", "getIPaddr", "getMACaddr", "getCurrSSID", "getCurrBSSID", "getCurrRSSI", "getCurrEnct", "scanNetworks", "startServerTcp", "getStateTcp", "dataSentTcp", "availDataTcp", "getDataTcp", "startClientTcp", "stopClientTcp", "getClientStateTcp",
|
||||
// 0x30 -> 0x3f
|
||||
"disconnect", NULL, "getIdxRSSI", "getIdxEnct", "reqHostByName", "getHostByName", "startScanNetworks", "getFwVersion", NULL, "sendUDPdata", "getRemoteData", "getTime", "getIdxBSSID", "getIdxChannel", "ping", "getSocket",
|
||||
// 0x40 -> 0x4f
|
||||
"setClientCert", "setCertKey", NULL, NULL, "sendDataTcp", "getDataBufTcp", "insertDataBuf", NULL, NULL, NULL, "wpa2EntSetIdentity", "wpa2EntSetUsername", "wpa2EntSetPassword", "wpa2EntSetCACert", "wpa2EntSetCertKey", "wpa2EntEnable",
|
||||
// 0x50 -> 0x5f
|
||||
"setPinMode", "setDigitalWrite", "setAnalogWrite", "getDigitalRead", "getAnalogRead", NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL,
|
||||
// 0x60 -> 0x6f
|
||||
"writeFile", "readFile", "deleteFile", "existsFile", "downloadFile", "applyOTA", "renameFile", "downloadOTA", NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL,
|
||||
// Low-level BSD-like sockets functions.
|
||||
// 0x70 -> 0x7f
|
||||
"socket_socket",
|
||||
"socket_close",
|
||||
"socket_errno",
|
||||
"socket_bind",
|
||||
"socket_listen",
|
||||
"socket_accept",
|
||||
"socket_connect",
|
||||
"socket_send",
|
||||
"socket_recv",
|
||||
"socket_sendto",
|
||||
"socket_recvfrom",
|
||||
"socket_ioctl",
|
||||
"socket_poll",
|
||||
"socket_setsockopt",
|
||||
"socket_getsockopt",
|
||||
"socket_getpeername"
|
||||
};
|
||||
#endif
|
||||
|
||||
CommandHandlerClass::CommandHandlerClass()
|
||||
{
|
||||
}
|
||||
|
||||
#if defined(CONFIG_IDF_TARGET_ESP32)
|
||||
static const int GPIO_IRQ = 0;
|
||||
#endif
|
||||
|
||||
#if defined(CONFIG_IDF_TARGET_ESP32C6)
|
||||
#elif defined(CONFIG_IDF_TARGET_ESP32C6)
|
||||
static const int GPIO_IRQ = 9;
|
||||
#endif
|
||||
|
||||
|
|
@ -2438,10 +2508,12 @@ void CommandHandlerClass::begin()
|
|||
|
||||
for (int i = 0; i < MAX_SOCKETS; i++) {
|
||||
socketTypes[i] = NO_MODE;
|
||||
socketMutex[i] = xSemaphoreCreateMutex();
|
||||
}
|
||||
|
||||
_updateGpio0PinSemaphore = xSemaphoreCreateCounting(2, 0);
|
||||
|
||||
// change priority to higher than loop task() to prevent socket connected() and available() race condition
|
||||
xTaskCreatePinnedToCore(CommandHandlerClass::gpio0Updater, "gpio0Updater", 8192, NULL, 1, NULL,
|
||||
CONFIG_FREERTOS_NUMBER_OF_CORES-1);
|
||||
}
|
||||
|
|
@ -2454,6 +2526,13 @@ int CommandHandlerClass::handle(const uint8_t command[], uint8_t response[])
|
|||
int responseLength = 0;
|
||||
|
||||
if (command[0] == START_CMD && command[1] < NUM_COMMAND_HANDLERS) {
|
||||
#if defined(CMAKE_BUILD_TYPE_DEBUG) && 0
|
||||
const char* cmdStr = (command[1] < sizeof(commandStrings) / sizeof(commandStrings[0])) ? commandStrings[command[1]] : NULL;
|
||||
if (cmdStr) {
|
||||
ets_printf("Command %u: %s\r\n", command[1], cmdStr);
|
||||
}
|
||||
#endif
|
||||
|
||||
CommandHandlerType commandHandlerType = commandHandlers[command[1]];
|
||||
|
||||
if (commandHandlerType) {
|
||||
|
|
@ -2489,38 +2568,44 @@ void CommandHandlerClass::updateGpio0Pin()
|
|||
{
|
||||
xSemaphoreTake(_updateGpio0PinSemaphore, portMAX_DELAY);
|
||||
|
||||
bool available = false;
|
||||
int available = 0;
|
||||
int i;
|
||||
for (i = 0; i < MAX_SOCKETS; i++) {
|
||||
if (pdFALSE == xSemaphoreTake(socketMutex[i], 0)) {
|
||||
continue; // skip if mutex is not available
|
||||
}
|
||||
|
||||
for (int i = 0; i < MAX_SOCKETS; i++) {
|
||||
if (socketTypes[i] == TCP_MODE) {
|
||||
if (tcpServers[i] && (tcpServers[i].hasClient() || tcpServers[i].accept())) {
|
||||
available = true;
|
||||
available = 1;
|
||||
break;
|
||||
} else if (tcpClients[i] && tcpClients[i].connected() && tcpClients[i].available()) {
|
||||
available = true;
|
||||
available = 1;
|
||||
break;
|
||||
}
|
||||
}
|
||||
|
||||
if (socketTypes[i] == UDP_MODE && (udps[i].available() || udps[i].parsePacket())) {
|
||||
available = true;
|
||||
available = 1;
|
||||
break;
|
||||
}
|
||||
|
||||
if (socketTypes[i] == TLS_MODE && tlsClients[i] && tlsClients[i].connected() && tlsClients[i].available()) {
|
||||
available = true;
|
||||
if (socketTypes[i] == TLS_MODE && tlsClients[i].connected() && tlsClients[i].available()) {
|
||||
available = 1;
|
||||
break;
|
||||
}
|
||||
|
||||
xSemaphoreGive(socketMutex[i]);
|
||||
}
|
||||
|
||||
if (available) {
|
||||
xSemaphoreGive(socketMutex[i]); // break early, not giving back semaphore yet
|
||||
digitalWrite(GPIO_IRQ, HIGH);
|
||||
} else {
|
||||
digitalWrite(GPIO_IRQ, LOW);
|
||||
}
|
||||
|
||||
vTaskDelay(1);
|
||||
// vTaskDelay(1);
|
||||
}
|
||||
|
||||
void CommandHandlerClass::onWiFiReceive()
|
||||
|
|
@ -2531,7 +2616,9 @@ void CommandHandlerClass::onWiFiReceive()
|
|||
void CommandHandlerClass::handleWiFiReceive()
|
||||
{
|
||||
if (xPortInIsrContext()) {
|
||||
xSemaphoreGiveFromISR(_updateGpio0PinSemaphore, NULL);
|
||||
BaseType_t xHigherPriorityTaskWoken = pdFALSE;
|
||||
xSemaphoreGiveFromISR(_updateGpio0PinSemaphore, &xHigherPriorityTaskWoken);
|
||||
portYIELD_FROM_ISR(xHigherPriorityTaskWoken);
|
||||
} else {
|
||||
xSemaphoreGive(_updateGpio0PinSemaphore);
|
||||
}
|
||||
|
|
|
|||
|
|
@ -21,9 +21,9 @@ CONFIG_LOG_DEFAULT_LEVEL_WARN=y
|
|||
CONFIG_LWIP_IRAM_OPTIMIZATION=y
|
||||
CONFIG_LWIP_MAX_SOCKETS=6
|
||||
CONFIG_LWIP_TCP_SYNMAXRTX=6
|
||||
CONFIG_LWIP_TCP_MSS=1436
|
||||
CONFIG_LWIP_TCP_SND_BUF_DEFAULT=5744
|
||||
CONFIG_LWIP_TCP_WND_DEFAULT=5744
|
||||
CONFIG_LWIP_TCPIP_TASK_STACK_SIZE=2560
|
||||
# CONFIG_LWIP_TCP_MSS=1436
|
||||
# CONFIG_LWIP_TCP_SND_BUF_DEFAULT=5744
|
||||
# CONFIG_LWIP_TCP_WND_DEFAULT=5744
|
||||
# CONFIG_LWIP_TCPIP_TASK_STACK_SIZE=4096
|
||||
CONFIG_MBEDTLS_PSK_MODES=y
|
||||
CONFIG_MBEDTLS_KEY_EXCHANGE_PSK=y
|
||||
|
|
|
|||
Loading…
Reference in a new issue