Skip to content

Commit d7f7c6b

Browse files
BarbatosBarbatos
Barbatos
authored and
Barbatos
committed
fix(plugins): block duplicate import, improve error messages and security tips
- Reject duplicate-address import by default, add --force flag to override - Add friendly error messages when --password-file or --key-file not found - Print security tips after keystore new/import (aligned with geth output) - Add file existence checks in KeystoreCliUtils and KeystoreUpdate
1 parent 6c9fab2 commit d7f7c6b

6 files changed

Lines changed: 127 additions & 16 deletions

File tree

‎plugins/src/main/java/common/org/tron/plugins/KeystoreCliUtils.java‎

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,11 @@ private KeystoreCliUtils() {
3030

3131
static String readPassword(File passwordFile) throws IOException {
3232
if (passwordFile != null) {
33+
if (!passwordFile.exists()) {
34+
System.err.println("Password file not found: " + passwordFile.getPath()
35+
+ ". Omit --password-file for interactive input.");
36+
return null;
37+
}
3338
if (passwordFile.length() > MAX_FILE_SIZE) {
3439
System.err.println("Password file too large (max 1KB).");
3540
return null;
@@ -133,6 +138,33 @@ static String stripLineEndings(String s) {
133138
return s.substring(0, end);
134139
}
135140

141+
static boolean checkFileExists(File file, String label) {
142+
if (file != null && !file.exists()) {
143+
System.err.println(label + " not found: " + file.getPath());
144+
return false;
145+
}
146+
return true;
147+
}
148+
149+
static void printSecurityTips(String address, String fileName) {
150+
System.out.println();
151+
System.out.println("Public address of the key: " + address);
152+
System.out.println("Path of the secret key file: " + fileName);
153+
System.out.println();
154+
System.out.println(
155+
"- You can share your public address with anyone."
156+
+ " Others need it to interact with you.");
157+
System.out.println(
158+
"- You must NEVER share the secret key with anyone!"
159+
+ " The key controls access to your funds!");
160+
System.out.println(
161+
"- You must BACKUP your key file!"
162+
+ " Without the key, it's impossible to access account funds!");
163+
System.out.println(
164+
"- You must REMEMBER your password!"
165+
+ " Without the password, it's impossible to decrypt the key!");
166+
}
167+
136168
static void setOwnerOnly(File file) {
137169
try {
138170
Files.setPosixFilePermissions(file.toPath(), OWNER_ONLY);

‎plugins/src/main/java/common/org/tron/plugins/KeystoreImport.java‎

Lines changed: 22 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -43,9 +43,16 @@ public class KeystoreImport implements Callable<Integer> {
4343
description = "Use SM2 algorithm instead of ECDSA")
4444
private boolean sm2;
4545

46+
@Option(names = {"--force"},
47+
description = "Allow import even if address already exists")
48+
private boolean force;
49+
4650
@Override
4751
public Integer call() {
4852
try {
53+
if (!KeystoreCliUtils.checkFileExists(keyFile, "Key file")) {
54+
return 1;
55+
}
4956
KeystoreCliUtils.ensureDirectory(keystoreDir);
5057

5158
String privateKey = readPrivateKey();
@@ -77,15 +84,22 @@ public Integer call() {
7784
return 1;
7885
}
7986
String address = Credentials.create(keyPair).getAddress();
80-
warnIfAddressExists(keystoreDir, address);
87+
String existingFile = findExistingKeystore(keystoreDir, address);
88+
if (existingFile != null && !force) {
89+
System.err.println("Keystore for address " + address
90+
+ " already exists: " + existingFile
91+
+ ". Use --force to import anyway.");
92+
return 1;
93+
}
8194
String fileName = WalletUtils.generateWalletFile(password, keyPair, keystoreDir, true);
8295
KeystoreCliUtils.setOwnerOnly(new File(keystoreDir, fileName));
8396
if (json) {
8497
KeystoreCliUtils.printJson(KeystoreCliUtils.jsonMap(
8598
"address", address, "file", fileName));
8699
} else {
87-
System.out.println("Imported keystore: " + fileName);
88-
System.out.println("Address: " + address);
100+
System.out.println("Imported keystore successfully");
101+
KeystoreCliUtils.printSecurityTips(address,
102+
new File(keystoreDir, fileName).getPath());
89103
}
90104
return 0;
91105
} catch (CipherException e) {
@@ -137,13 +151,13 @@ private boolean isValidPrivateKey(String key) {
137151
return !StringUtils.isEmpty(key) && HEX_PATTERN.matcher(key).matches();
138152
}
139153

140-
private void warnIfAddressExists(File dir, String address) {
154+
private String findExistingKeystore(File dir, String address) {
141155
if (!dir.exists() || !dir.isDirectory()) {
142-
return;
156+
return null;
143157
}
144158
File[] files = dir.listFiles((d, name) -> name.endsWith(".json"));
145159
if (files == null) {
146-
return;
160+
return null;
147161
}
148162
com.fasterxml.jackson.databind.ObjectMapper mapper =
149163
KeystoreCliUtils.mapper();
@@ -152,13 +166,12 @@ private void warnIfAddressExists(File dir, String address) {
152166
org.tron.keystore.WalletFile wf =
153167
mapper.readValue(file, org.tron.keystore.WalletFile.class);
154168
if (address.equals(wf.getAddress())) {
155-
System.err.println("WARNING: keystore for address "
156-
+ address + " already exists: " + file.getName());
157-
return;
169+
return file.getName();
158170
}
159171
} catch (Exception e) {
160172
// Skip invalid files
161173
}
162174
}
175+
return null;
163176
}
164177
}

‎plugins/src/main/java/common/org/tron/plugins/KeystoreNew.java‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -53,8 +53,9 @@ public Integer call() {
5353
KeystoreCliUtils.printJson(KeystoreCliUtils.jsonMap(
5454
"address", address, "file", fileName));
5555
} else {
56-
System.out.println("Generated keystore: " + fileName);
57-
System.out.println("Address: " + address);
56+
System.out.println("Your new key was generated");
57+
KeystoreCliUtils.printSecurityTips(address,
58+
new File(keystoreDir, fileName).getPath());
5859
}
5960
return 0;
6061
} catch (CipherException e) {

‎plugins/src/main/java/common/org/tron/plugins/KeystoreUpdate.java‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,11 @@ public Integer call() {
5959
String newPassword;
6060

6161
if (passwordFile != null) {
62+
if (!passwordFile.exists()) {
63+
System.err.println("Password file not found: " + passwordFile.getPath()
64+
+ ". Omit --password-file for interactive input.");
65+
return 1;
66+
}
6267
if (passwordFile.length() > 1024) {
6368
System.err.println("Password file too large (max 1KB).");
6469
return 1;

‎plugins/src/test/java/org/tron/plugins/KeystoreImportTest.java‎

Lines changed: 53 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -174,7 +174,7 @@ public void testImportKeyFileWithWhitespace() throws Exception {
174174
}
175175

176176
@Test
177-
public void testImportDuplicateAddress() throws Exception {
177+
public void testImportDuplicateAddressBlocked() throws Exception {
178178
File dir = tempFolder.newFolder("keystore-dup");
179179
SignInterface keyPair = SignUtils.getGeneratedRandomSign(
180180
SecureRandom.getInstance("NativePRNG"), true);
@@ -185,22 +185,70 @@ public void testImportDuplicateAddress() throws Exception {
185185
File pwFile = tempFolder.newFile("pw-dup.txt");
186186
Files.write(pwFile.toPath(), "test123456".getBytes(StandardCharsets.UTF_8));
187187

188-
// Import same key twice
188+
// First import succeeds
189189
CommandLine cmd1 = new CommandLine(new Toolkit());
190190
assertEquals(0, cmd1.execute("keystore", "import",
191191
"--keystore-dir", dir.getAbsolutePath(),
192192
"--key-file", keyFile.getAbsolutePath(),
193193
"--password-file", pwFile.getAbsolutePath()));
194194

195+
// Second import of same key is blocked
195196
CommandLine cmd2 = new CommandLine(new Toolkit());
196-
assertEquals(0, cmd2.execute("keystore", "import",
197+
assertEquals("Duplicate import should be blocked", 1,
198+
cmd2.execute("keystore", "import",
199+
"--keystore-dir", dir.getAbsolutePath(),
200+
"--key-file", keyFile.getAbsolutePath(),
201+
"--password-file", pwFile.getAbsolutePath()));
202+
203+
File[] files = dir.listFiles((d, name) -> name.endsWith(".json"));
204+
assertNotNull(files);
205+
assertEquals("Should still have only 1 keystore", 1, files.length);
206+
}
207+
208+
@Test
209+
public void testImportDuplicateAddressWithForce() throws Exception {
210+
File dir = tempFolder.newFolder("keystore-force");
211+
SignInterface keyPair = SignUtils.getGeneratedRandomSign(
212+
SecureRandom.getInstance("NativePRNG"), true);
213+
String privateKeyHex = ByteArray.toHexString(keyPair.getPrivateKey());
214+
215+
File keyFile = tempFolder.newFile("force.key");
216+
Files.write(keyFile.toPath(), privateKeyHex.getBytes(StandardCharsets.UTF_8));
217+
File pwFile = tempFolder.newFile("pw-force.txt");
218+
Files.write(pwFile.toPath(), "test123456".getBytes(StandardCharsets.UTF_8));
219+
220+
// First import
221+
CommandLine cmd1 = new CommandLine(new Toolkit());
222+
assertEquals(0, cmd1.execute("keystore", "import",
197223
"--keystore-dir", dir.getAbsolutePath(),
198224
"--key-file", keyFile.getAbsolutePath(),
199225
"--password-file", pwFile.getAbsolutePath()));
200226

201-
// Should create two separate files (timestamped names differ)
227+
// Second import with --force succeeds
228+
CommandLine cmd2 = new CommandLine(new Toolkit());
229+
assertEquals(0, cmd2.execute("keystore", "import",
230+
"--keystore-dir", dir.getAbsolutePath(),
231+
"--key-file", keyFile.getAbsolutePath(),
232+
"--password-file", pwFile.getAbsolutePath(),
233+
"--force"));
234+
202235
File[] files = dir.listFiles((d, name) -> name.endsWith(".json"));
203236
assertNotNull(files);
204-
assertEquals("Duplicate import should create 2 separate files", 2, files.length);
237+
assertEquals("Force import should create 2 files", 2, files.length);
238+
}
239+
240+
@Test
241+
public void testImportKeyFileNotFound() throws Exception {
242+
File dir = tempFolder.newFolder("keystore-nokey");
243+
File pwFile = tempFolder.newFile("pw-nokey.txt");
244+
Files.write(pwFile.toPath(), "test123456".getBytes(StandardCharsets.UTF_8));
245+
246+
CommandLine cmd = new CommandLine(new Toolkit());
247+
int exitCode = cmd.execute("keystore", "import",
248+
"--keystore-dir", dir.getAbsolutePath(),
249+
"--key-file", "/tmp/nonexistent-key-file.txt",
250+
"--password-file", pwFile.getAbsolutePath());
251+
252+
assertEquals("Should fail when key file not found", 1, exitCode);
205253
}
206254
}

‎plugins/src/test/java/org/tron/plugins/KeystoreNewTest.java‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -180,6 +180,18 @@ public void testNewKeystoreSpecialCharPassword() throws Exception {
180180
assertNotNull(creds.getAddress());
181181
}
182182

183+
@Test
184+
public void testNewKeystorePasswordFileNotFound() throws Exception {
185+
File dir = tempFolder.newFolder("keystore-nopw");
186+
187+
CommandLine cmd = new CommandLine(new Toolkit());
188+
int exitCode = cmd.execute("keystore", "new",
189+
"--keystore-dir", dir.getAbsolutePath(),
190+
"--password-file", "/tmp/nonexistent-pw.txt");
191+
192+
assertEquals("Should fail when password file not found", 1, exitCode);
193+
}
194+
183195
@Test
184196
public void testNewKeystoreDirIsFile() throws Exception {
185197
File notADir = tempFolder.newFile("not-a-dir");

0 commit comments

Comments
 (0)