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

s2n-tls does not check if entropy_fd is still open #4324

Closed
matshch opened this issue Dec 13, 2023 · 1 comment · Fixed by #4352
Closed

s2n-tls does not check if entropy_fd is still open #4324

matshch opened this issue Dec 13, 2023 · 1 comment · Fixed by #4352

Comments

@matshch
Copy link

matshch commented Dec 13, 2023

Problem:

Some programs close all non-standard file descriptors when creating a fork process to make some kind of a sandbox and to limit the attack surface. Unfortunately, this forces s2n-tls to stuck in the s2n_rand_urandom_impl loop if the library was already initialized in the parent process and when the child process tries to establish TLS connection, because the library expects entropy_fd to still be a valid file descriptor pointing to the /dev/urandom. In the best case the process is just stuck forever receiving EBADF errors, in the worst case RNG is initialized with some data stolen from another file descriptor.

Solution:

There are several ways to fix this:

Implications of each solution should be evaluated and the best fitting one should be implemented.

  • Does this change what S2N sends over the wire? It changes which random numbers are used in cryptography in the described case of the file descriptor misuse (or if something is send at all). If /dev/urandom file descriptor is not closed by the code outside of this library, nothing changes.
  • Does this change any public APIs? No.
  • Which versions of TLS will this impact? Any.

Requirements / Acceptance Criteria:

s2n-tls should properly process errors during read of entropy_fd.

Users of the library should understand how to safely close file descriptors if they are required to take some action to fix this issue.

@maddeleine
Copy link
Contributor

Thanks for opening this issue. We'll look into it and see what we can do to fix this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants