fix(crypto): harden crypto module security and robustness#6652
Closed
Federico2014 wants to merge 5 commits intotronprotocol:developfrom
Closed
fix(crypto): harden crypto module security and robustness#6652Federico2014 wants to merge 5 commits intotronprotocol:developfrom
Federico2014 wants to merge 5 commits intotronprotocol:developfrom
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What does this PR do?
This PR hardens the security and robustness of the
cryptomodule across five areas:Remove unused
topicsListfrom shielded TRC20 log APIstopicsListfield fromScanShieldedTRC20NotesByIvkandScanShieldedTRC20NotesByOvkAPI responses, as it was never populatedand exposed misleading empty data.
api.proto,RpcApiService, HTTP servlet handlers, andWalletaccordingly.
api.protoremoves thetopicsListfield from shielded TRC20 scanresponses. This field was never populated by the server (always returned
empty), so existing clients will see no behavioral difference.
RpcApiServicesTest.Shielded transaction API security enhancement
Walletfor shielded transactionbuilders (
ZenTransactionBuilder,ShieldedTRC20ParametersBuilder).sprout-verifying.keyremoved from the repository root; the file wasnot referenced by any code and its presence in source control was
unnecessary.
allowShieldedTransactionApifromtrueto
falseto reduce exposure by default, since some shielded transactionAPIs require passing private keys as parameters. Updated
config.confwith an explicit warning advising users to only invoke these APIs locally.
ShieldedTransferActuatorTestandArgsTestto cover newvalidation paths.
SM2 deterministic signing and robustness (
SM2Signer.java)RandomDSAKCalculatorwithHMacDSAKCalculatorfordeterministic nonce generation (RFC 6979 equivalent), eliminating
nonce-reuse risk from the random K path.
init,generateSM3Hash,generateHashSignature,verifySignature,verifyHashSignature,calculateE).SM2KeyTestwith additional edge-input cases.ECKey input validation (
ECKey.java)fromPrivate,fromPublicOnly,and recovery APIs, preventing silent truncation of oversized key material.
ECKeyTestwith negative test cases for invalid inputs.Signature and private key validation (
Rsv.java,SignUtils.java,WitnessInitializer.java)r,s,vcomponents inRsv.SignUtilsbefore signing.WitnessInitializerto reject malformed private keys on nodestartup.
Why are these changes required?
The existing crypto code lacked defensive validation at API and library
boundaries, creating exposure to:
r,s,v)by default on public-facing nodes
These issues were identified during an internal security review of the
cryptomodule.This PR has been tested by:
ECKeyTest,SM2KeyTest,WalletMockTest,RpcApiServicesTest,ShieldedTransferActuatorTest,ArgsTest,WitnessInitializerTest./gradlew clean build -x test)Extra details
allowShieldedTransactionApidefaulting to
truemust now explicitly set it inconfig.conf.