Azure support multiline secret values (#827)

Implements #780

CHANGELOG.md

@@ -2,6 +2,7 @@
 
 ## Next release
 ### Features Added
+- Azure bulk mode support for loading multiline (`\n` delimited, up to 200) keys per secret.
 - Hashicorp connection properties can now override http protocol to HTTP/1.1 from the default of HTTP/2. [#817](https://github.com/ConsenSys/web3signer/pull/817)
 - Add --key-config-path as preferred alias to --key-store-path [#826](https://github.com/Consensys/web3signer/pull/826)
 - Add eth_SignTransaction RPC method under the eth1 subcommand [#822](https://github.com/ConsenSys/web3signer/pull/822)

acceptance-tests/build.gradle

@@ -76,6 +76,10 @@ dependencies {
   testImplementation 'io.zonky.test:embedded-postgres'
   testImplementation 'org.bouncycastle:bcprov-jdk18on'
 
+  testImplementation 'com.azure:azure-identity'
+  testImplementation 'com.azure:azure-security-keyvault-keys'
+  testImplementation 'com.azure:azure-security-keyvault-secrets'
+
   gatlingImplementation configurations.getByName("testImplementation")
 
   implementation 'software.amazon.awssdk:auth'

acceptance-tests/src/test/java/tech/pegasys/web3signer/tests/bulkloading/AzureKeyVaultAcceptanceTest.java

@@ -13,8 +13,8 @@
 package tech.pegasys.web3signer.tests.bulkloading;
 
 import static org.assertj.core.api.Assertions.assertThat;
-import static org.hamcrest.Matchers.contains;
 import static org.hamcrest.Matchers.equalTo;
+import static org.hamcrest.Matchers.hasItem;
 import static org.hamcrest.Matchers.hasSize;
 
 import tech.pegasys.web3signer.dsl.signer.SignerConfigurationBuilder;
@@ -62,7 +62,7 @@ void ensureSecretsInKeyVaultAreLoadedAndReportedViaPublicKeysApi() {
     startSigner(configBuilder.build());
 
     final Response response = signer.callApiPublicKeys(KeyType.BLS);
-    response.then().statusCode(200).contentType(ContentType.JSON).body("", contains(EXPECTED_KEY));
+    response.then().statusCode(200).contentType(ContentType.JSON).body("", hasItem(EXPECTED_KEY));
 
     // Since our Azure vault contains some invalid keys, the healthcheck would return 503.
     final Response healthcheckResponse = signer.healthcheck();
@@ -72,10 +72,10 @@ void ensureSecretsInKeyVaultAreLoadedAndReportedViaPublicKeysApi() {
         .contentType(ContentType.JSON)
         .body("status", equalTo("DOWN"));
 
-    // keys loaded would still be 1 though
+    // keys loaded would still be >= 1 though
     final String jsonBody = healthcheckResponse.body().asString();
     int keysLoaded = getAzureBulkLoadingData(jsonBody, "keys-loaded");
-    assertThat(keysLoaded).isEqualTo(1);
+    assertThat(keysLoaded).isGreaterThanOrEqualTo(1);
   }
 
   @ParameterizedTest(name = "{index} - Using config file: {0}")
@@ -94,7 +94,7 @@ void azureSecretsViaTag(boolean useConfigFile) {
     startSigner(configBuilder.build());
 
     final Response response = signer.callApiPublicKeys(KeyType.BLS);
-    response.then().statusCode(200).contentType(ContentType.JSON).body("", contains(EXPECTED_KEY));
+    response.then().statusCode(200).contentType(ContentType.JSON).body("", hasItem(EXPECTED_KEY));
 
     // the tag filter will return only valid keys. The healtcheck should be UP
     final Response healthcheckResponse = signer.healthcheck();
@@ -163,6 +163,6 @@ void envVarsAreUsedToDefaultAzureParams() {
     startSigner(configBuilder.build());
 
     final Response response = signer.callApiPublicKeys(KeyType.BLS);
-    response.then().statusCode(200).contentType(ContentType.JSON).body("", contains(EXPECTED_KEY));
+    response.then().statusCode(200).contentType(ContentType.JSON).body("", hasItem(EXPECTED_KEY));
   }
 }

acceptance-tests/src/test/java/tech/pegasys/web3signer/tests/bulkloading/AzureKeyVaultMultiValueAcceptanceTest.java

@@ -0,0 +1,187 @@
+/*
+ * Copyright 2020 ConsenSys AG.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on
+ * an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations under the License.
+ */
+package tech.pegasys.web3signer.tests.bulkloading;
+
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.hamcrest.Matchers.containsInAnyOrder;
+import static org.hamcrest.Matchers.equalTo;
+
+import tech.pegasys.teku.bls.BLSKeyPair;
+import tech.pegasys.teku.bls.BLSPublicKey;
+import tech.pegasys.teku.bls.BLSSecretKey;
+import tech.pegasys.web3signer.BLSTestUtil;
+import tech.pegasys.web3signer.dsl.signer.SignerConfigurationBuilder;
+import tech.pegasys.web3signer.dsl.utils.DefaultAzureKeyVaultParameters;
+import tech.pegasys.web3signer.signing.KeyType;
+import tech.pegasys.web3signer.signing.config.AzureKeyVaultParameters;
+import tech.pegasys.web3signer.tests.AcceptanceTestBase;
+
+import java.util.List;
+import java.util.Map;
+import java.util.stream.Collectors;
+
+import com.azure.core.credential.TokenCredential;
+import com.azure.core.exception.ResourceNotFoundException;
+import com.azure.core.util.polling.SyncPoller;
+import com.azure.identity.ClientSecretCredentialBuilder;
+import com.azure.security.keyvault.secrets.SecretClient;
+import com.azure.security.keyvault.secrets.SecretClientBuilder;
+import com.azure.security.keyvault.secrets.models.DeletedSecret;
+import com.azure.security.keyvault.secrets.models.KeyVaultSecret;
+import com.azure.security.keyvault.secrets.models.SecretProperties;
+import io.restassured.http.ContentType;
+import io.restassured.response.Response;
+import io.vertx.core.json.JsonObject;
+import org.apache.tuweni.bytes.Bytes32;
+import org.junit.jupiter.api.AfterAll;
+import org.junit.jupiter.api.BeforeAll;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.condition.EnabledIfEnvironmentVariable;
+
+@EnabledIfEnvironmentVariable(
+    named = "AZURE_CLIENT_ID",
+    matches = ".*",
+    disabledReason = "AZURE_CLIENT_ID env variable is required")
+@EnabledIfEnvironmentVariable(
+    named = "AZURE_CLIENT_SECRET",
+    matches = ".*",
+    disabledReason = "AZURE_CLIENT_SECRET env variable is required")
+@EnabledIfEnvironmentVariable(
+    named = "AZURE_TENANT_ID",
+    matches = ".*",
+    disabledReason = "AZURE_TENANT_ID env variable is required")
+@EnabledIfEnvironmentVariable(
+    named = "AZURE_KEY_VAULT_NAME",
+    matches = ".*",
+    disabledReason = "AZURE_KEY_VAULT_NAME env variable is required")
+public class AzureKeyVaultMultiValueAcceptanceTest extends AcceptanceTestBase {
+
+  private static final String CLIENT_ID = System.getenv("AZURE_CLIENT_ID");
+  private static final String CLIENT_SECRET = System.getenv("AZURE_CLIENT_SECRET");
+  private static final String TENANT_ID = System.getenv("AZURE_TENANT_ID");
+  private static final String VAULT_NAME = System.getenv("AZURE_KEY_VAULT_NAME");
+
+  private static final Map<String, String> TAG_FILTER = Map.of("ENV", "MULTILINE-TEST");
+  private static final String SECRET_NAME = "TEST-MULTILINE-KEY";
+
+  private static List<BLSKeyPair> multiValueKeys;
+
+  @BeforeAll
+  static void setup() {
+    multiValueKeys = findAndCreateAzureMultiValueKeysIfNotExist();
+    assertThat(multiValueKeys).hasSize(200);
+  }
+
+  @AfterAll
+  static void cleanupAzureResources() {
+    final SecretClient azureSecretClient = buildAzureSecretClient();
+    final SyncPoller<DeletedSecret, Void> deletedSecretVoidSyncPoller =
+        azureSecretClient.beginDeleteSecret(SECRET_NAME);
+    deletedSecretVoidSyncPoller.poll();
+    deletedSecretVoidSyncPoller.waitForCompletion();
+  }
+
+  @Test
+  void ensureMultiValueSecretsAreBulkLoadedAndReportedViaPublicKeysApi() {
+    // filter based on tag
+
+    final AzureKeyVaultParameters azureParams =
+        new DefaultAzureKeyVaultParameters(
+            VAULT_NAME, CLIENT_ID, TENANT_ID, CLIENT_SECRET, TAG_FILTER);
+
+    final SignerConfigurationBuilder configBuilder =
+        new SignerConfigurationBuilder().withMode("eth2").withAzureKeyVaultParameters(azureParams);
+
+    startSigner(configBuilder.build());
+
+    final List<String> publicKeys =
+        multiValueKeys.stream()
+            .map(BLSKeyPair::getPublicKey)
+            .map(BLSPublicKey::toHexString)
+            .collect(Collectors.toList());
+
+    signer
+        .callApiPublicKeys(KeyType.BLS)
+        .then()
+        .statusCode(200)
+        .contentType(ContentType.JSON)
+        .body("", containsInAnyOrder(publicKeys.toArray(String[]::new)));
+
+    // the tag filter will return only valid keys. The healtcheck should be UP
+    final Response healthcheckResponse = signer.healthcheck();
+    healthcheckResponse
+        .then()
+        .statusCode(200)
+        .contentType(ContentType.JSON)
+        .body("status", equalTo("UP"));
+
+    // keys loaded reported in healthcheck response should total of be multiline keys
+    final String jsonBody = healthcheckResponse.body().asString();
+    int keysLoaded = getHealthCheckAzureBulkLoadKeysCountStat(jsonBody, "keys-loaded");
+    assertThat(keysLoaded).isEqualTo(publicKeys.size());
+  }
+
+  private static int getHealthCheckAzureBulkLoadKeysCountStat(
+      String healthCheckJsonBody, String dataKey) {
+    JsonObject jsonObject = new JsonObject(healthCheckJsonBody);
+    return jsonObject.getJsonArray("checks").stream()
+        .filter(o -> "keys-check".equals(((JsonObject) o).getString("id")))
+        .flatMap(o -> ((JsonObject) o).getJsonArray("checks").stream())
+        .filter(o -> "azure-bulk-loading".equals(((JsonObject) o).getString("id")))
+        .mapToInt(o -> ((JsonObject) ((JsonObject) o).getValue("data")).getInteger(dataKey))
+        .findFirst()
+        .orElse(-1);
+  }
+
+  private static List<BLSKeyPair> findAndCreateAzureMultiValueKeysIfNotExist() {
+    final SecretClient azureSecretClient = buildAzureSecretClient();
+    try {
+      final String multiValueSecrets = azureSecretClient.getSecret(SECRET_NAME).getValue();
+      return multiValueSecrets
+          .lines()
+          .map(key -> new BLSKeyPair(BLSSecretKey.fromBytes(Bytes32.fromHexString(key))))
+          .collect(Collectors.toList());
+    } catch (final ResourceNotFoundException e) {
+      final StringBuilder multilineSecret = new StringBuilder();
+      for (int i = 0; i < 200; i++) {
+        multilineSecret
+            .append(BLSTestUtil.randomKeyPair(i).getSecretKey().toBytes().toHexString())
+            .append("\n");
+      }
+      // create multiline secrets
+      final KeyVaultSecret keyVaultSecret =
+          new KeyVaultSecret(SECRET_NAME, multilineSecret.toString());
+      final SecretProperties secretProperties = new SecretProperties();
+      secretProperties.setTags(TAG_FILTER);
+      keyVaultSecret.setProperties(secretProperties);
+
+      return azureSecretClient
+          .setSecret(keyVaultSecret)
+          .getValue()
+          .lines()
+          .map(key -> new BLSKeyPair(BLSSecretKey.fromBytes(Bytes32.fromHexString(key))))
+          .collect(Collectors.toList());
+    }
+  }
+
+  private static SecretClient buildAzureSecretClient() {
+    final TokenCredential tokenCredential =
+        new ClientSecretCredentialBuilder()
+            .clientId(CLIENT_ID)
+            .clientSecret(CLIENT_SECRET)
+            .tenantId(TENANT_ID)
+            .build();
+    final String vaultUrl = String.format("https://%s.vault.azure.net", VAULT_NAME);
+    return new SecretClientBuilder().vaultUrl(vaultUrl).credential(tokenCredential).buildClient();
+  }
+}

core/src/main/java/tech/pegasys/web3signer/core/Eth2Runner.java

@@ -273,6 +273,11 @@ protected ArtifactSignerProvider createArtifactSignerProvider(
 
           if (azureKeyVaultParameters.isAzureKeyVaultEnabled()) {
             LOG.info("Bulk loading keys from Azure key vault ... ");
+            /*
+             Note: Azure supports 25K bytes per secret. https://learn.microsoft.com/en-us/azure/key-vault/secrets/about-secrets
+             Each raw bls private key in hex format is approximately 100 bytes. We should store about 200 or fewer
+             `\n` delimited keys per secret.
+            */
             final MappedResults<ArtifactSigner> azureResult = loadAzureSigners();
             LOG.info(
                 "Keys loaded from Azure: [{}], with error count: [{}]",
@@ -397,7 +402,10 @@ final MappedResults<ArtifactSigner> loadAzureSigners() {
                 new BLSKeyPair(BLSSecretKey.fromBytes(Bytes32.wrap(privateKeyBytes)));
             return new BlsArtifactSigner(keyPair, SignerOrigin.AZURE);
           } catch (final Exception e) {
-            LOG.error("Failed to load secret named {} from azure key vault.", name);
+            LOG.error(
+                "Failed to load secret named {} from azure key vault due to: {}.",
+                name,
+                e.getMessage());
             return null;
           }
         },

keystorage/src/main/java/tech/pegasys/web3signer/keystorage/azure/AzureKeyVault.java

@@ -13,6 +13,7 @@
 package tech.pegasys.web3signer.keystorage.azure;
 
 import tech.pegasys.web3signer.keystorage.common.MappedResults;
+import tech.pegasys.web3signer.keystorage.common.SecretValueMapperUtil;
 
 import java.util.Map;
 import java.util.Optional;
@@ -127,15 +128,11 @@ public <R> MappedResults<R> mapSecrets(
                           sp -> {
                             try {
                               final KeyVaultSecret secret = secretClient.getSecret(sp.getName());
-                              final R obj = mapper.apply(sp.getName(), secret.getValue());
-                              if (obj != null) {
-                                result.add(obj);
-                              } else {
-                                LOG.warn(
-                                    "Mapped '{}' to a null object, and was discarded",
-                                    sp.getName());
-                                errorCount.incrementAndGet();
-                              }
+                              final MappedResults<R> multiResult =
+                                  SecretValueMapperUtil.mapSecretValue(
+                                      mapper, sp.getName(), secret.getValue());
+                              result.addAll(multiResult.getValues());
+                              errorCount.addAndGet(multiResult.getErrorCount());
                             } catch (final Exception e) {
                               LOG.warn(
                                   "Failed to map secret '{}' to requested object type.",

keystorage/src/main/java/tech/pegasys/web3signer/keystorage/common/SecretValueMapperUtil.java

@@ -28,6 +28,12 @@ public class SecretValueMapperUtil {
   public static <R> MappedResults<R> mapSecretValue(
       BiFunction<String, String, R> mapper, String secretName, String secretValue) {
     final AtomicInteger errorCount = new AtomicInteger(0);
+
+    // secretValue is the value received from remote vault. It should not be null.
+    if (secretValue == null) {
+      return MappedResults.errorResult();
+    }
+
     final Set<R> result =
         Streams.mapWithIndex(
                 secretValue.lines(),