diff --git a/.github/workflows/codeql.yml b/.github/workflows/codeql.yml index 9c3af93f787..f818cc5ad65 100644 --- a/.github/workflows/codeql.yml +++ b/.github/workflows/codeql.yml @@ -34,6 +34,9 @@ jobs: - name: Checkout repository uses: actions/checkout@v5 + - name: Validate Gradle wrapper + uses: gradle/actions/wrapper-validation@v6 + # Initializes the CodeQL tools for scanning. - name: Initialize CodeQL uses: github/codeql-action/init@v4 diff --git a/.github/workflows/integration-test-single-node.yml b/.github/workflows/integration-test-single-node.yml index 3c56843c799..3ce0cc248a3 100644 --- a/.github/workflows/integration-test-single-node.yml +++ b/.github/workflows/integration-test-single-node.yml @@ -25,6 +25,9 @@ jobs: - name: Checkout java-tron uses: actions/checkout@v5 + - name: Validate Gradle wrapper + uses: gradle/actions/wrapper-validation@v6 + - name: Set up JDK 8 uses: actions/setup-java@v5 with: diff --git a/.github/workflows/pr-build.yml b/.github/workflows/pr-build.yml index f35538c0961..bd38c3d8cd8 100644 --- a/.github/workflows/pr-build.yml +++ b/.github/workflows/pr-build.yml @@ -36,6 +36,9 @@ jobs: steps: - uses: actions/checkout@v5 + - name: Validate Gradle wrapper + uses: gradle/actions/wrapper-validation@v6 + - name: Set up JDK ${{ matrix.java }} uses: actions/setup-java@v5 with: @@ -72,6 +75,9 @@ jobs: steps: - uses: actions/checkout@v5 + - name: Validate Gradle wrapper + uses: gradle/actions/wrapper-validation@v6 + - name: Set up JDK 17 uses: actions/setup-java@v5 with: @@ -126,6 +132,9 @@ jobs: - name: Checkout code uses: actions/checkout@v5 + - name: Validate Gradle wrapper + uses: gradle/actions/wrapper-validation@v6 + - name: Check Java version run: java -version @@ -183,6 +192,9 @@ jobs: - name: Checkout code uses: actions/checkout@v5 + - name: Validate Gradle wrapper + uses: gradle/actions/wrapper-validation@v6 + - name: Check Java version run: java -version @@ -249,6 +261,9 @@ jobs: with: ref: ${{ github.event.pull_request.base.sha }} + - name: Validate Gradle wrapper + uses: gradle/actions/wrapper-validation@v6 + - name: Cache Gradle packages uses: actions/cache@v5 with: diff --git a/.github/workflows/pr-check.yml b/.github/workflows/pr-check.yml index 506a823a4f7..5ec401916d3 100644 --- a/.github/workflows/pr-check.yml +++ b/.github/workflows/pr-check.yml @@ -103,6 +103,9 @@ jobs: steps: - uses: actions/checkout@v5 + - name: Validate Gradle wrapper + uses: gradle/actions/wrapper-validation@v6 + - name: Set up Python uses: actions/setup-python@v5 with: diff --git a/.github/workflows/release-build.yml b/.github/workflows/release-build.yml index d67e2418bf6..c98747dadc8 100644 --- a/.github/workflows/release-build.yml +++ b/.github/workflows/release-build.yml @@ -54,6 +54,9 @@ jobs: with: ref: ${{ github.sha }} + - name: Validate Gradle wrapper + uses: gradle/actions/wrapper-validation@v6 + - name: Cache Gradle packages uses: actions/cache@v5 with: @@ -121,6 +124,9 @@ jobs: with: ref: ${{ github.sha }} + - name: Validate Gradle wrapper + uses: gradle/actions/wrapper-validation@v6 + - name: Set up Temurin 17 uses: actions/setup-java@v5 with: diff --git a/README.md b/README.md index edf99c4df92..63bce4963f3 100644 --- a/README.md +++ b/README.md @@ -161,6 +161,9 @@ Fill in the private key of your SR account into the `localwitness` list in the c ] ``` + +> **Note**: Plain private keys in `localwitness` (and the `-p/--private-key` CLI flag) are deprecated and insecure — they are readable by anyone with access to the config file, process list, or shell history. Use the encrypted [`localwitnesskeystore`](https://tronprotocol.github.io/documentation-en/using_javatron/toolkit/) instead. + Check [Starting a Block Production Node](https://tronprotocol.github.io/documentation-en/using_javatron/installing_javatron/#starting-a-block-production-node) for more details. You could also test the process by connecting to a testnet or setting up a private network. diff --git a/build.gradle b/build.gradle index 65e72c0fb73..00d7db1405a 100644 --- a/build.gradle +++ b/build.gradle @@ -72,7 +72,6 @@ subprojects { buildscript { repositories { mavenCentral() - maven { url 'https://jitpack.io' } maven { url "https://plugins.gradle.org/m2/" } @@ -84,10 +83,9 @@ subprojects { } repositories { - mavenLocal() mavenCentral() maven { url 'https://repo.spring.io/plugins-release' } - maven { url 'https://jitpack.io' } + mavenLocal() } dependencies { diff --git a/common/src/main/java/org/tron/core/config/args/NodeConfig.java b/common/src/main/java/org/tron/core/config/args/NodeConfig.java index 91945b5a73b..0fc26a645f7 100644 --- a/common/src/main/java/org/tron/core/config/args/NodeConfig.java +++ b/common/src/main/java/org/tron/core/config/args/NodeConfig.java @@ -208,6 +208,9 @@ public static class HttpConfig { public static class RpcConfig { public static final int DEFAULT_MAX_CONCURRENT_CALLS_PER_CONNECTION = 100; + // Secure built-in default for gRPC connection lifetime (idle & max age). + // Applied by postProcess() when the configured value is 0 (the default). + public static final long DEFAULT_MAX_CONNECTION_LIFETIME_IN_MILLIS = 60_000L; private boolean enable = true; private int port = 50051; @@ -373,11 +376,39 @@ private void postProcess() { rpc.maxConcurrentCallsPerConnection = RpcConfig.DEFAULT_MAX_CONCURRENT_CALLS_PER_CONNECTION; } + // node.rpc.maxRstStream and node.rpc.secondsPerWindow only take effect + // together (RpcService applies RST limiting only when both are > 0). A + // half-configured pair would silently disable RST_STREAM flood + // protection — warn loudly so the misconfiguration is visible. + if (rpc.maxRstStream < 0 || rpc.secondsPerWindow < 0) { + throw new TronError("node.rpc.maxRstStream and node.rpc.secondsPerWindow " + + "must be non-negative, got: maxRstStream=" + rpc.maxRstStream + + ", secondsPerWindow=" + rpc.secondsPerWindow, PARAMETER_INIT); + } + if ((rpc.maxRstStream > 0 && rpc.secondsPerWindow <= 0) + || (rpc.maxRstStream <= 0 && rpc.secondsPerWindow > 0)) { + logger.warn("node.rpc.maxRstStream and node.rpc.secondsPerWindow only " + + "take effect together: exactly one of them is set while the other " + + "is 0, so RST_STREAM flood protection is DISABLED. Set both to " + + "positive values to enable it, or leave both at 0 (got: " + + "maxRstStream={}, secondsPerWindow={})", + rpc.maxRstStream, rpc.secondsPerWindow); + } + if (rpc.maxConnectionIdleInMillis < 0 || rpc.maxConnectionAgeInMillis < 0) { + throw new TronError("node.rpc.maxConnectionIdleInMillis and node.rpc.maxConnectionAgeInMillis " + + "must be non-negative, got: maxConnectionIdleInMillis=" + rpc.maxConnectionIdleInMillis + + ", maxConnectionAgeInMillis=" + rpc.maxConnectionAgeInMillis, PARAMETER_INIT); + } + // 0 (the default) means "use the secure built-in default" (60s), NOT + // unlimited: the old 0 -> Long.MAX_VALUE conversion silently turned an + // unset or explicitly-zero value into an unbounded connection lifetime. + // Negative values are rejected outright. Operators who want a longer + // lifetime must set an explicit positive value. if (rpc.maxConnectionIdleInMillis == 0) { - rpc.maxConnectionIdleInMillis = Long.MAX_VALUE; + rpc.maxConnectionIdleInMillis = RpcConfig.DEFAULT_MAX_CONNECTION_LIFETIME_IN_MILLIS; } if (rpc.maxConnectionAgeInMillis == 0) { - rpc.maxConnectionAgeInMillis = Long.MAX_VALUE; + rpc.maxConnectionAgeInMillis = RpcConfig.DEFAULT_MAX_CONNECTION_LIFETIME_IN_MILLIS; } // validateSignThreadNum: 0 = auto-detect diff --git a/common/src/main/resources/reference.conf b/common/src/main/resources/reference.conf index d8c483d932a..019f1d22e86 100644 --- a/common/src/main/resources/reference.conf +++ b/common/src/main/resources/reference.conf @@ -295,10 +295,10 @@ node { # HTTP/2 flow control window (bytes), default 1MB flowControlWindow = 1048576 - # Connection idle timeout (ms). Connections idle longer than this are gracefully terminated. 0 = no limit + # Connection idle timeout (ms). Connections idle longer than this are gracefully terminated. 0 = secure default (60 s) maxConnectionIdleInMillis = 0 - # Connection max age (ms). 0 = no limit + # Connection max age (ms). 0 = secure default (60 s) maxConnectionAgeInMillis = 0 # Maximum gRPC message size in bytes (default 4194304, ~4MB). diff --git a/common/src/test/java/org/tron/core/config/args/NodeConfigTest.java b/common/src/test/java/org/tron/core/config/args/NodeConfigTest.java index bcb8b09dd7a..3e838feb43f 100644 --- a/common/src/test/java/org/tron/core/config/args/NodeConfigTest.java +++ b/common/src/test/java/org/tron/core/config/args/NodeConfigTest.java @@ -107,8 +107,10 @@ public void testRpcDefaultsFromReference() { assertEquals(NodeConfig.RpcConfig.DEFAULT_MAX_CONCURRENT_CALLS_PER_CONNECTION, rpc.getMaxConcurrentCallsPerConnection()); assertEquals(1048576, rpc.getFlowControlWindow()); - assertEquals(9223372036854775807L, rpc.getMaxConnectionIdleInMillis()); - assertEquals(9223372036854775807L, rpc.getMaxConnectionAgeInMillis()); + // reference.conf keeps 0 (the default marker); postProcess converts 0 to + // the secure built-in 60s default, so the effective default is 60s. + assertEquals(60000L, rpc.getMaxConnectionIdleInMillis()); + assertEquals(60000L, rpc.getMaxConnectionAgeInMillis()); assertEquals(4194304, rpc.getMaxMessageSize()); assertEquals(8192, rpc.getMaxHeaderListSize()); assertEquals(1, rpc.getMinEffectiveConnection()); @@ -146,6 +148,93 @@ public void testRpcNegativeConcurrentCallsRejected() { "node.rpc.maxConcurrentCallsPerConnection must be non-negative, got: -1")); } + @Test + public void testRpcNegativeIdleAndAgeRejected() { + Config config = withRef( + "node { rpc { maxConnectionIdleInMillis = -1, maxConnectionAgeInMillis = 60000 } }"); + + TronError exception = assertThrows(TronError.class, + () -> NodeConfig.fromConfig(config)); + + assertTrue(exception.getMessage().contains( + "node.rpc.maxConnectionIdleInMillis and node.rpc.maxConnectionAgeInMillis " + + "must be non-negative, got: maxConnectionIdleInMillis=-1")); + + Config config2 = withRef( + "node { rpc { maxConnectionIdleInMillis = 60000, maxConnectionAgeInMillis = -5 } }"); + + exception = assertThrows(TronError.class, + () -> NodeConfig.fromConfig(config2)); + + assertTrue(exception.getMessage().contains( + "maxConnectionAgeInMillis=-5")); + } + + @Test + public void testRpcExplicitZeroIdleAndAgeFallsBackToSecureDefault() { + // Explicit 0 no longer means unlimited: postProcess converts 0 to the + // secure built-in 60s default. Operators wanting a longer lifetime must + // set an explicit positive value. + Config config = withRef( + "node { rpc { maxConnectionIdleInMillis = 0, maxConnectionAgeInMillis = 0 } }"); + NodeConfig nc = NodeConfig.fromConfig(config); + assertEquals(NodeConfig.RpcConfig.DEFAULT_MAX_CONNECTION_LIFETIME_IN_MILLIS, + nc.getRpc().getMaxConnectionIdleInMillis()); + assertEquals(NodeConfig.RpcConfig.DEFAULT_MAX_CONNECTION_LIFETIME_IN_MILLIS, + nc.getRpc().getMaxConnectionAgeInMillis()); + } + + // ----- maxRstStream / secondsPerWindow: must be configured together ----- + + @Test + public void testRpcRstPairBothZeroOk() { + // Default: feature off. Must not throw. + NodeConfig nc = NodeConfig.fromConfig(withRef()); + assertEquals(0, nc.getRpc().getMaxRstStream()); + assertEquals(0, nc.getRpc().getSecondsPerWindow()); + } + + @Test + public void testRpcRstPairBothPositiveOk() { + Config config = withRef( + "node { rpc { maxRstStream = 1000, secondsPerWindow = 60 } }"); + NodeConfig nc = NodeConfig.fromConfig(config); + assertEquals(1000, nc.getRpc().getMaxRstStream()); + assertEquals(60, nc.getRpc().getSecondsPerWindow()); + } + + private static TronError rstError(String hocon) { + Config config = withRef("node { rpc { " + hocon + " } }"); + return assertThrows(TronError.class, () -> NodeConfig.fromConfig(config)); + } + + @Test + public void testRpcRstPairHalfConfiguredWarnsButStarts() { + // Only maxRstStream set: RST flood protection stays disabled, startup + // continues with a warning. + Config config = withRef("node { rpc { maxRstStream = 100 } }"); + NodeConfig nc = NodeConfig.fromConfig(config); + assertEquals(100, nc.getRpc().getMaxRstStream()); + assertEquals(0, nc.getRpc().getSecondsPerWindow()); + + // Only secondsPerWindow set: same trap, same warn-and-continue. + config = withRef("node { rpc { secondsPerWindow = 60 } }"); + nc = NodeConfig.fromConfig(config); + assertEquals(0, nc.getRpc().getMaxRstStream()); + assertEquals(60, nc.getRpc().getSecondsPerWindow()); + } + + @Test + public void testRpcRstNegativeValuesRejected() { + TronError exception = rstError("maxRstStream = -1, secondsPerWindow = 60"); + assertTrue(exception.getMessage().contains( + "must be non-negative, got: maxRstStream=-1")); + + exception = rstError("maxRstStream = 100, secondsPerWindow = -5"); + assertTrue(exception.getMessage().contains( + "must be non-negative, got: maxRstStream=100, secondsPerWindow=-5")); + } + @Test public void testRpcUserOverrideExplicitValues() { Config config = withRef( diff --git a/crypto/src/main/java/org/tron/keystore/Wallet.java b/crypto/src/main/java/org/tron/keystore/Wallet.java index d63525b1e4d..7c05673f77b 100644 --- a/crypto/src/main/java/org/tron/keystore/Wallet.java +++ b/crypto/src/main/java/org/tron/keystore/Wallet.java @@ -63,9 +63,32 @@ public class Wallet { private static final int CURRENT_VERSION = 3; private static final String CIPHER = "aes-128-ctr"; + // Upper bounds for untrusted keystore KDF cost parameters. A hostile + // keystore file may declare arbitrary KDF strengths: extreme scrypt + // parameters make Bouncy Castle either allocate unbounded memory + // (n*r*128 bytes — e.g. n=2^22, r=8 requests 4 GiB and dies with OOM) + // or compute for hours, and out-of-range values can crash the JVM with + // NegativeArraySizeException / ArithmeticException inside SCrypt.generate. + // All values here are rejected by validationError() before any KDF runs. + private static final int SCRYPT_N_MIN = 1 << 12; + private static final int SCRYPT_N_MAX = 1 << 20; + private static final int SCRYPT_R_MAX = 8; + private static final int SCRYPT_P_MAX = 8; + private static final long SCRYPT_MEMORY_MAX_BYTES = 1L << 30; // 128 * r * n cap = 1 GiB + private static final int KDF_DKLEN_MAX = 128; + private static final int PBKDF2_C_MAX = 1 << 20; + public static WalletFile create(String password, SignInterface sign, int n, int p) throws CipherException { + // Keep create() symmetric with validate()/decrypt(): reject KDF cost + // parameters that the decryption path would refuse, so create() can + // never produce a wallet file that only this class rejects. + String paramError = scryptKdfParamsError(DKLEN, n, R, p); + if (paramError != null) { + throw new CipherException(paramError); + } + byte[] salt = generateRandomBytes(32); byte[] derivedKey = generateDerivedScryptKey(password.getBytes(UTF_8), salt, n, R, p, DKLEN); @@ -194,7 +217,16 @@ public static SignInterface decrypt(String password, WalletFile walletFile, int p = scryptKdfParams.getP(); int r = scryptKdfParams.getR(); byte[] salt = ByteArray.fromHexString(scryptKdfParams.getSalt()); - derivedKey = generateDerivedScryptKey(password.getBytes(UTF_8), salt, n, r, p, dklen); + try { + derivedKey = generateDerivedScryptKey(password.getBytes(UTF_8), salt, n, r, p, dklen); + } catch (RuntimeException e) { + // Defense-in-depth: any residual KDF engine failure (e.g. an + // IllegalArgumentException from Bouncy Castle) must surface as a + // clean CipherException, never as a raw RuntimeException stack. + // Errors (OutOfMemoryError etc.) are deliberately not caught — + // they remain fatal. + throw new CipherException("Scrypt key derivation failed", e); + } } else if (kdfParams instanceof WalletFile.Aes128CtrKdfParams) { WalletFile.Aes128CtrKdfParams aes128CtrKdfParams = (WalletFile.Aes128CtrKdfParams) crypto.getKdfparams(); @@ -202,7 +234,11 @@ public static SignInterface decrypt(String password, WalletFile walletFile, String prf = aes128CtrKdfParams.getPrf(); byte[] salt = ByteArray.fromHexString(aes128CtrKdfParams.getSalt()); - derivedKey = generateAes128CtrDerivedKey(password.getBytes(UTF_8), salt, c, prf); + try { + derivedKey = generateAes128CtrDerivedKey(password.getBytes(UTF_8), salt, c, prf); + } catch (RuntimeException e) { + throw new CipherException("Pbkdf2 key derivation failed", e); + } } else { throw new CipherException("Unable to deserialize params: " + crypto.getKdf()); } @@ -237,11 +273,15 @@ public static SignInterface decrypt(String password, WalletFile walletFile, /** * Returns a description of the first schema violation found in * {@code walletFile}, or {@code null} if the file matches the supported - * V3 keystore shape (current version, known cipher, known KDF). + * V3 keystore shape (current version, known cipher, known KDF, and KDF + * cost parameters within safe bounds). * - *

Shared by {@link #validate(WalletFile)} (which throws the message) - * and {@link #isValidKeystoreFile(WalletFile)} (which returns boolean - * for discovery-style filtering). + *

KDF cost bounds are enforced here — the single chokepoint shared by + * {@link #validate(WalletFile)} (which throws the message) and + * {@link #isValidKeystoreFile(WalletFile)} (which returns boolean for + * discovery-style filtering) — so untrusted keystore files can never + * drive the KDF into unbounded memory allocation, multi-hour CPU + * exhaustion, or engine-level crashes before {@link #decrypt} runs. */ private static String validationError(WalletFile walletFile) { if (walletFile.getVersion() != CURRENT_VERSION) { @@ -259,6 +299,58 @@ private static String validationError(WalletFile walletFile) { if (kdf == null || (!kdf.equals(PBKDF2) && !kdf.equals(SCRYPT))) { return "KDF type is not supported"; } + WalletFile.KdfParams kdfParams = crypto.getKdfparams(); + if (kdfParams == null) { + // Structural stubs (e.g. tooling fixtures) may omit kdfparams entirely; + // there are no cost parameters to bound. Real keystores always carry them. + return null; + } + if (SCRYPT.equals(kdf)) { + if (!(kdfParams instanceof WalletFile.ScryptKdfParams)) { + return "Scrypt KDF params are missing or malformed"; + } + WalletFile.ScryptKdfParams params = (WalletFile.ScryptKdfParams) kdfParams; + return scryptKdfParamsError(params.getDklen(), params.getN(), params.getR(), params.getP()); + } + if (!(kdfParams instanceof WalletFile.Aes128CtrKdfParams)) { + return "Pbkdf2 KDF params are missing or malformed"; + } + WalletFile.Aes128CtrKdfParams params = (WalletFile.Aes128CtrKdfParams) kdfParams; + return pbkdf2KdfParamsError(params.getDklen(), params.getC()); + } + + private static String scryptKdfParamsError(int dklen, int n, int r, int p) { + // dklen must yield at least 32 bytes: generateMac copies derivedKey[16..32) + // and AES-128 needs bytes [0..16); smaller keys throw raw ArrayIndexOutOfBoundsException. + if (dklen < 32 || dklen > KDF_DKLEN_MAX) { + return "Scrypt dklen is out of range [32, " + KDF_DKLEN_MAX + "]: " + dklen; + } + if (n < SCRYPT_N_MIN || n > SCRYPT_N_MAX || (n & (n - 1)) != 0) { + return "Scrypt n must be a power of 2 in [" + SCRYPT_N_MIN + ", " + SCRYPT_N_MAX + + "]: " + n; + } + if (r < 1 || r > SCRYPT_R_MAX) { + return "Scrypt r is out of range [1, " + SCRYPT_R_MAX + "]: " + r; + } + if (p < 1 || p > SCRYPT_P_MAX) { + return "Scrypt p is out of range [1, " + SCRYPT_P_MAX + "]: " + p; + } + // Compute in long arithmetic: hostile r * n must not overflow int. + long memoryBytes = 128L * r * n; + if (memoryBytes > SCRYPT_MEMORY_MAX_BYTES) { + return "Scrypt memory requirement 128*r*n=" + memoryBytes + + " exceeds the " + SCRYPT_MEMORY_MAX_BYTES + " byte limit"; + } + return null; + } + + private static String pbkdf2KdfParamsError(int dklen, int c) { + if (dklen < 1 || dklen > KDF_DKLEN_MAX) { + return "Pbkdf2 dklen is out of range [1, " + KDF_DKLEN_MAX + "]: " + dklen; + } + if (c < 1 || c > PBKDF2_C_MAX) { + return "Pbkdf2 iteration count c is out of range [1, " + PBKDF2_C_MAX + "]: " + c; + } return null; } diff --git a/crypto/src/main/java/org/tron/keystore/WalletUtils.java b/crypto/src/main/java/org/tron/keystore/WalletUtils.java index 2ce100823d9..00aed572be6 100644 --- a/crypto/src/main/java/org/tron/keystore/WalletUtils.java +++ b/crypto/src/main/java/org/tron/keystore/WalletUtils.java @@ -19,12 +19,14 @@ import java.time.format.DateTimeFormatter; import java.util.Collections; import java.util.EnumSet; +import java.util.NoSuchElementException; import java.util.Scanner; import java.util.Set; import lombok.extern.slf4j.Slf4j; import org.apache.commons.lang3.StringUtils; import org.tron.common.crypto.SignInterface; import org.tron.core.exception.CipherException; +import org.tron.core.exception.TronError; /** * Utility functions for working with Wallet files. @@ -233,6 +235,14 @@ public static String inputPassword() { while (true) { if (cons != null) { char[] pwd = cons.readPassword("password: "); + // Console EOF (e.g. Ctrl+D) returns null; fail fast instead of + // crashing with a raw NullPointerException. + if (pwd == null) { + throw new TronError( + "password input closed (EOF) while reading from the console; " + + "restart with --password or provide an interactive TTY", + TronError.ErrCode.WITNESS_KEYSTORE_LOAD); + } password = String.valueOf(pwd); } else { // Preserve the full password including embedded whitespace. @@ -241,7 +251,18 @@ public static String inputPassword() { // staple" to "correct" when piped via stdin (e.g. echo ... | java). // stripPasswordLine only removes the UTF-8 BOM and trailing line // terminators — internal whitespace is part of the password. - password = stripPasswordLine(in.nextLine()); + String line; + try { + line = in.nextLine(); + } catch (NoSuchElementException e) { + // Piped stdin reached EOF before a password line was available; + // fail fast instead of leaking a raw NoSuchElementException. + throw new TronError( + "password input closed (EOF): piped stdin provided no password " + + "line; restart with --password or provide an interactive TTY", + e, TronError.ErrCode.WITNESS_KEYSTORE_LOAD); + } + password = stripPasswordLine(line); } if (passwordValid(password)) { return password; diff --git a/docs/configuration.md b/docs/configuration.md index d021326a15e..ee613f3fcdd 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -106,7 +106,7 @@ node { solidityPort = 50061 # Maximum concurrent calls per connection. 0 uses the secure default of 100. maxConcurrentCallsPerConnection = 100 - # Idle connection timeout (ms). 0 = no limit. + # Idle connection timeout (ms). 0 = secure default (60 s). maxConnectionIdleInMillis = 0 # Minimum active connections required before broadcasting transactions. minEffectiveConnection = 1 @@ -151,12 +151,13 @@ storage.properties = [ ### Block Production (Super Representatives) ```hocon -# Plain private key (use localwitnesskeystore for production) +# DEPRECATED: plain private key, insecure (anyone reading the config file +# sees the keys). Use localwitnesskeystore for production. localwitness = [ "your-private-key-hex" ] -# Recommended: keystore file +# Recommended: encrypted keystore file # localwitnesskeystore = [ # "/path/to/localwitnesskeystore.json" # ] diff --git a/framework/src/main/java/org/tron/core/config/args/Args.java b/framework/src/main/java/org/tron/core/config/args/Args.java index 0bca242606e..e093d00f2bf 100644 --- a/framework/src/main/java/org/tron/core/config/args/Args.java +++ b/framework/src/main/java/org/tron/core/config/args/Args.java @@ -905,6 +905,9 @@ private static void initLocalWitnesses(Config config, CLIParameter cmd) { // path 1: CLI --private-key if (StringUtils.isNotBlank(cmd.privateKey)) { + logger.warn("Passing the witness private key via -p/--private-key is deprecated and " + + "insecure: the key is visible in the process list and shell history. Use " + + "localwitnesskeystore in the config file instead."); localWitnesses = WitnessInitializer.initFromCLIPrivateKey( cmd.privateKey, cmd.witnessAddress); return; @@ -914,6 +917,8 @@ private static void initLocalWitnesses(Config config, CLIParameter cmd) { // path 2: config localwitness (private key list) if (!lwConfig.getPrivateKeys().isEmpty()) { + logger.warn("Storing plain witness private keys in 'localwitness' is insecure: any " + + "reader of the config file sees the keys. Use localwitnesskeystore instead."); localWitnesses = WitnessInitializer.initFromCFGPrivateKey( lwConfig.getPrivateKeys(), lwConfig.getAccountAddress()); return; diff --git a/framework/src/main/java/org/tron/core/config/args/CLIParameter.java b/framework/src/main/java/org/tron/core/config/args/CLIParameter.java index 4f056a32e3a..6bc18f5996b 100644 --- a/framework/src/main/java/org/tron/core/config/args/CLIParameter.java +++ b/framework/src/main/java/org/tron/core/config/args/CLIParameter.java @@ -38,6 +38,7 @@ public class CLIParameter { @Parameter(names = {"-w", "--witness"}, description = "Is witness node") public boolean witness; + @Deprecated @Parameter(names = {"-p", "--private-key"}, description = "Witness private key") public String privateKey; diff --git a/framework/src/test/java/org/tron/core/config/args/ArgsTest.java b/framework/src/test/java/org/tron/core/config/args/ArgsTest.java index 36b8a3269c1..4001fc36bae 100644 --- a/framework/src/test/java/org/tron/core/config/args/ArgsTest.java +++ b/framework/src/test/java/org/tron/core/config/args/ArgsTest.java @@ -108,7 +108,9 @@ public void get() { .assertEquals(NettyServerBuilder .DEFAULT_FLOW_CONTROL_WINDOW, parameter.getFlowControlWindow()); Assert.assertEquals(60000L, parameter.getMaxConnectionIdleInMillis()); - Assert.assertEquals(Long.MAX_VALUE, parameter.getMaxConnectionAgeInMillis()); + // config-test.conf leaves maxConnectionAgeInMillis unset; postProcess + // converts the 0 default to the secure built-in 60s lifetime default. + Assert.assertEquals(60000L, parameter.getMaxConnectionAgeInMillis()); Assert.assertEquals(GrpcUtil.DEFAULT_MAX_MESSAGE_SIZE, parameter.getMaxMessageSize()); Assert.assertEquals(GrpcUtil.DEFAULT_MAX_MESSAGE_SIZE, parameter.getHttpMaxMessageSize()); Assert.assertEquals(GrpcUtil.DEFAULT_MAX_MESSAGE_SIZE, parameter.getJsonRpcMaxMessageSize()); diff --git a/framework/src/test/java/org/tron/keystore/WalletDecryptBoundsTest.java b/framework/src/test/java/org/tron/keystore/WalletDecryptBoundsTest.java new file mode 100644 index 00000000000..e799e54fa3d --- /dev/null +++ b/framework/src/test/java/org/tron/keystore/WalletDecryptBoundsTest.java @@ -0,0 +1,290 @@ +package org.tron.keystore; + +import static org.junit.Assert.assertArrayEquals; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertThrows; +import static org.junit.Assert.assertTrue; +import static org.junit.Assert.fail; + +import com.fasterxml.jackson.databind.DeserializationFeature; +import com.fasterxml.jackson.databind.ObjectMapper; +import org.junit.Test; +import org.tron.common.crypto.SignInterface; +import org.tron.common.crypto.SignUtils; +import org.tron.common.utils.Utils; +import org.tron.core.exception.CipherException; + +/** + * Verifies that untrusted keystore KDF cost parameters are bounded before + * any key-derivation work happens. A hostile keystore previously drove + * Bouncy Castle into {@code NegativeArraySizeException} (huge scrypt n), + * a ~4 GiB allocation / OOM (n=2^22, r=8), {@code ArithmeticException} + * (r*n overflow), or a multi-hour pbkdf2 CPU hang. All of these must now + * be rejected by {@link Wallet#validate(WalletFile)} (which is called at + * the top of {@link Wallet#decrypt}), never reaching the KDF engine. + * + *

All rejection tests are fast by construction: validation performs no + * KDF execution, and no test ever invokes decrypt with scrypt parameters + * that pass the bounds checks (except the deliberately corrupted + * low-cost keystore used for the CipherException wrap test). + */ +public class WalletDecryptBoundsTest { + + private static final ObjectMapper OBJECT_MAPPER = new ObjectMapper() + .configure(DeserializationFeature.FAIL_ON_UNKNOWN_PROPERTIES, false); + + private static final String SCRYPT_TEMPLATE = + "{\"version\":3,\"address\":\"addr\",\"id\":\"test\",\"crypto\":{" + + "\"cipher\":\"aes-128-ctr\",\"ciphertext\":\"00\"," + + "\"cipherparams\":{\"iv\":\"00\"},\"mac\":\"00\",\"kdf\":\"scrypt\"," + + "\"kdfparams\":{\"dklen\":%d,\"n\":%d,\"r\":%d,\"p\":%d,\"salt\":\"00\"}}}"; + + private static final String PBKDF2_TEMPLATE = + "{\"version\":3,\"address\":\"addr\",\"id\":\"test\",\"crypto\":{" + + "\"cipher\":\"aes-128-ctr\",\"ciphertext\":\"00\"," + + "\"cipherparams\":{\"iv\":\"00\"},\"mac\":\"00\",\"kdf\":\"pbkdf2\"," + + "\"kdfparams\":{\"dklen\":%d,\"c\":%d,\"prf\":\"hmac-sha256\",\"salt\":\"00\"}}}"; + + private static WalletFile parseKeystore(String json) throws Exception { + return OBJECT_MAPPER.readValue(json, WalletFile.class); + } + + private static CipherException validateFailure(String json) throws Exception { + WalletFile walletFile = parseKeystore(json); + return assertThrows(CipherException.class, () -> Wallet.validate(walletFile)); + } + + // ---------- hostile scrypt n ---------- + + @Test + public void testValidateRejectsScryptNNegativeArrayCase() throws Exception { + // n=2^23 previously crashed inside SCrypt.generate with + // NegativeArraySizeException; must be rejected before the KDF runs. + CipherException err = validateFailure(String.format(SCRYPT_TEMPLATE, + 32, 1 << 23, 8, 1)); + assertTrue(err.getMessage(), err.getMessage().contains("n")); + } + + @Test + public void testValidateRejectsScryptNOomCase() throws Exception { + // n=2^22, r=8 requests 128*r*n = 4 GiB and previously died with OOM; + // validation must reject it (fast, no allocation). + long start = System.nanoTime(); + CipherException err = validateFailure(String.format(SCRYPT_TEMPLATE, + 32, 1 << 22, 8, 1)); + assertTrue(err.getMessage(), err.getMessage().contains("n")); + // Rejection is pure validation: must be far cheaper than any KDF run. + assertTrue("rejection must happen before allocation", + System.nanoTime() - start < 5_000_000_000L); + } + + @Test + public void testValidateRejectsScryptNBelowFloor() throws Exception { + CipherException err = validateFailure(String.format(SCRYPT_TEMPLATE, + 32, 1 << 11, 8, 1)); + assertTrue(err.getMessage(), err.getMessage().contains("Scrypt n")); + } + + @Test + public void testValidateRejectsScryptNAboveCap() throws Exception { + CipherException err = validateFailure(String.format(SCRYPT_TEMPLATE, + 32, 1 << 21, 8, 1)); + assertTrue(err.getMessage(), err.getMessage().contains("Scrypt n")); + } + + @Test + public void testValidateRejectsScryptNNotPowerOfTwo() throws Exception { + CipherException err = validateFailure(String.format(SCRYPT_TEMPLATE, + 32, 12345, 8, 1)); + assertTrue(err.getMessage(), err.getMessage().contains("Scrypt n")); + } + + // ---------- hostile scrypt r / p / dklen ---------- + + @Test + public void testValidateRejectsScryptRArithmeticCase() throws Exception { + // r=2^24 with n*r overflow previously crashed inside + // SCrypt.generate with ArithmeticException. + CipherException err = validateFailure(String.format(SCRYPT_TEMPLATE, + 32, 1 << 13, 1 << 24, 1)); + assertTrue(err.getMessage(), err.getMessage().contains("Scrypt r")); + } + + @Test + public void testValidateRejectsScryptPAboveCap() throws Exception { + CipherException err = validateFailure(String.format(SCRYPT_TEMPLATE, + 32, 1 << 13, 8, 9)); + assertTrue(err.getMessage(), err.getMessage().contains("Scrypt p")); + } + + @Test + public void testValidateRejectsOversizedDklen() throws Exception { + CipherException err = validateFailure(String.format(SCRYPT_TEMPLATE, + 4096, 1 << 13, 8, 1)); + assertTrue(err.getMessage(), err.getMessage().contains("Scrypt dklen")); + } + + @Test + public void testValidateRejectsScryptDklenBelowMacFloor() throws Exception { + // dklen=16 passes the old < 1 sanity floor but is too short for + // generateMac (needs derivedKey[16..32)) — validation must reject it + // before any KDF runs, otherwise a raw ArrayIndexOutOfBoundsException + // escapes decrypt(). + CipherException err = validateFailure(String.format(SCRYPT_TEMPLATE, + 16, 1 << 12, 1, 1)); + assertTrue(err.getMessage(), err.getMessage().contains("Scrypt dklen")); + assertTrue(err.getMessage(), err.getMessage().contains("[32")); + } + + // ---------- missing kdfparams ---------- + // + // Note: a keystore JSON that omits "kdfparams" entirely fails at + // deserialization (Jackson external type id), which is already a safe + // rejection. The validation chokepoint itself is exercised here via + // programmatically built WalletFile objects with a null kdfparams. + + private static WalletFile walletWithKdfparams( + String kdf, WalletFile.KdfParams kdfParams) { + WalletFile walletFile = new WalletFile(); + walletFile.setVersion(3); + walletFile.setAddress("addr"); + WalletFile.Crypto crypto = new WalletFile.Crypto(); + crypto.setCipher("aes-128-ctr"); + crypto.setCiphertext("00"); + WalletFile.CipherParams cipherParams = new WalletFile.CipherParams(); + cipherParams.setIv("00"); + crypto.setCipherparams(cipherParams); + crypto.setMac("00"); + crypto.setKdf(kdf); + crypto.setKdfparams(kdfParams); + walletFile.setCrypto(crypto); + return walletFile; + } + + @Test + public void testValidateToleratesMissingScryptKdfparams() { + // Structural stubs without kdfparams are tolerated (KeystoreCliUtilsTest + // fixture contract); decrypt() still fails safely because the + // ClassCastException (RuntimeException) is wrapped into CipherException. + WalletFile walletFile = walletWithKdfparams("scrypt", null); + try { + Wallet.validate(walletFile); + } catch (CipherException e) { + fail("missing kdfparams must be tolerated, got: " + e.getMessage()); + } + assertThrows(CipherException.class, + () -> Wallet.decrypt("password123", walletFile, true)); + } + + @Test + public void testValidateToleratesMissingPbkdf2Kdfparams() { + WalletFile walletFile = walletWithKdfparams("pbkdf2", null); + try { + Wallet.validate(walletFile); + } catch (CipherException e) { + fail("missing kdfparams must be tolerated, got: " + e.getMessage()); + } + assertThrows(CipherException.class, + () -> Wallet.decrypt("password123", walletFile, true)); + } + + // ---------- hostile pbkdf2 ---------- + + @Test + public void testValidateRejectsPbkdf2MaxIterations() throws Exception { + // c=2147483647 is a multi-hour CPU hang; must be rejected up front. + CipherException err = validateFailure(String.format(PBKDF2_TEMPLATE, + 32, Integer.MAX_VALUE)); + assertTrue(err.getMessage(), err.getMessage().contains("Pbkdf2 iteration count")); + } + + @Test + public void testValidateRejectsPbkdf2OversizedDklen() throws Exception { + CipherException err = validateFailure(String.format(PBKDF2_TEMPLATE, + 4096, 262144)); + assertTrue(err.getMessage(), err.getMessage().contains("Pbkdf2 dklen")); + } + + // ---------- behavior freeze: legitimate keystores still pass ---------- + + @Test + public void testValidateAcceptsStandardDefaults() throws Exception { + // Wallet.createStandard: n=2^18, r=8, p=1, dklen=32. + WalletFile standard = parseKeystore(String.format(SCRYPT_TEMPLATE, + 32, 1 << 18, 8, 1)); + Wallet.validate(standard); + assertTrue(Wallet.isValidKeystoreFile(standard)); + + // Wallet.createLight: n=2^12, r=8, p=6, dklen=32. + WalletFile light = parseKeystore(String.format(SCRYPT_TEMPLATE, + 32, 1 << 12, 8, 6)); + Wallet.validate(light); + assertTrue(Wallet.isValidKeystoreFile(light)); + } + + @Test + public void testValidateAcceptsUpperBoundaryParams() throws Exception { + // n=2^20, r=8, p=1 sits exactly at the cap (128*r*n = 1 GiB) and is a + // sane (if heavy) generator choice: validation must accept it. The + // test only asserts validation — the actual scrypt run is never + // executed here. + WalletFile boundary = parseKeystore(String.format(SCRYPT_TEMPLATE, + 128, 1 << 20, 8, 1)); + Wallet.validate(boundary); + assertTrue(Wallet.isValidKeystoreFile(boundary)); + } + + @Test + public void testValidateAcceptsPbkdf2Defaults() throws Exception { + WalletFile walletFile = parseKeystore(String.format(PBKDF2_TEMPLATE, + 32, 262144)); + Wallet.validate(walletFile); + assertTrue(Wallet.isValidKeystoreFile(walletFile)); + } + + // ---------- decrypt wrap: in-bounds but corrupted keystore ---------- + + @Test + public void testDecryptCorruptedKeystoreThrowsCipherException() throws Exception { + // In-bounds low-cost scrypt params (fast), but tampered MAC: + // decrypt must fail with a clean CipherException, never leak a raw + // RuntimeException stack. + String json = String.format(SCRYPT_TEMPLATE, 32, 1 << 12, 1, 1) + .replace("\"mac\":\"00\"", + "\"mac\":\"0102030405060708090a0b0c0d0e0f101112131415161718191a1b1c1d1e1f20\""); + WalletFile walletFile = parseKeystore(json); + Wallet.validate(walletFile); + + CipherException err = assertThrows(CipherException.class, + () -> Wallet.decrypt("password123", walletFile, true)); + assertEquals("Invalid password provided", err.getMessage()); + } + + // ---------- create(): KDF params validated symmetric with decrypt() ---------- + + @Test + public void testCreateRejectsOutOfBoundsKdfParams() { + // p=16 exceeds the SCRYPT_P_MAX=8 bound that validate()/decrypt() enforce, + // so create(n=2^18, p=16) would previously produce a wallet file that only + // this class would later refuse to decrypt. Must throw before any KDF run. + SignInterface keyPair = SignUtils.getGeneratedRandomSign(Utils.getRandom(), true); + + CipherException err = assertThrows(CipherException.class, + () -> Wallet.create("password123", keyPair, 1 << 18, 16)); + assertTrue(err.getMessage(), err.getMessage().contains("Scrypt p")); + } + + @Test(timeout = 60000) + public void testCreateAcceptsInBoundsKdfParams() throws Exception { + // n=2^12 (4096), p=1 is well within the enforced bounds; create must + // succeed and the resulting file must pass validate() and decrypt(). + SignInterface keyPair = SignUtils.getGeneratedRandomSign(Utils.getRandom(), true); + byte[] originalKey = keyPair.getPrivateKey(); + + WalletFile walletFile = Wallet.create("password123", keyPair, 1 << 12, 1); + Wallet.validate(walletFile); + + SignInterface recovered = Wallet.decrypt("password123", walletFile, true); + assertArrayEquals(originalKey, recovered.getPrivateKey()); + } +} diff --git a/framework/src/test/java/org/tron/keystore/WalletUtilsInputPasswordTest.java b/framework/src/test/java/org/tron/keystore/WalletUtilsInputPasswordTest.java index 64752b9ca49..fc43331fc18 100644 --- a/framework/src/test/java/org/tron/keystore/WalletUtilsInputPasswordTest.java +++ b/framework/src/test/java/org/tron/keystore/WalletUtilsInputPasswordTest.java @@ -2,6 +2,7 @@ import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertThrows; import java.io.ByteArrayInputStream; import java.io.InputStream; @@ -9,6 +10,7 @@ import org.junit.After; import org.junit.Before; import org.junit.Test; +import org.tron.core.exception.TronError; /** * Verifies that {@link WalletUtils#inputPassword()} preserves the full @@ -95,6 +97,23 @@ public void testInputPasswordPreservesLeadingAndTrailingSpaces() { " with spaces ", pw); } + // ---------- EOF fail-fast tests ---------- + + @Test(timeout = 5000) + public void testInputPasswordPipedStdinEofThrowsTronError() { + // Non-TTY branch: Scanner.nextLine() throws NoSuchElementException at + // EOF; it must be translated into a fail-fast TronError. + // (The TTY branch's Console.readPassword() null-on-EOF check cannot be + // exercised under JUnit, where System.console() is always null and + // java.io.Console is final — covered by code review only.) + System.setIn(new ByteArrayInputStream(new byte[0])); + + TronError err = assertThrows(TronError.class, WalletUtils::inputPassword); + + assertEquals("Piped stdin EOF must map to the keystore-load error code", + TronError.ErrCode.WITNESS_KEYSTORE_LOAD, err.getErrCode()); + } + @Test(timeout = 10000) public void testInputPassword2TwicePipedPreservesInternalWhitespace() { // M1: verifies the double-read path (inputPassword2Twice → inputPassword() diff --git a/framework/src/test/java/org/tron/program/KeystoreFactoryDeprecationTest.java b/framework/src/test/java/org/tron/program/KeystoreFactoryDeprecationTest.java index 860980d21e5..7c77b734ac0 100644 --- a/framework/src/test/java/org/tron/program/KeystoreFactoryDeprecationTest.java +++ b/framework/src/test/java/org/tron/program/KeystoreFactoryDeprecationTest.java @@ -1,5 +1,6 @@ package org.tron.program; +import static org.junit.Assert.assertThrows; import static org.junit.Assert.assertTrue; import java.io.ByteArrayInputStream; @@ -12,6 +13,7 @@ import org.junit.Test; import org.tron.common.TestConstants; import org.tron.core.config.args.Args; +import org.tron.core.exception.TronError; /** * Verifies the deprecated --keystore-factory CLI. @@ -115,19 +117,18 @@ public void testQuitCommand() throws Exception { @Test(timeout = 10000) public void testGenKeystoreTriggersError() throws Exception { - // genkeystore reads password via a nested Scanner, which conflicts - // with the outer Scanner and throws "No line found". The error is - // caught and logged, and the REPL continues. + // genkeystore reads the password via WalletUtils.inputPassword; the + // scripted stdin hits EOF on the second read, so the prompt now throws + // TronError (KEY-01 fix) instead of being caught by the REPL loop. ByteArrayOutputStream outContent = new ByteArrayOutputStream(); System.setOut(new PrintStream(outContent)); System.setIn(new ByteArrayInputStream("genkeystore\nexit\n".getBytes())); - KeystoreFactory.start(); + assertThrows(TronError.class, KeystoreFactory::start); String out = outContent.toString("UTF-8"); assertTrue("genKeystore should prompt for password", out.contains("Please input password")); - assertTrue("REPL should continue to exit", out.contains("Exit")); } @Test(timeout = 10000) diff --git a/gradle/verification-metadata.xml b/gradle/verification-metadata.xml index 6a3e641d5d6..163b793c105 100644 --- a/gradle/verification-metadata.xml +++ b/gradle/verification-metadata.xml @@ -3,11 +3,6 @@ true false - - - - - @@ -1255,6 +1250,9 @@ + + + diff --git a/gradle/wrapper/gradle-wrapper.properties b/gradle/wrapper/gradle-wrapper.properties index 3994438e229..a2ccb4862b3 100644 --- a/gradle/wrapper/gradle-wrapper.properties +++ b/gradle/wrapper/gradle-wrapper.properties @@ -1,5 +1,6 @@ distributionBase=GRADLE_USER_HOME distributionPath=wrapper/dists distributionUrl=https\://services.gradle.org/distributions/gradle-7.6.4-bin.zip +distributionSha256Sum=bed1da33cca0f557ab13691c77f38bb67388119e4794d113e051039b80af9bb1 zipStoreBase=GRADLE_USER_HOME zipStorePath=wrapper/dists diff --git a/plugins/README.md b/plugins/README.md index f14e070c01a..857ee229c3f 100644 --- a/plugins/README.md +++ b/plugins/README.md @@ -222,3 +222,5 @@ When using `--password-file` with `update`, the file must contain exactly two li - `--sm2`: Use SM2 algorithm instead of ECDSA (for `new` and `import`). - `--json`: Output in JSON format for scripting. - `-h | --help`: Provide the help info. + +> **Note:** `--password-file` and `--key-file` must be owner-only (`chmod 600`). Files readable or writable by group/other are rejected — create them with a restrictive umask or run `chmod 600 ` before use. diff --git a/plugins/src/main/java/common/org/tron/plugins/KeystoreCliUtils.java b/plugins/src/main/java/common/org/tron/plugins/KeystoreCliUtils.java index 6959a7f8177..9d7f3c5a92e 100644 --- a/plugins/src/main/java/common/org/tron/plugins/KeystoreCliUtils.java +++ b/plugins/src/main/java/common/org/tron/plugins/KeystoreCliUtils.java @@ -15,7 +15,11 @@ import java.nio.file.Path; import java.nio.file.StandardOpenOption; import java.nio.file.attribute.BasicFileAttributes; +import java.nio.file.attribute.PosixFileAttributeView; +import java.nio.file.attribute.PosixFilePermission; +import java.nio.file.attribute.PosixFilePermissions; import java.util.Arrays; +import java.util.EnumSet; import java.util.HashSet; import java.util.LinkedHashMap; import java.util.Map; @@ -81,6 +85,20 @@ static byte[] readRegularFile(File file, long maxSize, String label, PrintWriter return null; } + // Refuse secret files readable or writable by group/other. Only owner + // bits are allowed; filesystems without POSIX attributes are skipped. + PosixFileAttributeView posixView = Files.getFileAttributeView( + file.toPath(), PosixFileAttributeView.class, LinkOption.NOFOLLOW_LINKS); + if (posixView != null + && !EnumSet.of(PosixFilePermission.OWNER_READ, PosixFilePermission.OWNER_WRITE, + PosixFilePermission.OWNER_EXECUTE) + .containsAll(posixView.readAttributes().permissions())) { + err.println(label + " must be owner-only (no group/other access), got " + + PosixFilePermissions.toString(posixView.readAttributes().permissions()) + + ": " + file.getPath()); + return null; + } + int size = (int) attrs.size(); java.util.Set openOptions = new HashSet<>(); openOptions.add(StandardOpenOption.READ); diff --git a/plugins/src/test/java/org/tron/plugins/KeystoreCliUtilsTest.java b/plugins/src/test/java/org/tron/plugins/KeystoreCliUtilsTest.java index 29782af91e2..ccbd1092b10 100644 --- a/plugins/src/test/java/org/tron/plugins/KeystoreCliUtilsTest.java +++ b/plugins/src/test/java/org/tron/plugins/KeystoreCliUtilsTest.java @@ -7,6 +7,7 @@ import static org.junit.Assert.assertTrue; import java.io.File; +import java.io.IOException; import java.io.PrintWriter; import java.io.StringWriter; import java.nio.charset.StandardCharsets; @@ -165,6 +166,7 @@ public void testCheckFileExistsPresent() throws Exception { public void testReadPasswordFromFile() throws Exception { File pwFile = tempFolder.newFile("pw.txt"); Files.write(pwFile.toPath(), "goodpassword".getBytes(StandardCharsets.UTF_8)); + makeOwnerOnly(pwFile); StringWriter err = new StringWriter(); String pw = KeystoreCliUtils.readPassword(pwFile, new PrintWriter(err)); assertEquals("goodpassword", pw); @@ -174,6 +176,7 @@ public void testReadPasswordFromFile() throws Exception { public void testReadPasswordFromFileWithLineEndings() throws Exception { File pwFile = tempFolder.newFile("pw-crlf.txt"); Files.write(pwFile.toPath(), "goodpassword\r\n".getBytes(StandardCharsets.UTF_8)); + makeOwnerOnly(pwFile); StringWriter err = new StringWriter(); String pw = KeystoreCliUtils.readPassword(pwFile, new PrintWriter(err)); assertEquals("goodpassword", pw); @@ -184,6 +187,7 @@ public void testReadPasswordFromFileWithBom() throws Exception { File pwFile = tempFolder.newFile("pw-bom.txt"); Files.write(pwFile.toPath(), "\uFEFFgoodpassword".getBytes(StandardCharsets.UTF_8)); + makeOwnerOnly(pwFile); StringWriter err = new StringWriter(); String pw = KeystoreCliUtils.readPassword(pwFile, new PrintWriter(err)); assertEquals("goodpassword", pw); @@ -205,6 +209,7 @@ public void testReadPasswordFileTooLarge() throws Exception { public void testReadPasswordFileShort() throws Exception { File pwFile = tempFolder.newFile("pw-short.txt"); Files.write(pwFile.toPath(), "abc".getBytes(StandardCharsets.UTF_8)); + makeOwnerOnly(pwFile); StringWriter err = new StringWriter(); String pw = KeystoreCliUtils.readPassword(pwFile, new PrintWriter(err)); assertNull(pw); @@ -270,6 +275,7 @@ public void testPrintSecurityTipsIncludesAddressAndFile() { public void testReadRegularFileSuccess() throws Exception { File f = tempFolder.newFile("regular.txt"); Files.write(f.toPath(), "hello".getBytes(StandardCharsets.UTF_8)); + makeOwnerOnly(f); StringWriter err = new StringWriter(); byte[] bytes = KeystoreCliUtils.readRegularFile(f, 1024, "File", @@ -339,6 +345,7 @@ public void testReadRegularFileRefusesDirectory() throws Exception { @Test public void testReadRegularFileEmptyFile() throws Exception { File f = tempFolder.newFile("empty.txt"); + makeOwnerOnly(f); StringWriter err = new StringWriter(); byte[] bytes = KeystoreCliUtils.readRegularFile(f, 1024, "File", @@ -346,4 +353,39 @@ public void testReadRegularFileEmptyFile() throws Exception { assertNotNull(bytes); assertEquals(0, bytes.length); } + + private static void makeOwnerOnly(File f) throws IOException { + org.junit.Assume.assumeTrue("POSIX permissions required", + Files.getFileAttributeView(f.toPath(), + java.nio.file.attribute.PosixFileAttributeView.class) != null); + Files.setPosixFilePermissions(f.toPath(), + java.nio.file.attribute.PosixFilePermissions.fromString("rw-------")); + } + + @Test + public void testReadRegularFileRejectsGroupReadable() throws Exception { + File f = tempFolder.newFile("group-readable.txt"); + Files.write(f.toPath(), "hello".getBytes(StandardCharsets.UTF_8)); + makeOwnerOnly(f); + Files.setPosixFilePermissions(f.toPath(), + java.nio.file.attribute.PosixFilePermissions.fromString("rw-r-----")); + StringWriter err = new StringWriter(); + byte[] bytes = KeystoreCliUtils.readRegularFile(f, 1024, + "Password file", new PrintWriter(err)); + assertNull(bytes); + assertTrue("expected owner-only error, got: " + err, + err.toString().contains("owner-only")); + } + + @Test + public void testReadRegularFileAcceptsOwnerOnly() throws Exception { + File f = tempFolder.newFile("owner-only.txt"); + Files.write(f.toPath(), "hello".getBytes(StandardCharsets.UTF_8)); + makeOwnerOnly(f); + StringWriter err = new StringWriter(); + byte[] bytes = KeystoreCliUtils.readRegularFile(f, 1024, + "Password file", new PrintWriter(err)); + assertNotNull(bytes); + assertEquals("hello", new String(bytes, StandardCharsets.UTF_8)); + } } diff --git a/plugins/src/test/java/org/tron/plugins/KeystoreImportTest.java b/plugins/src/test/java/org/tron/plugins/KeystoreImportTest.java index 577889fe196..5034e59a7da 100644 --- a/plugins/src/test/java/org/tron/plugins/KeystoreImportTest.java +++ b/plugins/src/test/java/org/tron/plugins/KeystoreImportTest.java @@ -6,6 +6,7 @@ import static org.junit.Assert.assertTrue; import java.io.File; +import java.io.IOException; import java.nio.charset.StandardCharsets; import java.nio.file.Files; import java.security.SecureRandom; @@ -37,9 +38,11 @@ public void testImportWithKeyFileAndPasswordFile() throws Exception { File keyFile = tempFolder.newFile("private.key"); Files.write(keyFile.toPath(), privateKeyHex.getBytes(StandardCharsets.UTF_8)); + makeOwnerOnly(keyFile); File pwFile = tempFolder.newFile("password.txt"); Files.write(pwFile.toPath(), "test123456".getBytes(StandardCharsets.UTF_8)); + makeOwnerOnly(pwFile); CommandLine cmd = new CommandLine(new Toolkit()); int exitCode = cmd.execute("keystore", "import", @@ -65,9 +68,11 @@ public void testImportInvalidKeyTooShort() throws Exception { File dir = tempFolder.newFolder("keystore-bad"); File keyFile = tempFolder.newFile("bad.key"); Files.write(keyFile.toPath(), "abcdef1234".getBytes(StandardCharsets.UTF_8)); + makeOwnerOnly(keyFile); File pwFile = tempFolder.newFile("pw.txt"); Files.write(pwFile.toPath(), "test123456".getBytes(StandardCharsets.UTF_8)); + makeOwnerOnly(pwFile); CommandLine cmd = new CommandLine(new Toolkit()); int exitCode = cmd.execute("keystore", "import", @@ -85,9 +90,11 @@ public void testImportInvalidKeyNonHex() throws Exception { Files.write(keyFile.toPath(), "zzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzz" .getBytes(StandardCharsets.UTF_8)); + makeOwnerOnly(keyFile); File pwFile = tempFolder.newFile("pw.txt"); Files.write(pwFile.toPath(), "test123456".getBytes(StandardCharsets.UTF_8)); + makeOwnerOnly(pwFile); CommandLine cmd = new CommandLine(new Toolkit()); int exitCode = cmd.execute("keystore", "import", @@ -103,6 +110,7 @@ public void testImportNoTtyNoKeyFile() throws Exception { File dir = tempFolder.newFolder("keystore-notty"); File pwFile = tempFolder.newFile("pw2.txt"); Files.write(pwFile.toPath(), "test123456".getBytes(StandardCharsets.UTF_8)); + makeOwnerOnly(pwFile); // No --key-file and System.console() is null in CI CommandLine cmd = new CommandLine(new Toolkit()); @@ -123,8 +131,10 @@ public void testImportWithSm2() throws Exception { File keyFile = tempFolder.newFile("sm2.key"); Files.write(keyFile.toPath(), privateKeyHex.getBytes(StandardCharsets.UTF_8)); + makeOwnerOnly(keyFile); File pwFile = tempFolder.newFile("pw-sm2.txt"); Files.write(pwFile.toPath(), "test123456".getBytes(StandardCharsets.UTF_8)); + makeOwnerOnly(pwFile); CommandLine cmd = new CommandLine(new Toolkit()); int exitCode = cmd.execute("keystore", "import", @@ -155,8 +165,10 @@ public void testImportKeyFileWithWhitespace() throws Exception { File keyFile = tempFolder.newFile("ws.key"); Files.write(keyFile.toPath(), (" " + privateKeyHex + " \n\n").getBytes(StandardCharsets.UTF_8)); + makeOwnerOnly(keyFile); File pwFile = tempFolder.newFile("pw-ws.txt"); Files.write(pwFile.toPath(), "test123456".getBytes(StandardCharsets.UTF_8)); + makeOwnerOnly(pwFile); CommandLine cmd = new CommandLine(new Toolkit()); int exitCode = cmd.execute("keystore", "import", @@ -183,8 +195,10 @@ public void testImportDuplicateAddressBlocked() throws Exception { File keyFile = tempFolder.newFile("dup.key"); Files.write(keyFile.toPath(), privateKeyHex.getBytes(StandardCharsets.UTF_8)); + makeOwnerOnly(keyFile); File pwFile = tempFolder.newFile("pw-dup.txt"); Files.write(pwFile.toPath(), "test123456".getBytes(StandardCharsets.UTF_8)); + makeOwnerOnly(pwFile); // First import succeeds CommandLine cmd1 = new CommandLine(new Toolkit()); @@ -219,8 +233,10 @@ public void testImportDuplicateAddressWithForce() throws Exception { File keyFile = tempFolder.newFile("force.key"); Files.write(keyFile.toPath(), privateKeyHex.getBytes(StandardCharsets.UTF_8)); + makeOwnerOnly(keyFile); File pwFile = tempFolder.newFile("pw-force.txt"); Files.write(pwFile.toPath(), "test123456".getBytes(StandardCharsets.UTF_8)); + makeOwnerOnly(pwFile); // First import CommandLine cmd1 = new CommandLine(new Toolkit()); @@ -247,6 +263,7 @@ public void testImportKeyFileNotFound() throws Exception { File dir = tempFolder.newFolder("keystore-nokey"); File pwFile = tempFolder.newFile("pw-nokey.txt"); Files.write(pwFile.toPath(), "test123456".getBytes(StandardCharsets.UTF_8)); + makeOwnerOnly(pwFile); CommandLine cmd = new CommandLine(new Toolkit()); int exitCode = cmd.execute("keystore", "import", @@ -268,8 +285,10 @@ public void testImportWith0xPrefix() throws Exception { File keyFile = tempFolder.newFile("0x.key"); Files.write(keyFile.toPath(), ("0x" + privateKeyHex).getBytes(StandardCharsets.UTF_8)); + makeOwnerOnly(keyFile); File pwFile = tempFolder.newFile("pw-0x.txt"); Files.write(pwFile.toPath(), "test123456".getBytes(StandardCharsets.UTF_8)); + makeOwnerOnly(pwFile); CommandLine cmd = new CommandLine(new Toolkit()); int exitCode = cmd.execute("keystore", "import", @@ -295,8 +314,10 @@ public void testImportWith0XUppercasePrefix() throws Exception { File keyFile = tempFolder.newFile("0X.key"); Files.write(keyFile.toPath(), ("0X" + privateKeyHex).getBytes(StandardCharsets.UTF_8)); + makeOwnerOnly(keyFile); File pwFile = tempFolder.newFile("pw-0X.txt"); Files.write(pwFile.toPath(), "test123456".getBytes(StandardCharsets.UTF_8)); + makeOwnerOnly(pwFile); CommandLine cmd = new CommandLine(new Toolkit()); int exitCode = cmd.execute("keystore", "import", @@ -320,8 +341,10 @@ public void testImportWarnsOnCorruptedFile() throws Exception { File keyFile = tempFolder.newFile("warn.key"); Files.write(keyFile.toPath(), privateKeyHex.getBytes(StandardCharsets.UTF_8)); + makeOwnerOnly(keyFile); File pwFile = tempFolder.newFile("pw-warn.txt"); Files.write(pwFile.toPath(), "test123456".getBytes(StandardCharsets.UTF_8)); + makeOwnerOnly(pwFile); java.io.StringWriter out = new java.io.StringWriter(); java.io.StringWriter err = new java.io.StringWriter(); @@ -352,8 +375,10 @@ public void testImportKeystoreFilePermissions() throws Exception { File keyFile = tempFolder.newFile("perm.key"); Files.write(keyFile.toPath(), privateKeyHex.getBytes(StandardCharsets.UTF_8)); + makeOwnerOnly(keyFile); File pwFile = tempFolder.newFile("pw-perm.txt"); Files.write(pwFile.toPath(), "test123456".getBytes(StandardCharsets.UTF_8)); + makeOwnerOnly(pwFile); CommandLine cmd = new CommandLine(new Toolkit()); int exitCode = cmd.execute("keystore", "import", @@ -393,6 +418,7 @@ public void testImportRefusesSymlinkKeyFile() throws Exception { File pwFile = tempFolder.newFile("pw-symlink.txt"); Files.write(pwFile.toPath(), "test123456".getBytes(StandardCharsets.UTF_8)); + makeOwnerOnly(pwFile); java.io.StringWriter err = new java.io.StringWriter(); CommandLine cmd = new CommandLine(new Toolkit()); @@ -418,9 +444,11 @@ public void testImportRefusesSymlinkPasswordFile() throws Exception { File keyFile = tempFolder.newFile("sym-pw.key"); Files.write(keyFile.toPath(), ByteArray.toHexString(keyPair.getPrivateKey()).getBytes(StandardCharsets.UTF_8)); + makeOwnerOnly(keyFile); File realPwFile = tempFolder.newFile("real-pw.txt"); Files.write(realPwFile.toPath(), "test123456".getBytes(StandardCharsets.UTF_8)); + makeOwnerOnly(realPwFile); File pwSymlink = new File(tempFolder.getRoot(), "pw-symlink.txt"); Files.createSymbolicLink(pwSymlink.toPath(), realPwFile.toPath()); @@ -453,8 +481,10 @@ public void testImportDuplicateCheckSkipsInvalidVersion() throws Exception { File keyFile = tempFolder.newFile("ver.key"); Files.write(keyFile.toPath(), privateKeyHex.getBytes(StandardCharsets.UTF_8)); + makeOwnerOnly(keyFile); File pwFile = tempFolder.newFile("pw-ver.txt"); Files.write(pwFile.toPath(), "test123456".getBytes(StandardCharsets.UTF_8)); + makeOwnerOnly(pwFile); CommandLine cmd = new CommandLine(new Toolkit()); int exitCode = cmd.execute("keystore", "import", @@ -486,8 +516,10 @@ public void testImportDuplicateScanSkipsSymlinkedEntry() throws Exception { File keyFile = tempFolder.newFile("dup-sym.key"); Files.write(keyFile.toPath(), privateKeyHex.getBytes(StandardCharsets.UTF_8)); + makeOwnerOnly(keyFile); File pwFile = tempFolder.newFile("pw-dup-sym.txt"); Files.write(pwFile.toPath(), "test123456".getBytes(StandardCharsets.UTF_8)); + makeOwnerOnly(pwFile); java.io.StringWriter err = new java.io.StringWriter(); CommandLine cmd = new CommandLine(new Toolkit()); @@ -515,9 +547,11 @@ public void testImportRejectsMultiLinePasswordFile() throws Exception { File keyFile = tempFolder.newFile("multi.key"); Files.write(keyFile.toPath(), privateKeyHex.getBytes(StandardCharsets.UTF_8)); + makeOwnerOnly(keyFile); File pwFile = tempFolder.newFile("multi-pw.txt"); Files.write(pwFile.toPath(), "oldpass123\nnewpass456".getBytes(StandardCharsets.UTF_8)); + makeOwnerOnly(pwFile); java.io.StringWriter err = new java.io.StringWriter(); CommandLine cmd = new CommandLine(new Toolkit()); @@ -534,4 +568,12 @@ public void testImportRejectsMultiLinePasswordFile() throws Exception { assertTrue("No keystore should have been created", files == null || files.length == 0); } + + private static void makeOwnerOnly(File f) throws IOException { + org.junit.Assume.assumeTrue("POSIX permissions required", + Files.getFileAttributeView(f.toPath(), + java.nio.file.attribute.PosixFileAttributeView.class) != null); + Files.setPosixFilePermissions(f.toPath(), + java.nio.file.attribute.PosixFilePermissions.fromString("rw-------")); + } } diff --git a/plugins/src/test/java/org/tron/plugins/KeystoreNewTest.java b/plugins/src/test/java/org/tron/plugins/KeystoreNewTest.java index 26e3a9b9764..dbcdf8c2eb7 100644 --- a/plugins/src/test/java/org/tron/plugins/KeystoreNewTest.java +++ b/plugins/src/test/java/org/tron/plugins/KeystoreNewTest.java @@ -5,6 +5,7 @@ import static org.junit.Assert.assertTrue; import java.io.File; +import java.io.IOException; import java.io.PrintWriter; import java.io.StringWriter; import java.nio.charset.StandardCharsets; @@ -27,6 +28,7 @@ public void testNewKeystoreWithPasswordFile() throws Exception { File dir = tempFolder.newFolder("keystore"); File pwFile = tempFolder.newFile("password.txt"); Files.write(pwFile.toPath(), "test123456".getBytes(StandardCharsets.UTF_8)); + makeOwnerOnly(pwFile); StringWriter out = new StringWriter(); StringWriter err = new StringWriter(); @@ -55,6 +57,7 @@ public void testNewKeystoreJsonOutput() throws Exception { File dir = tempFolder.newFolder("keystore-json"); File pwFile = tempFolder.newFile("password-json.txt"); Files.write(pwFile.toPath(), "test123456".getBytes(StandardCharsets.UTF_8)); + makeOwnerOnly(pwFile); StringWriter out = new StringWriter(); StringWriter err = new StringWriter(); @@ -79,6 +82,7 @@ public void testNewKeystoreInvalidPassword() throws Exception { File dir = tempFolder.newFolder("keystore-bad"); File pwFile = tempFolder.newFile("short.txt"); Files.write(pwFile.toPath(), "abc".getBytes(StandardCharsets.UTF_8)); + makeOwnerOnly(pwFile); StringWriter err = new StringWriter(); CommandLine cmd = new CommandLine(new Toolkit()); @@ -97,6 +101,7 @@ public void testNewKeystoreCustomDir() throws Exception { File dir = new File(tempFolder.getRoot(), "custom/nested/dir"); File pwFile = tempFolder.newFile("pw.txt"); Files.write(pwFile.toPath(), "test123456".getBytes(StandardCharsets.UTF_8)); + makeOwnerOnly(pwFile); CommandLine cmd = new CommandLine(new Toolkit()); int exitCode = cmd.execute("keystore", "new", @@ -128,6 +133,7 @@ public void testNewKeystoreEmptyPassword() throws Exception { File dir = tempFolder.newFolder("keystore-empty"); File pwFile = tempFolder.newFile("empty.txt"); Files.write(pwFile.toPath(), "".getBytes(StandardCharsets.UTF_8)); + makeOwnerOnly(pwFile); StringWriter err = new StringWriter(); CommandLine cmd = new CommandLine(new Toolkit()); @@ -146,6 +152,7 @@ public void testNewKeystoreWithSm2() throws Exception { File dir = tempFolder.newFolder("keystore-sm2"); File pwFile = tempFolder.newFile("pw-sm2.txt"); Files.write(pwFile.toPath(), "test123456".getBytes(StandardCharsets.UTF_8)); + makeOwnerOnly(pwFile); CommandLine cmd = new CommandLine(new Toolkit()); int exitCode = cmd.execute("keystore", "new", @@ -170,6 +177,7 @@ public void testNewKeystoreSpecialCharPassword() throws Exception { File pwFile = tempFolder.newFile("pw-special.txt"); String password = "p@$$w0rd!#%^&*()_+-=[]{}"; Files.write(pwFile.toPath(), password.getBytes(StandardCharsets.UTF_8)); + makeOwnerOnly(pwFile); CommandLine cmd = new CommandLine(new Toolkit()); int exitCode = cmd.execute("keystore", "new", @@ -203,6 +211,7 @@ public void testNewKeystoreDirIsFile() throws Exception { File notADir = tempFolder.newFile("not-a-dir"); File pwFile = tempFolder.newFile("pw-dir.txt"); Files.write(pwFile.toPath(), "test123456".getBytes(StandardCharsets.UTF_8)); + makeOwnerOnly(pwFile); CommandLine cmd = new CommandLine(new Toolkit()); int exitCode = cmd.execute("keystore", "new", @@ -219,6 +228,7 @@ public void testNewKeystorePasswordFileTooLarge() throws Exception { byte[] bigContent = new byte[1025]; java.util.Arrays.fill(bigContent, (byte) 'a'); Files.write(pwFile.toPath(), bigContent); + makeOwnerOnly(pwFile); StringWriter err = new StringWriter(); CommandLine cmd = new CommandLine(new Toolkit()); @@ -238,6 +248,7 @@ public void testNewKeystorePasswordFileWithBom() throws Exception { File pwFile = tempFolder.newFile("bom.txt"); Files.write(pwFile.toPath(), ("\uFEFF" + "test123456").getBytes(StandardCharsets.UTF_8)); + makeOwnerOnly(pwFile); CommandLine cmd = new CommandLine(new Toolkit()); int exitCode = cmd.execute("keystore", "new", @@ -259,6 +270,7 @@ public void testNewKeystoreFilePermissions() throws Exception { File dir = tempFolder.newFolder("keystore-perms"); File pwFile = tempFolder.newFile("pw-perms.txt"); Files.write(pwFile.toPath(), "test123456".getBytes(StandardCharsets.UTF_8)); + makeOwnerOnly(pwFile); CommandLine cmd = new CommandLine(new Toolkit()); int exitCode = cmd.execute("keystore", "new", @@ -289,6 +301,7 @@ public void testNewKeystoreRejectsMultiLinePasswordFile() throws Exception { File pwFile = tempFolder.newFile("multi-line.txt"); Files.write(pwFile.toPath(), "oldpass123\nnewpass456".getBytes(StandardCharsets.UTF_8)); + makeOwnerOnly(pwFile); StringWriter err = new StringWriter(); CommandLine cmd = new CommandLine(new Toolkit()); @@ -305,4 +318,12 @@ public void testNewKeystoreRejectsMultiLinePasswordFile() throws Exception { assertTrue("No keystore should have been created", files == null || files.length == 0); } + + private static void makeOwnerOnly(File f) throws IOException { + org.junit.Assume.assumeTrue("POSIX permissions required", + Files.getFileAttributeView(f.toPath(), + java.nio.file.attribute.PosixFileAttributeView.class) != null); + Files.setPosixFilePermissions(f.toPath(), + java.nio.file.attribute.PosixFilePermissions.fromString("rw-------")); + } } diff --git a/plugins/src/test/java/org/tron/plugins/KeystoreUpdateTest.java b/plugins/src/test/java/org/tron/plugins/KeystoreUpdateTest.java index ed8f81acd32..1ca4d595aad 100644 --- a/plugins/src/test/java/org/tron/plugins/KeystoreUpdateTest.java +++ b/plugins/src/test/java/org/tron/plugins/KeystoreUpdateTest.java @@ -7,6 +7,7 @@ import com.fasterxml.jackson.databind.ObjectMapper; import java.io.File; +import java.io.IOException; import java.io.PrintWriter; import java.io.StringWriter; import java.nio.charset.StandardCharsets; @@ -48,6 +49,7 @@ public void testUpdatePassword() throws Exception { File pwFile = tempFolder.newFile("passwords.txt"); Files.write(pwFile.toPath(), (oldPassword + "\n" + newPassword).getBytes(StandardCharsets.UTF_8)); + makeOwnerOnly(pwFile); CommandLine cmd = new CommandLine(new Toolkit()); int exitCode = cmd.execute("keystore", "update", address, @@ -84,6 +86,7 @@ public void testUpdateWrongOldPassword() throws Exception { File pwFile = tempFolder.newFile("wrong.txt"); Files.write(pwFile.toPath(), ("wrongpass1\nnewpass456").getBytes(StandardCharsets.UTF_8)); + makeOwnerOnly(pwFile); StringWriter err = new StringWriter(); CommandLine cmd = new CommandLine(new Toolkit()); @@ -114,6 +117,7 @@ public void testUpdateNonExistentAddress() throws Exception { File pwFile = tempFolder.newFile("pw.txt"); Files.write(pwFile.toPath(), ("test123456\nnewpass789").getBytes(StandardCharsets.UTF_8)); + makeOwnerOnly(pwFile); StringWriter err = new StringWriter(); CommandLine cmd = new CommandLine(new Toolkit()); @@ -142,6 +146,7 @@ public void testUpdateNewPasswordTooShort() throws Exception { File pwFile = tempFolder.newFile("shortpw.txt"); Files.write(pwFile.toPath(), (password + "\nabc").getBytes(StandardCharsets.UTF_8)); + makeOwnerOnly(pwFile); StringWriter err = new StringWriter(); CommandLine cmd = new CommandLine(new Toolkit()); @@ -171,6 +176,7 @@ public void testUpdateWithWindowsLineEndings() throws Exception { File pwFile = tempFolder.newFile("crlf.txt"); Files.write(pwFile.toPath(), (oldPassword + "\r\n" + newPassword + "\r\n").getBytes(StandardCharsets.UTF_8)); + makeOwnerOnly(pwFile); CommandLine cmd = new CommandLine(new Toolkit()); int exitCode = cmd.execute("keystore", "update", creds.getAddress(), @@ -200,6 +206,7 @@ public void testUpdateJsonOutput() throws Exception { File pwFile = tempFolder.newFile("pw-json.txt"); Files.write(pwFile.toPath(), (oldPassword + "\n" + newPassword).getBytes(StandardCharsets.UTF_8)); + makeOwnerOnly(pwFile); StringWriter out = new StringWriter(); StringWriter err = new StringWriter(); @@ -238,6 +245,7 @@ public void testUpdateWarnsOnCorruptedFile() throws Exception { File pwFile = tempFolder.newFile("pw-corrupt.txt"); Files.write(pwFile.toPath(), (password + "\nnewpass789").getBytes(StandardCharsets.UTF_8)); + makeOwnerOnly(pwFile); StringWriter out = new StringWriter(); StringWriter err = new StringWriter(); @@ -267,6 +275,7 @@ public void testUpdatePasswordFileOnlyOneLine() throws Exception { File pwFile = tempFolder.newFile("oneline.txt"); Files.write(pwFile.toPath(), "onlyoldpassword".getBytes(StandardCharsets.UTF_8)); + makeOwnerOnly(pwFile); StringWriter err = new StringWriter(); CommandLine cmd = new CommandLine(new Toolkit()); @@ -302,6 +311,7 @@ public void testUpdatePasswordFileThreeLines() throws Exception { File pwFile = tempFolder.newFile("threeline.txt"); Files.write(pwFile.toPath(), (oldPassword + "\nnewpass456\nnewpass456").getBytes(StandardCharsets.UTF_8)); + makeOwnerOnly(pwFile); StringWriter err = new StringWriter(); CommandLine cmd = new CommandLine(new Toolkit()); @@ -387,6 +397,7 @@ public void testUpdateSm2Keystore() throws Exception { File pwFile = tempFolder.newFile("pw-sm2.txt"); Files.write(pwFile.toPath(), (oldPassword + "\n" + newPassword).getBytes(StandardCharsets.UTF_8)); + makeOwnerOnly(pwFile); CommandLine cmd = new CommandLine(new Toolkit()); int exitCode = cmd.execute("keystore", "update", creds.getAddress(), @@ -420,6 +431,7 @@ public void testUpdateMultipleKeystoresSameAddress() throws Exception { File pwFile = tempFolder.newFile("pw-multi.txt"); Files.write(pwFile.toPath(), (password + "\nnewpass789").getBytes(StandardCharsets.UTF_8)); + makeOwnerOnly(pwFile); StringWriter err = new StringWriter(); CommandLine cmd = new CommandLine(new Toolkit()); @@ -451,6 +463,7 @@ public void testUpdatePasswordFileTooLarge() throws Exception { byte[] bigContent = new byte[1025]; java.util.Arrays.fill(bigContent, (byte) 'a'); Files.write(pwFile.toPath(), bigContent); + makeOwnerOnly(pwFile); StringWriter err = new StringWriter(); CommandLine cmd = new CommandLine(new Toolkit()); @@ -481,6 +494,7 @@ public void testUpdatePasswordFileWithBom() throws Exception { File pwFile = tempFolder.newFile("bom.txt"); Files.write(pwFile.toPath(), ("\uFEFF" + oldPassword + "\n" + newPassword).getBytes(StandardCharsets.UTF_8)); + makeOwnerOnly(pwFile); CommandLine cmd = new CommandLine(new Toolkit()); int exitCode = cmd.execute("keystore", "update", creds.getAddress(), @@ -502,6 +516,7 @@ public void testUpdateNonExistentKeystoreDir() throws Exception { File pwFile = tempFolder.newFile("pw-nodir.txt"); Files.write(pwFile.toPath(), ("oldpass123\nnewpass456").getBytes(StandardCharsets.UTF_8)); + makeOwnerOnly(pwFile); StringWriter err = new StringWriter(); CommandLine cmd = new CommandLine(new Toolkit()); @@ -522,6 +537,7 @@ public void testUpdateKeystoreDirIsFile() throws Exception { File pwFile = tempFolder.newFile("pw-notdir.txt"); Files.write(pwFile.toPath(), ("oldpass123\nnewpass456").getBytes(StandardCharsets.UTF_8)); + makeOwnerOnly(pwFile); StringWriter err = new StringWriter(); CommandLine cmd = new CommandLine(new Toolkit()); @@ -552,6 +568,7 @@ public void testUpdateWithOldMacLineEndings() throws Exception { File pwFile = tempFolder.newFile("cr.txt"); Files.write(pwFile.toPath(), (oldPassword + "\r" + newPassword + "\r").getBytes(StandardCharsets.UTF_8)); + makeOwnerOnly(pwFile); CommandLine cmd = new CommandLine(new Toolkit()); int exitCode = cmd.execute("keystore", "update", creds.getAddress(), @@ -584,6 +601,7 @@ public void testUpdateSkipsInvalidVersionKeystores() throws Exception { File pwFile = tempFolder.newFile("pw-badver.txt"); Files.write(pwFile.toPath(), (password + "\nnewpass789").getBytes(StandardCharsets.UTF_8)); + makeOwnerOnly(pwFile); StringWriter err = new StringWriter(); CommandLine cmd = new CommandLine(new Toolkit()); @@ -624,6 +642,7 @@ public void testUpdateRejectsTamperedAddressKeystore() throws Exception { File pwFile = tempFolder.newFile("pw-tampered.txt"); Files.write(pwFile.toPath(), (password + "\nnewpass789").getBytes(StandardCharsets.UTF_8)); + makeOwnerOnly(pwFile); StringWriter err = new StringWriter(); CommandLine cmd = new CommandLine(new Toolkit()); @@ -654,6 +673,7 @@ public void testUpdatePreservesCorrectDerivedAddress() throws Exception { File pwFile = tempFolder.newFile("pw-derived.txt"); Files.write(pwFile.toPath(), (oldPassword + "\n" + newPassword).getBytes(StandardCharsets.UTF_8)); + makeOwnerOnly(pwFile); CommandLine cmd = new CommandLine(new Toolkit()); int exitCode = cmd.execute("keystore", "update", originalAddress, @@ -702,6 +722,7 @@ public void testUpdateNarrowsLoosePermissionsTo0600() throws Exception { File pwFile = tempFolder.newFile("pw-perms.txt"); Files.write(pwFile.toPath(), (oldPassword + "\n" + newPassword).getBytes(StandardCharsets.UTF_8)); + makeOwnerOnly(pwFile); CommandLine cmd = new CommandLine(new Toolkit()); int exitCode = cmd.execute("keystore", "update", creds.getAddress(), @@ -738,6 +759,7 @@ public void testUpdateLegacyTipFiresWhenPasswordHasWhitespace() throws Exception File pwFile = tempFolder.newFile("pw-ws.txt"); Files.write(pwFile.toPath(), ("correct horse battery staple\nnewpass789").getBytes(StandardCharsets.UTF_8)); + makeOwnerOnly(pwFile); StringWriter err = new StringWriter(); CommandLine cmd = new CommandLine(new Toolkit()); @@ -769,6 +791,7 @@ public void testUpdateLegacyTipSuppressedWhenPasswordHasNoWhitespace() throws Ex File pwFile = tempFolder.newFile("pw-nows.txt"); Files.write(pwFile.toPath(), ("wrongpassword\nnewpass789").getBytes(StandardCharsets.UTF_8)); + makeOwnerOnly(pwFile); StringWriter err = new StringWriter(); CommandLine cmd = new CommandLine(new Toolkit()); @@ -808,6 +831,7 @@ public void testUpdateScanSkipsSymlinkedEntry() throws Exception { File pwFile = tempFolder.newFile("pw-update-sym.txt"); Files.write(pwFile.toPath(), (oldPassword + "\n" + newPassword).getBytes(StandardCharsets.UTF_8)); + makeOwnerOnly(pwFile); StringWriter err = new StringWriter(); CommandLine cmd = new CommandLine(new Toolkit()); @@ -820,4 +844,12 @@ public void testUpdateScanSkipsSymlinkedEntry() throws Exception { assertTrue("Scan must warn about the symlinked entry, got: " + err.toString(), err.toString().contains("Warning: skipping symbolic link: evil.json")); } + + private static void makeOwnerOnly(File f) throws IOException { + org.junit.Assume.assumeTrue("POSIX permissions required", + Files.getFileAttributeView(f.toPath(), + java.nio.file.attribute.PosixFileAttributeView.class) != null); + Files.setPosixFilePermissions(f.toPath(), + java.nio.file.attribute.PosixFilePermissions.fromString("rw-------")); + } }