From b60a3633c10b6f3d8b980a3306ee9a1bfbbe5c86 Mon Sep 17 00:00:00 2001 From: Jorijn van der Graaf Date: Sun, 11 Oct 2026 15:55:19 +0200 Subject: [PATCH] =?UTF-8?q?ipsec,=20imsd,=20ims-pdn-up:=20the=20SA=20reade?= =?UTF-8?q?r=20and=20the=20holds=20key=20compare=20in=20canonical=20form;?= =?UTF-8?q?=20LOCAL=20is=20canonicalised,=20PCSCF=3D=20accepted=20in=20RFC?= =?UTF-8?q?=205952=20=C2=A74=20form=20only?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ParseExistingSa and SaLocals compared PCSCF= and LOCAL= as text against `ip xfrm` output, which prints RFC 5952 form, so an upper-case or uncompressed PCSCF= found no SAs and the restart registered fresh — on KPN into the live-SA refusal; the holds file's pdn key compared the same way. Both sides of each of those compares now go through imsd::util::CanonicalAddr, LOCAL is canonicalised once on the way in, and PCSCF= is accepted in that one spelling only — upper case, leading zeros or an uncompressed run are refused at bring-up with the spelling to use, a dotted-quad tail (the form ip prints for an IPv4-mapped address, which CanonicalIp6 does not read) with the rule alone, a bracketed one by the bring-up script, which routes the value as typed, with the rule (no brackets) — and a comma-separated PCSCF= (a list) with its own message instead of reaching the socket layer: one spelling, instead of a leniency both consumers of the setting would have to share. Tested: crafter-build test, 7 passed; the upper-case PCSCF= refused with the spelling to use, a bracketed one refused by the script, and the spelling they name resuming, in builds of this series on the dev phone (KPN). --- AGENTS.md | 41 ++++++++++++++++++++++++++++++++----- README.md | 4 ++-- implementations/main.cpp | 18 ++++++++++++---- interfaces/Imsd-Ipsec.cppm | 24 ++++++++++++++-------- packaging/ims-pdn-up.sh | 1 + packaging/imsd.post-upgrade | 6 ++++-- tests/Ipsec/main.cpp | 12 +++++++++++ 7 files changed, 84 insertions(+), 22 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 54b4f1a..0eb0c29 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -73,7 +73,7 @@ in every source but the re-exporting `Imsd.cppm`, clang + libc++ + lld. The rule behind the split: **`imsd-core` stays GLib-free and free of I/O so that every decision in it has a unit test.** New protocol logic goes into a partition with a test. The shell (`main.cpp`) holds protocol decisions no -test reaches — some older than 0.3.10, some added by 0.3.10, 0.3.11 or 0.3.12: the reg-event +test reaches — some older than 0.3.10, some added by 0.3.10, 0.3.11, 0.3.12 or 1.0.0: the reg-event subscription schedule (§7.4: its retry ladder, Retry-After clamp, Timer F, renewal point and RFC 6665 reasons); the registration sequence and its transactions (`AwaitFinal`/`AwaitInitialFinal`: @@ -101,7 +101,9 @@ closed when the call ends, a reconnect cycle's or the bring-up resume's wait ended by it — the reducer's verdict is pinned, the reaction is not); and the holds file's codec (`PersistHolds`/`LoadHolds`: aging by boot seconds, the pdn/boot match, defaults for absent keys — proven on the phone only, captures -45- and 70- of 2026-09-25-polkit-voice). For the schedule, a reducer next to +45- and 70- of 2026-09-25-polkit-voice; the `pdn` compare in canonical form on +both sides (1.0.0) is unbenched: no capture stages a holds file whose `pdn` is +spelled unlike `LOCAL`). For the schedule, a reducer next to `RegRecovery`, with a Subscription-State parser in `Imsd:Sip` pinned by `tests/Sip`, is the follow-up that moves it; a pure format/parse pair in `Imsd:Engine` is the holds file's. @@ -431,7 +433,25 @@ The daemon's own bring-up (`BringUp()` in `main.cpp`): re-REGISTER (`RESUME via existing SA`, then `REGISTERED (resumed + true-refreshed)`). This avoids the network's fresh-SA throttle. `RESUME=0` forces a fresh registration. A resume the network refuses is not fatal: the - fresh registration replaces its SAs. + fresh registration replaces its SAs. The SA reader, the holds file's `pdn` + key, `LOCAL` and `PCSCF` compare addresses in RFC 5952 §4 form + (`imsd::util::CanonicalAddr`: lower-case hex, leading zeros dropped, the + longest zero run compressed — what `ip xfrm` prints for a plain IPv6 + address; an IPv4-mapped address is read in that hex form only, never with + the dotted tail `ip` prints), and `PCSCF=` is accepted in that one spelling + only: upper case, leading zeros or an uncompressed run are refused at + bring-up with the one to use (`PCSCF='…' is not written as ip prints it; + use PCSCF=…`), a dotted-quad tail with the rule but no spelling, a + bracketed value by the bring-up script with the rule (`write the bare + address, as ip prints it (no brackets)`), a comma-separated `PCSCF=` (a + list) with its own message (`must be one address`; in the packaged unit the + bring-up script, which runs first, fails a list before the daemon sees it) + — one spelling, so the + bring-up script, which routes the value as typed, and the daemon agree on + it. Before 1.0.0 an upper-case, zero-padded or uncompressed `PCSCF=` was + accepted and, compared as text against `ip xfrm`, matched no SA and + registered fresh at every restart; an upgraded phone with such a setting + stops at bring-up with the spelling to use. 4. **Fresh registration** otherwise (`FRESH registration`): an unprotected REGISTER on `PCSCF_PORT` (UDP, always), the 401 with the AKA challenge and the `Security-Server` list, the AKA run over a UIM logical channel @@ -531,7 +551,11 @@ configures the muxed netdev. Idempotent; a non-zero exit fails the unit. - Sets the PDN MTU ModemManager reports on the link (1280 on the FP6), adds the address, waits out DAD, and installs a host route to `PCSCF` (`dev `, no gateway — a raw-IP netdev without header_ops). The PDN - is IPv6-only, so `PCSCF` must be an IPv6 address; anything else on the IMS + is IPv6-only, so `PCSCF` must be an IPv6 address in RFC 5952 §4 form (§7.2 step 3: what + `ip` prints for a plain IPv6 address; an IPv4-mapped address in hex only — + a bracketed value is refused here with that hint, an upper-case or + uncompressed one by the daemon with the spelling to use once the route + step took it); anything else on the IMS network (its DNS servers, a traceroute target) needs its own host route (`ip -6 route add dev qmapmux0.0`). imsd installs a host route per media peer itself. @@ -647,7 +671,9 @@ packages' `post-upgrade`/`post-install` scripts mark the units `daemon-reload` when the unit is not-found because its file was just reinstalled) and pmOS's systemd trigger runs `enqueue-marked`. A call in progress ends. The restart resumes the registration over its existing SAs, -or goes on waiting out a hold the network had set (§7.4). A stopped unit +or goes on waiting out a hold the network had set (§7.4) — or, with an +upper-case, zero-padded or uncompressed `PCSCF=` (accepted until 0.3.12), +stops at bring-up with the spelling to use (§7.2 step 3). A stopped unit keeps no marker and `enqueue-marked` leaves it stopped (dev phone, systemd 262~rc3, 2026-10-03). @@ -774,6 +800,11 @@ Lines the operator (and the scripts) look for, all prefixed `imsd: `: until the next fresh attempt`, `X has left DEV: the registration on it is gone; staying on Y; next: …`, `X is no longer on DEV: the registration on it is gone; on the PDN's own address Y now; next: …`. +- `PCSCF='…' is not written as ip prints it; use PCSCF=…` (under `bring-up + FAILED:`) — one accepted spelling of the setting (§7.2 step 3); the + bring-up script's `ims-pdn-up: PCSCF '…': write the bare address, as ip + prints it (no brackets)` (its own prefix: a script line) is the same rule + for a bracketed value. - `engine fatal: … — exiting for systemd restart`. - `reg-event SUBSCRIBE sent`, and the binding's state from NOTIFY. - `audio session: …` at start and at each call's media start; `CODECS override active: …`. diff --git a/README.md b/README.md index 3116d23..c72a52d 100644 --- a/README.md +++ b/README.md @@ -98,10 +98,10 @@ session and root may place, answer and end calls; an ssh login may not. | Variable | Default | Meaning | |---|---|---| -| `PCSCF` | *(none — required)* | P-CSCF address (IPv6); without it registration fails with a clear error | +| `PCSCF` | *(none — required)* | P-CSCF address (IPv6), exactly one — not a comma-separated list — in RFC 5952 §4 form (lowercase, leading zeros dropped, the longest zero run compressed, no brackets — what `ip` prints for a plain IPv6 address; an IPv4-mapped address only as hex, `::ffff:a00:1` — the dotted spelling RFC 5952 §5 and `ip` use for it is refused); upper case, leading zeros, an uncompressed run or brackets are refused at start with the spelling to use (in the packaged unit the bring-up script refuses brackets first, with the rule), a dotted-quad tail with the rule; before 1.0.0 an upper-case, zero-padded or uncompressed `PCSCF=` matched no SA in `ip xfrm`'s text and registered fresh at every restart — an upgraded phone with such a setting stops at start with the spelling to use; without it registration fails with a clear error | | `PCSCF_PORT` | `5060` | unprotected SIP port for the initial REGISTER | | `DEV` | `qmapmux0.0` | ims-PDN netdev; the packaged service passes the real one via `/run/imsd.env` | -| `LOCAL` | *(auto)* | UE address. The packaged service gets the PDN address ModemManager assigned, via `/run/imsd.env` from `ims-pdn-up.sh`; a manual run prefers a global IPv6 on `DEV` that is neither temporary nor `dynamic` (RA-derived), then any non-temporary one, then the first (the kernel's temporary SLAAC address is listed first, and a registration bound to it is lost when the kernel retires it) | +| `LOCAL` | *(auto)* | UE address (compared in RFC 5952 form). The packaged service gets the PDN address ModemManager assigned, via `/run/imsd.env` from `ims-pdn-up.sh`; a manual run prefers a global IPv6 on `DEV` that is neither temporary nor `dynamic` (RA-derived), then any non-temporary one, then the first (the kernel's temporary SLAAC address is listed first, and a registration bound to it is lost when the kernel retires it) | | `USER_AGENT` | `imsd/` | REGISTER User-Agent. Some networks fingerprint UAs; setting your device's stock build string reproduces the stock modem's registration exactly. Empty omits the header | | `STATE_FILE` | `/var/lib/imsd/imsreg.state` (root) / `$XDG_STATE_HOME/imsd/imsreg.state` | persisted registration context for warm resume | | `HOLDS_FILE` | `/run/imsd.holds` (root) / `$XDG_RUNTIME_DIR/imsd.holds` (`/tmp/imsd.holds` without a runtime dir) | the waits the network imposed (a `Retry-After`, the refusal schedule), where that schedule stands and whether the old registration is gone (`old_gone`), kept across a restart of imsd for the PDN they were set on — not across a reboot; `/dev/null` switches the file off (only a regular file is ever removed; anything else is emptied). `imsd-handover.sh` honours it too | diff --git a/implementations/main.cpp b/implementations/main.cpp index da21229..809870b 100644 --- a/implementations/main.cpp +++ b/implementations/main.cpp @@ -835,7 +835,7 @@ std::optional LoadHolds(const std::string& pdn) char buf[512]; std::size_t n; while ((n = std::fread(buf, 1, sizeof buf, f)) > 0) s.append(buf, n); std::fclose(f); - if (QuotedAfter(s, "\"pdn\": \"").value_or("") != pdn) return std::nullopt; + if (imsd::util::CanonicalAddr(QuotedAfter(s, "\"pdn\": \"").value_or("")) != imsd::util::CanonicalAddr(pdn)) return std::nullopt; if (QuotedAfter(s, "\"boot\": \"").value_or("") != BootId()) return std::nullopt; long elapsed = BootSeconds() - IntAfter(s, "\"written_at\": ").value_or(0); imsd::engine::RegRecovery::Held h; @@ -915,7 +915,7 @@ public: // No baked-in default: the P-CSCF is carrier infrastructure, learned // from the IMS PDN's PCO on a stock stack. Until PCO discovery is // implemented, PCSCF= must be configured (see README). - pcscf_ = EnvOr("PCSCF", ""); + pcscf_ = imsd::util::CanonicalAddr(EnvOr("PCSCF", "")); pcscfPort_ = std::atoi(EnvOr("PCSCF_PORT", "5060").c_str()); dev_ = EnvOr("DEV", "qmapmux0.0"); outDir_ = EnvOr("OUT_DIR", StateDir()); @@ -956,7 +956,7 @@ public: } } - void SetLocal(std::string l) { local_ = l; pdnLocal_ = std::move(l); } + void SetLocal(std::string l) { local_ = imsd::util::CanonicalAddr(l); pdnLocal_ = local_; } void SetDevMode(bool d) { devMode_ = d; } const std::string& Local() const { return local_; } @@ -1199,6 +1199,15 @@ private: // ---- bring-up: warm resume if an SA + state file exist, else fresh ---- void BringUp() { if (pcscf_.empty()) throw std::runtime_error("PCSCF not set — configure the carrier's P-CSCF address " "(P-CSCF discovery from the PDN's PCO is not implemented yet)"); + if (pcscf_.contains(',')) throw std::runtime_error(std::format("PCSCF='{}' must be one address", pcscf_)); + // PCSCF= is accepted in one spelling only: the one `ip` prints (RFC + // 5952 — what the journal and `ip xfrm` show). Another spelling is + // refused with the one to use rather than normalised, so the bring-up + // script, which routes the value as typed, and the daemon agree on it. + if (std::string raw = EnvOr("PCSCF", ""); !raw.empty()) { + auto canon = imsd::util::CanonicalIp6(raw); + if (!canon || *canon != raw) throw std::runtime_error(std::format("PCSCF='{}' is not written as ip prints it; use PCSCF={}", raw, canon.value_or(""))); + } auto card = RunCapture({"qmicli", "-d", "qrtr://0", "--uim-get-card-status"}); auto sel = imsd::aka::ParseCardStatus(card.out); if (!sel) throw std::runtime_error("no ready USIM found"); @@ -2518,7 +2527,8 @@ int main(int argc, char** argv) { std::println(std::cerr, "imsd: no global IPv6 on {} — is the ims PDN up?", dev); return 1; } - Log(std::format("LOCAL={} P-CSCF=[{}]:{}", local, EnvOr("PCSCF", "(unset)"), EnvOr("PCSCF_PORT", "5060"))); + local = imsd::util::CanonicalAddr(local); + Log(std::format("LOCAL={} P-CSCF=[{}]:{}", local, imsd::util::CanonicalAddr(EnvOr("PCSCF", "(unset)")), EnvOr("PCSCF_PORT", "5060"))); if (!session) LogAudioSession(); imsd::bus::AuthorizeCalls = !session; diff --git a/interfaces/Imsd-Ipsec.cppm b/interfaces/Imsd-Ipsec.cppm index df29ace..25eb0eb 100644 --- a/interfaces/Imsd-Ipsec.cppm +++ b/interfaces/Imsd-Ipsec.cppm @@ -374,23 +374,28 @@ export namespace imsd::ipsec { // state: (src==LOCAL,dst==P): reqid1->spiPs reqid2->spiPc // (src==P,dst==LOCAL): reqid1->spiUc reqid2->spiUs // policy: dir out, reqid1->portPs=dport, reqid2->portPc=dport + // Addresses are compared in RFC 5952 form on both sides (the kernel prints + // that form; PCSCF=/LOCAL= arrive in any spelling), so the spelling in + // /etc/imsd.env can no longer hide a registration's SAs. // Returns nullopt unless spiPs, spiPc, portPs, portPc are all found. std::optional ParseExistingSa(std::string_view stateText, std::string_view policyText, std::string_view pcscf, std::string_view local) { + const std::string p = imsd::util::CanonicalAddr(pcscf); + const std::string l = imsd::util::CanonicalAddr(local); ExistingSa sa; bool havePs = false; bool havePc = false; bool havePortPs = false; bool havePortPc = false; for (std::string_view b : detail::SrcBlocks(stateText)) { - std::string_view src = detail::TokenAfter(b, "src "); - std::string_view dst = detail::TokenAfter(b, "dst "); + const std::string src = imsd::util::CanonicalAddr(detail::TokenAfter(b, "src ")); + const std::string dst = imsd::util::CanonicalAddr(detail::TokenAfter(b, "dst ")); auto spi = detail::ParseHex32(detail::TokenAfter(b, "spi ")); auto reqid = detail::ParseDec(detail::TokenAfter(b, "reqid ")); if (!spi || !reqid) continue; - if (src == local && dst == pcscf && *reqid == 1) { sa.spiPs = *spi; havePs = true; } - if (src == local && dst == pcscf && *reqid == 2) { sa.spiPc = *spi; havePc = true; } - if (src == pcscf && dst == local && *reqid == 1) sa.spiUc = *spi; - if (src == pcscf && dst == local && *reqid == 2) sa.spiUs = *spi; + if (src == l && dst == p && *reqid == 1) { sa.spiPs = *spi; havePs = true; } + if (src == l && dst == p && *reqid == 2) { sa.spiPc = *spi; havePc = true; } + if (src == p && dst == l && *reqid == 1) sa.spiUc = *spi; + if (src == p && dst == l && *reqid == 2) sa.spiUs = *spi; } for (std::string_view b : detail::SrcBlocks(policyText)) { if (b.find("dir out") == std::string_view::npos) continue; @@ -438,11 +443,12 @@ export namespace imsd::ipsec { // order: where a registration made on another address of the link (the // kernel's temporary one, before 0.3.10 — ims ledger F18) still lives. std::vector SaLocals(std::string_view stateText, std::string_view pcscf) { + const std::string p = imsd::util::CanonicalAddr(pcscf); std::vector locals; for (std::string_view b : detail::SrcBlocks(stateText)) { - std::string_view src = detail::TokenAfter(b, "src "); - if (src.empty() || detail::TokenAfter(b, "dst ") != pcscf) continue; - if (std::ranges::find(locals, src) == locals.end()) locals.emplace_back(src); + const std::string src = imsd::util::CanonicalAddr(detail::TokenAfter(b, "src ")); + if (src.empty() || imsd::util::CanonicalAddr(detail::TokenAfter(b, "dst ")) != p) continue; + if (std::ranges::find(locals, src) == locals.end()) locals.push_back(src); } return locals; } diff --git a/packaging/ims-pdn-up.sh b/packaging/ims-pdn-up.sh index 191c2b5..e5fe660 100644 --- a/packaging/ims-pdn-up.sh +++ b/packaging/ims-pdn-up.sh @@ -302,6 +302,7 @@ fi PCSCF=${PCSCF:-$(envval PCSCF)} case "$PCSCF" in '') log "PCSCF not configured: no P-CSCF host route (imsd needs PCSCF= in /etc/imsd.env)" ;; + *\[*) log "PCSCF '$PCSCF': write the bare address, as ip prints it (no brackets)"; exit 1 ;; *:*) OUT=$(ip -6 route replace "$PCSCF/128" dev "$IFACE" 2>&1) || { log "host route to P-CSCF $PCSCF failed: $(squash "$OUT")"; exit 1; } log "host route to P-CSCF $PCSCF dev $IFACE" ;; diff --git a/packaging/imsd.post-upgrade b/packaging/imsd.post-upgrade index 4e295e8..4fba473 100755 --- a/packaging/imsd.post-upgrade +++ b/packaging/imsd.post-upgrade @@ -6,8 +6,10 @@ # trigger restarts every unit marked needs-restart (enqueue-marked). A # changed bus policy is live the moment apk writes it (dbus-broker-launch # reloads on its own), so it must not be left facing the old daemon. The -# restart resumes the registration over its existing SAs, or goes on -# waiting when the network had imsd wait; a call in progress ends. +# restart resumes the registration over its existing SAs, goes on waiting +# when the network had imsd wait, or — an upper-case, zero-padded or uncompressed PCSCF=, +# accepted until 0.3.12 — stops at bring-up with the spelling to use; a +# call in progress ends. # # Also installed as imsd.post-install: `apk del imsd` removes the files # and leaves the daemon running, so a later `apk add` of the package is diff --git a/tests/Ipsec/main.cpp b/tests/Ipsec/main.cpp index 1f63107..d7383d6 100644 --- a/tests/Ipsec/main.cpp +++ b/tests/Ipsec/main.cpp @@ -78,6 +78,18 @@ int main() { Check(sa && sa->spiPc == 0x0526f2c1u, "spi_pc (UE->P reqid2)"); Check(sa && sa->securityServer == "ipsec-3gpp; q=0.1; alg=hmac-sha-1-96; ealg=aes-cbc; " "spi-c=86438593; spi-s=86438592; port-c=33421; port-s=6000", "reconstructed Security-Server value"); } + // the environment's spelling must not matter: upper case, uncompressed, + // bracketed — the kernel prints RFC 5952 form and both sides are + // canonicalised before the compare + { + auto sa = ParseExistingSa(State, Policy, "2001:DB8::105", "2001:DB8:1111:2222:3333:4444:5555:DB43"); + Check(sa && sa->spiPs == 0x0526f2c0u && sa->portPs == 6000, "ParseExistingSa: upper-case spellings find the SAs"); + auto sa2 = ParseExistingSa(State, Policy, "[2001:0db8:0000:0000:0000:0000:0000:0105]", "2001:db8:1111:2222:3333:4444:5555:db43"); + Check(sa2 && sa2->spiPc == 0x0526f2c1u, "ParseExistingSa: uncompressed, bracketed P-CSCF finds the SAs"); + Check(!ParseExistingSa(State, Policy, "2001:db8::106", "2001:db8:1111:2222:3333:4444:5555:db43"), "ParseExistingSa: another P-CSCF still finds nothing"); + auto locals = SaLocals(State, "2001:DB8:0:0:0:0:0:105"); + Check(locals.size() == 1 && locals[0] == "2001:db8:1111:2222:3333:4444:5555:db43", "SaLocals: any spelling of the P-CSCF, canonical locals out"); + } // incomplete input -> nullopt Check(!ParseExistingSa("src a dst b\n\tspi 0x1 reqid 1 mode transport\n", "", "b", "a").has_value(), "missing pieces -> nullopt"); {