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

Adds Prediction Resistance to the DRBG and RDRAND if supported #248

Merged
merged 1 commit into from
Jul 2, 2016
Merged

Adds Prediction Resistance to the DRBG and RDRAND if supported #248

merged 1 commit into from
Jul 2, 2016

Conversation

SalusaSecondus
Copy link
Contributor

This adds prediction resistance to the DRBG used by S2N (as defined in the appropriate NIST standard). Additionally, RDRAND (the Intel hardware based RNG) is used for reseeding if the processor supports it. The assembly to access RDRAND requires a 64-bit system, however I do not believe that there are any 32 bit systems which support RDRAND. Finally, as commented, the raw op-codes for RDRAND are used rather than the mnemonic due to an unfortunate need to support some older assemblers which do not (yet) recognize this instruction.

This code has not yet undergone code-review by someone other than myself.

/* After initial seeding, pivot to RDRAND if available and not overridden */
if (drbg->entropy_generator == NULL && s2n_cpu_supports_rdrand()) {
drbg->entropy_generator = s2n_get_rdrand_data;
}
Copy link
Contributor

Choose a reason for hiding this comment

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

If s2n_cpu_supports_rdrand() isn't true, we'll end up having no PR but also no reseeding at all, even past the usage limit. Should we plug in get urandom in that case? It'll be slow but safe.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Even with no RDRAND we will still have PR. Check out lines 94-99. If the entropy generator is not set in the s2n_drbg object, then it falls back to urandom.

https://github.com/awslabs/s2n/pull/248/files#diff-c03ac19053399e59511956db1159e2e8R98

Copy link
Contributor

Choose a reason for hiding this comment

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

Ahh, that makes sense. I missed that.

@colmmacc colmmacc merged commit 95769f4 into aws:master Jul 2, 2016
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.

2 participants