Skip to content

Commit 640becc

Browse files
committed
Fix UNAS camera misclassification and Cloudflare IP tolerance (#561) (#578)
* Exclude UNAS/Drive devices from camera classification (#561) UNAS devices share Ubiquiti OUI prefixes (e.g., A8:9C:6C) with Protect cameras, causing the MAC OUI detector to misclassify them as cameras. The V2 device API returns drive_devices alongside protect_devices. We now deserialize that array, collect the MACs, and positively classify them as NAS at the highest detection priority - before MAC OUI can misfire. * Update sponsorship stats to 217k LoC and 6,100 tests * Tolerate management IPs in Cloudflare source restriction lists Users commonly add a few personal/management IPs (home, office, family) alongside Cloudflare ranges in their port forward source restrictions. The strict "every entry must be CF" check was classifying these as non-Cloudflare restrictions, triggering a misleading recommendation. Now allows up to 10 individual host IPs (/32 or bare IP) alongside CF ranges. Non-CF subnets still fail since they indicate a different restriction strategy.
1 parent c11314f commit 640becc

8 files changed

Lines changed: 325 additions & 6 deletions

File tree

src/NetworkOptimizer.Audit/Services/DeviceTypeDetectionService.cs

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -169,6 +169,29 @@ private DeviceDetectionResult DetectDeviceTypeCore(
169169
};
170170
}
171171

172+
// Priority -0.5: Known UNAS/Drive devices (from V2 API drive_devices array).
173+
// These share Ubiquiti OUI prefixes with cameras but are NAS storage devices.
174+
if (_protectCameras != null && !string.IsNullOrEmpty(client?.Mac) &&
175+
_protectCameras.IsDriveDevice(client.Mac))
176+
{
177+
_logger?.LogDebug("[Detection] '{DisplayName}': Known UNAS/Drive device (confirmed by controller API)",
178+
displayName);
179+
return new DeviceDetectionResult
180+
{
181+
Category = ClientDeviceCategory.NAS,
182+
Source = DetectionSource.UniFiFingerprint,
183+
ConfidenceScore = 100,
184+
VendorName = "Ubiquiti",
185+
ProductName = client.Name,
186+
RecommendedNetwork = NetworkPurpose.Corporate,
187+
Metadata = new Dictionary<string, object>
188+
{
189+
["detection_method"] = "unifi_network_api",
190+
["mac"] = client.Mac
191+
}
192+
};
193+
}
194+
172195
// Priority 0: Check for obvious name keywords that should OVERRIDE fingerprint
173196
// This handles cases where vendor fingerprint is wrong (e.g., Cync plugs detected as cameras)
174197
var obviousNameResult = CheckObviousNameOverride(client?.Name, client?.Hostname, client?.Oui);
@@ -1358,6 +1381,25 @@ public DeviceDetectionResult DetectFromMac(string macAddress)
13581381
if (string.IsNullOrEmpty(macAddress))
13591382
return DeviceDetectionResult.Unknown;
13601383

1384+
// Check for known UNAS/Drive devices before any other detection
1385+
if (_protectCameras != null && _protectCameras.IsDriveDevice(macAddress))
1386+
{
1387+
_logger?.LogDebug("[Detection] MAC {Mac}: Known UNAS/Drive device (confirmed by controller API)", macAddress);
1388+
return new DeviceDetectionResult
1389+
{
1390+
Category = ClientDeviceCategory.NAS,
1391+
Source = DetectionSource.UniFiFingerprint,
1392+
ConfidenceScore = 100,
1393+
VendorName = "Ubiquiti",
1394+
RecommendedNetwork = NetworkPurpose.Corporate,
1395+
Metadata = new Dictionary<string, object>
1396+
{
1397+
["detection_method"] = "unifi_network_api",
1398+
["mac"] = macAddress
1399+
}
1400+
};
1401+
}
1402+
13611403
// First, check if we have this MAC in client history (for fingerprint data)
13621404
if (_clientHistoryByMac != null &&
13631405
_clientHistoryByMac.TryGetValue(macAddress.ToLowerInvariant(), out var historyClient))

src/NetworkOptimizer.Core/Helpers/CloudflareIpRanges.cs

Lines changed: 28 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -49,12 +49,17 @@ public static class CloudflareIpRanges
4949
/// </summary>
5050
public static readonly string[] AllRanges = [.. IPv4Ranges, .. IPv6Ranges];
5151

52+
private const int MaxNonCloudflareHosts = 10;
53+
5254
/// <summary>
53-
/// Check if a list of IP addresses/CIDRs represents a Cloudflare-only restriction.
54-
/// Returns true if every entry in the list is a known Cloudflare range.
55+
/// Check if a list of IP addresses/CIDRs represents a Cloudflare-restricted configuration.
56+
/// Returns true if the list is primarily Cloudflare ranges, tolerating a small number of
57+
/// individual host IPs (e.g., management/home IPs) that people commonly add alongside CF ranges.
58+
/// Non-Cloudflare entries must be single hosts (no CIDR or /32 or /128) - large non-CF subnets
59+
/// indicate a different restriction strategy, not a Cloudflare list with extras.
5560
/// </summary>
5661
/// <param name="addresses">List of IPs or CIDRs from a firewall group or source restriction</param>
57-
/// <returns>True if all addresses match known Cloudflare ranges</returns>
62+
/// <returns>True if the list is a Cloudflare IP restriction (possibly with a few management IPs)</returns>
5863
public static bool IsCloudflareOnly(IEnumerable<string>? addresses)
5964
{
6065
if (addresses == null)
@@ -64,13 +69,31 @@ public static bool IsCloudflareOnly(IEnumerable<string>? addresses)
6469
if (list.Count == 0)
6570
return false;
6671

72+
int cloudflareCount = 0;
73+
int nonCloudflareHostCount = 0;
74+
6775
foreach (var address in list)
6876
{
69-
if (!IsCloudflareAddress(address))
77+
if (IsCloudflareAddress(address))
78+
{
79+
cloudflareCount++;
80+
continue;
81+
}
82+
83+
var trimmed = address.Trim();
84+
bool isSingleHost = !trimmed.Contains('/')
85+
|| trimmed.EndsWith("/32")
86+
|| trimmed.EndsWith("/128");
87+
88+
if (!isSingleHost)
89+
return false;
90+
91+
nonCloudflareHostCount++;
92+
if (nonCloudflareHostCount > MaxNonCloudflareHosts)
7093
return false;
7194
}
7295

73-
return true;
96+
return cloudflareCount > 0;
7497
}
7598

7699
/// <summary>

src/NetworkOptimizer.Core/Models/ProtectCamera.cs

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -157,6 +157,24 @@ public bool IsNvr(string? mac)
157157
/// </summary>
158158
public IEnumerable<ProtectCamera> GetAll() => _cameras.Values;
159159

160+
/// <summary>
161+
/// MACs of known UNAS/Drive devices (from drive_devices in V2 API).
162+
/// These share Ubiquiti OUI prefixes with cameras but are NOT cameras.
163+
/// </summary>
164+
private readonly HashSet<string> _driveDeviceMacs = new(StringComparer.OrdinalIgnoreCase);
165+
166+
public void AddDriveDevice(string mac)
167+
{
168+
_driveDeviceMacs.Add(mac.ToLowerInvariant());
169+
}
170+
171+
public bool IsDriveDevice(string? mac)
172+
{
173+
return !string.IsNullOrEmpty(mac) && _driveDeviceMacs.Contains(mac);
174+
}
175+
176+
public int DriveDeviceCount => _driveDeviceMacs.Count;
177+
160178
/// <summary>
161179
/// Create an empty collection
162180
/// </summary>

src/NetworkOptimizer.UniFi/Models/UniFiProtectDeviceResponse.cs

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,9 @@ public class UniFiAllDevicesResponse
2222

2323
[JsonPropertyName("led_devices")]
2424
public List<UniFiProtectDeviceResponse>? LedDevices { get; set; }
25+
26+
[JsonPropertyName("drive_devices")]
27+
public List<UniFiProtectDeviceResponse>? DriveDevices { get; set; }
2528
}
2629

2730
/// <summary>

src/NetworkOptimizer.UniFi/UniFiApiClient.cs

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -782,6 +782,25 @@ public async Task<ProtectCameraCollection> GetProtectCamerasAsync(CancellationTo
782782
}
783783

784784
_logger.LogInformation("Found {Count} Protect devices requiring Security VLAN", result.Count);
785+
786+
if (allDevices.DriveDevices != null)
787+
{
788+
foreach (var drive in allDevices.DriveDevices)
789+
{
790+
if (!string.IsNullOrEmpty(drive.Mac))
791+
{
792+
result.AddDriveDevice(drive.Mac);
793+
_logger.LogDebug("Found Drive device: {Name} ({Model}) - MAC: {Mac}",
794+
drive.Name, drive.Model, drive.Mac);
795+
}
796+
}
797+
798+
if (result.DriveDeviceCount > 0)
799+
{
800+
_logger.LogInformation("Found {Count} Drive (UNAS) devices excluded from camera detection", result.DriveDeviceCount);
801+
}
802+
}
803+
785804
return result;
786805
}
787806

src/NetworkOptimizer.Web/Services/SponsorshipService.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@ private static readonly (string Quip, string ActionText)[] Tiers =
3838
("Still free. Still no VC funding. Still powered by coffee and spite.", "Fund the spite"),
3939

4040
// Level 6: 31-40 uses - stats flex
41-
("167,000 lines of code. 5,600 tests. One guy on 2 acres in Arkansas. Still cheaper than UI Ground shipping.", "Buy him lunch"),
41+
("217,000 lines of code. 6,100 tests. One guy on 2 acres in Arkansas. Still cheaper than UI Ground shipping.", "Buy him lunch"),
4242

4343
// Level 7: 41-50 uses - former employer dig
4444
("You've used this more than some employers used my code. Just saying.", "Money me"),

tests/NetworkOptimizer.Audit.Tests/Services/DeviceTypeDetectionServiceTests.cs

Lines changed: 170 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2710,4 +2710,174 @@ public void ProtectCameraCollection_IsNvr_ReturnsFalseForNull()
27102710
}
27112711

27122712
#endregion
2713+
2714+
#region UNAS / Drive Device Tests (Issue #561)
2715+
2716+
[Fact]
2717+
public void DetectDeviceType_UnasWithCameraOui_ClassifiedAsNasWhenDriveDeviceKnown()
2718+
{
2719+
// Arrange - UNAS Pro 4 shares OUI A8:9C:6C with Protect cameras
2720+
var service = new DeviceTypeDetectionService();
2721+
var cameras = new ProtectCameraCollection();
2722+
cameras.AddDriveDevice("a8:9c:6c:06:0c:cc");
2723+
service.SetProtectCameras(cameras);
2724+
2725+
var client = new UniFiClientResponse
2726+
{
2727+
Mac = "a8:9c:6c:06:0c:cc",
2728+
Name = "storage-core"
2729+
};
2730+
2731+
// Act
2732+
var result = service.DetectDeviceType(client);
2733+
2734+
// Assert - should be NAS, not Camera
2735+
result.Category.Should().Be(ClientDeviceCategory.NAS);
2736+
result.ConfidenceScore.Should().Be(100);
2737+
result.VendorName.Should().Be("Ubiquiti");
2738+
result.Metadata!["detection_method"].Should().Be("unifi_network_api");
2739+
}
2740+
2741+
[Fact]
2742+
public void DetectDeviceType_UnasSecondNic_ClassifiedAsNasWhenDriveDeviceKnown()
2743+
{
2744+
// Arrange - UNAS with 10G NIC (second MAC, unnamed)
2745+
var service = new DeviceTypeDetectionService();
2746+
var cameras = new ProtectCameraCollection();
2747+
cameras.AddDriveDevice("a8:9c:6c:06:0c:cd");
2748+
service.SetProtectCameras(cameras);
2749+
2750+
var client = new UniFiClientResponse
2751+
{
2752+
Mac = "a8:9c:6c:06:0c:cd",
2753+
Name = "a8:9c:6c:06:0c:cd"
2754+
};
2755+
2756+
// Act
2757+
var result = service.DetectDeviceType(client);
2758+
2759+
// Assert
2760+
result.Category.Should().Be(ClientDeviceCategory.NAS);
2761+
result.ConfidenceScore.Should().Be(100);
2762+
}
2763+
2764+
[Fact]
2765+
public void DetectFromMac_UnasWithCameraOui_ClassifiedAsNasWhenDriveDeviceKnown()
2766+
{
2767+
// Arrange - DetectFromMac is a separate code path used for offline detection
2768+
var service = new DeviceTypeDetectionService();
2769+
var cameras = new ProtectCameraCollection();
2770+
cameras.AddDriveDevice("a8:9c:6c:06:0c:cc");
2771+
service.SetProtectCameras(cameras);
2772+
2773+
// Act
2774+
var result = service.DetectFromMac("a8:9c:6c:06:0c:cc");
2775+
2776+
// Assert
2777+
result.Category.Should().Be(ClientDeviceCategory.NAS);
2778+
result.ConfidenceScore.Should().Be(100);
2779+
}
2780+
2781+
[Fact]
2782+
public void DetectDeviceType_ActualProtectCamera_StillDetectedAsCamera()
2783+
{
2784+
// Arrange - real camera with same OUI prefix should still be detected
2785+
var service = new DeviceTypeDetectionService();
2786+
var cameras = new ProtectCameraCollection();
2787+
cameras.Add("a8:9c:6c:1e:76:e4", "G4 Bullet", null, isNvr: false);
2788+
cameras.AddDriveDevice("a8:9c:6c:06:0c:cc");
2789+
service.SetProtectCameras(cameras);
2790+
2791+
var client = new UniFiClientResponse
2792+
{
2793+
Mac = "a8:9c:6c:1e:76:e4",
2794+
Name = "cam-frontdoor"
2795+
};
2796+
2797+
// Act
2798+
var result = service.DetectDeviceType(client);
2799+
2800+
// Assert - camera should still be detected via ProtectCameraCollection
2801+
result.Category.Should().Be(ClientDeviceCategory.Camera);
2802+
result.ConfidenceScore.Should().Be(100);
2803+
result.Metadata!["detection_method"].Should().Be("unifi_protect_api");
2804+
}
2805+
2806+
[Fact]
2807+
public void DetectDeviceType_UnasWithoutDriveData_FallsBackToMacOui()
2808+
{
2809+
// Arrange - if drive_devices isn't available, MAC OUI still fires (backward compat)
2810+
var service = new DeviceTypeDetectionService();
2811+
// No SetProtectCameras call - simulates API failure or no V2 data
2812+
2813+
var client = new UniFiClientResponse
2814+
{
2815+
Mac = "a8:9c:6c:06:0c:cc",
2816+
Name = "storage-core"
2817+
};
2818+
2819+
// Act
2820+
var result = service.DetectDeviceType(client);
2821+
2822+
// Assert - falls back to MAC OUI (Camera) since we have no drive data
2823+
// This is expected - without the V2 API data, we can't distinguish
2824+
result.Category.Should().Be(ClientDeviceCategory.Camera);
2825+
}
2826+
2827+
[Fact]
2828+
public void ProtectCameraCollection_IsDriveDevice_ReturnsTrueForKnownMac()
2829+
{
2830+
var collection = new ProtectCameraCollection();
2831+
collection.AddDriveDevice("a8:9c:6c:06:0c:cc");
2832+
2833+
collection.IsDriveDevice("a8:9c:6c:06:0c:cc").Should().BeTrue();
2834+
collection.IsDriveDevice("A8:9C:6C:06:0C:CC").Should().BeTrue();
2835+
}
2836+
2837+
[Fact]
2838+
public void ProtectCameraCollection_IsDriveDevice_ReturnsFalseForUnknownMac()
2839+
{
2840+
var collection = new ProtectCameraCollection();
2841+
collection.AddDriveDevice("a8:9c:6c:06:0c:cc");
2842+
2843+
collection.IsDriveDevice("aa:bb:cc:dd:ee:ff").Should().BeFalse();
2844+
collection.IsDriveDevice(null).Should().BeFalse();
2845+
collection.IsDriveDevice("").Should().BeFalse();
2846+
}
2847+
2848+
[Fact]
2849+
public void ProtectCameraCollection_DriveDeviceCount_ReturnsCorrectCount()
2850+
{
2851+
var collection = new ProtectCameraCollection();
2852+
collection.DriveDeviceCount.Should().Be(0);
2853+
2854+
collection.AddDriveDevice("a8:9c:6c:06:0c:cc");
2855+
collection.AddDriveDevice("a8:9c:6c:06:0c:cd");
2856+
collection.DriveDeviceCount.Should().Be(2);
2857+
}
2858+
2859+
[Fact]
2860+
public void DetectDeviceType_DriveDeviceTakesPriorityOverNamePattern()
2861+
{
2862+
// Arrange - even if name matches a pattern, drive device detection should win
2863+
var service = new DeviceTypeDetectionService();
2864+
var cameras = new ProtectCameraCollection();
2865+
cameras.AddDriveDevice("a8:9c:6c:06:0c:cc");
2866+
service.SetProtectCameras(cameras);
2867+
2868+
var client = new UniFiClientResponse
2869+
{
2870+
Mac = "a8:9c:6c:06:0c:cc",
2871+
Name = "My Security Camera" // misleading name
2872+
};
2873+
2874+
// Act
2875+
var result = service.DetectDeviceType(client);
2876+
2877+
// Assert - drive device classification wins
2878+
result.Category.Should().Be(ClientDeviceCategory.NAS);
2879+
result.ConfidenceScore.Should().Be(100);
2880+
}
2881+
2882+
#endregion
27132883
}

0 commit comments

Comments
 (0)