Skip to content

Commit dad236b

Browse files
committed
fix(sip): resolve identify-match hostnames via Redis cache + worker
Closes two bugs in res_pjsip_endpoint_identifier_ip handling: 1. Boot DNS race — PJLIB resolves match= once at module load and never re-resolves. A DNS outage during boot left identify empty forever, sending all incoming hostname-ITSP calls (Mango, Novofon, any SRV-only provider) into the anonymous context until manual reload. 2. Issue #1066 regression — commit 6e4d8bb removed DNS from getIncomingContextId, so two providers with different hostnames but the same resolved IP started landing in different dialplan contexts. Architecture: - SIPConf writes only IP/CIDR literals into pjsip.conf match=. Hostnames go into a Redis pending-hosts set (DB1). - WorkerSipDnsResolver (5-min tick, two-stage concurrent SRV → A+AAAA batch via proc_open+nslookup with real wall-clock deadline) populates pjsip:identify:resolved:<host> JSON entries with 7-day TTL. - ReloadPJSIPIdentifyAction (mutex-protected) regenerates pjsip.conf and issues only `module reload res_pjsip_endpoint_identifier_ip.so`, plus optional dialplan reload when the canonical IP shifted. No full pjsip reload, no calls dropped. Defers when needAsteriskRestart() is dirty so a topology change from the operator is never swallowed. - getIncomingContextId now uses the canonical (sort()[0]) IP from the Redis cache, so two hostnames resolving to a shared IP collapse onto one incoming context (#1066 fix). - SaveRecordAction synchronously warms the cache with a 2-second budget so the first regeneration after a Save already has the canonical IP. Security hardening: - IpAddressHelper::isPublicIp filters loopback/RFC1918/link-local out of every DNS write path (worker, warmup, defense-in-depth on read). - isGlobalUnicast now canonicalises via inet_pton->inet_ntop and rejects 6to4 (2002::/16), Teredo (2001:0000::/32), and RFC 3849 doc-prefix (2001:db8::/32), preventing the IPv6-side bypass where 2002:0a00:: encodes 10.0.0.0 RFC1918. - DnsResolver::parseSrvOutput whitelists [a-z0-9._-] and caps target length to 253 bytes. - DataStructure tightens host/address regex; updateAdditionalHosts requires isIpOrCidr. Tests: - +25 cases in SIPConfTest (string helpers + #1066 invariant snapshots with stubResolvedCache helper using public IPs). - +18 cases in new DnsResolverTest (BusyBox/GNU parser fixtures, SRV cap, dedup, port-suffix strip, IPv4/IPv6 filtering).
1 parent 07d8793 commit dad236b

11 files changed

Lines changed: 2397 additions & 22 deletions

File tree

src/Core/Asterisk/Configs/ExtensionsConf.php

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -314,6 +314,14 @@ public static function getExtenByDid(string $did):string
314314
*/
315315
public static function reload(): void
316316
{
317+
// The off-work-times generator inside generateConfig() calls
318+
// SIPConf::getIncomingContextId() which consults a process-local memo
319+
// of resolved IPs. When this reload action runs standalone (i.e.
320+
// without an intervening SIPConf::generateConfig that would have
321+
// reset the memo), the memo can serve IPs from a previous tick.
322+
// Drop it before regenerating to guarantee context-name freshness.
323+
SIPConf::resetResolvedIpsMemo();
324+
317325
$conf = new self();
318326
$conf->generateConfig();
319327

src/Core/Asterisk/Configs/SIPConf.php

Lines changed: 398 additions & 11 deletions
Large diffs are not rendered by default.

src/Core/Utilities/DnsResolver.php

Lines changed: 456 additions & 0 deletions
Large diffs are not rendered by default.

src/Core/Utilities/IpAddressHelper.php

Lines changed: 51 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -361,7 +361,20 @@ public static function validateHostname(string $hostname): bool
361361
* Check if IPv6 address is a global unicast address (publicly routable)
362362
*
363363
* Global unicast addresses are defined by RFC 4291 as 2000::/3
364-
* (addresses starting with binary 001, which means first hex digit is 2 or 3)
364+
* (addresses starting with binary 001, which means first hex digit is 2 or 3).
365+
*
366+
* Additionally rejects three sub-ranges that fall inside 2000::/3 but are
367+
* NOT safe for use as a SIP peer's match-IP:
368+
* - 2001:0000::/32 — Teredo tunnelling (RFC 4380); the encoded IPv4
369+
* inside can be loopback/RFC1918 of the tunnel relay.
370+
* - 2001:db8::/32 — documentation prefix (RFC 3849); never appears in
371+
* legitimate production traffic.
372+
* - 2002::/16 — 6to4 tunnelling (RFC 3056); hextets 2-3 encode an
373+
* IPv4 address. `2002:7f00:1::` encodes 127.0.0.1
374+
* and `2002:0a00::/24` encodes 10.0.0.0/8, giving
375+
* attacker-controlled DNS an IPv6 path to the
376+
* loopback/RFC1918 bypass that isPublicIp() blocks
377+
* on the IPv4 side.
365378
*
366379
* @param string $ip IPv6 address to check (can include CIDR notation)
367380
* @return bool True if the address is a global unicast IPv6 address
@@ -375,11 +388,45 @@ public static function isGlobalUnicast(string $ip): bool
375388
// Remove CIDR notation if present
376389
$ipWithoutCidr = explode('/', $ip)[0];
377390

378-
// Normalize to lowercase and trim
379-
$normalized = strtolower(trim($ipWithoutCidr));
391+
// Canonicalise via inet_pton → inet_ntop so subsequent prefix checks
392+
// always see the RFC 5952 form (lowercase, shortest representation,
393+
// leading zeros removed). Without this step, attacker-controlled
394+
// inputs like "2001:0db8::1" or "2001:0:0:0::1" would slip past
395+
// str_starts_with checks that compare against canonical strings.
396+
$binary = @inet_pton(trim($ipWithoutCidr));
397+
if ($binary === false) {
398+
return false;
399+
}
400+
$normalized = @inet_ntop($binary);
401+
if ($normalized === false) {
402+
return false;
403+
}
404+
$normalized = strtolower($normalized);
380405

381406
// Global Unicast: 2000::/3 (addresses starting with 2 or 3)
382-
return preg_match('/^[23]/', $normalized) === 1;
407+
if (preg_match('/^[23]/', $normalized) !== 1) {
408+
return false;
409+
}
410+
411+
// Reject 6to4 (2002::/16) — encoded IPv4 may be loopback/RFC1918.
412+
if (str_starts_with($normalized, '2002:')) {
413+
return false;
414+
}
415+
416+
// Reject the RFC 3849 documentation prefix (2001:db8::/32). Canonical
417+
// form is "2001:db8:" or the bare "2001:db8::".
418+
if (str_starts_with($normalized, '2001:db8:') || $normalized === '2001:db8::') {
419+
return false;
420+
}
421+
422+
// Reject Teredo (2001:0000::/32). In canonical form the all-zero
423+
// second group collapses to either an empty hextet ("2001::") or
424+
// a literal zero ("2001:0:"). The regex covers both shapes.
425+
if (preg_match('/^2001:0?:/', $normalized) === 1 || str_starts_with($normalized, '2001::')) {
426+
return false;
427+
}
428+
429+
return true;
383430
}
384431

385432
/**

src/Core/Workers/Cron/WorkerSafeScriptsCore.php

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,7 @@
4646
use MikoPBX\Core\Workers\WorkerRemoveOldRecords;
4747
use MikoPBX\Core\Workers\WorkerS3Upload;
4848
use MikoPBX\Core\Workers\WorkerS3CacheCleaner;
49+
use MikoPBX\Core\Workers\WorkerSipDnsResolver;
4950
use MikoPBX\Core\Workers\WorkerSoundFilesInit;
5051
use MikoPBX\Core\Workers\WorkerWav2Webm;
5152
use MikoPBX\Common\Models\StorageSettings;
@@ -553,6 +554,7 @@ private function prepareWorkersList(): array
553554
WorkerLogRotate::class,
554555
WorkerRemoveOldRecords::class,
555556
WorkerNotifyAdministrator::class,
557+
WorkerSipDnsResolver::class,
556558
WorkerWav2Webm::class,
557559
],
558560
];
Lines changed: 164 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,164 @@
1+
<?php
2+
3+
namespace MikoPBX\Core\Workers\Libs\WorkerModelsEvents\Actions;
4+
5+
use MikoPBX\Common\Providers\MutexProvider;
6+
use MikoPBX\Core\Asterisk\Configs\ExtensionsConf;
7+
use MikoPBX\Core\Asterisk\Configs\SIPConf;
8+
use MikoPBX\Core\System\Processes;
9+
use MikoPBX\Core\System\SystemMessages;
10+
use MikoPBX\Core\System\Util;
11+
use Throwable;
12+
13+
/**
14+
* Narrow reload triggered by WorkerSipDnsResolver when provider hostnames
15+
* resolve to a new set of IPs. Regenerates pjsip.conf so the freshly cached
16+
* resolved IPs flow into `identify match=`, then issues only
17+
* `module reload res_pjsip_endpoint_identifier_ip.so` instead of a full
18+
* `pjsip reload` / `core reload` — active calls and registrations stay up.
19+
*
20+
* **Strict contract: only non-destructive Asterisk commands.** This action
21+
* runs every 5 minutes from a DNS TTL refresh and MUST NOT drop calls.
22+
*
23+
* If the topology hash is dirty (admin recently changed extipaddr, SIP
24+
* port, timezone, etc.) we SKIP the entire regeneration and defer to the
25+
* normal WorkerModelsEvents pipeline. Reason: `SIPConf::generateConfig()`
26+
* unconditionally overwrites the persisted topology hash from inside
27+
* `generateGeneralPj()`, so running it here would silently clear the
28+
* dirty marker — the next `ReloadPJSIPAction` from WorkerModelsEvents
29+
* would see matching hashes, skip `safeRestart()`, and the operator's
30+
* topology change would be lost. The DNS resolver's job is to keep the
31+
* identify cache fresh; the rest of the topology belongs to the model-
32+
* events pipeline. Skipping costs at most one 5-minute window of DNS
33+
* staleness; swallowing a topology change is far worse.
34+
*
35+
* Optional parameter:
36+
* - 'canonicalChanged' (bool, default false) — when true, the canonical
37+
* (smallest) IP of at least one hostname changed, which renames the
38+
* incoming-context produced by {@see SIPConf::getIncomingContextId()}.
39+
* In that case extensions.conf must be regenerated under the SAME
40+
* mutex so pjsip.conf endpoint.context and the dialplan section name
41+
* stay in lock-step; a brief Redis blip between two unsynchronised
42+
* writes could otherwise leave them out of sync.
43+
*
44+
* Serialized with {@see SIPConf::reload} via the {@see SIPConf::MUTEX_CONF_WRITE}
45+
* mutex.
46+
*/
47+
class ReloadPJSIPIdentifyAction implements ReloadActionInterface
48+
{
49+
public function execute(array $parameters = []): void
50+
{
51+
$di = \Phalcon\Di\Di::getDefault();
52+
if ($di === null) {
53+
return;
54+
}
55+
56+
try {
57+
$di->get(MutexProvider::SERVICE_NAME)->synchronized(
58+
SIPConf::MUTEX_CONF_WRITE,
59+
static fn() => self::regenerateAndReload($parameters),
60+
timeout: 10,
61+
ttl: 30
62+
);
63+
} catch (Throwable $e) {
64+
// Could not acquire — another writer has the lock right now.
65+
// The competing writer will publish a fresh pjsip.conf with our
66+
// cached IPs anyway (it calls the same generateConfig() under the
67+
// same lock), so we degrade silently without producing noise.
68+
SystemMessages::sysLogMsg(
69+
__METHOD__,
70+
'identify-only reload skipped: ' . $e->getMessage(),
71+
LOG_INFO
72+
);
73+
}
74+
}
75+
76+
/**
77+
* Body of the action, executed under MUTEX_CONF_WRITE.
78+
*
79+
* Always non-destructive: regenerate pjsip.conf → `module reload
80+
* res_pjsip_endpoint_identifier_ip.so`, plus optional `dialplan reload`
81+
* via ExtensionsConf::reload() when the canonical IP shifted. Neither
82+
* command hangs up live channels. We deliberately do NOT call
83+
* `safeRestart()` here even if topology is dirty — see class docblock.
84+
*/
85+
private static function regenerateAndReload(array $parameters): void
86+
{
87+
$sip = new SIPConf();
88+
89+
// Bail out when the persisted topology hash diverges from the
90+
// current settings — running generateConfig() here would overwrite
91+
// the hash and silently swallow the admin's pending topology
92+
// change. The normal model-events pipeline will pick up the dirty
93+
// marker, regenerate, and call safeRestart() under its own mutex.
94+
if ($sip->needAsteriskRestart()) {
95+
SystemMessages::sysLogMsg(
96+
__METHOD__,
97+
'identify-only reload skipped: topology dirty, deferring to ReloadPJSIPAction',
98+
LOG_INFO
99+
);
100+
return;
101+
}
102+
103+
// Drop any stale resolved-IP memo so generateConfig() reads fresh
104+
// values from Redis for every hostname referenced in pjsip.conf.
105+
SIPConf::resetResolvedIpsMemo();
106+
107+
$sip->generateConfig();
108+
109+
$asterisk = Util::which('asterisk');
110+
$output = [];
111+
$exitCode = 0;
112+
Processes::mwExec(
113+
"$asterisk -rx 'module reload res_pjsip_endpoint_identifier_ip.so'",
114+
$output,
115+
$exitCode
116+
);
117+
118+
if ($exitCode !== 0) {
119+
// The DNS cache update succeeded and pjsip.conf was regenerated,
120+
// but the running Asterisk did NOT pick up the new identify set.
121+
// Operators must know — without this log line the system falls
122+
// into the exact failure mode this patch is meant to prevent
123+
// (incoming calls landing in anonymous despite a healthy cache).
124+
SystemMessages::sysLogMsg(
125+
__METHOD__,
126+
'asterisk module reload returned exit=' . $exitCode . ': ' . implode("\n", $output),
127+
LOG_ERR
128+
);
129+
return;
130+
}
131+
132+
// Canonical-IP shift renames the incoming-context produced by
133+
// SIPConf::getIncomingContextId(). pjsip.conf endpoint.context was
134+
// just rewritten with the new name; extensions.conf must be brought
135+
// along under the same lock so a concurrent Redis blip cannot leave
136+
// the two referencing different section names. Asterisk's
137+
// `dialplan reload` (issued inside ExtensionsConf::reload) is
138+
// non-destructive on live channels — established calls finish in
139+
// the old context section, new INVITEs land in the new one.
140+
$canonicalChanged = !empty($parameters['canonicalChanged']);
141+
if ($canonicalChanged) {
142+
try {
143+
ExtensionsConf::reload();
144+
SystemMessages::sysLogMsg(
145+
__METHOD__,
146+
'Reloaded dialplan after canonical IP change',
147+
LOG_INFO
148+
);
149+
} catch (Throwable $e) {
150+
SystemMessages::sysLogMsg(
151+
__METHOD__,
152+
'Dialplan reload after canonical change failed: ' . $e->getMessage(),
153+
LOG_ERR
154+
);
155+
}
156+
}
157+
158+
SystemMessages::sysLogMsg(
159+
__METHOD__,
160+
'Reloaded res_pjsip_endpoint_identifier_ip after DNS-driven identify update',
161+
LOG_INFO
162+
);
163+
}
164+
}

0 commit comments

Comments
 (0)