Skip to content

Comments

drbg_get_entropy: force a reseed before calling ssleay_rand_bytes() [first attempt]#7247

Closed
mspncp wants to merge 2 commits intoopenssl:OpenSSL_1_0_2-stablefrom
mspncp:pr-drbg-get-entropy-force-reseed
Closed

drbg_get_entropy: force a reseed before calling ssleay_rand_bytes() [first attempt]#7247
mspncp wants to merge 2 commits intoopenssl:OpenSSL_1_0_2-stablefrom
mspncp:pr-drbg-get-entropy-force-reseed

Conversation

@mspncp
Copy link
Contributor

@mspncp mspncp commented Sep 17, 2018

Fixes #7240

In FIPS mode, the default FIPS DRBG uses the drbg_get_entropy()
callback to reseed itself, which is provided by the wrapping
libcrypto library. This callback in turn uses ssleay_rand_bytes()
to generate random bytes.

Now ssleay_rand_bytes() calls RAND_poll() once on first call to
seed itself, but RAND_poll() is never called again (unless the
application calls RAND_poll() explicitely). This implies that
whenever the DRBG reseeds itself (which happens every 2^14
generate requests) this happens without obtaining fresh random
data from the operating system's entropy sources.

This patch forces a reseed from system entropy sources on every
call to drbg_get_entropy(). In contrary to the automatic reseeding
of the DRBG in master, this reseeding does not break applications
running in a chroot() environment (see c7504ae), because the
SSLEAY PRNG does not maintain an error state. (It does not even
check the return value of RAND_poll() on its instantiation.)

In the worst case, if no random device is available for reseeding,
no fresh entropy will be added to the SSLEAY PRNG but it will happily
continue to generate random bytes as 'entropy' input for the DRBG's
reseeding, which is just as good (or bad) as before this patch.

Checklist
  • documentation is added or updated
  • tests are added or updated

@mspncp mspncp added the branch: 1.0.2 Applies to OpenSSL_1_0_2-stable branch (EOL) label Sep 17, 2018
@mspncp mspncp requested review from kroeckx and paulidale September 17, 2018 22:11
@mspncp
Copy link
Contributor Author

mspncp commented Sep 17, 2018

Thanks Pauli. I'll wait for Kurts opinion nevertheless before merging.

Fixes openssl#7240

In FIPS mode, the default FIPS DRBG uses the drbg_get_entropy()
callback to reseed itself, which is provided by the wrapping
libcrypto library. This callback in turn uses ssleay_rand_bytes()
to generate random bytes.

Now ssleay_rand_bytes() calls RAND_poll() once on first call to
seed itself, but RAND_poll() is never called again (unless the
application calls RAND_poll() explicitely). This implies that
whenever the DRBG reseeds itself (which happens every 2^14
generate requests) this happens without obtaining fresh random
data from the operating system's entropy sources.

This patch forces a reseed from system entropy sources on every
call to drbg_get_entropy(). In contrary to the automatic reseeding
of the DRBG in master, this reseeding does not break applications
running in a chroot() environment (see c7504ae), because the
SSLEAY PRNG does not maintain an error state. (It does not even
check the return value of RAND_poll() on its instantiation.)

In the worst case, if no random device is available for reseeding,
no fresh entropy will be added to the SSLEAY PRNG but it will happily
continue to generate random bytes as 'entropy' input for the DRBG's
reseeding, which is just as good (or bad) as before this patch.
@mspncp mspncp force-pushed the pr-drbg-get-entropy-force-reseed branch from 237a376 to 86e2ab4 Compare September 17, 2018 22:51
@mspncp
Copy link
Contributor Author

mspncp commented Sep 17, 2018

Just added Fixes #7240 to the commit message. No source code changes.

@mspncp
Copy link
Contributor Author

mspncp commented Sep 18, 2018

@paulidale in view of #7240 (comment) I decided to remove the note about the locking from the comment. Please reconfirm.

@mspncp
Copy link
Contributor Author

mspncp commented Sep 18, 2018

@kroeckx are you interested in reviewing this pr?

@mspncp mspncp added the approval: review pending This pull request needs review by a committer label Sep 18, 2018
Copy link
Contributor

@kaduk kaduk left a comment

Choose a reason for hiding this comment

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

Kind of an awkward workaround, but I don't think we have scope to do broader changes on this branch.

@kroeckx
Copy link
Member

kroeckx commented Sep 18, 2018 via email

@mspncp
Copy link
Contributor Author

mspncp commented Sep 18, 2018

Kind of an awkward workaround, but I don't think we have scope to do broader changes on this branch.

@kaduk maybe you are right. Does this alternative fix #7259 look less awkward to you?

@mspncp mspncp removed the request for review from kroeckx September 18, 2018 18:04
@mspncp mspncp removed the approval: review pending This pull request needs review by a committer label Sep 18, 2018
@mspncp mspncp changed the title drbg_get_entropy: force a reseed before calling ssleay_rand_bytes() drbg_get_entropy: force a reseed before calling ssleay_rand_bytes() [first attempt] Sep 19, 2018
@mspncp
Copy link
Contributor Author

mspncp commented Sep 19, 2018

Closing this pull request in favour of #7259.

@mspncp mspncp closed this Sep 19, 2018
@mspncp mspncp deleted the pr-drbg-get-entropy-force-reseed branch September 19, 2018 19:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

branch: 1.0.2 Applies to OpenSSL_1_0_2-stable branch (EOL)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants