Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: use Map for tags for AWS secrets manager and kms #1055

Merged
merged 13 commits into from
Feb 4, 2025
Merged
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
Loading