fix(proto-gtpv2c): correct IPCP receive handling in PCO

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>

Author: VerifiedOrganic

CHANGELOG.md

@@ -13,6 +13,35 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
   can install one sink allocation in both the gNMI and NETCONF server cores
   without a product-local forwarding newtype; calls and errors are forwarded
   unchanged.
+- **Correlated IPCP Configure-Nak receive in PCO/APCO — `opc-proto-gtpv2c`:**
+  `IpcpNakCorrelation`, `PcoDecoded`, `PcoIpcpDiscard`, `PcoIpcpDiscardReason`
+  and `PcoAddressConfiguration::decode_network_contents_correlated`. RFC 1661
+  §5.3 states verbatim: "On reception of a Configure-Nak, the Identifier field
+  MUST match that of the last transmitted Configure-Request. Invalid packets are
+  silently discarded." The previous decoder took no expected Identifier and so
+  structurally could not correlate, which let the first non-zero address in any
+  Configure-Nak become the session's DNS answer. The new entry point takes the
+  caller's outstanding-request position and discards a Nak that does not match
+  it. `IpcpNakCorrelation::none` is the `Default` and discards every
+  Configure-Nak, so the fail-closed position is the one a caller reaches by
+  accident; `expecting(identifier)` is the RFC-permissive constructor;
+  `for_request(sent)` correlates against a request this SDK encoded.
+  Every unit dropped on correlation, every unit dropped as malformed, and every
+  DNS option skipped as unsolicited is reported through
+  `PcoDecoded::ipcp_discards` with a reason code and a unit position and never
+  an address or an Identifier value, extending the existing
+  `PcoAddressConfiguration` redaction contract. Four drops stay deliberately
+  silent and record no entry, as before: a well-formed code other than
+  Configure-Nak, an unknown option type, an echoed RFC 1877 all-zero address,
+  and a repeated option whose slot is already filled.
+  `for_request` additionally declines a DNS option this side never solicited.
+  **That filter is engineering judgement, not a specification requirement:** RFC
+  1661 §5.3 permits a Configure-Nak to append Configuration Options the peer
+  desires that were not in the Configure-Request, so the unit is kept and only
+  the option is skipped, reported as `UnsolicitedOption`. Use `expecting` for
+  the permissive reading. It is not an on-path control either — the Identifier
+  is visible on the wire, and `IpcpDnsRequest::identifier` is documented as
+  opaque with nothing obliging a caller to vary it.
 - **First-owner activation for destination-scoped steering — `opc-ipsec-lb`:**
   in `HostXdpFenceDomain::PerOwnershipKey` the only public owner-map writer was
   the fenced re-pin coordinator, and a re-pin cannot be formed for a fresh SA --
@@ -122,6 +151,44 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
   explicitly replace a v1 trail before enabling v2 writes; the SDK does not
   silently reseal it. A v2 anchor whose version tag alone is downgraded fails
   authentication and is not classified as benign legacy data.
+- **A malformed IPCP unit is discarded unit-locally instead of failing the whole
+  PCO/APCO value — `opc-proto-gtpv2c` (breaking, behavioural; no signature
+  changes):** `decode_network_contents` propagated an IPCP fault out of the
+  whole value, destroying P-CSCF and DNS containers that had already parsed.
+  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 stripped of Protocol and Padding; the
+  unit's outer container boundary is validated before its contents are read, so
+  the sibling boundaries are recoverable and the siblings are now kept. Within
+  one unit the disposition is atomic: an option that parsed before a later
+  malformed one in the same packet is dropped with it, which the unit decoder
+  now expresses in its type by returning no `Result` at all and merging only on
+  whole-unit success. `PcoDecodeError::IpcpHeaderTruncated`, `IpcpLengthInvalid`,
+  `IpcpOptionTruncated`, `IpcpOptionLengthInvalid` and
+  `IpcpDnsOptionLengthInvalid` stop being returned from the decode entry points;
+  they remain public, constructible and reachable through
+  `PcoIpcpDiscardReason::Malformed`, and no variant was removed or reordered.
+  **This is a reject-to-accept flip: a caller matching on those five variants
+  will stop seeing them.**
+- **`decode_network_contents` no longer surfaces IPCP-supplied DNS —
+  `opc-proto-gtpv2c` (breaking, behavioural; signature unchanged):** holding no
+  Identifier it cannot satisfy RFC 1661 §5.3, so the fail-closed answer is to
+  supply nothing. `ipcp_primary_dns`/`ipcp_secondary_dns` stay `None`; for a
+  value whose only DNS source was the IPCP reply `is_empty()` reports empty and
+  the caller's configured-DNS fallback fires as that predicate already
+  documents; and `dns_server_ipv4_all()` equals
+  `dns_server_ipv4` under this path. Migration:
+  `decode_network_contents_correlated(value, IpcpNakCorrelation::for_request(sent.ipcp_dns))?.into_configuration()`.
+  The function is deliberately **not** `#[deprecated]`: it stays the correct call
+  for a value that carries only containers.
+  These two changes have opposite signs and are declared as such — the decoder
+  is now **more** permissive about a malformed sibling unit and **less**
+  permissive about an uncorrelated reply. Both move toward RFC 1661 fidelity;
+  neither is simply hardening.
+  Unchanged, stated explicitly: `PcoAddressConfiguration`'s fields, derives and
+  redacting `Debug`; every `PcoDecodeError` variant and its `as_str`;
+  container-framing fatality; whole-value rejection for a wrong-length address
+  container; the IPv4 Link MTU local skip; and the entire encode path,
+  `PcoRequest`, `IpcpDnsRequest`, `PcscfRequest` and `PcscfAddressRequest`.
 - **`RePinAuditEvent` carries a correlation digest, not the live transition
   secret — `opc-ipsec-lb` (breaking: `transition_id: OwnershipTransitionId` is
   replaced by `correlation_id: RePinAuditCorrelationId`):** the coordinator
@@ -319,6 +386,20 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
     fixture, so no message that already round-tripped moves on the wire.
 
 ### Fixed
+- **Documentation corrections in the PCO codec — `opc-proto-gtpv2c`:** the RFC
+  1661 citation on the Configure-Ack option-echo rule said §5.3, which is
+  Configure-Nak; §5.2 is Configure-Ack. `dns_server_ipv4_all` and the crate
+  README claimed the accessor "drops duplicates" generally, but the container
+  list is cloned verbatim and only the two IPCP-sourced addresses are checked
+  against it; the wording now says so, and the behaviour is deliberately
+  unchanged because a repeated address container is something the peer actually
+  sent and collapsing it would destroy that evidence. The
+  `decode_network_contents` contract said "parsing is all-or-nothing" beside a
+  comment that does cite a clause, which read as spec-compelled; whole-value
+  rejection for a wrong-length **address** container is relabelled as this
+  codec's configuration-atomicity policy, which TS 24.008 does not require. The
+  comment claiming the IPCP Identifier "carries nothing this decoder interprets"
+  is deleted.
 - **P-CSCF Re-selection support is no longer emittable on its own --
   `opc-proto-gtpv2c` (breaking to `PcoRequest`):** TS 24.008 10.5.6.3 says of
   container `0x0012` that "This PCO parameter may be present only if a
@@ -610,16 +691,16 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
   - *Receive.* Emitting a request whose answer is discarded would deliver
     nothing, so `PcoAddressConfiguration` now decodes the reply into
     `ipcp_primary_dns` and `ipcp_secondary_dns`. Only a Configure-Nak is read
-    for addresses: RFC 1661 5.3 has a Configure-Ack echo the request's options
+    for addresses: RFC 1661 §5.2 has a Configure-Ack echo the request's options
     verbatim, so it conveys no server, and an echoed all-zero address is not
-    treated as one. `dns_server_ipv4_all()` merges the container and IPCP
-    sources and drops duplicates, so a caller cannot silently miss the
-    mechanism its peer chose.
+    treated as one. `dns_server_ipv4_all()` reports both the container and IPCP
+    sources, so a caller cannot silently miss the mechanism its peer chose. Both
+    the correlation requirement and the disposition of a malformed unit are
+    superseded within this same unreleased section; see the correlated-receive
+    entry under Added and the unit-local-discard entry under Changed.
   - Both structs gain fields, which breaks exhaustive struct literals;
     `..PcoRequest::none()` and `..Default::default()` are unaffected. Five new
-    `PcoDecodeError` variants report malformed IPCP framing, and a malformed
-    unit for this now-supported identifier rejects the whole value, matching
-    how a known container with a bad length is already handled. `Debug` reports
+    `PcoDecodeError` variants report malformed IPCP framing. `Debug` reports
     presence, never addresses.
 
 - **Credential-rotation observability closes three residual gaps —

crates/opc-proto-gtpv2c/CONFORMANCE.md

@@ -364,11 +364,19 @@ coverage.
      the all-zero address that requests a peer-supplied value. Because the
      configuration protocol options list occupies octets 4..w and the
      additional parameters list w+1..z, the unit is encoded ahead of every
-     container. On receive, only a Configure-Nak is read for addresses: a
-     Configure-Ack echoes the request's options verbatim and so conveys no
-     server, and an echoed all-zero address is not treated as one. A malformed
-     unit for this now-supported identifier rejects the whole value, matching
-     how a known container with a bad length is handled.
+     container. On receive, only a Configure-Nak whose Identifier matches the
+     outstanding Configure-Request is read for addresses, as RFC 1661 5.3
+     requires; the uncorrelated `decode_network_contents` entry point holds no
+     Identifier and so reads none. A Configure-Ack echoes the request's options
+     verbatim and so conveys no server, and an echoed all-zero address is not
+     treated as one. A malformed unit for this identifier is discarded
+     unit-locally and reported through `PcoDecoded::ipcp_discards`, and its
+     sibling containers survive: RFC 1661's discard unit is the packet and
+     10.5.6.3 maps one unit to one such packet, so a fault inside a unit whose
+     outer container boundary already validated does not reach the value. That
+     is unlike a known address container with a bad length, which still rejects
+     the whole value under this codec's configuration-atomicity policy, for
+     which the specification states no receiver disposition.
    - The IPv4 Link MTU container `0x0010` is supported in both directions, with
      the direction-dependent shape Table 10.5.154 assigns: zero-length request
      MS to network, two-octet value network to MS. 10.5.6.3 states that a

crates/opc-proto-gtpv2c/README.md

@@ -125,10 +125,40 @@ control-plane stack.
 - `PcoRequest::ipcp_dns` emits an RFC 1332 IPCP Configure-Request carrying the
   RFC 1877 Primary (129) and Secondary (131) DNS Server Address options, which
   TS 24.008 10.5.6.3 places in the configuration protocol options list ahead of
-  every container. The decoder reads the peer's Configure-Nak answer into
-  `ipcp_primary_dns`/`ipcp_secondary_dns`. A peer may serve DNS by either
-  mechanism, so prefer `PcoAddressConfiguration::dns_server_ipv4_all`, which
-  merges both sources and drops duplicates.
+  every container. `PcoAddressConfiguration::decode_network_contents_correlated`
+  reads the peer's Configure-Nak answer into
+  `ipcp_primary_dns`/`ipcp_secondary_dns`, but only once the reply is
+  correlated: RFC 1661 5.3 says "On reception of a Configure-Nak, the Identifier
+  field MUST match that of the last transmitted Configure-Request. Invalid
+  packets are silently discarded." Pass `IpcpNakCorrelation::for_request` built
+  from the `IpcpDnsRequest` that was sent. `IpcpNakCorrelation::none` is the
+  `Default`, and the uncorrelated `decode_network_contents` uses it, so that
+  entry point surfaces no IPCP-supplied DNS at all. Every unit dropped on
+  correlation, every unit dropped as malformed, and every DNS option skipped as
+  unsolicited is reported through `PcoDecoded::ipcp_discards`, which carries
+  reason codes and unit positions and never an address or an Identifier value.
+  Four drops are deliberately silent and record no entry, as before: a
+  well-formed code other than Configure-Nak, an unknown option type, an echoed
+  RFC 1877 all-zero address, and a repeated option whose slot is already filled.
+- A malformed `0x8021` unit is discarded unit-locally rather than failing the
+  whole PCO/APCO value: RFC 1661's discard unit is the packet, TS 24.008
+  10.5.6.3 maps one `0x8021` unit to one RFC 1661 packet, and the unit's outer
+  container boundary has already been validated, so the sibling P-CSCF and DNS
+  containers are recoverable and survive. Within one unit the disposition is
+  atomic -- an option that parsed before a later malformed one in the same
+  packet is dropped with it. Container framing and wrong-length address
+  containers still reject the whole value.
+- `for_request` additionally declines a DNS option this side never solicited.
+  That is engineering judgement and not a specification requirement: RFC 1661
+  5.3 permits a Configure-Nak to append options the peer desires that were not
+  in the Configure-Request, so the unit is kept and only the option is skipped.
+  Use `IpcpNakCorrelation::expecting` for the RFC-permissive reading, which
+  accepts both DNS options.
+- A peer may serve DNS by either mechanism, so prefer
+  `PcoAddressConfiguration::dns_server_ipv4_all`. Container addresses come
+  first, in wire order and with their multiplicity preserved; the IPCP primary
+  and secondary addresses follow, each appended only if the list does not
+  already contain it. The container list itself is never deduplicated.
 - `PcoRequest::ipv4_link_mtu` emits the zero-length IPv4 Link MTU Request
   container `0x0010`, and `PcoAddressConfiguration::ipv4_link_mtu` carries the
   two-octet value the network returns under the same identifier. TS 24.008

crates/opc-proto-gtpv2c/src/lib.rs

@@ -32,7 +32,10 @@
 //! unaccompanied combination does not compile. [`PcoRequest`] also emits an
 //! IPCP (`0x8021`) Configure-Request for the RFC 1877 DNS options, ahead of
 //! the containers as TS 24.008 10.5.6.3 positions it, and
-//! [`PcoAddressConfiguration`] reads the peer's Configure-Nak answer, and the
+//! [`PcoAddressConfiguration::decode_network_contents_correlated`] reads the
+//! peer's Configure-Nak answer once [`IpcpNakCorrelation`] correlates it to the
+//! Identifier that was sent, as RFC 1661 5.3 requires; a malformed `0x8021`
+//! unit is discarded on its own and leaves its sibling containers standing. The
 //! IPv4 Link MTU container `0x0010` is carried in both directions.
 //! Product code remains responsible for deciding when optional policy-owned
 //! values apply and for obtaining them from AAA/HSS or local configuration.
@@ -127,11 +130,11 @@ pub use ie::{
 };
 pub use message::{Message, OwnedMessage};
 pub use pco::{
-    IpcpDnsRequest, PcoAddressConfiguration, PcoDecodeError, PcoRequest, PcscfAddressRequest,
-    PcscfRequest, PCO_CONTAINER_DNS_SERVER_IPV4, PCO_CONTAINER_DNS_SERVER_IPV6,
-    PCO_CONTAINER_IPV4_LINK_MTU, PCO_CONTAINER_P_CSCF_IPV4, PCO_CONTAINER_P_CSCF_IPV6,
-    PCO_CONTAINER_P_CSCF_RESELECTION_SUPPORT, PCO_HEADER_PPP_FOR_IP_PDN, PCO_MAX_CONTAINERS,
-    PCO_PROTOCOL_IPCP,
+    IpcpDnsRequest, IpcpNakCorrelation, PcoAddressConfiguration, PcoDecodeError, PcoDecoded,
+    PcoIpcpDiscard, PcoIpcpDiscardReason, PcoRequest, PcscfAddressRequest, PcscfRequest,
+    PCO_CONTAINER_DNS_SERVER_IPV4, PCO_CONTAINER_DNS_SERVER_IPV6, PCO_CONTAINER_IPV4_LINK_MTU,
+    PCO_CONTAINER_P_CSCF_IPV4, PCO_CONTAINER_P_CSCF_IPV6, PCO_CONTAINER_P_CSCF_RESELECTION_SUPPORT,
+    PCO_HEADER_PPP_FOR_IP_PDN, PCO_MAX_CONTAINERS, PCO_PROTOCOL_IPCP,
 };
 #[allow(deprecated)]
 pub use s2b::{