From 285d14e996faefdde06e279d6a63e95d23461c63 Mon Sep 17 00:00:00 2001 From: Denozordec Date: Sun, 4 Oct 2026 23:04:01 +0700 Subject: [PATCH] android: 2.13.15 - exponential direct-dst cooldown, double-start guard, flap-aware auto-connect Log-driven fixes from the 10-04 diag/charon analysis: - direct_tcp: negative-cache window now escalates 45s x2 up to 5 min (Telegram was opening ~3000 connects/h to each of two dead hosts, waking the radio for every leaked 45s probe); skip the unconditional fresh-socket retry on ECONNREFUSED (a refusal is deterministic on the network the socket is bound to; roam-time sockets are already aborted by reprotect) - new dead-destination diag line (dst, fail count, cooldown) via JNI callback when the window escalates or hits every 100th failure - VpnStateService: double-start guard - a second connect() for the same profile while a healthy attempt is in flight is ignored (four starters raced through here within 20 s after the 2.13.14 self-update) - NetworkMonitor/AutoConnectManager: flap detection (>=2 kind changes in 5 min stretches the auto-connect delay to 10 s; a Wi-Fi flap produced a 7-second-lived session and a wasted IKE handshake) - hardcode diag TAGs obfuscated by R8 ("a: update: up to date" -> "UpdateManager: ..."), same for UpdateInstaller/VpnShortcutManager --- src/frontends/android/app/build.gradle | 4 +- .../android/logic/CharonVpnService.java | 15 ++++ .../android/logic/VpnStateService.java | 37 +++++++-- .../logic/autoconnect/AutoConnectManager.kt | 12 +-- .../logic/autoconnect/NetworkMonitor.kt | 39 +++++++++ .../logic/shortcuts/VpnShortcutManager.kt | 3 +- .../android/logic/updates/UpdateInstaller.kt | 3 +- .../android/logic/updates/UpdateManager.kt | 4 +- .../main/jni/libandroidbridge/charonservice.c | 34 ++++++++ .../main/jni/libandroidbridge/charonservice.h | 8 ++ .../jni/libandroidbridge/routing/direct_tcp.c | 82 +++++++++++++++---- 11 files changed, 210 insertions(+), 31 deletions(-) diff --git a/src/frontends/android/app/build.gradle b/src/frontends/android/app/build.gradle index d460062b9..5539d2246 100644 --- a/src/frontends/android/app/build.gradle +++ b/src/frontends/android/app/build.gradle @@ -37,8 +37,8 @@ android { minSdkVersion 21 targetSdkVersion 36 - versionCode 151 - versionName "2.13.14" + versionCode 152 + versionName "2.13.15" /* Base URL of the update storage, trailing slash included */ buildConfigField "String", "UPDATE_BASE_URL", "\"${updateBaseUrl}\"" diff --git a/src/frontends/android/app/src/main/java/org/strongswan/android/logic/CharonVpnService.java b/src/frontends/android/app/src/main/java/org/strongswan/android/logic/CharonVpnService.java index 5dab55e3c..4e6066158 100644 --- a/src/frontends/android/app/src/main/java/org/strongswan/android/logic/CharonVpnService.java +++ b/src/frontends/android/app/src/main/java/org/strongswan/android/logic/CharonVpnService.java @@ -1450,6 +1450,21 @@ public class CharonVpnService extends VpnService implements Runnable, VpnStateSe } } + /** + * Called via JNI when a direct destination's failure cooldown escalates + * (the window grew) or crosses a failure milestone: surfaces the dead + * hosts that apps keep hammering directly, with the current penalty. + * + * @param dst preformatted "ip:port" of the destination + * @param fails failed connect cycles recorded for this destination + * @param cooldownS current cooldown window in seconds + */ + public void logDeadDestination(String dst, int fails, int cooldownS) + { + DiagLog.i(TAG, "direct tcp: dst=" + dst + " dead: " + fails + + " failed connects, cooldown " + cooldownS + "s"); + } + /** * Initialization of charon, provided by libandroidbridge.so * diff --git a/src/frontends/android/app/src/main/java/org/strongswan/android/logic/VpnStateService.java b/src/frontends/android/app/src/main/java/org/strongswan/android/logic/VpnStateService.java index c069d2f59..1d7d95784 100644 --- a/src/frontends/android/app/src/main/java/org/strongswan/android/logic/VpnStateService.java +++ b/src/frontends/android/app/src/main/java/org/strongswan/android/logic/VpnStateService.java @@ -88,6 +88,11 @@ public class VpnStateService extends Service private boolean mRetryFrozen; /* timestamp of the last network-change-triggered immediate retry */ private long mLastNetworkRetry; + /* double-start guard: profile and issue time of the last connect() that + * got through, cleared by disconnect() */ + private static final long CONNECT_DEDUPE_MS = 10000; + private String mLastConnectUuid; + private long mLastConnectIssued; private static final String TAG = VpnStateService.class.getSimpleName(); @@ -330,9 +335,12 @@ public class VpnStateService extends Service */ public void disconnect(boolean autoManaged) { - /* reset any potential retry timer and error state */ + /* reset any potential retry timer and error state, and re-arm the + * double-start guard (a connect after a deliberate disconnect is a + * fresh request and must not be deduped) */ resetRetryTimer(); setError(ErrorState.NO_ERROR); + mLastConnectIssued = 0; if (autoManaged) { /* the auto-connect feature parked the tunnel on a trusted network; @@ -400,6 +408,26 @@ public class VpnStateService extends Service /* pass the previous password along */ profileInfo.putString(VpnProfileDataSource.KEY_PASSWORD, mProfile.getPassword()); } + String uuid = profileInfo.getString(VpnProfileDataSource.KEY_UUID); + if (uuid == null && mProfile != null) + { + uuid = mProfile.getUUID().toString(); + } + /* double-start guard: while a healthy connect attempt for the same + * profile is in flight (or was just issued and the service has not + * reached startConnection() yet) a second start intent only churns + * the daemon - after an app self-update up to four starters (update + * receiver, process-start auto-connect, watchdog, UI) raced through + * here within 20 seconds. Deliberate retries during an error backoff + * (mError set) and requests for another profile always pass. */ + if (uuid != null && uuid.equals(mLastConnectUuid) && mError == ErrorState.NO_ERROR + && (mState == State.CONNECTING + || ((mState == State.DISABLED || mState == State.DISCONNECTING) + && SystemClock.uptimeMillis() - mLastConnectIssued < CONNECT_DEDUPE_MS))) + { + DiagLog.i(TAG, "connect: same profile already connecting, ignored"); + return; + } if (fromScratch) { /* reset if this is a manual retry or a new connection */ @@ -432,13 +460,10 @@ public class VpnStateService extends Service /* remember that a connection is expected so the watchdog can restore * it if this process dies (Always-on VPN is restarted by the system * and bypasses this path anyway) */ - String uuid = profileInfo.getString(VpnProfileDataSource.KEY_UUID); - if (uuid == null && mProfile != null) - { - uuid = mProfile.getUUID().toString(); - } if (uuid != null) { + mLastConnectUuid = uuid; + mLastConnectIssued = SystemClock.uptimeMillis(); SharedPreferences prefs = PreferenceManager.getDefaultSharedPreferences(context); if (!uuid.equals(prefs.getString(Constants.PREF_WATCHDOG_EXPECTED_PROFILE, null))) { diff --git a/src/frontends/android/app/src/main/java/org/strongswan/android/logic/autoconnect/AutoConnectManager.kt b/src/frontends/android/app/src/main/java/org/strongswan/android/logic/autoconnect/AutoConnectManager.kt index 0da4091c0..04ef583d2 100644 --- a/src/frontends/android/app/src/main/java/org/strongswan/android/logic/autoconnect/AutoConnectManager.kt +++ b/src/frontends/android/app/src/main/java/org/strongswan/android/logic/autoconnect/AutoConnectManager.kt @@ -286,11 +286,13 @@ class AutoConnectManager private constructor( return } /* Wi-Fi at the edge of range flaps with a ~10-15 s period: a park - * followed by a quick "untrusted" flip must not bounce the tunnel - * through full reconnects. Right after a park require a longer - * stable-cellular window before starting (the runnable re-evaluates - * and quietly gives up if the trusted Wi-Fi returned in between). */ - val delay = if (SystemClock.elapsedRealtime() - lastParkAt < RECENT_PARK_WINDOW_MS) + * followed by a quick "untrusted" flip - or a network kind that keeps + * bouncing - must not bounce the tunnel through full reconnects (one + * such flap produced a 7-second-lived session and a wasted IKE + * handshake). Require a longer stable-untrusted window before + * starting (the runnable re-evaluates and quietly gives up if a + * trusted Wi-Fi returned in between). */ + val delay = if (SystemClock.elapsedRealtime() - lastParkAt < RECENT_PARK_WINDOW_MS || monitor.isFlapping()) { FLAP_CONNECT_DELAY_MS } diff --git a/src/frontends/android/app/src/main/java/org/strongswan/android/logic/autoconnect/NetworkMonitor.kt b/src/frontends/android/app/src/main/java/org/strongswan/android/logic/autoconnect/NetworkMonitor.kt index f3608a112..da57751da 100644 --- a/src/frontends/android/app/src/main/java/org/strongswan/android/logic/autoconnect/NetworkMonitor.kt +++ b/src/frontends/android/app/src/main/java/org/strongswan/android/logic/autoconnect/NetworkMonitor.kt @@ -23,6 +23,7 @@ import android.net.wifi.WifiInfo import android.os.Build import android.os.Handler import android.os.Looper +import android.os.SystemClock import android.util.Log import org.strongswan.android.logic.DiagLog import java.util.concurrent.ConcurrentHashMap @@ -51,6 +52,8 @@ class NetworkMonitor( private var registered = false private var lastReported: AutoConnectManager.NetworkKind? = null private var ssidRetries = 0 + /* rolling log of network-kind change times, for flap detection (main thread only) */ + private val kindChangeTimes = ArrayDeque() private val events = object { @@ -134,6 +137,7 @@ class NetworkMonitor( ssids.clear() lastReported = null ssidRetries = 0 + kindChangeTimes.clear() } fun forceEvaluate() @@ -305,10 +309,42 @@ class NetworkMonitor( Log.i(TAG, "Network kind $lastReported -> $kind") DiagLog.i(TAG, "network kind $lastReported -> $kind") lastReported = kind + recordKindChange(SystemClock.elapsedRealtime()) onNetworkChanged(kind) } } + /** + * Remember a kind change for [isFlapping], dropping entries that fell out + * of the flap window (plus a hard cap so a pathological callback storm + * cannot grow the log unbounded). + */ + private fun recordKindChange(now: Long) + { + kindChangeTimes.addLast(now) + while (kindChangeTimes.size > KIND_CHANGE_LOG_MAX || + (kindChangeTimes.isNotEmpty() && now - kindChangeTimes.first() > FLAP_WINDOW_MS)) + { + kindChangeTimes.removeFirst() + } + } + + /** + * Whether the network kind is bouncing: at least [FLAP_MIN_CHANGES] kind + * changes (including the latest one) within [FLAP_WINDOW_MS]. Callers + * stretch out auto-connect delays while the underlying network settles + * instead of bouncing the tunnel through short-lived sessions. + */ + fun isFlapping(): Boolean + { + val now = SystemClock.elapsedRealtime() + while (kindChangeTimes.isNotEmpty() && now - kindChangeTimes.first() > FLAP_WINDOW_MS) + { + kindChangeTimes.removeFirst() + } + return kindChangeTimes.size >= FLAP_MIN_CHANGES + } + private val evaluateRunnable = Runnable { evaluateNow() } companion object @@ -317,5 +353,8 @@ class NetworkMonitor( private const val DEBOUNCE_MS = 1500L private const val SSID_RETRY_MS = 400L private const val SSID_RETRY_MAX = 8 + private const val FLAP_WINDOW_MS = 5 * 60_000L + private const val FLAP_MIN_CHANGES = 2 + private const val KIND_CHANGE_LOG_MAX = 16 } } diff --git a/src/frontends/android/app/src/main/java/org/strongswan/android/logic/shortcuts/VpnShortcutManager.kt b/src/frontends/android/app/src/main/java/org/strongswan/android/logic/shortcuts/VpnShortcutManager.kt index a473c0956..644a84224 100644 --- a/src/frontends/android/app/src/main/java/org/strongswan/android/logic/shortcuts/VpnShortcutManager.kt +++ b/src/frontends/android/app/src/main/java/org/strongswan/android/logic/shortcuts/VpnShortcutManager.kt @@ -45,7 +45,8 @@ class VpnShortcutManager private constructor(private val context: Context) { companion object { - private val TAG = VpnShortcutManager::class.java.simpleName + /* hardcoded: the runtime class name is obfuscated by R8 in release builds */ + private const val TAG = "VpnShortcutManager" /** slots always occupied by the connect/disconnect/toggle shortcuts */ private const val RESERVED_SLOTS = 3 diff --git a/src/frontends/android/app/src/main/java/org/strongswan/android/logic/updates/UpdateInstaller.kt b/src/frontends/android/app/src/main/java/org/strongswan/android/logic/updates/UpdateInstaller.kt index 71a5e40ea..25fb6e6c7 100644 --- a/src/frontends/android/app/src/main/java/org/strongswan/android/logic/updates/UpdateInstaller.kt +++ b/src/frontends/android/app/src/main/java/org/strongswan/android/logic/updates/UpdateInstaller.kt @@ -31,7 +31,8 @@ import java.io.File */ object UpdateInstaller { - private val TAG = UpdateInstaller::class.java.simpleName + /* hardcoded: the runtime class name is obfuscated by R8 in release builds */ + private const val TAG = "UpdateInstaller" fun install(context: Context, apk: File) { diff --git a/src/frontends/android/app/src/main/java/org/strongswan/android/logic/updates/UpdateManager.kt b/src/frontends/android/app/src/main/java/org/strongswan/android/logic/updates/UpdateManager.kt index f472ab031..2d90cbcdf 100644 --- a/src/frontends/android/app/src/main/java/org/strongswan/android/logic/updates/UpdateManager.kt +++ b/src/frontends/android/app/src/main/java/org/strongswan/android/logic/updates/UpdateManager.kt @@ -103,7 +103,9 @@ class UpdateManager private constructor(private val context: Context) { companion object { - private val TAG = UpdateManager::class.java.simpleName + /* hardcoded: the runtime class name is obfuscated by R8 in release + * builds, which turned the diag tag into "a:" */ + private const val TAG = "UpdateManager" private const val UPDATE_DIR = "update" private const val APK_NAME = "update.apk" diff --git a/src/frontends/android/app/src/main/jni/libandroidbridge/charonservice.c b/src/frontends/android/app/src/main/jni/libandroidbridge/charonservice.c index c6cd37e08..e9f61d022 100644 --- a/src/frontends/android/app/src/main/jni/libandroidbridge/charonservice.c +++ b/src/frontends/android/app/src/main/jni/libandroidbridge/charonservice.c @@ -393,6 +393,40 @@ failed: androidjni_detach_thread(); } +/** + * Tell Java a direct destination is dead and how long it is being refused + * for (diagnostics; the engine calls this when the cooldown window escalates + * or a failure milestone is crossed). Best effort. + */ +void charonservice_log_dead_destination(const char *dst, uint32_t fails, + uint32_t cooldown_s) +{ + private_charonservice_t *this = (private_charonservice_t*)charonservice; + JNIEnv *env; + jmethodID method_id; + jstring jdst; + + if (!this || !dst) + { + return; + } + + androidjni_attach_thread(&env); + jdst = (*env)->NewStringUTF(env, dst); + method_id = (*env)->GetMethodID(env, android_charonvpnservice_class, + "logDeadDestination", + "(Ljava/lang/String;II)V"); + if (!jdst || !method_id) + { + goto failed; + } + (*env)->CallVoidMethod(env, this->vpn_service, method_id, + jdst, (jint)fails, (jint)cooldown_s); +failed: + androidjni_exception_occurred(env); + androidjni_detach_thread(); +} + /** * Converts the given Java array of byte arrays (byte[][]) to a linked list * of chunk_t objects. diff --git a/src/frontends/android/app/src/main/jni/libandroidbridge/charonservice.h b/src/frontends/android/app/src/main/jni/libandroidbridge/charonservice.h index efbd746b9..fc1c23c8f 100644 --- a/src/frontends/android/app/src/main/jni/libandroidbridge/charonservice.h +++ b/src/frontends/android/app/src/main/jni/libandroidbridge/charonservice.h @@ -190,4 +190,12 @@ void charonservice_log_connection_owner(const struct sockaddr *local, const struct sockaddr *remote, const char *dst); +/** + * Log (via Java) a direct destination whose failure cooldown escalated (the + * window grew) or crossed a failure milestone: surfaces the dead hosts apps + * keep hammering. Best effort. + */ +void charonservice_log_dead_destination(const char *dst, uint32_t fails, + uint32_t cooldown_s); + #endif /** CHARONSERVICE_H_ @}*/ diff --git a/src/frontends/android/app/src/main/jni/libandroidbridge/routing/direct_tcp.c b/src/frontends/android/app/src/main/jni/libandroidbridge/routing/direct_tcp.c index fa8c7fa01..42ea2bab2 100644 --- a/src/frontends/android/app/src/main/jni/libandroidbridge/routing/direct_tcp.c +++ b/src/frontends/android/app/src/main/jni/libandroidbridge/routing/direct_tcp.c @@ -67,15 +67,32 @@ /** * Negative cache: after NEG_FAILS consecutive failed connect cycles a - * destination is refused outright (RST without a socket) for - * NEG_COOLDOWN_MS, so an app hammering a dead host cannot burn sockets, - * JNI protect() roundtrips and log lines. Cleared on success, on a network - * change (reprotect) and after NEG_EXPIRY_MS without failures. + * destination is refused outright (RST without a socket), so an app + * hammering a dead host cannot burn sockets, JNI protect() roundtrips and + * log lines. The refusal window starts at NEG_COOLDOWN_MS and doubles per + * further failed cycle up to NEG_COOLDOWN_MAX_MS - a chronically dead host + * then costs one probe per window instead of a constant 45 s cadence (one + * app was observed opening ~3000 connects/hour to each of two dead hosts, + * waking the radio for every leaked probe). Cleared on success, on a + * network change (reprotect) and after NEG_EXPIRY_MS without failures. */ #define NEG_FAILS 3 #define NEG_COOLDOWN_MS 45000 +#define NEG_COOLDOWN_MAX_MS (5 * 60 * 1000) #define NEG_EXPIRY_MS (10 * 60 * 1000) +/** + * Report the dead destination at every NEG_REPORT_FAILS-th failure even + * after the window has maxed out, so long-running outages stay visible + */ +#define NEG_REPORT_FAILS 100 + +/** + * neg_record_fail() flags + */ +#define NEG_ARMED (1<<0) /* cooldown (re)armed right now */ +#define NEG_ESCALATED (1<<1) /* window grew or milestone crossed */ + /** * Minimum spacing between failure log lines for the same destination */ @@ -92,6 +109,7 @@ typedef struct { bool used; struct sockaddr_storage dest; uint32_t fails; + uint32_t cooldown_ms; uint64_t last_fail, cooldown_until, last_log; } neg_entry_t; @@ -411,11 +429,14 @@ static bool neg_should_log(private_direct_tcp_t *this, const struct sockaddr_sto } /** - * Record a failed connect cycle. Returns TRUE when the destination just - * entered its cooldown (the caller may log/attribute the owner once here). + * Record a failed connect cycle. Returns NEG_ARMED when the destination + * (re)entered its cooldown right now (the caller may log/attribute the owner + * once here), plus NEG_ESCALATED when the window grew or a failure milestone + * was crossed (worth a dead-destination diag line). fails/window report the + * armed state for that line and may be NULL. */ -static bool neg_record_fail(private_direct_tcp_t *this, const struct sockaddr_storage *ss, - uint64_t now) +static int neg_record_fail(private_direct_tcp_t *this, const struct sockaddr_storage *ss, + uint64_t now, uint32_t *fails, uint32_t *window) { neg_entry_t *entry = NULL, *oldest = &this->neg[0]; size_t i; @@ -452,15 +473,32 @@ static bool neg_record_fail(private_direct_tcp_t *this, const struct sockaddr_st else if (now - entry->last_fail > NEG_EXPIRY_MS) { /* silence long enough: start over */ entry->fails = 0; + entry->cooldown_ms = 0; } entry->fails++; entry->last_fail = now; if (entry->fails >= NEG_FAILS && now >= entry->cooldown_until) { - entry->cooldown_until = now + NEG_COOLDOWN_MS; - return TRUE; + uint32_t prev = entry->cooldown_ms; + + entry->cooldown_ms = entry->cooldown_ms ? + min(entry->cooldown_ms * 2, NEG_COOLDOWN_MAX_MS) : NEG_COOLDOWN_MS; + entry->cooldown_until = now + entry->cooldown_ms; + if (fails) + { + *fails = entry->fails; + } + if (window) + { + *window = entry->cooldown_ms; + } + if (entry->cooldown_ms > prev || entry->fails % NEG_REPORT_FAILS == 0) + { + return NEG_ARMED | NEG_ESCALATED; + } + return NEG_ARMED; } - return FALSE; + return 0; } /** @@ -581,7 +619,7 @@ static lwip_err_t on_accept(void *arg, struct tcp_pcb *newpcb, lwip_err_t err) if (fd < 0) { DIRECT_STATS_ADD(this->stats->relay_errors, 1); - if (neg_record_fail(this, &ss, now)) + if (neg_record_fail(this, &ss, now, NULL, NULL) & NEG_ARMED) { charonservice_log_connection_owner((struct sockaddr*)&org, (struct sockaddr*)&ss, dest); @@ -955,13 +993,20 @@ METHOD(direct_tcp_t, process_poll, void, } else { + uint32_t fails = 0, window = 0; + int flags; + format_dest(&relay->dest, dest, sizeof(dest)); do_log = neg_should_log(this, &relay->dest, now); - if (!relay->retried) + if (!relay->retried && err != ECONNREFUSED) { /* one retry on a fresh socket: the first attempt may have * been pinned to a network that vanished mid-roam (its * EHOSTUNREACH can take over a minute to surface, leaving - * the connection hanging) */ + * the connection hanging). Skipped for ECONNREFUSED - + * the peer actively refused on the network this socket is + * bound to, so a second socket there hits the same + * refusal, and a mid-roam socket is already aborted by + * reprotect() before its error can surface here. */ int nfd; if (do_log) @@ -981,7 +1026,8 @@ METHOD(direct_tcp_t, process_poll, void, err = errno; } DIRECT_STATS_ADD(this->stats->relay_errors, 1); - if (neg_record_fail(this, &relay->dest, now)) + flags = neg_record_fail(this, &relay->dest, now, &fails, &window); + if (flags & NEG_ARMED) { /* destination just entered its cooldown: name the app * that owns the connection while its socket still * exists (relay_destroy aborts the pcb below) */ @@ -989,6 +1035,12 @@ METHOD(direct_tcp_t, process_poll, void, (struct sockaddr*)&relay->origin, (struct sockaddr*)&relay->dest, dest); } + if (flags & NEG_ESCALATED) + { /* the cooldown window just grew (or a milestone was + * crossed): make the dead host visible in diag.log */ + charonservice_log_dead_destination(dest, fails, + (window + 999) / 1000); + } if (do_log) { DBG1(DBG_NET, "direct tcp: connect failed dst=%s: %s",