71 lines
5.1 KiB
Markdown
71 lines
5.1 KiB
Markdown
# Conventions & load-bearing invariants
|
|
|
|
## Code style
|
|
|
|
- Match the surrounding code: dense, explanatory comments that say **why** (the data path is
|
|
full of non-obvious protocol/platform reasoning — keep that). Kotlin idioms, coroutines with
|
|
explicit dispatchers, `runCatching` at boundaries.
|
|
- Keep platform glue thin and honest. Comments must not over-claim (a probe *detects* a bad
|
|
Shadowsocks key but usually can't *name* it — say exactly that).
|
|
- No new dependencies without a strong reason — a deliberate property of this app is a tiny
|
|
dependency surface (no crypto lib; AEAD is on platform JCA).
|
|
|
|
## Commits
|
|
|
|
- **No `Co-Authored-By` trailer.** Keep each commit message **≤ 30 words.**
|
|
- Split work into logical, per-subsystem commits.
|
|
|
|
## Load-bearing invariants — DO NOT "fix" these without reading
|
|
|
|
Each of these looks like a bug or an easy cleanup and has already cost a regression. Verify
|
|
against the code and the harness before touching.
|
|
|
|
1. **`protect()` returning `false` is benign.** Our own sockets are already excluded by the
|
|
app filter, so `protect()` may return false with no harm. Log, don't throw. (Throwing here
|
|
once broke *every* upstream connection.)
|
|
2. **IPv6 is routed into the tun only when `settings.ipv6`.** hev does not drop unconfigured-
|
|
family packets — it forwards a v6 CONNECT to a v6-incapable proxy (`rep=5`), stalling
|
|
Happy-Eyeballs apps. With v6 absent, Android adds `::/0 unreachable`, so v6 is blackholed
|
|
(no leak). `dnsServerFor()` coerces a v6-literal DNS to `1.1.1.1` while v6 is off — keep it.
|
|
3. **SOCKS5 UDP socket is `connect()`-ed to the relay.** RFC 1928 §7 anti-spoof = the kernel
|
|
`(IP, port)` filter; that's also the ephemeral-port DNS-spoof defence. Do **not** replace it
|
|
with a host-only userspace filter (weaker — drops the port check). Matches sing-box/canon.
|
|
4. **`isUnroutableRelay()` rewrite is intentional and broader than canon.** A SOCKS5 UDP
|
|
ASSOCIATE BND.ADDR that is wildcard/loopback/private/CGNAT/ULA is rewritten to the proxy
|
|
host. This fixes real servers (e.g. 3proxy returning `10.x`) that would otherwise black-hole
|
|
all UDP/DNS. Narrowing it to wildcard-only would reintroduce a device-confirmed bug.
|
|
5. **Shadowsocks reader EOF semantics.** EOF on a *frame boundary* = clean end of stream
|
|
(return -1). EOF *inside* a frame = truncation = error. And a zero-length chunk is **valid**
|
|
(canon emits/accepts empty keep-alive frames) — reject only `> MAX_PAYLOAD`.
|
|
6. **The local SOCKS5 bridge requires random per-session auth.** It's the boundary that stops a
|
|
non-whitelisted local app from using the loopback proxy. Don't make it no-auth.
|
|
7. **ChaCha20 cipher name is `ChaCha20/Poly1305/NoPadding`** (Conscrypt registers the slash name
|
|
since API 28; the hyphenated alias only exists on newer Mainline). Not a typo.
|
|
8. **`ConfigStore` never wipes on a decode failure.** It snapshots last-known-good before each
|
|
write and recovers from it; only a genuine `SerializationException`/`IllegalArgumentException`
|
|
counts as corruption (let `Error`/OOM propagate so a transient failure doesn't wipe servers).
|
|
9. **MTU 8500 is the baseline** (not a tuned value); the engine config and tun must agree.
|
|
10. **Domain routing recovers domains via hev mapped-DNS (fake-IP); the DIRECT path must bypass
|
|
it.** When `routedSites` is non-empty, `buildEngineConfig()` enables a `mapdns:` block and the
|
|
tun DNS is pointed at the synthetic resolver `198.18.0.2`. The fake-IP pool is `198.19.0.0/16`
|
|
— it clears the `198.18.0.1/32` tun address and is deliberately NOT in the `isUnroutableRelay()`
|
|
CGNAT/private set (those fake IPs become domains before reaching any upstream, so that relay
|
|
path is never involved). `DirectDialer` resolves on the **underlying non-VPN network** and
|
|
`protect()`s the socket: resolving via the default resolver would return a fake IP and loop the
|
|
"direct" flow back into the tun. The direct dialer also requires the underlying network to carry
|
|
`NET_CAPABILITY_INTERNET` — a device can expose an IMS/MMTEL cellular network (NOT_VPN, but no
|
|
INTERNET) that can't resolve, which silently broke every direct dial until filtered out
|
|
(device-confirmed). Mapped-DNS is IPv4-only, so v6 is force-suppressed while sites are active —
|
|
don't "restore" it. Keep the fake-IP cache sized to the whole pool (`MAPDNS_CACHE_SIZE` ==
|
|
`~netmask`): the LRU evicts by the *cache* limit, not the pool, and an evicted-but-still-referenced
|
|
fake IP reverse-maps to nothing and dead-ends.
|
|
|
|
## The "no universal core" decision
|
|
|
|
Replacing the Kotlin backend (SS crypto + local SOCKS5 + upstream clients) with sing-box /
|
|
sing-tun / mihomo / libbox was evaluated and **rejected**: those are **GPLv3** (live
|
|
enforcement) and would force open-sourcing this closed app, and any Go/Rust runtime breaks the
|
|
small-APK goal. `hev-socks5-tunnel` is MIT and stays. The only defensible strategic swap is
|
|
`shadowsocks-rust` (MIT) for the SS crypto surface *only*, and only if Kotlin-SS upkeep ever
|
|
dominates — soak-test behind hev before deleting anything. Our SS crypto is verified correct
|
|
against shadowsocks-rust and sing-shadowsocks.
|