Crash_Override/OVMS3-idf
1
0
Fork 0
Code Issues Pull requests Projects Releases Wiki Activity

protocomm security : memory leaks fixed

Browse source 
List of changes:
* Corner case exceptions are properly handled to ensure release of memory occupied by security infrastructure
* fixed erroneous cleanup of security instance by protocomm_console
This commit is contained in:
Anurag Kar 2019-02-08 14:42:49 +05:30 committed by bot
parent 093c7b176d
commit 85e8dea26a
4 changed files with 127 additions and 75 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

44
components/protocomm/src/common/protocomm.c
View file

@ -52,6 +52,19 @@ void protocomm_delete(protocomm_t *pc)
free(it);
}
/* Free memory allocated to version string */
if (pc->ver) {
free((void *)pc->ver);
}
/* Free memory allocated to security */
if (pc->sec && pc->sec->cleanup) {
pc->sec->cleanup();
}
if (pc->pop) {
free(pc->pop);
}
free(pc);
}
@ -140,11 +153,14 @@ esp_err_t protocomm_req_handle(protocomm_t *pc, const char *ep_name, uint32_t se
const uint8_t *inbuf, ssize_t inlen,
uint8_t **outbuf, ssize_t *outlen)
{
if ((pc == NULL) || (ep_name == NULL)) {
if (!pc || !ep_name || !outbuf || !outlen) {
ESP_LOGE(TAG, "Invalid params %p %p", pc, ep_name);
return ESP_ERR_INVALID_ARG;
}
*outbuf = NULL;
*outlen = 0;
protocomm_ep_t *ep = search_endpoint(pc, ep_name);
if (!ep) {
ESP_LOGE(TAG, "No registered endpoint for %s", ep_name);
@ -166,19 +182,23 @@ esp_err_t protocomm_req_handle(protocomm_t *pc, const char *ep_name, uint32_t se
}
ssize_t dec_inbuf_len = inlen;
pc->sec->decrypt(session_id, inbuf, inlen, dec_inbuf, &dec_inbuf_len);
ret = pc->sec->decrypt(session_id, inbuf, inlen, dec_inbuf, &dec_inbuf_len);
if (ret != ESP_OK) {
ESP_LOGE(TAG, "Decryption of response failed for endpoint %s", ep_name);
free(dec_inbuf);
return ret;
}
/* Invoke the request handler */
uint8_t *plaintext_resp;
ssize_t plaintext_resp_len;
uint8_t *plaintext_resp = NULL;
ssize_t plaintext_resp_len = 0;
ret = ep->req_handler(session_id,
dec_inbuf, dec_inbuf_len,
&plaintext_resp, &plaintext_resp_len,
ep->priv_data);
if (ret != ESP_OK) {
ESP_LOGE(TAG, "Request handler for %s failed", ep_name);
*outbuf = NULL;
*outlen = 0;
free(plaintext_resp);
free(dec_inbuf);
return ret;
}
@ -189,12 +209,20 @@ esp_err_t protocomm_req_handle(protocomm_t *pc, const char *ep_name, uint32_t se
uint8_t *enc_resp = (uint8_t *) malloc(plaintext_resp_len);
if (!enc_resp) {
ESP_LOGE(TAG, "Failed to allocate decrypt buf len %d", inlen);
free(plaintext_resp);
return ESP_ERR_NO_MEM;
}
ssize_t enc_resp_len = plaintext_resp_len;
pc->sec->encrypt(session_id, plaintext_resp, plaintext_resp_len,
enc_resp, &enc_resp_len);
ret = pc->sec->encrypt(session_id, plaintext_resp, plaintext_resp_len,
enc_resp, &enc_resp_len);
if (ret != ESP_OK) {
ESP_LOGE(TAG, "Encryption of response failed for endpoint %s", ep_name);
free(enc_resp);
free(plaintext_resp);
return ret;
}
/* We no more need plaintext response */
free(plaintext_resp);

7
components/protocomm/src/security/security0.c
View file

@ -36,6 +36,8 @@ static esp_err_t sec0_session_setup(uint32_t session_id,
S0SessionResp *s0resp = (S0SessionResp *) malloc(sizeof(S0SessionResp));
if (!out || !s0resp) {
ESP_LOGE(TAG, "Error allocating response");
free(out);
free(s0resp);
return ESP_ERR_NO_MEM;
}
sec0_payload__init(out);
@ -79,6 +81,7 @@ static esp_err_t sec0_req_handler(const protocomm_security_pop_t *pop, uint32_t
}
if (req->sec_ver != protocomm_security0.ver) {
ESP_LOGE(TAG, "Security version mismatch. Closing connection");
session_data__free_unpacked(req, NULL);
return ESP_ERR_INVALID_ARG;
}
@ -86,12 +89,12 @@ static esp_err_t sec0_req_handler(const protocomm_security_pop_t *pop, uint32_t
ret = sec0_session_setup(session_id, req, &resp, pop);
if (ret != ESP_OK) {
ESP_LOGE(TAG, "Session setup error %d", ret);
session_data__free_unpacked(req, NULL);
return ESP_FAIL;
}
session_data__free_unpacked(req, NULL);
resp.sec_ver = req->sec_ver;
session_data__free_unpacked(req, NULL);
*outlen = session_data__get_packed_size(&resp);
*outbuf = (uint8_t *) malloc(*outlen);

147
components/protocomm/src/security/security1.c
View file

@ -38,8 +38,9 @@ static const char* TAG = "security1";
#define PUBLIC_KEY_LEN 32
#define SZ_RANDOM 16
#define SESSION_STATE_1 1 /* Session in state 1 */
#define SESSION_STATE_SETUP 2 /* Session setup successful */
#define SESSION_STATE_CMD0 0 /* Session is not setup */
#define SESSION_STATE_CMD1 1 /* Session is not setup */
#define SESSION_STATE_DONE 2 /* Session setup successful */
typedef struct session {
/* Session data */
@ -82,22 +83,12 @@ static esp_err_t handle_session_command1(uint32_t session_id,
uint8_t check_buf[PUBLIC_KEY_LEN];
int mbed_err;
if (!cur_session) {
ESP_LOGE(TAG, "Data on session endpoint without session establishment");
if (cur_session->state != SESSION_STATE_CMD1) {
ESP_LOGE(TAG, "Invalid state of session %d (expected %d)", SESSION_STATE_CMD1, cur_session->state);
return ESP_ERR_INVALID_STATE;
}
if (session_id != cur_session->id) {
ESP_LOGE(TAG, "Invalid session");
return ESP_ERR_INVALID_STATE;
}
if (!in) {
ESP_LOGE(TAG, "Empty session data");
return ESP_ERR_INVALID_ARG;
}
/* Initialise crypto context */
/* Initialize crypto context */
mbedtls_aes_init(&cur_session->ctx_aes);
memset(cur_session->stb, 0, sizeof(cur_session->stb));
cur_session->nc_off = 0;
@ -109,6 +100,7 @@ static esp_err_t handle_session_command1(uint32_t session_id,
sizeof(cur_session->sym_key)*8);
if (mbed_err != 0) {
ESP_LOGE(TAG, "Failure at mbedtls_aes_setkey_enc with error code : -0x%x", -mbed_err);
mbedtls_aes_free(&cur_session->ctx_aes);
return ESP_FAIL;
}
@ -118,6 +110,7 @@ static esp_err_t handle_session_command1(uint32_t session_id,
in->sc1->client_verify_data.data, check_buf);
if (mbed_err != 0) {
ESP_LOGE(TAG, "Failure at mbedtls_aes_crypt_ctr with error code : -0x%x", -mbed_err);
mbedtls_aes_free(&cur_session->ctx_aes);
return ESP_FAIL;
}
@ -127,19 +120,30 @@ static esp_err_t handle_session_command1(uint32_t session_id,
if (mbedtls_ssl_safer_memcmp(check_buf, cur_session->device_pubkey,
sizeof(cur_session->device_pubkey)) != 0) {
ESP_LOGE(TAG, "Key mismatch. Close connection");
mbedtls_aes_free(&cur_session->ctx_aes);
return ESP_FAIL;
}
Sec1Payload *out = (Sec1Payload *) malloc(sizeof(Sec1Payload));
sec1_payload__init(out);
SessionResp1 *out_resp = (SessionResp1 *) malloc(sizeof(SessionResp1));
session_resp1__init(out_resp);
if (!out || !out_resp) {
ESP_LOGE(TAG, "Error allocating memory for response1");
free(out);
free(out_resp);
mbedtls_aes_free(&cur_session->ctx_aes);
return ESP_ERR_NO_MEM;
}
sec1_payload__init(out);
session_resp1__init(out_resp);
out_resp->status = STATUS__Success;
uint8_t *outbuf = (uint8_t *) malloc(PUBLIC_KEY_LEN);
if (!outbuf) {
ESP_LOGE(TAG, "Error allocating ciphertext buffer");
free(out);
free(out_resp);
mbedtls_aes_free(&cur_session->ctx_aes);
return ESP_ERR_NO_MEM;
}
@ -149,6 +153,10 @@ static esp_err_t handle_session_command1(uint32_t session_id,
cur_session->client_pubkey, outbuf);
if (mbed_err != 0) {
ESP_LOGE(TAG, "Failure at mbedtls_aes_crypt_ctr with error code : -0x%x", -mbed_err);
free(outbuf);
free(out);
free(out_resp);
mbedtls_aes_free(&cur_session->ctx_aes);
return ESP_FAIL;
}
@ -163,8 +171,8 @@ static esp_err_t handle_session_command1(uint32_t session_id,
resp->proto_case = SESSION_DATA__PROTO_SEC1;
resp->sec1 = out;
ESP_LOGD(TAG, "Session successful");
cur_session->state = SESSION_STATE_DONE;
ESP_LOGD(TAG, "Secure session established successfully");
return ESP_OK;
}
@ -172,32 +180,21 @@ static esp_err_t handle_session_command0(uint32_t session_id,
SessionData *req, SessionData *resp,
const protocomm_security_pop_t *pop)
{
ESP_LOGD(TAG, "Request to handle setup0_command");
Sec1Payload *in = (Sec1Payload *) req->sec1;
esp_err_t ret;
int mbed_err;
if (!cur_session) {
ESP_LOGE(TAG, "Data on session endpoint without session establishment");
if (cur_session->state != SESSION_STATE_CMD0) {
ESP_LOGE(TAG, "Invalid state of session %d (expected %d)", SESSION_STATE_CMD0, cur_session->state);
return ESP_ERR_INVALID_STATE;
}
if (session_id != cur_session->id) {
ESP_LOGE(TAG, "Invalid session");
return ESP_ERR_INVALID_STATE;
}
if (!in) {
ESP_LOGE(TAG, "Empty session data");
return ESP_ERR_INVALID_ARG;
}
if (in->sc0->client_pubkey.len != PUBLIC_KEY_LEN) {
ESP_LOGE(TAG, "Invalid public key length");
return ESP_ERR_INVALID_ARG;
}
cur_session->state = SESSION_STATE_1;
mbedtls_ecdh_context *ctx_server = malloc(sizeof(mbedtls_ecdh_context));
mbedtls_entropy_context *entropy = malloc(sizeof(mbedtls_entropy_context));
mbedtls_ctr_drbg_context *ctr_drbg = malloc(sizeof(mbedtls_ctr_drbg_context));
@ -315,8 +312,10 @@ static esp_err_t handle_session_command0(uint32_t session_id,
Sec1Payload *out = (Sec1Payload *) malloc(sizeof(Sec1Payload));
SessionResp0 *out_resp = (SessionResp0 *) malloc(sizeof(SessionResp0));
if (!out || !out_resp) {
ESP_LOGE(TAG, "Error allocating memory for response");
ret = ESP_FAIL;
ESP_LOGE(TAG, "Error allocating memory for response0");
ret = ESP_ERR_NO_MEM;
free(out);
free(out_resp);
goto exit_cmd0;
}
@ -338,6 +337,8 @@ static esp_err_t handle_session_command0(uint32_t session_id,
resp->proto_case = SESSION_DATA__PROTO_SEC1;
resp->sec1 = out;
cur_session->state = SESSION_STATE_CMD1;
ESP_LOGD(TAG, "Session setup phase1 done");
ret = ESP_OK;
@ -361,6 +362,21 @@ static esp_err_t sec1_session_setup(uint32_t session_id,
Sec1Payload *in = (Sec1Payload *) req->sec1;
esp_err_t ret;
if (!cur_session) {
ESP_LOGE(TAG, "Invalid session context data");
return ESP_ERR_INVALID_ARG;
}
if (session_id != cur_session->id) {
ESP_LOGE(TAG, "Invalid session ID : %d (expected %d)", session_id, cur_session->id);
return ESP_ERR_INVALID_STATE;
}
if (!in) {
ESP_LOGE(TAG, "Empty session data");
return ESP_ERR_INVALID_ARG;
}
switch (in->msg) {
case SEC1_MSG_TYPE__Session_Command0:
ret = handle_session_command0(session_id, req, resp, pop);
@ -411,30 +427,30 @@ static void sec1_session_setup_cleanup(uint32_t session_id, SessionData *resp)
return;
}
static esp_err_t sec1_init()
static esp_err_t sec1_close_session(uint32_t session_id)
{
return ESP_OK;
}
static esp_err_t sec1_cleanup()
{
if (cur_session) {
ESP_LOGD(TAG, "Closing current session with id %u", cur_session->id);
mbedtls_aes_free(&cur_session->ctx_aes);
bzero(cur_session, sizeof(session_t));
free(cur_session);
cur_session = NULL;
if (!cur_session || cur_session->id != session_id) {
ESP_LOGE(TAG, "Attempt to close invalid session");
return ESP_ERR_INVALID_ARG;
}
if (cur_session->state == SESSION_STATE_DONE) {
/* Free AES context data */
mbedtls_aes_free(&cur_session->ctx_aes);
}
bzero(cur_session, sizeof(session_t));
free(cur_session);
cur_session = NULL;
return ESP_OK;
}
static esp_err_t sec1_new_session(uint32_t session_id)
{
if (cur_session && cur_session->id != session_id) {
if (cur_session) {
/* Only one session is allowed at a time */
ESP_LOGE(TAG, "Closing old session with id %u", cur_session->id);
sec1_cleanup();
} else if (cur_session && cur_session->id == session_id) {
return ESP_OK;
sec1_close_session(cur_session->id);
}
cur_session = (session_t *) calloc(1, sizeof(session_t));
@ -447,16 +463,17 @@ static esp_err_t sec1_new_session(uint32_t session_id)
return ESP_OK;
}
static esp_err_t sec1_close_session(uint32_t session_id)
static esp_err_t sec1_init()
{
if (!cur_session || cur_session->id != session_id) {
ESP_LOGE(TAG, "Attempt to close invalid session");
return ESP_ERR_INVALID_ARG;
}
return ESP_OK;
}
bzero(cur_session, sizeof(session_t));
free(cur_session);
cur_session = NULL;
static esp_err_t sec1_cleanup()
{
if (cur_session) {
ESP_LOGD(TAG, "Closing current session with id %u", cur_session->id);
sec1_close_session(cur_session->id);
}
return ESP_OK;
}
@ -469,8 +486,15 @@ static esp_err_t sec1_decrypt(uint32_t session_id,
}
if (!cur_session || cur_session->id != session_id) {
ESP_LOGE(TAG, "Session with ID %d not found", session_id);
return ESP_ERR_INVALID_STATE;
}
if (cur_session->state != SESSION_STATE_DONE) {
ESP_LOGE(TAG, "Secure session not established");
return ESP_ERR_INVALID_STATE;
}
*outlen = inlen;
int ret = mbedtls_aes_crypt_ctr(&cur_session->ctx_aes, inlen, &cur_session->nc_off,
cur_session->rand, cur_session->stb, inbuf, outbuf);
@ -497,6 +521,7 @@ static esp_err_t sec1_req_handler(const protocomm_security_pop_t *pop, uint32_t
}
if (req->sec_ver != protocomm_security1.ver) {
ESP_LOGE(TAG, "Security version mismatch. Closing connection");
session_data__free_unpacked(req, NULL);
return ESP_ERR_INVALID_ARG;
}
@ -504,12 +529,12 @@ static esp_err_t sec1_req_handler(const protocomm_security_pop_t *pop, uint32_t
ret = sec1_session_setup(session_id, req, &resp, pop);
if (ret != ESP_OK) {
ESP_LOGE(TAG, "Session setup error %d", ret);
session_data__free_unpacked(req, NULL);
return ESP_FAIL;
}
session_data__free_unpacked(req, NULL);
resp.sec_ver = req->sec_ver;
session_data__free_unpacked(req, NULL);
*outlen = session_data__get_packed_size(&resp);
*outbuf = (uint8_t *) malloc(*outlen);

4
components/protocomm/src/transports/protocomm_console.c
View file

@ -121,10 +121,6 @@ static void protocomm_console_task(void *arg)
}
}
if (pc_console->sec && pc_console->sec->cleanup) {
pc_console->sec->cleanup();
}
pc_console = NULL;
esp_console_deinit();