fix(config,toolkit): harden keystore, grpc config and build supply chain by warku123 · Pull Request #6986 · tronprotocol/java-tron

warku123 · 2026-09-21T09:46:24Z

What does this PR do?

Six independent hardening changes.

Keystore password input fails fast on EOF — WalletUtils.inputPassword() now terminates with TronError(WITNESS_KEYSTORE_LOAD) instead of crashing with a raw NPE (Console.readPassword() returns null) or leaking NoSuchElementException (Scanner.nextLine()) when stdin closes mid-prompt. Covered by WalletUtilsInputPasswordTest.
Keystore KDF parameter bounds — create/decrypt/validate now enforce bounds on untrusted keystore KDF parameters before any KDF runs: scrypt n power-of-2 in [4096, 1048576], r<=8, p<=8, 128rn<=1 GiB, dklen in [32,128]; pbkdf2 c<=2^20. Wallet.create(...) applies the same checks so a keystore cannot be created that a later decrypt would reject. Prevents huge allocations/OOM, integer-overflow exceptions, and multi-hour CPU hangs from hostile keystore files. Covered by WalletDecryptBoundsTest.
CLI secret flag deprecated — --private-key is marked @Deprecated and logs a WARN at startup when used (it exposes the witness key to the process list and shell history); README and docs/configuration.md point to localwitnesskeystore instead. --password is left untouched pending a real non-interactive alternative.
gRPC connection defaults — the default maxConnectionIdleInMillis is now 60000 (60 s), set in both reference.conf and the NodeConfig field initializer, so idle connections are reclaimed by default. Zero semantics are unchanged: an explicit 0 still means unlimited, and maxConnectionAgeInMillis keeps its legacy meaning (0 = unlimited). Covered by NodeConfigTest.
Toolkit password/key file permissions — --password-file/--key-file must now be owner-only (no group/other access) on POSIX systems; otherwise the tool refuses the file with an actionable error message. Docs note the required chmod 600. Covered by KeystoreCliUtilsTest and the keystore command tests.
CI wrapper validation — gradle/actions/wrapper-validation added to all build workflows to verify the Gradle wrapper JAR on every run.

Why are these changes required?

Untrusted keystore files are attacker-controlled input: unbounded KDF cost parameters turn a single decrypt call into memory exhaustion or multi-hour CPU consumption, and malformed inputs escape as raw runtime exceptions.
Password entry must fail with an actionable error, not an NPE, and secrets should not be passed on the command line.
The Gradle wrapper JAR is a build-time supply-chain entry point and should be verified in CI.

This PR has been tested by:

Unit Tests
- WalletDecryptBoundsTest — hostile keystores (oversized scrypt, overflow, huge pbkdf2), boundary-valid cases, and create-side bounds
- WalletUtilsInputPasswordTest — piped stdin EOF
- NodeConfigTest — default maxConnectionIdleInMillis from reference.conf and explicit 0 for idle/age meaning unlimited; ConfigParityGateTest keeps bean and config defaults aligned
- KeystoreCliUtilsTest / KeystoreImportTest / KeystoreNewTest / KeystoreUpdateTest — permission gate and command flows
- :common:test :crypto:test :plugins:test full suites green
Manual Testing — clean-clone :framework:compileTestJava plus targeted tests green

Moved out of this PR during review

protoc-gen-grpc-java 1.60.0 → 1.83.0 unification — dedicated dependency-upgrade PR (1.60 stays pinned for CentOS 7 compatibility).
--password deprecation + WARN — deferred until a non-interactive alternative (e.g. --password-file) exists.

Follow up

Not included: Toolkit DbMove failure-path/symlink handling, start.sh hardening — proposed separately.

Extra details

No behavior change for keystores within the bounds, for explicitly positive gRPC settings, for interactive password entry, or for successful builds.

inputPassword() now throws TronError(WITNESS_KEYSTORE_LOAD) when the password source closes before a line is available: the TTY branch null-checks Console.readPassword() (EOF raised a raw NullPointerException) and the non-TTY branch catches Scanner NoSuchElementException from a piped stdin EOF. Add piped-EOF test coverage; the TTY null-check is review-only (System.console() is always null under JUnit).

Untrusted keystore KDF cost parameters previously drove Bouncy Castle into NegativeArraySizeException (huge scrypt n), a ~4GiB allocation/OOM (n=2^22,r=8), ArithmeticException (r*n overflow), or a multi-hour pbkdf2 CPU hang. validationError() now enforces bounds as the single chokepoint shared by validate() and isValidKeystoreFile(): scrypt n power-of-2 in [2^12,2^20], r<=8, p<=8, 128*r*n<=1GiB (long arithmetic), dklen<=128; pbkdf2 c<=2^20, dklen<=128; missing/malformed kdfparams rejected. decrypt() additionally wraps both KDF invocations so residual RuntimeExceptions surface as CipherException (Errors stay fatal). Add hostile-keystore and boundary-valid test coverage.

@deprecated

…skeystore Passing key material on the command line (-p/--private-key, --password) exposes it in the process list and shell history, and plaintext keys in the localwitness config option are readable by anyone with access to the config file. Mark both CLI flags @deprecated (scheduled for removal in a future release, non-breaking for now) and log actionable warnings when these plain-key paths are used at witness startup, pointing users to the encrypted localwitnesskeystore. Update README and configuration docs accordingly.

…-configured RST pair

… tighten dependency sources - gradle-wrapper.properties: add official distributionSha256Sum for Gradle 7.6.4 bin zip (bed1da33cca0f557ab13691c77f38bb67388119e4794d113e051039b80af9bb1) - CI: run gradle/actions/wrapper-validation before any ./gradlew invocation in pr-build, pr-check, codeql, release-build, and integration-test workflows - build.gradle: move mavenLocal() after mavenCentral and drop the JitPack repository from both buildscript and subprojects blocks; all com.github.* dependencies (jsonrpc4j, java-semver, metrics-influxdb, software-and-algorithms) resolve from Maven Central - verification-metadata.xml: remove blanket SNAPSHOT trust rule and the javadoc/sources artifact trust rules; strict checksum verification now applies to every artifact

… image off CentOS 7 Use rootProject.grpcVersion (1.83.0) for protoc-gen-grpc-java on all architectures; the 1.60.0 pin only existed for the EOL CentOS 7 docker base (grpc-java#11371). Rebase the image onto rockylinux:8 (matching CI) and install JDK 8 via dnf (java-1.8.0-openjdk-devel) instead of the MD5-only verified Oracle 8u202 tarball; strip the devel package after the build so the runtime keeps only the JRE. Sync verification metadata: drop the 1.60.0 component, add protoc-gen-grpc-java-1.83.0-linux-x86_64.exe sha256 from Maven Central.

- readRegularFile now rejects files readable/writable by group or other on POSIX filesystems - covers --password-file (KeystoreNew/KeystoreImport/KeystoreUpdate) and --key-file (KeystoreImport) inputs - non-POSIX filesystems are skipped (view == null)

…ore API and docs - Dockerfile: install 'which' (gradlew and bin/FullNode launcher need it); keep headless JRE when removing openjdk-devel (clean_requirements_on_remove=0) - NodeConfig: reject negative maxConnectionIdleInMillis/maxConnectionAgeInMillis at config validation instead of failing late in the Netty builder - Args: warn on whitespace-only --password as well (isNotEmpty) - Wallet.create: validate scrypt n/p bounds, symmetric with validate()/decrypt() - docs/configuration.md + reference.conf: 0 means secure default (60 s), not 'no limit' - plugins/README.md: document owner-only (chmod 600) requirement for --password-file/--key-file - tests: negative idle/age rejection; create() bounds rejection + roundtrip

Per review feedback, docker image/entrypoint changes will be handled in a dedicated ops-side change instead of this audit-fix PR.

Per review feedback: --password has no replacement yet (interactive input cannot run headless), so deprecating it is premature. Keep the startup WARN; re-deprecate once --password-file or equivalent lands.

Reviewer prefers no nagging until a real alternative exists; the WARN returns together with the deprecation once --password-file lands.

…ies-audit-v4.8.3-r483 # Conflicts: # gradle/verification-metadata.xml

…view Restore the blanket trust rules (javadoc/sources jars and *-SNAPSHOT) removed in BUILD-01: they are load-bearing for IDE source/javadoc attachment and internal SNAPSHOT workflows even though CI does not exercise them.

…cation schema

…review

…ystems

SeriousCoding789

Please update your PR description.

halibobo1205 · 2026-10-10T11:37:09Z

    }
    if (rpc.maxConnectionAgeInMillis == 0) {
-      rpc.maxConnectionAgeInMillis = Long.MAX_VALUE;
+      rpc.maxConnectionAgeInMillis = RpcConfig.DEFAULT_MAX_CONNECTION_LIFETIME_IN_MILLIS;


[SHOULD] This changes the max connection age on every default node, not only on misconfigured ones.

4.8.2.3: maxConnectionAgeInMillis = 0 (the reference.conf default; the shipped config.conf sets only maxConnectionIdleInMillis) became Long.MAX_VALUE, i.e. no max age. reference.conf documented it as "0 = no limit".

Here: 0 becomes 60 000 ms. So after upgrade, every FullNode/Solidity/PBFT gRPC port sends GOAWAY to each connection after about 60 s (grpc-java adds ±10% jitter). An operator who explicitly wrote 0 to mean "no limit" silently gets the opposite.

The description mentions the new 0 → 60 s mapping, but nothing tells an operator at runtime or at upgrade time. maxConcurrentCallsPerConnection = 0 went through the same kind of change in 2fd5455 and got a WARN (L371-376) plus an "Upgrade note" in common/src/main/java/org/tron/core/config/README.md and docs/configuration.md. This change has neither, and that README's sample still shows # maxConnectionAgeInMillis = with no default.

Suggestion: apply the 60 s default to maxConnectionIdleInMillis only, and keep age 0 = disabled. If a bounded age is really wanted:

use separate idle/age constants;

pick a minutes-scale age and write it visibly in reference.conf/config.conf;

WARN when an explicit 0 is reinterpreted;

add an Upgrade note that says how to get "unlimited" back (grpc-java treats any value ≥ 1000 days, e.g. 86400000000, as infinite).

Addressed in f55e3f4: the 60 s default now applies to maxConnectionIdleInMillis only; maxConnectionAgeInMillis=0 keeps the legacy unlimited semantics (mapped to Long.MAX_VALUE, which Netty treats as max-age GOAWAY fully disabled). Negative values still rejected; explicit positive values honored. reference.conf comment updated accordingly.

…e default maxConnectionAgeInMillis=0 restores legacy semantics (unlimited) instead of forcing a 60s max age, which would churn active connections on every default node. Idle timeout keeps the 60s secure default; negative values still rejected; positive overrides honored. Per review.

…ies-audit-v4.8.3-r483 # Conflicts: # plugins/src/test/java/org/tron/plugins/KeystoreImportTest.java # plugins/src/test/java/org/tron/plugins/KeystoreNewTest.java # plugins/src/test/java/org/tron/plugins/KeystoreUpdateTest.java

…ies-audit-v4.8.3-r483 # Conflicts: # framework/src/test/java/org/tron/keystore/WalletUtilsInputPasswordTest.java # framework/src/test/java/org/tron/program/KeystoreFactoryDeprecationTest.java

…edResponseWrapperTest)

halibobo1205 · 2026-10-12T09:38:56Z

      rpc.maxConcurrentCallsPerConnection =
          RpcConfig.DEFAULT_MAX_CONCURRENT_CALLS_PER_CONNECTION;
    }
+    // node.rpc.maxRstStream and node.rpc.secondsPerWindow only take effect


Checked by #7015, cc @waynercheung .

Done — dropped our RST window validation and its tests in 3a3794b since #7015 covers it more completely. Kept the idle/age defaults and negative checks, which #7015 doesn't touch.

…7015

halibobo1205

This branch has conflicts that must be resolved.

…ies-audit-v4.8.3-r483

Federico2014 · 2026-10-13T10:32:10Z

+  // ---------- decrypt wrap: in-bounds but corrupted keystore ----------
+
+  @Test
+  public void testDecryptCorruptedKeystoreThrowsCipherException() throws Exception {


[NIT] This test exercises a MAC mismatch after successful key derivation, so it would still pass if the new catch (RuntimeException) block were removed.

Please add a case that exercises the exception wrapping itself. For example, n=65536, r=1, p=1, dklen=32 passes the current validation but causes SCrypt.generate() to throw IllegalArgumentException. Assert that decrypt() throws CipherException with the message "Scrypt key derivation failed" and preserves the original cause.

Added in afbe671 (testDecryptWrapsScryptRuntimeException): builds an in-bounds keystore (dklen=32, r=1, p=1), asserts decrypt() throws CipherException with message "Scrypt key derivation failed" and the original IllegalArgumentException preserved as cause.

Used n=131072 instead of 65536 — both values trigger BouncyCastle's r==1 / N>=65536 rejection, 131072 just sits deeper inside the throw region so the test stays robust to any future boundary tightening.

Also mutation-checked: with the catch block removed the test fails (raw IAE escapes), so it genuinely pins the wrapping.

…semantics

…imeout at connection limit, tronprotocol#6871 docs/AGENTS.md)

warku123 · 2026-10-14T06:48:47Z

Final state of the maxConnectionIdleInMillis default change

Following the review feedback, the gRPC part of this PR ended up as a pure default-value change — no runtime validation semantics were added or removed:

Default maxConnectionIdleInMillis is 60000 (60 s), aligned in both reference.conf and the NodeConfig field initializer (the config-parity test requires the two to match).
An explicit 0 still means unlimited — postProcess() maps it to Long.MAX_VALUE, exactly as before.
maxConnectionAgeInMillis is untouched: default 0 = unlimited, connections are never retired by age.
The negative-value rejection and the half-configured RST-pair WARN originally proposed here were dropped — RST limiting landed separately in fix(rpc): apply RST_STREAM frame limit by default #7015.

Tests: NodeConfigTest covers the reference.conf default (60000) and explicit-zero unlimited for both idle and age; ConfigParityGateTest passes with the aligned defaults.

The PR description has also been updated in sync to reflect this final state.

* release_v4.8.3: chore(deps): upgrade jsonrpc4j from 1.6 to 1.7 (tronprotocol#7030) fix(config,toolkit): harden keystore, grpc config and build supply chain (tronprotocol#6986)

warku123 added 14 commits September 15, 2026 17:48

fix(config): harden gRPC connection lifetime default and warn on half…

d8b8bd1

…-configured RST pair

fix(docker): stop persisting full node command line to command.txt

f4f946e

fix(build): restore ProtocGenVersion 1.60 pin for centos7 compatibility

7c67b40

revert(docker): drop docker-side changes from this PR

e963859

Per review feedback, docker image/entrypoint changes will be handled in a dedicated ops-side change instead of this audit-fix PR.

fix(cli): drop @deprecated from --password pending a real alternative

fbe5bf9

Per review feedback: --password has no replacement yet (interactive input cannot run headless), so deprecating it is premature. Keep the startup WARN; re-deprecate once --password-file or equivalent lands.

fix(cli): drop --password startup WARN per review

52971f5

Reviewer prefers no nagging until a real alternative exists; the WARN returns together with the deprecation once --password-file lands.

Merge remote-tracking branch 'upstream/release_v4.8.3' into fix/g-ser…

fa5b0cb

…ies-audit-v4.8.3-r483 # Conflicts: # gradle/verification-metadata.xml

github-actions Bot requested review from 317787106 and halibobo1205 September 21, 2026 09:46

warku123 mentioned this pull request Sep 21, 2026

fix(config,toolkit): harden keystore, grpc config and build supply chain warku123/java-tron#17

Closed

SeriousCoding789 added this to java-tron Sep 22, 2026

SeriousCoding789 added this to the GreatVoyage-v4.8.3 milestone Sep 22, 2026

317787106 reviewed Sep 23, 2026

View reviewed changes

Comment thread gradle/verification-metadata.xml

317787106 reviewed Sep 23, 2026

View reviewed changes

Comment thread plugins/src/test/java/org/tron/plugins/KeystoreNewTest.java

warku123 added 4 commits September 23, 2026 11:42

fix(build): place trusted-artifacts inside <configuration> per verifi…

6b4f194

…cation schema

fix(build): restore upstream build.gradle and wrapper properties per …

094834c

…review

test(toolkit): run platform-independent keystore tests on non-POSIX s…

c306405

…ystems

SeriousCoding789 self-requested a review October 1, 2026 01:49

SeriousCoding789 approved these changes Oct 1, 2026

View reviewed changes

warku123 requested a review from 317787106 October 7, 2026 14:48

halibobo1205 reviewed Oct 10, 2026

View reviewed changes

warku123 added 2 commits October 12, 2026 14:01

Merge remote-tracking branch 'upstream/release_v4.8.3' into fix/g-ser…

954b777

…ies-audit-v4.8.3-r483 # Conflicts: # framework/src/test/java/org/tron/keystore/WalletUtilsInputPasswordTest.java # framework/src/test/java/org/tron/program/KeystoreFactoryDeprecationTest.java

warku123 requested a review from halibobo1205 October 12, 2026 09:14

chore: retrigger CI (flaky debian11 test JVM crash in upstream Buffer…

0bc24b8

…edResponseWrapperTest)

halibobo1205 reviewed Oct 12, 2026

View reviewed changes

refactor(config): drop rst window validation covered by tronprotocol#…

3a3794b

…7015

warku123 requested a review from halibobo1205 October 12, 2026 10:55

halibobo1205 reviewed Oct 13, 2026

View reviewed changes

SeriousCoding789 requested a review from Federico2014 October 13, 2026 09:35

Merge remote-tracking branch 'upstream/release_v4.8.3' into fix/g-ser…

f2dbe6c

…ies-audit-v4.8.3-r483

warku123 requested a review from halibobo1205 October 13, 2026 09:56

halibobo1205 approved these changes Oct 13, 2026

View reviewed changes

Federico2014 reviewed Oct 13, 2026

View reviewed changes

test(keystore): cover scrypt runtime-exception wrapping in decrypt

afbe671

Federico2014 approved these changes Oct 13, 2026

View reviewed changes

warku123 added 3 commits October 14, 2026 12:01

fix(config): set recommended grpc idle timeout without changing zero …

23d96dc

…semantics

Merge upstream/release_v4.8.3 (a901f85: tronprotocol#7020 http idle t…

6bdc458

…imeout at connection limit, tronprotocol#6871 docs/AGENTS.md)

fix(config): align grpc idle default with reference config

cb2d9ca

317787106 approved these changes Oct 14, 2026

View reviewed changes

kuny0707 merged commit a5f43f7 into tronprotocol:release_v4.8.3 Oct 14, 2026
13 checks passed

github-project-automation Bot moved this to Done in java-tron Oct 14, 2026

warku123 deleted the fix/g-series-audit-v4.8.3-r483 branch October 14, 2026 07:11

Navigation Menu

Repository navigation

fix(config,toolkit): harden keystore, grpc config and build supply chain - #6986

fix(config,toolkit): harden keystore, grpc config and build supply chain#6986
kuny0707 merged 28 commits into
tronprotocol:release_v4.8.3from
warku123:fix/g-series-audit-v4.8.3-r483

warku123 commented Sep 21, 2026 •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

SeriousCoding789 left a comment

Uh oh!

halibobo1205 Oct 10, 2026

Uh oh!

warku123 Oct 12, 2026

Uh oh!

halibobo1205 Oct 12, 2026

Uh oh!

warku123 Oct 12, 2026

Uh oh!

halibobo1205 left a comment

Uh oh!

Federico2014 Oct 13, 2026 •

edited

Loading

Uh oh!

warku123 Oct 13, 2026

Uh oh!

warku123 commented Oct 14, 2026 •

edited

Loading

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

Conversation

warku123 commented Sep 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

SeriousCoding789 left a comment

Choose a reason for hiding this comment

Uh oh!

halibobo1205 Oct 10, 2026

Choose a reason for hiding this comment

Uh oh!

warku123 Oct 12, 2026

Choose a reason for hiding this comment

Uh oh!

halibobo1205 Oct 12, 2026

Choose a reason for hiding this comment

Uh oh!

warku123 Oct 12, 2026

Choose a reason for hiding this comment

Uh oh!

halibobo1205 left a comment

Choose a reason for hiding this comment

Uh oh!

Federico2014 Oct 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

warku123 Oct 13, 2026

Choose a reason for hiding this comment

Uh oh!

warku123 commented Oct 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

warku123 commented Sep 21, 2026 •

edited

Loading

Federico2014 Oct 13, 2026 •

edited

Loading

warku123 commented Oct 14, 2026 •

edited

Loading