Skip to content

Commit

Permalink
PIA-1263 Remove unnecessary SessionConfig.interface field
Browse files Browse the repository at this point in the history
We only care about the bindIp
  • Loading branch information
kp-john-mair committed Feb 18, 2024
1 parent 4eea6fd commit 942be44
Show file tree
Hide file tree
Showing 6 changed files with 18 additions and 18 deletions.
20 changes: 10 additions & 10 deletions ProxyExtension/IO/FlowHandler.swift
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,7 @@ import NIO
// * The bindIp - which we use to bind ipv4 sockets to change default routing behaviour
// * The eventLoopGroup - required to setup the NIO Inbound Handlers
struct SessionConfig {
var bindIp: String { interface.ip4()! }
let interface: NetworkInterfaceProtocol
let bindIp: String
// We need to make this optional so that we can
// leave it nil in tests - tests do not use an EventLoopGroup
let eventLoopGroup: MultiThreadedEventLoopGroup!
Expand Down Expand Up @@ -36,13 +35,9 @@ final class FlowHandler: FlowHandlerProtocol {
}

public func handleNewFlow(_ flow: Flow, vpnState: VpnState) -> Bool {
let sessionConfig = SessionConfig(
interface: NetworkInterface(interfaceName: vpnState.bindInterface),
eventLoopGroup: eventLoopGroup)

switch FlowPolicy.policyFor(flow: flow, vpnState: vpnState) {
case .proxy:
return startProxySession(flow: flow, sessionConfig: sessionConfig)
return startProxySession(flow: flow, vpnState: vpnState)
case .block:
log(.info, "blocking a vpnOnly flow from \(flow.sourceAppSigningIdentifier)")
flow.closeReadAndWrite()
Expand All @@ -53,14 +48,19 @@ final class FlowHandler: FlowHandlerProtocol {
}
}

private func startProxySession(flow: Flow, sessionConfig: SessionConfig) -> Bool {
guard sessionConfig.interface.ip4() != nil else {
log(.error, "Cannot find ipv4 ip for interface: \(sessionConfig.interface.interfaceName)" +
private func startProxySession(flow: Flow, vpnState: VpnState) -> Bool {
let interface = NetworkInterface(interfaceName: vpnState.bindInterface)

// Verify we have a valid bindIp - if not, trace it and ignore the flow
guard let bindIp = interface.ip4() else {
log(.error, "Cannot find ipv4 ip for interface: \(interface.interfaceName)" +
" - ignoring matched flow: \(flow.sourceAppSigningIdentifier)")
// TODO: Should block the flow instead - especially for vpnOnly flows?
return false
}

let sessionConfig = SessionConfig(bindIp: bindIp, eventLoopGroup: eventLoopGroup)

flow.openFlow { error in
guard error == nil else {
log(.error, "\(flow.sourceAppSigningIdentifier) \"\(error!.localizedDescription)\" in \(String(describing: flow.self)) open()")
Expand Down
4 changes: 2 additions & 2 deletions ProxyTests/ChannelCreatorTCPTest.swift
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ final class ChannelCreatorTCPTest: QuickSpec {
// Use an invalid IP. Even though it's well-formed it is
// invalid because it won't match any
// Ip assigned to any of the local interfaces
interface: MockNetworkInterface(ip: "1.1.1.1"),
bindIp: "1.1.1.1",
eventLoopGroup: MultiThreadedEventLoopGroup(numberOfThreads: 1))

let channelCreator = ChannelCreatorTCP(id: 0, flow: mockFlow, config: config)
Expand All @@ -45,7 +45,7 @@ final class ChannelCreatorTCPTest: QuickSpec {
let mockFlow = MockFlowTCP()
let config = SessionConfig(
// Garbage ip - not well-formed.
interface: MockNetworkInterface(ip: "asdfasd"),
bindIp: "asdfasd",
eventLoopGroup: MultiThreadedEventLoopGroup(numberOfThreads: 1))

let channelCreator = ChannelCreatorTCP(id: 0, flow: mockFlow, config: config)
Expand Down
6 changes: 3 additions & 3 deletions ProxyTests/ChannelCreatorUDPTest.swift
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ final class ChannelCreatorUDPTest: QuickSpec {
let mockFlow = MockFlowUDP()
let config = SessionConfig(
// Use a localhost IP - a bind is guaranteed to succeed
interface: MockNetworkInterface(ip: "127.0.0.1"),
bindIp: "127.0.0.1",
eventLoopGroup: MultiThreadedEventLoopGroup(numberOfThreads: 1))

let channelCreator = ChannelCreatorUDP(id: 0, flow: mockFlow, config: config)
Expand All @@ -39,7 +39,7 @@ final class ChannelCreatorUDPTest: QuickSpec {
// Use an invalid IP. Even though it's well-formed it is
// invalid because it won't match any
// Ip assigned to any of the local interfaces
interface: MockNetworkInterface(ip: "8.8.8.8"),
bindIp: "8.8.8.8",
eventLoopGroup: MultiThreadedEventLoopGroup(numberOfThreads: 1))

let channelCreator = ChannelCreatorUDP(id: 0, flow: mockFlow, config: config)
Expand All @@ -65,7 +65,7 @@ final class ChannelCreatorUDPTest: QuickSpec {
let mockFlow = MockFlowUDP()
let config = SessionConfig(
// Use an invalid IP - garbage, not even an ip
interface: MockNetworkInterface(ip: "asdfasd"),
bindIp: "asdfasd",
eventLoopGroup: MultiThreadedEventLoopGroup(numberOfThreads: 1))

let channelCreator = ChannelCreatorUDP(id: 0, flow: mockFlow, config: config)
Expand Down
2 changes: 1 addition & 1 deletion ProxyTests/DefaultProxySessionFactoryTest.swift
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ import NIO
class DefaultProxySessionFactoryTest: QuickSpec {
override class func spec() {
let sessionConfig = SessionConfig(
interface: MockNetworkInterface(),
bindIp: "", // Not used
// We don't need this in tests, and it's not used anyway
// since we set an explicit channel (using the session.channel setter)
eventLoopGroup: nil)
Expand Down
2 changes: 1 addition & 1 deletion ProxyTests/ProxySessionTCPTest.swift
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ import NIO
class ProxySessionTCPTest: QuickSpec {
override class func spec() {
let sessionConfig = SessionConfig(
interface: MockNetworkInterface(),
bindIp: "", // Not used
// We don't need this in tests, and it's not used anyway
// since we set an explicit channel (using the session.channel setter)
eventLoopGroup: nil)
Expand Down
2 changes: 1 addition & 1 deletion ProxyTests/ProxySessionUDPTest.swift
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ import NIO
class ProxySessionUDPTest: QuickSpec {
override class func spec() {
let sessionConfig = SessionConfig(
interface: MockNetworkInterface(),
bindIp: "", // Not used,
// We don't need this in tests, and it's not used anyway
// since we set an explicit channel (using the session.channel setter)
eventLoopGroup: nil)
Expand Down

0 comments on commit 942be44

Please sign in to comment.