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

Improve documentation about fork safety #6968

Closed
piru opened this issue Apr 26, 2021 · 5 comments
Closed

Improve documentation about fork safety #6968

piru opened this issue Apr 26, 2021 · 5 comments

Comments

@piru
Copy link

piru commented Apr 26, 2021

libcurl Security Considerations documentation could be improved to include section about fork safety.

Currently if libcurl is used in with NSS backend, fork() will effectively clone the random generator state. Unlike openssl and friends, NSS doesn't automatically reseed the RNG when the process id changes. This means that NSS will generate same random byte stream for both forks, possibly leading to identical temporary keys and/or tokens being used. In some scenarios this could lead to security vulnerability being introduced.

At least in my testing calling SECMOD_RestartModules(PR_TRUE); in the child after fork() seemed a reliable way to reseed the RNG.

It is a bit unfortunately that such backend specific code needs to be used, but as far as I can tell there is no easy way to have this fixed in a generic way, at least if libcurl doesn't want to add some magic that tries to identify fork()ing.

Note that some arc4random() implementations might be affected in a similar manner. However, this is implementation specific. For example libbsd commonly used in linux distros appears to be fork safe. MacOS arcrandom() is safe. I didn't test iOS one, but I'd assume it's safe as well.

@piru
Copy link
Author

piru commented Apr 26, 2021

It was pointed out that SECMOD_RestartModules might be resetting too much - https://bugzilla.redhat.com/800304
guess we'd need to figure out how to properly reseed the RNG before we can give advice.

@bagder
Copy link
Member

bagder commented May 5, 2021

@piru do you have any suggested wording for this? I feel I'm not that educated on the details to do a good job here.

@piru
Copy link
Author

piru commented May 5, 2021

Here's a good resource with some links to some other projects with a similar issues:

I guess it could be something along the lines of:

The random number generator used by your selected SSL backed may not be fork() safe. This means that upon fork(), both the parent and the child process share the same internal random number generator state. Effectively this means that both forks will have identical random numbers generated for them, if same operations are performed by both forks. This can lead to security issues if same key material is generated for connections towards different security context (effectively it is possible that attacker could predict some tokens and keys generated for other connections).

To remedy this, you may need to reseed the random number generator in the child after fork(). The current understanding is that at least the NSS ssl backend is affected by this.

Of course the issue here is that I currently have no clue what the correct method for reseeding for NSS really is. I also am not 100% certain that all other backends are safe. openssl (+ derivates very likely) and GnuTLS are not affected by this.

@jay
Copy link
Member

jay commented May 5, 2021

I don't think being able to fork() is something we should guarantee, it just seems to iffy to me. libcurl has various dependencies which may not be fork safe and I don't even know that it is on its own. Practically I guess you could reinitialize by global cleanup+init, however you can't really be sure it has reinitialized unless you know everywhere curl_global_init is called. For example, if curl_global_init is called twice it will reference count (initialization is 2) and then you fork and you call curl_global_cleanup (initialization is 1 so it doesn't actually deinit) + curl_global_init (initialization is 2 again) it's not going to reinitialize.

@jay jay added the libcurl API label May 5, 2021
@piru
Copy link
Author

piru commented May 6, 2021

Just documenting that libcurl doesn't guarantee correct behaviour if fork() is used after curl_global_init is fine as well. Then it would be up to whoever attempts that anyway to make it work in their use case.

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

No branches or pull requests

3 participants