A session that reconnects several times under the same cookie (roaming,
DPD, a new-tunnel rekey) currently reports an Acct-Session-Time far
larger than its real duration: each reconnected segment reports uptime
measured cumulatively from the original session start, and sec-mod sums
these cumulative per-segment values instead of taking the final one.
Add a test that drives three cookie-resumed segments (simulating
reconnects via SIGKILL, as tests/test-cookie-timeout does, so the
session survives between segments) with idle gaps in between, then
performs a clean final disconnect (SIGTERM), which ocserv reports as an
explicit user disconnect and closes the accounting session immediately.
It checks the Stop record's Acct-Session-Time is close to the real
elapsed wall-clock time rather than the sum of the individual segments'
cumulative uptimes.
This test currently fails against unpatched master.
Signed-off-by: Nikos Mavrogiannopoulos <n.mavrogiannopoulos@gmail.com>
When a certificate contains a single group there is no need to
request the user to select. Auto-select the group.
Resolves: #692
Signed-off-by: Nikos Mavrogiannopoulos <n.mavrogiannopoulos@gmail.com>
Group names are stored in fixed-size (MAX_GROUPNAME_SIZE) worker
buffers, so a configured select-group entry that does not fit can
never be matched against a client-supplied group name. Warn the
administrator about such entries at config-load time, similar to
other configuration sanity checks in check_cfg().
Signed-off-by: Nikos Mavrogiannopoulos <n.mavrogiannopoulos@gmail.com>
DATA_MTU(ws, mtu) performs unsigned subtraction. A client advertising
X-CSTP-Base-MTU or X-CSTP-MTU smaller than the combined DTLS overhead
(IP + UDP + DTLS record + crypto) causes the result to wrap to ~UINT_MAX,
which then reaches memset(), tun_read(), and IPC calls.
Fix: enforce MIN_MTU(ws) (800 for IPv4, 1280 for IPv6) as the lower
bound when accepting client-supplied link_mtu and tunnel_mtu values.
Values below the floor are logged and ignored; the server's own MTU
is used instead. Add a defense-in-depth lower-bound check in
link_mtu_set() and a runtime assert after calc_mtu_values() that fires
in CI if a future cipher or protocol change erodes the safety margin.
Resolves: #717
Signed-off-by: Nikos Mavrogiannopoulos <n.mavrogiannopoulos@gmail.com>
Sending a Cookie header with a webvpncontext= value whose base64-decoded
length exceeds SID_SIZE crashes the worker with SIGSEGV before it can
send an HTTP response. The test detects both pre- and post-response
crashes: a connection reset (HTTP 000) and a "died with sigsegv" entry
in the server log respectively.
Relates: #719
Signed-off-by: Nikos Mavrogiannopoulos <n.mavrogiannopoulos@gmail.com>
Add test-pam-abort, which verifies that pam_auth_deinit() correctly
resumes a suspended PAM coroutine before calling pam_end().
The test posts a username-only HTTP request to trigger SEC_AUTH_INIT,
leaving the PAM coroutine suspended in PAM_S_WAIT_FOR_PASS while the
worker exits without sending a password. The stale pre-auth entry is
cleaned up by the sec-mod maintenance cycle (driven by the new
sec-mod-db-cleanup-time config knob, set to 3 s in the test config).
Bug detection is provided by pam_abort_test.so, a small PAM module that
registers a pam_set_data() cleanup which calls abort() if pam_end() fires
while conv->conv() has not yet returned. Without the fix, sec-mod would
abort and the subsequent authentication check would fail.
Relates: #741
Signed-off-by: Nikos Mavrogiannopoulos <n.mavrogiannopoulos@gmail.com>
Certain pam modules such as pam_sss with AD/Kerberos and multi-factor
authentication requires significantly more stack than the previous
1 MB limit.
On Linux, allocate the coroutine stack with mmap and place a PROT_NONE
guard page immediately below it. This turns a stack overflow into an
immediate SIGSEGV rather than silent corruption of adjacent heap memory
to better detect similar cases.
Fixes: #657
Relates: #619
Signed-off-by: Nikos Mavrogiannopoulos <n.mavrogiannopoulos@gmail.com>
ocserv always logged to the syslog daemon(3) facility, making it
impossible to route its messages separately from other daemons.
Adds a syslog-facility config key (and --syslog-facility CLI flag)
accepting daemon/user/auth/authpriv/local0-local7; defaults to daemon.
Resolves: #691
Signed-off-by: Nikos Mavrogiannopoulos <n.mavrogiannopoulos@gmail.com>
Connects an openconnect client, floods ICMP traffic so the server
accumulates ICMP replies in cstp_send(), then freezes the client with
SIGSTOP to prevent the TCP receive buffer from draining. Asserts that
the worker session disappears from occtl within DEFAULT_SOCKET_TIMEOUT
plus margin, which would never happen with the old infinite retry loop.
Relates: #638
Signed-off-by: Nikos Mavrogiannopoulos <n.mavrogiannopoulos@gmail.com>
ICMPv6 was never excluded from idle-timeout accounting because is_data()
inspected data[9] for both IPv4 and IPv6. Offset 9 is the Protocol field
in the IPv4 fixed header (RFC 791), but the Next Header field in the IPv6
fixed header sits at offset 6 (RFC 8200 §3). As a result, the idle timer
was reset on every ICMPv6 packet, preventing the server from disconnecting
clients that were sending only control traffic.
Resolves: #724
Signed-off-by: Nikos Mavrogiannopoulos <n.mavrogiannopoulos@gmail.com>
This removes certificate and CA handlers not used by the openconnect
client. This is a hardening measure to further reduce the attack surface
of the worker process.
Signed-off-by: Nikos Mavrogiannopoulos <n.mavrogiannopoulos@gmail.com>
When two HTTP requests arrived in the same TLS read buffer, a single
llhttp_execute() call would fire callbacks for both requests inline.
Because http_req_reset() is not called between them, ws->req ended up
reflecting the second request's URL and headers, silently discarding
the first. In the worst case, body bytes from the first request
accumulated alongside the second request's body.
Fix this by registering an on_message_begin callback that returns
HPE_PAUSED when an existing message is detected.
Resolves: #716
Signed-off-by: Nikos Mavrogiannopoulos <n.mavrogiannopoulos@gmail.com>
Bound memory growth in the worker for unauthenticated connections by
enforcing HTTP headers limit in addition to HTTP body limit.
Resolves: #712
Signed-off-by: Nikos Mavrogiannopoulos <n.mavrogiannopoulos@gmail.com>
ipcalc was used only to convert dotted-decimal subnet masks to CIDR
prefix lengths (e.g. 255.255.0.0 -> 16), replaced with a POSIX shell
script.
Relates: #709
Signed-off-by: Nikos Mavrogiannopoulos <n.mavrogiannopoulos@gmail.com>
When processing a RADIUS Access-Accept with Framed-IPv6-Prefix, the code
passed the wrong value for it. Corrected by passing the actual prefix.
Fixes: #710
Signed-off-by: Nikos Mavrogiannopoulos <n.mavrogiannopoulos@gmail.com>
Introduce a protobuf-generated ReloadableConfig (cfg.proto) to hold all
fields that reload on SIGHUP, and separate them from static_cfg_st, which
holds fields that require a server restart. Named vhosts inherit from the
default vhost via a pack/unpack round-trip. Adding a new config field
only requires editing cfg.proto. Struct and accessor names (ReloadableConfig,
static_cfg_st, GETRCONFIG, GETSCONFIG) now reflect each field's lifetime.
A new unit test covers the full inheritance path.
Resolves: #705
Signed-off-by: Nikos Mavrogiannopoulos <n.mavrogiannopoulos@gmail.com>
Per proxy protocol v2 spec §2.2.6, PP2_SUBTYPE_SSL_CN (0x22) is a
sub-TLV inside the PP2_TYPE_SSL body, not a top-level TLV in the TLV
stream. The previous code looked for 0x22 at the top level, where
haproxy never sends it, so client certificate CN was never extracted
via proxy protocol.
Fix parse_ssl_tlvs() to scan the bytes after the fixed pp2_tlv_ssl
header as a nested sub-TLV loop when cert_auth_ok is set.
Also fix htons() -> ntohs() for the TLV length byte-swap (functionally
identical but semantically correct for a network-to-host conversion),
and update the misleading comment that claimed the field was
little-endian.
Add tests/proxyproto-v2.c, a unit test that feeds a binary proxy
protocol v2 packet with PP2_TYPE_SSL + PP2_SUBTYPE_SSL_CN sub-TLV
through parse_proxy_proto_header() and verifies that cert_auth_ok and
cert_username are populated correctly. Also covers verify!=0, missing
CERT_SESS flag, no CN sub-TLV, and TCP conn_type (TLV parsing skipped).
Signed-off-by: Nikos Mavrogiannopoulos <n.mavrogiannopoulos@gmail.com>
Ensure global options are properly copied to virtual hosts when not
explicitly specified.
This change also:
- Documents global options that cannot be overridden in virtual hosts.
- Adds a 'VIRTUAL HOSTS' section to the man page to clarify behavior.
Closes#698
Signed-off-by: Grigory Trenin <grigory.trenin@gmail.com>
Regression test for the group-separator=comma option: verifies that
OU= Class attributes with comma-separated group names (as sent by
Freeradius) are correctly parsed when group-separator=comma is set.
Relates: #428
Signed-off-by: Nikos Mavrogiannopoulos <n.mavrogiannopoulos@gmail.com>
This validates that restrict-user-to-ports and
restrict-user-to-routes are enforced by the fw script.
The test verifies three cases after connecting with a config that
allows only TCP 80 and advertises a single route:
- allowed port + advertised route: connection succeeds
- denied port + advertised route: rejected at port level
- allowed port + non-advertised route: rejected at route level
Signed-off-by: Nikos Mavrogiannopoulos <n.mavrogiannopoulos@gmail.com>
This tests: show status, reload, show iroutes, disconnect id,
show sessions, show ip ban points.
Signed-off-by: Nikos Mavrogiannopoulos <n.mavrogiannopoulos@gmail.com>
The sleep-connect-script blocked on both connect and disconnect invocations.
When the server shuts down, two disconnect scripts race past the test -f
check simultaneously and both sleep 600s, exceeding the test timeout.
Signed-off-by: Nikos Mavrogiannopoulos <n.mavrogiannopoulos@gmail.com>
