Skip to content

Commit 87a06a9

Browse files
authored
fix(common): verify HTTPS certificate identity (#3307)
A custom truststore previously allowed a trusted certificate with the wrong server identity. HTTPS requests now retain OkHttp's default hostname verification: trust alone is insufficient, and a matching DNS or IP Subject Alternative Name is required. The public HostNameVerifier(String) constructor remains available.
1 parent 6df8dd9 commit 87a06a9

5 files changed

Lines changed: 153 additions & 48 deletions

File tree

‎docs/CONTRIBUTING.md‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -58,10 +58,12 @@ and automatic cancellation and retries. Report what you actually validated and a
5858
Install/package the current reactor before regenerating the inventory:
5959

6060
```bash
61-
mvn install -DskipTests -Dmaven.javadoc.skip=true
61+
mvn clean install -DskipTests -Dmaven.javadoc.skip=true
6262
bash install-dist/scripts/dependency/regenerate_known_dependencies.sh
6363
```
6464

65+
Clean rebuilds the current revision's distributions to avoid stale dependency jars.
66+
It also removes root-level `*.tar.gz` distribution archives; other revision directories remain.
6567
The build must produce Server, PD and Store distributions for the same current revision.
6668
It skips tests and is not validation evidence. The inventory combines Maven runtime
6769
dependencies with flat and nested distribution jars, including Spring Boot `BOOT-INF/lib`

‎docs/shared-foundation-migration.md‎

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -71,17 +71,20 @@ Id id = IdGenerator.of("alice");
7171

7272
| Previous entry | Shared entry or migration |
7373
|----------------|---------------------------|
74-
| `org.apache.hugegraph.backend.id.*` | `org.apache.hugegraph.id.*` |
74+
| Shared types in `org.apache.hugegraph.backend.id` | `org.apache.hugegraph.id.*` |
7575
| `org.apache.hugegraph.schema.*` metadata | `org.apache.hugegraph.struct.schema.*` |
76-
| `org.apache.hugegraph.backend.query.*` | `org.apache.hugegraph.query.*` |
76+
| Shared types in `org.apache.hugegraph.backend.query` | `org.apache.hugegraph.query.*` |
77+
| `org.apache.hugegraph.backend.query.serializer.*` | `org.apache.hugegraph.query.serializer.*` |
7778
| `org.apache.hugegraph.backend.store.Shard` | `org.apache.hugegraph.backend.Shard` |
7879
| `org.apache.hugegraph.backend.store.BackendEntry.BackendColumn` | `org.apache.hugegraph.backend.BackendColumn` |
7980
| `org.apache.hugegraph.structure.HugeIndex` | `org.apache.hugegraph.structure.Index` |
8081
| Shared bytes/encoding in backend serializers | `org.apache.hugegraph.serializer.*` |
8182
| Core `HugeException` | `org.apache.hugegraph.exception.HugeException` |
8283
| `org.apache.hugegraph.SchemaGraph` / `SchemaDriver` | `org.apache.hugegraph.store.schema.*` |
8384

84-
The table applies to these shared types, not every class in a package. Schema mutation builders and backend-specific serializers remain in core. There is no general compatibility package for removed core classes. Relocated types also change method descriptors that expose IDs, schema, queries and indexes, so update implementations and call sites and recompile every affected integration against matching artifacts. Changing source imports does not make old binaries compatible.
85+
Keep the core imports for `SnowflakeIdGenerator` and the query execution helpers
86+
`QueryResults`, `ConditionQueryFlatten`, `EdgesQueryIterator`, `QueryBatch` and `QueryResultContext`.
87+
The table applies to shared types, not entire packages. `SchemaManager`, schema mutation builders and backend-specific serializers remain in core. There is no general compatibility package for removed core classes. Relocated types also change method descriptors that expose IDs, schema, queries and indexes, so update implementations and call sites and recompile every affected integration against matching artifacts. Changing source imports does not make old binaries compatible.
8588

8689
### Check API and SPI implementations
8790

@@ -165,7 +168,10 @@ pd:
165168
cluster: hg
166169
```
167170
168-
The environment equivalent is `PD_CLUSTER`. Match Server's `cluster` when `usePD=true`, or the graph's `pd.cluster` when `usePD=false`. One Store process cannot reuse its schema driver across conflicting namespaces.
171+
The environment equivalent is `PD_CLUSTER`. Match Server's `cluster` when `usePD=true`.
172+
With `usePD=false`, the first HStore graph opened binds the process-wide `MetaManager` to its `pd.cluster`;
173+
a later graph's explicit conflicting value is logged and ignored. Configure all HStore graphs in the
174+
process with the same namespace. Store rejects reuse of its schema driver under a conflicting namespace.
169175

170176
Keep backend graph-name components such as `DEFAULT/hugegraph/g` (`graphspace/store/table`). The REST identity `DEFAULT-hugegraph` is not a metadata key.
171177

‎hugegraph-commons/hugegraph-common/README.md‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,15 @@ The shared REST client encodes request bodies using the charset declared in
1616
`Content-Type`, or UTF-8 when no charset is declared. JSON and text requests do
1717
not depend on the JVM's default charset. Gzip compresses those same encoded bytes.
1818

19+
## HTTPS certificate verification
20+
21+
A custom trust store configures which certificate authorities or certificates the
22+
REST client trusts. It does not disable server identity verification: the
23+
certificate must also contain a Subject Alternative Name matching the requested
24+
DNS name or IP address. Certificates with a different identity or no matching
25+
Subject Alternative Name are rejected, including certificates explicitly trusted
26+
by the configured store. This uses OkHttp's default hostname verifier.
27+
1928
## Components
2029

2130
- Lock: atomic lock, key lock, lock group and lock manager

‎hugegraph-commons/hugegraph-common/src/main/java/org/apache/hugegraph/rest/AbstractRestClient.java‎

Lines changed: 9 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,6 @@
2020
import java.io.FileInputStream;
2121
import java.io.IOException;
2222
import java.io.UnsupportedEncodingException;
23-
import java.net.URI;
2423
import java.net.URLEncoder;
2524
import java.nio.charset.StandardCharsets;
2625
import java.security.KeyStore;
@@ -31,7 +30,6 @@
3130
import java.util.concurrent.TimeUnit;
3231

3332
import javax.net.ssl.HostnameVerifier;
34-
import javax.net.ssl.HttpsURLConnection;
3533
import javax.net.ssl.SSLContext;
3634
import javax.net.ssl.SSLSession;
3735
import javax.net.ssl.SSLSocketFactory;
@@ -211,7 +209,7 @@ private OkHttpClient buildOkHttpClient(RestClientConfig config) {
211209
}
212210

213211
// ssl
214-
configSsl(builder, this.baseUrl, config.getTrustStoreFile(),
212+
configSsl(builder, config.getTrustStoreFile(),
215213
config.getTrustStorePassword());
216214

217215
// Execute builder callback before builder.build() for user configs
@@ -233,7 +231,7 @@ private OkHttpClient buildOkHttpClient(RestClientConfig config) {
233231
}
234232

235233
@SneakyThrows
236-
private void configSsl(OkHttpClient.Builder builder, String url, String trustStoreFile,
234+
private void configSsl(OkHttpClient.Builder builder, String trustStoreFile,
237235
String trustStorePass) {
238236
if (StringUtils.isBlank(trustStoreFile) || StringUtils.isBlank(trustStorePass)) {
239237
return;
@@ -244,8 +242,7 @@ private void configSsl(OkHttpClient.Builder builder, String url, String trustSto
244242
sslContext.init(null, new TrustManager[]{trustManager}, null);
245243
SSLSocketFactory sslSocketFactory = sslContext.getSocketFactory();
246244

247-
builder.sslSocketFactory(sslSocketFactory, trustManager)
248-
.hostnameVerifier(new HostNameVerifier(url));
245+
builder.sslSocketFactory(sslSocketFactory, trustManager);
249246
}
250247

251248
@Override
@@ -465,26 +462,20 @@ private X509TrustManager trustManagerForCertificates(String trustStoreFile,
465462
return (X509TrustManager) trustManagers[0];
466463
}
467464

465+
/**
466+
* Compatibility wrapper using the same certificate identity verification as OkHttp.
467+
*/
468468
public static class HostNameVerifier implements HostnameVerifier {
469469

470-
private final String url;
470+
private static final HostnameVerifier DEFAULT_VERIFIER = new OkHttpClient().hostnameVerifier();
471471

472472
public HostNameVerifier(String url) {
473-
if (!url.startsWith("http://") && !url.startsWith("https://")) {
474-
url = "http://" + url;
475-
}
476-
url = URI.create(url).getHost();
477-
this.url = url;
473+
// Keep the public constructor; a URL is not evidence of the peer's identity.
478474
}
479475

480476
@Override
481477
public boolean verify(String hostname, SSLSession session) {
482-
if (!this.url.isEmpty() && this.url.endsWith(hostname)) {
483-
return true;
484-
} else {
485-
HostnameVerifier verifier = HttpsURLConnection.getDefaultHostnameVerifier();
486-
return verifier.verify(hostname, session);
487-
}
478+
return DEFAULT_VERIFIER.verify(hostname, session);
488479
}
489480
}
490481
}

‎hugegraph-commons/hugegraph-common/src/test/java/org/apache/hugegraph/unit/rest/RestClientTest.java‎

Lines changed: 122 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -17,16 +17,29 @@
1717

1818
package org.apache.hugegraph.unit.rest;
1919

20-
import java.security.NoSuchAlgorithmException;
20+
import java.io.InputStream;
21+
import java.io.OutputStream;
22+
import java.net.InetSocketAddress;
23+
import java.nio.charset.StandardCharsets;
24+
import java.nio.file.Files;
25+
import java.nio.file.Path;
26+
import java.nio.file.Paths;
27+
import java.security.KeyStore;
28+
import java.util.ArrayList;
29+
import java.util.Arrays;
30+
import java.util.Comparator;
2131
import java.util.HashMap;
32+
import java.util.List;
2233
import java.util.Map;
2334
import java.util.UUID;
2435
import java.util.concurrent.TimeUnit;
25-
import java.util.function.BiFunction;
36+
import java.util.stream.Stream;
2637

38+
import javax.net.ssl.KeyManagerFactory;
2739
import javax.net.ssl.SSLContext;
40+
import javax.net.ssl.SSLHandshakeException;
41+
import javax.net.ssl.SSLPeerUnverifiedException;
2842
import javax.net.ssl.SSLSession;
29-
import javax.net.ssl.SSLSessionContext;
3043

3144
import org.apache.hugegraph.rest.AbstractRestClient;
3245
import org.apache.hugegraph.rest.ClientException;
@@ -43,6 +56,8 @@
4356
import com.google.common.collect.ImmutableList;
4457
import com.google.common.collect.ImmutableMap;
4558
import com.google.common.net.HttpHeaders;
59+
import com.sun.net.httpserver.HttpsConfigurator;
60+
import com.sun.net.httpserver.HttpsServer;
4661

4762
import lombok.SneakyThrows;
4863
import okhttp3.HttpUrl;
@@ -145,30 +160,112 @@ public void testPostHttpsWithTokenAndAllParams() {
145160
}
146161

147162
@Test
148-
public void testHostNameVerifier() {
149-
BiFunction<String, String, Boolean> verifer = (url, hostname) -> {
150-
AbstractRestClient.HostNameVerifier verifier;
151-
SSLSession session;
152-
try {
153-
SSLSessionContext sc = SSLContext.getDefault()
154-
.getClientSessionContext();
155-
session = sc.getSession(new byte[]{11});
156-
verifier = new AbstractRestClient.HostNameVerifier(url);
157-
} catch (NoSuchAlgorithmException e) {
158-
throw new RuntimeException(e);
163+
public void testHttpsCertificateIdentity() throws Exception {
164+
Path directory = Files.createTempDirectory("rest-client-tls-");
165+
try {
166+
Path validStore = createServerStore(directory, "valid", "dns:localhost,ip:127.0.0.1");
167+
Path wrongStore = createServerStore(directory, "wrong", "dns:other.example");
168+
Path noSanStore = createServerStore(directory, "no-san", null);
169+
assertHttpsRequest(validStore, validStore, "localhost", null);
170+
assertHttpsRequest(validStore, validStore, "127.0.0.1", null);
171+
assertHttpsRequest(wrongStore, wrongStore, "localhost", SSLPeerUnverifiedException.class);
172+
assertHttpsRequest(noSanStore, noSanStore, "localhost", SSLPeerUnverifiedException.class);
173+
assertHttpsRequest(validStore, wrongStore, "localhost", SSLHandshakeException.class);
174+
} finally {
175+
try (Stream<Path> paths = Files.walk(directory)) {
176+
for (Path path : (Iterable<Path>) paths.sorted(Comparator.reverseOrder())::iterator) {
177+
Files.deleteIfExists(path);
178+
}
159179
}
160-
return verifier.verify(hostname, session);
161-
};
180+
}
181+
}
162182

163-
Assert.assertTrue(verifer.apply("http://baidu.com", "baidu.com"));
164-
Assert.assertTrue(verifer.apply("http://test1.baidu.com", "baidu.com"));
165-
Assert.assertTrue(verifer.apply("http://test2.baidu.com", "baidu.com"));
166-
Assert.assertFalse(verifer.apply("http://baidu2.com", "baidu.com"));
167-
Assert.assertTrue(verifer.apply("http://baidu.com", ""));
168-
Assert.assertTrue(verifer.apply("baidu.com", "baidu.com"));
169-
Assert.assertTrue(verifer.apply("http://baidu.com/test", "baidu.com"));
170-
Assert.assertTrue(verifer.apply("baidu.com/test/abc", "baidu.com"));
171-
Assert.assertFalse(verifer.apply("baidu.com.sina.com", "baidu.com"));
183+
private static Path createServerStore(Path directory, String name, String san) throws Exception {
184+
Path store = directory.resolve(name + ".p12");
185+
List<String> command = new ArrayList<>(Arrays.asList(
186+
Paths.get(System.getProperty("java.home"), "bin", "keytool").toString(),
187+
"-genkeypair", "-alias", "server", "-keyalg", "RSA", "-keysize", "2048",
188+
"-validity", "2", "-dname", "CN=localhost", "-storetype", "PKCS12",
189+
"-keystore", store.toString(), "-storepass", "test-password",
190+
"-keypass", "test-password", "-noprompt"));
191+
if (san != null) {
192+
command.addAll(Arrays.asList("-ext", "SAN=" + san));
193+
}
194+
Path log = directory.resolve(name + ".log");
195+
Process process = new ProcessBuilder(command).redirectErrorStream(true)
196+
.redirectOutput(log.toFile()).start();
197+
try {
198+
boolean finished = process.waitFor(30, TimeUnit.SECONDS);
199+
Assert.assertTrue("keytool timed out", finished);
200+
Assert.assertEquals(Files.readString(log, StandardCharsets.UTF_8), 0, process.exitValue());
201+
} finally {
202+
if (process.isAlive()) {
203+
process.destroyForcibly();
204+
process.waitFor(5, TimeUnit.SECONDS);
205+
}
206+
}
207+
return store;
208+
}
209+
210+
private static KeyStore readStore(Path path) throws Exception {
211+
KeyStore store = KeyStore.getInstance("PKCS12");
212+
try (InputStream input = Files.newInputStream(path)) {
213+
store.load(input, "test-password".toCharArray());
214+
}
215+
return store;
216+
}
217+
218+
private static void assertHttpsRequest(Path serverStore, Path trustedStore, String hostname,
219+
Class<? extends Throwable> failure) throws Exception {
220+
KeyStore keys = readStore(serverStore);
221+
KeyManagerFactory keyManagers = KeyManagerFactory.getInstance(KeyManagerFactory.getDefaultAlgorithm());
222+
keyManagers.init(keys, "test-password".toCharArray());
223+
SSLContext context = SSLContext.getInstance("TLS");
224+
context.init(keyManagers.getKeyManagers(), null, null);
225+
HttpsServer server = HttpsServer.create(new InetSocketAddress("127.0.0.1", 0), 0);
226+
server.setHttpsConfigurator(new HttpsConfigurator(context));
227+
server.createContext("/probe", exchange -> {
228+
byte[] response = "{}".getBytes(StandardCharsets.UTF_8);
229+
exchange.sendResponseHeaders(200, response.length);
230+
try (OutputStream output = exchange.getResponseBody()) {
231+
output.write(response);
232+
}
233+
});
234+
Path trustStore = Files.createTempFile(serverStore.getParent(), "trust-", ".jks");
235+
server.start();
236+
try {
237+
KeyStore trusted = KeyStore.getInstance("JKS");
238+
trusted.load(null, null);
239+
trusted.setCertificateEntry("server", readStore(trustedStore).getCertificate("server"));
240+
try (OutputStream output = Files.newOutputStream(trustStore)) {
241+
trusted.store(output, "test-password".toCharArray());
242+
}
243+
RestClientConfig config = RestClientConfig.builder().timeout(5000)
244+
.trustStoreFile(trustStore.toString()).trustStorePassword("test-password").build();
245+
String url = "https://" + hostname + ":" + server.getAddress().getPort();
246+
RestClient client = new AbstractRestClient(url, config) {
247+
@Override
248+
protected void checkStatus(Response response, int... statuses) {
249+
Assert.assertEquals(200, response.code());
250+
}
251+
};
252+
try {
253+
if (failure == null) {
254+
Assert.assertEquals(200, client.get("probe").status());
255+
} else {
256+
org.junit.Assert.assertThrows(failure, () -> client.get("probe"));
257+
}
258+
} finally {
259+
client.close();
260+
}
261+
SSLSession session = Mockito.mock(SSLSession.class);
262+
Mockito.when(session.getPeerCertificates()).thenReturn(keys.getCertificateChain("server"));
263+
Assert.assertEquals(failure != SSLPeerUnverifiedException.class,
264+
new AbstractRestClient.HostNameVerifier(url).verify(hostname, session));
265+
} finally {
266+
server.stop(0);
267+
Files.deleteIfExists(trustStore);
268+
}
172269
}
173270

174271
@Test

0 commit comments

Comments
 (0)