Skip to content

Commit

Permalink
Merge pull request #309 from rsksmart/config-fix-wip
Browse files Browse the repository at this point in the history 
Security Config Refactor
  • Loading branch information
@marcos-iov
marcos-iov authored Aug 23, 2024
2 parents 461dd1d + 413fd23 commit b5fc935
Show file tree
Hide file tree
Showing 9 changed files with 118 additions and 79 deletions.
94 changes: 50 additions & 44 deletions src/main/java/co/rsk/federate/FedNodeRunner.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@

import co.rsk.NodeRunner;
import co.rsk.bitcoinj.core.BtcECKey;
import co.rsk.core.RskAddress;
import co.rsk.federate.signing.hsm.HSMVersion;
import co.rsk.peg.constants.BridgeConstants;
import co.rsk.federate.adapter.ThinConverter;
Expand All @@ -32,6 +33,7 @@
import co.rsk.federate.btcreleaseclient.BtcReleaseClientStorageSynchronizer;
import co.rsk.federate.config.PowpegNodeSystemProperties;
import co.rsk.federate.signing.config.SignerConfig;
import co.rsk.federate.signing.config.SignerType;
import co.rsk.federate.signing.hsm.config.PowHSMConfig;
import co.rsk.federate.io.*;
import co.rsk.federate.log.BtcLogMonitor;
Expand Down Expand Up @@ -72,7 +74,7 @@
*/

public class FedNodeRunner implements NodeRunner {
private static final Logger LOGGER = LoggerFactory.getLogger(FedNodeRunner.class);
private static final Logger logger = LoggerFactory.getLogger(FedNodeRunner.class);
private final BtcToRskClient btcToRskClientActive;
private final BtcToRskClient btcToRskClientRetiring;
private final BtcReleaseClient btcReleaseClient;
Expand Down Expand Up @@ -125,53 +127,57 @@ public FedNodeRunner(

@Override
public void run() throws Exception {
LOGGER.debug("[run] Starting RSK");
logger.debug("[run] Starting RSK");
signer = buildSigner();

try {
PowHSMConfig powHsmConfig = new PowHSMConfig(
config.signerConfig(BTC.getId()));

HSMClientProtocol protocol =
hsmClientProtocolFactory.buildHSMClientProtocolFromConfig(powHsmConfig);

int hsmVersion = protocol.getVersion();
LOGGER.debug("[run] Using HSM version {}", hsmVersion);

if (HSMVersion.isPowHSM(hsmVersion)) {
hsmBookkeepingClient = buildBookKeepingClient(
protocol, powHsmConfig);
hsmBookkeepingService = buildBookKeepingService(
hsmBookkeepingClient, powHsmConfig);
}
} catch (SignerException e) {
LOGGER.info("[run] PowHSM config was not found", e);
SignerConfig btcSignerConfig = config.signerConfig(BTC.getId());
if (btcSignerConfig != null && btcSignerConfig.getSignerType() == SignerType.HSM) {
startBookkeepingServices();
}

if (!this.checkFederateRequirements()) {
LOGGER.error("[run] Error validating Fed-Node Requirements");
logger.error("[run] Error validating Fed-Node Requirements");
return;
}

LOGGER.info("[run] Signers: {}", signer.getVersionString());
logger.info("[run] Signers: {}", signer.getVersionString());
configureFederatorSupport();
fullNodeRunner.run();
startFederate();

signer.addListener(l -> {
LOGGER.error("[run] Signer informed unrecoverable state, shutting down", l);
logger.error("[run] Signer informed unrecoverable state, shutting down", l);
this.shutdown();
});

LOGGER.info("[run] Federated node started");
LOGGER.info("[run] RSK address: {}", Hex.toHexString(this.member.getRskPublicKey().getAddress()));
RskAddress pegnatoryRskAddress = new RskAddress(this.member.getRskPublicKey().getAddress());
logger.info("[run] Federated node started");
logger.info("[run] RSK address: {}", pegnatoryRskAddress);
}

private void startBookkeepingServices() throws SignerException, HSMClientException {
PowHSMConfig powHsmConfig = new PowHSMConfig(
config.signerConfig(BTC.getId()));

HSMClientProtocol protocol =
hsmClientProtocolFactory.buildHSMClientProtocolFromConfig(powHsmConfig);

int hsmVersion = protocol.getVersion();
logger.debug("[run] Using HSM version {}", hsmVersion);

if (HSMVersion.isPowHSM(hsmVersion)) {
hsmBookkeepingClient = buildBookKeepingClient(
protocol, powHsmConfig);
hsmBookkeepingService = buildBookKeepingService(
hsmBookkeepingClient, powHsmConfig);
}
}

private void configureFederatorSupport() throws SignerException {
BtcECKey btcPublicKey = signer.getPublicKey(BTC.getKeyId()).toBtcKey();
ECKey rskPublicKey = signer.getPublicKey(RSK.getKeyId()).toEthKey();
ECKey mstKey = signer.getPublicKey(MST.getKeyId()).toEthKey();
LOGGER.info(
logger.info(
"[configureFederatorSupport] BTC public key: {}. RSK public key: {}. MST public key: {}",
btcPublicKey,
rskPublicKey,
Expand All @@ -196,14 +202,14 @@ private ECDSASigner buildSigner() {
ECDSASigner createdSigner = buildSignerFromKey(keyId.getKeyId());
compositeSigner.addSigner(createdSigner);
} catch (SignerException e) {
LOGGER.error("[buildSigner] Error trying to build signer with key id {}. Detail: {}", keyId, e.getMessage());
logger.error("[buildSigner] Error trying to build signer with key id {}. Detail: {}", keyId, e.getMessage());
} catch (Exception e) {
LOGGER.error("[buildSigner] Error creating signer {}. Detail: {}", keyId, e.getMessage());
logger.error("[buildSigner] Error creating signer {}. Detail: {}", keyId, e.getMessage());
throw e;
}
});

LOGGER.debug("[buildSigner] Signers created");
logger.debug("[buildSigner] Signers created");

return compositeSigner;
}
Expand All @@ -214,7 +220,7 @@ private HSMBookkeepingClient buildBookKeepingClient(

HSMBookkeepingClient bookKeepingClient = hsmBookKeepingClientProvider.getHSMBookKeepingClient(protocol);
bookKeepingClient.setMaxChunkSizeToHsm(powHsmConfig.getMaxChunkSizeToHsm());
LOGGER.info("[buildBookKeepingClient] HSMBookkeeping client built for HSM version: {}", bookKeepingClient.getVersion());
logger.info("[buildBookKeepingClient] HSMBookkeeping client built for HSM version: {}", bookKeepingClient.getVersion());
return bookKeepingClient;
}

Expand All @@ -236,7 +242,7 @@ private HSMBookkeepingService buildBookKeepingService(
powHsmConfig.getInformerInterval(),
powHsmConfig.isStopBookkeepingScheduler()
);
LOGGER.info("[buildBookKeepingService] HSMBookkeeping Service built for HSM version: {}", bookKeepingClient.getVersion());
logger.info("[buildBookKeepingService] HSMBookkeeping Service built for HSM version: {}", bookKeepingClient.getVersion());
return service;
}

Expand Down Expand Up @@ -270,7 +276,7 @@ private boolean checkFederateRequirements() {
}

private void startFederate() throws Exception {
LOGGER.debug("[startFederate] Starting Federation Behaviour");
logger.debug("[startFederate] Starting Federation Behaviour");
if (config.isFederatorEnabled()) {
// Set up a federation watcher to trigger starts and stops of the
// btc to rsk client upon federation changes
Expand Down Expand Up @@ -304,8 +310,8 @@ private void startFederate() throws Exception {
btcLogMonitor.start();
rskLogMonitor.start();
if (hsmBookkeepingService != null) {
hsmBookkeepingService.addListener((e) -> {
LOGGER.error("[startFederate] HSM bookkeeping service informed unrecoverable state, shutting down", e);
hsmBookkeepingService.addListener(e -> {
logger.error("[startFederate] HSM bookkeeping service informed unrecoverable state, shutting down", e);
this.shutdown();
});
hsmBookkeepingService.start();
Expand Down Expand Up @@ -344,15 +350,15 @@ private void startFederate() throws Exception {
public void onActiveFederationChange(Optional<Federation> oldFederation, Federation newFederation) {
String oldFederationAddress = oldFederation.map(f -> f.getAddress().toString()).orElse("NONE");
String newFederationAddress = newFederation.getAddress().toString();
LOGGER.debug(String.format("[onActiveFederationChange] Active federation change: from %s to %s", oldFederationAddress, newFederationAddress));
logger.debug(String.format("[onActiveFederationChange] Active federation change: from %s to %s", oldFederationAddress, newFederationAddress));
triggerClientChange(btcToRskClientActive, Optional.of(newFederation));
}

@Override
public void onRetiringFederationChange(Optional<Federation> oldFederation, Optional<Federation> newFederation) {
String oldFederationAddress = oldFederation.map(f -> f.getAddress().toString()).orElse("NONE");
String newFederationAddress = newFederation.map(f -> f.getAddress().toString()).orElse("NONE");
LOGGER.debug(String.format("[onRetiringFederationChange] Retiring federation change: from %s to %s", oldFederationAddress, newFederationAddress));
logger.debug(String.format("[onRetiringFederationChange] Retiring federation change: from %s to %s", oldFederationAddress, newFederationAddress));
triggerClientChange(btcToRskClientRetiring, newFederation);
}
});
Expand All @@ -363,18 +369,18 @@ public void onRetiringFederationChange(Optional<Federation> oldFederation, Optio

@PreDestroy
public void tearDown() {
LOGGER.debug("[tearDown] FederateRunner tearDown starting...");
logger.debug("[tearDown] FederateRunner tearDown starting...");

this.stop();

LOGGER.debug("[tearDown] FederateRunner tearDown finished.");
logger.debug("[tearDown] FederateRunner tearDown finished.");
}

private void shutdown() {
try {
this.tearDown();
} catch(Exception e){
LOGGER.error("[shutdown] FederateRunner teardown failed", e);
logger.error("[shutdown] FederateRunner teardown failed", e);
}
System.exit(-1);
}
Expand All @@ -386,18 +392,18 @@ private void triggerClientChange(BtcToRskClient client, Optional<Federation> fed
// Only start if this federator is part of the new federation
if (federation.isPresent() && federation.get().isMember(this.member)) {
String federationAddress = federation.get().getAddress().toString();
LOGGER.debug("[triggerClientChange] Starting lock and release clients since I belong to federation {}", federationAddress);
LOGGER.info("[triggerClientChange] Joined to {} federation", federationAddress);
logger.debug("[triggerClientChange] Starting lock and release clients since I belong to federation {}", federationAddress);
logger.info("[triggerClientChange] Joined to {} federation", federationAddress);
client.start(federation.get());
btcReleaseClient.start(federation.get());
} else {
LOGGER.warn("[triggerClientChange] This federator node is not part of the new federation. Check your configuration for signers BTC, RSK and MST keys");
logger.warn("[triggerClientChange] This federator node is not part of the new federation. Check your configuration for signers BTC, RSK and MST keys");
}
}

@Override
public void stop() {
LOGGER.info("[stop] Shutting down Federation node");
logger.info("[stop] Shutting down Federation node");
if (bitcoinWrapper != null) {
bitcoinWrapper.stop();
}
Expand All @@ -413,7 +419,7 @@ public void stop() {
}

fullNodeRunner.stop();
LOGGER.info("[stop] Federation node Shut down.");
logger.info("[stop] Federation node Shut down.");
}

private BitcoinWrapper createAndSetupBitcoinWrapper(
Expand Down
46 changes: 25 additions & 21 deletions src/main/java/co/rsk/federate/signing/ECDSASignerFactory.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
package co.rsk.federate.signing;

import co.rsk.federate.signing.config.SignerConfig;
import co.rsk.federate.signing.config.SignerType;
import co.rsk.federate.signing.hsm.config.PowHSMConfig;
import co.rsk.federate.signing.hsm.SignerException;
import co.rsk.federate.signing.hsm.client.HSMClientProtocol;
Expand All @@ -34,32 +35,35 @@ public ECDSASigner buildFromConfig(SignerConfig config) throws SignerException {
if (config == null) {
throw new SignerException("'signers' entry not found in config file.");
}
String type = config.getType();
SignerType type = config.getSignerType();
logger.debug("[buildFromConfig] SignerConfig type {}", type);
switch (type) {
case "keyFile":
case KEYFILE:
return new ECDSASignerFromFileKey(
new KeyId(config.getId()),
config.getConfig().getString("path")
new KeyId(config.getId()),
config.getConfig().getString("path")
);
case "hsm":
try {
PowHSMConfig powHSMConfig = new PowHSMConfig(config);
HSMClientProtocol hsmClientProtocol =
new HSMClientProtocolFactory().buildHSMClientProtocolFromConfig(powHSMConfig);
HSMSigningClientProvider hsmSigningClientProvider = new HSMSigningClientProvider(hsmClientProtocol, config.getId());
ECDSAHSMSigner signer = new ECDSAHSMSigner(hsmSigningClientProvider);
// Add the key mapping
String hsmKeyId = config.getConfig().getString("keyId");
signer.addKeyMapping(new KeyId(config.getId()), hsmKeyId);
return signer;
} catch (Exception e) {
String message = "Something went wrong while trying to build HSM Signer";
logger.debug("[buildFromConfig] {} - {}", message, e.getMessage());
throw new RuntimeException(e.getMessage());
}
case HSM:
return buildHSMFromConfig(config);
default:
throw new RuntimeException(String.format("Unsupported signer type: %s", type));
throw new IllegalArgumentException(String.format("Unsupported signer type: %s", type));
}
}

private ECDSAHSMSigner buildHSMFromConfig(SignerConfig config) throws SignerException {
PowHSMConfig powHSMConfig = new PowHSMConfig(config);
HSMClientProtocol hsmClientProtocol = new HSMClientProtocolFactory().buildHSMClientProtocolFromConfig(
powHSMConfig
);
HSMSigningClientProvider hsmSigningClientProvider = new HSMSigningClientProvider(
hsmClientProtocol,
config.getId()
);
ECDSAHSMSigner signer = new ECDSAHSMSigner(hsmSigningClientProvider);
// Add the key mapping
String hsmKeyId = config.getConfig().getString("keyId");
signer.addKeyMapping(new KeyId(config.getId()), hsmKeyId);

return signer;
}
}
6 changes: 3 additions & 3 deletions src/main/java/co/rsk/federate/signing/config/SignerConfig.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,20 +7,20 @@ public class SignerConfig {
private static final String SIGNER_TYPE_PATH = "type";

private final String id;
private final String type;
private final SignerType type;
private final Config config;

public SignerConfig(String keyId, Config config) {
this.id = keyId;
this.type = config.getString(SIGNER_TYPE_PATH);
this.type = SignerType.fromConfigValue(config.getString(SIGNER_TYPE_PATH));
this.config = config.withoutPath(SIGNER_TYPE_PATH);
}

public String getId() {
return id;
}

public String getType() {
public SignerType getSignerType() {
return type;
}

Expand Down
25 changes: 25 additions & 0 deletions src/main/java/co/rsk/federate/signing/config/SignerType.java
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
package co.rsk.federate.signing.config;

public enum SignerType {
KEYFILE("keyFile"),
HSM("hsm");

private final String type;

SignerType(String signerType) {
this.type = signerType;
}

public static SignerType fromConfigValue(String configValue) {
for (SignerType signerType : values()) {
if (signerType.getType().equalsIgnoreCase(configValue)) {
return signerType;
}
}
throw new IllegalArgumentException(String.format("Unsupported signer type: %s", configValue));
}

public String getType() {
return type;
}
}
4 changes: 2 additions & 2 deletions src/main/java/co/rsk/federate/signing/hsm/config/PowHSMConfig.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
import co.rsk.federate.signing.hsm.SignerException;
import co.rsk.bitcoinj.core.NetworkParameters;
import co.rsk.federate.signing.config.SignerConfig;
import co.rsk.federate.signing.config.SignerType;
import co.rsk.federate.signing.hsm.HSMClientException;
import co.rsk.federate.signing.hsm.HSMUnsupportedTypeException;
import co.rsk.federate.signing.hsm.client.HSMBookkeepingClient;
Expand All @@ -17,12 +18,11 @@
public class PowHSMConfig {

private static final Logger logger = LoggerFactory.getLogger(PowHSMConfig.class);
private static final String SIGNER_TYPE = "hsm";

private final Config config;

public PowHSMConfig(SignerConfig signerConfig) throws SignerException {
if (signerConfig == null || !signerConfig.getType().equals(SIGNER_TYPE)) {
if (signerConfig == null || signerConfig.getSignerType() != SignerType.HSM) {
throw new SignerException("Signer config is not for PowHSM");
}

Expand Down
2 changes: 2 additions & 0 deletions src/test/java/co/rsk/federate/config/PowpegNodeSystemPropertiesTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.when;

import co.rsk.federate.signing.config.SignerType;
import java.time.Duration;
import java.util.ArrayList;
import java.util.Arrays;
Expand Down Expand Up @@ -303,6 +304,7 @@ void signerConfig_whenSignerConfigExists_shouldReturnSignerConfig() {
when(mockSignersConfig.getObject(existingKey)).thenReturn(mockConfigSignerObject);
when(mockConfigSignerObject.toConfig()).thenReturn(mockSignerConfig);
when(mockSignerConfig.hasPath("type")).thenReturn(true);
when(mockSignerConfig.getString("type")).thenReturn(SignerType.HSM.getType());

SignerConfig result = powpegNodeSystemProperties.signerConfig(existingKey);

Expand Down
Loading

0 comments on commit b5fc935

Please sign in to comment.