jwed/central-unifi
1
0
Fork 0
Code Issues 1 Pull Requests Actions Packages Projects Releases Wiki Activity

The portal derives the device MAC instead of trusting the caller

Browse Source
This commit is contained in:
jwed
2026-08-21 13:18:30 -04:00
parent ccd6fc49b2
commit 4322d07b40
4 changed files with 168 additions and 21 deletions
Download Patch File Download Diff File Expand all files Collapse all files

36
src/Http/Controllers/PortalController.php
View File

@@ -133,30 +133,24 @@ class PortalController extends Controller
return redirect()->route('login');
}
$clientMac = strtolower($request->input('mac', $request->input('id', '')));
$ssid = $request->input('ssid', '');
$apMac = $request->input('ap', '');
$apMac = (string) $request->input('ap', '');
$ssid = (string) $request->input('ssid', '');
if (! $clientMac) {
return response('Missing MAC address', 400);
$device = app(\Dashboard\Unifi\Services\DeviceResolver::class)->resolve($request, $apMac);
if (! $device->ok) {
return Inertia::render('Unifi/PortalDenied', [
'reason' => match ($device->reason) {
'ap_mismatch' => 'That device is not connected to this access point. Reconnect and try again.',
'proxy_untrusted' => 'Wi-Fi sign-in is misconfigured and cannot identify your device. Please tell IT.',
default => 'Still connecting — wait a moment and try again.',
},
'resolveFailure' => $device->reason,
]);
}
// SECURITY: everything identifying the device is request-supplied, and this
// endpoint WRITES network state (authorizeGuest puts a MAC on the mapped VLAN).
// Before this check any signed-in account — a student, a parent — could call
// GET /portal/wifi/callback?mac=<someone else's MAC>&url=…
// and admit a device they do not own, up to max_devices.
//
// A well-formed MAC is the floor, not a fix: it stops the parameter being used
// to smuggle junk into authorizeGuest and into the PortalSession row, but it
// cannot prove the caller owns the device. Properly binding it needs the AP
// handshake — UniFi can sign the redirect, or we ask the controller whether this
// MAC is CURRENTLY associated to $apMac before admitting it. Both need testing
// against real hardware, so they are deliberately not guessed at here.
$clientMac = str_replace('-', ':', $clientMac);
if (! preg_match('/^([0-9a-f]{2}:){5}[0-9a-f]{2}$/', $clientMac)) {
return response('Malformed MAC address', 400);
}
$clientMac = $device->mac;
$isRandomized = $device->isRandomized;
// Find the VLAN mapping for this user's group/OU
$mapping = $this->resolveMapping($user);

104
src/Services/DeviceResolver.php Normal file
View File

@@ -0,0 +1,104 @@
<?php
namespace Dashboard\Unifi\Services;
use Dashboard\Unifi\Support\MacAddress;
use Illuminate\Http\Request;
use Illuminate\Support\Facades\Log;
/** The outcome of trying to identify the device making a portal request. */
class ResolvedDevice
{
public function __construct(
public bool $ok,
public ?string $mac = null,
public ?string $reason = null,
public bool $isRandomized = false,
public bool $macParameterDisagreed = false,
public array $client = [],
) {}
}
/**
* Identify the device that made a captive-portal request, or refuse.
*
* The `?mac=` parameter is NOT trusted. It is read only so a disagreement with
* the derived value can be logged — that disagreement is somebody trying to
* authorize a device they do not own, and before this nothing anywhere noticed.
*
* Every path refuses rather than guessing. The cost of that is real: if
* TrustProxies is misconfigured, `proxy_untrusted` is returned for everyone and
* nobody gets online, which is why it is a distinct reason the portal page can
* report instead of a generic miss.
*/
class DeviceResolver
{
public function __construct(private UnifiApiClient $unifi) {}
/** Attempts, and the pause between them, for a device that has just associated. */
private const ATTEMPTS = 3;
private const PAUSE_US = 750_000;
public function resolve(Request $request, string $apMac): ResolvedDevice
{
$ip = $request->ip();
/*
* A device that has only just associated may have no IP yet, so UniFi's
* client table does not know it. Retry briefly rather than telling a
* pupil their device is not connected when it is about to be.
*
* Bounded on purpose: this runs inside a web request, and a longer wait
* would hold a worker per connecting device at exactly the moment a
* class of them arrives at once.
*/
$client = null;
for ($attempt = 1; $attempt <= self::ATTEMPTS; $attempt++) {
$client = $this->unifi->findClientByIp($ip);
if ($client) {
break;
}
if ($attempt < self::ATTEMPTS) {
usleep(self::PAUSE_US);
}
}
if (! $client) {
// A forwarded header present while the request IP is still the raw
// socket address means TrustProxies is not applying, so every lookup
// here is doomed for the same reason. Say which failure this is.
$forwarded = $request->headers->get('X-Forwarded-For');
$raw = $request->server('REMOTE_ADDR');
$reason = ($forwarded && $ip === $raw) ? 'proxy_untrusted' : 'no_client';
return new ResolvedDevice(ok: false, reason: $reason);
}
$mac = MacAddress::normalise($client['mac'] ?? null);
if (! $mac) {
return new ResolvedDevice(ok: false, reason: 'no_client');
}
$clientAp = MacAddress::normalise($client['ap_mac'] ?? null);
if (! $clientAp || $clientAp !== MacAddress::normalise($apMac)) {
return new ResolvedDevice(ok: false, reason: 'ap_mismatch');
}
$supplied = MacAddress::normalise($request->input('mac'));
$disagreed = $supplied !== null && $supplied !== $mac;
if ($disagreed) {
Log::warning('unifi.portal: supplied MAC disagreed with the derived one', [
'derived' => $mac, 'supplied' => $supplied, 'ip' => $ip, 'ap' => $apMac,
]);
}
return new ResolvedDevice(
ok: true,
mac: $mac,
isRandomized: MacAddress::isRandomized($mac),
macParameterDisagreed: $disagreed,
client: $client,
);
}
}

18
src/Services/UnifiApiClient.php
View File

@@ -253,6 +253,24 @@ class UnifiApiClient
return ! empty($data);
}
/**
* The client currently holding this IP, or null.
*
* Reads the cached client list rather than querying per IP: the portal calls
* this once per connection attempt, and a class arriving at once would
* otherwise be one controller round trip each.
*/
public function findClientByIp(string $ip): ?array
{
foreach ($this->getActiveClients() as $client) {
if (($client['ip'] ?? null) === $ip) {
return $client;
}
}
return null;
}
public function kickClient(string $mac): array
{
Cache::forget('unifi:clients');

31
src/Support/MacAddress.php Normal file
View File

@@ -0,0 +1,31 @@
<?php
namespace Dashboard\Unifi\Support;
/**
* Pure MAC helpers. No dependencies, so they are cheap to test and safe to call
* from anywhere.
*/
class MacAddress
{
/** Lower-case, colon-separated, or null if it is not a MAC at all. */
public static function normalise(?string $mac): ?string
{
$mac = strtolower(str_replace('-', ':', trim((string) $mac)));
return preg_match('/^([0-9a-f]{2}:){5}[0-9a-f]{2}$/', $mac) ? $mac : null;
}
/**
* Is this a private (randomized) address rather than a real manufacturer one?
*
* The locally-administered bit is the second-least-significant bit of the
* first octet. iOS "Private Wi-Fi Address" and Android per-network
* randomization both set it. This is why OUI is useless for identifying
* device types: a randomized MAC has no real manufacturer prefix.
*/
public static function isRandomized(string $mac): bool
{
return (bool) (hexdec(substr($mac, 0, 2)) & 0b10);
}
}