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

Refactor dependency resolution out of deployment handler #636

Closed
wants to merge 13 commits into from
Closed

Refactor dependency resolution out of deployment handler #636

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
5e9f388
Refactor dependency resolution out of deployment handler
sameerzuberi Nov 15, 2024
846ffa4
Merge branch 'main' into refactor-deployments
sameerzuberi Nov 15, 2024
22e6b11
Resolve merge conflicts
sameerzuberi Nov 15, 2024
27eded1
Make memory non static and memcpy buffers
sameerzuberi Nov 15, 2024
49526a7
Replace char parameters with GglBuffer
sameerzuberi Nov 18, 2024
518e46a
Merge branch 'main' into refactor-deployments
sameerzuberi Nov 18, 2024
6071693
Fix headers
sameerzuberi Nov 18, 2024
2b32a7e
Fix assert statements
sameerzuberi Nov 21, 2024
29e5aa8
Merge branch 'main' into refactor-deployments
sameerzuberi Nov 21, 2024
5673e1a
Add bool header
sameerzuberi Nov 21, 2024
05967f4
Merge branch 'main' into refactor-deployments
sameerzuberi Nov 21, 2024
212c8b9
Merge branch 'main' into refactor-deployments
sameerzuberi Nov 22, 2024
9e24eba
Add assert header to deployment_handler
sameerzuberi Nov 22, 2024
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
9 changes: 4 additions & 5 deletions ggdeploymentd/src/dependency_resolver.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,21 +74,20 @@ static GglError get_device_thing_groups(GglBuffer *response) {
.gghttplib_root_ca_path = config.rootca_path,
.gghttplib_p_key_path = config.pkey_path };

char *thing_name = NULL;
uint8_t thing_name_arr[128];
GglBuffer thing_name = GGL_BUF(thing_name_arr);
ret = get_thing_name(&thing_name);
if (ret != GGL_ERR_OK) {
GGL_LOGE("Failed to get thing name.");
return ret;
}

static uint8_t uri_path_buf[PATH_MAX];
uint8_t uri_path_buf[PATH_MAX];
GglByteVec uri_path_vec = GGL_BYTE_VEC(uri_path_buf);
ret = ggl_byte_vec_append(
&uri_path_vec, GGL_STR("greengrass/v2/coreDevices/")
);
ggl_byte_vec_chain_append(
&ret, &uri_path_vec, ggl_buffer_from_null_term(thing_name)
);
ggl_byte_vec_chain_append(&ret, &uri_path_vec, thing_name);
ggl_byte_vec_chain_append(&ret, &uri_path_vec, GGL_STR("/thingGroups"));
if (ret != GGL_ERR_OK) {
GGL_LOGE("Failed to create thing groups call uri.");
Expand Down
5 changes: 5 additions & 0 deletions ggdeploymentd/src/deployment_configuration.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
// SPDX-License-Identifier: Apache-2.0

#include "deployment_configuration.h"
#include <assert.h>
#include <ggl/buffer.h>
#include <ggl/core_bus/gg_config.h>
#include <ggl/error.h>
Expand Down Expand Up @@ -93,6 +94,7 @@ GglError get_thing_name(GglBuffer *thing_name) {
return ret;
}

assert(thing_name->len <= resp.len);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that this assert is wrong. Should it not be thing_name->len > resp.len?

we should also fail gracefully in case the lengths do no match.
I think it can be done as such:

if(thing_name->len <= resp.len) {
    assert(false);
    return GGL_ERR_FAILURE;
}

This would allow us to fail gracefully even when the assert is not defined in production environments.

memcpy(thing_name->data, resp.data, resp.len);
thing_name->len = resp.len;
return GGL_ERR_OK;
Expand All @@ -110,6 +112,7 @@ GglError get_root_ca_path(GglBuffer *root_ca_path) {
return ret;
}

assert(root_ca_path->len <= resp.len);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This assert also seems wrong no?

memcpy(root_ca_path->data, resp.data, resp.len);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need to verify that the buffer is at least as long as resp.data before copying over the data. Similarly in other places

root_ca_path->len = resp.len;
return GGL_ERR_OK;
Expand All @@ -133,6 +136,7 @@ GglError get_tes_cred_url(GglBuffer *tes_cred_url) {
return ret;
}

assert(tes_cred_url->len <= resp.len);
memcpy(tes_cred_url->data, resp.data, resp.len);
tes_cred_url->len = resp.len;
return GGL_ERR_OK;
Expand All @@ -157,6 +161,7 @@ GglError get_posix_user(GglBuffer *posix_user) {
return ret;
}

assert(posix_user->len <= resp.len);
memcpy(posix_user->data, resp.data, resp.len);
posix_user->len = resp.len;
return GGL_ERR_OK;
Expand Down
61 changes: 35 additions & 26 deletions ggdeploymentd/src/deployment_handler.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@
#include <ggl/vector.h>
#include <ggl/zip.h>
#include <limits.h>
#include <linux/limits.h>
#include <string.h>
#include <stdbool.h>
#include <stdint.h>
Expand Down Expand Up @@ -874,54 +875,58 @@ static void handle_deployment(
return;
}

char *thing_name = NULL;
uint8_t thing_name_arr[128];
GglBuffer thing_name = GGL_BUF(thing_name_arr);
ret = get_thing_name(&thing_name);
if (ret != GGL_ERR_OK) {
GGL_LOGE("Failed to get thing name.");
return;
}

char *root_ca_path = NULL;
uint8_t root_ca_path_arr[PATH_MAX];
GglBuffer root_ca_path = GGL_BUF(root_ca_path_arr);
ret = get_root_ca_path(&root_ca_path);
if (ret != GGL_ERR_OK) {
GGL_LOGE("Failed to get rootCaPath.");
return;
}

char *tes_cred_url = NULL;
uint8_t tes_cred_arr[128];
GglBuffer tes_cred_url = GGL_BUF(tes_cred_arr);
ret = get_tes_cred_url(&tes_cred_url);
if (ret != GGL_ERR_OK) {
GGL_LOGE("Failed to get tes credentials url.");
return;
}

char *posix_user = NULL;
uint8_t posix_user_arr[128];
GglBuffer posix_user = GGL_BUF(posix_user_arr);
ret = get_posix_user(&posix_user);
if (ret != GGL_ERR_OK) {
GGL_LOGE("Failed to get posix_user.");
return;
}
if (strlen(posix_user) < 1) {
if (posix_user.len < 1) {
GGL_LOGE("Run with default posix user is not set.");
return;
}
bool colon_found = false;
char *group;
for (size_t j = 0; j < strlen(posix_user); j++) {
if (posix_user[j] == ':') {
posix_user[j] = '\0';
for (size_t j = 0; j < posix_user.len; j++) {
if (posix_user.data[j] == ':') {
posix_user.data[j] = '\0';
colon_found = true;
group = &posix_user[j + 1];
group = (char *) &posix_user.data[j + 1];
break;
}
}
if (!colon_found) {
group = posix_user;
group = (char *) posix_user.data;
}

static Recipe2UnitArgs recipe2unit_args;
memset(&recipe2unit_args, 0, sizeof(Recipe2UnitArgs));
recipe2unit_args.user = posix_user;
recipe2unit_args.user = (char *) posix_user.data;
recipe2unit_args.group = group;

recipe2unit_args.component_name = pair->key;
Expand Down Expand Up @@ -1369,54 +1374,58 @@ static void handle_deployment(
return;
}

char *thing_name = NULL;
uint8_t thing_name_arr[128];
GglBuffer thing_name = GGL_BUF(thing_name_arr);
ret = get_thing_name(&thing_name);
if (ret != GGL_ERR_OK) {
GGL_LOGE("Failed to get thing name.");
return;
}

GglByteVec region = GGL_BYTE_VEC(config.region);
ret = get_region(&region);
uint8_t root_ca_path_arr[PATH_MAX];
GglBuffer root_ca_path = GGL_BUF(root_ca_path_arr);
ret = get_root_ca_path(&root_ca_path);
if (ret != GGL_ERR_OK) {
GGL_LOGE("Failed to get region.");
GGL_LOGE("Failed to get rootCaPath.");
return;
}

char *root_ca_path = NULL;
ret = get_root_ca_path(&root_ca_path);
uint8_t tes_cred_arr[128];
GglBuffer tes_cred_url = GGL_BUF(tes_cred_arr);
ret = get_tes_cred_url(&tes_cred_url);
if (ret != GGL_ERR_OK) {
GGL_LOGE("Failed to get rootCaPath.");
GGL_LOGE("Failed to get tes credentials url.");
return;
}

char *posix_user = NULL;
uint8_t posix_user_arr[128];
GglBuffer posix_user = GGL_BUF(posix_user_arr);
ret = get_posix_user(&posix_user);
if (ret != GGL_ERR_OK) {
GGL_LOGE("Failed to get posix_user.");
return;
}
if (strlen(posix_user) < 1) {
if (posix_user.len < 1) {
GGL_LOGE("Run with default posix user is not set.");
return;
}
bool colon_found = false;
char *group;
for (size_t i = 0; i < strlen(posix_user); i++) {
if (posix_user[i] == ':') {
posix_user[i] = '\0';
for (size_t j = 0; j < posix_user.len; j++) {
if (posix_user.data[j] == ':') {
posix_user.data[j] = '\0';
colon_found = true;
group = &posix_user[i + 1];
group = (char *) &posix_user.data[j + 1];
break;
}
}
if (!colon_found) {
group = posix_user;
group = (char *) posix_user.data;
}

static Recipe2UnitArgs recipe2unit_args;
memset(&recipe2unit_args, 0, sizeof(Recipe2UnitArgs));
recipe2unit_args.user = posix_user;
recipe2unit_args.user = (char *) posix_user.data;
recipe2unit_args.group = group;

GGL_LOGI(
Expand Down
Loading