Skip to content

Conversation

@mspncp
Copy link
Contributor

@mspncp mspncp commented Jun 9, 2019

The first commit of this pull request is the (unmodified) cherry-pick of #9084, the second adds the CHANGES entries:

82c4bcb Revert the DEVRANDOM_WAIT feature (cherry picked from commit c19c5a6)
55df89e Add CHANGES entries for the DEVRANDOM_WAIT feature and its removal.

The DEVRANDOM_WAIT feature added a select() call to wait for the
`/dev/random` device to become readable before reading from the
`/dev/urandom` device. It was introduced in commit 38023b8
in order to mitigate the fact that the `/dev/urandom` device
does not block until the initial seeding of the kernel CSPRNG
has completed, contrary to the behaviour of the `getrandom()`
system call.

It turned out that this change had negative side effects on the
performance which were not acceptable. After some discussion it
was decided to revert this feature and leave it up to the OS
resp. the platform maintainer to ensure a proper initialization
during early boot time.

Fixes openssl#9078

This partially reverts commit 38023b8.

(cherry picked from commit c19c5a6)
@mspncp mspncp requested review from t-j-h and vdukhovni June 9, 2019 00:32
@mspncp mspncp mentioned this pull request Jun 9, 2019
2 tasks
@mspncp mspncp force-pushed the pr-revert-devrandom-wait-111 branch from 32fffba to 55df89e Compare June 9, 2019 00:40
On older Linux systems where the getrandom() system call is not available,
OpenSSL normally uses the /dev/urandom device for seeding its CSPRNG.
Contrary to getrandom(), the /dev/urandom device will not block during
early boot when the kernel CSPRNG has not been initially seeded yet.
Copy link
Member

Choose a reason for hiding this comment

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

"initially seeded yet" is a tautology - either "initially seeded" or "seeded yet".

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll go for "has not been seeded yet."

Copy link
Member

@t-j-h t-j-h left a comment

Choose a reason for hiding this comment

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

The wording should be adjusted - but I'll leave that to you.

@t-j-h t-j-h added the approval: done This pull request has the required number of approvals label Jun 9, 2019
Copy link

@vdukhovni vdukhovni left a comment

Choose a reason for hiding this comment

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

Approved, with minor nits on the phrasing of the change log.

/dev/random device to become readable before reading from the
/dev/urandom device.

It turned out that this change had negative side effects on the

Choose a reason for hiding this comment

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

A bit clumsy, but just removing the word "the" will be close enough.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok. I'll remove the "the" here and in the commit message.

levitte pushed a commit that referenced this pull request Jun 9, 2019
The DEVRANDOM_WAIT feature added a select() call to wait for the
`/dev/random` device to become readable before reading from the
`/dev/urandom` device. It was introduced in commit 38023b8
in order to mitigate the fact that the `/dev/urandom` device
does not block until the initial seeding of the kernel CSPRNG
has completed, contrary to the behaviour of the `getrandom()`
system call.

It turned out that this change had negative side effects on
performance which were not acceptable. After some discussion it
was decided to revert this feature and leave it up to the OS
resp. the platform maintainer to ensure a proper initialization
during early boot time.

Fixes #9078

This partially reverts commit 38023b8.

Reviewed-by: Tim Hudson <tjh@openssl.org>
Reviewed-by: Viktor Dukhovni <viktor@openssl.org>

(cherry picked from commit a08714e)

(Merged from #9118)
levitte pushed a commit that referenced this pull request Jun 9, 2019
Reviewed-by: Tim Hudson <tjh@openssl.org>
Reviewed-by: Viktor Dukhovni <viktor@openssl.org>
(Merged from #9118)
@mspncp
Copy link
Contributor Author

mspncp commented Jun 9, 2019

Merged, thank you!

247b8a0 Add CHANGES entries for the DEVRANDOM_WAIT feature and its removal
ad416c8 Revert the DEVRANDOM_WAIT feature

@mspncp mspncp closed this Jun 9, 2019
@mspncp mspncp added the branch: 1.1.1 Applies to OpenSSL_1_1_1-stable branch (EOL) label Jun 9, 2019
themiron pushed a commit to RMerl/asuswrt-merlin.ng that referenced this pull request Jun 12, 2019
The DEVRANDOM_WAIT feature added a select() call to wait for the
`/dev/random` device to become readable before reading from the
`/dev/urandom` device. It was introduced in commit 38023b87f037
in order to mitigate the fact that the `/dev/urandom` device
does not block until the initial seeding of the kernel CSPRNG
has completed, contrary to the behaviour of the `getrandom()`
system call.

It turned out that this change had negative side effects on
performance which were not acceptable. After some discussion it
was decided to revert this feature and leave it up to the OS
resp. the platform maintainer to ensure a proper initialization
during early boot time.

Fixes #9078

This partially reverts commit 38023b87f037.

Reviewed-by: Tim Hudson <tjh@openssl.org>
Reviewed-by: Viktor Dukhovni <viktor@openssl.org>

(cherry picked from commit a08714e18131b1998faa0113e5bd4024044654ac)

(Merged from openssl/openssl#9118)
@mspncp mspncp deleted the pr-revert-devrandom-wait-111 branch June 27, 2019 15:05
john9527 pushed a commit to john9527/asuswrt-merlin that referenced this pull request Feb 13, 2020
The DEVRANDOM_WAIT feature added a select() call to wait for the
`/dev/random` device to become readable before reading from the
`/dev/urandom` device. It was introduced in commit 38023b87f037
in order to mitigate the fact that the `/dev/urandom` device
does not block until the initial seeding of the kernel CSPRNG
has completed, contrary to the behaviour of the `getrandom()`
system call.

It turned out that this change had negative side effects on
performance which were not acceptable. After some discussion it
was decided to revert this feature and leave it up to the OS
resp. the platform maintainer to ensure a proper initialization
during early boot time.

Fixes #9078

This partially reverts commit 38023b87f037.

Reviewed-by: Tim Hudson <tjh@openssl.org>
Reviewed-by: Viktor Dukhovni <viktor@openssl.org>

(cherry picked from commit a08714e18131b1998faa0113e5bd4024044654ac)

(Merged from openssl/openssl#9118)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approval: done This pull request has the required number of approvals branch: 1.1.1 Applies to OpenSSL_1_1_1-stable branch (EOL)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants