Security hardening: enforce connection limit, sanitise errors, harden TLS#304
Open
balat wants to merge 7 commits into
Open
Security hardening: enforce connection limit, sanitise errors, harden TLS#304balat wants to merge 7 commits into
balat wants to merge 7 commits into
Conversation
The <maxconnected> limit was tracked but never applied: the accept loop ignored it. Drive conduit's global accept throttle with the configured value so the limit takes effect again.
handle_error returned Printexc.to_string of the exception as the response body for every status except 404, disclosing exception constructor names and arguments (file paths, Unix_error details, ...) to clients. Return the generic status reason phrase instead, or the explicit message an extension exposes through Ext_http_error. The exception detail stays in the logs.
The control command pipe was group-writable (0o660), letting any local user in the server's group inject commands (shutdown, reload, clearcache). Only the server's own user needs to write to it.
Require TLS 1.2 at minimum (TLS 1.0/1.1 deprecated, RFC 8996) and prefer the server's cipher order on conduit's OpenSSL server context. Also apply the <ciphers>, <dhfile> and <curve> entries of the <ssl> configuration, which were parsed but never passed to the TLS context.
- Document the TLS 1.2 minimum and the <ciphers>/<dhfile>/<curve> options, and replace the obsolete 1024-bit key advice. - Add an HTTP->HTTPS redirect recipe using Redirectmod and the <ssl/> condition of Accesscontrol. - State honestly that <timeout> is currently not enforced (no per-connection idle timeout), that the server does not defend against Slowloris on its own, and recommend a reverse proxy for Internet-facing deployments. Remove the documentation of the non-implemented <keepalivetimeout> option.
Self-contained cram tests (POSIX sh, no bash helper) for the security hardening: - security.t: the command pipe is created 0o600 (F8); a handler error returns a generic body, never the OCaml exception (F4). - security-tls.t: an HTTPS listener accepts TLS 1.2 and refuses TLS 1.1 (F5).
The manual claimed the server drops privileges when launched as root. It no longer does: <user>/<group> are deprecated and ignored. Document this, advise against running as root, and list the supported ways to serve ports 80/443 as an unprivileged user. Remove the misleading <user>/<group> lines from the sample configurations.
e46f173 to
bb621ad
Compare
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
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.
Security hardening for closer-to-Internet exposure
Following a security audit focused on the question "can ocsigenserver be exposed
directly to the Internet, or does it still need a reverse proxy?". The honest
answer remains: keep a reverse-proxy / TLS-terminating front for now (it
supplies read timeouts, TLS hardening and ACME). But several genuine gaps were
fixed, where existing configuration was silently ignored.
Fixes
<maxconnected>(default 350) wastracked but never applied; the accept loop ignored it. It now drives
conduit's global accept throttle.
handle_errorreturnedPrintexc.to_string exnas the body for every status except 404, disclosingexception constructor names and arguments (file paths,
Unix_error, ...).Clients now get the generic HTTP status reason phrase (or an extension's
explicit
Ext_http_errormessage); the detail stays in the logs.RFC 8996) and prefer the server's cipher order. The
<ciphers>,<dhfile>and
<curve>entries of<ssl>, until now parsed but never applied, are nowhonoured.
0o600. Was0o660(group-writable), letting any localuser in the server's group inject control commands (shutdown, reload, ...).
Documentation
advice) and an HTTP→HTTPS redirect recipe (Redirectmod +
<ssl/>condition).<timeout>is currently not enforced: there is noper-connection idle timeout, so the server does not defend against Slowloris
on its own. A reverse proxy is recommended for Internet-facing deployments.
Removed the documentation of the non-implemented
<keepalivetimeout>.<user>/<group>section, which falsely claimed the serverdrops privileges when run as root. They are deprecated and ignored; the doc
now advises against running as root and lists ways to serve 80/443
unprivileged (reverse proxy /
cap_net_bind_service/ port redirection).Tests
Self-contained cram tests (POSIX sh): command pipe
0o600, generic error body,and an HTTPS handshake accepting TLS 1.2 / refusing TLS 1.1.
Not addressed (intentionally)
total-connection-lifetime cap, which would break comet/long-poll and slow
large downloads. A real idle timeout needs upstream cohttp support; documented
as a limitation instead.
validation remain backlog.