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.

Sign up for GitHub

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

lib: updatehub: Improve security #24154

Merged
merged 3 commits into from
Apr 17, 2020
Merged

lib: updatehub: Improve security #24154

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
361380e
lib: updatehub: Fix variable-size string copy
nandojve Apr 15, 2020
6625818
lib: updatehub: Refact to use bin2hex
nandojve Apr 5, 2020
bc1493a
lib: updatehub: Improve probe security
nandojve Apr 7, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions lib/updatehub/shell.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,11 +55,11 @@ static int cmd_info(const struct shell *shell, size_t argc, char **argv)
ARG_UNUSED(argc);
ARG_UNUSED(argv);

char *device_id = k_malloc(DEVICE_ID_MAX_SIZE);
char *device_id = k_malloc(DEVICE_ID_HEX_MAX_SIZE);
char *firmware_version = k_malloc(BOOT_IMG_VER_STRLEN_MAX);

updatehub_get_device_identity(device_id, DEVICE_ID_HEX_MAX_SIZE);
updatehub_get_firmware_version(firmware_version, BOOT_IMG_VER_STRLEN_MAX);
updatehub_get_device_identity(device_id, DEVICE_ID_MAX_SIZE);

shell_fprintf(shell, SHELL_NORMAL, "Unique device id: %s\n",
device_id);
Expand Down
131 changes: 88 additions & 43 deletions lib/updatehub/updatehub.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,8 @@ LOG_MODULE_REGISTER(updatehub);
#define COAP_MAX_RETRY 3
#define MAX_IP_SIZE 30

#define SHA256_HEX_DIGEST_SIZE ((TC_SHA256_DIGEST_SIZE * 2) + 1)

#if defined(CONFIG_UPDATEHUB_CE)
#define UPDATEHUB_SERVER CONFIG_UPDATEHUB_SERVER
#else
Expand All @@ -66,13 +68,31 @@ static struct updatehub_context {
} ctx;

static struct update_info {
char package_uid[TC_SHA256_BLOCK_SIZE + 1];
char sha256sum_image[TC_SHA256_BLOCK_SIZE + 1];
char package_uid[SHA256_HEX_DIGEST_SIZE];
char sha256sum_image[SHA256_HEX_DIGEST_SIZE];
int image_size;
} update_info;

static struct k_delayed_work updatehub_work_handle;

static int bin2hex_str(u8_t *bin, size_t bin_len, char *str, size_t str_buf_len)
{
if (bin == NULL || str == NULL) {
return -1;
}

/* ensures at least an empty string */
if (str_buf_len < 1) {
return -2;
}

memset(str, 0, str_buf_len);
/* str_buf_len - 1 ensure space for \0 */
bin2hex(bin, bin_len, str, str_buf_len - 1);

return 0;
}

static void wait_fds(void)
{
if (poll(ctx.fds, ctx.nfds, NETWORK_TIMEOUT) < 0) {
Expand All @@ -91,8 +111,6 @@ static int metadata_hash_get(char *metadata)
{
struct tc_sha256_state_struct sha256sum;
unsigned char hash[TC_SHA256_DIGEST_SIZE];
char buffer[3];
int buffer_len = 0;

if (tc_sha256_init(&sha256sum) == 0) {
return -1;
Expand All @@ -106,13 +124,9 @@ static int metadata_hash_get(char *metadata)
return -1;
}

memset(update_info.package_uid, 0, TC_SHA256_BLOCK_SIZE + 1);
for (int i = 0; i < TC_SHA256_DIGEST_SIZE; i++) {
snprintk(buffer, sizeof(buffer), "%02x",
hash[i]);
buffer_len = buffer_len + strlen(buffer);
strncat(&update_info.package_uid[i], buffer,
MIN(TC_SHA256_BLOCK_SIZE, buffer_len));
if (bin2hex_str(hash, TC_SHA256_DIGEST_SIZE,
update_info.package_uid, SHA256_HEX_DIGEST_SIZE)) {
return -1;
}

return 0;
Expand Down Expand Up @@ -324,26 +338,22 @@ static int send_request(enum coap_msgtype msgtype, enum coap_method method,

static bool install_update_cb_sha256(void)
{
u8_t image_hash[TC_SHA256_DIGEST_SIZE];
char buffer[3], sha256_image_dowloaded[TC_SHA256_BLOCK_SIZE + 1];
int i, buffer_len = 0;
u8_t hash[TC_SHA256_DIGEST_SIZE];
char sha256[SHA256_HEX_DIGEST_SIZE];

if (tc_sha256_final(image_hash, &ctx.sha256sum) < 1) {
if (tc_sha256_final(hash, &ctx.sha256sum) < 1) {
LOG_ERR("Could not finish sha256sum");
return false;
}

memset(&sha256_image_dowloaded, 0, TC_SHA256_BLOCK_SIZE + 1);
for (i = 0; i < TC_SHA256_DIGEST_SIZE; i++) {
snprintk(buffer, sizeof(buffer), "%02x", image_hash[i]);
buffer_len = buffer_len + strlen(buffer);
strncat(&sha256_image_dowloaded[i], buffer,
MIN(TC_SHA256_BLOCK_SIZE, buffer_len));
if (bin2hex_str(hash, TC_SHA256_DIGEST_SIZE,
sha256, SHA256_HEX_DIGEST_SIZE)) {
LOG_ERR("Could not create sha256sum hex representation");
return false;
}

if (strncmp(sha256_image_dowloaded,
update_info.sha256sum_image,
strlen(update_info.sha256sum_image)) != 0) {
if (strncmp(sha256, update_info.sha256sum_image,
SHA256_HEX_DIGEST_SIZE) != 0) {
LOG_ERR("SHA256SUM of image are not the same");
ctx.code_status = UPDATEHUB_DOWNLOAD_ERROR;
return false;
Expand Down Expand Up @@ -497,15 +507,15 @@ static int report(enum updatehub_state state)
struct report report;
int ret = -1;
const char *exec = state_name(state);
char *device_id = k_malloc(DEVICE_ID_MAX_SIZE);
char *device_id = k_malloc(DEVICE_ID_HEX_MAX_SIZE);
char *firmware_version = k_malloc(BOOT_IMG_VER_STRLEN_MAX);

if (device_id == NULL || firmware_version == NULL) {
LOG_ERR("Could not alloc device_id or firmware_version memory");
goto error;
}

if (!updatehub_get_device_identity(device_id, DEVICE_ID_MAX_SIZE)) {
if (!updatehub_get_device_identity(device_id, DEVICE_ID_HEX_MAX_SIZE)) {
goto error;
}

Expand Down Expand Up @@ -577,22 +587,23 @@ static int report(enum updatehub_state state)
return ret;
}

static void probe_cb(char *metadata)
static void probe_cb(char *metadata, size_t metadata_size)
{
struct coap_packet reply;
char tmp[MAX_PAYLOAD_SIZE];
char tmp[MAX_DOWNLOAD_DATA];
size_t tmp_len;
int rcvd = -1;

wait_fds();

rcvd = recv(ctx.sock, metadata, MAX_PAYLOAD_SIZE, MSG_DONTWAIT);
rcvd = recv(ctx.sock, tmp, MAX_DOWNLOAD_DATA, MSG_DONTWAIT);
if (rcvd <= 0) {
LOG_ERR("Could not receive data");
ctx.code_status = UPDATEHUB_NETWORKING_ERROR;
return;
}

if (coap_packet_parse(&reply, metadata, rcvd, NULL, 0) < 0) {
if (coap_packet_parse(&reply, tmp, rcvd, NULL, 0) < 0) {
LOG_ERR("Invalid data received");
ctx.code_status = UPDATEHUB_DOWNLOAD_ERROR;
return;
Expand All @@ -604,10 +615,25 @@ static void probe_cb(char *metadata)
return;
}

memset(&tmp, 0, MAX_PAYLOAD_SIZE);
memcpy(tmp, reply.data + reply.offset, reply.max_len - reply.offset);
memset(metadata, 0, MAX_PAYLOAD_SIZE);
memcpy(metadata, tmp, strlen(tmp));
/* check if we have buffer space to receive payload */
if (metadata_size < (reply.max_len - reply.offset)) {
LOG_ERR("There is no buffer available");
ctx.code_status = UPDATEHUB_METADATA_ERROR;
return;
}

memcpy(metadata, reply.data + reply.offset,
reply.max_len - reply.offset);

/* ensures payload have a valid string with size lower
* than metadata_size
*/
tmp_len = strlen(metadata);
if (tmp_len >= metadata_size) {
LOG_ERR("Invalid metadata data received");
ctx.code_status = UPDATEHUB_METADATA_ERROR;
return;
}

ctx.code_status = UPDATEHUB_OK;

Expand All @@ -620,11 +646,13 @@ enum updatehub_response updatehub_probe(void)
struct resp_probe_some_boards metadata_some_boards;
struct resp_probe_any_boards metadata_any_boards;

char *metadata = k_malloc(MAX_PAYLOAD_SIZE);
char *metadata_copy = k_malloc(MAX_PAYLOAD_SIZE);
char *device_id = k_malloc(DEVICE_ID_MAX_SIZE);
char *metadata = k_malloc(MAX_DOWNLOAD_DATA);
char *metadata_copy = k_malloc(MAX_DOWNLOAD_DATA);
char *device_id = k_malloc(DEVICE_ID_HEX_MAX_SIZE);
char *firmware_version = k_malloc(BOOT_IMG_VER_STRLEN_MAX);

size_t sha256size;

if (device_id == NULL || firmware_version == NULL ||
metadata == NULL || metadata_copy == NULL) {
LOG_ERR("Could not alloc probe memory");
Expand All @@ -643,7 +671,7 @@ enum updatehub_response updatehub_probe(void)
goto error;
}

if (!updatehub_get_device_identity(device_id, DEVICE_ID_MAX_SIZE)) {
if (!updatehub_get_device_identity(device_id, DEVICE_ID_HEX_MAX_SIZE)) {
ctx.code_status = UPDATEHUB_METADATA_ERROR;
goto error;
}
Expand Down Expand Up @@ -674,8 +702,7 @@ enum updatehub_response updatehub_probe(void)
goto cleanup;
}

memset(metadata, 0, MAX_PAYLOAD_SIZE);
probe_cb(metadata);
probe_cb(metadata, MAX_DOWNLOAD_DATA);

if (ctx.code_status != UPDATEHUB_OK) {
goto cleanup;
Expand Down Expand Up @@ -703,9 +730,18 @@ enum updatehub_response updatehub_probe(void)
goto cleanup;
}

sha256size = strlen(
metadata_any_boards.objects[1].objects.sha256sum) + 1;

if (sha256size != SHA256_HEX_DIGEST_SIZE) {
LOG_ERR("SHA256 size is invalid");
ctx.code_status = UPDATEHUB_METADATA_ERROR;
goto cleanup;
}

memcpy(update_info.sha256sum_image,
metadata_any_boards.objects[1].objects.sha256sum,
strlen(metadata_any_boards.objects[1].objects.sha256sum));
SHA256_HEX_DIGEST_SIZE);
update_info.image_size = metadata_any_boards.objects[1].objects.size;
} else {
if (!is_compatible_hardware(&metadata_some_boards)) {
Expand All @@ -714,10 +750,19 @@ enum updatehub_response updatehub_probe(void)
UPDATEHUB_INCOMPATIBLE_HARDWARE;
goto cleanup;
}

sha256size = strlen(
metadata_any_boards.objects[1].objects.sha256sum) + 1;

if (sha256size != SHA256_HEX_DIGEST_SIZE) {
LOG_ERR("SHA256 size is invalid");
ctx.code_status = UPDATEHUB_METADATA_ERROR;
goto cleanup;
}

memcpy(update_info.sha256sum_image,
metadata_some_boards.objects[1].objects.sha256sum,
strlen(metadata_some_boards.objects[1]
.objects.sha256sum));
SHA256_HEX_DIGEST_SIZE);
update_info.image_size =
metadata_some_boards.objects[1].objects.size;
}
Expand Down
15 changes: 4 additions & 11 deletions lib/updatehub/updatehub_device.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,23 +7,16 @@

bool updatehub_get_device_identity(char *id, int id_max_len)
{
int i, id_len = 0;
char buf[3];
u8_t hwinfo_id[id_max_len];
u8_t hwinfo_id[DEVICE_ID_BIN_MAX_SIZE];
size_t length;

length = hwinfo_get_device_id(hwinfo_id, sizeof(hwinfo_id) - 1);
length = hwinfo_get_device_id(hwinfo_id, DEVICE_ID_BIN_MAX_SIZE);
if (length <= 0) {
return false;
}

memset(id, 0, id_max_len);
length = bin2hex(hwinfo_id, length, id, id_max_len - 1);

for (i = 0; i < length; i++) {
snprintk(buf, sizeof(buf), "%02x", hwinfo_id[i]);
id_len = strlen(id);
strncat(id, buf, id_max_len - id_len);
}

return true;
return length > 0;
}
3 changes: 2 additions & 1 deletion lib/updatehub/updatehub_device.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,8 @@
#include <zephyr.h>
#include <drivers/hwinfo.h>

#define DEVICE_ID_MAX_SIZE 65
#define DEVICE_ID_BIN_MAX_SIZE 64
#define DEVICE_ID_HEX_MAX_SIZE ((DEVICE_ID_BIN_MAX_SIZE * 2) + 1)

bool updatehub_get_device_identity(char *id, int id_max_len);

Expand Down