[Feature] Validate ECKey inputs and improve key handling
I maintainer di solito rispondono entro 1 giorno
Valutazione
- Difficoltà
- 4/5
- Tempo stimato
- 3-5 giorni
- Idoneità per principianti
- 40/100
- Tipo di issue
- Funzionalità
- Chiarezza
- Specificata chiaramente
- Stato di attività
- Attiva
- Stack tecnologico
- java
- Ambito
- cryptography, security
Direzione di ricerca
The ECKey class in the codebase handles secp256k1 key pairs. Start by locating the ECKey class and its constructors, focusing on private and public key validation methods. Review the keystore decryption path for temporary buffer cleanup. Write tests to verify input validation, equality symmetry, and that signing/recovery results remain unchanged for valid inputs. Check that modifications to returned arrays from getAddress() and getNodeId() do not affect cached values.
Scritto dal modello di indicizzazione a partire dal testo della issue.
Descrizione
Summary
Validate keys when they enter ECKey, protect its cached values from accidental modification, and clear temporary private-key bytes after keystore decryption.
What is the problem?
ECKey represents a secp256k1 key pair and is used for operations such as transaction and witness signing. Several entry points handle invalid input or key state inconsistently:
- Invalid keys can fail too late. Private-key input is not consistently checked for size and numeric range before conversion or curve calculations. Public-key construction does not consistently enforce the supported encoding, curve, and point-validity rules.
- A private key can be paired with the wrong public key. For example, an object can contain private key A and public key B. Its address is derived from B, while signature generation uses A.
sign()then tries to match the recovered public key to B; if no match is found, it throws. This can cause recoverable signing to fail after the object has already been constructed. - Callers can change cached values.
getAddress()andgetNodeId()return their internal arrays. Modifying a returned array also changes what the object returns later. - Equality can be asymmetric. A public-only key and a full key pair can represent the same public key, yet
publicOnly.equals(fullKey)can betruewhilefullKey.equals(publicOnly)isfalse. This violates the symmetry required by Java'sequals()contract. - Some APIs promise unsupported behavior or expose secrets. Constructors accept arbitrary cryptographic providers, although signing expects Bouncy Castle keys.
toStringWithPrivate()includes the private key in its output and can expose it if logged. - Keystore errors and cleanup need a consistent boundary. ECDSA private-key validation failures during keystore decryption can escape as
IllegalArgumentExceptioninstead ofCipherException. The temporary array containing the decrypted private key is not explicitly cleared.
Expected behavior
| Area | Required behavior |
|---|---|
| Private keys | Interpret byte arrays as unsigned big-endian integers and check their length before numeric conversion. Accept only values in 1 <= key < n, where n is the secp256k1 group order. Private-key import methods reject null and empty input. |
| Public keys | Accept 33-byte encodings starting with 0x02 or 0x03, or 65-byte encodings starting with 0x04. Decode bytes on secp256k1 and reject invalid points and the point at infinity. For ECPoint input, also verify that its curve is secp256k1. |
| Key pairs | When both components are supplied, verify that the public key is derived from the private key. Preserve public-only construction with a valid public key and no private key. |
| Cached values and equality | Return copies of address and node ID arrays. Define equality by public-key identity, consistently with hashCode(), regardless of private-key presence or the public key's input encoding. |
| Providers and helper APIs | Use Bouncy Castle for key generation and signing. Remove APIs that bypass key-pair checks or include private keys in string output. |
| Keystore decryption | Wrap ECDSA private-key validation failures in CipherException, preserving the cause. Clear the temporary decrypted private-key array on success, private-key validation failure, and declared-address mismatch. Shared cleanup applies to both ECDSA and SM2 decryption. |
Private-key byte arrays of 1–32 bytes remain supported when their value is valid. Also accept a valid 33-byte representation produced by BigInteger.toByteArray(): one leading 0x00 followed by a 32-byte magnitude whose high bit is set. That extra byte marks a positive number; it does not make the key invalid. Other encodings longer than 32 bytes are rejected.
The validation scope includes these helper entry points:
publicKeyFromPrivate()validates the private scalar before deriving the public key.fromNodeId()requires a 64-byte X/Y representation and applies public-key validation throughfromPublicOnly().signatureToKey()andrecoverFromSignature()apply the strengthened public-key invariants when constructing a returnedECKey.
These requirements do not imply identical exception types for every malformed argument to every helper. In particular, the existing fromNodeId(null) behavior is not being standardized by this change.
Clearing the temporary array reduces how long that copy remains in memory. The returned key object still retains the private key needed for signing.
Compatibility and migration
This changes the public Java API. Downstream callers using removed methods must update their code and recompile. Changes to equality and returned-array sharing can also affect callers that still compile successfully.
| Existing usage | Required change |
|---|---|
Provider-based ECKey constructors |
Use ECKey(SecureRandom) for generation, or the supported import constructors/factories. Arbitrary-provider support is removed. |
fromPrivateAndPrecalculatedPublic |
Use the validated ECKey(BigInteger, ECPoint) constructor when supplying both components. |
toStringWithPrivate() |
Removed without a private-key-printing replacement. Use toString() for diagnostic output. |
Expecting fromPrivate(byte[]) to return null for null or empty input |
Validate the input or handle IllegalArgumentException. |
| Handling invalid decrypted ECDSA keystore keys | Handle CipherException; the original validation exception is retained as its cause. |
Distinguishing public-only and full keys using equals(), sets, or map keys |
Both now have the same identity when their public keys match. Check private-key availability separately if needed. |
Relying on shared arrays from getAddress() or getNodeId() |
Returned arrays are independent copies. Modifying them no longer changes the object's cached value; use content comparison rather than array reference identity. |
Preserve the existing signing and recovery results for valid inputs. This work must not change transaction/block signature acceptance or TVM ecrecover behavior. Recovery helpers that return ECKey must enforce the strengthened public-key invariants, so malformed inputs may be rejected differently.
Configuration, consensus rules, and on-chain behavior remain unchanged. SM2 algorithm and key-validation changes are outside this issue; temporary-buffer cleanup applies to the shared keystore decryption path.
Validation
The acceptance criteria are:
- Input boundaries: Cover invalid scalar values, unsigned byte interpretation, oversized encodings, and valid 33-byte sign-padded keys. Reject malformed public-key encodings, invalid points, points on another curve supplied as
ECPoint, and mismatched key pairs. Accept valid pairs and public-only construction. ExercisepublicKeyFromPrivate()andfromNodeId()as well as constructors and import factories. - Equality and cached values: Check equality in both directions between public-only and full keys, and between keys constructed from compressed and uncompressed encodings. Equal keys must have equal hash codes. Modifying returned address or node ID arrays must not change cached values.
- Signing and recovery: Verify signing and recovery results for valid inputs, including existing signature vectors. Verify that
signatureToKey()andrecoverFromSignature()reject recovered public points that violate the new invariants. Cover transaction/block signature acceptance and TVMecrecovercompatibility. - Real invalid ECDSA keystores: Use a correct password, a valid MAC, and decryptable ciphertext whose plaintext is an invalid private scalar, such as zero or
n. Confirm that failure comes from private-key validation and is wrapped inCipherException, rather than from password, MAC, or file errors. - Temporary-buffer cleanup: Inspect the actual decrypted array after success and declared-address mismatch for both ECDSA and SM2, and after ECDSA private-key validation failure. Mocking or instrumentation may be used to observe that array, but must not replace all real invalid-key import coverage.
- Witness loading: Load a keystore containing a real invalid ECDSA private key through the witness initialization path. Confirm
WITNESS_KEYSTORE_LOADwithCipherExceptionand the original validation exception in the cause chain. Keep the existingWitnessInitializerproduction logic; an injected exception alone does not satisfy this end-to-end acceptance case.
- Lingua principale
- Java
- Stelle
- 4.2k
- Fork
- 1.8k
- Merge medio
- 3g 21h
- PR unite (30g)
- 13
Preparare l'ambiente
- Nessun Dockerfile né file Docker Compose
- Ha un modello di pull request
- Leggi la guida per i contributori
Come iniziare
- Leggi tutta la issue e poi la guida ai contributi del progetto.
- Commenta sulla issue per dire che te ne occupi tu — evita che due persone facciano lo stesso lavoro.
- Fai un fork del repository e lavora su un branch.
- Apri una pull request che faccia riferimento al numero della issue.
Altre issue di tronprotocol/java-tron
-
type:feature
Difficoltà 4/5 3-5 giorni Idoneità per principianti 35/100
tronprotocol/java-tron#7013 ·
I maintainer di solito rispondono entro 1 giorno
-
type:feature
Difficoltà 5/5 Più di una settimana Idoneità per principianti 25/100
tronprotocol/java-tron#6989 · 2 commenti ·
I maintainer di solito rispondono entro 1 giorno
-
Robustness fixes for multiple issuesForse già presa @xxo1shine l’ha presa 17 giorni fa. Aperta
Difficoltà 5/5 Più di una settimana Idoneità per principianti 35/100
tronprotocol/java-tron#6969 · 8 commenti ·
I maintainer di solito rispondono entro 1 giorno
-
[Feature] decouple json-rpc filter processing from ManagerForse già presa @0xbigapple l’ha presa 16 giorni fa. Apertatype:feature
Difficoltà 5/5 Più di una settimana Idoneità per principianti 48/100
tronprotocol/java-tron#6963 · 8 commenti ·
I maintainer di solito rispondono entro 1 giorno
-
[Feature] Remove unused SM2/SM3 crypto engineForse già presa @Federico2014 l’ha presa 15 giorni fa. Apertatype:feature
Difficoltà 5/5 Più di una settimana Idoneità per principianti 28/100
tronprotocol/java-tron#6959 · 3 commenti ·
I maintainer di solito rispondono entro 1 giorno
Tutte le issue di tronprotocol/java-tron
Issue simili
-
backend
Difficoltà 2/5 1-3 ore Idoneità per principianti 76/100
bcgov/nr-forest-client#2524 ·
I maintainer di solito rispondono entro 1 giorno
-
Difficoltà 2/5 1-3 ore Idoneità per principianti 67/100
I maintainer di solito rispondono entro 1 giorno
-
Difficoltà 1/5 Meno di un'ora Idoneità per principianti 74/100
I maintainer di solito rispondono entro 1 giorno
-
team:Lumberjack
Difficoltà 2/5 1-3 ore Idoneità per principianti 76/100
OpenLiberty/open-liberty#35998 ·
I maintainer di solito rispondono entro 1 giorno
-
[BUG] SQS SendMessageBatch accepts more than 10 entries instead of TooManyEntriesInBatchRequestForse già presa Una pull request collegata a questa issue è aperta o già unita. Aperta
Difficoltà 2/5 1-3 ore Idoneità per principianti 67/100
floci-io/floci#5319 · 1 commento ·
I maintainer di solito rispondono entro 1 giorno