Improve fork detection

[torben-hansen]

Security issue notifications

If you discover a potential security issue in s2n we ask that you notify
AWS Security via our vulnerability reporting page. Please do not create a public github issue.

Problem:

Practical impact from identified issues

s2n has always-on prediction resistance for the CTR-DRBG. This means that the thread-local drbg states are randomised every time one is used to generate randomness. The randomisation occurs before generating randomness. Hence, there is no direct practical impact as a result of the issues described in this document.

Why care then?
Fork detection is a defense-in-depth mechanism. And, of course, if prediction resistance is ever relaxed, a false assumption could be made that fork detection works in a complete way.

Problem

s2n employs two different mechanisms to detect forks: MADV_WIPEONFORK and pthread_atfork(). The former is more comprehensive than the latter. For example, pthread_atfork() doesn’t execute the registered child callback if a fork is performed directly through e.g. clone().

We first describe how fork detection is implemented in s2n and then describe two different issues:

• Issue 1: MADV_WIPEOFORK is never (in fact, can never be) enabled as currently implemented.
• Issue 2: the fork detection implementation does not randomise the drbg state in all fork cases.

Fork detection in s2n

Fork detection using MADV_WIPEONFORK is a stateful system. s2n inspects a special memory page that is managed by the kernel. The kernel zeroises the memory page if it detects a fork. By inspecting the memory page, s2n can detect a fork and execute a drbg state randomisation in the forked process. Below is a description of how this is implemented in s2n.

Global memory is used by s2n as the special memory page zero_when_forked_page, which either has the value 0 (signals a fork has occurred) and 1 (signals no action needed). A thread-local pointer zero_if_forked_ptr holds a reference to zero_when_forked_page.

The second fork detection mechanism uses pthread_atfork() that registers a callback s2n_on_fork. The callback function zeroises zero_when_forked_page directly:
https://github.com/aws/s2n-tls/blob/main/utils/s2n_random.c#L131

void s2n_on_fork(void)
{
    zero_if_forked = 1;
}

zero_if_forked is a convenience macro that dereferences zero_if_forked_ptr.

Issue 1: MADV_WIPEONFORK is never (can never be) enabled

MADV_WIPEONFORK is instantiated as follows:

pagesize = s2n_mem_get_page_size();

/* We need a single-aligned page for our protected memory region */
RESULT_ENSURE(posix_memalign(&zeroed_when_forked_page, pagesize, pagesize) == S2N_SUCCESS, S2N_ERR_OPEN_RANDOM);

#if defined(MADV_WIPEONFORK)
    RESULT_ENSURE(madvise(zeroed_when_forked_page, pagesize, MADV_WIPEONFORK) == S2N_SUCCESS, S2N_ERR_OPEN_RANDOM);
#endif

There are 3 distinct sub-issues (tested on AL2 and Ubuntu 20.04).

madvise and MADV_WIPEONFORK's header file not included

The header file defining MADV_WIPEONFORK and declaring the function madvise() is not included in the s2n_random.c compilation unit. Hence

#if defined(MADV_WIPEONFORK)
    RESULT_ENSURE(madvise(zeroed_when_forked_page, pagesize, MADV_WIPEONFORK) == S2N_SUCCESS, S2N_ERR_OPEN_RANDOM);
#endif

is never executed.

Solution

Include the header file ys/mman.h (https://man7.org/linux/man-pages/man2/madvise.2.html). Noting that not all systems define MADV_WIPEONFORK in this header file.

Feature test macro requirements not met

Including the header file sys/mman.h is actually not enough, because there are also feature test macro requirements for madvise().

https://man7.org/linux/man-pages/man2/madvise.2.html

       madvise():
           Since glibc 2.19:
               _DEFAULT_SOURCE
           Up to and including glibc 2.19:
               _BSD_SOURCE

This is not defined in s2n_random.c.

Solution

The solution is to define a feature test macro. In addition, define the pre-processor directive MADV_WIPEFORK if kernel headers doesn’t define it as expected; kernel should support the operation anyway. Something like this worked in my small test - but should be verified more thoroughly:

#define _GNU_SOURCE 1
#include <sys/mman.h>

#if defined(MADV_WIPEONFORK)
    static_assert(MADV_WIPEONFORK == 18, "MADV_WIPEONFORK is not 18");
#else
    #define MADV_WIPEONFORK 18
#endif

The above implementation is not strictly C99 because of the use of static_assert().

MADV_WIPEOFORK tagged memory page is not necessarily private and anonymous

MADV_WIPEFORK requires:
https://man7.org/linux/man-pages/man2/madvise.2.html

The MADV_WIPEONFORK operation can be applied only to private anonymous pages (see mmap(2)).

However, this is not explicitly satisfied in the implementation. This doesn’t seem to be an issue on the platforms I tested on, though. It does seem to be verified by the Linux Kernel though https://elixir.bootlin.com/linux/v5.11-rc6/source/mm/madvise.c#L98.

Solution

mmap can be used to produce aligned memory with private MAP_PRIVATE and anonymous MAP_ANONYMOUS pages.

An implementation could be:

  page_size = s2n_mem_get_page_size();
  addr = mmap(NULL, (size_t)page_size, PROT_READ | PROT_WRITE,
                    MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);

where addr points to the memory page that is going to be tagged with MADV_WIPEONFORK.

Issue 2: current fork detection mechanism implementation does not cover all fork use-cases

First we make two observations about the fork detection implementation:

• Each thread uses the same global memory zero_when_forked_page referenced through thread-local pointers zero_if_forked_ptr to signal whether a fork has occurred.
• If a fork is detected, the thread that first reaches the end of s2n_defend_if_forked (unconditionally) resets zero_when_forked_page back to a “not detected” state. There is no synchronisation.

Now, let's say we have a process P1 that has an initialised s2n drbg:

1. Fork P1. This creates a new child process P2 of P1. We now have one thread in P2: call this thread T1.
2. Because of fork detection, this sets *zero_when_forked_page to 0 in T1. The s2n drbg states in P1 and T1 are duplicated at this point in time.
3. Spawn a new thread T2 in P2. Note that we have *zero_if_forked_ptr = 0 in T2.
4. In T2 make a call to s2n_get_{public,private}_random_data which will initialise the thread-local drbg states in T2.
5. After initilisation in T2, the memory page *zero_when_forked_page is set to 1 and the thread-local pointer zero_if_forked_ptr points to zero_when_forked_page.

Any subsequent calls to s2n_get_{public,private}_random_data() by T1 (in P2) will not randomise the thread-local drbg states because the global memory is now *zero_when_forked_page != 0.

P1 and T2 have duplicated drbg states.

Tiny PoC: looking at output from calling s2n_get_public_random_data() in T2, T1, and P1.

./test_fork_thread 

Label: [P2] Fork child new thread (T2)
drbg output: f3 82 b 8d ec f5 b6 ad 2f ea
 
Label: [P2] Fork child (T1)
drbg output: b4 36 86 5d b5 b5 5 3c 7f b1 

Label: [P1]
drbg output: b4 36 86 5d b5 b5 5 3c 7f b1 

This was tested after fixing other issues with the MADV_WIPEONFORK implementation

Solution:

Implement fork generation number that abstracts the fork detection and provides a synchronisation mechanisms that lets each thread handle forking events.

• Does this change what S2N sends over the wire? No
• Does this change any public APIs? No
• Which versions of TLS will this impact? Not TLS-specific

[torben-hansen]

Issues resolved by the following series of commits:

• 31d99d2
• 5739bcd
• 8a01fa5
• 247bf31
• 8314a96
• da03bc9
• ac56bca
• 9b7761b
• f1c121e

These commits entails:

• Replacing fork detection with a new method using a fork generation number.
• Expanding testing facilities.
• Bug fixes.