Repository navigation
Conversation
modsecurity_init() calls curl_global_init(CURL_GLOBAL_ALL) on every (re)start and nothing ever called curl_global_cleanup(). Each graceful restart therefore initialised libcurl's TLS backend once more on top of the previous state. With a GnuTLS backed libcurl that leaks the random source set up per init, and after a few thousand reloads the process fails with GnuTLS errors until a full restart. curl_global_cleanup() is now registered as a cleanup of the configuration pool that was passed to modsecurity_init(), so each init is released when that configuration goes away. The early version that cleaned up per utilisation broke mod_ssl with OpenSSL before 1.1.0; tying it to the configuration pool keeps one init/cleanup pair per configuration generation, the same lifetime mod_ssl's own state has. Fixes owasp-modsecurity#3590 Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0181jBv5JdNQviwDzfnkf1Gy
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configuration
You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
This branch has not been deployed
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
What
modsecurity_init()(apache2/modsecurity.c) still callscurl_global_init(CURL_GLOBAL_ALL)on every (re)start, but now registerscurl_global_cleanup()as a cleanup of the configuration pool it was given, so every init is released when that configuration generation goes away. The old comment explaining why there was no cleanup is replaced by one explaining the pairing.Why
Nothing in the module ever called
curl_global_cleanup(), so each graceful restart initialised libcurl's TLS backend once more on top of the previous state. With a GnuTLS-backed libcurl (the reporter's build) the resources set up per init are never released, and #3590 reports the process failing with GnuTLS errors after a few thousand reloads until a full restart.The early implementation that cleaned up per utilisation broke mod_ssl with OpenSSL before 1.1.0, which is why the cleanup was removed. Tying it to the configuration pool instead keeps exactly one init/cleanup pair per configuration generation, the same lifetime mod_ssl's own state has, and modern OpenSSL/GnuTLS no longer tear down global state on the last
curl_global_cleanup().Tests
No regression test: the harness cannot drive restarts. Verified in Debian bookworm containers (apache2 2.4.62, prefork, mod_ssl loaded on a TLS vhost, one
@containsdeny rule) with a graceful-restart loop that checks HTTP, HTTPS and a blocked request afterwards and grepserror.logforsegfault|gnutls|exit signal|curl:v2/masterSo the reported failure does not reproduce with Debian's GnuTLS 3.7.9 within 3000 reloads; the reporter's environment (older GnuTLS, thousands of reloads over weeks) could not be matched here. What the runs establish is that the pairing is safe with mod_ssl loaded under both curl backends. The change itself follows from the missing cleanup in the code, not from a local reproduction, and the issue stays marked as pending confirmation from a build with this change in the reporter's setup.
References
Fixes #3590
🤖 Generated with Claude Code