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

Jump to bottom

Remove redundant ifdef DEVICE_TRNG from DeviceKey #9576

Merged
merged 1 commit into from
Feb 13, 2019
Merged

Remove redundant ifdef DEVICE_TRNG from DeviceKey #9576

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
3 changes: 2 additions & 1 deletion features/device_key/source/DeviceKey.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -260,7 +260,7 @@ int DeviceKey::generate_key_by_random(uint32_t *output, size_t size)
return DEVICEKEY_INVALID_PARAM;
}

#if DEVICE_TRNG
Copy link
Contributor

Choose a reason for hiding this comment

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

This section of code is only safe if either the device has a TRNG, or MBEDTLS_ENTROPY_NV_SEED is turned on. The mbedtls_entropy_func() might catch it if this is not the case and return an error, but it depends on the Mbed TLS configuration. I think it would be safer to add && defined(MBEDTLS_ENTROPY_NV_SEED) to the condition than remove the macro completely. Would that solve the problem?

Copy link

Choose a reason for hiding this comment

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

@yanesca What will happen if there is no entropy seed? Will it be better || defined(MBEDTLS_ENTROPY_NV_SEED)?

Copy link
Contributor

Choose a reason for hiding this comment

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

If a component registers its own weak entropy sources as strong or enables the HAVEGE source, then this function returns low entropy keys. Yes, I think that would be better: it would make explicit that we only provide output if TRNG or entropy seed is present.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@yanesca thanks.
I have fixed it as you suggested.

#if defined(DEVICE_TRNG) || defined(MBEDTLS_ENTROPY_NV_SEED)
uint32_t test_buff[DEVICE_KEY_32BYTE / sizeof(int)];
mbedtls_entropy_context *entropy = new mbedtls_entropy_context;
mbedtls_entropy_init(entropy);
Expand All @@ -276,6 +276,7 @@ int DeviceKey::generate_key_by_random(uint32_t *output, size_t size)

mbedtls_entropy_free(entropy);
delete entropy;

#endif

return ret;
Expand Down