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

Hashicorp connection, override http protocol version #817

Merged
merged 4 commits into from
Jun 27, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,4 +1,12 @@
# Changelog

## Next release
### Features Added
- Hashicorp connection properties can now override http protocol to HTTP/1.1 from the default of HTTP/2. [#817](https://github.com/ConsenSys/web3signer/pull/817)

### Bugs fixed

---
## 23.6.0

As part of our ongoing commitment to deliver the best remote signing solutions, we are announcing a change in our product offerings.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,9 @@ public void createKeyStoreYamlFileAt(
}

public void createHashicorpYamlFileAt(
final Path metadataFilePath, final HashicorpSigningParams node) {
final Path metadataFilePath,
final HashicorpSigningParams node,
Optional<String> httpProtocolVersion) {
try {
final Map<String, String> signingMetadata = new HashMap<>();

Expand All @@ -131,6 +133,8 @@ public void createHashicorpYamlFileAt(
signingMetadata.put("token", node.getVaultToken());
signingMetadata.put("keyType", node.getKeyType().toString());

httpProtocolVersion.ifPresent(version -> signingMetadata.put("httpProtocolVersion", version));

createYamlFile(metadataFilePath, signingMetadata);
} catch (final Exception e) {
throw new RuntimeException("Unable to construct hashicorp yaml file", e);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -64,11 +64,11 @@ void keyCanBeExtractedFromVault() {
Collections.singletonMap(SECRET_KEY, SECRET_VALUE), KEY_SUBPATH);

final ConnectionParameters connectionParameters =
new ConnectionParameters(
hashicorpNode.getHost(),
Optional.of(hashicorpNode.getPort()),
Optional.empty(),
Optional.of(30_000L));
ConnectionParameters.newBuilder()
.withServerHost(hashicorpNode.getHost())
.withServerPort(hashicorpNode.getPort())
.withTimeoutMs(30_000L)
.build();
final KeyDefinition key =
new KeyDefinition(
hashicorpNode.getHttpApiPathForSecret(KEY_SUBPATH),
Expand Down Expand Up @@ -97,11 +97,12 @@ void keyCanBeExtractedFromVaultOverTlsUsingWhitelist(@TempDir final Path testDir
final TlsOptions tlsOptions =
new TlsOptions(Optional.of(TrustStoreType.WHITELIST), fingerprintFile, null);
final ConnectionParameters connectionParameters =
new ConnectionParameters(
hashicorpNode.getHost(),
Optional.of(hashicorpNode.getPort()),
Optional.of(tlsOptions),
Optional.of(30_000L));
ConnectionParameters.newBuilder()
.withServerHost(hashicorpNode.getHost())
.withServerPort(hashicorpNode.getPort())
.withTlsOptions(tlsOptions)
.withTimeoutMs(30_000L)
.build();
final KeyDefinition key =
new KeyDefinition(
hashicorpNode.getHttpApiPathForSecret(KEY_SUBPATH),
Expand All @@ -128,11 +129,12 @@ void canConnectToHashicorpVaultUsingPkcs12Certificate(@TempDir final Path testDi
final TlsOptions tlsOptions =
new TlsOptions(Optional.of(TrustStoreType.PKCS12), trustStorePath, TRUST_STORE_PASSWORD);
final ConnectionParameters connectionParameters =
new ConnectionParameters(
hashicorpNode.getHost(),
Optional.of(hashicorpNode.getPort()),
Optional.of(tlsOptions),
Optional.of(30_000L));
ConnectionParameters.newBuilder()
.withServerHost(hashicorpNode.getHost())
.withServerPort(hashicorpNode.getPort())
.withTlsOptions(tlsOptions)
.withTimeoutMs(30_000L)
.build();
final KeyDefinition key =
new KeyDefinition(
hashicorpNode.getHttpApiPathForSecret(KEY_SUBPATH),
Expand All @@ -159,11 +161,12 @@ void canConnectToHashicorpVaultUsingJksCertificate(@TempDir final Path testDir)
final TlsOptions tlsOptions =
new TlsOptions(Optional.of(TrustStoreType.JKS), trustStorePath, TRUST_STORE_PASSWORD);
final ConnectionParameters connectionParameters =
new ConnectionParameters(
hashicorpNode.getHost(),
Optional.of(hashicorpNode.getPort()),
Optional.of(tlsOptions),
Optional.of(30_000L));
ConnectionParameters.newBuilder()
.withServerHost(hashicorpNode.getHost())
.withServerPort(hashicorpNode.getPort())
.withTlsOptions(tlsOptions)
.withTimeoutMs(30_000L)
.build();
final KeyDefinition key =
new KeyDefinition(
hashicorpNode.getHttpApiPathForSecret(KEY_SUBPATH),
Expand All @@ -189,11 +192,12 @@ void canConnectToHashicorpVaultUsingPemCertificate(@TempDir final Path testDir)
final TlsOptions tlsOptions =
new TlsOptions(Optional.of(TrustStoreType.PEM), trustStorePath, null);
final ConnectionParameters connectionParameters =
new ConnectionParameters(
hashicorpNode.getHost(),
Optional.of(hashicorpNode.getPort()),
Optional.of(tlsOptions),
Optional.of(30_000L));
ConnectionParameters.newBuilder()
.withServerHost(hashicorpNode.getHost())
.withServerPort(hashicorpNode.getPort())
.withTlsOptions(tlsOptions)
.withTimeoutMs(30_000L)
.build();
final KeyDefinition key =
new KeyDefinition(
hashicorpNode.getHttpApiPathForSecret(KEY_SUBPATH),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@
import org.junit.jupiter.api.condition.EnabledIfEnvironmentVariable;
import org.junit.jupiter.api.condition.EnabledIfEnvironmentVariables;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.CsvSource;
import org.junit.jupiter.params.provider.EnumSource;

public class BlsSigningAcceptanceTest extends SigningAcceptanceTestBase {
Expand Down Expand Up @@ -120,7 +121,32 @@ public void ableToSignUsingHashicorp() throws JsonProcessingException {
final Path keyConfigFile = testDirectory.resolve(configFilename + ".yaml");
METADATA_FILE_HELPERS.createHashicorpYamlFileAt(
keyConfigFile,
new HashicorpSigningParams(hashicorpNode, secretPath, secretName, KeyType.BLS));
new HashicorpSigningParams(hashicorpNode, secretPath, secretName, KeyType.BLS),
Optional.empty());

signAndVerifySignature(ArtifactType.BLOCK);
} finally {
hashicorpNode.shutdown();
}
}

@ParameterizedTest(name = "{index} - Using http protocol version: {0}, with TLS: {1}")
@CsvSource({"HTTP_1_1, true", "HTTP_2, true", "HTTP_1_1, false", "HTTP_2, false"})
public void ableToSignUsingHashicorpWithHttpProtocolOverride(
final String httpProtocolVersion, boolean withTLS) throws JsonProcessingException {
final String configFilename = KEY_PAIR.getPublicKey().toString().substring(2);
final HashicorpNode hashicorpNode = HashicorpNode.createAndStartHashicorp(withTLS);
try {
final String secretPath = "acceptanceTestSecretPath";
final String secretName = "secretName";

hashicorpNode.addSecretsToVault(singletonMap(secretName, PRIVATE_KEY), secretPath);

final Path keyConfigFile = testDirectory.resolve(configFilename + ".yaml");
METADATA_FILE_HELPERS.createHashicorpYamlFileAt(
keyConfigFile,
new HashicorpSigningParams(hashicorpNode, secretPath, secretName, KeyType.BLS),
Optional.ofNullable(httpProtocolVersion));

signAndVerifySignature(ArtifactType.BLOCK);
} finally {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
import java.nio.file.Path;
import java.security.SignatureException;
import java.security.interfaces.ECPublicKey;
import java.util.Optional;

import com.google.common.io.Resources;
import io.restassured.response.Response;
Expand Down Expand Up @@ -81,7 +82,9 @@ public void signDataWithKeyFromHashicorp() {
new HashicorpSigningParams(hashicorpNode, secretPath, secretName, KeyType.SECP256K1);

METADATA_FILE_HELPERS.createHashicorpYamlFileAt(
testDirectory.resolve(PUBLIC_KEY_HEX_STRING + ".yaml"), hashicorpSigningParams);
testDirectory.resolve(PUBLIC_KEY_HEX_STRING + ".yaml"),
hashicorpSigningParams,
Optional.empty());

signAndVerifySignature();
} finally {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,6 @@ class HashicorpIntegrationTest {
private static final String ROOT_TOKEN = "token";
private static final String EXPECTED_KEY_STRING =
"8f2a55949038a9610f50fb23b5883af3b4ecb3c3bb792cbcefbd1542c692be63";
private static final long TIMEOUT_MILLISECONDS = 10_000;

private final HashicorpConnectionFactory factory = new HashicorpConnectionFactory();

Expand All @@ -58,11 +57,10 @@ void hashicorpVaultReturnsEncryptionKey() {
.withBody("{\"data\":{\"data\":{\"value\":\"" + EXPECTED_KEY_STRING + "\"}}}"));

final ConnectionParameters connectionParameters =
new ConnectionParameters(
DEFAULT_HOST,
Optional.of(clientAndServer.getLocalPort()),
Optional.empty(),
Optional.of(TIMEOUT_MILLISECONDS));
ConnectionParameters.newBuilder()
.withServerHost(DEFAULT_HOST)
.withServerPort(clientAndServer.getLocalPort())
.build();
final KeyDefinition key = new KeyDefinition(KEY_PATH, Optional.empty(), ROOT_TOKEN);

final HashicorpConnection connection = factory.create(connectionParameters);
Expand Down Expand Up @@ -92,11 +90,11 @@ void hashicorpVaultReturnsEncryptionKeyOverTls() {
Path.of(keyStoreFactory.keyStoreFileName),
KeyStoreFactory.KEY_STORE_PASSWORD);
final ConnectionParameters connectionParameters =
new ConnectionParameters(
DEFAULT_HOST,
Optional.of(clientAndServer.getLocalPort()),
Optional.of(tlsOptions),
Optional.of(TIMEOUT_MILLISECONDS));
ConnectionParameters.newBuilder()
.withServerHost(DEFAULT_HOST)
.withServerPort(clientAndServer.getLocalPort())
.withTlsOptions(tlsOptions)
.build();
final KeyDefinition key = new KeyDefinition(KEY_PATH, Optional.empty(), ROOT_TOKEN);

final HashicorpConnection connection = factory.create(connectionParameters);
Expand All @@ -113,11 +111,11 @@ void exceptionThrownWhenHashicorpVaultAccessTimeout() throws IOException {
.respond(response().withDelay(TimeUnit.SECONDS, 5));

final ConnectionParameters connectionParameters =
new ConnectionParameters(
DEFAULT_HOST,
Optional.of(clientAndServer.getLocalPort()),
Optional.empty(),
Optional.of(1L));
ConnectionParameters.newBuilder()
.withServerHost(DEFAULT_HOST)
.withServerPort(clientAndServer.getLocalPort())
.withTimeoutMs(1L)
.build();
final KeyDefinition key = new KeyDefinition(KEY_PATH, Optional.empty(), ROOT_TOKEN);

final HashicorpConnection connection = factory.create(connectionParameters);
Expand All @@ -132,11 +130,10 @@ void exceptionThrownWhenHashicorpVaultReturnInvalidStatusCode() throws IOExcepti
clientAndServer.when(request().withPath(".*")).respond(response().withStatusCode(500));

final ConnectionParameters connectionParameters =
new ConnectionParameters(
DEFAULT_HOST,
Optional.of(clientAndServer.getLocalPort()),
Optional.empty(),
Optional.of(TIMEOUT_MILLISECONDS));
ConnectionParameters.newBuilder()
.withServerHost(DEFAULT_HOST)
.withServerPort(clientAndServer.getLocalPort())
.build();
final KeyDefinition key = new KeyDefinition(KEY_PATH, Optional.empty(), ROOT_TOKEN);

final HashicorpConnection connection = factory.create(connectionParameters);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,8 @@ private HttpClient getHttpClient(ConnectionParameters connectionParameters) {
_key -> {
final HttpClient.Builder httpClientBuilder =
HttpClient.newBuilder()
.followRedirects(HttpClient.Redirect.NORMAL)
.version(connectionParameters.getHttpProtocolVersion())
.connectTimeout(Duration.ofMillis(connectionParameters.getTimeoutMilliseconds()));
try {
if (connectionParameters.getTlsOptions().isPresent()) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,39 +12,40 @@
*/
package tech.pegasys.web3signer.keystorage.hashicorp.config;

import static com.google.common.base.Preconditions.checkNotNull;

import java.net.URI;
import java.net.http.HttpClient;
import java.util.Optional;

public class ConnectionParameters {
private static final Long DEFAULT_TIMEOUT_MILLISECONDS = 10_000L;
private static final Integer DEFAULT_SERVER_PORT = 8200;
private final String serverHost;
private final int serverPort;
private final Optional<TlsOptions> tlsOptions;
private final long timeoutMs;
private final HttpClient.Version httpProtocolVersion;

private final URI vaultURI;

public static Builder newBuilder() {
return new Builder();
}

/* Optional parameters will be set to their defaults when connecting */
public ConnectionParameters(
private ConnectionParameters(
final String serverHost,
final Optional<Integer> serverPort,
final Optional<TlsOptions> tlsOptions,
final Optional<Long> timeoutMs) {
this.serverHost = serverHost;
this.serverPort = serverPort.orElse(DEFAULT_SERVER_PORT);
final Optional<Long> timeoutMs,
final Optional<HttpClient.Version> httpProtocolVersion) {
this.tlsOptions = tlsOptions;
this.timeoutMs = timeoutMs.orElse(DEFAULT_TIMEOUT_MILLISECONDS);
this.httpProtocolVersion = httpProtocolVersion.orElse(HttpClient.Version.HTTP_2);
final String scheme = tlsOptions.isPresent() ? "https" : "http";
this.vaultURI = URI.create(String.format("%s://%s:%d", scheme, serverHost, this.serverPort));
}

public String getServerHost() {
return serverHost;
}

public int getServerPort() {
return serverPort;
this.vaultURI =
URI.create(
String.format(
"%s://%s:%d", scheme, serverHost, serverPort.orElse(DEFAULT_SERVER_PORT)));
}

public Optional<TlsOptions> getTlsOptions() {
Expand All @@ -58,4 +59,49 @@ public long getTimeoutMilliseconds() {
public URI getVaultURI() {
return vaultURI;
}

public HttpClient.Version getHttpProtocolVersion() {
return httpProtocolVersion;
}

public static final class Builder {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice improvement 👍

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I realised that this class is used in our tests as well and we were forced to set values that were not required by the specific tests. Builder patterns neatly allows the defaults to be set.

private String serverHost;
private Optional<Integer> serverPort = Optional.empty();
private Optional<TlsOptions> tlsOptions = Optional.empty();
private Optional<Long> timeoutMs = Optional.empty();
private Optional<HttpClient.Version> httpProtocolVersion = Optional.empty();

Builder() {}

public Builder withServerHost(final String serverHost) {
this.serverHost = serverHost;
return this;
}

public Builder withServerPort(final Integer serverPort) {
this.serverPort = Optional.ofNullable(serverPort);
return this;
}

public Builder withTlsOptions(final TlsOptions tlsOptions) {
this.tlsOptions = Optional.ofNullable(tlsOptions);
return this;
}

public Builder withTimeoutMs(final Long timeoutMs) {
this.timeoutMs = Optional.ofNullable(timeoutMs);
return this;
}

public Builder withHttpProtocolVersion(final HttpClient.Version httpProtocolVersion) {
this.httpProtocolVersion = Optional.ofNullable(httpProtocolVersion);
return this;
}

public ConnectionParameters build() {
checkNotNull(serverHost, "Hashicorp host cannot be null");
return new ConnectionParameters(
serverHost, serverPort, tlsOptions, timeoutMs, httpProtocolVersion);
}
}
}
Loading