Repository navigation
Conversation
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.
…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
| <trust file=".*-javadoc[.]jar" regex="true"/> | ||
| <trust file=".*-sources[.]jar" regex="true"/> | ||
| <trust group=".*" name=".*" version=".*-SNAPSHOT" regex="true"/> | ||
| </trusted-artifacts> |
There was a problem hiding this comment.
[MUST] Don't remove it.
There was a problem hiding this comment.
Restored per review. One correction: the first restore commit 03c88f1 mistakenly placed the block as a sibling of <configuration>, which broke Gradle verification-schema parsing and failed CI across the board. Fixed in 6b4f194 — trusted-artifacts now sits inside <configuration> right after verify-signatures, and the file is byte-identical to upstream. Verified locally with a Gradle run before pushing.
There was a problem hiding this comment.
Follow-up on the build supply-chain hardening scope: after re-evaluating the invasiveness of the broader changes, they have been migrated out of this PR — build.gradle is restored to upstream (JitPack repositories and mavenLocal() ordering), the distributionSha256Sum pin in gradle-wrapper.properties has been removed, and verification-metadata.xml is byte-identical to upstream with trusted-artifacts restored (094834c, 6b4f194).
The only supply-chain measure remaining in this PR is the CI-level wrapper-validation step in the workflows, which validates the gradle-wrapper.jar itself against Gradle's official checksums and carries no upgrade-maintenance coupling.
The other remediation approaches (repository allow-listing, distribution checksum pinning, tightening dependency verification) proved too risky to land safely in a patch-release hardening PR — they interact with the release build and local developer workflows in ways that need a dedicated discussion. We will address them in a separate follow-up rather than in this PR.
| File dir = tempFolder.newFolder("keystore"); | ||
| File pwFile = tempFolder.newFile("password.txt"); | ||
| Files.write(pwFile.toPath(), "test123456".getBytes(StandardCharsets.UTF_8)); | ||
| makeOwnerOnly(pwFile); |
There was a problem hiding this comment.
[SHOULD] Please avoid using Assume.assumeTrue() inside this shared helper. On filesystems without POSIX attributes, it skips the entire calling test, including otherwise platform-independent tests for keystore creation, import, and password updates. Production code explicitly supports these filesystems by skipping the permission check.
Please make makeOwnerOnly() a no-op when POSIX attributes are unavailable, and keep the assumption only in tests specifically verifying POSIX permissions. The same change applies to the helpers in KeystoreCliUtilsTest, KeystoreImportTest, and KeystoreUpdateTest.
There was a problem hiding this comment.
Fixed in c306405. The shared makeOwnerOnly helpers no longer call Assume.assumeTrue — on non-POSIX filesystems they now skip only the chmod step and return, mirroring the production fail-open path in KeystoreCliUtils (posixView == null → permission check skipped). This un-skips the ~69 platform-independent cases that consume the helper (CRLF/BOM/SM2/large-file/password-format across the 4 keystore test classes) on Windows. Tests asserting POSIX permission behavior keep their own method-level assumes, and the missing one was added to testReadRegularFileRejectsGroupReadable, which chmods unconditionally after the helper. Behavior on Linux CI is unchanged.
…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.
SeriousCoding789
left a comment
There was a problem hiding this comment.
Please update your PR description.
What does this PR do?
Six independent hardening changes, one commit each.
Keystore password input fails fast on EOF —
WalletUtils.inputPassword()now terminates withTronError(WITNESS_KEYSTORE_LOAD)instead of crashing with a raw NPE (Console.readPassword()returns null) or leakingNoSuchElementException(Scanner.nextLine()) when stdin closes mid-prompt. Covered byWalletUtilsInputPasswordTest.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 byWalletDecryptBoundsTest.CLI secret flag deprecated —
--private-keyis marked@Deprecatedand logs a WARN at startup when used (it exposes the witness key to the process list and shell history); README anddocs/configuration.mdpoint tolocalwitnesskeystoreinstead.--passwordis left untouched pending a real non-interactive alternative.gRPC connection defaults —
maxConnectionIdleInMillis/maxConnectionAgeInMillis= 0 now selects a built-in 60 s default instead ofLong.MAX_VALUE(unbounded); negative values fail fast withTronError; a half-configured RST window pair logs a loud WARN that flood protection is disabled. Covered byNodeConfigTest.Toolkit password/key file permissions —
--password-file/--key-filemust 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 requiredchmod 600. Covered byKeystoreCliUtilsTestand the keystore command tests.CI wrapper validation —
gradle/actions/wrapper-validationadded to all build workflows to verify the Gradle wrapper JAR on every run.Why are these changes required?
This PR has been tested by:
WalletDecryptBoundsTest— hostile keystores (oversized scrypt, overflow, huge pbkdf2), boundary-valid cases, and create-side boundsWalletUtilsInputPasswordTest— piped stdin EOFNodeConfigTest— defaults, explicit zero, half-configured pair, negative valuesKeystoreCliUtilsTest/KeystoreImportTest/KeystoreNewTest/KeystoreUpdateTest— permission gate and command flows:common:test :crypto:test :plugins:testfull suites green:framework:compileTestJavaplus targeted tests greenMoved out of this PR during review
protoc-gen-grpc-java1.60.0 → 1.83.0 unification — dedicated dependency-upgrade PR (1.60 stays pinned for CentOS 7 compatibility).--passworddeprecation + WARN — deferred until a non-interactive alternative (e.g.--password-file) exists.Follow up
DbMovefailure-path/symlink handling,start.shhardening — proposed separately.Extra details