From 8ed405295833236bf2eeb9135551a3a77560fed9 Mon Sep 17 00:00:00 2001 From: Jason Frame Date: Fri, 7 Jul 2023 11:28:01 +1000 Subject: [PATCH 01/27] Azure secp bulk loading --- .../runner/CmdLineParamsDefaultImpl.java | 48 +++++++----- .../dsl/signer/runner/Web3SignerRunner.java | 14 ++-- .../AzureKeyVaultAcceptanceTest.java | 55 +++++++++----- .../subcommands/Eth1SubCommand.java | 9 +++ .../pegasys/web3signer/core/Eth1Runner.java | 63 +++++++++++++--- .../pegasys/web3signer/core/Eth2Runner.java | 4 +- .../web3signer/core/config/Eth1Config.java | 3 + .../keystorage/azure/AzureKeyVault.java | 25 +++++++ .../AWSBulkLoadingArtifactSignerProvider.java | 4 +- .../BlsKeystoreBulkLoader.java | 4 +- .../bulkloading/SecpAzureBulkLoader.java | 74 +++++++++++++++++++ .../azure/AzureKeyVaultSignerFactory.java | 1 + .../signing/BlsKeystoreBulkLoaderTest.java | 1 + 13 files changed, 246 insertions(+), 59 deletions(-) rename signing/src/main/java/tech/pegasys/web3signer/signing/{ => bulkloading}/AWSBulkLoadingArtifactSignerProvider.java (92%) rename signing/src/main/java/tech/pegasys/web3signer/signing/{ => bulkloading}/BlsKeystoreBulkLoader.java (96%) create mode 100644 signing/src/main/java/tech/pegasys/web3signer/signing/bulkloading/SecpAzureBulkLoader.java diff --git a/acceptance-tests/src/test/java/tech/pegasys/web3signer/dsl/signer/runner/CmdLineParamsDefaultImpl.java b/acceptance-tests/src/test/java/tech/pegasys/web3signer/dsl/signer/runner/CmdLineParamsDefaultImpl.java index f07a2d2e0..a74d419dd 100644 --- a/acceptance-tests/src/test/java/tech/pegasys/web3signer/dsl/signer/runner/CmdLineParamsDefaultImpl.java +++ b/acceptance-tests/src/test/java/tech/pegasys/web3signer/dsl/signer/runner/CmdLineParamsDefaultImpl.java @@ -99,26 +99,7 @@ public List createCmdLineParams() { params.addAll(createEth2Args()); if (signerConfig.getAzureKeyVaultParameters().isPresent()) { - final AzureKeyVaultParameters azureParams = signerConfig.getAzureKeyVaultParameters().get(); - params.add("--azure-vault-enabled=true"); - params.add("--azure-vault-auth-mode"); - params.add(azureParams.getAuthenticationMode().name()); - params.add("--azure-vault-name"); - params.add(azureParams.getKeyVaultName()); - params.add("--azure-client-id"); - params.add(azureParams.getClientId()); - params.add("--azure-client-secret"); - params.add(azureParams.getClientSecret()); - params.add("--azure-tenant-id"); - params.add(azureParams.getTenantId()); - - azureParams - .getTags() - .forEach( - (tagName, tagValue) -> { - params.add("--azure-secrets-tags"); - params.add(tagName + "=" + tagValue); - }); + createAzureArgs(params); } if (signerConfig.getKeystoresParameters().isPresent()) { final KeystoresParameters keystoresParameters = signerConfig.getKeystoresParameters().get(); @@ -143,6 +124,10 @@ public List createCmdLineParams() { params.add("--chain-id"); params.add(Long.toString(signerConfig.getChainIdProvider().id())); params.addAll(createDownstreamTlsArgs()); + + if (signerConfig.getAzureKeyVaultParameters().isPresent()) { + createAzureArgs(params); + } } return params; @@ -331,6 +316,29 @@ private Collection awsBulkLoadingOptions( return params; } + private void createAzureArgs(final List params) { + final AzureKeyVaultParameters azureParams = signerConfig.getAzureKeyVaultParameters().get(); + params.add("--azure-vault-enabled=true"); + params.add("--azure-vault-auth-mode"); + params.add(azureParams.getAuthenticationMode().name()); + params.add("--azure-vault-name"); + params.add(azureParams.getKeyVaultName()); + params.add("--azure-client-id"); + params.add(azureParams.getClientId()); + params.add("--azure-client-secret"); + params.add(azureParams.getClientSecret()); + params.add("--azure-tenant-id"); + params.add(azureParams.getTenantId()); + + azureParams + .getTags() + .forEach( + (tagName, tagValue) -> { + params.add("--azure-secrets-tags"); + params.add(tagName + "=" + tagValue); + }); + } + private List createSubCommandArgs() { final List params = new ArrayList<>(); diff --git a/acceptance-tests/src/test/java/tech/pegasys/web3signer/dsl/signer/runner/Web3SignerRunner.java b/acceptance-tests/src/test/java/tech/pegasys/web3signer/dsl/signer/runner/Web3SignerRunner.java index 2a612bd80..03baedc13 100644 --- a/acceptance-tests/src/test/java/tech/pegasys/web3signer/dsl/signer/runner/Web3SignerRunner.java +++ b/acceptance-tests/src/test/java/tech/pegasys/web3signer/dsl/signer/runner/Web3SignerRunner.java @@ -48,13 +48,13 @@ public abstract class Web3SignerRunner { private static final String METRICS_PORT_KEY = "metrics-port"; public static Web3SignerRunner createRunner(final SignerConfiguration signerConfig) { - if (Boolean.getBoolean("acctests.runWeb3SignerAsProcess")) { - LOG.info("Web3Signer running as a process."); - return new Web3SignerProcessRunner(signerConfig); - } else { - LOG.info("Web3Signer running in a thread."); - return new Web3SignerThreadRunner(signerConfig); - } + // if (Boolean.getBoolean("acctests.runWeb3SignerAsProcess")) { + // LOG.info("Web3Signer running as a process."); + // return new Web3SignerProcessRunner(signerConfig); + // } else { + LOG.info("Web3Signer running in a thread."); + return new Web3SignerThreadRunner(signerConfig); + // } } protected Web3SignerRunner(final SignerConfiguration signerConfig) { diff --git a/acceptance-tests/src/test/java/tech/pegasys/web3signer/tests/bulkloading/AzureKeyVaultAcceptanceTest.java b/acceptance-tests/src/test/java/tech/pegasys/web3signer/tests/bulkloading/AzureKeyVaultAcceptanceTest.java index e01776e62..d1bc0c60a 100644 --- a/acceptance-tests/src/test/java/tech/pegasys/web3signer/tests/bulkloading/AzureKeyVaultAcceptanceTest.java +++ b/acceptance-tests/src/test/java/tech/pegasys/web3signer/tests/bulkloading/AzureKeyVaultAcceptanceTest.java @@ -13,16 +13,21 @@ 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 static org.hamcrest.Matchers.hasItem; +import static org.hamcrest.Matchers.hasItems; import static org.hamcrest.Matchers.hasSize; +import org.hamcrest.Matcher; +import org.hamcrest.Matchers; 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 io.restassured.http.ContentType; @@ -32,6 +37,7 @@ import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.Test; import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.EnumSource; import org.junit.jupiter.params.provider.ValueSource; public class AzureKeyVaultAcceptanceTest extends AcceptanceTestBase { @@ -40,8 +46,11 @@ public class AzureKeyVaultAcceptanceTest extends AcceptanceTestBase { 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 String EXPECTED_KEY = - "0x989d34725a2bfc3f15105f3f5fc8741f436c25ee1ee4f948e425d6bcb8c56bce6e06c269635b7e985a7ffa639e2409bf"; + private static final List BLS_EXPECTED_KEYS = List.of( + "0x989d34725a2bfc3f15105f3f5fc8741f436c25ee1ee4f948e425d6bcb8c56bce6e06c269635b7e985a7ffa639e2409bf"); + + private static final List SECP_EXPECTED_KEYS = List.of("0xfb854fd5249656ecf91d4acfc23209297b47a8e9615209ffa097405cdc53767608edd5d809b56a28c2b864cf601d43e9f8ad27c9d630769bd017a24247cf7482", + "0x964f00253459f1f43c7a7720a0db09a328d4ee6f18838015023135d7fc921f1448de34d05de7a1f72a7b5c9f6c76931d7ab33d0f0846ccce5452063bd20f5809"); @BeforeAll public static void setup() { @@ -51,18 +60,21 @@ public static void setup() { Assumptions.assumeTrue(VAULT_NAME != null, "Set AZURE_KEY_VAULT_NAME environment variable"); } - @Test - void ensureSecretsInKeyVaultAreLoadedAndReportedViaPublicKeysApi() { + @ParameterizedTest + @EnumSource(KeyType.class) + void ensureSecretsInKeyVaultAreLoadedAndReportedViaPublicKeysApi(final KeyType keyType) { final AzureKeyVaultParameters azureParams = new DefaultAzureKeyVaultParameters(VAULT_NAME, CLIENT_ID, TENANT_ID, CLIENT_SECRET); final SignerConfigurationBuilder configBuilder = - new SignerConfigurationBuilder().withMode("eth2").withAzureKeyVaultParameters(azureParams); + new SignerConfigurationBuilder() + .withMode(calculateMode(keyType)) + .withAzureKeyVaultParameters(azureParams); startSigner(configBuilder.build()); - final Response response = signer.callApiPublicKeys(KeyType.BLS); - response.then().statusCode(200).contentType(ContentType.JSON).body("", hasItem(EXPECTED_KEY)); + final Response response = signer.callApiPublicKeys(keyType); + response.then().statusCode(200).contentType(ContentType.JSON).body("", hasItems(expectedKeys(keyType))); // Since our Azure vault contains some invalid keys, the healthcheck would return 503. final Response healthcheckResponse = signer.healthcheck(); @@ -94,7 +106,7 @@ void azureSecretsViaTag(boolean useConfigFile) { startSigner(configBuilder.build()); final Response response = signer.callApiPublicKeys(KeyType.BLS); - response.then().statusCode(200).contentType(ContentType.JSON).body("", hasItem(EXPECTED_KEY)); + response.then().statusCode(200).contentType(ContentType.JSON).body("", hasItem(BLS_EXPECTED_KEYS)); // the tag filter will return only valid keys. The healtcheck should be UP final Response healthcheckResponse = signer.healthcheck(); @@ -113,16 +125,14 @@ void azureSecretsViaTag(boolean useConfigFile) { } private static int getAzureBulkLoadingData(String healthCheckJsonBody, String dataKey) { - JsonObject jsonObject = new JsonObject(healthCheckJsonBody); - int keysLoaded = - 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); - return keysLoaded; + final 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); } @Test @@ -163,6 +173,13 @@ void envVarsAreUsedToDefaultAzureParams() { startSigner(configBuilder.build()); final Response response = signer.callApiPublicKeys(KeyType.BLS); - response.then().statusCode(200).contentType(ContentType.JSON).body("", hasItem(EXPECTED_KEY)); + response.then().statusCode(200).contentType(ContentType.JSON).body("", hasItem(BLS_EXPECTED_KEYS)); + } + + private String[] expectedKeys(final KeyType keyType) { + final List keys = keyType == KeyType.BLS ? + BLS_EXPECTED_KEYS + : SECP_EXPECTED_KEYS; + return keys.toArray(new String[0]); } } diff --git a/commandline/src/main/java/tech/pegasys/web3signer/commandline/subcommands/Eth1SubCommand.java b/commandline/src/main/java/tech/pegasys/web3signer/commandline/subcommands/Eth1SubCommand.java index 837fe5e7f..d413e54f3 100644 --- a/commandline/src/main/java/tech/pegasys/web3signer/commandline/subcommands/Eth1SubCommand.java +++ b/commandline/src/main/java/tech/pegasys/web3signer/commandline/subcommands/Eth1SubCommand.java @@ -18,6 +18,7 @@ import static tech.pegasys.web3signer.commandline.DefaultCommandValues.PORT_FORMAT_HELP; import static tech.pegasys.web3signer.commandline.util.RequiredOptionsUtil.checkIfRequiredOptionsAreInitialized; +import tech.pegasys.web3signer.commandline.PicoCliAzureKeyVaultParameters; import tech.pegasys.web3signer.commandline.annotations.RequiredOption; import tech.pegasys.web3signer.commandline.config.client.PicoCliClientTlsOptions; import tech.pegasys.web3signer.core.Eth1Runner; @@ -26,6 +27,7 @@ import tech.pegasys.web3signer.core.config.client.ClientTlsOptions; import tech.pegasys.web3signer.core.service.jsonrpc.handlers.signing.ChainIdProvider; import tech.pegasys.web3signer.core.service.jsonrpc.handlers.signing.ConfigurationChainId; +import tech.pegasys.web3signer.signing.config.AzureKeyVaultParameters; import java.net.URI; import java.net.URISyntaxException; @@ -140,6 +142,8 @@ public void setDownstreamHttpPath(final String path) { @CommandLine.Mixin private PicoCliClientTlsOptions clientTlsOptions; + @CommandLine.Mixin private PicoCliAzureKeyVaultParameters azureKeyVaultParameters; + @Override public Runner createRunner() { return new Eth1Runner(config, this); @@ -204,4 +208,9 @@ public Optional getClientTlsOptions() { public ChainIdProvider getChainId() { return new ConfigurationChainId(chainId); } + + @Override + public AzureKeyVaultParameters getAzureKeyVaultConfig() { + return azureKeyVaultParameters; + } } diff --git a/core/src/main/java/tech/pegasys/web3signer/core/Eth1Runner.java b/core/src/main/java/tech/pegasys/web3signer/core/Eth1Runner.java index c9997c698..3dec6c7a8 100644 --- a/core/src/main/java/tech/pegasys/web3signer/core/Eth1Runner.java +++ b/core/src/main/java/tech/pegasys/web3signer/core/Eth1Runner.java @@ -12,8 +12,11 @@ */ package tech.pegasys.web3signer.core; +import static tech.pegasys.web3signer.core.config.HealthCheckNames.KEYS_CHECK_AZURE_BULK_LOADING; import static tech.pegasys.web3signer.signing.KeyType.SECP256K1; +import io.vertx.core.json.JsonObject; +import io.vertx.ext.healthchecks.Status; import tech.pegasys.web3signer.core.config.BaseConfig; import tech.pegasys.web3signer.core.config.Eth1Config; import tech.pegasys.web3signer.core.service.DownstreamPathCalculator; @@ -33,10 +36,15 @@ import tech.pegasys.web3signer.core.service.jsonrpc.handlers.internalresponse.EthSignResultProvider; import tech.pegasys.web3signer.core.service.jsonrpc.handlers.internalresponse.EthSignTransactionResultProvider; import tech.pegasys.web3signer.core.service.jsonrpc.handlers.internalresponse.InternalResponseHandler; +import tech.pegasys.web3signer.keystorage.azure.AzureKeyVault; +import tech.pegasys.web3signer.keystorage.common.MappedResults; import tech.pegasys.web3signer.keystorage.hashicorp.HashicorpConnectionFactory; +import tech.pegasys.web3signer.signing.ArtifactSigner; import tech.pegasys.web3signer.signing.ArtifactSignerProvider; import tech.pegasys.web3signer.signing.EthSecpArtifactSigner; import tech.pegasys.web3signer.signing.SecpArtifactSignature; +import tech.pegasys.web3signer.signing.bulkloading.SecpAzureBulkLoader; +import tech.pegasys.web3signer.signing.config.AzureKeyVaultFactory; import tech.pegasys.web3signer.signing.config.DefaultArtifactSignerProvider; import tech.pegasys.web3signer.signing.config.SignerLoader; import tech.pegasys.web3signer.signing.config.metadata.Secp256k1ArtifactSignerFactory; @@ -50,6 +58,7 @@ import com.fasterxml.jackson.databind.DeserializationFeature; import com.fasterxml.jackson.databind.ObjectMapper; +import com.google.common.collect.Lists; import io.vertx.core.Vertx; import io.vertx.core.http.HttpClient; import io.vertx.core.http.HttpMethod; @@ -58,12 +67,15 @@ import io.vertx.ext.web.handler.BodyHandler; import io.vertx.ext.web.handler.ResponseContentTypeHandler; import io.vertx.ext.web.impl.BlockingHandlerDecorator; +import org.apache.logging.log4j.LogManager; +import org.apache.logging.log4j.Logger; import org.hyperledger.besu.plugin.services.MetricsSystem; public class Eth1Runner extends Runner { public static final String PUBLIC_KEYS_PATH = "/api/v1/eth1/publicKeys"; public static final String ROOT_PATH = "/"; public static final String SIGN_PATH = "/api/v1/eth1/sign/:identifier"; + private static final Logger LOG = LogManager.getLogger(); private final Eth1Config eth1Config; private final HttpResponseFactory responseFactory = new HttpResponseFactory(); @@ -134,6 +146,7 @@ protected ArtifactSignerProvider createArtifactSignerProvider( final Vertx vertx, final MetricsSystem metricsSystem) { return new DefaultArtifactSignerProvider( () -> { + final List signers = Lists.newArrayList(); final AzureKeyVaultSignerFactory azureFactory = new AzureKeyVaultSignerFactory(); final HashicorpConnectionFactory hashicorpConnectionFactory = new HashicorpConnectionFactory(); @@ -150,16 +163,34 @@ protected ArtifactSignerProvider createArtifactSignerProvider( EthSecpArtifactSigner::new, true); - return new SignerLoader(baseConfig.keystoreParallelProcessingEnabled()) - .load( - baseConfig.getKeyConfigPath(), - "yaml", - new YamlSignerParser( - List.of(ethSecpArtifactSignerFactory), - YamlMapperFactory.createYamlMapper( - baseConfig.getKeyStoreConfigFileMaxSize()))) - .getValues(); + final MappedResults results = + new SignerLoader(baseConfig.keystoreParallelProcessingEnabled()) + .load( + baseConfig.getKeyConfigPath(), + "yaml", + new YamlSignerParser( + List.of(ethSecpArtifactSignerFactory), + YamlMapperFactory.createYamlMapper( + baseConfig.getKeyStoreConfigFileMaxSize()))); + signers.addAll(results.getValues()); + } + + if (eth1Config.getAzureKeyVaultConfig().isAzureKeyVaultEnabled()) { + LOG.info("Bulk loading keys from Azure key vault ... "); + final AzureKeyVault azureKeyVault = + AzureKeyVaultFactory.createAzureKeyVault(eth1Config.getAzureKeyVaultConfig()); + final SecpAzureBulkLoader secpAzureBulkLoader = + new SecpAzureBulkLoader(azureKeyVault, azureFactory); + final MappedResults azureResult = + secpAzureBulkLoader.load(eth1Config.getAzureKeyVaultConfig()); + LOG.info( + "Keys loaded from Azure: [{}], with error count: [{}]", + azureResult.getValues().size(), + azureResult.getErrorCount()); + registerSignerLoadingHealthCheck(KEYS_CHECK_AZURE_BULK_LOADING, azureResult); + signers.addAll(azureResult.getValues()); } + return signers; }); } @@ -201,4 +232,18 @@ private RequestMapper createRequestMapper( return requestMapper; } + + private void registerSignerLoadingHealthCheck( + final String name, final MappedResults result) { + super.registerHealthCheckProcedure( + name, + promise -> { + final JsonObject statusJson = + new JsonObject() + .put("keys-loaded", result.getValues().size()) + .put("error-count", result.getErrorCount()); + promise.complete( + result.getErrorCount() > 0 ? Status.KO(statusJson) : Status.OK(statusJson)); + }); + } } diff --git a/core/src/main/java/tech/pegasys/web3signer/core/Eth2Runner.java b/core/src/main/java/tech/pegasys/web3signer/core/Eth2Runner.java index 4fa5b35f0..acce8c1d0 100644 --- a/core/src/main/java/tech/pegasys/web3signer/core/Eth2Runner.java +++ b/core/src/main/java/tech/pegasys/web3signer/core/Eth2Runner.java @@ -36,15 +36,15 @@ import tech.pegasys.web3signer.keystorage.azure.AzureKeyVault; import tech.pegasys.web3signer.keystorage.common.MappedResults; import tech.pegasys.web3signer.keystorage.hashicorp.HashicorpConnectionFactory; -import tech.pegasys.web3signer.signing.AWSBulkLoadingArtifactSignerProvider; import tech.pegasys.web3signer.signing.ArtifactSigner; import tech.pegasys.web3signer.signing.ArtifactSignerProvider; import tech.pegasys.web3signer.signing.BlsArtifactSignature; import tech.pegasys.web3signer.signing.BlsArtifactSigner; -import tech.pegasys.web3signer.signing.BlsKeystoreBulkLoader; import tech.pegasys.web3signer.signing.FileValidatorManager; import tech.pegasys.web3signer.signing.KeystoreFileManager; import tech.pegasys.web3signer.signing.ValidatorManager; +import tech.pegasys.web3signer.signing.bulkloading.AWSBulkLoadingArtifactSignerProvider; +import tech.pegasys.web3signer.signing.bulkloading.BlsKeystoreBulkLoader; import tech.pegasys.web3signer.signing.config.AwsSecretsManagerParameters; import tech.pegasys.web3signer.signing.config.AzureKeyVaultFactory; import tech.pegasys.web3signer.signing.config.AzureKeyVaultParameters; diff --git a/core/src/main/java/tech/pegasys/web3signer/core/config/Eth1Config.java b/core/src/main/java/tech/pegasys/web3signer/core/config/Eth1Config.java index 98ed3fabf..febccb5d3 100644 --- a/core/src/main/java/tech/pegasys/web3signer/core/config/Eth1Config.java +++ b/core/src/main/java/tech/pegasys/web3signer/core/config/Eth1Config.java @@ -14,6 +14,7 @@ import tech.pegasys.web3signer.core.config.client.ClientTlsOptions; import tech.pegasys.web3signer.core.service.jsonrpc.handlers.signing.ChainIdProvider; +import tech.pegasys.web3signer.signing.config.AzureKeyVaultParameters; import java.time.Duration; import java.util.Optional; @@ -39,4 +40,6 @@ public interface Eth1Config { Optional getClientTlsOptions(); ChainIdProvider getChainId(); + + AzureKeyVaultParameters getAzureKeyVaultConfig(); } diff --git a/keystorage/src/main/java/tech/pegasys/web3signer/keystorage/azure/AzureKeyVault.java b/keystorage/src/main/java/tech/pegasys/web3signer/keystorage/azure/AzureKeyVault.java index ae9565c26..1ddba16fb 100644 --- a/keystorage/src/main/java/tech/pegasys/web3signer/keystorage/azure/AzureKeyVault.java +++ b/keystorage/src/main/java/tech/pegasys/web3signer/keystorage/azure/AzureKeyVault.java @@ -15,12 +15,14 @@ import tech.pegasys.web3signer.keystorage.common.MappedResults; import tech.pegasys.web3signer.keystorage.common.SecretValueMapperUtil; +import java.util.List; import java.util.Map; import java.util.Optional; import java.util.Set; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.atomic.AtomicInteger; import java.util.function.BiFunction; +import java.util.stream.Collectors; import com.azure.core.credential.TokenCredential; import com.azure.core.exception.ResourceNotFoundException; @@ -31,6 +33,7 @@ import com.azure.security.keyvault.keys.KeyClientBuilder; import com.azure.security.keyvault.keys.cryptography.CryptographyClient; import com.azure.security.keyvault.keys.cryptography.CryptographyClientBuilder; +import com.azure.security.keyvault.keys.models.KeyProperties; import com.azure.security.keyvault.keys.models.KeyVaultKey; import com.azure.security.keyvault.secrets.SecretClient; import com.azure.security.keyvault.secrets.SecretClientBuilder; @@ -148,6 +151,18 @@ public MappedResults mapSecrets( return MappedResults.newInstance(result, errorCount.intValue()); } + public List listKeyNames(final Map tags) { + return keyClient + .listPropertiesOfKeys() + .streamByPage() + .flatMap( + keyPage -> + keyPage.getValue().parallelStream() + .filter(keyProperties -> keyPropertiesPredicate(tags, keyProperties)) + .map(kp -> kp.getName())) + .collect(Collectors.toList()); + } + private static boolean secretPropertiesPredicate( final Map tags, final SecretProperties secretProperties) { if (tags == null || tags.isEmpty()) { @@ -157,4 +172,14 @@ private static boolean secretPropertiesPredicate( return secretProperties.getTags() != null // return false if remote secret doesn't have any tags && secretProperties.getTags().entrySet().containsAll(tags.entrySet()); } + + private static boolean keyPropertiesPredicate( + final Map tags, final KeyProperties keyProperties) { + if (tags == null || tags.isEmpty()) { + return true; // we don't want to filter if user-supplied tags map is empty + } + + return keyProperties.getTags() != null // return false if remote secret doesn't have any tags + && keyProperties.getTags().entrySet().containsAll(tags.entrySet()); + } } diff --git a/signing/src/main/java/tech/pegasys/web3signer/signing/AWSBulkLoadingArtifactSignerProvider.java b/signing/src/main/java/tech/pegasys/web3signer/signing/bulkloading/AWSBulkLoadingArtifactSignerProvider.java similarity index 92% rename from signing/src/main/java/tech/pegasys/web3signer/signing/AWSBulkLoadingArtifactSignerProvider.java rename to signing/src/main/java/tech/pegasys/web3signer/signing/bulkloading/AWSBulkLoadingArtifactSignerProvider.java index 187b5fb31..f33db9b4a 100644 --- a/signing/src/main/java/tech/pegasys/web3signer/signing/AWSBulkLoadingArtifactSignerProvider.java +++ b/signing/src/main/java/tech/pegasys/web3signer/signing/bulkloading/AWSBulkLoadingArtifactSignerProvider.java @@ -10,13 +10,15 @@ * 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.signing; +package tech.pegasys.web3signer.signing.bulkloading; import tech.pegasys.teku.bls.BLSKeyPair; import tech.pegasys.teku.bls.BLSSecretKey; import tech.pegasys.web3signer.keystorage.aws.AwsSecretsManager; import tech.pegasys.web3signer.keystorage.aws.AwsSecretsManagerProvider; import tech.pegasys.web3signer.keystorage.common.MappedResults; +import tech.pegasys.web3signer.signing.ArtifactSigner; +import tech.pegasys.web3signer.signing.BlsArtifactSigner; import tech.pegasys.web3signer.signing.config.AwsSecretsManagerFactory; import tech.pegasys.web3signer.signing.config.AwsSecretsManagerParameters; import tech.pegasys.web3signer.signing.config.metadata.SignerOrigin; diff --git a/signing/src/main/java/tech/pegasys/web3signer/signing/BlsKeystoreBulkLoader.java b/signing/src/main/java/tech/pegasys/web3signer/signing/bulkloading/BlsKeystoreBulkLoader.java similarity index 96% rename from signing/src/main/java/tech/pegasys/web3signer/signing/BlsKeystoreBulkLoader.java rename to signing/src/main/java/tech/pegasys/web3signer/signing/bulkloading/BlsKeystoreBulkLoader.java index c735fa47f..23ec4c722 100644 --- a/signing/src/main/java/tech/pegasys/web3signer/signing/BlsKeystoreBulkLoader.java +++ b/signing/src/main/java/tech/pegasys/web3signer/signing/bulkloading/BlsKeystoreBulkLoader.java @@ -10,7 +10,7 @@ * 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.signing; +package tech.pegasys.web3signer.signing.bulkloading; import tech.pegasys.signers.bls.keystore.KeyStore; import tech.pegasys.signers.bls.keystore.KeyStoreLoader; @@ -19,6 +19,8 @@ import tech.pegasys.teku.bls.BLSKeyPair; import tech.pegasys.teku.bls.BLSSecretKey; import tech.pegasys.web3signer.keystorage.common.MappedResults; +import tech.pegasys.web3signer.signing.ArtifactSigner; +import tech.pegasys.web3signer.signing.BlsArtifactSigner; import tech.pegasys.web3signer.signing.config.metadata.SignerOrigin; import java.io.IOException; diff --git a/signing/src/main/java/tech/pegasys/web3signer/signing/bulkloading/SecpAzureBulkLoader.java b/signing/src/main/java/tech/pegasys/web3signer/signing/bulkloading/SecpAzureBulkLoader.java new file mode 100644 index 000000000..b3d317735 --- /dev/null +++ b/signing/src/main/java/tech/pegasys/web3signer/signing/bulkloading/SecpAzureBulkLoader.java @@ -0,0 +1,74 @@ +/* + * Copyright 2023 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.signing.bulkloading; + +import tech.pegasys.web3signer.keystorage.azure.AzureKeyVault; +import tech.pegasys.web3signer.keystorage.common.MappedResults; +import tech.pegasys.web3signer.signing.ArtifactSigner; +import tech.pegasys.web3signer.signing.EthSecpArtifactSigner; +import tech.pegasys.web3signer.signing.config.AzureKeyVaultParameters; +import tech.pegasys.web3signer.signing.secp256k1.Signer; +import tech.pegasys.web3signer.signing.secp256k1.azure.AzureConfig; +import tech.pegasys.web3signer.signing.secp256k1.azure.AzureKeyVaultSignerFactory; + +import java.util.List; +import java.util.Optional; +import java.util.concurrent.atomic.AtomicInteger; +import java.util.stream.Collectors; + +public class SecpAzureBulkLoader { + + private final AzureKeyVault azureKeyVault; + private final AzureKeyVaultSignerFactory azureKeyVaultSignerFactory; + + public SecpAzureBulkLoader( + final AzureKeyVault azureKeyVault, + final AzureKeyVaultSignerFactory azureKeyVaultSignerFactory) { + this.azureKeyVault = azureKeyVault; + this.azureKeyVaultSignerFactory = azureKeyVaultSignerFactory; + } + + public MappedResults load(final AzureKeyVaultParameters azureKeyVaultParameters) { + final List keyNames = azureKeyVault.listKeyNames(azureKeyVaultParameters.getTags()); + + final AtomicInteger errorCount = new AtomicInteger(0); + final List collect = + keyNames.stream() + .map( + keyName -> { + try { + return Optional.of( + new EthSecpArtifactSigner(createSigner(keyName, azureKeyVaultParameters))); + } catch (final Exception e) { + errorCount.incrementAndGet(); + return Optional.empty(); + } + }) + .flatMap(Optional::stream) + .collect(Collectors.toList()); + + return MappedResults.newInstance(collect, errorCount.get()); + } + + private Signer createSigner( + final String keyName, final AzureKeyVaultParameters azureKeyVaultParameters) { + return azureKeyVaultSignerFactory.createSigner( + new AzureConfig( + azureKeyVaultParameters.getKeyVaultName(), + keyName, + "", + azureKeyVaultParameters.getClientId(), + azureKeyVaultParameters.getClientSecret(), + azureKeyVaultParameters.getTenantId())); + } +} diff --git a/signing/src/main/java/tech/pegasys/web3signer/signing/secp256k1/azure/AzureKeyVaultSignerFactory.java b/signing/src/main/java/tech/pegasys/web3signer/signing/secp256k1/azure/AzureKeyVaultSignerFactory.java index aaf53a5dc..25ee42f72 100644 --- a/signing/src/main/java/tech/pegasys/web3signer/signing/secp256k1/azure/AzureKeyVaultSignerFactory.java +++ b/signing/src/main/java/tech/pegasys/web3signer/signing/secp256k1/azure/AzureKeyVaultSignerFactory.java @@ -80,6 +80,7 @@ public Signer createSigner(final AzureConfig config) { final Bytes rawPublicKey = Bytes.concatenate(Bytes.wrap(jsonWebKey.getX()), Bytes.wrap(jsonWebKey.getY())); final boolean useDeprecatedCurveName = DEPRECATED_CURVE_NAME.equals(curveName); + LOG.info("Created azure vault for key name={}, publicKey={}", config.getKeyName(), rawPublicKey); return new AzureKeyVaultSigner(config, rawPublicKey, needsToHash, useDeprecatedCurveName); } } diff --git a/signing/src/test/java/tech/pegasys/web3signer/signing/BlsKeystoreBulkLoaderTest.java b/signing/src/test/java/tech/pegasys/web3signer/signing/BlsKeystoreBulkLoaderTest.java index 82c528ddb..21702e127 100644 --- a/signing/src/test/java/tech/pegasys/web3signer/signing/BlsKeystoreBulkLoaderTest.java +++ b/signing/src/test/java/tech/pegasys/web3signer/signing/BlsKeystoreBulkLoaderTest.java @@ -18,6 +18,7 @@ import tech.pegasys.web3signer.BLSTestUtil; import tech.pegasys.web3signer.KeystoreUtil; import tech.pegasys.web3signer.keystorage.common.MappedResults; +import tech.pegasys.web3signer.signing.bulkloading.BlsKeystoreBulkLoader; import java.io.IOException; import java.nio.file.Files; From c504fbf22c8ba57270ee1afd1a009f204deccaa0 Mon Sep 17 00:00:00 2001 From: Jason Frame Date: Mon, 10 Jul 2023 15:11:44 +1000 Subject: [PATCH 02/27] Load keys in parallel using mapKeyProperties --- .../keystorage/azure/AzureKeyVault.java | 53 +++++++++++++------ .../bulkloading/SecpAzureBulkLoader.java | 48 +++++------------ 2 files changed, 50 insertions(+), 51 deletions(-) diff --git a/keystorage/src/main/java/tech/pegasys/web3signer/keystorage/azure/AzureKeyVault.java b/keystorage/src/main/java/tech/pegasys/web3signer/keystorage/azure/AzureKeyVault.java index 1ddba16fb..9284d9441 100644 --- a/keystorage/src/main/java/tech/pegasys/web3signer/keystorage/azure/AzureKeyVault.java +++ b/keystorage/src/main/java/tech/pegasys/web3signer/keystorage/azure/AzureKeyVault.java @@ -15,14 +15,13 @@ import tech.pegasys.web3signer.keystorage.common.MappedResults; import tech.pegasys.web3signer.keystorage.common.SecretValueMapperUtil; -import java.util.List; import java.util.Map; import java.util.Optional; import java.util.Set; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.atomic.AtomicInteger; import java.util.function.BiFunction; -import java.util.stream.Collectors; +import java.util.function.Function; import com.azure.core.credential.TokenCredential; import com.azure.core.exception.ResourceNotFoundException; @@ -151,23 +150,46 @@ public MappedResults mapSecrets( return MappedResults.newInstance(result, errorCount.intValue()); } - public List listKeyNames(final Map tags) { - return keyClient - .listPropertiesOfKeys() - .streamByPage() - .flatMap( - keyPage -> - keyPage.getValue().parallelStream() - .filter(keyProperties -> keyPropertiesPredicate(tags, keyProperties)) - .map(kp -> kp.getName())) - .collect(Collectors.toList()); + public MappedResults mapKeyProperties( + final Function mapper, final Map tags) { + final Set result = ConcurrentHashMap.newKeySet(); + final AtomicInteger errorCount = new AtomicInteger(0); + try { + keyClient + .listPropertiesOfKeys() + .streamByPage() + .forEach( + keyPage -> + keyPage.getValue().parallelStream() + .filter(keyProperties -> keyPropertiesPredicate(tags, keyProperties)) + .forEach( + kp -> { + try { + final R value = mapper.apply(kp); + result.add(value); + } catch (final Exception e) { + LOG.warn( + "Failed to map secret '{}' to requested object type.", + kp.getName()); + errorCount.incrementAndGet(); + } + })); + } catch (final Exception e) { + LOG.error("Unexpected error during Azure map-secrets", e); + errorCount.incrementAndGet(); + } + + return MappedResults.newInstance(result, errorCount.intValue()); + } + + private static boolean isEmptyTags(final Map tags) { + return tags == null || tags.isEmpty(); } private static boolean secretPropertiesPredicate( final Map tags, final SecretProperties secretProperties) { - if (tags == null || tags.isEmpty()) { + if (isEmptyTags(tags)) return true; // we don't want to filter if user-supplied tags map is empty - } return secretProperties.getTags() != null // return false if remote secret doesn't have any tags && secretProperties.getTags().entrySet().containsAll(tags.entrySet()); @@ -175,9 +197,8 @@ private static boolean secretPropertiesPredicate( private static boolean keyPropertiesPredicate( final Map tags, final KeyProperties keyProperties) { - if (tags == null || tags.isEmpty()) { + if (isEmptyTags(tags)) return true; // we don't want to filter if user-supplied tags map is empty - } return keyProperties.getTags() != null // return false if remote secret doesn't have any tags && keyProperties.getTags().entrySet().containsAll(tags.entrySet()); diff --git a/signing/src/main/java/tech/pegasys/web3signer/signing/bulkloading/SecpAzureBulkLoader.java b/signing/src/main/java/tech/pegasys/web3signer/signing/bulkloading/SecpAzureBulkLoader.java index b3d317735..9b08c4004 100644 --- a/signing/src/main/java/tech/pegasys/web3signer/signing/bulkloading/SecpAzureBulkLoader.java +++ b/signing/src/main/java/tech/pegasys/web3signer/signing/bulkloading/SecpAzureBulkLoader.java @@ -17,17 +17,10 @@ import tech.pegasys.web3signer.signing.ArtifactSigner; import tech.pegasys.web3signer.signing.EthSecpArtifactSigner; import tech.pegasys.web3signer.signing.config.AzureKeyVaultParameters; -import tech.pegasys.web3signer.signing.secp256k1.Signer; import tech.pegasys.web3signer.signing.secp256k1.azure.AzureConfig; import tech.pegasys.web3signer.signing.secp256k1.azure.AzureKeyVaultSignerFactory; -import java.util.List; -import java.util.Optional; -import java.util.concurrent.atomic.AtomicInteger; -import java.util.stream.Collectors; - public class SecpAzureBulkLoader { - private final AzureKeyVault azureKeyVault; private final AzureKeyVaultSignerFactory azureKeyVaultSignerFactory; @@ -39,36 +32,21 @@ public SecpAzureBulkLoader( } public MappedResults load(final AzureKeyVaultParameters azureKeyVaultParameters) { - final List keyNames = azureKeyVault.listKeyNames(azureKeyVaultParameters.getTags()); - - final AtomicInteger errorCount = new AtomicInteger(0); - final List collect = - keyNames.stream() - .map( - keyName -> { - try { - return Optional.of( - new EthSecpArtifactSigner(createSigner(keyName, azureKeyVaultParameters))); - } catch (final Exception e) { - errorCount.incrementAndGet(); - return Optional.empty(); - } - }) - .flatMap(Optional::stream) - .collect(Collectors.toList()); - - return MappedResults.newInstance(collect, errorCount.get()); + return azureKeyVault.mapKeyProperties( + kp -> createSigner(kp.getName(), azureKeyVaultParameters), + azureKeyVaultParameters.getTags()); } - private Signer createSigner( + private EthSecpArtifactSigner createSigner( final String keyName, final AzureKeyVaultParameters azureKeyVaultParameters) { - return azureKeyVaultSignerFactory.createSigner( - new AzureConfig( - azureKeyVaultParameters.getKeyVaultName(), - keyName, - "", - azureKeyVaultParameters.getClientId(), - azureKeyVaultParameters.getClientSecret(), - azureKeyVaultParameters.getTenantId())); + return new EthSecpArtifactSigner( + azureKeyVaultSignerFactory.createSigner( + new AzureConfig( + azureKeyVaultParameters.getKeyVaultName(), + keyName, + "", + azureKeyVaultParameters.getClientId(), + azureKeyVaultParameters.getClientSecret(), + azureKeyVaultParameters.getTenantId()))); } } From b3977199db3b0b511998fe4491fd8b7ec2e868c8 Mon Sep 17 00:00:00 2001 From: Jason Frame Date: Tue, 11 Jul 2023 10:56:38 +1000 Subject: [PATCH 03/27] AzureKeyVault tests --- .../keystorage/azure/AzureKeyVaultTest.java | 44 +++++++++++++++++++ 1 file changed, 44 insertions(+) diff --git a/keystorage/src/test/java/tech/pegasys/web3signer/keystorage/azure/AzureKeyVaultTest.java b/keystorage/src/test/java/tech/pegasys/web3signer/keystorage/azure/AzureKeyVaultTest.java index d60a39253..4d6f17b41 100644 --- a/keystorage/src/test/java/tech/pegasys/web3signer/keystorage/azure/AzureKeyVaultTest.java +++ b/keystorage/src/test/java/tech/pegasys/web3signer/keystorage/azure/AzureKeyVaultTest.java @@ -23,6 +23,7 @@ import java.util.Map; import java.util.Optional; +import com.azure.security.keyvault.keys.models.KeyProperties; import org.assertj.core.api.Assertions; import org.junit.jupiter.api.Assumptions; import org.junit.jupiter.api.BeforeAll; @@ -98,6 +99,19 @@ void secretsCanBeMappedUsingCustomMappingFunction() { Assertions.assertThat(testKeyEntry.get().getValue()).isEqualTo(EXPECTED_KEY); } + @Test + void keyPropertiesCanBeMappedUsingCustomMappingFunction() { + final AzureKeyVault azureKeyVault = + createUsingClientSecretCredentials(CLIENT_ID, CLIENT_SECRET, TENANT_ID, VAULT_NAME); + + final MappedResults result = + azureKeyVault.mapKeyProperties(KeyProperties::getName, Collections.emptyMap()); + final Collection entries = result.getValues(); + final Optional testKeyEntry = + entries.stream().filter(e -> e.equals("TestKey")).findAny(); + Assertions.assertThat(testKeyEntry).hasValue("TestKey"); + } + @Test void mapSecretsUsingTags() { final AzureKeyVault azureKeyVault = @@ -119,6 +133,20 @@ void mapSecretsUsingTags() { Assertions.assertThat(result.getErrorCount()).isZero(); } + @Test + void mapKeyPropertiesUsingTags() { + final AzureKeyVault azureKeyVault = + createUsingClientSecretCredentials(CLIENT_ID, CLIENT_SECRET, TENANT_ID, VAULT_NAME); + + final MappedResults result = + azureKeyVault.mapKeyProperties( + KeyProperties::getName, Map.of("Used-For", "Signers Acceptance Test")); + final Collection entries = result.getValues(); + final Optional testKeyEntry = + entries.stream().filter(e -> e.equals("TestKey2")).findAny(); + Assertions.assertThat(testKeyEntry).hasValue("TestKey2"); + } + @Test void mapSecretsWhenTagsDoesNotExist() { final AzureKeyVault azureKeyVault = @@ -134,6 +162,22 @@ void mapSecretsWhenTagsDoesNotExist() { Assertions.assertThat(result.getErrorCount()).isZero(); } + @Test + void mapKeyPropertiesWhenTagsDoesNotExist() { + final AzureKeyVault azureKeyVault = + createUsingClientSecretCredentials(CLIENT_ID, CLIENT_SECRET, TENANT_ID, VAULT_NAME); + + final MappedResults result = + azureKeyVault.mapKeyProperties( + KeyProperties::getName, Map.of("INVALID_TAG", "INVALID_TEST")); + + // The key vault is not expected to have any secrets with above tags. + Assertions.assertThat(result.getValues()).isEmpty(); + + // we should not encounter any error count + Assertions.assertThat(result.getErrorCount()).isZero(); + } + @Test void azureVaultThrowsAwayObjectsWhichFailMapper() { final AzureKeyVault azureKeyVault = From 6960fffe96efacf7a6c4159d0a5a361248be3288 Mon Sep 17 00:00:00 2001 From: Jason Frame Date: Tue, 11 Jul 2023 10:56:51 +1000 Subject: [PATCH 04/27] spotless --- .../pegasys/web3signer/core/Eth1Runner.java | 30 +++++++++---------- .../azure/AzureKeyVaultSignerFactory.java | 3 +- 2 files changed, 17 insertions(+), 16 deletions(-) diff --git a/core/src/main/java/tech/pegasys/web3signer/core/Eth1Runner.java b/core/src/main/java/tech/pegasys/web3signer/core/Eth1Runner.java index 3dec6c7a8..029cca18f 100644 --- a/core/src/main/java/tech/pegasys/web3signer/core/Eth1Runner.java +++ b/core/src/main/java/tech/pegasys/web3signer/core/Eth1Runner.java @@ -15,8 +15,6 @@ import static tech.pegasys.web3signer.core.config.HealthCheckNames.KEYS_CHECK_AZURE_BULK_LOADING; import static tech.pegasys.web3signer.signing.KeyType.SECP256K1; -import io.vertx.core.json.JsonObject; -import io.vertx.ext.healthchecks.Status; import tech.pegasys.web3signer.core.config.BaseConfig; import tech.pegasys.web3signer.core.config.Eth1Config; import tech.pegasys.web3signer.core.service.DownstreamPathCalculator; @@ -62,6 +60,8 @@ import io.vertx.core.Vertx; import io.vertx.core.http.HttpClient; import io.vertx.core.http.HttpMethod; +import io.vertx.core.json.JsonObject; +import io.vertx.ext.healthchecks.Status; import io.vertx.ext.web.Router; import io.vertx.ext.web.client.WebClientOptions; import io.vertx.ext.web.handler.BodyHandler; @@ -184,9 +184,9 @@ protected ArtifactSignerProvider createArtifactSignerProvider( final MappedResults azureResult = secpAzureBulkLoader.load(eth1Config.getAzureKeyVaultConfig()); LOG.info( - "Keys loaded from Azure: [{}], with error count: [{}]", - azureResult.getValues().size(), - azureResult.getErrorCount()); + "Keys loaded from Azure: [{}], with error count: [{}]", + azureResult.getValues().size(), + azureResult.getErrorCount()); registerSignerLoadingHealthCheck(KEYS_CHECK_AZURE_BULK_LOADING, azureResult); signers.addAll(azureResult.getValues()); } @@ -234,16 +234,16 @@ private RequestMapper createRequestMapper( } private void registerSignerLoadingHealthCheck( - final String name, final MappedResults result) { + final String name, final MappedResults result) { super.registerHealthCheckProcedure( - name, - promise -> { - final JsonObject statusJson = - new JsonObject() - .put("keys-loaded", result.getValues().size()) - .put("error-count", result.getErrorCount()); - promise.complete( - result.getErrorCount() > 0 ? Status.KO(statusJson) : Status.OK(statusJson)); - }); + name, + promise -> { + final JsonObject statusJson = + new JsonObject() + .put("keys-loaded", result.getValues().size()) + .put("error-count", result.getErrorCount()); + promise.complete( + result.getErrorCount() > 0 ? Status.KO(statusJson) : Status.OK(statusJson)); + }); } } diff --git a/signing/src/main/java/tech/pegasys/web3signer/signing/secp256k1/azure/AzureKeyVaultSignerFactory.java b/signing/src/main/java/tech/pegasys/web3signer/signing/secp256k1/azure/AzureKeyVaultSignerFactory.java index 25ee42f72..73ea9dfc2 100644 --- a/signing/src/main/java/tech/pegasys/web3signer/signing/secp256k1/azure/AzureKeyVaultSignerFactory.java +++ b/signing/src/main/java/tech/pegasys/web3signer/signing/secp256k1/azure/AzureKeyVaultSignerFactory.java @@ -80,7 +80,8 @@ public Signer createSigner(final AzureConfig config) { final Bytes rawPublicKey = Bytes.concatenate(Bytes.wrap(jsonWebKey.getX()), Bytes.wrap(jsonWebKey.getY())); final boolean useDeprecatedCurveName = DEPRECATED_CURVE_NAME.equals(curveName); - LOG.info("Created azure vault for key name={}, publicKey={}", config.getKeyName(), rawPublicKey); + LOG.info( + "Created azure vault for key name={}, publicKey={}", config.getKeyName(), rawPublicKey); return new AzureKeyVaultSigner(config, rawPublicKey, needsToHash, useDeprecatedCurveName); } } From a32247adb5eb1f4eceaa4cf75295567de05da552 Mon Sep 17 00:00:00 2001 From: Jason Frame Date: Tue, 11 Jul 2023 13:05:17 +1000 Subject: [PATCH 05/27] remove forkjoin pool hack --- .../keystorage/azure/AzureKeyVault.java | 2 ++ .../signing/config/SignerLoader.java | 31 +++---------------- 2 files changed, 6 insertions(+), 27 deletions(-) diff --git a/keystorage/src/main/java/tech/pegasys/web3signer/keystorage/azure/AzureKeyVault.java b/keystorage/src/main/java/tech/pegasys/web3signer/keystorage/azure/AzureKeyVault.java index 9284d9441..6c8332867 100644 --- a/keystorage/src/main/java/tech/pegasys/web3signer/keystorage/azure/AzureKeyVault.java +++ b/keystorage/src/main/java/tech/pegasys/web3signer/keystorage/azure/AzureKeyVault.java @@ -19,6 +19,7 @@ import java.util.Optional; import java.util.Set; import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.Executors; import java.util.concurrent.atomic.AtomicInteger; import java.util.function.BiFunction; import java.util.function.Function; @@ -59,6 +60,7 @@ public static AzureKeyVault createUsingClientSecretCredentials( .clientId(clientId) .clientSecret(clientSecret) .tenantId(tenantId) + .executorService(Executors.newCachedThreadPool()) .build(); return new AzureKeyVault(tokenCredential, vaultName); diff --git a/signing/src/main/java/tech/pegasys/web3signer/signing/config/SignerLoader.java b/signing/src/main/java/tech/pegasys/web3signer/signing/config/SignerLoader.java index fb170d8ca..42a2a797d 100644 --- a/signing/src/main/java/tech/pegasys/web3signer/signing/config/SignerLoader.java +++ b/signing/src/main/java/tech/pegasys/web3signer/signing/config/SignerLoader.java @@ -31,7 +31,6 @@ import java.util.Map; import java.util.Objects; import java.util.Set; -import java.util.concurrent.ForkJoinPool; import java.util.concurrent.atomic.AtomicInteger; import java.util.concurrent.atomic.AtomicLong; import java.util.stream.Collectors; @@ -47,9 +46,9 @@ public class SignerLoader { private static final Logger LOG = LogManager.getLogger(); private static final long FILES_PROCESSED_TO_REPORT = 10; - private static final int MAX_FORK_JOIN_THREADS = 5; - // enable or disable parallel streams to convert and load private keys from metadata files - private final boolean useParallelStreams; + private final boolean + useParallelStreams; // whether to parallel streams to convert and load private keys from + // metadata files private static final Map metadataConfigFilesPathCache = new HashMap<>(); @@ -195,25 +194,15 @@ private MappedResults mapMetadataToArtifactSigner( "Converting signing metadata to Artifact Signer using {} streams ...", useParallelStreams ? "parallel" : "sequential"); - // use custom fork-join pool instead of common. Limit number of threads to avoid Azure bug - ForkJoinPool forkJoinPool = null; try { if (useParallelStreams) { - forkJoinPool = new ForkJoinPool(numberOfThreads()); - return forkJoinPool - .submit( - () -> mapToArtifactSigner(signingMetadataCollection.parallelStream(), signerParser)) - .get(); + return mapToArtifactSigner(signingMetadataCollection.parallelStream(), signerParser); } else { return mapToArtifactSigner(signingMetadataCollection.stream(), signerParser); } } catch (final Exception e) { LOG.error("Unexpected error in processing configuration files: {}", e.getMessage(), e); return MappedResults.errorResult(); - } finally { - if (forkJoinPool != null) { - forkJoinPool.shutdown(); - } } } @@ -258,16 +247,4 @@ private void renderException(final Throwable t, final String filename) { ExceptionUtils.getRootCauseMessage(t)); LOG.debug(ExceptionUtils.getStackTrace(t)); } - - private int numberOfThreads() { - // try to allocate between 1-5 threads (based on processor cores) to process files in parallel - int defaultNumberOfThreads = Runtime.getRuntime().availableProcessors() / 2; - - if (defaultNumberOfThreads >= MAX_FORK_JOIN_THREADS) { - return MAX_FORK_JOIN_THREADS; - } else if (defaultNumberOfThreads < 1) { - return 1; - } - return defaultNumberOfThreads; - } } From 5858e758067487bc080506e695a34919b5a82b01 Mon Sep 17 00:00:00 2001 From: Jason Frame Date: Tue, 18 Jul 2023 13:07:24 +1000 Subject: [PATCH 06/27] add additional eth1 secp tests to AzureKeyVaultAcceptanceTest --- .../AzureKeyVaultAcceptanceTest.java | 95 ++++++++++++------- 1 file changed, 59 insertions(+), 36 deletions(-) diff --git a/acceptance-tests/src/test/java/tech/pegasys/web3signer/tests/bulkloading/AzureKeyVaultAcceptanceTest.java b/acceptance-tests/src/test/java/tech/pegasys/web3signer/tests/bulkloading/AzureKeyVaultAcceptanceTest.java index d1bc0c60a..f140cd717 100644 --- a/acceptance-tests/src/test/java/tech/pegasys/web3signer/tests/bulkloading/AzureKeyVaultAcceptanceTest.java +++ b/acceptance-tests/src/test/java/tech/pegasys/web3signer/tests/bulkloading/AzureKeyVaultAcceptanceTest.java @@ -13,14 +13,10 @@ 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 static org.hamcrest.Matchers.hasItem; import static org.hamcrest.Matchers.hasItems; import static org.hamcrest.Matchers.hasSize; -import org.hamcrest.Matcher; -import org.hamcrest.Matchers; import tech.pegasys.web3signer.dsl.signer.SignerConfigurationBuilder; import tech.pegasys.web3signer.dsl.utils.DefaultAzureKeyVaultParameters; import tech.pegasys.web3signer.signing.KeyType; @@ -29,16 +25,17 @@ import java.util.List; import java.util.Map; +import java.util.stream.Stream; import io.restassured.http.ContentType; import io.restassured.response.Response; import io.vertx.core.json.JsonObject; import org.junit.jupiter.api.Assumptions; import org.junit.jupiter.api.BeforeAll; -import org.junit.jupiter.api.Test; import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.Arguments; import org.junit.jupiter.params.provider.EnumSource; -import org.junit.jupiter.params.provider.ValueSource; +import org.junit.jupiter.params.provider.MethodSource; public class AzureKeyVaultAcceptanceTest extends AcceptanceTestBase { @@ -46,11 +43,15 @@ public class AzureKeyVaultAcceptanceTest extends AcceptanceTestBase { 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 List BLS_EXPECTED_KEYS = List.of( - "0x989d34725a2bfc3f15105f3f5fc8741f436c25ee1ee4f948e425d6bcb8c56bce6e06c269635b7e985a7ffa639e2409bf"); + private static final String[] BLS_EXPECTED_KEYS = + List.of( + "0x989d34725a2bfc3f15105f3f5fc8741f436c25ee1ee4f948e425d6bcb8c56bce6e06c269635b7e985a7ffa639e2409bf") + .toArray(new String[0]); - private static final List SECP_EXPECTED_KEYS = List.of("0xfb854fd5249656ecf91d4acfc23209297b47a8e9615209ffa097405cdc53767608edd5d809b56a28c2b864cf601d43e9f8ad27c9d630769bd017a24247cf7482", - "0x964f00253459f1f43c7a7720a0db09a328d4ee6f18838015023135d7fc921f1448de34d05de7a1f72a7b5c9f6c76931d7ab33d0f0846ccce5452063bd20f5809"); + private static final String[] SECP_EXPECTED_KEYS = + List.of( + "0xa95663509e608da3c2af5a48eb4315321f8430cbed5518a44590cc9d367f01dc72ebbc583fc7d94f9fdc20eb6e162c9f8cb35be8a91a3b1d32a63ecc10be4e08") + .toArray(new String[0]); @BeforeAll public static void setup() { @@ -74,7 +75,11 @@ void ensureSecretsInKeyVaultAreLoadedAndReportedViaPublicKeysApi(final KeyType k startSigner(configBuilder.build()); final Response response = signer.callApiPublicKeys(keyType); - response.then().statusCode(200).contentType(ContentType.JSON).body("", hasItems(expectedKeys(keyType))); + response + .then() + .statusCode(200) + .contentType(ContentType.JSON) + .body("", hasItems(expectedKeys(keyType))); // Since our Azure vault contains some invalid keys, the healthcheck would return 503. final Response healthcheckResponse = signer.healthcheck(); @@ -90,25 +95,29 @@ void ensureSecretsInKeyVaultAreLoadedAndReportedViaPublicKeysApi(final KeyType k assertThat(keysLoaded).isGreaterThanOrEqualTo(1); } - @ParameterizedTest(name = "{index} - Using config file: {0}") - @ValueSource(booleans = {true, false}) - void azureSecretsViaTag(boolean useConfigFile) { + @ParameterizedTest(name = "{index} - KeyType: {0}, using config file: {1}") + @MethodSource("azureSecretsViaTag") + void azureSecretsViaTag(final KeyType keyType, boolean useConfigFile) { final AzureKeyVaultParameters azureParams = new DefaultAzureKeyVaultParameters( VAULT_NAME, CLIENT_ID, TENANT_ID, CLIENT_SECRET, Map.of("ENV", "TEST")); final SignerConfigurationBuilder configBuilder = new SignerConfigurationBuilder() - .withMode("eth2") + .withMode(calculateMode(keyType)) .withUseConfigFile(useConfigFile) .withAzureKeyVaultParameters(azureParams); startSigner(configBuilder.build()); - final Response response = signer.callApiPublicKeys(KeyType.BLS); - response.then().statusCode(200).contentType(ContentType.JSON).body("", hasItem(BLS_EXPECTED_KEYS)); + final Response response = signer.callApiPublicKeys(keyType); + response + .then() + .statusCode(200) + .contentType(ContentType.JSON) + .body("", hasItems(expectedKeys(keyType))); - // the tag filter will return only valid keys. The healtcheck should be UP + // the tag filter will return only valid keys. The healthcheck should be UP final Response healthcheckResponse = signer.healthcheck(); healthcheckResponse .then() @@ -124,6 +133,14 @@ void azureSecretsViaTag(boolean useConfigFile) { assertThat(errorCount).isZero(); } + private static Stream azureSecretsViaTag() { + return Stream.of( + Arguments.arguments(KeyType.BLS, false), + Arguments.arguments(KeyType.BLS, true), + Arguments.arguments(KeyType.SECP256K1, false), + Arguments.arguments(KeyType.SECP256K1, true)); + } + private static int getAzureBulkLoadingData(String healthCheckJsonBody, String dataKey) { final JsonObject jsonObject = new JsonObject(healthCheckJsonBody); return jsonObject.getJsonArray("checks").stream() @@ -135,17 +152,20 @@ private static int getAzureBulkLoadingData(String healthCheckJsonBody, String da .orElse(-1); } - @Test - void invalidVaultParametersFailsToLoadKeys() { + @ParameterizedTest + @EnumSource(KeyType.class) + void invalidVaultParametersFailsToLoadKeys(final KeyType keyType) { final AzureKeyVaultParameters azureParams = new DefaultAzureKeyVaultParameters("nonExistentVault", CLIENT_ID, TENANT_ID, CLIENT_SECRET); final SignerConfigurationBuilder configBuilder = - new SignerConfigurationBuilder().withMode("eth2").withAzureKeyVaultParameters(azureParams); + new SignerConfigurationBuilder() + .withMode(calculateMode(keyType)) + .withAzureKeyVaultParameters(azureParams); startSigner(configBuilder.build()); - final Response response = signer.callApiPublicKeys(KeyType.BLS); + final Response response = signer.callApiPublicKeys(keyType); response.then().statusCode(200).contentType(ContentType.JSON).body("", hasSize(0)); signer @@ -156,30 +176,33 @@ void invalidVaultParametersFailsToLoadKeys() { .body("status", equalTo("DOWN")); } - @Test - void envVarsAreUsedToDefaultAzureParams() { + @ParameterizedTest + @EnumSource(KeyType.class) + void envVarsAreUsedToDefaultAzureParams(final KeyType keyType) { // This ensures env vars correspond to the WEB3SIGNER__