Verifies that with a per-user /127 network the client is leased
network + 1 and the server takes the network address, that an explicit
IPv6 address equal to the server's /127 address is rejected, and that on
a wider network network + 1 is now leasable while the server takes the
network address. Replaces the p2p-net test, which could not connect
(ns.sh was not sourced) and had no occtl socket configured.
Relates: #714
Signed-off-by: Nikos Mavrogiannopoulos <n.mavrogiannopoulos@gmail.com>
IPv6 address leases were starting at the network address + 1, following
the IPv4 convention where the network address is reserved. However,
IPv6 doesn't have such a restriction, and that behaviour is inconsistent
with IPv6 standards.
Use the IPv6 network address as the server-side tunnel address instead
of network address + 1, as described in REQ-MAIN-NET-006. This fix is
particularly important for point-to-point /127 networks with only 2
addresses. It also prevents a client with ipv6-subnet-prefix < 128 from
being leased the first subnet, which contained the server address.
This changes the server-side address of every IPv6 deployment; the
existing tests are updated accordingly.
Resolves: #714
Signed-off-by: Dimitri Papadopoulos <3350651-DimitriPapadopoulos@users.noreply.gitlab.com>
Add REQ-MAIN-NET-006, the IPv6 counterpart of REQ-MAIN-NET-004: the
server takes the network address of ipv6-network, so on a /127
point-to-point network (RFC 6164) the client is leased the other address.
Document this in sample.config, noting that earlier versions used the
network address + 1.
Resolve the open OC-PROTO-CONN-007 / REQ-PROTO-CONN-007 notes: ocserv
follows "server address first" for both address families, and diverges
from the /127 recommendation for X-CSTP-Address-IP6 since it sends
ipv6-subnet-prefix.
Relates: #714
Signed-off-by: Nikos Mavrogiannopoulos <n.mavrogiannopoulos@gmail.com>
tests/scripts/vpnc-script saved the default route to ./defaultroute,
which all tests running in parallel from build/tests share. A client
could thus restore another test's route in its own namespace ("Cannot
find device ocen1c<pid>"), leaving it without a route to the server.
Use a unique name per script.
Adds REQ-GEN-TEST-013.
Signed-off-by: Nikos Mavrogiannopoulos <n.mavrogiannopoulos@gmail.com>
The generated-network subsystem accepted a network when its first host
address did not answer ping. That misses a local subnet whose other
hosts are in use, hosts that drop ICMP, and fails open when ping cannot
run at all (no CAP_NET_RAW in a container), since "no reply" and "ping
failed" look the same; each accepted draw also cost about two seconds.
Query the routing table instead: a drawn network or endpoint address is
redrawn while a route in any table lies inside it or covers it, which
includes the local addresses. Default routes and covering routes shorter
than the private block drawn from (e.g. a VPN client's 0.0.0.0/1) are
ignored. A failing ip command, or 100 rejected draws, aborts the test
instead of accepting the network. random-net.sh and random-net2.sh use
the same check for the ns.sh endpoint addresses.
Update REQ-GEN-TEST-011 accordingly, and REQ-GEN-TEST-012 to make ip a
hard dependency like ipcalc.
Signed-off-by: Nikos Mavrogiannopoulos <n.mavrogiannopoulos@gmail.com>
The check used a quoted right-hand side in [[ =~ ]], which bash treats
as a literal string, and a character class [vpns|tun] where an
alternation was meant; it never matched, so the test could not fail on
wrong route-add-cmd output. Compare the whole line with grep -Ex instead
and print the actual contents on failure.
Signed-off-by: Nikos Mavrogiannopoulos <n.mavrogiannopoulos@gmail.com>
Convert every test to REQ-GEN-TEST-010. Addresses configured on an
interface come from random-vpnnet.sh: templates use @VPNNET_BASE@ and
friends, keeping the original netmask and route formats; the vhost
tests allocate a distinct network per vhost with alloc_vpnnet4; the
firewall tests take the address of the blocked host from one; and the
RADIUS users file becomes a template materialized per test with
update_raddb(), so the tests no longer pin VPNNET to fixed ranges.
Addresses that are only data (routes, no-routes, iroutes, DNS servers,
Framed-Route, the pools of tests without a TUN device) move to
documentation ranges, keeping distinct networks distinct, and the
assertions follow; multiple-routes pushes 256 distinct routes, taken
from 198.18.0.0/15.
Add tests/check-test-addresses.py, run by the test-addresses-check job
in the preliminaries CI stage, which rejects any literal address outside
the ranges REQ-GEN-TEST-010 allows.
Relates: #714
Signed-off-by: Nikos Mavrogiannopoulos <n.mavrogiannopoulos@gmail.com>
The test only needs some traffic on the session for its accounting
checks, which require a non-zero Acct-Input-Octets. iperf3 through
the tunnel makes the worker exceed its RLIMIT_DATA headroom under
ASAN, which counts the sanitizer's shadow reservation as data.
Send pings instead.
Signed-off-by: Nikos Mavrogiannopoulos <n.mavrogiannopoulos@gmail.com>
Add REQ-GEN-TEST-010..012. Addresses a test configures on an interface
(VPN networks, explicit IPs, ...) must come from random-net.sh.
That way a test can never collide with the local network of the machine
running it.
common.sh substitutes @NAME@, @NAME_BASE@ and @NAME_ADDR@ for every
allocated network, adds @CONFIG_DIR@ and update_config_dir() to template
per-user and per-group config directories, adds update_raddb() to give a
RADIUS test a private raddb directory with a templated users file, and
fails with a clear message when a template uses a @VPN...@ placeholder
whose variable is unset, instead of producing a broken config.
Signed-off-by: Nikos Mavrogiannopoulos <n.mavrogiannopoulos@gmail.com>
Replace the duplicated mask31 byte arrays and memcmp() calls with a
single helper that compares the netmask against 255.255.255.254, and
refresh the src/ip-lease.c line citations in REQ-MAIN-NET-001/002/004.
Relates: #714
Signed-off-by: Nikos Mavrogiannopoulos <n.mavrogiannopoulos@outlook.com>
Verifies that a client with a per-user /31 network is leased the
non-network address while the server takes the network address, that
an explicit IP equal to the server's /31 address is rejected, and that
network/broadcast addresses remain reserved for /30 networks.
Relates: #714
Signed-off-by: Nikos Mavrogiannopoulos <n.mavrogiannopoulos@outlook.com>
RFC 3021 makes an exception for /31 networks: the network and broadcast
addresses are not reserved, since they are not needed in a point-to-point
context, and a /31 network contains only two addresses.
In that case, start at network address instead of network address + 1.
Relates: #714
Signed-off-by: Dimitri Papadopoulos <3350651-DimitriPapadopoulos@users.noreply.gitlab.com>
inih truncated section names to 50 bytes, of which "vhost:" took six, so
a virtual host name longer than 43 characters was silently shortened: it
no longer matched the hostname clients send in SNI, and two hosts sharing
those first 43 characters collapsed into one. Make the section buffer size
overridable and raise it, and reject a name that cannot be stored in full
or that exceeds MAX_VHOST_NAME_LEN (253, the DNS maximum), rather than
accepting a shortened one. Covered by tests/test-vhost-name-length, which
drives ocserv -t.
Resolves: #759
Signed-off-by: Katie Hudson <41780955-Ocelot5k@users.noreply.gitlab.com>
Removed introduced by mistake requirement. This is currently
under discussion in !579.
Signed-off-by: Nikos Mavrogiannopoulos <n.mavrogiannopoulos@gmail.com>
Close every open accounting session from the sec-mod shutdown cleanup path without waiting for individual RADIUS responses. With radcli, send one best-effort Accounting-Stop to the first configured server using zero timeout and retries; with legacy freeradius-client, skip the shutdown send rather than block termination.
Preserve accumulated counters, existing terminate causes, and the disconnect-time uptime snapshot of retained sessions. Clear the previous connection segment cause after a successful cookie resumption so an unset current cause continues to map to Lost-Service.
Document the shutdown and reconnect contracts as separate requirements and extend the RADIUS integration test to cover retained, resumed, and active sessions plus first-server-only delivery.
Resolves: #643
Signed-off-by: Alex Protsko <fidget2015@yahoo.com>
Also adds REQ-GEN-TECH-007 requiring that general improvements to
actively-maintained vendored subtrees be sent upstream first.
Signed-off-by: Nikos Mavrogiannopoulos <n.mavrogiannopoulos@gmail.com>
When certificate authentication is configured as optional ensure that
there is sufficient information for tracing the authentication used
but do not log under LOG_ERR a missing certificate that was not required.
Resolves: #744
Signed-off-by: Nikos Mavrogiannopoulos <n.mavrogiannopoulos@gmail.com>
Malicious clients cannot send CMD_SEC_AUTH_INIT arbitrarily: a worker's
pid has one client_entry_st attached to it. A repeated SEC_AUTH_INIT
is accepted (replacing the previous entry) only when the prior
attempt already ended in PS_AUTH_FAILED with no session attached
(in_use == 0), matching the legitimate case for it (GSSAPI/certificate
falling back to the next auth method).
Resolves: #249
Signed-off-by: Nikos Mavrogiannopoulos <n.mavrogiannopoulos@gmail.com>
A cached resume entry can end up with zero-length session_data, which is
not valid resumption data.
Signed-off-by: Nikos Mavrogiannopoulos <n.mavrogiannopoulos@gmail.com>
TLS 1.3 has no renegotiation and provides transparent rekeys but this
was not used by the server. Instead rekey-method=ssl was silently
downgraded to new-tunnel, causing a full tunnel/TUN-device rebuild (and
a brief data-path interruption) on every rekey whenever a client
negotiated TLS 1.3.
This commit simplifies that handling by taking advantage of TLS 1.3's own
rekey mechanism instead: the server now performs the "ssl" rekey on
TLS 1.3 sessions via the standard TLS 1.3 KeyUpdate message.
TLS <= 1.2 rekey behavior (client-driven rehandshake, gated on RFC
5746 safe renegotiation) is unchanged.
Resolves: #745
Signed-off-by: Nikos Mavrogiannopoulos <n.mavrogiannopoulos@gmail.com>
Commit 5cf457b4 ("Removed the listen-clear-file config option", Dec
2020) deleted the only code path that could ever construct a
SOCK_TYPE_UNIX worker connection, but left every branch that handled
it in place. Since then ws->session has been unconditionally non-NULL
and ws->conn_type unconditionally not SOCK_TYPE_UNIX for every worker,
making all of the following unreachable:
- tlslib.c: recv_remaining(), _cstp_recv_packet() (the non-TLS CSTP
reassembly path hardened in the previous commit), and
tls_has_session_cert() (zero callers) deleted outright; cstp_cork/
cstp_uncork/cstp_send/cstp_recv_packet/cstp_recv/cstp_close/
cstp_fatal_close collapsed to their TLS-only body.
- worker-http.c, worker-auth.c, worker-vpn.c, main.c: dead
ws->session == NULL / ws->conn_type == SOCK_TYPE_UNIX branches
removed or simplified to their live half.
- worker-proxyproto.c: parse_ssl_tlvs() and its TLV structs/macros
removed. This proxy-protocol SSL-CN extraction was itself only
ever invoked from the same dead SOCK_TYPE_UNIX branch, so it has
been as unreachable as the rest since 2020 despite a recent,
otherwise-correct bug fix.
- tests/cstp-recv.c deleted (exercised only the removed reassembly
path); tests/proxyproto-v2.c trimmed to the still-live IPv6
address-parsing regression test.
- doc/sample.config: dropped the stale "TCP or UNIX socket" wording
for listen-proxy-proto left over from the same 2020 removal; only
a TCP socket is ever listened on for the proxy protocol now.
Narrows the worker's attack surface to the code paths a client can
actually reach, and removes a source of wasted maintenance effort.
Signed-off-by: Nikos Mavrogiannopoulos <n.mavrogiannopoulos@gmail.com>
recv_remaining() is used only on the non-TLS CSTP path (a UNIX socket
proxying plaintext CSTP in front of ocserv). On a recv() failure or
peer close mid-read, it discarded the error and returned whatever
partial byte count it had accumulated so far. Since every caller only
checks "ret <= 0" and otherwise trusts the count as a complete read,
a truncated body could come back as a positive, non-zero total that
looked like success: _cstp_recv_packet() would then report the full
8+pktlen size to its caller with only part of the buffer actually
populated from the network, feeding stale/uninitialized bytes into
parse_cstp_data() as if they were received client data.
Make the contract unambiguous: recv_remaining() now returns either
exactly the requested byte count or a negative error - never a
partial positive count a caller could mistake for success.
Signed-off-by: Nikos Mavrogiannopoulos <n.mavrogiannopoulos@gmail.com>
ip_route_sanity_check() was a format normalizer, not a validator: any
dot-free route (all IPv6, or arbitrary text) returned success
unexamined, and a dotted-netmask IPv4 route passed through unchanged.
Since route_adddel() substitutes the validated route into
route-add-cmd/route-del-cmd and executes it via `/bin/sh -c` as root,
a route string carrying shell metacharacters that survived this check
was a root command-injection vector.
Rewrite the check to fully parse the route as an IPv4 or IPv6 address
plus prefix, or the literal keyword "default" (the documented
all-traffic-through-VPN shortcut, checked separately by config.c after
this function runs), and reject anything left over. Numeric IPv4
prefixes are still normalized to a dotted netmask as before.
Adds REQ-MAIN-SEC-008 and tests/route-sanity-check.c, confirmed
against real config/test usage (including "route = default") so the
stricter validation doesn't regress documented syntax.
Signed-off-by: Nikos Mavrogiannopoulos <n.mavrogiannopoulos@gmail.com>
main read cookie.data[0] (to select sec_mod_instance_index) before
validating the cookie's length. A worker sending an empty cookie
unpacks with cookie.data == NULL (protobuf-c "required bytes"
semantics), so the read crashed the root main process, tearing down
every connected client.
Validate cookie.data/len immediately after unpacking, before the
first byte is read. handle_auth_cookie_req()'s own later check
remains as defense-in-depth.
Adds REQ-IPC-018.
Signed-off-by: Nikos Mavrogiannopoulos <n.mavrogiannopoulos@gmail.com>
Fix worker crashes on musl-based systems when isolate-workers = true by allowing munmap, mremap, and madvise in the worker seccomp filter.
Move seccomp coverage to Alpine CI and use oc_syslog for seccomp trap diagnostics instead of direct write()-based output.
Keep glibc backtrace diagnostics as the default and allow musl builds to select the syscall-only fallback with the assume-glibc Meson option.
Resolves: #749
Signed-off-by: Alex Protsko <fidget2015@yahoo.com>
RFC 2866 (5.5) allows an Access-Request to carry Acct-Session-Id and
requires the same value in the session's Accounting-Requests. Sending it
already at authentication time lets the RADIUS server correlate the two
exchanges by a single per-session key (e.g. for rlm_ippool, so concurrent
sessions of the same user from the same client do not collide on one IP
lease).
Documented as REQ-AUTH-AUTH-025, with a positive check in tests/radius
that the id sent as Acct-Session-Id in the Access-Request matches the
Accounting-Request.
Signed-off-by: Dmitrii <dimmispencer@gmail.com>
Resolves: #751
