fix(proto-gtpv2c): correct IPCP receive handling in PCO/APCO - #602
Merged
Conversation
VerifiedOrganic
force-pushed
the
fix/587-ipcp-receive-correctness
branch
2 times, most recently
from
July 28, 2026 04:41
48a922a to
f26e6d3
Compare
Two defects in the PCO/APCO IPCP decoder, both peer-triggerable. An IPCP Configure-Nak was accepted with no correlation. RFC 1661 5.3 requires the Identifier to match the last transmitted Configure-Request and says invalid packets are silently discarded; the decoder recorded the Identifier as carrying nothing it interprets, and the entry point took no expected value, so it structurally could not correlate. The first non-zero address in any Nak became the session's DNS answer. A malformed IPCP unit also destroyed unrelated siblings. RFC 1661's discard unit is the packet, and TS 24.008 10.5.6.3 maps one 0x8021 unit to one RFC 1661 packet. The unit's outer container boundary is already validated, so sibling boundaries are recoverable and the correct disposition is a unit-local discard. decode_network_contents_correlated takes an IpcpNakCorrelation carrying the caller's outstanding request. Default and none() fail closed and discard every Nak. expecting() is the RFC-permissive reading, since 5.3 lets a peer append options that were not requested; for_request() additionally declines unsolicited options, which is labelled engineering judgement rather than a requirement. The existing entry point keeps its signature and delegates with no correlation. Intra-unit atomicity is structural, not commentary: the unit decoder parses into scratch state and merges only on whole-unit success, so an option parsed before a later malformed one in the same packet cannot survive it. Discards are reported as bounded evidence carrying an index and a reason, never an address or an Identifier value. Malformed known address containers still reject the whole value. TS 24.008 states no receiver disposition there, so that is this codec's configuration-atomicity policy and is now documented as such rather than as a specification requirement. Refs #587 Signed-off-by: VerifiedOrganic <verifiedorganic@sent.com>
Treat every registered network-to-MS container as the monotonic boundary between configuration protocols and additional parameters. Validate the syntax of non-Nak configuration packets without claiming full PPP response semantics. Signed-off-by: VerifiedOrganic <verifiedorganic@sent.com>
VerifiedOrganic
force-pushed
the
fix/587-ipcp-receive-correctness
branch
from
July 28, 2026 12:18
f26e6d3 to
c2020bb
Compare
Contributor
Author
|
Approved and merged: CI green and logic verified. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Corrects peer-reachable IPCP receive handling in PCO/APCO (refs #587):
The first implementation treated IPCP and additional-parameter containers as freely interleavable. That contradicted the wire grammar and allowed a correlated DNS Nak after the additional-parameters boundary. The repair uses a monotonic boundary and classifies the full registered network-to-MS identifier space from TS 24.008 V20.0.0 Table 10.5.154, including unsupported, reserved, and operator-specific identifiers. An unsupported configuration protocol such as LCP does not start that boundary.
Discard evidence remains bounded and redaction-safe: it carries only a unit index and reason, never an address or Identifier value. The public discard-reason enum was already non-exhaustive.
Compatibility
No existing function signature or public item is removed. Behaviour changes are deliberate: uncorrelated and out-of-order IPCP DNS is no longer surfaced, malformed IPCP is contained to its unit, and malformed non-Nak configuration option framing is reported as a unit-local discard.
Verification
The repository does not publish a line-coverage check for this PR; validation relies on the required CI matrix and named regression coverage.