diff --git a/app/network/Shared/Utilities/SplitRuleHostInput.swift b/app/network/Shared/Utilities/SplitRuleHostInput.swift new file mode 100644 index 00000000..d7517a56 --- /dev/null +++ b/app/network/Shared/Utilities/SplitRuleHostInput.swift @@ -0,0 +1,242 @@ +// +// SplitRuleHostInput.swift +// URnetwork +// +// What a hand-typed split rule host is allowed to be. +// +// The invariant this file exists to hold: NEVER be more permissive than the +// Go matcher. That matcher has no error channel -- anything it cannot parse +// as a wildcard, a prefix or an address is filed as an exact host name +// (connect/ip_block_action.go:385-406), so a typo produces a rule that is +// created, persisted, mirrored, seeded, counted in the "N split rules" card +// and matched never. There is nowhere downstream to report that. Rejecting a +// value the matcher would have accepted only costs the user a rephrase; +// accepting one it will never match costs them a rule they believe is +// working. When in doubt, reject. +// +// Kept free of SwiftUI and of the SDK so it can be tested directly. The +// durable fix is one normalizer in Go called by the matcher itself and bound +// through gomobile, the way host-name collapsing already works; until that +// lands this is the second implementation of a grammar that has one owner. +// + +import Foundation +import Network + +enum SplitRuleHostError: Equatable { + case notAscii + case badName + case badWildcard + case badRange + case duplicate + /// Another value in the same rule already matches everything this would. + case covered(by: String) +} + +struct SplitRuleHostValidation: Equatable { + /// The value as it will be stored, lowercased and trimmed, with an IP + /// range masked to its network address the way the matcher keys it. + let normalized: String? + let error: SplitRuleHostError? + /// Set when the stored value differs from what was typed, so the change + /// is something the user agreed to rather than something Go did quietly. + let note: String? + + var isAccepted: Bool { normalized != nil && error == nil } +} + +enum SplitRuleHostInput { + + /// Longest legal DNS name, and longest legal label. + private static let maxNameLength = 253 + private static let maxLabelLength = 63 + + /// Validates one typed value against the values already in the rule. + static func validate(_ raw: String, existing: [String] = []) -> SplitRuleHostValidation { + let host = raw.trimmingCharacters(in: .whitespacesAndNewlines).lowercased() + if host.isEmpty { + return SplitRuleHostValidation(normalized: nil, error: nil, note: nil) + } + // the matcher lowercases and compares bytes; a resolved name arrives + // as punycode, so a unicode name here could only ever be dead + guard host.allSatisfy({ $0.isASCII }) else { + return rejected(.notAscii) + } + + let normalized: String + var note: String? = nil + + if let base = host.dropPrefixIfPresent("**.") ?? host.dropPrefixIfPresent("*.") { + guard isValidName(base) else { + return rejected(.badWildcard) + } + normalized = host + } else if host.contains("/") { + guard let masked = maskedPrefix(host) else { + return rejected(.badRange) + } + normalized = masked + if masked != host { + note = masked + } + } else if let address = normalizedAddress(host) { + normalized = address + if address != host { + note = address + } + } else { + guard isValidName(host) else { + return rejected(.badName) + } + normalized = host + } + + if existing.contains(normalized) { + return rejected(.duplicate) + } + if let cover = existing.first(where: { covers($0, normalized) }) { + return SplitRuleHostValidation(normalized: nil, error: .covered(by: cover), note: nil) + } + return SplitRuleHostValidation(normalized: normalized, error: nil, note: note) + } + + private static func rejected(_ error: SplitRuleHostError) -> SplitRuleHostValidation { + SplitRuleHostValidation(normalized: nil, error: error, note: nil) + } + + /// A dotted name of at least two labels. Single-label names parse as + /// exact hosts in Go but can never match a resolved destination, so they + /// are refused here rather than stored as a rule that does nothing. + static func isValidName(_ name: String) -> Bool { + guard !name.isEmpty, name.count <= maxNameLength, !name.hasSuffix(".") else { + return false + } + let labels = name.split(separator: ".", omittingEmptySubsequences: false) + guard 2 <= labels.count else { + return false + } + return labels.allSatisfy { label in + guard !label.isEmpty, label.count <= maxLabelLength else { + return false + } + guard !label.hasPrefix("-"), !label.hasSuffix("-") else { + return false + } + return label.allSatisfy { $0.isLowercaseASCIILetter || $0.isASCIIDigit || $0 == "-" } + } + } + + /// An address, in the form the matcher stores it: an ipv4-mapped ipv6 + /// address is unmapped there, so it is unmapped here too. + static func normalizedAddress(_ value: String) -> String? { + if let v4 = IPv4Address(value) { + return v4.debugDescription + } + guard let v6 = IPv6Address(value) else { + return nil + } + // ::ffff:1.2.3.4 -> 1.2.3.4, matching netip's Unmap() + let bytes = [UInt8](v6.rawValue) + if bytes.count == 16, + bytes[0..<10].allSatisfy({ $0 == 0 }), + bytes[10] == 0xff, bytes[11] == 0xff, + let mapped = IPv4Address(Data(bytes[12..<16]), nil) { + return mapped.debugDescription + } + return v6.debugDescription + } + + /// A CIDR range, masked to its network address. Go masks it before + /// matching, so `192.168.1.42/24` becomes `192.168.1.0/24` there; doing + /// it here means the chip shows what is actually in force. + static func maskedPrefix(_ value: String) -> String? { + let parts = value.split(separator: "/", omittingEmptySubsequences: false) + guard parts.count == 2, + let bits = Int(parts[1]), + !parts[1].isEmpty else { + return nil + } + let address = String(parts[0]) + if let v4 = IPv4Address(address) { + guard 0...32 ~= bits, let masked = mask(v4.rawValue, bits: bits, of: 4), + let result = IPv4Address(masked, nil) else { + return nil + } + return "\(result.debugDescription)/\(bits)" + } + if let v6 = IPv6Address(address) { + guard 0...128 ~= bits, let masked = mask(v6.rawValue, bits: bits, of: 16), + let result = IPv6Address(masked, nil) else { + return nil + } + return "\(result.debugDescription)/\(bits)" + } + return nil + } + + private static func mask(_ raw: Data, bits: Int, of byteCount: Int) -> Data? { + var bytes = [UInt8](raw) + guard bytes.count == byteCount else { + return nil + } + for index in 0.. Bool { + if let base = wildcard.dropPrefixIfPresent("**.") { + return candidate == base || candidate.hasSuffix("." + base) + } + if let base = wildcard.dropPrefixIfPresent("*.") { + return candidate.hasSuffix("." + base) + } + return false + } + + /// The reason a value was refused, for the line under the field. + static func message(for error: SplitRuleHostError) -> String { + switch error { + case .notAscii: + return String(localized: "Use the ASCII form of the name.") + case .badName: + return String(localized: "Enter a host name like example.com.") + case .badWildcard: + return String(localized: "A wildcard needs a name after it, like *.example.com.") + case .badRange: + return String(localized: "Enter an IP range like 10.0.0.0/8.") + case .duplicate: + return String(localized: "Already in this rule.") + case .covered(let by): + return String(format: String(localized: "Already covered by %@ in this rule."), by) + } + } +} + +private extension String { + func dropPrefixIfPresent(_ prefix: String) -> String? { + hasPrefix(prefix) ? String(dropFirst(prefix.count)) : nil + } +} + +private extension Character { + var isLowercaseASCIILetter: Bool { "a"..."z" ~= self } + var isASCIIDigit: Bool { "0"..."9" ~= self } +} diff --git a/app/network/Shared/ViewModels/BlockActionsStore.swift b/app/network/Shared/ViewModels/BlockActionsStore.swift index 331472d0..82cc5dbb 100644 --- a/app/network/Shared/ViewModels/BlockActionsStore.swift +++ b/app/network/Shared/ViewModels/BlockActionsStore.swift @@ -176,6 +176,9 @@ class BlockActionsStore: ObservableObject { */ @Published private(set) var blockActions: [BlockActionItem] = [] @Published private(set) var splitRules: [SplitRuleItem] = [] + /// A rule may be created from here only once this process has a list it + /// can vouch for -- see `createRule`. + @Published private(set) var canCreateRule: Bool = false @Published private(set) var allowedCount: Int = 0 @Published private(set) var blockedCount: Int = 0 @@ -322,6 +325,7 @@ class BlockActionsStore: ObservableObject { blockActions = [] splitRules = [] sdkOverrides = [] + canCreateRule = false allowedCount = 0 blockedCount = 0 exitsByIp = [:] @@ -590,6 +594,16 @@ class BlockActionsStore: ObservableObject { if afterEdit || device.getConnected() { persistOverrides() } + // Latched here rather than in `setup` because the change listeners are + // registered before the first read, so a connect landing in between + // cannot be missed. Monotonic: a disconnect clears the remote's + // service but never its last known list, so a base that was real stays + // real for this device's life. The local state is optional -- a build + // with no app group has none -- and that degrades correctly, to + // "connected only", which is the safe half of the test. + if !canCreateRule, device.getConnected() || localState?.getBlockActionOverrides() != nil { + canCreateRule = true + } } /** @@ -634,8 +648,19 @@ class BlockActionsStore: ObservableObject { * creates a split rule applying `mode` to the selected host values; * see `SplitRuleMode` */ + /// Refuses to create until this process has seen the whole list, because + /// creating against a list it has not seen would DELETE the rest. + /// + /// With the rpc down the remote builds the full list it will push on the + /// next connect out of whatever base it can see, and an unseeded base is + /// empty -- so a rule created then would replace the extension's saved + /// rules rather than join them (see `DeviceManager.initDevice`). The base + /// is real once either the mirror exists or the device has connected, and + /// tapping an activity row could never reach this state because the + /// activity list is empty with no rpc. A hand-written rule can, so the + /// check has to live here and not only on the affordance. func createRule(hosts: [String], mode: SplitRuleMode) { - guard let device = self.device, !hosts.isEmpty else { + guard let device = self.device, !hosts.isEmpty, canCreateRule else { return } let override = SdkBlockActionOverride() diff --git a/app/network/Shared/Views/Stats/SplitRulesView.swift b/app/network/Shared/Views/Stats/SplitRulesView.swift index 5e97863d..d9ee880a 100644 --- a/app/network/Shared/Views/Stats/SplitRulesView.swift +++ b/app/network/Shared/Views/Stats/SplitRulesView.swift @@ -42,6 +42,7 @@ struct SplitRulesView: View { @State private var topBaseline: CGFloat? = nil private static let topMarkerId = "split-rules-top" + private static let newRuleTargetId = "split-rules-new" private var pendingCount: Int { let displayedIds = Set(displayedActions.map { $0.id }) @@ -99,8 +100,10 @@ struct SplitRulesView: View { header: sectionHeader("Rules") ) { + addRuleRow + if blockActionsStore.splitRules.isEmpty { - Text("Tap traffic below to route it locally or hold it to one provider.") + Text("Add a host above, or tap traffic below to route it locally or hold it to one provider.") .font(themeManager.currentTheme.secondaryBodyFont) .foregroundColor(themeManager.currentTheme.textFaintColor) .listRowBackground(Color.clear) @@ -320,6 +323,61 @@ struct SplitRulesView: View { return values } + /** + * Adds a rule for a host that has not been seen in the activity below -- + * the only way to write a rule for traffic that has not happened yet. + * + * Disabled until the store has a list it can vouch for: creating against + * a list this process has not seen would replace the extension's saved + * rules rather than join them (see `BlockActionsStore.createRule`). That + * is one connect away, and the note below says so. + */ + @ViewBuilder + private var addRuleRow: some View { + VStack(alignment: .leading, spacing: 4) { + Button(action: { + editorTarget = EditorTarget( + id: Self.newRuleTargetId, + candidates: [], + selected: [], + ruleId: nil, + // route-locally is the only mode that always takes effect + // on its own: a merge replaces an existing route override + // when the incoming one is local, so a first hand-written + // rule in another mode could silently do nothing + mode: .excluded + ) + }) { + HStack(spacing: 8) { + Image(systemName: "plus.circle.fill") + .foregroundColor( + blockActionsStore.canCreateRule + ? .urGreen + : themeManager.currentTheme.textFaintColor + ) + Text("Add a rule") + .font(themeManager.currentTheme.bodyFont) + .foregroundColor( + blockActionsStore.canCreateRule + ? themeManager.currentTheme.textColor + : themeManager.currentTheme.textFaintColor + ) + Spacer() + } + .contentShape(Rectangle()) + } + .buttonStyle(.plain) + .disabled(!blockActionsStore.canCreateRule) + + if !blockActionsStore.canCreateRule { + Text("Connect once to load the rules you already have.") + .font(themeManager.currentTheme.secondaryBodyFont) + .foregroundColor(themeManager.currentTheme.textFaintColor) + } + } + .listRowBackground(Color.clear) + } + private func sectionHeader(_ text: LocalizedStringKey) -> some View { Text(text) .font(themeManager.currentTheme.secondaryBodyFont) @@ -620,6 +678,11 @@ struct SplitRuleEditorView: View { @State private var selection: Set @State private var mode: SplitRuleMode + /// Hosts typed here, newest first, ahead of the ones this rule was opened + /// with. Kept separate from `candidates` so the list is a plain function + /// of both and there is no state to keep in step. + @State private var addedHosts: [String] = [] + @State private var newHost: String = "" init(candidates: [String], initialSelection: Set, ruleId: String?, initialMode: SplitRuleMode) { self.candidates = candidates @@ -632,6 +695,26 @@ struct SplitRuleEditorView: View { ruleId != nil } + private var editableCandidates: [String] { + addedHosts + candidates + } + + private var validation: SplitRuleHostValidation { + SplitRuleHostInput.validate(newHost, existing: editableCandidates) + } + + /// Adds the typed host and selects it: a value typed by hand is one the + /// user wants in the rule, so making them tick it as well is a step that + /// only ever costs them the rule silently doing nothing. + private func addTypedHost() { + guard let host = validation.normalized else { + return + } + addedHosts.insert(host, at: 0) + selection.insert(host) + newHost = "" + } + var body: some View { VStack(alignment: .leading, spacing: 0) { @@ -675,7 +758,9 @@ struct SplitRuleEditorView: View { .padding(.horizontal) List { - ForEach(candidates, id: \.self) { host in + hostEntryRow + + ForEach(editableCandidates, id: \.self) { host in HStack { Text(host) .font(themeManager.currentTheme.bodyFont) @@ -709,7 +794,7 @@ struct SplitRuleEditorView: View { UrButton( text: isEditing ? "Update" : "Create", action: { - let hosts = candidates.filter { selection.contains($0) } + let hosts = editableCandidates.filter { selection.contains($0) } if let ruleId = ruleId { blockActionsStore.updateRule(id: ruleId, hosts: hosts, mode: mode) } else { @@ -743,6 +828,55 @@ struct SplitRuleEditorView: View { * one exclusive mode choice: a radio-style circle with the mode title * and what it does */ + /** + * Type a host, a wildcard or an IP range into the rule. + * + * The matcher has no error channel -- a value it cannot parse is filed as + * an exact host name and simply never matches -- so the grammar is + * checked here and the reason is said out loud. See `SplitRuleHostInput`. + */ + @ViewBuilder + private var hostEntryRow: some View { + let validation = self.validation + VStack(alignment: .leading, spacing: 4) { + HStack(spacing: 8) { + TextField("example.com, *.example.com, 10.0.0.0/8", text: $newHost) + .font(themeManager.currentTheme.bodyFont) + .foregroundColor(themeManager.currentTheme.textColor) + .autocorrectionDisabled() + #if os(iOS) + .keyboardType(.URL) + .textInputAutocapitalization(.never) + #endif + .onSubmit { addTypedHost() } + + Button(action: { addTypedHost() }) { + Image(systemName: "plus.circle.fill") + .foregroundColor( + validation.isAccepted + ? .urGreen + : themeManager.currentTheme.textFaintColor + ) + } + .buttonStyle(.plain) + .disabled(!validation.isAccepted) + } + + // silent while the field is empty: a rejection reason before + // anything has been typed reads as an error the user caused + if let error = validation.error { + Text(SplitRuleHostInput.message(for: error)) + .font(themeManager.currentTheme.secondaryBodyFont) + .foregroundColor(themeManager.currentTheme.dangerColor) + } else if let note = validation.note { + Text(String(format: String(localized: "Saved as %@"), note)) + .font(themeManager.currentTheme.secondaryBodyFont) + .foregroundColor(themeManager.currentTheme.textMutedColor) + } + } + .listRowBackground(Color.clear) + } + private func modeRow( _ rowMode: SplitRuleMode, title: LocalizedStringKey, diff --git a/app/networkTests/SplitRuleHostInputTests.swift b/app/networkTests/SplitRuleHostInputTests.swift new file mode 100644 index 00000000..65b75974 --- /dev/null +++ b/app/networkTests/SplitRuleHostInputTests.swift @@ -0,0 +1,151 @@ +// +// SplitRuleHostInputTests.swift +// networkTests +// +// The grammar behind the hand-typed split rule host. +// +// These matter more than most input validation because the failure is +// silent. The Go matcher files anything it cannot parse as an exact host +// name and there is no error channel anywhere on the write path, so an +// accepted-but-wrong value becomes a rule that is created, persisted, +// mirrored, counted on the connect card and matched never. Every case below +// that asserts a REJECTION is guarding that, not tidiness. +// + +import Testing +import Foundation +@testable import URnetwork + +struct SplitRuleHostInputTests { + + @Test func plainNamesAreAccepted() { + #expect(SplitRuleHostInput.validate("example.com").normalized == "example.com") + #expect(SplitRuleHostInput.validate("a1366.dscapi6.akamai.net").normalized == "a1366.dscapi6.akamai.net") + #expect(SplitRuleHostInput.validate("my-host.example.co.uk").normalized == "my-host.example.co.uk") + } + + @Test func inputIsTrimmedAndLowercased() { + #expect(SplitRuleHostInput.validate(" Example.COM ").normalized == "example.com") + } + + /// The matcher lowercases and compares bytes, and a resolved name arrives + /// as punycode, so a unicode name could only ever be a dead rule. + @Test func unicodeNamesAreRefused() { + #expect(SplitRuleHostInput.validate("münchen.de").error == .notAscii) + } + + /// Parses in Go as an exact host, but nothing resolves to it, so it would + /// be a rule that does nothing. + @Test func singleLabelNamesAreRefused() { + #expect(SplitRuleHostInput.validate("localhost").error == .badName) + #expect(SplitRuleHostInput.validate("router").error == .badName) + } + + @Test func malformedNamesAreRefused() { + #expect(SplitRuleHostInput.validate("example..com").error == .badName) + #expect(SplitRuleHostInput.validate("-example.com").error == .badName) + #expect(SplitRuleHostInput.validate("example-.com").error == .badName) + #expect(SplitRuleHostInput.validate("example.com.").error == .badName) + #expect(SplitRuleHostInput.validate("exa mple.com").error == .badName) + } + + @Test func wildcardsAreAccepted() { + #expect(SplitRuleHostInput.validate("*.example.com").normalized == "*.example.com") + #expect(SplitRuleHostInput.validate("**.example.com").normalized == "**.example.com") + } + + @Test func wildcardsNeedAName() { + #expect(SplitRuleHostInput.validate("*.").error == .badWildcard) + #expect(SplitRuleHostInput.validate("**.").error == .badWildcard) + #expect(SplitRuleHostInput.validate("*.com").error == .badWildcard) + } + + @Test func addressesAreAccepted() { + #expect(SplitRuleHostInput.validate("1.2.3.4").normalized == "1.2.3.4") + #expect(SplitRuleHostInput.validate("2001:db8::1").normalized == "2001:db8::1") + } + + /// The matcher unmaps an ipv4-mapped ipv6 address before keying on it, so + /// storing the mapped form would key something the matcher never looks up. + @Test func mappedAddressesAreUnmapped() { + #expect(SplitRuleHostInput.validate("::ffff:1.2.3.4").normalized == "1.2.3.4") + } + + /// Go masks a prefix before matching, so an unmasked one is silently + /// rewritten there. Doing it here means the chip shows what is in force. + @Test func rangesAreMaskedToTheirNetwork() { + let validation = SplitRuleHostInput.validate("192.168.1.42/24") + #expect(validation.normalized == "192.168.1.0/24") + #expect(validation.note == "192.168.1.0/24") + } + + /// Prefix lengths that end inside a byte. Every case here traps rather + /// than fails when the mask is built in Int, and the field is validated on + /// every keystroke -- so typing "10.0.0.0/24" crashed the app at the "2", + /// on its way to a length the earlier tests did cover. + @Test func rangesThatEndInsideAByteAreMasked() { + #expect(SplitRuleHostInput.validate("10.1.2.3/12").normalized == "10.0.0.0/12") + #expect(SplitRuleHostInput.validate("192.168.1.130/25").normalized == "192.168.1.128/25") + #expect(SplitRuleHostInput.validate("192.168.1.42/2").normalized == "192.0.0.0/2") + #expect(SplitRuleHostInput.validate("2001:db8::1/36").normalized == "2001:db8::/36") + } + + /// Every prefix length must be survivable, because the user types through + /// all of them one keystroke at a time. + @Test func everyPrefixLengthIsSurvivable() { + for bits in 0...32 { + _ = SplitRuleHostInput.validate("192.168.1.42/\(bits)") + } + for bits in 0...128 { + _ = SplitRuleHostInput.validate("2001:db8::1/\(bits)") + } + } + + @Test func alreadyMaskedRangesCarryNoNote() { + let validation = SplitRuleHostInput.validate("10.0.0.0/8") + #expect(validation.normalized == "10.0.0.0/8") + #expect(validation.note == nil) + } + + @Test func malformedRangesAreRefused() { + #expect(SplitRuleHostInput.validate("10.0.0.0/").error == .badRange) + #expect(SplitRuleHostInput.validate("10.0.0.0/33").error == .badRange) + #expect(SplitRuleHostInput.validate("example.com/24").error == .badRange) + #expect(SplitRuleHostInput.validate("2001:db8::/129").error == .badRange) + } + + @Test func anEmptyFieldIsNeitherAcceptedNorAnError() { + let validation = SplitRuleHostInput.validate(" ") + #expect(validation.normalized == nil) + #expect(validation.error == nil) + #expect(!validation.isAccepted) + } + + @Test func duplicatesAreRefused() { + #expect(SplitRuleHostInput.validate("example.com", existing: ["example.com"]).error == .duplicate) + // the dedupe is against the NORMALIZED form, not the typed one + #expect(SplitRuleHostInput.validate("EXAMPLE.com", existing: ["example.com"]).error == .duplicate) + } + + /// The rule row collapses a bare name into a wildcard that already covers + /// it, so adding both would show one chip where two values were typed. + @Test func valuesAlreadyCoveredByAWildcardAreRefused() { + #expect(SplitRuleHostInput.validate("a.example.com", existing: ["*.example.com"]).error + == .covered(by: "*.example.com")) + #expect(SplitRuleHostInput.validate("example.com", existing: ["**.example.com"]).error + == .covered(by: "**.example.com")) + // *. is subdomains only, so the bare base is not covered by it + #expect(SplitRuleHostInput.validate("example.com", existing: ["*.example.com"]).isAccepted) + // an unrelated name is not covered + #expect(SplitRuleHostInput.validate("example.org", existing: ["*.example.com"]).isAccepted) + } + + @Test func everyRejectionHasSomethingToSay() { + let errors: [SplitRuleHostError] = [ + .notAscii, .badName, .badWildcard, .badRange, .duplicate, .covered(by: "*.example.com"), + ] + for error in errors { + #expect(!SplitRuleHostInput.message(for: error).isEmpty) + } + } +}