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

lms: Move merkle tree generation to heap allocation #6595

Merged
merged 1 commit into from
Dec 23, 2022
Merged

lms: Move merkle tree generation to heap allocation #6595

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
47 changes: 32 additions & 15 deletions library/lms.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,10 +65,9 @@

/* Currently only support H=10 */
#define H_TREE_HEIGHT_MAX 10
#define MERKLE_TREE_NODE_AM_MAX (1u << (H_TREE_HEIGHT_MAX + 1u))
#define MERKLE_TREE_NODE_AM(type) (1u << (MBEDTLS_LMS_H_TREE_HEIGHT(type) + 1u))
#define MERKLE_TREE_LEAF_NODE_AM(type) (1u << MBEDTLS_LMS_H_TREE_HEIGHT(type))
#define MERKLE_TREE_INTERNAL_NODE_AM(type) (1u << MBEDTLS_LMS_H_TREE_HEIGHT(type))
#define MERKLE_TREE_NODE_AM(type) ((size_t) 1 << (MBEDTLS_LMS_H_TREE_HEIGHT(type) + 1u))
#define MERKLE_TREE_LEAF_NODE_AM(type) ((size_t) 1 << MBEDTLS_LMS_H_TREE_HEIGHT(type))
#define MERKLE_TREE_INTERNAL_NODE_AM(type) ((size_t) 1 << MBEDTLS_LMS_H_TREE_HEIGHT(type))

#define D_CONST_LEN (2)
static const unsigned char D_LEAF_CONSTANT_BYTES[D_CONST_LEN] = {0x82, 0x82};
Expand Down Expand Up @@ -499,13 +498,21 @@ static int get_merkle_path( mbedtls_lms_private_t *ctx,
unsigned int leaf_node_id,
unsigned char *path )
{
unsigned char tree[MERKLE_TREE_NODE_AM_MAX][MBEDTLS_LMS_M_NODE_BYTES_MAX];
const size_t node_bytes = MBEDTLS_LMS_M_NODE_BYTES(ctx->params.type);
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
const size_t node_bytes = MBEDTLS_LMS_M_NODE_BYTES(ctx->params.type);
const size_t node_bytes = MBEDTLS_LMS_M_NODE_BYTES( ctx->params.type );

Copy link
Contributor

Choose a reason for hiding this comment

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

We're switching to the new style at the end of the month (or maybe first week of January). So let's not be picky about it now.

unsigned int curr_node_id = leaf_node_id;
unsigned int adjacent_node_id;
unsigned char *tree = NULL;
unsigned int height;
int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED;

ret = calculate_merkle_tree( ctx, ( unsigned char * )tree );
tree = mbedtls_calloc( MERKLE_TREE_NODE_AM(ctx->params.type),
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
tree = mbedtls_calloc( MERKLE_TREE_NODE_AM(ctx->params.type),
tree = mbedtls_calloc( MERKLE_TREE_NODE_AM( ctx->params.type ),

node_bytes );
if ( tree == NULL )
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
if ( tree == NULL )
if( tree == NULL )
return MBEDTLS_ERR_LMS_ALLOC_FAILED;

{
return MBEDTLS_ERR_LMS_ALLOC_FAILED;
}

ret = calculate_merkle_tree( ctx, tree );
if( ret != 0 )
{
goto exit;
Expand All @@ -516,17 +523,18 @@ static int get_merkle_path( mbedtls_lms_private_t *ctx,
{
adjacent_node_id = curr_node_id ^ 1;

memcpy( &path[height * MBEDTLS_LMS_M_NODE_BYTES(ctx->params.type)],
&tree[adjacent_node_id],
MBEDTLS_LMS_M_NODE_BYTES(ctx->params.type) );
memcpy( &path[height * node_bytes],
&tree[adjacent_node_id * node_bytes], node_bytes );
Copy link
Contributor

Choose a reason for hiding this comment

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

Why the second parameter (source address) has been changed?
&tree[adjacent_node_id] -> &tree[adjacent_node_id * node_bytes]

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The type of tree changed, now it's a flat unsigned char array.


curr_node_id >>=1;
}

ret = 0;

exit:
mbedtls_platform_zeroize( tree, sizeof( tree ) );
mbedtls_platform_zeroize( tree, node_bytes *
MERKLE_TREE_NODE_AM(ctx->params.type) );
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
MERKLE_TREE_NODE_AM(ctx->params.type) );
MERKLE_TREE_NODE_AM( ctx->params.type ) );

mbedtls_free ( tree );
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
mbedtls_free ( tree );
mbedtls_free( tree );


return( ret );
}
Expand Down Expand Up @@ -659,8 +667,9 @@ int mbedtls_lms_generate_private_key( mbedtls_lms_private_t *ctx,
int mbedtls_lms_calculate_public_key( mbedtls_lms_public_t *ctx,
const mbedtls_lms_private_t *priv_ctx )
{
unsigned char tree[MERKLE_TREE_NODE_AM_MAX][MBEDTLS_LMS_M_NODE_BYTES_MAX];
const size_t node_bytes = MBEDTLS_LMS_M_NODE_BYTES(priv_ctx->params.type);
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
const size_t node_bytes = MBEDTLS_LMS_M_NODE_BYTES(priv_ctx->params.type);
const size_t node_bytes = MBEDTLS_LMS_M_NODE_BYTES( priv_ctx->params.type );

int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED;
unsigned char *tree = NULL;

if( ! priv_ctx->have_private_key )
{
Expand All @@ -679,25 +688,33 @@ int mbedtls_lms_calculate_public_key( mbedtls_lms_public_t *ctx,
return( MBEDTLS_ERR_LMS_BAD_INPUT_DATA );
}

tree = mbedtls_calloc( MERKLE_TREE_NODE_AM(priv_ctx->params.type),
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
tree = mbedtls_calloc( MERKLE_TREE_NODE_AM(priv_ctx->params.type),
tree = mbedtls_calloc( MERKLE_TREE_NODE_AM( priv_ctx->params.type ),

node_bytes );
if ( tree == NULL )
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
if ( tree == NULL )
if ( tree == NULL )
return MBEDTLS_ERR_LMS_ALLOC_FAILED;

{
return MBEDTLS_ERR_LMS_ALLOC_FAILED;
}

memcpy( &ctx->params, &priv_ctx->params,
sizeof( mbedtls_lmots_parameters_t ) );

ret = calculate_merkle_tree( priv_ctx, ( unsigned char * )tree );
ret = calculate_merkle_tree( priv_ctx, tree );
if( ret != 0 )
{
goto exit;
}

/* Root node is always at position 1, due to 1-based indexing */
memcpy( ctx->T_1_pub_key, &tree[1],
MBEDTLS_LMS_M_NODE_BYTES(ctx->params.type) );
memcpy( ctx->T_1_pub_key, &tree[node_bytes], node_bytes );
Copy link
Contributor

Choose a reason for hiding this comment

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

Why the second parameter (source address) has been changed?
&tree[1] -> &tree[node_bytes]

The comment above informs why index 1 was used here which is not the case now.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Type of tree changed to a flat unsigned char array. Any suggestions on how to reword the comment?


ctx->have_public_key = 1;

ret = 0;

exit:
mbedtls_platform_zeroize( tree, sizeof( tree ) );
mbedtls_platform_zeroize( tree, node_bytes *
MERKLE_TREE_NODE_AM(priv_ctx->params.type) );
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
MERKLE_TREE_NODE_AM(priv_ctx->params.type) );
MERKLE_TREE_NODE_AM( priv_ctx->params.type ) );

mbedtls_free ( tree );
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
mbedtls_free ( tree );
mbedtls_free( tree );


return( ret );
}
Expand Down