hiero-ledger / hiero-ledger/hiero-consensus-node
Bug: Local key can be overwritten on failed rekey operation
- Dominant language
- Java
- Stars
- 406
- Forks
- 226
- Avg merge
- 3d 4h
- Merged PRs (30d)
- 210
Description
## Description
During a Yahcli rekey operation, if the rekey op fails, repetition of the rekey command leads to _complete erasure_ of the original key on disk. We need to modify Yahcli to ensure Yahcli never replaces an existing key on disk, and that it restores the original key in the event of a failed operation.
## Bug Reproduction
Assume an account _X_ exists on a network, with a corresponding key file `accountX.pem` on local disk. Our goal is to rekey _X_ with the following command:
```
docker run -it -v $(pwd):/launch gcr.io/hedera-registry/yahcli:0.68 \
--fixed-fee 100000000 --node-account 3 --network TheNetwork -p 950 accounts rekey -g 12345
```
where 'TheNetwork' is the network we'll submit the transaction to, and '12345' is the account number.
The following sequence of events leads to the irrecoverable loss of `account12345.pem` on local disk:
* User invokes the command
* Yahcli renames `account12345.pem` to `account12345.pem.backup`
* Yahcli generates a new key in memory and writes the new key to disk as `account12345.pem`
* Yahcli generates the crypto update transaction to rotate the old key; it signs with the original key (also loaded in memory) and the new generated key
* Yahcli attempts to submit the transaction, but fails with an uncaught exception–e.g. an invalid node account ID was used in the command, or an account's balance wasn't available for query
* User sees output similar to he following:
```
.i. Exported a newly generated key in PEM format to TheNetwork/keys/account12345.pem
.!. FAILED to re-key account 0.0.12345
```
* User invokes the rekey command _again_ for account 12345
**At this point, the original `account12345.pem` file is no longer present on disk!**
## Explanation
What happens in this example is the original account key, which was renamed `account12345.pem.backup`, is never restored to `account12345.pem` _because of the uncaught exception_.
Yahcli's implementation reads keys from disk, so a rekey operation currently requires to first run a key generation operation. The new key generation op creates a new key and writes it to disk as `accountXXX.pem`. The rekey operation can then simply read the existing `accountXXX.pem` file from disk when generating the crypto update transaction. However, because the operation fails with an _uncaught_ exception, both files remain on disk–the original key as `account12345.pem.backup`, and the new key as `account12345.pem`. Therefore, when the user invokes the rekey operation again, Yahcli will rename the current `account12345.pem`–which contains the new key from the _first_ invocation–to `account12345.pem.backup`, _thus overwriting the original key file_. Any further attempts to use any of the new keys generated prior to the rekey update will of course fail since the private key in `account12345.pem` has changed, but the network has not yet executed the crypto update transaction, and therefore expects the original private key.
Contributor guide
Research direction
Start with Yahcli's accounts rekey command and trace the rename, key write, and transaction submission sequence described in the reproduction, including account12345.pem and its .backup file. Done means a failed rekey never overwrites an existing local key and restores the original key; rerun the supplied Docker command to verify the failure path.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- java
- Domain
- cli, security
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Stale
- Clarity
- Mostly clear
- Newbie friendliness
- 45/100