Skip to content

Commit ac35dbd

Browse files
committed
Fail closed on WireGuard startup errors
1 parent 5b02d57 commit ac35dbd

5 files changed

Lines changed: 142 additions & 63 deletions

File tree

app/build.gradle

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ apply plugin: 'org.jetbrains.kotlin.android'
88
// single `./gradlew assemble` for contributors.
99
def wgbridgeSrcDir = file("$rootDir/wgbridge")
1010
def wgbridgeAar = file("$buildDir/wgbridge/wgbridge.aar")
11+
def gomobileVersion = 'v0.0.0-20260410095206-2cfb76559b7b'
1112

1213
// Locate the `go` binary. Android Studio launches Gradle with a sanitized
1314
// PATH that omits /usr/local/go/bin and Homebrew dirs, so falling back to
@@ -82,8 +83,8 @@ tasks.register('wgbridgeBind') {
8283
exec {
8384
environment env
8485
commandLine goBin, 'install',
85-
'golang.org/x/mobile/cmd/gomobile@latest',
86-
'golang.org/x/mobile/cmd/gobind@latest'
86+
"golang.org/x/mobile/cmd/gomobile@${gomobileVersion}",
87+
"golang.org/x/mobile/cmd/gobind@${gomobileVersion}"
8788
}
8889
if (!file(gomobileBin).canExecute() || !file(gobindBin).canExecute()) {
8990
throw new GradleException("Installed gomobile/gobind but ${gopath}/bin still missing one of them.")

app/src/main/java/eu/faircode/netguard/ServiceSinkhole.java

Lines changed: 44 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -185,6 +185,7 @@ public class ServiceSinkhole extends VpnService {
185185
private static final int NOTIFY_UPDATE = 8;
186186
public static final int NOTIFY_EXTERNAL = 9;
187187
private static final int NOTIFY_DOH_ERROR = 11;
188+
private static final int NOTIFY_WG_ERROR = 12;
188189

189190
public static final String EXTRA_COMMAND = "Command";
190191
private static final String EXTRA_REASON = "Reason";
@@ -548,7 +549,8 @@ private void start() {
548549
if (vpn == null)
549550
throw new StartFailedException(getString((R.string.msg_start_failed)));
550551

551-
startNative(vpn, listAllowed, listRule);
552+
if (!startNative(vpn, listAllowed, listRule))
553+
return;
552554

553555
// Start DoH proxy if enabled
554556
net.kollnig.missioncontrol.dns.DnsProxyServer.getInstance(ServiceSinkhole.this).start();
@@ -662,7 +664,8 @@ private void reload(boolean interactive) {
662664
if (vpn == null)
663665
throw new StartFailedException(getString((R.string.msg_start_failed)));
664666

665-
startNative(vpn, listAllowed, listRule);
667+
if (!startNative(vpn, listAllowed, listRule))
668+
return;
666669

667670
// Update DoH proxy state based on current settings
668671
net.kollnig.missioncontrol.dns.DnsProxyServer.getInstance(ServiceSinkhole.this).checkAndUpdateState();
@@ -1600,7 +1603,7 @@ else if (filter)
16001603
return builder;
16011604
}
16021605

1603-
private void startNative(final ParcelFileDescriptor vpn, List<Rule> listAllowed, List<Rule> listRule) {
1606+
private boolean startNative(final ParcelFileDescriptor vpn, List<Rule> listAllowed, List<Rule> listRule) {
16041607
SharedPreferences prefs = PreferenceManager.getDefaultSharedPreferences(ServiceSinkhole.this);
16051608
boolean log = prefs.getBoolean("log", false);
16061609
boolean log_app = prefs.getBoolean("log_app", true);
@@ -1649,8 +1652,12 @@ private void startNative(final ParcelFileDescriptor vpn, List<Rule> listAllowed,
16491652
vpn,
16501653
() -> jni_wireguard_start(),
16511654
() -> { jni_wireguard_stop(); return kotlin.Unit.INSTANCE; });
1652-
if (!wgOk)
1653-
Log.w(TAG, "WireGuard egress failed to start; falling back to direct");
1655+
if (!wgOk) {
1656+
String wgError = net.kollnig.missioncontrol.wg.WgEgress.INSTANCE.getLastError();
1657+
Log.w(TAG, "WireGuard egress failed to start; blocking traffic: " + wgError);
1658+
showWireGuardErrorNotification(wgError);
1659+
return false;
1660+
}
16541661

16551662
if (tunnelThread == null) {
16561663
Log.i(TAG, "Starting tunnel thread context=" + jni_context);
@@ -1671,6 +1678,8 @@ public void run() {
16711678
Log.i(TAG, "Started tunnel thread");
16721679
}
16731680
}
1681+
1682+
return true;
16741683
}
16751684

16761685
private void stopNative(ParcelFileDescriptor vpn) {
@@ -3404,6 +3413,35 @@ private void showDohErrorNotification() {
34043413
NotificationManagerCompat.from(this).notify(NOTIFY_DOH_ERROR, notification.build());
34053414
}
34063415

3416+
private void showWireGuardErrorNotification(String message) {
3417+
Intent main = new Intent(this, ActivitySettings.class);
3418+
PendingIntent pi = PendingIntentCompat.getActivity(this, NOTIFY_WG_ERROR, main,
3419+
PendingIntent.FLAG_UPDATE_CURRENT);
3420+
3421+
String detail = TextUtils.isEmpty(message)
3422+
? getString(R.string.msg_wg_blocked)
3423+
: getString(R.string.msg_wg_blocked_detail, message);
3424+
3425+
NotificationCompat.Builder builder = new NotificationCompat.Builder(this, "notify");
3426+
builder.setSmallIcon(R.drawable.ic_error_white_24dp)
3427+
.setContentTitle(getString(R.string.msg_wg_blocked_title))
3428+
.setContentText(detail)
3429+
.setContentIntent(pi)
3430+
.setColor(getResources().getColor(R.color.colorTrackerControl))
3431+
.setOngoing(false)
3432+
.setAutoCancel(true);
3433+
3434+
if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.LOLLIPOP)
3435+
builder.setCategory(NotificationCompat.CATEGORY_STATUS)
3436+
.setVisibility(NotificationCompat.VISIBILITY_SECRET);
3437+
3438+
NotificationCompat.BigTextStyle notification = new NotificationCompat.BigTextStyle(builder);
3439+
notification.bigText(detail);
3440+
3441+
if (Util.canNotify(this))
3442+
NotificationManagerCompat.from(this).notify(NOTIFY_WG_ERROR, notification.build());
3443+
}
3444+
34073445
private void showUpdateNotification(String name, String url) {
34083446
if (Util.isFDroidInstall())
34093447
return;
@@ -3433,6 +3471,7 @@ private void removeWarningNotifications() {
34333471
NotificationManagerCompat.from(this).cancel(NOTIFY_AUTOSTART);
34343472
NotificationManagerCompat.from(this).cancel(NOTIFY_ERROR);
34353473
NotificationManagerCompat.from(this).cancel(NOTIFY_DOH_ERROR);
3474+
NotificationManagerCompat.from(this).cancel(NOTIFY_WG_ERROR);
34363475
}
34373476

34383477
private class Builder extends VpnService.Builder {

app/src/main/java/net/kollnig/missioncontrol/wg/WgEgress.kt

Lines changed: 21 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ object WgEgress {
2525
private const val DEFAULT_MTU = 1420
2626

2727
@Volatile private var tunnel: WgTunnel? = null
28+
@Volatile private var lastError: String? = null
2829
private var currentConfig: String? = null
2930
private var currentTunFd: Int = -1
3031

@@ -34,9 +35,8 @@ object WgEgress {
3435
* no-op so reload-induced restarts don't re-handshake.
3536
*
3637
* Returns true on success or already-correct state. Returns false if
37-
* WG was supposed to start but failed; in that case the C side is left
38-
* with `wg_enabled = 0` so traffic isn't black-holed (fail-closed
39-
* applies only when WG is *running* and traffic should be protected).
38+
* WG was supposed to start but failed; in that case the caller must keep
39+
* the VPN from forwarding direct traffic so the user remains fail-closed.
4040
*/
4141
fun startOrUpdate(
4242
wgEnabled: Boolean,
@@ -48,6 +48,7 @@ object WgEgress {
4848
): Boolean {
4949
val wantRunning = wgEnabled && !configText.isNullOrEmpty()
5050
val desiredFd = vpnFd.fd
51+
lastError = null
5152

5253
if (!wantRunning) {
5354
if (tunnel != null) {
@@ -70,19 +71,22 @@ object WgEgress {
7071
val parsed = try {
7172
WgConfigParser.parse(configText!!)
7273
} catch (e: Exception) {
74+
lastError = "Invalid WireGuard config: ${e.message}"
7375
Log.e(TAG, "config parse: ${e.message}")
7476
return false
7577
}
7678

7779
val resolved = try {
7880
withResolvedEndpoints(parsed)
7981
} catch (e: Exception) {
82+
lastError = "WireGuard endpoint resolution failed: ${e.message}"
8083
Log.e(TAG, "endpoint resolve: ${e.message}")
8184
return false
8285
}
8386

8487
val rxFd = startSocketpair()
8588
if (rxFd < 0) {
89+
lastError = "Could not create WireGuard packet socket"
8690
Log.e(TAG, "jni_wireguard_start failed")
8791
return false
8892
}
@@ -101,9 +105,13 @@ object WgEgress {
101105
resolved.toUapi(), rxFd, desiredFd, mtu, protector, logger
102106
)
103107
} catch (e: Throwable) {
108+
lastError = "WireGuard tunnel failed to start: ${e.message ?: e.javaClass.simpleName}"
104109
Log.e(TAG, "Wgbridge.startTunnel failed", e)
110+
closeRawFd(rxFd)
105111
stopSocketpair()
106112
return false
113+
} finally {
114+
if (tunnel != null) closeRawFd(rxFd)
107115
}
108116

109117
currentConfig = configText
@@ -119,6 +127,8 @@ object WgEgress {
119127

120128
fun isRunning(): Boolean = tunnel != null
121129

130+
fun getLastError(): String? = lastError
131+
122132
private fun stopInternal(stopSocketpair: () -> Unit) {
123133
val t = tunnel
124134
tunnel = null
@@ -163,4 +173,12 @@ object WgEgress {
163173
val ip = addr.hostAddress ?: throw IllegalStateException("getHostAddress null for $host")
164174
return if (addr is java.net.Inet6Address) "[$ip]:$port" else "$ip:$port"
165175
}
176+
177+
private fun closeRawFd(fd: Int) {
178+
try {
179+
ParcelFileDescriptor.adoptFd(fd).close()
180+
} catch (e: Throwable) {
181+
Log.w(TAG, "close fd $fd failed", e)
182+
}
183+
}
166184
}

app/src/main/res/values/strings.xml

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -182,6 +182,9 @@
182182
<string name="summary_wg_enabled">Hides device IP. Tunnels TCP and UDP egress. Apps will see network errors during outages — fail-closed, no fall-through to direct.</string>
183183
<string name="summary_wg_config">Paste a [Interface] / [Peer] config from your provider</string>
184184
<string name="msg_wg_config_invalid">Invalid WireGuard config: %s</string>
185+
<string name="msg_wg_blocked_title">WireGuard tunnel unavailable</string>
186+
<string name="msg_wg_blocked">Internet access is blocked until WireGuard starts or the WireGuard setting is disabled.</string>
187+
<string name="msg_wg_blocked_detail">Internet access is blocked until WireGuard starts or the WireGuard setting is disabled. %s</string>
185188
<string name="summary_watchdog">Periodically check if TrackerControl is still running (enter zero to disable this option). This might result in extra battery usage.</string>
186189
<string name="summary_log_logcat">Research mode: identified trackers are logged to ADB and SNI extraction is enabled for better hostname detection. Retrieve logs with adb logcat using tag \'TC-Log\'.</string>
187190

wgbridge/bridge.go

Lines changed: 71 additions & 53 deletions
Original file line numberDiff line numberDiff line change
@@ -10,14 +10,14 @@
1010
// would be re-captured by NetGuard's TUN and loop forever.
1111
//
1212
// Build:
13-
// gomobile bind -target=android -androidapi 23 -o ../app/libs/wgbridge.aar .
13+
//
14+
// gomobile bind -target=android -androidapi 23 -o ../app/libs/wgbridge.aar .
1415
package wgbridge
1516

1617
import (
1718
"errors"
1819
"fmt"
1920
"os"
20-
"strconv"
2121
"sync"
2222

2323
"golang.org/x/sys/unix"
@@ -86,13 +86,7 @@ func StartTunnel(uapiConfig string, outboundRxFd int32, tunWriteFd int32, mtu in
8686
}
8787
t.events <- tun.EventUp
8888

89-
// Snapshot UDP fds before wireguard-go opens its outer sockets, so we
90-
// can identify exactly the new ones afterwards and protect those — and
91-
// only those. Avoids accidentally protecting unrelated sockets that
92-
// belong to anyone else in the process.
93-
udpBefore := snapshotUdpFds()
94-
95-
dev := device.NewDevice(t, conn.NewStdNetBind(), newDeviceLogger(logger))
89+
dev := device.NewDevice(t, newProtectedBind(protect), newDeviceLogger(logger))
9690

9791
if err := dev.IpcSet(uapiConfig); err != nil {
9892
dev.Close()
@@ -105,20 +99,78 @@ func StartTunnel(uapiConfig string, outboundRxFd int32, tunWriteFd int32, mtu in
10599
return nil, fmt.Errorf("device up: %w", err)
106100
}
107101

108-
// Protect each new UDP socket so its egress bypasses the VpnService TUN.
109-
udpAfter := snapshotUdpFds()
110-
for fd := range udpAfter {
111-
if _, existed := udpBefore[fd]; existed {
112-
continue
102+
return &Tunnel{dev: dev, tunDev: t}, nil
103+
}
104+
105+
type protectedBind struct {
106+
inner conn.Bind
107+
protect Protector
108+
}
109+
110+
func newProtectedBind(protect Protector) conn.Bind {
111+
return &protectedBind{inner: conn.NewStdNetBind(), protect: protect}
112+
}
113+
114+
func (b *protectedBind) Open(port uint16) ([]conn.ReceiveFunc, uint16, error) {
115+
fns, actualPort, err := b.inner.Open(port)
116+
if err != nil {
117+
return nil, 0, err
118+
}
119+
if err := b.protectOpenSockets(); err != nil {
120+
_ = b.inner.Close()
121+
return nil, 0, err
122+
}
123+
return fns, actualPort, nil
124+
}
125+
126+
func (b *protectedBind) Close() error { return b.inner.Close() }
127+
func (b *protectedBind) SetMark(mark uint32) error { return b.inner.SetMark(mark) }
128+
func (b *protectedBind) Send(bufs [][]byte, ep conn.Endpoint) error {
129+
return b.inner.Send(bufs, ep)
130+
}
131+
func (b *protectedBind) ParseEndpoint(s string) (conn.Endpoint, error) {
132+
return b.inner.ParseEndpoint(s)
133+
}
134+
func (b *protectedBind) BatchSize() int { return b.inner.BatchSize() }
135+
136+
func (b *protectedBind) protectOpenSockets() error {
137+
peek, ok := b.inner.(conn.PeekLookAtSocketFd)
138+
if !ok {
139+
return errors.New("bind does not expose socket fds")
140+
}
141+
142+
fds := make(map[int]struct{}, 2)
143+
for _, peekFn := range []func() (int, error){peek.PeekLookAtSocketFd4, peek.PeekLookAtSocketFd6} {
144+
fd, ok, err := safePeekSocketFd(peekFn)
145+
if err != nil {
146+
return err
113147
}
114-
if !protect.Protect(int32(fd)) {
115-
dev.Close()
116-
_ = t.Close()
117-
return nil, fmt.Errorf("VpnService.protect(%d) failed", fd)
148+
if ok && fd >= 0 {
149+
fds[fd] = struct{}{}
118150
}
119151
}
152+
if len(fds) == 0 {
153+
return errors.New("bind opened no protectable UDP sockets")
154+
}
120155

121-
return &Tunnel{dev: dev, tunDev: t}, nil
156+
for fd := range fds {
157+
if !b.protect.Protect(int32(fd)) {
158+
return fmt.Errorf("VpnService.protect(%d) failed", fd)
159+
}
160+
}
161+
return nil
162+
}
163+
164+
func safePeekSocketFd(peek func() (int, error)) (fd int, ok bool, err error) {
165+
defer func() {
166+
if recover() != nil {
167+
fd = -1
168+
ok = false
169+
err = nil
170+
}
171+
}()
172+
fd, err = peek()
173+
return fd, err == nil, err
122174
}
123175

124176
// Stop tears down wireguard-go and closes the duplicated fds.
@@ -181,40 +233,6 @@ func (t *socketpairTun) Close() error {
181233
return err2
182234
}
183235

184-
// snapshotUdpFds enumerates UDP sockets in the current process by reading
185-
// /proc/self/fd and filtering by readlink target prefix "socket:". A
186-
// secondary filter (SO_TYPE == SOCK_DGRAM) further narrows it to UDP. The
187-
// returned set's keys are the integer fds.
188-
func snapshotUdpFds() map[int]struct{} {
189-
out := make(map[int]struct{})
190-
entries, err := os.ReadDir("/proc/self/fd")
191-
if err != nil {
192-
return out
193-
}
194-
for _, e := range entries {
195-
fd, err := strconv.Atoi(e.Name())
196-
if err != nil {
197-
continue
198-
}
199-
// Skip stdio.
200-
if fd < 3 {
201-
continue
202-
}
203-
// Filter by socket type.
204-
stype, err := unix.GetsockoptInt(fd, unix.SOL_SOCKET, unix.SO_TYPE)
205-
if err != nil || stype != unix.SOCK_DGRAM {
206-
continue
207-
}
208-
// Filter by socket family.
209-
family, err := unix.GetsockoptInt(fd, unix.SOL_SOCKET, unix.SO_DOMAIN)
210-
if err != nil || (family != unix.AF_INET && family != unix.AF_INET6) {
211-
continue
212-
}
213-
out[fd] = struct{}{}
214-
}
215-
return out
216-
}
217-
218236
func newDeviceLogger(l Logger) *device.Logger {
219237
if l == nil {
220238
return &device.Logger{

0 commit comments

Comments
 (0)