Skip to content

Commit df91f45

Browse files
BarbatosBarbatos
authored andcommitted
feat(crypto): warn when SR keystore is a symbolic link
Aligns WalletUtils.loadCredentials with the Lighthouse-style policy: follow the symlink (preserving compatibility with legitimate SR deployments that organize keystores via symlinks — e.g. encrypted- volume mounts, container volume bindings, /opt/tron → /mnt/secrets layouts), but emit a logger.warn so operators have a chance to notice if a path they did not expect to be a symlink has become one. Hard-rejecting would silently break "SR fails to start" on upgrade for operators using these legitimate patterns; warn-and-proceed surfaces the situation without forcing a downtime cliff. The lstat probe uses LinkOption.NOFOLLOW_LINKS so an attacker cannot hide the symlink by racing the check; if the lstat itself fails for unrelated reasons (permission, fs error) we silently fall through and let the subsequent readValue surface the real error. Tests verify a symlinked keystore still loads end-to-end (the Lighthouse parity path) and that a regular-file roundtrip is unaffected.
1 parent b89790c commit df91f45

2 files changed

Lines changed: 65 additions & 0 deletions

File tree

crypto/src/main/java/org/tron/keystore/WalletUtils.java

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,8 +8,10 @@
88
import java.io.IOException;
99
import java.nio.file.AtomicMoveNotSupportedException;
1010
import java.nio.file.Files;
11+
import java.nio.file.LinkOption;
1112
import java.nio.file.Path;
1213
import java.nio.file.StandardCopyOption;
14+
import java.nio.file.attribute.BasicFileAttributes;
1315
import java.nio.file.attribute.PosixFilePermission;
1416
import java.nio.file.attribute.PosixFilePermissions;
1517
import java.time.ZoneOffset;
@@ -19,13 +21,15 @@
1921
import java.util.EnumSet;
2022
import java.util.Scanner;
2123
import java.util.Set;
24+
import lombok.extern.slf4j.Slf4j;
2225
import org.apache.commons.lang3.StringUtils;
2326
import org.tron.common.crypto.SignInterface;
2427
import org.tron.core.exception.CipherException;
2528

2629
/**
2730
* Utility functions for working with Wallet files.
2831
*/
32+
@Slf4j(topic = "keystore")
2933
public class WalletUtils {
3034

3135
private static final ObjectMapper objectMapper = new ObjectMapper();
@@ -117,10 +121,36 @@ public static void writeWalletFile(WalletFile walletFile, File destination)
117121

118122
public static Credentials loadCredentials(String password, File source, boolean ecKey)
119123
throws IOException, CipherException {
124+
warnIfSymbolicLink(source);
120125
WalletFile walletFile = objectMapper.readValue(source, WalletFile.class);
121126
return Credentials.create(Wallet.decrypt(password, walletFile, ecKey));
122127
}
123128

129+
/**
130+
* Emit a warning if {@code source} is a symbolic link. The keystore is still
131+
* read (following the symlink), preserving compatibility with legitimate
132+
* deployments that use symlinks to organize keystore files (e.g.
133+
* {@code /opt/tron/keystore/witness.json} -> {@code /mnt/encrypted/...},
134+
* container volume-mount paths). The warning gives operators a chance to
135+
* notice if a path they did not expect to be a symlink has become one — for
136+
* example if an attacker with config-injection ability has redirected the
137+
* SR startup keystore. This mirrors how Ethereum consensus clients (e.g.
138+
* Lighthouse) handle a configured {@code voting_keystore_path}.
139+
*/
140+
private static void warnIfSymbolicLink(File source) {
141+
try {
142+
BasicFileAttributes attrs = Files.readAttributes(source.toPath(),
143+
BasicFileAttributes.class, LinkOption.NOFOLLOW_LINKS);
144+
if (attrs.isSymbolicLink()) {
145+
logger.warn("Keystore file is a symbolic link: {} — proceeding, "
146+
+ "but verify the symlink target points where you expect.",
147+
source.getPath());
148+
}
149+
} catch (IOException ignored) {
150+
// If we can't stat, let the subsequent readValue surface the real error.
151+
}
152+
}
153+
124154
public static String getWalletFileName(WalletFile walletFile) {
125155
DateTimeFormatter format = DateTimeFormatter.ofPattern(
126156
"'UTC--'yyyy-MM-dd'T'HH-mm-ss.nVV'--'");

framework/src/test/java/org/tron/keystore/WalletUtilsWriteTest.java

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -166,4 +166,39 @@ public void testWriteWalletFileCleansUpTempOnFailure() throws Exception {
166166
assertNotNull(tempFiles);
167167
assertEquals("Temp file must be cleaned up on failure", 0, tempFiles.length);
168168
}
169+
170+
// ---------- loadCredentials symlink behavior ----------
171+
172+
@Test
173+
public void testLoadCredentialsFollowsSymlinkButWarns() throws Exception {
174+
Assume.assumeTrue("Symlinks only tested on POSIX",
175+
!System.getProperty("os.name").toLowerCase().contains("win"));
176+
177+
File realDir = tempFolder.newFolder("load-symlink-target");
178+
SignInterface keyPair = SignUtils.getGeneratedRandomSign(Utils.getRandom(), true);
179+
String realName = WalletUtils.generateWalletFile("password123", keyPair, realDir, false);
180+
File realKeystore = new File(realDir, realName);
181+
182+
File linkDir = tempFolder.newFolder("load-symlink-link");
183+
File symlink = new File(linkDir, "witness.json");
184+
Files.createSymbolicLink(symlink.toPath(), realKeystore.toPath());
185+
186+
// Should NOT throw — Lighthouse-style: follow the symlink, log a warning
187+
// for the operator. Hard-rejecting would silently break legitimate SR
188+
// deployments that organize keystores via symlinks.
189+
Credentials creds =
190+
WalletUtils.loadCredentials("password123", symlink, true);
191+
assertNotNull(creds.getAddress());
192+
}
193+
194+
@Test
195+
public void testLoadCredentialsAcceptsRegularFile() throws Exception {
196+
File dir = tempFolder.newFolder("load-ok");
197+
SignInterface keyPair = SignUtils.getGeneratedRandomSign(Utils.getRandom(), true);
198+
String fileName = WalletUtils.generateWalletFile("password123", keyPair, dir, false);
199+
200+
Credentials creds =
201+
WalletUtils.loadCredentials("password123", new File(dir, fileName), true);
202+
assertNotNull(creds.getAddress());
203+
}
169204
}

0 commit comments

Comments
 (0)