Add Azure Signer to Multiplatform signing (Consensys#173)

Author: rain-on

acceptance-tests/src/test/java/tech/pegasys/ethsigner/tests/dsl/signer/TransactionSignerParamsSupplier.java

@@ -57,7 +57,7 @@ public Collection<String> get() {
       params.add(String.valueOf(hashicorpVaultPort));
     } else if (azureKeyVault != null) {
       params.add("azure-signer");
-      params.add("--keyvault-name");
+      params.add("--key-vault-name");
       params.add(azureKeyVault);
       params.add("--key-name");
       params.add("TestKey");

ethsigner/signer/azure/src/main/java/tech/pegasys/ethsigner/signer/azure/AzureConfig.java

@@ -12,28 +12,30 @@
  */
 package tech.pegasys.ethsigner.signer.azure;
 
+import static com.google.common.base.Preconditions.checkNotNull;
+
 public class AzureConfig {
-  private final String keyvaultName;
+  private final String keyVaultName;
   private final String keyName;
   private final String keyVersion;
   private final String clientId;
   private final String clientSecret;
 
   public AzureConfig(
-      String keyvaultName,
-      String keyName,
-      String keyVersion,
-      String clientId,
-      String clientSecret) {
-    this.keyvaultName = keyvaultName;
+      final String keyVaultName,
+      final String keyName,
+      final String keyVersion,
+      final String clientId,
+      final String clientSecret) {
+    this.keyVaultName = keyVaultName;
     this.keyName = keyName;
     this.keyVersion = keyVersion;
     this.clientId = clientId;
     this.clientSecret = clientSecret;
   }
 
-  public String getKeyvaultName() {
-    return keyvaultName;
+  public String getKeyVaultName() {
+    return keyVaultName;
   }
 
   public String getKeyName() {
@@ -51,4 +53,48 @@ public String getClientId() {
   public String getClientSecret() {
     return clientSecret;
   }
+
+  public static class AzureConfigBuilder {
+
+    private String keyVaultName;
+    private String keyName;
+    private String keyVersion;
+    private String clientId;
+    private String clientSecret;
+
+    public AzureConfigBuilder withKeyVaultName(final String keyVaultName) {
+      this.keyVaultName = keyVaultName;
+      return this;
+    }
+
+    public AzureConfigBuilder withKeyName(final String keyName) {
+      this.keyName = keyName;
+      return this;
+    }
+
+    public AzureConfigBuilder withKeyVersion(final String keyVersion) {
+      this.keyVersion = keyVersion;
+      return this;
+    }
+
+    public AzureConfigBuilder withClientId(final String clientId) {
+      this.clientId = clientId;
+      return this;
+    }
+
+    public AzureConfigBuilder withClientSecret(String clientSecret) {
+      this.clientSecret = clientSecret;
+      return this;
+    }
+
+    public AzureConfig build() {
+      checkNotNull(keyVaultName, "Key Vault Name was not set.");
+      checkNotNull(keyName, "Key Name was not set.");
+      checkNotNull(keyVersion, "Key Version was not set.");
+      checkNotNull(clientId, "Client Id was not set.");
+      checkNotNull(clientSecret, "Client Secret was not set.");
+
+      return new AzureConfig(keyVaultName, keyName, keyVersion, clientId, clientSecret);
+    }
+  }
 }

ethsigner/signer/azure/src/main/java/tech/pegasys/ethsigner/signer/azure/AzureKeyVaultTransactionSigner.java

@@ -55,7 +55,7 @@ public Signature sign(final byte[] data) {
 
     if (signature.length != 64) {
       throw new RuntimeException(
-          "Invalid signature from the keyvault signing service, must be 64 bytes long");
+          "Invalid signature from the key vault signing service, must be 64 bytes long");
     }
 
     // reference: blog by Tomislav Markovski

ethsigner/signer/azure/src/main/java/tech/pegasys/ethsigner/signer/azure/AzureKeyVaultTransactionSignerFactory.java

@@ -51,13 +51,13 @@ public TransactionSigner createSigner(final AzureConfig config) {
     checkNotNull(config, "Config must be specified");
     final KeyVaultClientCustom client =
         vaultAuthenticator.getAuthenticatedClient(config.getClientId(), config.getClientSecret());
-    final String baseUrl = constructAzureKeyVaultUrl(config.getKeyvaultName());
+    final String baseUrl = constructAzureKeyVaultUrl(config.getKeyVaultName());
 
     final JsonWebKey key;
-    final KeyIdentifier kid;
+    final KeyIdentifier keyIdentifier;
     try {
-      kid = new KeyIdentifier(baseUrl, config.getKeyName(), config.getKeyVersion());
-      key = client.getKey(kid.toString()).key();
+      keyIdentifier = new KeyIdentifier(baseUrl, config.getKeyName(), config.getKeyVersion());
+      key = client.getKey(keyIdentifier.toString()).key();
     } catch (final KeyVaultErrorException ex) {
       if (ex.response().raw().code() == 401) {
         LOG.debug(INACCESSIBLE_KEY_ERROR);
@@ -82,7 +82,7 @@ public TransactionSigner createSigner(final AzureConfig config) {
 
     final byte[] rawPublicKey = Bytes.concat(key.x(), key.y());
     final BigInteger publicKey = new BigInteger(1, rawPublicKey);
-    return new AzureKeyVaultTransactionSigner(client, kid.toString(), publicKey);
+    return new AzureKeyVaultTransactionSigner(client, keyIdentifier.toString(), publicKey);
   }
 
   public static String constructAzureKeyVaultUrl(final String keyVaultName) {

ethsigner/signer/azure/src/main/java/tech/pegasys/ethsigner/signer/azure/AzureSubCommand.java

@@ -34,11 +34,11 @@
 public class AzureSubCommand extends SignerSubCommand {
 
   @Option(
-      names = {"--keyvault-name"},
+      names = {"--keyvault-name, --key-vault-name"},
       description = "Name of the vault to access - used as the sub-domain to vault.azure.net",
       required = true,
       arity = "1")
-  private String keyvaultName;
+  private String keyVaultName;
 
   @Option(
       names = {"--key-name"},
@@ -54,7 +54,7 @@ public class AzureSubCommand extends SignerSubCommand {
 
   @Option(
       names = {"--client-id"},
-      description = "The ID used to authenticate with Azure keyvault",
+      description = "The ID used to authenticate with Azure key vault",
       required = true)
   private String clientId;
 
@@ -77,7 +77,7 @@ private TransactionSigner createSigner() throws TransactionSignerInitializationE
     }
 
     final AzureConfig config =
-        new AzureConfig(keyvaultName, keyName, keyVersion, clientId, clientSecret);
+        new AzureConfig(keyVaultName, keyName, keyVersion, clientId, clientSecret);
 
     final AzureKeyVaultTransactionSignerFactory factory =
         new AzureKeyVaultTransactionSignerFactory(new AzureKeyVaultAuthenticator());

ethsigner/signer/azure/src/test/java/tech/pegasys/ethsigner/signer/azure/AzureKeyVaultAuthenticatorTest.java

@@ -19,6 +19,7 @@
 import tech.pegasys.ethsigner.TransactionSignerInitializationException;
 import tech.pegasys.ethsigner.core.signing.Signature;
 import tech.pegasys.ethsigner.core.signing.TransactionSigner;
+import tech.pegasys.ethsigner.signer.azure.AzureConfig.AzureConfigBuilder;
 
 import java.math.BigInteger;
 
@@ -117,7 +118,7 @@ public void accessingNonExistentKeyVaultThrowsExceptionWithMessage() {
     final AzureConfigBuilder configBuilder = createValidConfigBuilder();
 
     final String invalidVaultName = "invalidKeyVault";
-    configBuilder.withKeyvaultName(invalidVaultName);
+    configBuilder.withKeyVaultName(invalidVaultName);
 
     final String expectedMessage =
         String.format(
@@ -166,50 +167,12 @@ public void nullClientAndOrSecretAreHandledCleanly() {
         .isInstanceOf(IllegalArgumentException.class);
   }
 
-  private static class AzureConfigBuilder {
-
-    private String keyvaultName;
-    private String keyName;
-    private String keyVersion;
-    private String clientId;
-    private String clientSecret;
-
-    public AzureConfigBuilder withKeyvaultName(String keyvaultName) {
-      this.keyvaultName = keyvaultName;
-      return this;
-    }
-
-    public AzureConfigBuilder withKeyName(String keyName) {
-      this.keyName = keyName;
-      return this;
-    }
-
-    public AzureConfigBuilder withKeyVersion(String keyVersion) {
-      this.keyVersion = keyVersion;
-      return this;
-    }
-
-    public AzureConfigBuilder withClientId(String clientId) {
-      this.clientId = clientId;
-      return this;
-    }
-
-    public AzureConfigBuilder withClientSecret(String clientSecret) {
-      this.clientSecret = clientSecret;
-      return this;
-    }
-
-    public AzureConfig build() {
-      return new AzureConfig(keyvaultName, keyName, keyVersion, clientId, clientSecret);
-    }
-  }
-
   private AzureConfigBuilder createValidConfigBuilder() {
     return new AzureConfigBuilder()
-        .withClientId(clientId)
-        .withClientSecret(clientSecret)
-        .withKeyVersion(validKeyVersion)
+        .withKeyVaultName("ethsignertestkey")
         .withKeyName("TestKey")
-        .withKeyvaultName("ethsignertestkey");
+        .withKeyVersion(validKeyVersion)
+        .withClientId(clientId)
+        .withClientSecret(clientSecret);
   }
 }

ethsigner/signer/multifile-based/src/main/java/tech/pegasys/ethsigner/signer/multifilebased/KeyPasswordLoader.java

@@ -49,7 +49,7 @@ Optional<KeyPasswordFile> loadKeyAndPasswordForAddress(final String address) {
             .collect(Collectors.toList());
 
     if (matchingKeys.size() > 1) {
-      LOG.error("Found multiple key/password matches for address {}", address);
+      LOG.error("Found multiple key/password matches for address " + address);
       return Optional.empty();
     } else if (matchingKeys.isEmpty()) {
       return Optional.empty();

ethsigner/signer/multiplatform/build.gradle

@@ -28,6 +28,7 @@ dependencies {
   implementation project(':ethsigner:commandline')
   implementation project(':ethsigner:signing-api')
   implementation project(':ethsigner:signer:file-based')
+  implementation project(':ethsigner:signer:azure')
 
   implementation 'info.picocli:picocli'
   implementation 'com.google.guava:guava'

ethsigner/signer/multiplatform/src/main/java/tech/pegasys/ethsigner/signer/multiplatform/MultiPlatformSubCommand.java

@@ -15,6 +15,8 @@
 import tech.pegasys.ethsigner.SignerSubCommand;
 import tech.pegasys.ethsigner.TransactionSignerInitializationException;
 import tech.pegasys.ethsigner.core.signing.TransactionSignerProvider;
+import tech.pegasys.ethsigner.signer.azure.AzureKeyVaultAuthenticator;
+import tech.pegasys.ethsigner.signer.azure.AzureKeyVaultTransactionSignerFactory;
 
 import java.nio.file.Path;
 
@@ -54,7 +56,12 @@ public TransactionSignerProvider createSignerFactory()
       throws TransactionSignerInitializationException {
     final SigningMetadataTomlConfigLoader signingMetadataTomlConfigLoader =
         new SigningMetadataTomlConfigLoader(directoryPath);
-    return new MultiPlatformTransactionSignerProvider(signingMetadataTomlConfigLoader);
+
+    final AzureKeyVaultTransactionSignerFactory azureFactory =
+        new AzureKeyVaultTransactionSignerFactory(new AzureKeyVaultAuthenticator());
+
+    return new MultiPlatformTransactionSignerProvider(
+        signingMetadataTomlConfigLoader, azureFactory);
   }
 
   @Override

ethsigner/signer/multiplatform/src/main/java/tech/pegasys/ethsigner/signer/multiplatform/MultiPlatformTransactionSignerProvider.java

@@ -15,7 +15,10 @@
 import tech.pegasys.ethsigner.TransactionSignerInitializationException;
 import tech.pegasys.ethsigner.core.signing.TransactionSigner;
 import tech.pegasys.ethsigner.core.signing.TransactionSignerProvider;
+import tech.pegasys.ethsigner.signer.azure.AzureKeyVaultTransactionSignerFactory;
 import tech.pegasys.ethsigner.signer.filebased.FileBasedSignerFactory;
+import tech.pegasys.ethsigner.signer.multiplatform.metadata.AzureSigningMetadataFile;
+import tech.pegasys.ethsigner.signer.multiplatform.metadata.FileBasedSigningMetadataFile;
 
 import java.util.Objects;
 import java.util.Optional;
@@ -25,41 +28,69 @@
 import org.apache.logging.log4j.LogManager;
 import org.apache.logging.log4j.Logger;
 
-public class MultiPlatformTransactionSignerProvider implements TransactionSignerProvider {
+public class MultiPlatformTransactionSignerProvider
+    implements TransactionSignerProvider, MultiSignerFactory {
 
   private static final Logger LOG = LogManager.getLogger();
 
   private final SigningMetadataTomlConfigLoader signingMetadataTomlConfigLoader;
+  private final AzureKeyVaultTransactionSignerFactory azureFactory;
 
   MultiPlatformTransactionSignerProvider(
-      final SigningMetadataTomlConfigLoader signingMetadataTomlConfigLoader) {
+      final SigningMetadataTomlConfigLoader signingMetadataTomlConfigLoader,
+      final AzureKeyVaultTransactionSignerFactory azureFactory) {
     this.signingMetadataTomlConfigLoader = signingMetadataTomlConfigLoader;
+    this.azureFactory = azureFactory;
   }
 
   @Override
   public Optional<TransactionSigner> getSigner(final String address) {
-    return signingMetadataTomlConfigLoader.loadMetadataForAddress(address).map(this::createSigner);
+    return signingMetadataTomlConfigLoader
+        .loadMetadataForAddress(address)
+        .map(metadataFile -> metadataFile.createSigner(this));
   }
 
   @Override
   public Set<String> availableAddresses() {
     return signingMetadataTomlConfigLoader.loadAvailableSigningMetadataTomlConfigs().stream()
-        .map(this::createSigner)
+        .map(metadataFile -> metadataFile.createSigner(this))
         .filter(Objects::nonNull)
         .map(TransactionSigner::getAddress)
         .collect(Collectors.toSet());
   }
 
-  private TransactionSigner createSigner(final FileBasedSigningMetadataFile signingMetadataFile) {
+  @Override
+  public TransactionSigner createSigner(final AzureSigningMetadataFile metadataFile) {
+    final TransactionSigner signer;
+    try {
+      signer = azureFactory.createSigner(metadataFile.getConfig());
+    } catch (final TransactionSignerInitializationException e) {
+      LOG.error("Failed to construct Azure signer from " + metadataFile.getBaseFilename());
+      return null;
+    }
+
+    final String signerAddress = signer.getAddress();
+    if (!signerAddress.equals(metadataFile.getBaseFilename())) {
+      LOG.error(
+          String.format(
+              "Azure signer's Ethereum Address (%s) does not align with metadata filename (%s)",
+              signerAddress, metadataFile.getBaseFilename()));
+      return null;
+    }
+    LOG.info("Loaded signer for address {}", signerAddress);
+    return signer;
+  }
+
+  @Override
+  public TransactionSigner createSigner(final FileBasedSigningMetadataFile metadataFile) {
     try {
       final TransactionSigner signer =
           FileBasedSignerFactory.createSigner(
-              signingMetadataFile.getKeyPath(), signingMetadataFile.getPasswordPath());
-      LOG.debug("Loaded signer with key '{}'", signingMetadataFile.getKeyPath().getFileName());
+              metadataFile.getKeyPath(), metadataFile.getPasswordPath());
+      LOG.debug("Loaded signer with key '{}'", metadataFile.getKeyPath().getFileName());
       return signer;
     } catch (final TransactionSignerInitializationException e) {
-      LOG.warn(
-          "Unable to load signer with key '{}'", signingMetadataFile.getKeyPath().getFileName(), e);
+      LOG.error("Unable to load signer with key " + metadataFile.getKeyPath().getFileName(), e);
       return null;
     }
   }