server: fix misleading data length validation log message
The format string "%u < %u <= %u" was printed with arguments (min_len, data->len, max_len), placing the offending length in the middle and mislabeling it. This is confusing. Let's print the actual length followed by the expected interval instead.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Change-Id: Ice95c1f8ad1aa8de259364bd70eba0db8918b19e
server: do not abort process on short conn message
conn_read_cb() used OSMO_ASSERT() to check that the received message holds at least a full osmo_pcap_data header. Although conn_segmentation_cb2() should only ever hand up complete frames, asserting on a length derived from network input means a framing anomaly would abort the entire server (taking down all other clients' captures). Close the offending connection gracefully instead, consistent with the other error paths in this function.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Change-Id: Ia102ff918ef8152d212e10a860f5dc70efec880b
rx_link_hdr() takes ownership of msg on success (rx_link() only frees it on failure). Both branches that call update_conn_file_hdr_msg() free msg, but when an identical link header was already stored neither branch ran and msg was leaked.
This happens on every duplicate PKT_LINK_HDR, e.g. a client that periodically resends its header. Free msg explicitly in that case.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Change-Id: I79344fe942342f2a736878142b3cf036fc982eef
verify_cert_cb() retrieved the gnutls session pointer and passed it to gnutls_certificate_verify_peers3() as the expected hostname. But the session pointer is set to the osmo_tls_session struct (it is needed by cert_callback()), not a hostname string. Hostname matching was therefore performed against raw struct bytes, rendering verification meaningless and potentially reading out of bounds, even when "tls verify-cert" was enabled.
Store the configured hostname in struct osmo_tls_session and have verify_cert_cb() read it from there. Also drop the stray gnutls_certificate_verify_peers3() call in the client setup: it ran before any handshake (so there were no peer certificates yet) and its result was ignored; the real verification happens via the registered callback during the handshake.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Change-Id: If64950a698bfcfbf556a37ef1be3e68abc124384
tls: do not treat GNUTLS_E_AGAIN/INTERRUPTED as fatal on read
osmo_tls_client_bfd_cb() treated any non-positive return from gnutls_record_recv() as a fatal error and tore down the session. On a non-blocking socket gnutls_record_recv() can return GNUTLS_E_AGAIN or GNUTLS_E_INTERRUPTED (both negative but non-fatal), which would drop an otherwise healthy TLS session. Handle them as retryable, mirroring the existing logic in tls_write().
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Change-Id: If2f842b202dd08c07dffe3770c51cf0ce886beee
client: fix off-by-one in wrapped pcap stats counter
When a libpcap stats counter (ps_recv/ps_drop/ps_ifdrop) wraps around UINT_MAX, get_psbl_wrapped_ctr() computed the delta as (UINT_MAX - old_val) + new_val, omitting the single increment that takes the counter from UINT_MAX through zero. Add the missing +1 so the reported delta matches the real number of increments.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Change-Id: I66581910dbd1e955831a6ff913042059ad4994a7
The GPRS-NS/BSSGP filter assumed a fixed 20-byte IPv4 header (IP_LEN) when locating the UDP header and payload. When the captured packet carries IPv4 options (ip_hl > 5), udp_data/payload_data pointed into the middle of the headers and check_gprs() parsed garbage, classifying packets incorrectly.
Use the actual header length from ip_hl, reject malformed headers (ip_hl < 5), and re-validate that the larger headers fit within the captured length before computing the payload.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Change-Id: Iac1fa9cc2a3c06cbe19c3e7799a0b335f2e3dda9
server: fix NULL deref of file_hdr_msg when store is disabled
When a connection has storing disabled (no store), conn->file_hdr_msg is never populated. The previous link-header handling skipped the first branch (gated on conn->store) and fell through to the comparison branch, which dereferenced the still-NULL conn->file_hdr_msg, crashing the server on the first PKT_LINK_HDR from such a client.
Gate the whole header tracking on conn->store and simply free the message when not storing, since osmo_pcap_conn_restart_trace() already no-ops in that case.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Change-Id: I419e1b66d07307c3e49294984887c153cd8494c3
zmq_msg_send() only transfers ownership of the message to ZeroMQ on success. On failure the caller retains ownership, so the previously init'd zmq_msg_t was leaked on every failed publish. Close it explicitly on the error path.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Change-Id: I501b1bf55bede4e69fa5d9b3f38d87341482ff49
client: Fix 32-bit overflow when computing pcapng EPB timestamp
tv_sec * 1000 * 1000 was evaluated in int arithmetic. Where time_t / tv_sec is 32-bit, this overflows for any tv_sec > ~2147, corrupting the 64-bit timestamp_usec well before the year 2038. Cast tv_sec to uint64_t before the multiplication so the whole expression is computed in 64 bits.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Change-Id: I20d3282b8cba1675ce2d0860e66027e5ee8245ef
conn->tls_hostname defaults to NULL and can be cleared via "no tls hostname". Writing it unconditionally emitted a "tls hostname (null)" line, producing a config that does not re-parse. Guard it like the other optional tls fields.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Change-Id: I5b920337409d8c9fa1edb8d47177882cf0a6c4e7
server: vty: validate rotate-localtime modulus against the new interval
apply_rotate_localtime() computed the maximum allowed modulus from pcap_server->rotate_localtime.intv, the currently-stored (old) interval, rather than the intv argument being applied. On first configuration the stored interval is the default 0, so the switch hit the default case and rejected an otherwise valid command; when changing intervals the modulus was bounds-checked against the wrong interval. Switch on intv instead.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Change-Id: I0b367d4e255db3208b41e12adec682026b99cc18
vty: clamp configured snaplen to the wire-framing limit
osmo-pcap carries each captured packet in a frame whose length field (struct osmo_pcap_data.len) is a uint16_t, so any snaplen above ~64 KiB cannot be transported and is silently clamped by the server when sizing its receive buffer (calc_data_max_len() caps at UINT16_MAX). The VTY, however, advertised libpcap's MAXIMUM_SNAPLEN (262144), misleading users into configuring values that never take effect.
Introduce OSMO_PCAP_MAX_SNAPLEN (65535) and, in both the client "pcap snaplen" and server "max-snaplen" handlers, warn and cap the value to it when a larger one is given. The command syntax keeps the <1-262144> range for backwards compatibility so existing configs still parse; the help text now documents the effective 64 KiB limit.
Related: SYS#8099 Related: 6d2f7c52 ("server: Limit rx buffer size to UINT16_MAX") Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Change-Id: Ia56cad48e8cefe8ae103f2f7d2e037bf28438b71
