Accept getrandom syscall being unknown in mbedtls_platform_entropy_poll() for linux #2117
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
added 2 commits
October 18, 2018 12:12
This commit fixes issue Mbed-TLS#1212 related to platform-specific entropy polling in an syscall-emulated environment. Previously, the implementation of the entropy gathering function `mbedtls_platform_entropy_poll()` for linux machines used the following logic to determine how to obtain entropy from the kernel: 1. If the getrandom() system call identifier SYS_getrandom is present and the kernel version is 3.17 or higher, use syscall( SYS_getrandom, ... ) 2. Otherwise, fall back to reading from /dev/random. There are two issues with this: 1. Portability: When cross-compiling the code for a different architecture and running it through system call emulation in qemu, qemu reports the host kernel version through uname but, as of v.2.5.0, doesn't support emulating the getrandom() syscall. This leads to `mbedtls_platform_entropy_poll()` failing even though reading from /dev/random would have worked. 2. Style: Extracting the linux kernel version from the output of `uname` is slightly tedious. This commit fixes both by implementing the suggestion in Mbed-TLS#1212: - It removes the kernel-version detection through uname(). - Instead, it checks whether `syscall( SYS_getrandom, ... )` fails with errno set to ENOSYS indicating an unknown system call. If so, it falls through to trying to read from /dev/random. Fixes Mbed-TLS#1212.
hanno-becker
added
enhancement
mbed TLS team
needs-review
hanno-becker
requested review from
k-stachowiak
and removed request for
RonEld
k-stachowiak
previously approved these changes
Oct 19, 2018
hanno-becker
requested review from
mpg
and removed request for
gilles-peskine-arm
mpg
previously approved these changes
Oct 29, 2018
|
Note: the issue fixed by this PR is labeled as a bug and the ChangeLog entry in under "Bugfix", so for consistency I'm relabeling this PR as "bug", hence "needs backports". |
mpg
added
bug
needs-backports
|
@hanno-arm Could you please investigate and fix the Ci failures too? Jenkins probably wants you to rebase on development so that it finds its config file, and Travis tells us |
hanno-becker
removed
the
needs-review
|
retest |
|
Failure in Travis CI: |
|
@sbutcher-arm I couldn't reproduce this so far, and the CI log doesn't give any information on what went wrong. Merged |
|
@mpg @k-stachowiak Could you please re-review this PR? |
|
@sbutcher-arm I don't know how |
mpg
previously approved these changes
Nov 5, 2018
k-stachowiak
previously approved these changes
Nov 5, 2018
|
retest |
Wasn't spotted earlier because it's guarded by `! HAVE_GETRANDOM`.
hanno-becker
force-pushed
the
iotssl-1948
branch
from
1535150 to
9772da8
Compare
|
@sbutcher-arm I removed the merge-commit as requested. |
|
@sbutcher-arm @mpg @k-stachowiak Removing the merge commit rewrote the history starting for the last trivial commit 9772da8. Could you please re-review? |
k-stachowiak
approved these changes
Nov 6, 2018
mpg
approved these changes
Nov 7, 2018
mpg
removed
needs-backports
simonbutcher
added
the
approved
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Context: This PR fixes #1212 related to platform-specific entropy polling in runs using qemu user emulation.
Summary: Previously, the implementation of the entropy gathering function
mbedtls_platform_entropy_poll()for linux machines used the following logic to determine how to obtain entropy from the kernel:getrandom()system call identifierSYS_getrandomis present and the kernel version is 3.17 or higher, usesyscall( SYS_getrandom, ... )/dev/random.There are two issues with this:
Portability: When cross-compiling the code for a different architecture and running it through system call emulation in qemu, qemu reports the host kernel version through
unamebut, as of v.2.5.0, doesn't support emulating thegetrandom()syscall. This leads tombedtls_platform_entropy_poll()failing even though reading from/dev/randomwould have worked.Complexity: Extracting the linux kernel version from the output of
unameis slightly tedious.This commit fixes both by implementing the suggestion in #1212:
syscall( SYS_getrandom, ... )fails witherrnoset toENOSYSindicating an unknown system call. If so, it falls through to trying to read from/dev/random.Fixes #1212.
Internal Reference: IOTSSL-1948.