Skip to content

Security updates into master - #51

Merged
DannyArends merged 10 commits into
masterfrom
development
Sep 5, 2026
Merged

DannyArends merged 10 commits into
masterfrom
development

Conversation

@DannyArends

Copy link
Copy Markdown
Owner

Security hardening, per-IP throttling fix, SSL unit tests, and an OpenSSL CI build with caching. 10 commits, 9 files changed, no files added or removed.

Summary

This branch closes four remotely reachable issues in the request and connection path, fixes an SSL build error, adds the first SSL unit tests, and stands up an OpenSSL build in CI so the ssl configuration is actually compiled and tested. It also includes small, behaviour-neutral refactors in the driver and accept path.

Security fixes (remotely exploitable)

CGI input-file protocol injection. The per-request .in file uses a line-based TYPE=KEY=VALUE protocol that the CGI side parses into trusted SERVER, POST, COOKIE, and FILES entries. Client-supplied POST field names and values (and the multipart filename and mime) were written into it without escaping, so a URL-decoded value containing a newline could inject additional protocol lines and forge variables such as REMOTE_ADDR, HTTPS, or SCRIPT_FILENAME. CR and LF are now stripped from these fields before they are written. Single-line values, which is everything that worked before, are unaffected.

httpoxy (CVE-2016-5385 class). Every request header was mapped into the CGI environment as HTTP_*, so a Proxy: request header became HTTP_PROXY and could redirect a script's outbound HTTP traffic. The Proxy header is now skipped when building the environment. No legitimate client sends it.

Host-header path traversal. The Host header flows through shorthost() into the filesystem root without hostname validation, so a Host containing .. reached path construction. Routing now rejects a shorthost containing .. or a null byte with the same response as an unknown domain. No valid hostname contains either.

Per-IP connection cap was not enforced. The per-IP counter was keyed on driver.ip, which returns 0.0.0.0 until openConnection runs, while the admission check keyed on the real peer IP, so the check always read zero and the 32-per-IP limit never fired. A single IP could fill the 2048-slot global queue and starve other clients. The peer address is now set on the driver at accept time, and the count is incremented at enqueue under the same lock as the check, so queued plus active connections per real IP are bounded. This also closes the check-then-increment race.

TLS / build fix

SSL_pending const mismatch. hasBuffered() is const, so its ssl pointer is const(SSL*), which could not bind to the ImportC binding's mutable ssl_st* parameter and broke the ssl build. The call now passes cast(SSL*) ssl. The underlying C function is genuinely const, so the cast asserts const-correctness the binding dropped rather than hiding anything. It is the only const method that touches an SSL function, so no other call sites change.

Tests

SSL unit tests added. The previously empty ssl.d unittest now covers SNI context matching (findContext / hasCertificate), including a case that documents the current endsWith suffix match accepting a look-alike host, and exercises generateKey against libcrypto (2048-bit keygen, PEM written and checked). These run only under dub test --config=ssl.

CI: build and cache OpenSSL

The ssl configuration was never compiled in CI because dub test builds the default configuration. CI now:

  • checks out submodules recursively and installs build-essential and perl,
  • builds the OpenSSL submodule (./Configure linux-x86_64 no-tests then make -j),
  • caches the built tree with actions/cache@v5, keyed on the pinned OpenSSL submodule commit, so the build is skipped on a cache hit,
  • runs Build and Test as separate steps, each covering the default and the --config=ssl pass.

Refactors (behaviour-neutral)

  • Driver ip and port are resolved once when the peer address is set and returned from cached fields, instead of re-parsing the address on every call.
  • Driver creation in accept() is a single ternary, with HTTPS aliased to HTTP in the non-SSL build so it compiles in both configurations without a static if or a manifest constant. secure is never true in a non-SSL build, so the alias is never constructed.

@DannyArends
DannyArends merged commit b111a72 into master Sep 5, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant