Changes

rebar.config: use a fork of rebar3_diameter_compiler

This fixes building on Erlang/OTP 29.  A pull request [1] has been
submitted upstream.  Once merged, we get switch back to upstream.

[1] https://github.com/carlosedp/rebar3_diameter_compiler/pull/28

Change-Id: I548a3fef6f8a23a548fb6376076b9d4a9345d692

rebar.config: use a fork of rebar3_diameter_compiler

This fixes building on Erlang/OTP 29.  A pull request [1] has been
submitted upstream.  Once merged, we get switch back to upstream.

[1] https://github.com/carlosedp/rebar3_diameter_compiler/pull/28

Change-Id: I548a3fef6f8a23a548fb6376076b9d4a9345d692

rebar.config: use a fork of rebar3_diameter_compiler

This fixes building on Erlang/OTP 29.  A pull request [1] has been
submitted upstream.  Once merged, we get switch back to upstream.

[1] https://github.com/carlosedp/rebar3_diameter_compiler/pull/28

Change-Id: I548a3fef6f8a23a548fb6376076b9d4a9345d692

rebar.config: use a fork of rebar3_diameter_compiler

This fixes building on Erlang/OTP 29.  A pull request [1] has been
submitted upstream.  Once merged, we get switch back to upstream.

[1] https://github.com/carlosedp/rebar3_diameter_compiler/pull/28

Change-Id: I548a3fef6f8a23a548fb6376076b9d4a9345d692

rebar.config: bump meck version: 0.9.2 -> v1.2.0

This fixes a compilation error with recent Erlang/OTP 29:

[  335s] ===> Compiling meck
[  335s] ===> Compiling _checkouts/meck/src/meck_matcher.erl failed
[  335s] meck_matcher.erl:76:6: 'catch ...' is deprecated; please use 'try ... catch ... end' instead.
[  335s] Compile directive 'nowarn_deprecated_catch' can be used to suppress
[  335s] warnings in selected modules.

Change-Id: If3409de1e09ac1b6ce3355d19e47ef0e160a2d3b
(cherry picked from commit a013155e8af1b4cca604e10f81dcb683a5e7812c)

Bump version: 0.4.1

Change-Id: I17c9806400a48b4cdf7be19a824e10c56620d655

rebar.config: bump meck version: 0.9.2 -> v1.2.0

This fixes a compilation error with recent Erlang/OTP 29:

[  335s] ===> Compiling meck
[  335s] ===> Compiling _checkouts/meck/src/meck_matcher.erl failed
[  335s] meck_matcher.erl:76:6: 'catch ...' is deprecated; please use 'try ... catch ... end' instead.
[  335s] Compile directive 'nowarn_deprecated_catch' can be used to suppress
[  335s] warnings in selected modules.

Change-Id: If3409de1e09ac1b6ce3355d19e47ef0e160a2d3b
(cherry picked from commit a013155e8af1b4cca604e10f81dcb683a5e7812c)

Bump version: 0.4.1

Change-Id: I17c9806400a48b4cdf7be19a824e10c56620d655

Bump version: 0.9.0.4-3248-dirty → 0.9.1

Change-Id: I0d06c6f15aa99be7609eac9afb6b9df8e6cf3dc5

Bump version: 0.9.0.4-3248-dirty → 0.9.1

Change-Id: I0d06c6f15aa99be7609eac9afb6b9df8e6cf3dc5

Add missing const for struct hostent *hostent

The hostent parameter is supposed to be const. This is being enforced in
c-ares 1.34.7, which is why osmo-sgsn currently fails to build in debian
unstable. The c-ares developers are preparing a new release that reverts
this change to avoid breakage with other users of libc-ares, and only
document that this should basically be const. But let's use the API as
intended so the compiler knows that osmo-sgsn is not supposed to modify
the hostent parameter.

Fix for:

  sgsn_ares.c:141:63: error: passing argument 4 of 'ares_gethostbyname' from incompatible pointer type [-Wincompatible-pointer-types]
    141 |         ares_gethostbyname(sgsn->ares_channel, name, AF_INET, ares_cb, cb_data);
        |                                                               ^~~~~~~
        |                                                               |
        |                                                               void (*)(void *, int,  int,  struct hostent *)
  /usr/include/ares.h:881:22: note: expected 'ares_host_callback' {aka 'void (*)(void *, int,  int,  const struct hostent *)'} but argument is of type 'void (*)(void *, int,  int,  struct hostent *)'

Related: https://github.com/c-ares/c-ares/pull/1060
Related: https://github.com/c-ares/c-ares/pull/1244
Change-Id: Ibe3debc5d3b7af5883001960c40efb296692f93d

Add missing const for struct hostent *hostent

The hostent parameter is supposed to be const. This is being enforced in
c-ares 1.34.7, which is why osmo-sgsn currently fails to build in debian
unstable. The c-ares developers are preparing a new release that reverts
this change to avoid breakage with other users of libc-ares, and only
document that this should basically be const. But let's use the API as
intended so the compiler knows that osmo-sgsn is not supposed to modify
the hostent parameter.

Fix for:

  sgsn_ares.c:141:63: error: passing argument 4 of 'ares_gethostbyname' from incompatible pointer type [-Wincompatible-pointer-types]
    141 |         ares_gethostbyname(sgsn->ares_channel, name, AF_INET, ares_cb, cb_data);
        |                                                               ^~~~~~~
        |                                                               |
        |                                                               void (*)(void *, int,  int,  struct hostent *)
  /usr/include/ares.h:881:22: note: expected 'ares_host_callback' {aka 'void (*)(void *, int,  int,  const struct hostent *)'} but argument is of type 'void (*)(void *, int,  int,  struct hostent *)'

Related: https://github.com/c-ares/c-ares/pull/1060
Related: https://github.com/c-ares/c-ares/pull/1244
Change-Id: Ibe3debc5d3b7af5883001960c40efb296692f93d

tcap_as_loadshare: Forward UDTS to the correct tcap-loadshare ASP

Related: SYS#8090
Change-Id: I2051aa9e84e553feb17daabb93ba3ce82358e0e4

tcap_as_loadshare: Forward UDTS to the correct tcap-loadshare ASP

Related: SYS#8090
Change-Id: I2051aa9e84e553feb17daabb93ba3ce82358e0e4

common: reset lchan meas state in gsm_lchan_release()

lchan->meas (including interf_meas_num and num_ul_meas) is normally
only reset by lchan_meas_reset() called from rsl_tx_chan_act_ack() on
RSL CHANNEL ACTIVATION.  Idle logical channels are never RSL-activated,
so their measurement state is never reset via that path.

On an OML link re-establishment, osmo-bts does not exit: abis.c tears
down the signalling links, the bts_shutdown FSM powers down all TRXs,
and then waits for reconnect.  The gsm_bts/trx/ts/lchan structures
remain in memory, so stale interf_meas_num survives the reconnect.

This is why we're seeing these ERRORs while running ttcn3-bts-test:

(bts=0,trx=2,ts=4,ss=6) Not enough room to store interference report (0dBm)

Add a lchan_meas_reset() call to gsm_lchan_release(), which is called
from gsm_ts_release() when the nm_channel_fsm enters state
NM_CHAN_ST_OP_DISABLED_NOTINSTALLED.  This is exactly the right
moment: the radio is fully stopped, so no new samples arrive.

Change-Id: I18dc9d30417b0c5b2e579660d4a087d93445f956
AI-Assisted: yes (Claude)

common: stop buffering UL measurements when SACCH is deactivated

When the BSC sends RSL DEACT SACCH, the per-SACCH UL measurement drain
stops (it runs on SACCH timing), but the producer in lchan_new_ul_meas()
keeps appending the measurement contributions from every received
TCH/SDCCH burst.  After one SACCH period (104 frames) the 104-slot
uplink measurement buffer fills up, yielding a flood of:

  NOTICE measurement.c:336 no space for uplink measurement, num_ul_meas=104

Add a bool sacch_active flag to gsm_lchan, set to true in the common
l1sap_chan_act() and clear in l1sap_chan_deact_sacch().  Guard
lchan_new_ul_meas() with this flag so that measurements are silently
discarded while SACCH is inactive - there is nothing to drain the
buffer and no SACCH channel on which to report the results to the BSC.

Change-Id: I3943c788cab5d2411b06ac681d4d412852bac0a7
AI-Assisted: yes (Claude)

common: flush paging queue in nm_bts_fsm's NOTINSTALLED on_enter

paging_reset() exists to flush stale paging records from the queue,
but was never called anywhere.  Stale paging records can accumulate
when the OML link goes down: osmo-bts does not exit, so the paging
queue survives into the next BSC session.

Call it alongside bts_cbch_reset() and bts_asci_notification_reset()
in st_op_disabled_notinstalled_on_enter(), which fires after all TRXs
are confirmed closed and before a new OML connection is accepted.

Change-Id: I109ab282986b68b68ba5c11859c44b771c0416fd
AI-Assisted: yes (Claude)

common: reset lchan meas state in gsm_lchan_release()

lchan->meas (including interf_meas_num and num_ul_meas) is normally
only reset by lchan_meas_reset() called from rsl_tx_chan_act_ack() on
RSL CHANNEL ACTIVATION.  Idle logical channels are never RSL-activated,
so their measurement state is never reset via that path.

On an OML link re-establishment, osmo-bts does not exit: abis.c tears
down the signalling links, the bts_shutdown FSM powers down all TRXs,
and then waits for reconnect.  The gsm_bts/trx/ts/lchan structures
remain in memory, so stale interf_meas_num survives the reconnect.

This is why we're seeing these ERRORs while running ttcn3-bts-test:

(bts=0,trx=2,ts=4,ss=6) Not enough room to store interference report (0dBm)

Add a lchan_meas_reset() call to gsm_lchan_release(), which is called
from gsm_ts_release() when the nm_channel_fsm enters state
NM_CHAN_ST_OP_DISABLED_NOTINSTALLED.  This is exactly the right
moment: the radio is fully stopped, so no new samples arrive.

Change-Id: I18dc9d30417b0c5b2e579660d4a087d93445f956
AI-Assisted: yes (Claude)

common: stop buffering UL measurements when SACCH is deactivated

When the BSC sends RSL DEACT SACCH, the per-SACCH UL measurement drain
stops (it runs on SACCH timing), but the producer in lchan_new_ul_meas()
keeps appending the measurement contributions from every received
TCH/SDCCH burst.  After one SACCH period (104 frames) the 104-slot
uplink measurement buffer fills up, yielding a flood of:

  NOTICE measurement.c:336 no space for uplink measurement, num_ul_meas=104

Add a bool sacch_active flag to gsm_lchan, set to true in the common
l1sap_chan_act() and clear in l1sap_chan_deact_sacch().  Guard
lchan_new_ul_meas() with this flag so that measurements are silently
discarded while SACCH is inactive - there is nothing to drain the
buffer and no SACCH channel on which to report the results to the BSC.

Change-Id: I3943c788cab5d2411b06ac681d4d412852bac0a7
AI-Assisted: yes (Claude)

common: flush paging queue in nm_bts_fsm's NOTINSTALLED on_enter

paging_reset() exists to flush stale paging records from the queue,
but was never called anywhere.  Stale paging records can accumulate
when the OML link goes down: osmo-bts does not exit, so the paging
queue survives into the next BSC session.

Call it alongside bts_cbch_reset() and bts_asci_notification_reset()
in st_op_disabled_notinstalled_on_enter(), which fires after all TRXs
are confirmed closed and before a new OML connection is accepted.

Change-Id: I109ab282986b68b68ba5c11859c44b771c0416fd
AI-Assisted: yes (Claude)

rebar.config: bump meck version: 0.9.2 -> v1.2.0

This fixes a compilation error with recent Erlang/OTP 29:

[  335s] ===> Compiling meck
[  335s] ===> Compiling _checkouts/meck/src/meck_matcher.erl failed
[  335s] meck_matcher.erl:76:6: 'catch ...' is deprecated; please use 'try ... catch ... end' instead.
[  335s] Compile directive 'nowarn_deprecated_catch' can be used to suppress
[  335s] warnings in selected modules.

Change-Id: If3409de1e09ac1b6ce3355d19e47ef0e160a2d3b

rebar.config: bump meck version: 0.9.2 -> v1.2.0

This fixes a compilation error with recent Erlang/OTP 29:

[  335s] ===> Compiling meck
[  335s] ===> Compiling _checkouts/meck/src/meck_matcher.erl failed
[  335s] meck_matcher.erl:76:6: 'catch ...' is deprecated; please use 'try ... catch ... end' instead.
[  335s] Compile directive 'nowarn_deprecated_catch' can be used to suppress
[  335s] warnings in selected modules.

Change-Id: If3409de1e09ac1b6ce3355d19e47ef0e160a2d3b

common: stop buffering UL measurements when SACCH is deactivated

When the BSC sends RSL DEACT SACCH, the per-SACCH UL measurement drain
stops (it runs on SACCH timing), but the producer in lchan_new_ul_meas()
keeps appending the measurement contributions from every received
TCH/SDCCH burst.  After one SACCH period (104 frames) the 104-slot
uplink measurement buffer fills up, yielding a flood of:

  NOTICE measurement.c:336 no space for uplink measurement, num_ul_meas=104

Add a bool sacch_active flag to gsm_lchan, set to true in the common
l1sap_chan_act() and clear in l1sap_chan_deact_sacch().  Guard
lchan_new_ul_meas() with this flag so that measurements are silently
discarded while SACCH is inactive - there is nothing to drain the
buffer and no SACCH channel on which to report the results to the BSC.

Change-Id: I3943c788cab5d2411b06ac681d4d412852bac0a7
AI-Assisted: yes (Claude)

common: stop buffering UL measurements when SACCH is deactivated

When the BSC sends RSL DEACT SACCH, the per-SACCH UL measurement drain
stops (it runs on SACCH timing), but the producer in lchan_new_ul_meas()
keeps appending the measurement contributions from every received
TCH/SDCCH burst.  After one SACCH period (104 frames) the 104-slot
uplink measurement buffer fills up, yielding a flood of:

  NOTICE measurement.c:336 no space for uplink measurement, num_ul_meas=104

Add a bool sacch_active flag to gsm_lchan, set to true in the common
l1sap_chan_act() and clear in l1sap_chan_deact_sacch().  Guard
lchan_new_ul_meas() with this flag so that measurements are silently
discarded while SACCH is inactive - there is nothing to drain the
buffer and no SACCH channel on which to report the results to the BSC.

Change-Id: I3943c788cab5d2411b06ac681d4d412852bac0a7
AI-Assisted: yes (Claude)

l1sap: fix duplicate RF RESOURCE INDICATION on clock bootstrap

The TTCN-3 test suite (ttcn3-bts-test) expects to receive exactly one
RF RESOURCE INDICATION message from each TRX during the bootstrap stage,
while waiting for all TRX to come up and be configured by the BSC.

l1sap_interf_meas_report() fires whenever bts->gsm_time.fn % period is
0, where period = intave * 104 (typically 624 frames).  Since CLCK.ind
with FN=0 satisfies this condition, a report is sent at the very
beginning of each clock epoch.

This was not a problem before commit fcfc4e83, because the first
CLCK.ind from the transciever was effectively a no-op: with
last_fn_timer.fn zero-initialised, the first indication at FN=0 yielded
elapsed_fn=0 (not > MAX_FN_SKEW), and the catch-up loop (while fn !=
last_fn_timer.fn) would not execute either.  Downlink scheduling only
started on the second CLCK.ind (at FN=102, which is > MAX_FN_SKEW),
and 102 % 624 != 0, so no RF RESOURCE INDICATION was triggered.

fcfc4e83 changed the logic so that Downlink scheduling now begins
immediately on the first CLCK.ind, via an unconditional call to
trx_setup_clock() -> bts_sched_fn(fn).  When fake_trx starts its frame
counter from FN=0, this immediately triggers l1sap_interf_meas_report()
because 0 % 624 == 0.  A second report follows ~2.88s later when the
periodic timer reaches FN=624, making the bootstrap logic
in ttcn3-bts-test unhappy.

Fix by shifting the trigger to (fn + 1) % period == 0, i.e. the report
fires at the last frame of each period rather than the first.  FN=0 now
yields (0+1) % 624 = 1 != 0, suppressing the spurious bootstrap report.
The periodic behaviour and report cadence are otherwise unchanged.

Change-Id: I6550178427b08e67c9763f0f37efff5b88960b1f
Related: fcfc4e83 ("osmo-bts-trx: fix spurious shutdown on first CLCK.ind from osmo-trx")
AI-Assisted: yes (Claude)

common: reset lchan meas state in gsm_ts_release()

lchan->meas (including interf_meas_num and num_ul_meas) is normally
only reset by lchan_meas_reset() called from rsl_tx_chan_act_ack() on
RSL CHANNEL ACTIVATION.  Idle logical channels are never RSL-activated,
so their measurement state is never reset via that path.

On an OML link re-establishment, osmo-bts does not exit: abis.c tears
down the signalling links, the bts_shutdown FSM powers down all TRXs,
and then waits for reconnect.  The gsm_bts/trx/ts/lchan structures
remain in memory, so stale interf_meas_num survives the reconnect.

This is why we're seeing these ERRORs while running ttcn3-bts-test:

(bts=0,trx=2,ts=4,ss=6) Not enough room to store interference report (0dBm)

Add a lchan_meas_reset() call to gsm_ts_release(), which is called
from nm_channel_fsm's NOTINSTALLED on_enter after the TRX is confirmed
closed.  This is exactly the right moment: the radio is fully stopped,
so no new samples arrive.

Change-Id: I18dc9d30417b0c5b2e579660d4a087d93445f956
AI-Assisted: yes (Claude)

common: stop buffering UL measurements when SACCH is deactivated

When the BSC sends RSL DEACT SACCH, the per-SACCH UL measurement drain
stops (it runs on SACCH timing), but the producer in lchan_new_ul_meas()
keeps appending the measurement contributions from every received
TCH/SDCCH burst.  After one SACCH period (104 frames) the 104-slot
uplink measurement buffer fills up, yielding a flood of:

  NOTICE measurement.c:336 no space for uplink measurement, num_ul_meas=104

Add a bool sacch_active flag to gsm_lchan, set to true in the common
l1sap_chan_act() and clear in l1sap_chan_deact_sacch().  Guard
lchan_new_ul_meas() with this flag so that measurements are silently
discarded while SACCH is inactive - there is nothing to drain the
buffer and no SACCH channel on which to report the results to the BSC.

Change-Id: I3943c788cab5d2411b06ac681d4d412852bac0a7
AI-Assisted: yes (Claude)

common: flush paging queue in nm_bts_fsm's NOTINSTALLED on_enter

paging_reset() exists to flush stale paging records from the queue,
but was never called anywhere.  Stale paging records can accumulate
when the OML link goes down: osmo-bts does not exit, so the paging
queue survives into the next BSC session.

Call it alongside bts_cbch_reset() and bts_asci_notification_reset()
in st_op_disabled_notinstalled_on_enter(), which fires after all TRXs
are confirmed closed and before a new OML connection is accepted.

Change-Id: I109ab282986b68b68ba5c11859c44b771c0416fd
AI-Assisted: yes (Claude)

oml: validate Intave Parameter range in SET BTS ATTR

3GPP TS 52.021 §9.4.24 defines valid range for the Intave Parameter
as 1..31, matching the fixed size of the per-lchan interference sample
buffer (interf_meas_dbm[31] in lchan.h).  Previously any uint8_t value
was accepted without validation, meaning a buggy BSC could send
intave=0 (silently disabling interference reporting) or intave>31
(causing a buffer overflow in gsm_lchan_interf_meas_push()).

Let's guard against that by NACKing the SET BTS ATTR message with
cause=NM_NACK_PARAM_RANGE if the value is outside the valid range.

Change-Id: Id4d3353d4397aaa2517091b020d38ee15e084e2c
AI-Assisted: yes (Claude)

l1sap: fix duplicate RF RESOURCE INDICATION on clock bootstrap

The TTCN-3 test suite (ttcn3-bts-test) expects to receive exactly one
RF RESOURCE INDICATION message from each TRX during the bootstrap stage,
while waiting for all TRX to come up and be configured by the BSC.

l1sap_interf_meas_report() fires whenever bts->gsm_time.fn % period is
0, where period = intave * 104 (typically 624 frames).  Since CLCK.ind
with FN=0 satisfies this condition, a report is sent at the very
beginning of each clock epoch.

This was not a problem before commit fcfc4e83, because the first
CLCK.ind from the transciever was effectively a no-op: with
last_fn_timer.fn zero-initialised, the first indication at FN=0 yielded
elapsed_fn=0 (not > MAX_FN_SKEW), and the catch-up loop (while fn !=
last_fn_timer.fn) would not execute either.  Downlink scheduling only
started on the second CLCK.ind (at FN=102, which is > MAX_FN_SKEW),
and 102 % 624 != 0, so no RF RESOURCE INDICATION was triggered.

fcfc4e83 changed the logic so that Downlink scheduling now begins
immediately on the first CLCK.ind, via an unconditional call to
trx_setup_clock() -> bts_sched_fn(fn).  When fake_trx starts its frame
counter from FN=0, this immediately triggers l1sap_interf_meas_report()
because 0 % 624 == 0.  A second report follows ~2.88s later when the
periodic timer reaches FN=624, making the bootstrap logic
in ttcn3-bts-test unhappy.

Fix by shifting the trigger to (fn + 1) % period == 0, i.e. the report
fires at the last frame of each period rather than the first.  FN=0 now
yields (0+1) % 624 = 1 != 0, suppressing the spurious bootstrap report.
The periodic behaviour and report cadence are otherwise unchanged.

Change-Id: I6550178427b08e67c9763f0f37efff5b88960b1f
Related: fcfc4e83 ("osmo-bts-trx: fix spurious shutdown on first CLCK.ind from osmo-trx")
AI-Assisted: yes (Claude)

common: reset lchan meas state in gsm_ts_release()

lchan->meas (including interf_meas_num and num_ul_meas) is normally
only reset by lchan_meas_reset() called from rsl_tx_chan_act_ack() on
RSL CHANNEL ACTIVATION.  Idle logical channels are never RSL-activated,
so their measurement state is never reset via that path.

On an OML link re-establishment, osmo-bts does not exit: abis.c tears
down the signalling links, the bts_shutdown FSM powers down all TRXs,
and then waits for reconnect.  The gsm_bts/trx/ts/lchan structures
remain in memory, so stale interf_meas_num survives the reconnect.

This is why we're seeing these ERRORs while running ttcn3-bts-test:

(bts=0,trx=2,ts=4,ss=6) Not enough room to store interference report (0dBm)

Add a lchan_meas_reset() call to gsm_ts_release(), which is called
from nm_channel_fsm's NOTINSTALLED on_enter after the TRX is confirmed
closed.  This is exactly the right moment: the radio is fully stopped,
so no new samples arrive.

Change-Id: I18dc9d30417b0c5b2e579660d4a087d93445f956
AI-Assisted: yes (Claude)

common: stop buffering UL measurements when SACCH is deactivated

When the BSC sends RSL DEACT SACCH, the per-SACCH UL measurement drain
stops (it runs on SACCH timing), but the producer in lchan_new_ul_meas()
keeps appending the measurement contributions from every received
TCH/SDCCH burst.  After one SACCH period (104 frames) the 104-slot
uplink measurement buffer fills up, yielding a flood of:

  NOTICE measurement.c:336 no space for uplink measurement, num_ul_meas=104

Add a bool sacch_active flag to gsm_lchan, set to true in the common
l1sap_chan_act() and clear in l1sap_chan_deact_sacch().  Guard
lchan_new_ul_meas() with this flag so that measurements are silently
discarded while SACCH is inactive - there is nothing to drain the
buffer and no SACCH channel on which to report the results to the BSC.

Change-Id: I3943c788cab5d2411b06ac681d4d412852bac0a7
AI-Assisted: yes (Claude)

common: flush paging queue in nm_bts_fsm's NOTINSTALLED on_enter

paging_reset() exists to flush stale paging records from the queue,
but was never called anywhere.  Stale paging records can accumulate
when the OML link goes down: osmo-bts does not exit, so the paging
queue survives into the next BSC session.

Call it alongside bts_cbch_reset() and bts_asci_notification_reset()
in st_op_disabled_notinstalled_on_enter(), which fires after all TRXs
are confirmed closed and before a new OML connection is accepted.

Change-Id: I109ab282986b68b68ba5c11859c44b771c0416fd
AI-Assisted: yes (Claude)

oml: validate Intave Parameter range in SET BTS ATTR

3GPP TS 52.021 §9.4.24 defines valid range for the Intave Parameter
as 1..31, matching the fixed size of the per-lchan interference sample
buffer (interf_meas_dbm[31] in lchan.h).  Previously any uint8_t value
was accepted without validation, meaning a buggy BSC could send
intave=0 (silently disabling interference reporting) or intave>31
(causing a buffer overflow in gsm_lchan_interf_meas_push()).

Let's guard against that by NACKing the SET BTS ATTR message with
cause=NM_NACK_PARAM_RANGE if the value is outside the valid range.

Change-Id: Id4d3353d4397aaa2517091b020d38ee15e084e2c
AI-Assisted: yes (Claude)

osmo-bts-trx: fix spurious clock skew shutdown after self-compensation

When the BTS runs ahead of the transceiver (elapsed_fn < 0),
trx_sched_clock() reschedules the timerfd to deliberately delay the
next FN.  osmo_timerfd_schedule() resets the timerfd and discards any
accumulated expirations, but last_fn_timer.tv was left pointing at
the previous callback.  The next trx_fn_timer_cb() then measures
elapsed_us all the way back to that previous callback - spanning the
deliberate delay (or any OS stall that preceded us) - and falsely
trips the "PC clock skew too high" check, shutting the BTS down
for no good reason.

Advance last_fn_timer.tv to the projected firing time of the
rescheduled timer so that the next callback measures roughly
one FN interval, as expected.

Change-Id: Icdb7db8abe70258ae008d9514b6608bd74bb2881
AI-Assisted: yes (Claude)
Related: OS#6794

osmo-bts-trx: fix spurious shutdown on first CLCK.ind from osmo-trx

osmo-trx starts its frame counter from a random value rather than 0.
When the first CLCK.ind arrives, last_fn_timer and last_clk_ind are
still zero-initialised (set by trx_sched_clock_started()), so:

* compute_elapsed_fn(0, fn) wraps to a large negative for any fn
  greater than hyperframe/2 (1357824), satisfying elapsed_fn < 0;
* compute_elapsed_us({0,0}, &tv_now) returns the full CLOCK_MONOTONIC
  uptime (potentially days), satisfying the error_us threshold.

Together these trip the stale-clock shutdown introduced in the previous
commit (0199c108), even though the transceiver is perfectly healthy:

DL1C NOTICE scheduler_trx.c:490 GSM clock started, waiting for clock indications
DL1C FATAL scheduler_trx.c:589 Stale CLCK.ind: fn=1456348 is 250957770198 us behind
DOML NOTICE bts_shutdown_fsm.c:268 BTS_SHUTDOWN(bts0){NONE}: Shutting down BTS, exit 1, reason: TRX clock skew too high

Fix by adding clk_ind_received to osmo_trx_clock_state.  On the first
CLCK.ind after a (re)start, skip all elapsed-time checks and directly
bootstrap the scheduler from the reported FN.  The stale-clock
detection remains fully active for every subsequent indication,
where last_clk_ind holds a real baseline.

Change-Id: I25e76e02d29fd8f88130d15d0adfe8d90a017924
Fixes: 0199c108 ("osmo-bts-trx: shut down on stale clock indication from transceiver")
Related: OS#7021

osmo-bts-trx: fix spurious clock skew shutdown after self-compensation

When the BTS runs ahead of the transceiver (elapsed_fn < 0),
trx_sched_clock() reschedules the timerfd to deliberately delay the
next FN.  osmo_timerfd_schedule() resets the timerfd and discards any
accumulated expirations, but last_fn_timer.tv was left pointing at
the previous callback.  The next trx_fn_timer_cb() then measures
elapsed_us all the way back to that previous callback - spanning the
deliberate delay (or any OS stall that preceded us) - and falsely
trips the "PC clock skew too high" check, shutting the BTS down
for no good reason.

Advance last_fn_timer.tv to the projected firing time of the
rescheduled timer so that the next callback measures roughly
one FN interval, as expected.

Change-Id: Icdb7db8abe70258ae008d9514b6608bd74bb2881
AI-Assisted: yes (Claude)
Related: OS#6794

osmo-bts-trx: fix spurious shutdown on first CLCK.ind from osmo-trx

osmo-trx starts its frame counter from a random value rather than 0.
When the first CLCK.ind arrives, last_fn_timer and last_clk_ind are
still zero-initialised (set by trx_sched_clock_started()), so:

* compute_elapsed_fn(0, fn) wraps to a large negative for any fn
  greater than hyperframe/2 (1357824), satisfying elapsed_fn < 0;
* compute_elapsed_us({0,0}, &tv_now) returns the full CLOCK_MONOTONIC
  uptime (potentially days), satisfying the error_us threshold.

Together these trip the stale-clock shutdown introduced in the previous
commit (0199c108), even though the transceiver is perfectly healthy:

DL1C NOTICE scheduler_trx.c:490 GSM clock started, waiting for clock indications
DL1C FATAL scheduler_trx.c:589 Stale CLCK.ind: fn=1456348 is 250957770198 us behind
DOML NOTICE bts_shutdown_fsm.c:268 BTS_SHUTDOWN(bts0){NONE}: Shutting down BTS, exit 1, reason: TRX clock skew too high

Fix by adding clk_ind_received to osmo_trx_clock_state.  On the first
CLCK.ind after a (re)start, skip all elapsed-time checks and directly
bootstrap the scheduler from the reported FN.  The stale-clock
detection remains fully active for every subsequent indication,
where last_clk_ind holds a real baseline.

Change-Id: I25e76e02d29fd8f88130d15d0adfe8d90a017924
Fixes: 0199c108 ("osmo-bts-trx: shut down on stale clock indication from transceiver")
Related: OS#7021

tls: fix broken certificate hostname verification

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.

Change-Id: If64950a698bfcfbf556a37ef1be3e68abc124384
AI-Assisted: yes (Claude)

tls: fix broken certificate hostname verification

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.

Change-Id: If64950a698bfcfbf556a37ef1be3e68abc124384
AI-Assisted: yes (Claude)

tls: fix broken certificate hostname verification

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.

Change-Id: If64950a698bfcfbf556a37ef1be3e68abc124384
AI-Assisted: yes (Claude)

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.

Change-Id: I419e1b66d07307c3e49294984887c153cd8494c3
AI-Assisted: yes (Claude)

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.

Change-Id: I419e1b66d07307c3e49294984887c153cd8494c3
AI-Assisted: yes (Claude)

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.

Change-Id: I419e1b66d07307c3e49294984887c153cd8494c3
AI-Assisted: yes (Claude)

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.

Change-Id: I0b367d4e255db3208b41e12adec682026b99cc18
AI-Assisted: yes (Claude)

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.

Change-Id: I0b367d4e255db3208b41e12adec682026b99cc18
AI-Assisted: yes (Claude)

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.

Change-Id: I0b367d4e255db3208b41e12adec682026b99cc18
AI-Assisted: yes (Claude)

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().

Change-Id: If2f842b202dd08c07dffe3770c51cf0ce886beee
AI-Assisted: yes (Claude)

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().

Change-Id: If2f842b202dd08c07dffe3770c51cf0ce886beee
AI-Assisted: yes (Claude)

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().

Change-Id: If2f842b202dd08c07dffe3770c51cf0ce886beee
AI-Assisted: yes (Claude)

client: honor IPv4 header length in GPRS filter

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.

Change-Id: Iac1fa9cc2a3c06cbe19c3e7799a0b335f2e3dda9
AI-Assisted: yes (Claude)

client: honor IPv4 header length in GPRS filter

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.

Change-Id: Iac1fa9cc2a3c06cbe19c3e7799a0b335f2e3dda9
AI-Assisted: yes (Claude)

client: honor IPv4 header length in GPRS filter

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.

Change-Id: Iac1fa9cc2a3c06cbe19c3e7799a0b335f2e3dda9
AI-Assisted: yes (Claude)

utils: fix typos

Change-Id: Ib8ede7cbfed9198f59fe2899fe4b68eac5ac9a23

utils/gsmtap-logsend: close infile before exit

Related: SYS#8093
Change-Id: I259cf951c52f7d859475f5f79c803692c29fbec3

utils: fix typos

Change-Id: Ib8ede7cbfed9198f59fe2899fe4b68eac5ac9a23

utils/gsmtap-logsend: close infile before exit

Related: SYS#8093
Change-Id: I259cf951c52f7d859475f5f79c803692c29fbec3

utils: fix typos

Change-Id: Ib8ede7cbfed9198f59fe2899fe4b68eac5ac9a23

utils/gsmtap-logsend: close infile before exit

Related: SYS#8093
Change-Id: I259cf951c52f7d859475f5f79c803692c29fbec3

server: vty: fix docs for cfg_server_rotate_localtime[_mod_n]

Change-Id: I6fd1d081bba035fbd9d72a1705ab58194edf274e
Fixes: 18747641 ("pcap-server: Make rotate-localtime feature configurable through VTY")

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.

Change-Id: Ia56cad48e8cefe8ae103f2f7d2e037bf28438b71
AI-Assisted: yes (Claude)
Related: 6d2f7c52 ("server: Limit rx buffer size to UINT16_MAX")
Related: SYS#8099

doc: clarify 'pcap snaplen' / 'max-snaplen'

Change-Id: Ic2c82173c12814d61f0ee9f7454b9e5dcb0c13f4
Related: SYS#8099

server: vty: fix docs for cfg_server_rotate_localtime[_mod_n]

Change-Id: I6fd1d081bba035fbd9d72a1705ab58194edf274e
Fixes: 18747641 ("pcap-server: Make rotate-localtime feature configurable through VTY")

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.

Change-Id: Ia56cad48e8cefe8ae103f2f7d2e037bf28438b71
AI-Assisted: yes (Claude)
Related: 6d2f7c52 ("server: Limit rx buffer size to UINT16_MAX")
Related: SYS#8099

doc: clarify 'pcap snaplen' / 'max-snaplen'

Change-Id: Ic2c82173c12814d61f0ee9f7454b9e5dcb0c13f4
Related: SYS#8099

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.

Change-Id: Ia56cad48e8cefe8ae103f2f7d2e037bf28438b71
AI-Assisted: yes (Claude)
Related: 6d2f7c52 ("server: Limit rx buffer size to UINT16_MAX")
Related: SYS#8099

server: vty: fix docs for cfg_server_rotate_localtime[_mod_n]

Change-Id: I6fd1d081bba035fbd9d72a1705ab58194edf274e
Fixes: 18747641 ("pcap-server: Make rotate-localtime feature configurable through VTY")

doc: clarify 'pcap snaplen' / 'max-snaplen'

Change-Id: Ic2c82173c12814d61f0ee9f7454b9e5dcb0c13f4
Related: SYS#8099

tls: fix broken certificate hostname verification

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.

Change-Id: If64950a698bfcfbf556a37ef1be3e68abc124384
AI-Assisted: yes (Claude)

tls: fix broken certificate hostname verification

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.

Change-Id: If64950a698bfcfbf556a37ef1be3e68abc124384
AI-Assisted: yes (Claude)

tls: fix broken certificate hostname verification

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.

Change-Id: If64950a698bfcfbf556a37ef1be3e68abc124384
AI-Assisted: yes (Claude)

server: fix zmq message leak on send failure

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.

Change-Id: I501b1bf55bede4e69fa5d9b3f38d87341482ff49
AI-Assisted: yes (Claude)

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.

Change-Id: Ia102ff918ef8152d212e10a860f5dc70efec880b
AI-Assisted: yes (Claude)

client: honor IPv4 header length in GPRS filter

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.

Change-Id: Iac1fa9cc2a3c06cbe19c3e7799a0b335f2e3dda9
AI-Assisted: yes (Claude)

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.

Change-Id: I66581910dbd1e955831a6ff913042059ad4994a7
AI-Assisted: yes (Claude)

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.

Change-Id: I419e1b66d07307c3e49294984887c153cd8494c3
AI-Assisted: yes (Claude)

server: fix msgb leak on duplicate link header

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.

Change-Id: I79344fe942342f2a736878142b3cf036fc982eef
AI-Assisted: yes (Claude)

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().

Change-Id: If2f842b202dd08c07dffe3770c51cf0ce886beee
AI-Assisted: yes (Claude)

tls: fix broken certificate hostname verification

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.

Change-Id: If64950a698bfcfbf556a37ef1be3e68abc124384
AI-Assisted: yes (Claude)

client: vty: do not print NULL tls hostname

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.

Change-Id: I5b920337409d8c9fa1edb8d47177882cf0a6c4e7
AI-Assisted: yes (Claude)

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.

Change-Id: I20d3282b8cba1675ce2d0860e66027e5ee8245ef
AI-Assisted: yes (Claude)

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.

Change-Id: I0b367d4e255db3208b41e12adec682026b99cc18
AI-Assisted: yes (Claude)

tls: fix broken certificate hostname verification

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.

Change-Id: If64950a698bfcfbf556a37ef1be3e68abc124384
AI-Assisted: yes (Claude)

client: vty: do not print NULL tls hostname

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.

Change-Id: I5b920337409d8c9fa1edb8d47177882cf0a6c4e7
AI-Assisted: yes (Claude)

server: fix msgb leak on duplicate link header

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.

Change-Id: I79344fe942342f2a736878142b3cf036fc982eef
AI-Assisted: yes (Claude)

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().

Change-Id: If2f842b202dd08c07dffe3770c51cf0ce886beee
AI-Assisted: yes (Claude)

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.

Change-Id: I66581910dbd1e955831a6ff913042059ad4994a7
AI-Assisted: yes (Claude)

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.

Change-Id: I419e1b66d07307c3e49294984887c153cd8494c3
AI-Assisted: yes (Claude)

client: honor IPv4 header length in GPRS filter

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.

Change-Id: Iac1fa9cc2a3c06cbe19c3e7799a0b335f2e3dda9
AI-Assisted: yes (Claude)

server: fix zmq message leak on send failure

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.

Change-Id: I501b1bf55bede4e69fa5d9b3f38d87341482ff49
AI-Assisted: yes (Claude)

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.

Change-Id: Ia102ff918ef8152d212e10a860f5dc70efec880b
AI-Assisted: yes (Claude)

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.

Change-Id: I20d3282b8cba1675ce2d0860e66027e5ee8245ef
AI-Assisted: yes (Claude)

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.

Change-Id: I0b367d4e255db3208b41e12adec682026b99cc18
AI-Assisted: yes (Claude)

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.

Change-Id: I66581910dbd1e955831a6ff913042059ad4994a7
AI-Assisted: yes (Claude)

client: honor IPv4 header length in GPRS filter

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.

Change-Id: Iac1fa9cc2a3c06cbe19c3e7799a0b335f2e3dda9
AI-Assisted: yes (Claude)

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().

Change-Id: If2f842b202dd08c07dffe3770c51cf0ce886beee
AI-Assisted: yes (Claude)

server: fix msgb leak on duplicate link header

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.

Change-Id: I79344fe942342f2a736878142b3cf036fc982eef
AI-Assisted: yes (Claude)

client: vty: do not print NULL tls hostname

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.

Change-Id: I5b920337409d8c9fa1edb8d47177882cf0a6c4e7
AI-Assisted: yes (Claude)

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.

Change-Id: Ia102ff918ef8152d212e10a860f5dc70efec880b
AI-Assisted: yes (Claude)

tls: fix broken certificate hostname verification

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.

Change-Id: If64950a698bfcfbf556a37ef1be3e68abc124384
AI-Assisted: yes (Claude)

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.

Change-Id: I419e1b66d07307c3e49294984887c153cd8494c3
AI-Assisted: yes (Claude)

server: fix zmq message leak on send failure

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.

Change-Id: I501b1bf55bede4e69fa5d9b3f38d87341482ff49
AI-Assisted: yes (Claude)

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.

Change-Id: I0b367d4e255db3208b41e12adec682026b99cc18
AI-Assisted: yes (Claude)

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.

Change-Id: I20d3282b8cba1675ce2d0860e66027e5ee8245ef
AI-Assisted: yes (Claude)

struct gsm_bts: drop unused ms_max_power

Change-Id: I0b02015db8b8e670eaff40c578f0474d9be9bb45

tests/meas: remove unused 'delta'

gcc 16.1.1 emits a -Wunused-but-set-variable warning.

Change-Id: I2540d701743caefb4bf54bb5b4ebe683d3257071

osmo-bts-trx: fix spurious clock skew shutdown after self-compensation

When the BTS runs ahead of the transceiver (elapsed_fn < 0),
trx_sched_clock() reschedules the timerfd to deliberately delay the
next FN.  osmo_timerfd_schedule() resets the timerfd and discards any
accumulated expirations, but last_fn_timer.tv was left pointing at
the previous callback.  The next trx_fn_timer_cb() then measures
elapsed_us all the way back to that previous callback - spanning the
deliberate delay (or any OS stall that preceded us) - and falsely
trips the "PC clock skew too high" check, shutting the BTS down
for no good reason.

Advance last_fn_timer.tv to the projected firing time of the
rescheduled timer so that the next callback measures roughly
one FN interval, as expected.

Change-Id: Icdb7db8abe70258ae008d9514b6608bd74bb2881
AI-Assisted: yes (Claude)
Related: OS#6794

common: track whether gsm_time has been initialized

l1sap_info_time_ind() used 'bts->gsm_time.fn != 0' as a proxy for
"we have a previous frame number to diff against".  This is unreliable:
Fn=0 is a _valid_ frame number, recurring on every hyperframe wrap.
If gsm_time.fn happened to be 0 and the next time indication jumped
forward by more than one frame, the real gap was silently swallowed.

It also gave no clean way to suppress the bogus "Invalid condition
detected: Frame difference is ..." message that appears when the PHY
(re)starts its TDMA frame number (e.g. from 0) on bring-up.

Introduce an explicit 'bts->gsm_time_valid' flag instead:

* l1sap_info_time_ind() treats the first indication of an epoch as
  having no gap (frames_expired = 0): no warning, no RACH-slot
  accounting;
* the flag is cleared in st_op_disabled_notinstalled_on_enter(), so
  each BTS bring-up starts a fresh clock epoch regardless of which
  FN the PHY reports first.

Change-Id: I7022b0ad084a0c224f7e8c04aca0648915b1a1c6
AI-Assisted: yes (Claude)
Related: OS#7020

osmo-bts-trx: shut down on stale clock indication from transceiver

We expect the transceiver to be a reliable, monotonic clock source.
If it reports an FN far behind our local timer (elapsed_fn < 0) while
far more wall-clock time elapsed than its FN advance accounts for,
its clock has likely stalled and the indication carries a stale frame
number.  Acting on it drags the scheduler backwards and re-transmits
already-sent TDMA frames, corrupting lchan-internal state(s).

Detect this and shut down the process, same rationale as the existing
"PC clock skew too high" check in trx_fn_timer_cb().

Change-Id: If787ab7ed70aa2dcb0389ceb58620c2302c3431a
AI-Assisted: yes (Claude)
Related: OS#7020, OS#6794

struct gsm_bts: drop unused ms_max_power

Change-Id: I0b02015db8b8e670eaff40c578f0474d9be9bb45

tests/meas: remove unused 'delta'

gcc 16.1.1 emits a -Wunused-but-set-variable warning.

Change-Id: I2540d701743caefb4bf54bb5b4ebe683d3257071

common: track whether gsm_time has been initialized

l1sap_info_time_ind() used 'bts->gsm_time.fn != 0' as a proxy for
"we have a previous frame number to diff against".  This is unreliable:
Fn=0 is a _valid_ frame number, recurring on every hyperframe wrap.
If gsm_time.fn happened to be 0 and the next time indication jumped
forward by more than one frame, the real gap was silently swallowed.

It also gave no clean way to suppress the bogus "Invalid condition
detected: Frame difference is ..." message that appears when the PHY
(re)starts its TDMA frame number (e.g. from 0) on bring-up.

Introduce an explicit 'bts->gsm_time_valid' flag instead:

* l1sap_info_time_ind() treats the first indication of an epoch as
  having no gap (frames_expired = 0): no warning, no RACH-slot
  accounting;
* the flag is cleared in st_op_disabled_notinstalled_on_enter(), so
  each BTS bring-up starts a fresh clock epoch regardless of which
  FN the PHY reports first.

Change-Id: I7022b0ad084a0c224f7e8c04aca0648915b1a1c6
AI-Assisted: yes (Claude)
Related: OS#7020

osmo-bts-trx: fix spurious clock skew shutdown after self-compensation

When the BTS runs ahead of the transceiver (elapsed_fn < 0),
trx_sched_clock() reschedules the timerfd to deliberately delay the
next FN.  osmo_timerfd_schedule() resets the timerfd and discards any
accumulated expirations, but last_fn_timer.tv was left pointing at
the previous callback.  The next trx_fn_timer_cb() then measures
elapsed_us all the way back to that previous callback - spanning the
deliberate delay (or any OS stall that preceded us) - and falsely
trips the "PC clock skew too high" check, shutting the BTS down
for no good reason.

Advance last_fn_timer.tv to the projected firing time of the
rescheduled timer so that the next callback measures roughly
one FN interval, as expected.

Change-Id: Icdb7db8abe70258ae008d9514b6608bd74bb2881
AI-Assisted: yes (Claude)
Related: OS#6794

osmo-bts-trx: shut down on stale clock indication from transceiver

We expect the transceiver to be a reliable, monotonic clock source.
If it reports an FN far behind our local timer (elapsed_fn < 0) while
far more wall-clock time elapsed than its FN advance accounts for,
its clock has likely stalled and the indication carries a stale frame
number.  Acting on it drags the scheduler backwards and re-transmits
already-sent TDMA frames, corrupting lchan-internal state(s).

Detect this and shut down the process, same rationale as the existing
"PC clock skew too high" check in trx_fn_timer_cb().

Change-Id: If787ab7ed70aa2dcb0389ceb58620c2302c3431a
AI-Assisted: yes (Claude)
Related: OS#7020, OS#6794

osmo-bts-trx: shut down on stale clock indication from transceiver

We expect the transceiver to be a reliable, monotonic clock source.
If it reports an FN far behind our local timer (elapsed_fn < 0) while
far more wall-clock time elapsed than its FN advance accounts for,
its clock has likely stalled and the indication carries a stale frame
number.  Acting on it drags the scheduler backwards and re-transmits
already-sent TDMA frames, corrupting lchan-internal state(s).

Detect this and shut down the process, same rationale as the existing
"PC clock skew too high" check in trx_fn_timer_cb().

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Change-Id: If787ab7ed70aa2dcb0389ceb58620c2302c3431a
Related: OS#7020, OS#6794

osmo-bts-trx: shut down on stale clock indication from transceiver

We expect the transceiver to be a reliable, monotonic clock source.
If it reports an FN far behind our local timer (elapsed_fn < 0) while
far more wall-clock time elapsed than its FN advance accounts for,
its clock has likely stalled and the indication carries a stale frame
number.  Acting on it drags the scheduler backwards and re-transmits
already-sent TDMA frames, corrupting lchan-internal state(s).

Detect this and shut down the process, same rationale as the existing
"PC clock skew too high" check in trx_fn_timer_cb().

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Change-Id: If787ab7ed70aa2dcb0389ceb58620c2302c3431a
Related: OS#7020, OS#6794

osmo-bts-trx: reset BTS GSM time on clock (re)start

trx_sched_clock_started() is called when the transceiver is powered
on, e.g. after an A-bis link re-establishment, which in ttcn3-bts-test
happens once per testcase.  The transceiver restarts its TDMA frame
number from ~0, but we only reset the per-TRX clock state (tcs),
leaving bts->gsm_time.fn at the previous epoch's value.

As a result the first (low) FN reported by the transceiver is mistaken
for a huge backwards jump, e.g.:

  l1sap.c:628 Invalid condition detected: Frame difference is 102-10386=2705364 > 1!

Reset bts->gsm_time to FN 0 here so each clock epoch starts clean.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Change-Id: Id8da126e460d3846a3be5bdb271553457fdd0590

osmo-bts-trx: reset BTS GSM time on clock (re)start

trx_sched_clock_started() is called when the transceiver is powered
on, e.g. after an A-bis link re-establishment, which in ttcn3-bts-test
happens once per testcase.  The transceiver restarts its TDMA frame
number from ~0, but we only reset the per-TRX clock state (tcs),
leaving bts->gsm_time.fn at the previous epoch's value.

As a result the first (low) FN reported by the transceiver is mistaken
for a huge backwards jump, e.g.:

  l1sap.c:628 Invalid condition detected: Frame difference is 102-10386=2705364 > 1!

Reset bts->gsm_time to FN 0 here so each clock epoch starts clean.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Change-Id: Id8da126e460d3846a3be5bdb271553457fdd0590

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

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

tls: fix broken certificate hostname verification

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

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

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

client: vty: do not print NULL tls hostname

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

client: honor IPv4 header length in GPRS filter

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 zmq message leak on send failure

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

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

server: fix msgb leak on duplicate link header

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

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

tls: fix broken certificate hostname verification

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

server: fix zmq message leak on send failure

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 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

client: honor IPv4 header length in GPRS filter

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

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

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

client: vty: do not print NULL tls hostname

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

server: fix msgb leak on duplicate link header

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

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

tls: fix broken certificate hostname verification

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: vty: do not print NULL tls hostname

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

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

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

server: fix zmq message leak on send failure

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: honor IPv4 header length in GPRS filter

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

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

server: fix msgb leak on duplicate link header

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

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: vty: fix docs for cfg_server_rotate_localtime[_mod_n]

Change-Id: I6fd1d081bba035fbd9d72a1705ab58194edf274e
Fixes: 18747641 ("pcap-server: Make rotate-localtime feature configurable through VTY")

server: vty: fix docs for cfg_server_rotate_localtime[_mod_n]

Change-Id: I6fd1d081bba035fbd9d72a1705ab58194edf274e
Fixes: 18747641 ("pcap-server: Make rotate-localtime feature configurable through VTY")

server: vty: fix docs for cfg_server_rotate_localtime[_mod_n]

Change-Id: I6fd1d081bba035fbd9d72a1705ab58194edf274e
Fixes: 18747641 ("pcap-server: Make rotate-localtime feature configurable through VTY")

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

doc: clarify 'pcap snaplen' / 'max-snaplen'

Change-Id: Ic2c82173c12814d61f0ee9f7454b9e5dcb0c13f4
Related: SYS#8099

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

doc: clarify 'pcap snaplen' / 'max-snaplen'

Change-Id: Ic2c82173c12814d61f0ee9f7454b9e5dcb0c13f4
Related: SYS#8099

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

doc: clarify 'pcap snaplen' / 'max-snaplen'

Change-Id: Ic2c82173c12814d61f0ee9f7454b9e5dcb0c13f4
Related: SYS#8099