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.

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

Session based communication with private servers #151

Draft
wants to merge 12 commits into
base: develop
Choose a base branch
from

Conversation

arnabCypherock
Copy link
Contributor

No description provided.

@arnabCypherock arnabCypherock self-assigned this Feb 7, 2023
@rohanagarwal94
Copy link

@ujjwal-cyph ujjwal-cyph changed the title PRF-4499 Split PR between session and core functionality Session based communication with private servers Feb 10, 2023
@arnabCypherock
Copy link
Contributor Author

@ujjwal-cyph I have made the required changes for the session initiation phase and added the necessary utility methods

src/controller_main.c Outdated Show resolved Hide resolved
common/libraries/util/session_utils.h Outdated Show resolved Hide resolved
@@ -0,0 +1,138 @@
#include "session_utils.h"
Copy link
Collaborator

Choose a reason for hiding this comment

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

add license headers

this comment is applicable to other files too.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done ✅

Copy link
Collaborator

Choose a reason for hiding this comment

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

license header missing in atecc_utils.c

common/libraries/util/session_utils.c Outdated Show resolved Hide resolved
common/libraries/util/session_utils.c Outdated Show resolved Hide resolved
common/libraries/util/session_utils.c Outdated Show resolved Hide resolved
common/libraries/util/session_utils.c Outdated Show resolved Hide resolved
@@ -171,6 +171,9 @@ Flash_Wallet wallet_for_flash;
MessageData msg_data;
ui_display_node *current_display_node = NULL;

// create a new session object
Session session;
Copy link
Collaborator

Choose a reason for hiding this comment

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

this need not be global. it can be local variable to the session.c.

&session_init_details);

if (!session_init(&session, &session_init_details)) {
LOG_CRITICAL("xxec %d:%d", false, __LINE__);
Copy link
Collaborator

Choose a reason for hiding this comment

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

log only required values. false is hardcoded here

uint8_t *data,
uint8_t *digest,
uint8_t *postfix,
atecc_data_t atecc_value) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

this is wrong. pass by value won't achieve what we want. the data updated by the function would not reflect in the original instance of the struct.

Suggested change
atecc_data_t atecc_value) {
atecc_data_t atecc_value) {

* Licensor: HODL TECH PTE LTD
*
******************************************************************************
*/
Copy link
Collaborator

Choose a reason for hiding this comment

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

full license is added only in the source files. headers only have the short version.

uint8_t data_key_id,
atecc_data_t *atecc_value);

auth_data_t atecc_sign(uint8_t *hash);
Copy link
Collaborator

Choose a reason for hiding this comment

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

doxygen comments

@ujjwal-cyph ujjwal-cyph marked this pull request as draft June 21, 2023 08:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants