Security Config Refactor

src/main/java/co/rsk/federate/FedNodeRunner.java

@@ -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;
@@ -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;
@@ -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;
@@ -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,
@@ -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;
     }
@@ -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;
     }
 
@@ -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;
     }
 
@@ -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
@@ -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();
@@ -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);
                 }
             });
@@ -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);
     }
@@ -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();
         }
@@ -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(

src/main/java/co/rsk/federate/signing/ECDSASignerFactory.java

@@ -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;
@@ -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;
+    }
 }

src/main/java/co/rsk/federate/signing/config/SignerConfig.java

@@ -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;
     }
 

src/main/java/co/rsk/federate/signing/config/SignerType.java

@@ -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;
+  }
+}

src/main/java/co/rsk/federate/signing/hsm/config/PowHSMConfig.java

@@ -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;
@@ -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");
     }
 

src/test/java/co/rsk/federate/config/PowpegNodeSystemPropertiesTest.java

@@ -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;
@@ -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);