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>
This adds checks in memory allocation, to address the following issue reported by coverity:
** CID 645850: Null pointer dereferences (FORWARD_NULL) /tests/ban-ips.c: 84 in main()
Signed-off-by: default avatarDimitri Papadopoulos <3350651-DimitriPapadopoulos@users.noreply.gitlab.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>
This fixes a -Wtype-limits warning encountered in builds without
PAM and RADIUS (e.g., OpenWRT). When the avail_acct_types array is
empty, ARRAY_SIZE evaluates to 0, making the unsigned comparison
always false.
Reported in #709.
Signed-off-by: Grigory Trenin <grigory.trenin@gmail.com>
Ensure that 'tunnel-all-dns' setting is honoured when overridden
in user or group-specific configuration files.
Resolves: #708
Signed-off-by: Grigory Trenin <grigory.trenin@gmail.com>
radcli 1.5.0 validates Message-Authenticator in RADIUS responses
CVE-2024-3596 (BlastRADIUS) and silently discards responses that lack
it. Make sure that the Message-Authenticator message is known to
the client via the dictionary.
Signed-off-by: Nikos Mavrogiannopoulos <n.mavrogiannopoulos@gmail.com>
All six radius tests used a fixed sleep 4 after starting radiusd and
ocserv. On slow or ASAN-instrumented hosts (CentOS 10 CI) this is
insufficient: freeradius with -xx debug logging takes longer than 4
seconds to load its modules, and even after binding UDP 1812 it continues
initializing its user database before it can process auth requests.
Signed-off-by: Nikos Mavrogiannopoulos <n.mavrogiannopoulos@gmail.com>
CONTRIBUTING.md: new AI Assistance Policy section stating that AI use is
assumed and requires no disclosure, that human accountability is what
matters, and that reviewers may request additional explanation for
submissions showing signs of unchecked generation.
Signed-off-by: Nikos Mavrogiannopoulos <n.mavrogiannopoulos@gmail.com>
Introduce a structured AI assistance framework for the project:
- AGENTS.md: single, comprehensive AI guidance file for all tools
(Claude Code, Codex, Copilot, Cursor). Covers the privilege-separation
architecture invariant, build/test instructions, code style (including
a rule against deep preprocessor conditionals), memory allocator policy,
IPC modification procedure, module-specific doc pointers, and a
contribution checklist split into agent-runnable and human-judgment items.
Includes a security disclosure gate that redirects potential vulnerability
reports to the confidential issue tracker before any public MR is opened.
- contrib/ai/personas/ocserv-core-dev.md: maintainer-facing persona with
project-specific protocols for anti-hallucination (GnuTLS/protobuf/seccomp
APIs), memory safety (talloc-first, gnutls_malloc exception), a taxonomy
of ocserv-specific vulnerability classes, and a self-verification protocol
that distinguishes what an agent can check automatically from what requires
human judgment.
- contrib/ai/personas/ocserv-contributor.md: external-contributor-facing
persona with mandatory architecture orientation, a prominent security
disclosure gate, five hard guardrails (privilege boundary, syscall
portability, GnuTLS-only, protobuf regeneration, talloc), and a
step-by-step workflow for features, bug fixes, and security fixes.
Signed-off-by: Nikos Mavrogiannopoulos <n.mavrogiannopoulos@gmail.com>
Protobuf files are removed from the repository and are only
auto-generated during dist.
Signed-off-by: Nikos Mavrogiannopoulos <n.mavrogiannopoulos@gmail.com>
All [scope: vhost (non-reloadable)] fields in static_cfg_st now inherit
from the default vhost when not explicitly set in a named-vhost section,
consistent with how ReloadableConfig fields already behave. This means
a named vhost that shares the same TLS cert, CA, auth method, or PKCS#11
pins as the default no longer has to repeat them.
Signed-off-by: Nikos Mavrogiannopoulos <n.mavrogiannopoulos@gmail.com>
NetworkConfig.name ('device' key) is global-only: the parser calls
error_on_vhost() and tun.c always reads it from the default vhost.
Annotate it as [scope: global] in cfg.proto to match the implementation.
Also add missing error_on_vhost() guards to the deprecated aliases
'use-seccomp' (for isolate-workers), 'use-dbus' (for use-occtl) and
'min-reauth-time' (for ban-time). Their canonical replacements already
reject vhost use; the aliases did not.
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>
Do not forward DTLS packets until the client is authenticated to
prevent the race between AUTH_COOKIE_REP and CMD_UDP_FD messages
on the command socket.
Closes#706
Signed-off-by: Grigory Trenin <grigory.trenin@gmail.com>
Document that there is no need to reload ocserv after modifying per-user
or per-group configuration.
Signed-off-by: Grigory Trenin <grigory.trenin@gmail.com>
Disable when targeting Linux kernels that do not support Unix socket
monitoring (UNIX_DIAG), like the default OpenWrt kernel.
This will avoid flooding the log with this error message:
NLM query failed
Signed-off-by: Dimitri Papadopoulos <3350651-DimitriPapadopoulos@users.noreply.gitlab.com>
recv_cookie_auth_reply() used > instead of != when validating
msg->session_id.len, allowing a shorter-than-expected length to
pass. ws->session_id is always consumed at full sizeof() by the
DTLS session setup; a short fill would leave stale tail bytes.
Align with the stricter != check used in recv_auth_reply().
proxy-proto: abort on zero-length unknown TLV in parse_ssl_tlvs()
An unknown TLV with length == 0 caused an infinite loop: the
AVAIL_HEADER_SIZE macro subtracted nothing and data did not
advance. Detect this and return early, consistent with how
AVAIL_HEADER_SIZE handles other malformed headers.
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>
handle_sec_mod_commands() lacked an upper-bound check on the `length`
field received from the sec-mod socket before passing it to talloc_size().
The worker command handler already applies a MAX_MSG_SIZE guard.
Replace the redundant (int)length < 0 cast (impossible for uint32_t) with
a length > MAX_MSG_SIZE check, matching the pattern in handle_worker_commands().
Also fix the format specifier from %d to %u for the uint32_t length.
Signed-off-by: Nikos Mavrogiannopoulos <n.mavrogiannopoulos@gmail.com>
On sec-mod exit, sec_mod_client_db_deinit() freed client entries via
talloc_free without first calling sec_auth_user_deinit(), leaving open
accounting sessions (RADIUS, PAM) and auth module state uncleaned.
Iterate over all entries and call sec_auth_user_deinit() before the
htable_clear/talloc_free teardown, matching the behaviour of the normal
del_client_entry -> clean_entry path.
Signed-off-by: Nikos Mavrogiannopoulos <n.mavrogiannopoulos@gmail.com>
protobuf-c sets absent string fields to NULL at runtime even when
marked required in the schema, contrary to the in-code comment.
Calling strlen(NULL) is undefined behaviour and crashes sec-mod.
A compromised worker could exploit this to deny authentication for
all connected clients.
Add an explicit NULL check before the strlen call, consistent with
the existing guard on the our_ip field.
Signed-off-by: Nikos Mavrogiannopoulos <n.mavrogiannopoulos@gmail.com>
