From 8ec5a5aadeeef1d760ddb67d8b6f16452c873e25 Mon Sep 17 00:00:00 2001 From: Sam Clark <3758302+goatgoose@users.noreply.github.com> Date: Wed, 10 Jan 2024 11:15:28 -0500 Subject: [PATCH] Check fd status before using urandom --- tests/unit/s2n_random_test.c | 63 ++++++++++++++++++ utils/s2n_random.c | 123 ++++++++++++++++++++++++++++------- utils/s2n_random.h | 9 +++ 3 files changed, 173 insertions(+), 22 deletions(-) diff --git a/tests/unit/s2n_random_test.c b/tests/unit/s2n_random_test.c index 658de06a5cb..3243952e8bb 100644 --- a/tests/unit/s2n_random_test.c +++ b/tests/unit/s2n_random_test.c @@ -48,6 +48,12 @@ #define NUMBER_OF_RANGE_FUNCTION_CALLS 200 #define MAX_REPEATED_OUTPUT 4 +S2N_RESULT s2n_rand_get_dev_urandom(struct s2n_rand_device **device); +S2N_RESULT s2n_rand_device_validate(struct s2n_rand_device *device); +int s2n_rand_init_impl(void); +int s2n_rand_cleanup_impl(void); +int s2n_rand_urandom_impl(void *ptr, uint32_t size); + struct random_test_case { const char *test_case_label; int (*test_case_cb)(struct random_test_case *test_case); @@ -788,6 +794,62 @@ static int s2n_random_rand_bytes_after_cleanup_cb(struct random_test_case *test_ return S2N_SUCCESS; } +static int s2n_random_invalid_urandom_fd_cb(struct random_test_case *test_case) +{ + EXPECT_SUCCESS(s2n_disable_atexit()); + + for (size_t test = 0; test <= 1; test++) { + struct s2n_rand_device *dev_urandom = NULL; + EXPECT_OK(s2n_rand_get_dev_urandom(&dev_urandom)); + EXPECT_NOT_NULL(dev_urandom); + EXPECT_EQUAL(dev_urandom->fd, -1); + + /* Validation should fail before initialization. */ + EXPECT_ERROR(s2n_rand_device_validate(dev_urandom)); + + EXPECT_SUCCESS(s2n_init()); + + /* Override the mix callback with urandom, in case rdrand is supported. */ + EXPECT_SUCCESS(s2n_rand_set_callbacks(s2n_rand_init_impl, s2n_rand_cleanup_impl, s2n_rand_urandom_impl, s2n_rand_urandom_impl)); + + /* Validation should succeed after initialization. */ + EXPECT_OK(s2n_rand_device_validate(dev_urandom)); + + EXPECT_TRUE(dev_urandom->fd > STDERR_FILENO); + if (test == 0) { + /* Close the file descriptor. */ + EXPECT_EQUAL(close(dev_urandom->fd), 0); + } else { + /* Make the file descriptor invalid by pointing it to STDERR. */ + dev_urandom->fd = STDERR_FILENO; + } + + /* Validation should fail when the file descriptor invalid. */ + EXPECT_ERROR(s2n_rand_device_validate(dev_urandom)); + + s2n_stack_blob(rand_data, 16, 16); + EXPECT_OK(s2n_get_public_random_data(&rand_data)); + + uint64_t public_bytes_used = 0; + EXPECT_OK(s2n_get_public_random_bytes_used(&public_bytes_used)); + + if (s2n_is_in_fips_mode()) { + /* The urandom implementation should not be in use when s2n-tls is in FIPS mode. */ + EXPECT_EQUAL(public_bytes_used, 0); + } else { + /* When the urandom implementation is used, the urandom file descriptor is re-opened and + * validation should succeed. + */ + EXPECT_OK(s2n_rand_device_validate(dev_urandom)); + EXPECT_TRUE(public_bytes_used > 0); + } + + EXPECT_SUCCESS(s2n_cleanup()); + } + + return S2N_SUCCESS; +} + struct random_test_case random_test_cases[] = { { "Random API.", s2n_random_test_case_default_cb, CLONE_TEST_DETERMINE_AT_RUNTIME, EXIT_SUCCESS }, { "Random API without prediction resistance.", s2n_random_test_case_without_pr_cb, CLONE_TEST_DETERMINE_AT_RUNTIME, EXIT_SUCCESS }, @@ -800,6 +862,7 @@ struct random_test_case random_test_cases[] = { */ { "Test failure.", s2n_random_test_case_failure_cb, CLONE_TEST_DETERMINE_AT_RUNTIME, 1 }, { "Test libcrypto's RAND engine is reset correctly after manual s2n_cleanup()", s2n_random_rand_bytes_after_cleanup_cb, CLONE_TEST_DETERMINE_AT_RUNTIME, EXIT_SUCCESS }, + { "Test getting entropy with an invalid file descriptor", s2n_random_invalid_urandom_fd_cb, CLONE_TEST_DETERMINE_AT_RUNTIME, EXIT_SUCCESS }, }; int main(int argc, char **argv) diff --git a/utils/s2n_random.c b/utils/s2n_random.c index 78154d85a7e..49b0d9a4b2a 100644 --- a/utils/s2n_random.c +++ b/utils/s2n_random.c @@ -75,8 +75,6 @@ #include "utils/s2n_result.h" #include "utils/s2n_safety.h" -#define ENTROPY_SOURCE "/dev/urandom" - #if defined(O_CLOEXEC) #define ENTROPY_FLAGS O_RDONLY | O_CLOEXEC #else @@ -92,7 +90,10 @@ /* Placeholder value for an uninitialized entropy file descriptor */ #define UNINITIALIZED_ENTROPY_FD -1 -static int entropy_fd = UNINITIALIZED_ENTROPY_FD; +static struct s2n_rand_device s2n_dev_urandom = { + .source = "/dev/urandom", + .fd = UNINITIALIZED_ENTROPY_FD, +}; struct s2n_rand_state { uint64_t cached_fork_generation_number; @@ -115,16 +116,23 @@ static __thread struct s2n_rand_state s2n_per_thread_rand_state = { .drbgs_initialized = false }; -static int s2n_rand_init_impl(void); -static int s2n_rand_cleanup_impl(void); -static int s2n_rand_urandom_impl(void *ptr, uint32_t size); -static int s2n_rand_rdrand_impl(void *ptr, uint32_t size); +int s2n_rand_init_impl(void); +int s2n_rand_cleanup_impl(void); +int s2n_rand_urandom_impl(void *ptr, uint32_t size); +int s2n_rand_rdrand_impl(void *ptr, uint32_t size); static s2n_rand_init_callback s2n_rand_init_cb = s2n_rand_init_impl; static s2n_rand_cleanup_callback s2n_rand_cleanup_cb = s2n_rand_cleanup_impl; static s2n_rand_seed_callback s2n_rand_seed_cb = s2n_rand_urandom_impl; static s2n_rand_mix_callback s2n_rand_mix_cb = s2n_rand_urandom_impl; +int s2n_rand_entropy_fd_close_ptr(int *fd) { + if (fd && *fd != UNINITIALIZED_ENTROPY_FD) { + close(*fd); + } + return S2N_SUCCESS; +} + /* non-static for SAW proof */ bool s2n_cpu_supports_rdrand() { @@ -332,8 +340,84 @@ S2N_RESULT s2n_get_private_random_bytes_used(uint64_t *bytes_used) return S2N_RESULT_OK; } -static int s2n_rand_urandom_impl(void *ptr, uint32_t size) +S2N_RESULT s2n_rand_get_dev_urandom(struct s2n_rand_device **device) +{ + RESULT_ENSURE_REF(device); + RESULT_ENSURE(s2n_in_unit_test(), S2N_ERR_NOT_IN_UNIT_TEST); + *device = &s2n_dev_urandom; + return S2N_RESULT_OK; +} + +S2N_RESULT s2n_rand_device_validate(struct s2n_rand_device *device) +{ + RESULT_ENSURE_REF(device); + RESULT_ENSURE_NE(device->fd, UNINITIALIZED_ENTROPY_FD); + + /* Ensure that the random device is still valid by comparing it to the current file descriptor + * status. From: + * https://github.com/openssl/openssl/blob/260d97229c467d17934ca3e2e0455b1b5c0994a6/providers/implementations/rands/seeding/rand_unix.c#L513 + */ + struct stat st = { 0 }; + RESULT_ENSURE(fstat(device->fd, &st) == 0, S2N_ERR_OPEN_RANDOM); + RESULT_ENSURE_EQ(device->dev, st.st_dev); + RESULT_ENSURE_EQ(device->ino, st.st_ino); + RESULT_ENSURE_EQ(device->rdev, st.st_rdev); + + /* Ensure that the mode is the same (equal to 0 when xor'd), but don't check the permission bits. */ + mode_t permission_mask = ~(S_IRWXU | S_IRWXG | S_IRWXO); + RESULT_ENSURE_EQ((device->mode ^ st.st_mode) & permission_mask, 0); + + return S2N_RESULT_OK; +} + +S2N_RESULT s2n_rand_device_open(struct s2n_rand_device *device) +{ + RESULT_ENSURE_REF(device); + RESULT_ENSURE_REF(device->source); + + DEFER_CLEANUP(int fd = -1, s2n_rand_entropy_fd_close_ptr); + while (fd < 0) { + fd = open(device->source, ENTROPY_FLAGS); + if (fd < 0 && errno != EINTR) { + RESULT_BAIL(S2N_ERR_OPEN_RANDOM); + } + } + + struct stat st = { 0 }; + RESULT_ENSURE(fstat(fd, &st) == 0, S2N_ERR_OPEN_RANDOM); + device->dev = st.st_dev; + device->ino = st.st_ino; + device->mode = st.st_mode; + device->rdev = st.st_rdev; + + device->fd = fd; + + /* Disable closing the file descriptor with defer cleanup */ + fd = UNINITIALIZED_ENTROPY_FD; + + return S2N_RESULT_OK; +} + +S2N_RESULT s2n_rand_device_get_valid_fd(struct s2n_rand_device *device, int *fd) { + RESULT_ENSURE_REF(device); + RESULT_ENSURE_REF(fd); + + if (s2n_result_is_error(s2n_rand_device_validate(device))) { + RESULT_GUARD(s2n_rand_device_open(device)); + } + + *fd = device->fd; + + return S2N_RESULT_OK; +} + +int s2n_rand_urandom_impl(void *ptr, uint32_t size) +{ + POSIX_ENSURE_REF(ptr); + + int entropy_fd = UNINITIALIZED_ENTROPY_FD; + POSIX_GUARD_RESULT(s2n_rand_device_get_valid_fd(&s2n_dev_urandom, &entropy_fd)); POSIX_ENSURE(entropy_fd != UNINITIALIZED_ENTROPY_FD, S2N_ERR_NOT_INITIALIZED); uint8_t *data = ptr; @@ -450,16 +534,9 @@ RAND_METHOD s2n_openssl_rand_method = { }; #endif -static int s2n_rand_init_impl(void) +int s2n_rand_init_impl(void) { -OPEN: - entropy_fd = open(ENTROPY_SOURCE, ENTROPY_FLAGS); - if (entropy_fd == -1) { - if (errno == EINTR) { - goto OPEN; - } - POSIX_BAIL(S2N_ERR_OPEN_RANDOM); - } + POSIX_GUARD_RESULT(s2n_rand_device_open(&s2n_dev_urandom)); if (s2n_cpu_supports_rdrand()) { s2n_rand_mix_cb = s2n_rand_rdrand_impl; @@ -502,12 +579,14 @@ S2N_RESULT s2n_rand_init(void) return S2N_RESULT_OK; } -static int s2n_rand_cleanup_impl(void) +int s2n_rand_cleanup_impl(void) { - POSIX_ENSURE(entropy_fd != UNINITIALIZED_ENTROPY_FD, S2N_ERR_NOT_INITIALIZED); + POSIX_ENSURE(s2n_dev_urandom.fd != UNINITIALIZED_ENTROPY_FD, S2N_ERR_NOT_INITIALIZED); - POSIX_GUARD(close(entropy_fd)); - entropy_fd = UNINITIALIZED_ENTROPY_FD; + if (s2n_result_is_ok(s2n_rand_device_validate(&s2n_dev_urandom))) { + POSIX_GUARD(close(s2n_dev_urandom.fd)); + } + s2n_dev_urandom.fd = UNINITIALIZED_ENTROPY_FD; return S2N_SUCCESS; } @@ -575,7 +654,7 @@ S2N_RESULT s2n_set_private_drbg_for_test(struct s2n_drbg drbg) * volatile is important to prevent the compiler from * re-ordering or optimizing the use of RDRAND. */ -static int s2n_rand_rdrand_impl(void *data, uint32_t size) +int s2n_rand_rdrand_impl(void *data, uint32_t size) { #if defined(__x86_64__) || defined(__i386__) struct s2n_blob out = { 0 }; diff --git a/utils/s2n_random.h b/utils/s2n_random.h index c0ad048bf27..72237054818 100644 --- a/utils/s2n_random.h +++ b/utils/s2n_random.h @@ -19,6 +19,15 @@ #include "utils/s2n_blob.h" #include "utils/s2n_result.h" +struct s2n_rand_device { + const char *source; + int fd; + dev_t dev; + ino_t ino; + mode_t mode; + dev_t rdev; +}; + S2N_RESULT s2n_rand_init(void); S2N_RESULT s2n_rand_cleanup(void); S2N_RESULT s2n_get_seed_entropy(struct s2n_blob *blob);