Skip to content

Commit

Permalink
Support signing_root in Eth2 Signing request body (#880)
Browse files Browse the repository at this point in the history
* Add `@JsonAlias` to allow supporting both signing_root and signingRoot in incoming signing requst body.
* Update errorprone to support Java 17 language constructs.
* Update errorprone version and fix toLowerCase/uppercase
* Add AT for signingRoot and signing_root
* changelog
  • Loading branch information
usmansaleem authored Aug 18, 2023
1 parent 8595ae2 commit ababf26
Show file tree
Hide file tree
Showing 16 changed files with 163 additions and 29 deletions.
4 changes: 3 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,8 @@
### Breaking Changes
- Use Java 17 for build and runtime. Remove Java 11 variant of docker image. zip/tar.gz distributions will require Java 17 or above to run Web3Signer.
- Eth2 Azure command line option --azure-secrets-tags is now deprecated and is replaced with --azure-tags. The --azure-secrets-tags option will be removed in a future release.

- Eth2 Signing request body: deprecating `signingRoot` in favor of `signing_root` property. `signingRoot` will be removed in a future release.
-
### Features Added
- Add support for SECP256K1 remote signing using AWS Key Management Service. [#501](https://github.com/Consensys/web3signer/issues/501)
- Azure bulk mode support for loading multiline (`\n` delimited, up to 200) keys per secret.
Expand All @@ -19,6 +20,7 @@
- Java 17 for build and runtime. [#870](https://github.com/Consensys/web3signer/pull/870)
- Update internal teku library to 23.8.0 [#876](https://github.com/Consensys/web3signer/pull/876)
- Add support for [Lukso network](https://lukso.network/) `--network=lukso`
- Deprecate `signingRoot` while currently supporting both `signingRoot` and `signing_root` in Eth2 signing request body.

### Bugs fixed
- Support long name aliases in environment variables and YAML configuration [#825](https://github.com/Consensys/web3signer/pull/825)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -157,12 +157,19 @@ public Response eth2Sign(
final Eth2SigningRequestBody ethSignBody,
final ContentType acceptMediaType)
throws JsonProcessingException {
return eth2Sign(
publicKey, ETH_2_INTERFACE_OBJECT_MAPPER.writeValueAsString(ethSignBody), acceptMediaType);
}

public Response eth2Sign(
final String publicKey, final String jsonBody, final ContentType acceptMediaType)
throws JsonProcessingException {
return given()
.baseUri(getUrl())
.contentType(ContentType.JSON)
.accept(acceptMediaType)
.pathParam("identifier", publicKey)
.body(ETH_2_INTERFACE_OBJECT_MAPPER.writeValueAsString(ethSignBody))
.body(jsonBody)
.log()
.all(true)
.post(signPath(BLS));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
import java.security.cert.CertificateException;
import java.security.cert.X509Certificate;
import java.util.List;
import java.util.Locale;
import java.util.Optional;
import java.util.StringJoiner;

Expand Down Expand Up @@ -75,7 +76,7 @@ public static String generateFingerprint(final X509Certificate cert)
joiner.add(String.format("%02X", b));
}

return joiner.toString().toLowerCase();
return joiner.toString().toLowerCase(Locale.ROOT);
}

public static Path createJksTrustStore(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
import java.security.NoSuchAlgorithmException;
import java.security.NoSuchProviderException;
import java.util.List;
import java.util.Locale;
import java.util.Set;

import com.fasterxml.jackson.core.JsonProcessingException;
Expand Down Expand Up @@ -60,8 +61,8 @@ void filecoinApisAreCounted(final boolean useConfigFile) {

final List<String> metricsOfInterest =
List.of(
"filecoin_" + SECP256K1.name().toLowerCase() + "_signing_request_count",
"filecoin_" + BLS.name().toLowerCase() + "_signing_request_count",
"filecoin_" + SECP256K1.name().toLowerCase(Locale.ROOT) + "_signing_request_count",
"filecoin_" + BLS.name().toLowerCase(Locale.ROOT) + "_signing_request_count",
"filecoin_total_request_count",
"filecoin_wallet_has_count",
"filecoin_wallet_list_count",
Expand Down Expand Up @@ -144,8 +145,8 @@ void signMetricIncrementsWhenSecpSignRequestReceived(@TempDir final Path testDir

final List<String> metricsOfInterest =
List.of(
"signing_" + SECP256K1.name().toLowerCase() + "_signing_duration_count",
"signing_" + SECP256K1.name().toLowerCase() + "_missing_identifier_count");
"signing_" + SECP256K1.name().toLowerCase(Locale.ROOT) + "_signing_duration_count",
"signing_" + SECP256K1.name().toLowerCase(Locale.ROOT) + "_missing_identifier_count");
final Set<String> initialMetrics = signer.getMetricsMatching(metricsOfInterest);
assertThat(initialMetrics).hasSize(metricsOfInterest.size());
assertThat(initialMetrics).allMatch(s -> s.endsWith("0.0"));
Expand All @@ -157,8 +158,10 @@ void signMetricIncrementsWhenSecpSignRequestReceived(@TempDir final Path testDir

assertThat(metricsAfterSign)
.containsOnly(
"signing_" + SECP256K1.name().toLowerCase() + "_signing_duration_count 1.0",
"signing_" + SECP256K1.name().toLowerCase() + "_missing_identifier_count 0.0");
"signing_" + SECP256K1.name().toLowerCase(Locale.ROOT) + "_signing_duration_count 1.0",
"signing_"
+ SECP256K1.name().toLowerCase(Locale.ROOT)
+ "_missing_identifier_count 0.0");
}

@Test
Expand All @@ -184,8 +187,8 @@ void signMetricIncrementsWhenBlsSignRequestReceived(@TempDir final Path testDire

final List<String> metricsOfInterest =
List.of(
"signing_" + BLS.name().toLowerCase() + "_signing_duration_count",
"signing_" + BLS.name().toLowerCase() + "_missing_identifier_count");
"signing_" + BLS.name().toLowerCase(Locale.ROOT) + "_signing_duration_count",
"signing_" + BLS.name().toLowerCase(Locale.ROOT) + "_missing_identifier_count");
final Set<String> initialMetrics = signer.getMetricsMatching(metricsOfInterest);
assertThat(initialMetrics).hasSize(metricsOfInterest.size());
assertThat(initialMetrics).allMatch(s -> s.endsWith("0.0"));
Expand All @@ -197,7 +200,7 @@ void signMetricIncrementsWhenBlsSignRequestReceived(@TempDir final Path testDire

assertThat(metricsAfterSign)
.containsOnly(
"signing_" + BLS.name().toLowerCase() + "_signing_duration_count 1.0",
"signing_" + BLS.name().toLowerCase() + "_missing_identifier_count 0.0");
"signing_" + BLS.name().toLowerCase(Locale.ROOT) + "_signing_duration_count 1.0",
"signing_" + BLS.name().toLowerCase(Locale.ROOT) + "_missing_identifier_count 0.0");
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,102 @@
/*
* 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.signing;

import static io.restassured.http.ContentType.ANY;
import static io.restassured.http.ContentType.JSON;
import static io.restassured.http.ContentType.TEXT;
import static org.assertj.core.api.Assertions.assertThat;

import tech.pegasys.teku.bls.BLS;
import tech.pegasys.teku.bls.BLSKeyPair;
import tech.pegasys.teku.bls.BLSPublicKey;
import tech.pegasys.teku.bls.BLSSecretKey;
import tech.pegasys.teku.bls.BLSSignature;
import tech.pegasys.teku.spec.SpecMilestone;
import tech.pegasys.teku.spec.networks.Eth2Network;
import tech.pegasys.web3signer.core.service.http.ArtifactType;
import tech.pegasys.web3signer.core.service.http.handlers.signing.eth2.Eth2SigningRequestBody;
import tech.pegasys.web3signer.dsl.signer.Signer;
import tech.pegasys.web3signer.dsl.utils.Eth2RequestUtils;
import tech.pegasys.web3signer.dsl.utils.MetadataFileHelpers;
import tech.pegasys.web3signer.signing.KeyType;

import java.nio.file.Path;

import com.fasterxml.jackson.core.JsonProcessingException;
import io.restassured.http.ContentType;
import io.restassured.response.Response;
import org.apache.tuweni.bytes.Bytes;
import org.apache.tuweni.bytes.Bytes32;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.ValueSource;

public class BlsSigningRootPropertyAcceptanceTest extends SigningAcceptanceTestBase {

private static final String PRIVATE_KEY =
"3ee2224386c82ffea477e2adf28a2929f5c349165a4196158c7f3a2ecca40f35";
private static final MetadataFileHelpers METADATA_FILE_HELPERS = new MetadataFileHelpers();
private static final BLSSecretKey KEY =
BLSSecretKey.fromBytes(Bytes32.fromHexString(PRIVATE_KEY));
private static final BLSKeyPair KEY_PAIR = new BLSKeyPair(KEY);
private static final BLSPublicKey PUBLIC_KEY = KEY_PAIR.getPublicKey();

@ParameterizedTest(name = "#{index} - Signing request with signing root property as: {0}")
@ValueSource(strings = {"signing_root", "signingRoot"})
public void deprecatedSigningRootPropertyWorks(final String signingRootProperty)
throws JsonProcessingException {
final String configFilename = PUBLIC_KEY.toString().substring(2);
final Path keyConfigFile = testDirectory.resolve(configFilename + ".yaml");
METADATA_FILE_HELPERS.createUnencryptedYamlFileAt(keyConfigFile, PRIVATE_KEY, KeyType.BLS);

signAndVerifySignature(signingRootProperty);
}

private void signAndVerifySignature(final String signingRootProperty)
throws JsonProcessingException {
final ArtifactType artifactType = ArtifactType.RANDAO_REVEAL;
final ContentType acceptMediaType = JSON;
setupMinimalWeb3Signer(artifactType);

// openapi
final Eth2SigningRequestBody request = Eth2RequestUtils.createCannedRequest(artifactType);
final String jsonBody = Signer.ETH_2_INTERFACE_OBJECT_MAPPER.writeValueAsString(request);

// by default, we should have deserialized to signing_root.
assertThat(jsonBody).containsOnlyOnce("signing_root").doesNotContain("signingRoot");
// modify before sending
final String modifiedJsonBody = jsonBody.replace("signing_root", signingRootProperty);
assertThat(modifiedJsonBody).containsOnlyOnce(signingRootProperty);

// send modified JSON containing signing_root or signingRoot
final Response response =
signer.eth2Sign(KEY_PAIR.getPublicKey().toString(), modifiedJsonBody, acceptMediaType);
final Bytes signature =
verifyAndGetSignatureResponse(response, expectedContentType(acceptMediaType));
final BLSSignature expectedSignature =
BLS.sign(KEY_PAIR.getSecretKey(), request.getSigningRoot());
assertThat(signature).isEqualTo(expectedSignature.toBytesCompressed());
}

private void setupMinimalWeb3Signer(final ArtifactType artifactType) {
switch (artifactType) {
case BLOCK_V2, SYNC_COMMITTEE_MESSAGE, SYNC_COMMITTEE_SELECTION_PROOF, SYNC_COMMITTEE_CONTRIBUTION_AND_PROOF -> setupEth2Signer(
Eth2Network.MINIMAL, SpecMilestone.ALTAIR);
default -> setupEth2Signer(Eth2Network.MINIMAL, SpecMilestone.PHASE0);
}
}

private ContentType expectedContentType(final ContentType acceptMediaType) {
return acceptMediaType == ANY || acceptMediaType == JSON ? JSON : TEXT;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@

import static tech.pegasys.web3signer.commandline.valueprovider.PrefixUtil.stripPrefix;

import java.util.Locale;
import java.util.Map;

import picocli.CommandLine.IDefaultValueProvider;
Expand All @@ -32,10 +33,10 @@ public String defaultValue(final ArgSpec argSpec) {
if (argSpec.isOption()) {
final OptionSpec optionSpec = (OptionSpec) argSpec;
final String qualifiedPrefix =
optionSpec.command().qualifiedName("_").replace("-", "_").toUpperCase();
optionSpec.command().qualifiedName("_").replace("-", "_").toUpperCase(Locale.ROOT);

for (final String alias : optionSpec.names()) {
final String key = stripPrefix(alias).replace("-", "_").toUpperCase();
final String key = stripPrefix(alias).replace("-", "_").toUpperCase(Locale.ROOT);
final String qualifiedKey = qualifiedPrefix + "_" + key;
final String value = environment.get(qualifiedKey);
if (value != null) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@

import java.math.BigInteger;
import java.util.List;
import java.util.Locale;
import java.util.stream.Collectors;

import com.google.common.io.BaseEncoding;
Expand Down Expand Up @@ -66,7 +67,8 @@ public String request(final Request<?, EthSendTransaction> request, final long c

final byte[] signedTransaction =
PrivateTransactionEncoder.signMessage(rawTransaction, chainId, credentials);
final String value = "0x" + BaseEncoding.base16().encode(signedTransaction).toLowerCase();
final String value =
"0x" + BaseEncoding.base16().encode(signedTransaction).toLowerCase(Locale.ROOT);
return request(value);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@

import java.math.BigInteger;
import java.util.List;
import java.util.Locale;

import com.google.common.io.BaseEncoding;
import io.vertx.core.json.Json;
Expand Down Expand Up @@ -65,7 +66,8 @@ public String request(final Request<?, EthSendTransaction> request, final long c
transaction.getString("data"));
final byte[] signedTransaction =
TransactionEncoder.signMessage(rawTransaction, chainId, credentials);
final String value = "0x" + BaseEncoding.base16().encode(signedTransaction).toLowerCase();
final String value =
"0x" + BaseEncoding.base16().encode(signedTransaction).toLowerCase(Locale.ROOT);
return request(value);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
import tech.pegasys.teku.api.schema.altair.ContributionAndProof;
import tech.pegasys.web3signer.core.service.http.ArtifactType;

import com.fasterxml.jackson.annotation.JsonAlias;
import com.fasterxml.jackson.annotation.JsonCreator;
import com.fasterxml.jackson.annotation.JsonProperty;
import org.apache.tuweni.bytes.Bytes;
Expand All @@ -43,7 +44,7 @@ public class Eth2SigningRequestBody {
@JsonCreator
public Eth2SigningRequestBody(
@JsonProperty(value = "type", required = true) final ArtifactType type,
@JsonProperty("signingRoot") final Bytes signingRoot,
@JsonProperty("signing_root") @JsonAlias("signingRoot") final Bytes signingRoot,
@JsonProperty("fork_info") final ForkInfo forkInfo,
@JsonProperty("block") final BeaconBlock block,
@JsonProperty("beacon_block") final BlockRequest blockRequest,
Expand Down Expand Up @@ -100,7 +101,8 @@ public AttestationData getAttestation() {
return attestation;
}

@JsonProperty("signingRoot")
@JsonProperty("signing_root")
@JsonAlias("signingRoot")
public Bytes getSigningRoot() {
return signingRoot;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@
import tech.pegasys.web3signer.signing.ArtifactSignerProvider;
import tech.pegasys.web3signer.signing.KeyType;

import java.util.Locale;

import org.hyperledger.besu.plugin.services.MetricsSystem;
import org.hyperledger.besu.plugin.services.metrics.Counter;
import org.hyperledger.besu.plugin.services.metrics.OperationTimer;
Expand All @@ -32,17 +34,17 @@ public HttpApiMetrics(
malformedRequestCounter =
metricsSystem.createCounter(
Web3SignerMetricCategory.HTTP,
keyType.name().toLowerCase() + "_malformed_request_count",
keyType.name().toLowerCase(Locale.ROOT) + "_malformed_request_count",
"Number of requests received which had illegally formatted body.");
signingTimer =
metricsSystem.createTimer(
Web3SignerMetricCategory.SIGNING,
keyType.name().toLowerCase() + "_signing_duration",
keyType.name().toLowerCase(Locale.ROOT) + "_signing_duration",
"Duration of a signing event");
missingSignerCounter =
metricsSystem.createCounter(
Web3SignerMetricCategory.SIGNING,
keyType.name().toLowerCase() + "_missing_identifier_count",
keyType.name().toLowerCase(Locale.ROOT) + "_missing_identifier_count",
"Number of signing operations requested, for keys which are not available");
metricsSystem.createIntegerGauge(
Web3SignerMetricCategory.SIGNING,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@
import tech.pegasys.web3signer.common.Web3SignerMetricCategory;
import tech.pegasys.web3signer.signing.KeyType;

import java.util.Locale;

import io.vertx.ext.web.RoutingContext;
import org.hyperledger.besu.plugin.services.MetricsSystem;
import org.hyperledger.besu.plugin.services.metrics.Counter;
Expand All @@ -35,12 +37,12 @@ public FcJsonRpcMetrics(final MetricsSystem metricsSystem) {
this.secpSigningRequestCounter =
metricsSystem.createCounter(
Web3SignerMetricCategory.FILECOIN,
KeyType.SECP256K1.name().toLowerCase() + "_signing_request_count",
KeyType.SECP256K1.name().toLowerCase(Locale.ROOT) + "_signing_request_count",
"Number of signing requests made for SECP256k1 keys");
this.blsSigningRequestCounter =
metricsSystem.createCounter(
Web3SignerMetricCategory.FILECOIN,
KeyType.BLS.name().toLowerCase() + "_signing_request_count",
KeyType.BLS.name().toLowerCase(Locale.ROOT) + "_signing_request_count",
"Number of signing requests made for BLS keys");
this.walletListRequestCounter =
metricsSystem.createCounter(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
import tech.pegasys.web3signer.core.service.jsonrpc.handlers.sendtransaction.NonceProvider;

import java.util.List;
import java.util.Locale;

import io.vertx.core.json.JsonObject;
import io.vertx.ext.web.RoutingContext;
Expand All @@ -44,7 +45,7 @@ public TransactionFactory(
}

public Transaction createTransaction(final RoutingContext context, final JsonRpcRequest request) {
final String method = request.getMethod().toLowerCase();
final String method = request.getMethod().toLowerCase(Locale.ROOT);
final VertxNonceRequestTransmitter nonceRequestTransmitter =
new VertxNonceRequestTransmitter(context.request().headers(), decoder, transmitterFactory);

Expand Down
2 changes: 1 addition & 1 deletion gradle/versions.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ dependencyManagement {
dependency 'com.fasterxml.jackson.core:jackson-databind:2.15.2'
dependency 'com.fasterxml.jackson.dataformat:jackson-dataformat-yaml:2.15.2'

dependencySet(group: 'com.google.errorprone', version: '2.17.0') {
dependencySet(group: 'com.google.errorprone', version: '2.21.1') {
entry 'error_prone_annotation'
entry 'error_prone_check_api'
entry 'error_prone_core'
Expand Down
Loading

0 comments on commit ababf26

Please sign in to comment.