Skip to content

Commit

Permalink
fix: use Map for tags for AWS secrets manager and kms (Consensys#1055)
Browse files Browse the repository at this point in the history
* Use Map for tags for AWS secrets manager and kms
* fix CmdLineParamsConfigFileImpl tags creation in dsl
* Use split regexp with tags Map option
* Fix Yaml Config File Parser to apply map values
* remove aws kms tag tests which are not relavant
* Update Aws KMS Acceptance Test.
* Fix DSL yaml config file generation to use YAML ObjectMapper instead of manual file creation
* Fix cmdline config file impl dsl
* Add changelog
* Removing tests which are not valid
* code cleanup
  • Loading branch information
usmansaleem authored Feb 4, 2025
1 parent da7dda0 commit 65ce049
Show file tree
Hide file tree
Showing 24 changed files with 581 additions and 752 deletions.
24 changes: 21 additions & 3 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,13 +6,31 @@
- The behavior of reload API endpoint has been modified due to issue [#1018][issue_1018] implemented by PR [#1054][pr_1054].
The reload API call will remove stale keys therefore they will not be return in public_keys endpoint neither will be
able to perform any signing requests.
- The AWS secrets manager and KMS tag filter cli options has been modified. Following cli options has been removed:
```
--aws-kms-tag-names-filter
--aws-kms-tag-values-filter
--aws-secrets-tag-names-filter
--aws-secrets-tag-values-filter
```
The above are replaced by:
```
--aws-kms-tag <TagName>=<TagValue>
--aws-secrets-tag <TagName>=<TagValue>
```
- `--Xworker-pool-size` deprecated cli option has been removed. Use `--vertx-worker-pool-size` instead.

[issue_1018]: https://github.com/Consensys/web3signer/issues/1018
[pr_1054]: https://github.com/Consensys/web3signer/pull/1054

### Features Added
- Remove stale keys during reload API call. [#1018][issue_1018] [#1054][pr_1054]
- Use single cli option to specify AWS KMS tag name/value pairs. [#1055][pr_1055]
- Use single cli option to specify AWS Secrets tag name/value pairs. [#1055][pr_1055]

### Bugs Fixed:
- AWS KMS tag filter behavior has been fixed. [#1055][pr_1055]

[issue_1018]: https://github.com/Consensys/web3signer/issues/1018
[pr_1054]: https://github.com/Consensys/web3signer/pull/1054
[pr_1055]: https://github.com/Consensys/web3signer/pull/1055

---
## 24.12.0
Expand Down

Large diffs are not rendered by default.

Original file line number Diff line number Diff line change
Expand Up @@ -17,17 +17,15 @@
import static tech.pegasys.web3signer.commandline.PicoCliAwsKmsParameters.AWS_KMS_ENABLED_OPTION;
import static tech.pegasys.web3signer.commandline.PicoCliAwsKmsParameters.AWS_KMS_REGION_OPTION;
import static tech.pegasys.web3signer.commandline.PicoCliAwsKmsParameters.AWS_KMS_SECRET_ACCESS_KEY_OPTION;
import static tech.pegasys.web3signer.commandline.PicoCliAwsKmsParameters.AWS_KMS_TAG_NAMES_FILTER_OPTION;
import static tech.pegasys.web3signer.commandline.PicoCliAwsKmsParameters.AWS_KMS_TAG_VALUES_FILTER_OPTION;
import static tech.pegasys.web3signer.commandline.PicoCliAwsKmsParameters.AWS_KMS_TAG_OPTION;
import static tech.pegasys.web3signer.commandline.PicoCliAwsSecretsManagerParameters.AWS_ENDPOINT_OVERRIDE_OPTION;
import static tech.pegasys.web3signer.commandline.PicoCliAwsSecretsManagerParameters.AWS_SECRETS_ACCESS_KEY_ID_OPTION;
import static tech.pegasys.web3signer.commandline.PicoCliAwsSecretsManagerParameters.AWS_SECRETS_AUTH_MODE_OPTION;
import static tech.pegasys.web3signer.commandline.PicoCliAwsSecretsManagerParameters.AWS_SECRETS_ENABLED_OPTION;
import static tech.pegasys.web3signer.commandline.PicoCliAwsSecretsManagerParameters.AWS_SECRETS_PREFIXES_FILTER_OPTION;
import static tech.pegasys.web3signer.commandline.PicoCliAwsSecretsManagerParameters.AWS_SECRETS_REGION_OPTION;
import static tech.pegasys.web3signer.commandline.PicoCliAwsSecretsManagerParameters.AWS_SECRETS_SECRET_ACCESS_KEY_OPTION;
import static tech.pegasys.web3signer.commandline.PicoCliAwsSecretsManagerParameters.AWS_SECRETS_TAG_NAMES_FILTER_OPTION;
import static tech.pegasys.web3signer.commandline.PicoCliAwsSecretsManagerParameters.AWS_SECRETS_TAG_VALUES_FILTER_OPTION;
import static tech.pegasys.web3signer.commandline.PicoCliAwsSecretsManagerParameters.AWS_SECRETS_TAG_OPTION;
import static tech.pegasys.web3signer.signing.config.KeystoresParameters.KEYSTORES_PASSWORDS_PATH;
import static tech.pegasys.web3signer.signing.config.KeystoresParameters.KEYSTORES_PASSWORD_FILE;
import static tech.pegasys.web3signer.signing.config.KeystoresParameters.KEYSTORES_PATH;
Expand Down Expand Up @@ -394,15 +392,13 @@ private Collection<String> awsSecretsManagerBulkLoadingOptions(
params.add(String.join(",", awsVaultParameters.getPrefixesFilter()));
}

if (!awsVaultParameters.getTagNamesFilter().isEmpty()) {
params.add(AWS_SECRETS_TAG_NAMES_FILTER_OPTION);
params.add(String.join(",", awsVaultParameters.getTagNamesFilter()));
}

if (!awsVaultParameters.getTagValuesFilter().isEmpty()) {
params.add(AWS_SECRETS_TAG_VALUES_FILTER_OPTION);
params.add(String.join(",", awsVaultParameters.getTagValuesFilter()));
}
awsVaultParameters
.getTags()
.forEach(
(key, value) -> {
params.add(AWS_SECRETS_TAG_OPTION);
params.add(key + "=" + value);
});

return params;
}
Expand Down Expand Up @@ -438,15 +434,13 @@ private Collection<String> awsKmsBulkLoadingOptions(final AwsVaultParameters aws
params.add(uri.toString());
});

if (!awsVaultParameters.getTagNamesFilter().isEmpty()) {
params.add(AWS_KMS_TAG_NAMES_FILTER_OPTION);
params.add(String.join(",", awsVaultParameters.getTagNamesFilter()));
}

if (!awsVaultParameters.getTagValuesFilter().isEmpty()) {
params.add(AWS_KMS_TAG_VALUES_FILTER_OPTION);
params.add(String.join(",", awsVaultParameters.getTagValuesFilter()));
}
awsVaultParameters
.getTags()
.forEach(
(key, value) -> {
params.add(AWS_KMS_TAG_OPTION);
params.add(key + "=" + value);
});

return params;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,20 +38,25 @@
import org.apache.tuweni.bytes.Bytes32;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.io.TempDir;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.ValueSource;
import org.web3j.crypto.ECKeyPair;
import org.web3j.crypto.Keys;
import org.web3j.utils.Numeric;

public class MetricsAcceptanceTest extends AcceptanceTestBase {

@Test
void missingSignerMetricIncreasesWhenUnmatchedRequestReceived() throws JsonProcessingException {
@ParameterizedTest(name = "{index} - Missing Signing Metrics using Config File: {0}")
@ValueSource(booleans = {true, false})
void missingSignerMetricIncreasesWhenUnmatchedRequestReceived(boolean useConfigFile)
throws JsonProcessingException {
final SignerConfiguration signerConfiguration =
new SignerConfigurationBuilder()
.withMetricsCategories("SIGNING")
.withMetricsCategories("SIGNING", "JVM")
.withMetricsEnabled(true)
.withMode("eth2")
.withNetwork("minimal")
.withUseConfigFile(useConfigFile)
.build();
startSigner(signerConfiguration);

Expand Down Expand Up @@ -81,7 +86,7 @@ void signMetricIncrementsWhenSecpSignRequestReceived(@TempDir final Path testDir

final SignerConfiguration signerConfiguration =
new SignerConfigurationBuilder()
.withMetricsCategories("SIGNING")
.withMetricsCategories("SIGNING", "JVM")
.withMetricsEnabled(true)
.withKeyStoreDirectory(testDirectory)
.withMode("eth1")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ public class AwsKmsAcceptanceTest extends AcceptanceTestBase {
: Optional.empty();
private AwsKmsUtil awsSecretsManagerUtil;

public record Key(String keyId, String publicKey) {}
public record Key(String keyId, String publicKey, Map<String, String> tags) {}

private final List<Key> keys = new ArrayList<>();

Expand All @@ -97,10 +97,26 @@ void setupAwsResources() {
Optional.empty(),
awsEndpointOverride);

for (int i = 0; i < 4; i++) {
final String keyId = awsSecretsManagerUtil.createKey(Map.of("TagName" + i, "TagValue" + i));
// Tags: Org=Protocols, Env=Dev
var protocolsKeysTags = Map.of("Org", "Protocols", "Env", "Prod");
var stakingKeysTags = Map.of("Org", "Staking", "Env", "Prod");
for (int i = 0; i < 2; i++) {
final String keyId = awsSecretsManagerUtil.createKey(protocolsKeysTags);
final ECPublicKey publicKey = awsSecretsManagerUtil.publicKey(keyId);
keys.add(new Key(keyId, EthPublicKeyUtils.toHexString(publicKey)));
keys.add(new Key(keyId, EthPublicKeyUtils.toHexString(publicKey), protocolsKeysTags));
}

for (int i = 0; i < 2; i++) {
final String keyId = awsSecretsManagerUtil.createKey(stakingKeysTags);
final ECPublicKey publicKey = awsSecretsManagerUtil.publicKey(keyId);
keys.add(new Key(keyId, EthPublicKeyUtils.toHexString(publicKey), stakingKeysTags));
}

// keys with no tags
for (int i = 0; i < 2; i++) {
final String keyId = awsSecretsManagerUtil.createKey(Map.of());
final ECPublicKey publicKey = awsSecretsManagerUtil.publicKey(keyId);
keys.add(new Key(keyId, EthPublicKeyUtils.toHexString(publicKey), Map.of()));
}
}

Expand All @@ -114,8 +130,8 @@ void keysAreLoadedFromAwsKmsAndReportedByPublicApi(final boolean useConfigFile)
.withRegion(AWS_REGION)
.withAccessKeyId(RO_AWS_ACCESS_KEY_ID)
.withSecretAccessKey(RO_AWS_SECRET_ACCESS_KEY)
.withTagNamesFilter(List.of("TagName0", "TagName1"))
.withTagValuesFilter(List.of("TagValue0", "TagValue1", "TagValue2"))
.withTag("Org", "Protocols")
.withTag("Env", "Prod")
.withEndpointOverride(awsEndpointOverride)
.build();

Expand Down Expand Up @@ -199,8 +215,8 @@ void keysAreLoadedFromAwsKmsWithEnvironmentAuthModeAndReportedByPublicApi(
AwsVaultParametersBuilder.anAwsParameters()
.withEnabled(true)
.withAuthenticationMode(AwsAuthenticationMode.ENVIRONMENT)
.withTagNamesFilter(List.of("TagName2", "TagName3"))
.withTagValuesFilter(List.of("TagValue0", "TagValue2", "TagValue3"))
.withTag("Org", "Staking")
.withTag("Env", "Prod")
.withEndpointOverride(awsEndpointOverride)
.build();

Expand All @@ -224,6 +240,93 @@ void keysAreLoadedFromAwsKmsWithEnvironmentAuthModeAndReportedByPublicApi(
hasSize(2));
}

@ParameterizedTest(name = "{index} - Using config file: {0}")
@ValueSource(booleans = {true, false})
void keysWithoutTagsAreLoadedFromAwsKmsAndReportedByPublicApi(final boolean useConfigFile) {
var awsVaultParameters =
AwsVaultParametersBuilder.anAwsParameters()
.withEnabled(true)
.withAuthenticationMode(AwsAuthenticationMode.SPECIFIED)
.withRegion(AWS_REGION)
.withAccessKeyId(RO_AWS_ACCESS_KEY_ID)
.withSecretAccessKey(RO_AWS_SECRET_ACCESS_KEY)
.withEndpointOverride(awsEndpointOverride)
.build();

var configBuilder =
new SignerConfigurationBuilder()
.withUseConfigFile(useConfigFile)
.withMode("eth1")
.withAwsParameters(awsVaultParameters);

startSigner(configBuilder.build());

var response = signer.callApiPublicKeys(KeyType.SECP256K1);
response
.then()
.statusCode(200)
.contentType(ContentType.JSON)
.body("", containsInAnyOrder(keys.stream().map(Key::publicKey).toArray()), "", hasSize(6));

var healthcheckResponse = signer.healthcheck();
healthcheckResponse
.then()
.statusCode(200)
.contentType(ContentType.JSON)
.body("status", equalTo("UP"));

var jsonBody = healthcheckResponse.body().asString();
var keysLoaded = getAwsBulkLoadingData(jsonBody, "keys-loaded");
assertThat(keysLoaded).isEqualTo(6);
}

@ParameterizedTest(name = "{index} - Using config file: {0}")
@ValueSource(booleans = {true, false})
void keysWitSingleTagAreLoadedFromAwsKmsAndReportedByPublicApi(final boolean useConfigFile) {
var awsVaultParameters =
AwsVaultParametersBuilder.anAwsParameters()
.withEnabled(true)
.withAuthenticationMode(AwsAuthenticationMode.SPECIFIED)
.withRegion(AWS_REGION)
.withAccessKeyId(RO_AWS_ACCESS_KEY_ID)
.withSecretAccessKey(RO_AWS_SECRET_ACCESS_KEY)
.withEndpointOverride(awsEndpointOverride)
.withTag("Env", "Prod")
.build();

var configBuilder =
new SignerConfigurationBuilder()
.withUseConfigFile(useConfigFile)
.withMode("eth1")
.withAwsParameters(awsVaultParameters);

startSigner(configBuilder.build());

var expectedKeysMatchingTag =
keys.stream()
.filter(key -> "Prod".equals(key.tags.get("Env")))
.map(Key::publicKey)
.toList()
.toArray();
var response = signer.callApiPublicKeys(KeyType.SECP256K1);
response
.then()
.statusCode(200)
.contentType(ContentType.JSON)
.body("", containsInAnyOrder(expectedKeysMatchingTag), "", hasSize(4));

var healthcheckResponse = signer.healthcheck();
healthcheckResponse
.then()
.statusCode(200)
.contentType(ContentType.JSON)
.body("status", equalTo("UP"));

var jsonBody = healthcheckResponse.body().asString();
var keysLoaded = getAwsBulkLoadingData(jsonBody, "keys-loaded");
assertThat(keysLoaded).isEqualTo(4);
}

@AfterAll
void cleanUpAwsResources() {
if (awsSecretsManagerUtil != null) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -113,8 +113,8 @@ void secretsAreLoadedFromAWSSecretsManagerAndReportedByPublicApi(final boolean u
.withAccessKeyId(RO_AWS_ACCESS_KEY_ID)
.withSecretAccessKey(RO_AWS_SECRET_ACCESS_KEY)
.withPrefixesFilter(List.of(awsSecretsManagerUtil.getSecretsManagerPrefix()))
.withTagNamesFilter(List.of("TagName0", "TagName1"))
.withTagValuesFilter(List.of("TagValue0", "TagValue1", "TagValue2"))
.withTag("TagName0", "TagValue0")
.withTag("TagName1", "TagValue1")
.withEndpointOverride(awsEndpointOverride)
.build();

Expand Down Expand Up @@ -186,8 +186,8 @@ void secretsAreLoadedFromAWSSecretsManagerWithEnvironmentAuthModeAndReportedByPu
.withEnabled(true)
.withAuthenticationMode(AwsAuthenticationMode.ENVIRONMENT)
.withPrefixesFilter(List.of(awsSecretsManagerUtil.getSecretsManagerPrefix()))
.withTagNamesFilter(List.of("TagName2", "TagName3"))
.withTagValuesFilter(List.of("TagValue0", "TagValue2", "TagValue3"))
.withTag("TagName2", "TagValue2")
.withTag("TagName3", "TagValue3")
.withEndpointOverride(awsEndpointOverride)
.build();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ void secretsAreLoadedFromAWSSecretsManagerAndReportedByPublicApi(final boolean u
.withAccessKeyId(RO_AWS_ACCESS_KEY_ID)
.withSecretAccessKey(RO_AWS_SECRET_ACCESS_KEY)
.withPrefixesFilter(List.of(awsSecretsManagerUtil.getSecretsManagerPrefix()))
.withTagNamesFilter(List.of("multivalue"))
.withTag("multivalue", "")
.withEndpointOverride(awsEndpointOverride)
.build();

Expand All @@ -127,7 +127,7 @@ void secretsAreLoadedFromAWSSecretsManagerAndReportedByPublicApi(final boolean u
blsKeyPairList.stream()
.map(BLSKeyPair::getPublicKey)
.map(BLSPublicKey::toHexString)
.collect(Collectors.toList());
.toList();

signer
.callApiPublicKeys(KeyType.BLS)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ public class CommitBoostGetPubKeysAcceptanceTest extends AcceptanceTestBase {
@TempDir private Path commitBoostPasswordDir;

@BeforeEach
void setup() throws Exception {
void setup() {
for (final BLSKeyPair blsKeyPair : consensusBlsKeys) {
// create consensus bls keystore
KeystoreUtil.createKeystore(blsKeyPair, keystoreDir, passwordDir, KEYSTORE_PASSWORD);
Expand Down Expand Up @@ -111,12 +111,14 @@ void listCommitBoostPublicKeys() {
assertThat(proxyBLSKeysMap.keySet()).contains(consensusKeyHex);

// verify if proxy BLS keys are present in the response
@SuppressWarnings("unchecked")
final List<String> responseProxyBlsKeys = (List<String>) responseKeyMap.get("proxy_bls");
final List<String> expectedProxyBLSKeys = getProxyBLSPubKeys(consensusKeyHex);
assertThat(responseProxyBlsKeys)
.containsExactlyInAnyOrder(expectedProxyBLSKeys.toArray(String[]::new));

// verify if proxy SECP keys are present in the response
@SuppressWarnings("unchecked")
final List<String> responseProxySECPKeys = (List<String>) responseKeyMap.get("proxy_ecdsa");
final List<String> expectedProxySECPKeys = getProxyECPubKeys(consensusKeyHex);
assertThat(responseProxySECPKeys)
Expand Down
Loading

0 comments on commit 65ce049

Please sign in to comment.