Skip to content
This repository was archived by the owner on Mar 5, 2024. It is now read-only.

Patch for #39 Sidechannel resistence of uECC_sign disabled #40

Merged
merged 2 commits into from
Sep 17, 2019

Conversation

winnietwo
Copy link
Contributor

I removed the g_rng_function variable from ecc_dh.c, as there is another one in ecc.c.
I access the g_rng_function variable in ecc.c by the uECC_get_rng(). In this way, side-channel resistance should be enabled again.

@winnietwo winnietwo marked this pull request as ready for review September 6, 2019 12:40
@mczraf mczraf self-requested a review September 10, 2019 21:44
@mczraf mczraf self-assigned this Sep 10, 2019
@mczraf
Copy link
Contributor

mczraf commented Sep 10, 2019

This patch effectively enables the provided side-channel countermeasure in the EC-DSA sign procedure. Thanks @winnietwo for your contribution to enhance TinyCrypt!

I have noticed that the same problem preventing EC-DSA sign to use the side-channel countermeasure was replicated in the EC-DH algorithm. I have fixed the EC-DH implementation in the commit above using the same strategy described by @winnietwo for EC-DSA.

Copy link
Contributor

@mczraf mczraf left a comment

Choose a reason for hiding this comment

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

This patch effectively enables the provided side-channel countermeasure for EC-DSA sign.

Copy link

@ceolin ceolin left a comment

Choose a reason for hiding this comment

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

I'm confuse here. This change just has impact if one sets its cryptographically-secure function using uECC_set_rng() and this is different from default_CSPRNG, right ? Is there a reason to have different random generators ? Anyway, the patch itself is good, it makes the code cleaner.

@winnietwo
Copy link
Contributor Author

Is there a reason to have different random generators?
The default random number generator is for Linux. On many non-Linux embedded devices, it's necessary to have your own implementation.

@winnietwo winnietwo closed this Sep 17, 2019
@mczraf mczraf reopened this Sep 17, 2019
@mczraf mczraf merged commit 5969b0e into intel:master Sep 17, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants