Skip to content

Commit 33523bd

Browse files
committed
fix: address review on MasterDnsVPN sidecar
- buildMasterDnsVpnConfig: wrap advancedJson parse in try-catch (malformed override must not crash config build / VPN start; logged + ignored). - Default encryption method 1 (XOR) -> 0 (None) in Bean/DataStore/XML: avoid false security (XOR with an empty key); user picks a cipher + key explicitly. - Fix KDoc to match the actual launch flags (-json <file>, not -json-base64). - masterdnsvpn.sh: validate Go >= 1.25 before building. - Simplify serverAddress placeholder in the settings init.
1 parent e40b538 commit 33523bd

6 files changed

Lines changed: 30 additions & 9 deletions

File tree

app/src/main/java/io/nekohasekai/sagernet/database/DataStore.kt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -258,7 +258,7 @@ object DataStore : OnPreferenceDataStoreChangeListener {
258258

259259
// MasterDnsVPN
260260
var mdvDomains by profileCacheStore.string(Key.MDV_DOMAINS)
261-
var mdvEncryptionMethod by profileCacheStore.stringToInt(Key.MDV_ENCRYPTION_METHOD) { 1 }
261+
var mdvEncryptionMethod by profileCacheStore.stringToInt(Key.MDV_ENCRYPTION_METHOD) { 0 }
262262
var mdvEncryptionKey by profileCacheStore.string(Key.MDV_ENCRYPTION_KEY)
263263
var mdvResolvers by profileCacheStore.string(Key.MDV_RESOLVERS)
264264
var mdvBalancingStrategy by profileCacheStore.stringToInt(Key.MDV_BALANCING_STRATEGY) { 3 }

app/src/main/java/io/nekohasekai/sagernet/fmt/masterdnsvpn/MasterDnsVpnBean.java

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,9 @@ public class MasterDnsVpnBean extends AbstractBean {
7575
public void initializeDefaultValues() {
7676
super.initializeDefaultValues();
7777
if (domains == null) domains = "";
78-
if (dataEncryptionMethod == null) dataEncryptionMethod = 1;
78+
// Default to None: encryption must match the server and needs a key, so don't
79+
// imply security with XOR + an empty key. The user picks a cipher + key explicitly.
80+
if (dataEncryptionMethod == null) dataEncryptionMethod = 0;
7981
if (encryptionKey == null) encryptionKey = "";
8082
if (resolvers == null) resolvers = "8.8.8.8\n1.1.1.1";
8183
if (resolverBalancingStrategy == null) resolverBalancingStrategy = 3;

app/src/main/java/io/nekohasekai/sagernet/fmt/masterdnsvpn/MasterDnsVpnFmt.kt

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -10,14 +10,15 @@
1010
package io.nekohasekai.sagernet.fmt.masterdnsvpn
1111

1212
import io.nekohasekai.sagernet.fmt.LOCALHOST
13+
import io.nekohasekai.sagernet.ktx.Logs
1314
import io.nekohasekai.sagernet.ktx.toStringPretty
1415
import org.json.JSONArray
1516
import org.json.JSONObject
1617

1718
/**
1819
* Builds the JSON config for the bundled MasterDnsVPN client (sidecar).
1920
*
20-
* The client is launched as `libmasterdnsvpn.so -json-base64 <b64> -resolvers <file>`.
21+
* The client is launched as `libmasterdnsvpn.so -json <cfg.json> -resolvers <file>`.
2122
* Resolvers are written to a separate file (the client always loads resolvers from a
2223
* file); see [resolverLines]. The config uses the client's uppercase config keys.
2324
*
@@ -72,9 +73,14 @@ fun MasterDnsVpnBean.buildMasterDnsVpnConfig(port: Int, protectPath: String): St
7273

7374
// Merge the advanced JSON override last so it can set/replace any key.
7475
if (advancedJson.isNotBlank()) {
75-
val extra = JSONObject(advancedJson)
76-
for (key in extra.keys()) {
77-
cfg.put(key, extra.get(key))
76+
// Malformed override JSON must not crash config generation / VPN start.
77+
try {
78+
val extra = JSONObject(advancedJson)
79+
for (key in extra.keys()) {
80+
cfg.put(key, extra.get(key))
81+
}
82+
} catch (e: Exception) {
83+
Logs.w("MasterDnsVPN: ignoring invalid advanced JSON override: ${e.message}")
7884
}
7985
}
8086

app/src/main/java/io/nekohasekai/sagernet/ui/profile/MasterDnsVpnSettingsActivity.kt

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -25,8 +25,8 @@ class MasterDnsVpnSettingsActivity : ProfileSettingsActivity<MasterDnsVpnBean>()
2525

2626
override fun MasterDnsVpnBean.init() {
2727
DataStore.profileName = name
28-
// serverAddress/Port are unused by this protocol but kept for the base editor.
29-
DataStore.serverAddress = if (serverAddress.isNullOrBlank()) "masterdnsvpn" else serverAddress
28+
// serverAddress/Port are unused by this protocol; the editor uses the MDV_* fields.
29+
DataStore.serverAddress = "masterdnsvpn"
3030
DataStore.mdvDomains = domains
3131
DataStore.mdvEncryptionMethod = dataEncryptionMethod
3232
DataStore.mdvEncryptionKey = encryptionKey

app/src/main/res/xml/masterdnsvpn_preferences.xml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@
1414
app:title="@string/mdv_domains"
1515
app:useSimpleSummaryProvider="true" />
1616
<moe.matsuri.nb4a.ui.SimpleMenuPreference
17-
app:defaultValue="1"
17+
app:defaultValue="0"
1818
app:entries="@array/mdv_encryption_method"
1919
app:entryValues="@array/mdv_encryption_method_value"
2020
app:icon="@drawable/ic_settings_password"

buildScript/lib/masterdnsvpn.sh

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,19 @@ fi
5555

5656
pushd "$WORK" >/dev/null
5757

58+
# MasterDnsVPN requires Go 1.25+. Fail fast with a clear message otherwise.
59+
if ! command -v go >/dev/null 2>&1; then
60+
echo "Error: go not found on PATH (MasterDnsVPN needs Go 1.25+)." >&2
61+
exit 1
62+
fi
63+
GO_VER="$(go env GOVERSION 2>/dev/null | sed 's/^go//')"
64+
GO_MAJOR="${GO_VER%%.*}"
65+
GO_REST="${GO_VER#*.}"; GO_MINOR="${GO_REST%%.*}"
66+
if [ "${GO_MAJOR:-0}" -lt 1 ] || { [ "${GO_MAJOR:-0}" -eq 1 ] && [ "${GO_MINOR:-0}" -lt 25 ]; }; then
67+
echo "Error: Go $GO_VER is too old; MasterDnsVPN needs Go 1.25+." >&2
68+
exit 1
69+
fi
70+
5871
build_abi() {
5972
local abi="$1" goarch="$2" cc="$3" goarm="$4"
6073
echo ">> building libmasterdnsvpn.so for $abi"

0 commit comments

Comments
 (0)