diff --git a/src/DnsmasqWebUI.Tests/Services/EffectiveConfig/CrossOption/CrossOptionRulesTests.cs b/src/DnsmasqWebUI.Tests/Services/EffectiveConfig/CrossOption/CrossOptionRulesTests.cs index 7fa6e8b..2f4625a 100644 --- a/src/DnsmasqWebUI.Tests/Services/EffectiveConfig/CrossOption/CrossOptionRulesTests.cs +++ b/src/DnsmasqWebUI.Tests/Services/EffectiveConfig/CrossOption/CrossOptionRulesTests.cs @@ -67,6 +67,9 @@ public class CrossOptionRulesTests var issues = rule.Evaluate(Ctx(cfg)); Assert.Single(issues); Assert.Equal(FieldIssueSeverity.Warning, issues[0].Severity); + Assert.Equal( + EffectiveConfigCrossOptionContext.FieldKeyForOption(DnsmasqConfKeys.Server), + issues[0].FieldKey); } [Theory] @@ -101,6 +104,166 @@ public class CrossOptionRulesTests Assert.Empty(rule.Evaluate(Ctx(cfg))); } + [Theory] + [InlineData("192.168.1.0/24,8.8.8.8")] + [InlineData("10.0.0.0/8,8.8.8.8")] + [InlineData("172.16.0.0/12,8.8.8.8")] + public void BogusPrivBlocksPrivateReverseServerRule_Warns_for_rfc1918_rev_server(string revServerValue) + { + var cfg = CrossOptionTestHelpers.BaselineConfig() with + { + BogusPriv = true, + RevServerValues = [revServerValue] + }; + var rule = new BogusPrivBlocksPrivateReverseServerRule(); + var issues = rule.Evaluate(Ctx(cfg)); + Assert.Single(issues); + Assert.Equal( + EffectiveConfigCrossOptionContext.FieldKeyForOption(DnsmasqConfKeys.RevServer), + issues[0].FieldKey); + } + + [Theory] + [InlineData("8.8.8.0/24,1.1.1.1")] + [InlineData("1.0.0.0/8,1.1.1.1")] + public void BogusPrivBlocksPrivateReverseServerRule_No_issue_for_public_rev_server(string revServerValue) + { + var cfg = CrossOptionTestHelpers.BaselineConfig() with + { + BogusPriv = true, + RevServerValues = [revServerValue] + }; + var rule = new BogusPrivBlocksPrivateReverseServerRule(); + Assert.Empty(rule.Evaluate(Ctx(cfg))); + } + + [Fact] + public void BogusPrivBlocksPrivateReverseServerRule_Emits_separate_issues_for_server_and_rev_server() + { + var cfg = CrossOptionTestHelpers.BaselineConfig() with + { + BogusPriv = true, + ServerValues = ["/0.168.192.in-addr.arpa/10.0.0.1"], + RevServerValues = ["192.168.2.0/24,8.8.8.8"] + }; + var rule = new BogusPrivBlocksPrivateReverseServerRule(); + var issues = rule.Evaluate(Ctx(cfg)); + Assert.Equal(2, issues.Count); + Assert.Contains( + issues, + i => i.FieldKey == EffectiveConfigCrossOptionContext.FieldKeyForOption(DnsmasqConfKeys.Server)); + Assert.Contains( + issues, + i => i.FieldKey == EffectiveConfigCrossOptionContext.FieldKeyForOption(DnsmasqConfKeys.RevServer)); + } + + [Fact] + public void BogusPrivBlocksPrivateReverseServerRule_Pending_rev_server_overrides_disk() + { + var cfg = CrossOptionTestHelpers.BaselineConfig() with + { + BogusPriv = true, + RevServerValues = ["8.8.8.0/24,1.1.1.1"] + }; + var pending = new[] + { + new PendingOptionChange( + "resolver", + DnsmasqConfKeys.RevServer, + Array.Empty(), + new[] { "192.168.1.0/24,8.8.8.8" }, + null) + }; + var ctx = new EffectiveConfigCrossOptionContext(CrossOptionTestHelpers.Status(cfg), pending); + var rule = new BogusPrivBlocksPrivateReverseServerRule(); + var issues = rule.Evaluate(ctx); + Assert.Single(issues); + Assert.Equal( + EffectiveConfigCrossOptionContext.FieldKeyForOption(DnsmasqConfKeys.RevServer), + issues[0].FieldKey); + } + + [Fact] + public void NoPingWithDhcpSequentialIpRule_Warns_when_both_set() + { + var cfg = CrossOptionTestHelpers.BaselineConfig() with + { + NoPing = true, + DhcpSequentialIp = true + }; + var rule = new NoPingWithDhcpSequentialIpRule(); + var issues = rule.Evaluate(Ctx(cfg)); + Assert.Single(issues); + Assert.Equal(FieldIssueSeverity.Warning, issues[0].Severity); + Assert.Equal( + EffectiveConfigCrossOptionContext.FieldKeyForOption(DnsmasqConfKeys.DhcpSequentialIp), + issues[0].FieldKey); + } + + [Theory] + [InlineData(true, false)] + [InlineData(false, true)] + [InlineData(false, false)] + public void NoPingWithDhcpSequentialIpRule_No_issue_unless_both(bool noPing, bool sequential) + { + var cfg = CrossOptionTestHelpers.BaselineConfig() with + { + NoPing = noPing, + DhcpSequentialIp = sequential + }; + var rule = new NoPingWithDhcpSequentialIpRule(); + Assert.Empty(rule.Evaluate(Ctx(cfg))); + } + + [Fact] + public void DnsPortZeroWithTftpOrPxeRule_Warns_when_port_zero_and_enable_tftp() + { + var cfg = CrossOptionTestHelpers.BaselineConfig() with + { + Port = 0, + EnableTftp = true + }; + var rule = new DnsPortZeroWithTftpOrPxeRule(); + var issues = rule.Evaluate(Ctx(cfg)); + Assert.Single(issues); + Assert.Equal( + EffectiveConfigCrossOptionContext.FieldKeyForOption(DnsmasqConfKeys.Port), + issues[0].FieldKey); + } + + [Fact] + public void DnsPortZeroWithTftpOrPxeRule_Warns_when_port_zero_and_pxe_service() + { + var cfg = CrossOptionTestHelpers.BaselineConfig() with + { + Port = 0, + PxeServiceValues = ["x86PC,Boot from network,pxelinux"] + }; + var rule = new DnsPortZeroWithTftpOrPxeRule(); + Assert.Single(rule.Evaluate(Ctx(cfg))); + } + + [Fact] + public void DnsPortZeroWithTftpOrPxeRule_No_issue_when_port_nonzero_even_with_netboot() + { + var cfg = CrossOptionTestHelpers.BaselineConfig() with + { + Port = 53, + EnableTftp = true, + PxeServiceValues = ["x86PC,Boot from network,pxelinux"] + }; + var rule = new DnsPortZeroWithTftpOrPxeRule(); + Assert.Empty(rule.Evaluate(Ctx(cfg))); + } + + [Fact] + public void DnsPortZeroWithTftpOrPxeRule_No_issue_when_port_zero_without_tftp_or_pxe() + { + var cfg = CrossOptionTestHelpers.BaselineConfig() with { Port = 0 }; + var rule = new DnsPortZeroWithTftpOrPxeRule(); + Assert.Empty(rule.Evaluate(Ctx(cfg))); + } + [Fact] public void DnssecPrerequisitesRule_Error_when_build_lacks_dnssec() { diff --git a/src/DnsmasqWebUI/Infrastructure/Services/EffectiveConfig/Validation/Rules/CrossOptionRules.cs b/src/DnsmasqWebUI/Infrastructure/Services/EffectiveConfig/Validation/Rules/CrossOptionRules.cs index 15d38d5..4dfd2c3 100644 --- a/src/DnsmasqWebUI/Infrastructure/Services/EffectiveConfig/Validation/Rules/CrossOptionRules.cs +++ b/src/DnsmasqWebUI/Infrastructure/Services/EffectiveConfig/Validation/Rules/CrossOptionRules.cs @@ -1,4 +1,6 @@ using System.Linq; +using System.Net; +using System.Net.Sockets; using System.Text.RegularExpressions; using DnsmasqWebUI.Infrastructure.Services.EffectiveConfig.Metadata; using DnsmasqWebUI.Infrastructure.Services.EffectiveConfig.Validation.Abstractions; @@ -86,10 +88,17 @@ public sealed class RebindExceptionsRequireStopDnsRebindRule : IEffectiveConfigC } } +/// +/// Man: bogus-priv avoids forwarding RFC1918 (etc.) reverse lookups; combined with explicit private reverse +/// forwarding via server= or rev-server=, PTR traffic may not reach the configured upstream (subtle). +/// public sealed class BogusPrivBlocksPrivateReverseServerRule : IEffectiveConfigCrossOptionRule { public string Id => "resolver.bogus-priv-private-reverse-server"; + private const string PrivateReverseMessage = + "bogus-priv takes priority over private reverse lookup forwarding, so those PTR queries may never reach the configured upstream server."; + public IReadOnlyList Evaluate(EffectiveConfigCrossOptionContext context) { var bogusPriv = context.GetBool(DnsmasqConfKeys.BogusPriv, cfg => cfg.BogusPriv); @@ -97,18 +106,31 @@ public sealed class BogusPrivBlocksPrivateReverseServerRule : IEffectiveConfigCr return []; var servers = context.GetMulti(DnsmasqConfKeys.Server, cfg => cfg.ServerValues); - var hasPrivateReverseServer = servers.Any(LooksLikeRfc1918InAddrArpaServer); + var revServers = context.GetMulti(DnsmasqConfKeys.RevServer, cfg => cfg.RevServerValues); + var serverMatch = servers.Any(LooksLikeRfc1918InAddrArpaServer); + var revMatch = revServers.Any(IsRfc1918RevServerIpv4Target); - if (!hasPrivateReverseServer) + if (!serverMatch && !revMatch) return []; - return - [ - new FieldIssue( + var issues = new List(); + if (serverMatch) + { + issues.Add(new FieldIssue( EffectiveConfigCrossOptionContext.FieldKeyForOption(DnsmasqConfKeys.Server), - "bogus-priv takes priority over private reverse lookup forwarding, so those PTR queries may never reach the configured upstream server.", - FieldIssueSeverity.Warning) - ]; + PrivateReverseMessage, + FieldIssueSeverity.Warning)); + } + + if (revMatch) + { + issues.Add(new FieldIssue( + EffectiveConfigCrossOptionContext.FieldKeyForOption(DnsmasqConfKeys.RevServer), + PrivateReverseMessage, + FieldIssueSeverity.Warning)); + } + + return issues; } // RFC1918 reverse zone labels only; avoids substring false positives (e.g. 210, 192.0.0.x, 172.15). @@ -133,6 +155,106 @@ public sealed class BogusPrivBlocksPrivateReverseServerRule : IEffectiveConfigCr return false; } + + /// True when the rev-server reverse target (IPv4 CIDR or host) overlaps RFC1918 space. + private static bool IsRfc1918RevServerIpv4Target(string line) + { + if (string.IsNullOrWhiteSpace(line)) + return false; + + var target = line.Split(',', 2)[0].Trim(); + var slash = target.IndexOf('/'); + var ipText = slash >= 0 ? target[..slash] : target; + if (!IPAddress.TryParse(ipText, out var ip) || ip.AddressFamily != AddressFamily.InterNetwork) + return false; + + var prefix = slash < 0 ? 32 : int.TryParse(target[(slash + 1)..], out var p) ? p : -1; + if (prefix is < 0 or > 32) + return false; + + var ipUint = ToIpv4UInt32(ip); + var mask = prefix == 0 ? 0u : uint.MaxValue << (32 - prefix); + var netStart = ipUint & mask; + var netEnd = netStart | ~mask; + + return OverlapsRfc1918Ipv4(netStart, netEnd); + } + + private static uint ToIpv4UInt32(IPAddress ip) + { + var b = ip.GetAddressBytes(); + return ((uint)b[0] << 24) | ((uint)b[1] << 16) | ((uint)b[2] << 8) | b[3]; + } + + private static bool OverlapsRfc1918Ipv4(uint netStart, uint netEnd) + { + // 10.0.0.0/8, 172.16.0.0/12, 192.168.0.0/16 + (uint s, uint e)[] blocks = + [ + (0x0A00_0000u, 0x0AFF_FFFFu), + (0xAC10_0000u, 0xAC1F_FFFFu), + (0xC0A8_0000u, 0xC0A8_FFFFu) + ]; + + foreach (var (bs, be) in blocks) + { + if (netStart <= be && bs <= netEnd) + return true; + } + + return false; + } +} + +/// +/// Man (2.80): no-ping with dhcp-sequential-ip breaks DHCP — documented regression/failure mode. +/// +public sealed class NoPingWithDhcpSequentialIpRule : IEffectiveConfigCrossOptionRule +{ + public string Id => "dhcp.no-ping-with-dhcp-sequential-ip"; + + public IReadOnlyList Evaluate(EffectiveConfigCrossOptionContext context) + { + var noPing = context.GetBool(DnsmasqConfKeys.NoPing, cfg => cfg.NoPing); + var sequential = context.GetBool(DnsmasqConfKeys.DhcpSequentialIp, cfg => cfg.DhcpSequentialIp); + if (!noPing || !sequential) + return []; + + return + [ + new FieldIssue( + EffectiveConfigCrossOptionContext.FieldKeyForOption(DnsmasqConfKeys.DhcpSequentialIp), + "no-ping together with dhcp-sequential-ip is a known-broken combination in dnsmasq; disable one or the other.", + FieldIssueSeverity.Warning) + ]; + } +} + +/// +/// Man: port=0 disables the DNS listener; TFTP/PXE still expect a working setup — 2.87 fixed crashes for port 0 with netboot. +/// +public sealed class DnsPortZeroWithTftpOrPxeRule : IEffectiveConfigCrossOptionRule +{ + public string Id => "dns.port-zero-with-tftp-or-pxe"; + + public IReadOnlyList Evaluate(EffectiveConfigCrossOptionContext context) + { + if (context.GetInt(DnsmasqConfKeys.Port, cfg => cfg.Port) != 0) + return []; + + var tftp = context.GetBool(DnsmasqConfKeys.EnableTftp, cfg => cfg.EnableTftp); + var pxe = context.GetMulti(DnsmasqConfKeys.PxeService, cfg => cfg.PxeServiceValues); + if (!tftp && pxe.Count == 0) + return []; + + return + [ + new FieldIssue( + EffectiveConfigCrossOptionContext.FieldKeyForOption(DnsmasqConfKeys.Port), + "port=0 disables the DNS listener. With enable-tftp or pxe-service, netboot can fail or hit dnsmasq bugs fixed only in recent releases—use a non-zero port or turn off TFTP/PXE unless DNS is intentionally off.", + FieldIssueSeverity.Warning) + ]; + } } public sealed class DnssecPrerequisitesRule : IEffectiveConfigCrossOptionRule @@ -280,6 +402,10 @@ public sealed class Filterwin2kSrvWarningRule : IEffectiveConfigCrossOptionRule } } +/// +/// Man: from 2.86, address=/domain/IP applies only to A/AAAA; other types forward unless local=/domain/ +/// restores earlier “whole domain” behavior. Precedence among address, server, and defaults was refined through 2.92. +/// public sealed class AddressLocalDnsmasq286CompatibilityRule : IEffectiveConfigCrossOptionRule { public string Id => "resolver.address-local-dnsmasq-286-compat"; @@ -311,7 +437,7 @@ public sealed class AddressLocalDnsmasq286CompatibilityRule : IEffectiveConfigCr [ new FieldIssue( EffectiveConfigCrossOptionContext.FieldKeyForOption(DnsmasqConfKeys.Address), - $"From dnsmasq 2.86, address= with a domain and IP may forward non-A/AAAA queries upstream unless the domain is also covered by local=. Add matching local= entries for: {preview}{extra}.", + $"Since dnsmasq 2.86, address= with a domain and IP answers only A/AAAA locally; other RR types are forwarded unless the domain is also covered by local= (see the dnsmasq man page). Add matching local= for: {preview}{extra}.", FieldIssueSeverity.Warning) ]; } @@ -395,3 +521,11 @@ public sealed class LocalServiceIgnoredByBindSettingsRule : IEffectiveConfigCros ]; } } + +/* + * Deferred cross-option: dhcp-authoritative vs DHCPv6 REBIND-without-lease (dnsmasq 2.92 changelog). + * Implement only when either: + * (1) the man page states a configuration-level requirement to set dhcp-authoritative for the affected DHCPv6 mode, or + * (2) this codebase can reliably detect stateful DHCPv6 (vs SLAAC-only) from DhcpRanges / RA flags. + * Otherwise prefer tooltips or external docs, not a static warning. + */