Skip to content

Commit c50a080

Browse files
authored
Merge pull request #318 from rennerdo30/fix/mobile-native-guard
fix(mobile): make the native-VPN safety gate enforced instead of documented
2 parents 1a3b581 + 5a608e8 commit c50a080

4 files changed

Lines changed: 266 additions & 24 deletions

File tree

mobile/README.md

Lines changed: 23 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -123,7 +123,9 @@ mobile/
123123
- **`src/native/BifrostVpn.ts`** — typed JS bridge. `isNativeVpnAvailable()`
124124
returns `false` whenever the native module is not linked (Expo Go, the current
125125
scaffold, web), and every method degrades to a safe no-op / clear error so the
126-
app keeps building and running against the REST VPN flow.
126+
app keeps building and running against the REST VPN flow. Separately,
127+
`NATIVE_DATA_PATH_IS_SECURE` gates *use* of the native path on the data path
128+
actually being a tunnel — see "Why it's gated off" below.
127129

128130
### What is still required (DEFERRED — needs platform toolchains)
129131

@@ -162,10 +164,26 @@ account for the NE entitlement, and a real Bifrost server to test against):
162164

163165
### Why it's gated off
164166

165-
The bridge defaults to the server-side VPN flow (`selectVpnMode()` returns
166-
`'server'` unless a real native module is linked). This keeps a known-insecure
167-
raw-UDP forwarder from ever running silently. Wiring the UI to the native path
168-
should happen only after step 1 above is complete and validated on-device.
167+
Two independent conditions must both hold before any traffic can take the native
168+
path, and only one of them is a build-system fact:
169+
170+
1. the native module is linked (`isNativeVpnAvailable()`), and
171+
2. `NATIVE_DATA_PATH_IS_SECURE` in `src/native/BifrostVpn.ts` is `true`.
172+
173+
Condition 2 is currently `false`, so `selectVpnMode()` returns `'server'` and
174+
`BifrostVpn.start()` refuses **even in a build where the module is linked**.
175+
That matters because applying the config plugin — step 2/3 below — is enough to
176+
satisfy condition 1 on its own; without condition 2, finishing the plumbing
177+
would silently start routing user traffic over the raw-UDP forwarder while the
178+
UI said "VPN".
179+
180+
`BifrostVpn.requestPermission()` is gated the same way: the app does not ask the
181+
OS for tunnel permission for a tunnel it will refuse to start.
182+
183+
`src/native/nativeVpn.test.ts` asserts all of this, including that the flag is
184+
`false`. Flipping the flag fails that suite by design — updating those
185+
assertions is how you record that a real data path has landed (step 1) and been
186+
validated on-device (step 4).
169187

170188
## Useful commands
171189

mobile/src/native/BifrostVpn.ts

Lines changed: 64 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -14,8 +14,12 @@
1414
// The current native packet forwarders are RAW-UDP PLACEHOLDERS, not a secure
1515
// tunnel. They must be replaced with a real WireGuard/OpenVPN data path
1616
// (e.g. wireguard-go via gomobile, or the WireGuardKit / wireguard-android
17-
// libraries) before being shipped or enabled by default. Do not present the
18-
// native path as "secure" until that work lands and is validated on-device.
17+
// libraries) before being shipped or enabled by default.
18+
//
19+
// That is enforced, not merely documented: NATIVE_DATA_PATH_IS_SECURE below
20+
// is false, so start() refuses and selectVpnMode() keeps returning 'server'
21+
// even in a build where the module *is* linked. Linking the native module is
22+
// a build-system step and must not by itself be enough to route traffic.
1923

2024
import { NativeModules, NativeEventEmitter, Platform } from 'react-native'
2125

@@ -87,8 +91,37 @@ const LINK_HINT =
8791
'Native VPN module is not linked. See mobile/README.md for the iOS Network ' +
8892
'Extension / Android VpnService build steps.'
8993

90-
const nativeModule: BifrostVpnNativeModule | undefined =
91-
(NativeModules as Record<string, BifrostVpnNativeModule | undefined>).BifrostVpn
94+
const PLACEHOLDER_HINT =
95+
'Native VPN data path is a raw-UDP placeholder, not a secure tunnel. ' +
96+
'Refusing to carry traffic over it. See NATIVE_DATA_PATH_IS_SECURE in ' +
97+
'src/native/BifrostVpn.ts.'
98+
99+
/**
100+
* Whether the on-device native data path is a real, validated secure tunnel.
101+
*
102+
* This is deliberately a hard-coded constant rather than configuration. The
103+
* native packet forwarders are raw-UDP placeholders (see the header comment),
104+
* so carrying traffic over them would present an insecure link to the user as
105+
* a VPN. Linking the native module is a build-system step -- applying the
106+
* config plugin is enough to make `isNativeVpnAvailable()` return true -- and
107+
* on its own that must not be sufficient to route traffic.
108+
*
109+
* Flip this to true only together with a real WireGuard/OpenVPN data path that
110+
* has been validated on-device. `nativeVpn.test.ts` asserts it is false and
111+
* will fail when it changes; that failure is the point, and updating those
112+
* assertions is the deliberate acknowledgement that the data path is real.
113+
*/
114+
export const NATIVE_DATA_PATH_IS_SECURE: boolean = false
115+
116+
/**
117+
* Look the native module up on every call rather than capturing it at import
118+
* time. TurboModules can be registered lazily, so a module captured during the
119+
* first import of this file may be missing even in a build that links it.
120+
*/
121+
function getNativeModule(): BifrostVpnNativeModule | undefined {
122+
return (NativeModules as Record<string, BifrostVpnNativeModule | undefined>)
123+
.BifrostVpn
124+
}
92125

93126
/**
94127
* Whether the on-device native VPN module is actually linked into this build.
@@ -98,17 +131,30 @@ const nativeModule: BifrostVpnNativeModule | undefined =
98131
* back to the server-side/client REST VPN flow otherwise.
99132
*/
100133
export function isNativeVpnAvailable(): boolean {
101-
return Platform.OS !== 'web' && nativeModule != null
134+
return Platform.OS !== 'web' && getNativeModule() != null
135+
}
136+
137+
/**
138+
* Whether the on-device native VPN may actually be used to carry traffic.
139+
*
140+
* This is the check callers want. `isNativeVpnAvailable()` answers only "is the
141+
* module linked?"; this also requires the data path to be a real tunnel, so a
142+
* build that links the placeholder still falls back to the server-side flow.
143+
*/
144+
export function isNativeVpnUsable(): boolean {
145+
return NATIVE_DATA_PATH_IS_SECURE && isNativeVpnAvailable()
102146
}
103147

104148
let eventEmitter: NativeEventEmitter | null = null
105149

106150
function getEmitter(): NativeEventEmitter | null {
107151
if (!isNativeVpnAvailable()) return null
108152
if (eventEmitter == null) {
109-
// nativeModule is non-null here per isNativeVpnAvailable().
153+
// The module is non-null here per isNativeVpnAvailable().
110154
eventEmitter = new NativeEventEmitter(
111-
nativeModule as unknown as ConstructorParameters<typeof NativeEventEmitter>[0]
155+
getNativeModule() as unknown as ConstructorParameters<
156+
typeof NativeEventEmitter
157+
>[0]
112158
)
113159
}
114160
return eventEmitter
@@ -132,30 +178,37 @@ const DISCONNECTED_STATUS: NativeVpnStatus = {
132178
*/
133179
export const BifrostVpn = {
134180
isAvailable: isNativeVpnAvailable,
181+
isUsable: isNativeVpnUsable,
135182

136183
async requestPermission(): Promise<boolean> {
137184
if (!isNativeVpnAvailable()) return false
138-
return nativeModule!.requestPermission()
185+
// Do not ask the OS for tunnel permission for a path we will refuse to
186+
// start; the prompt would imply a capability the build does not have.
187+
if (!NATIVE_DATA_PATH_IS_SECURE) return false
188+
return getNativeModule()!.requestPermission()
139189
},
140190

141191
async start(config: NativeVpnConfig): Promise<void> {
142192
if (!isNativeVpnAvailable()) {
143193
throw new Error(LINK_HINT)
144194
}
195+
if (!NATIVE_DATA_PATH_IS_SECURE) {
196+
throw new Error(PLACEHOLDER_HINT)
197+
}
145198
if (!config.serverAddress) {
146199
throw new Error('serverAddress is required to start the native VPN')
147200
}
148-
return nativeModule!.start(config)
201+
return getNativeModule()!.start(config)
149202
},
150203

151204
async stop(): Promise<void> {
152205
if (!isNativeVpnAvailable()) return
153-
return nativeModule!.stop()
206+
return getNativeModule()!.stop()
154207
},
155208

156209
async getStatus(): Promise<NativeVpnStatus> {
157210
if (!isNativeVpnAvailable()) return { ...DISCONNECTED_STATUS }
158-
return nativeModule!.getStatus()
211+
return getNativeModule()!.getStatus()
159212
},
160213

161214
/**

mobile/src/native/index.ts

Lines changed: 12 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,8 @@
77
export {
88
BifrostVpn,
99
isNativeVpnAvailable,
10+
isNativeVpnUsable,
11+
NATIVE_DATA_PATH_IS_SECURE,
1012
} from './BifrostVpn'
1113
export type {
1214
NativeVpnConfig,
@@ -21,17 +23,19 @@ import type { NativeVpnConfig } from './BifrostVpn'
2123
/**
2224
* Decide which VPN path to use.
2325
*
24-
* Returns 'native' only when the on-device native VPN module is actually linked
25-
* into the running build; otherwise 'server' so the app falls back to the
26-
* existing client/server REST VPN flow (api.enableVPN / api.disableVPN).
26+
* Returns 'native' only when the on-device native VPN module is linked into the
27+
* running build *and* its data path is a real secure tunnel; otherwise 'server'
28+
* so the app falls back to the client/server REST VPN flow (api.enableVPN /
29+
* api.disableVPN).
2730
*
28-
* NOTE: even when 'native' is returned, the current native data path is a
29-
* raw-UDP placeholder, not a secure tunnel (see README). Callers should treat
30-
* the native path as experimental and keep it opt-in until the real
31-
* WireGuard/OpenVPN integration lands.
31+
* The second condition is what keeps this honest. The native forwarders are
32+
* currently raw-UDP placeholders, and merely applying the config plugin would
33+
* make the module linked -- so gating on "linked" alone would silently route
34+
* user traffic over an insecure link labelled VPN. See
35+
* NATIVE_DATA_PATH_IS_SECURE in ./BifrostVpn.
3236
*/
3337
export function selectVpnMode(): 'native' | 'server' {
34-
return BifrostVpn.isAvailable() ? 'native' : 'server'
38+
return BifrostVpn.isUsable() ? 'native' : 'server'
3539
}
3640

3741
/**
Lines changed: 167 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,167 @@
1+
// Tests for the native VPN capability gate.
2+
//
3+
// The point of this file is the invariant in `refuses to route traffic over the
4+
// placeholder data path`: linking the native module must NOT be sufficient to
5+
// carry user traffic, because the native forwarders are raw-UDP placeholders.
6+
// The gate is a code path, not a comment, so it is testable -- and these
7+
// assertions fail the moment NATIVE_DATA_PATH_IS_SECURE is flipped, which is
8+
// the deliberate acknowledgement that a real tunnel has landed.
9+
10+
import { describe, it, mock, beforeEach } from 'node:test'
11+
import assert from 'node:assert/strict'
12+
13+
/** Mutable stand-in for react-native's NativeModules registry. */
14+
const nativeModules: Record<string, unknown> = {}
15+
16+
/** Records what the "native" side was actually asked to do. */
17+
interface NativeCalls {
18+
start: unknown[]
19+
stop: number
20+
requestPermission: number
21+
}
22+
23+
const calls: NativeCalls = { start: [], stop: 0, requestPermission: 0 }
24+
25+
const platform = { OS: 'ios' as string }
26+
27+
class FakeNativeEventEmitter {
28+
addListener(): { remove(): void } {
29+
return { remove() {} }
30+
}
31+
}
32+
33+
// `namedExports` is deprecated in favour of `exports` at runtime, but the
34+
// pinned @types/node does not declare `exports` on MockModuleOptions yet, and
35+
// `npm run typecheck` is a gate. Switch when the types catch up.
36+
mock.module('react-native', {
37+
namedExports: {
38+
NativeModules: nativeModules,
39+
NativeEventEmitter: FakeNativeEventEmitter,
40+
Platform: platform,
41+
},
42+
})
43+
44+
const { BifrostVpn, isNativeVpnAvailable, isNativeVpnUsable, NATIVE_DATA_PATH_IS_SECURE } =
45+
await import('./BifrostVpn.ts')
46+
const { selectVpnMode } = await import('./index.ts')
47+
48+
/** Install a fake linked native module, as a config plugin would. */
49+
function linkNativeModule(): void {
50+
nativeModules.BifrostVpn = {
51+
async requestPermission() {
52+
calls.requestPermission += 1
53+
return true
54+
},
55+
async start(config: unknown) {
56+
calls.start.push(config)
57+
},
58+
async stop() {
59+
calls.stop += 1
60+
},
61+
async getStatus() {
62+
return {
63+
connected: true,
64+
serverAddress: 'vpn.example',
65+
tunnelAddress: '10.0.0.2',
66+
bytesIn: 1,
67+
bytesOut: 2,
68+
}
69+
},
70+
}
71+
}
72+
73+
function unlinkNativeModule(): void {
74+
delete nativeModules.BifrostVpn
75+
}
76+
77+
beforeEach(() => {
78+
unlinkNativeModule()
79+
platform.OS = 'ios'
80+
calls.start = []
81+
calls.stop = 0
82+
calls.requestPermission = 0
83+
})
84+
85+
describe('native VPN capability gate', () => {
86+
it('declares the placeholder data path insecure', () => {
87+
assert.equal(
88+
NATIVE_DATA_PATH_IS_SECURE,
89+
false,
90+
'NATIVE_DATA_PATH_IS_SECURE must stay false while the native forwarders ' +
91+
'are raw-UDP placeholders. If a real, on-device-validated ' +
92+
'WireGuard/OpenVPN data path has landed, update this file together ' +
93+
'with the flag -- do not flip the flag alone.'
94+
)
95+
})
96+
97+
it('refuses to route traffic over the placeholder data path', async () => {
98+
linkNativeModule()
99+
100+
// The module is linked, so "is it available?" is now true...
101+
assert.equal(isNativeVpnAvailable(), true)
102+
103+
// ...but that must not be enough to use it.
104+
assert.equal(isNativeVpnUsable(), false)
105+
assert.equal(selectVpnMode(), 'server')
106+
107+
await assert.rejects(
108+
() => BifrostVpn.start({ serverAddress: 'vpn.example' }),
109+
/raw-UDP placeholder/
110+
)
111+
assert.deepEqual(calls.start, [], 'the native side must never be asked to start')
112+
})
113+
114+
it('does not prompt for OS tunnel permission it cannot honour', async () => {
115+
linkNativeModule()
116+
117+
assert.equal(await BifrostVpn.requestPermission(), false)
118+
assert.equal(
119+
calls.requestPermission,
120+
0,
121+
'no OS permission prompt for a tunnel that will refuse to start'
122+
)
123+
})
124+
125+
it('reports the module unavailable when it is not linked', async () => {
126+
assert.equal(isNativeVpnAvailable(), false)
127+
assert.equal(isNativeVpnUsable(), false)
128+
assert.equal(selectVpnMode(), 'server')
129+
130+
await assert.rejects(
131+
() => BifrostVpn.start({ serverAddress: 'vpn.example' }),
132+
/not linked/
133+
)
134+
})
135+
136+
it('reports the module unavailable on web even when linked', () => {
137+
linkNativeModule()
138+
platform.OS = 'web'
139+
140+
assert.equal(isNativeVpnAvailable(), false)
141+
assert.equal(selectVpnMode(), 'server')
142+
})
143+
144+
it('keeps stop and getStatus safe no-ops when unavailable', async () => {
145+
await BifrostVpn.stop()
146+
assert.equal(calls.stop, 0)
147+
148+
const status = await BifrostVpn.getStatus()
149+
assert.equal(status.connected, false)
150+
assert.equal(status.bytesIn, 0)
151+
})
152+
153+
it('still forwards stop to a linked module so a tunnel can always be torn down', async () => {
154+
linkNativeModule()
155+
156+
await BifrostVpn.stop()
157+
assert.equal(calls.stop, 1, 'stop must reach the native side regardless of the gate')
158+
})
159+
160+
it('resolves the native module lazily, not at import time', () => {
161+
// The module was absent when this file first imported the bridge; linking it
162+
// afterwards must still be observed.
163+
assert.equal(isNativeVpnAvailable(), false)
164+
linkNativeModule()
165+
assert.equal(isNativeVpnAvailable(), true)
166+
})
167+
})

0 commit comments

Comments
 (0)