You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@hugegraph.apache.org by ji...@apache.org on 2026/10/10 11:16:58 UTC

(hugegraph) branch master updated: fix(common): verify HTTPS certificate identity (#3307)

This is an automated email from the ASF dual-hosted git repository.

imbajin pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/hugegraph.git


The following commit(s) were added to refs/heads/master by this push:
     new 87a06a9b6 fix(common): verify HTTPS certificate identity (#3307)
87a06a9b6 is described below

commit 87a06a9b617e79e5d5b2dd3d75585f7f80d67a25
Author: imbajin <ji...@apache.org>
AuthorDate: Sat Oct 10 19:16:53 2026 +0800

    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.
---
 docs/CONTRIBUTING.md                               |   4 +-
 docs/shared-foundation-migration.md                |  14 +-
 hugegraph-commons/hugegraph-common/README.md       |   9 ++
 .../apache/hugegraph/rest/AbstractRestClient.java  |  27 ++--
 .../apache/hugegraph/unit/rest/RestClientTest.java | 147 +++++++++++++++++----
 5 files changed, 153 insertions(+), 48 deletions(-)

diff --git a/docs/CONTRIBUTING.md b/docs/CONTRIBUTING.md
index 32a1d0f87..58bf79c31 100644
--- a/docs/CONTRIBUTING.md
+++ b/docs/CONTRIBUTING.md
@@ -58,10 +58,12 @@ and automatic cancellation and retries. Report what you actually validated and a
 Install/package the current reactor before regenerating the inventory:
 
 ```bash
-mvn install -DskipTests -Dmaven.javadoc.skip=true
+mvn clean install -DskipTests -Dmaven.javadoc.skip=true
 bash install-dist/scripts/dependency/regenerate_known_dependencies.sh
 ```
 
+Clean rebuilds the current revision's distributions to avoid stale dependency jars.
+It also removes root-level `*.tar.gz` distribution archives; other revision directories remain.
 The build must produce Server, PD and Store distributions for the same current revision.
 It skips tests and is not validation evidence. The inventory combines Maven runtime
 dependencies with flat and nested distribution jars, including Spring Boot `BOOT-INF/lib`
diff --git a/docs/shared-foundation-migration.md b/docs/shared-foundation-migration.md
index 67795b659..f39dc5467 100644
--- a/docs/shared-foundation-migration.md
+++ b/docs/shared-foundation-migration.md
@@ -71,9 +71,10 @@ Id id = IdGenerator.of("alice");
 
 | Previous entry | Shared entry or migration |
 |----------------|---------------------------|
-| `org.apache.hugegraph.backend.id.*` | `org.apache.hugegraph.id.*` |
+| Shared types in `org.apache.hugegraph.backend.id` | `org.apache.hugegraph.id.*` |
 | `org.apache.hugegraph.schema.*` metadata | `org.apache.hugegraph.struct.schema.*` |
-| `org.apache.hugegraph.backend.query.*` | `org.apache.hugegraph.query.*` |
+| Shared types in `org.apache.hugegraph.backend.query` | `org.apache.hugegraph.query.*` |
+| `org.apache.hugegraph.backend.query.serializer.*` | `org.apache.hugegraph.query.serializer.*` |
 | `org.apache.hugegraph.backend.store.Shard` | `org.apache.hugegraph.backend.Shard` |
 | `org.apache.hugegraph.backend.store.BackendEntry.BackendColumn` | `org.apache.hugegraph.backend.BackendColumn` |
 | `org.apache.hugegraph.structure.HugeIndex` | `org.apache.hugegraph.structure.Index` |
@@ -81,7 +82,9 @@ Id id = IdGenerator.of("alice");
 | Core `HugeException` | `org.apache.hugegraph.exception.HugeException` |
 | `org.apache.hugegraph.SchemaGraph` / `SchemaDriver` | `org.apache.hugegraph.store.schema.*` |
 
-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.
+Keep the core imports for `SnowflakeIdGenerator` and the query execution helpers
+`QueryResults`, `ConditionQueryFlatten`, `EdgesQueryIterator`, `QueryBatch` and `QueryResultContext`.
+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.
 
 ### Check API and SPI implementations
 
@@ -165,7 +168,10 @@ pd:
   cluster: hg
 ```
 
-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.
+The environment equivalent is `PD_CLUSTER`. Match Server's `cluster` when `usePD=true`.
+With `usePD=false`, the first HStore graph opened binds the process-wide `MetaManager` to its `pd.cluster`;
+a later graph's explicit conflicting value is logged and ignored. Configure all HStore graphs in the
+process with the same namespace. Store rejects reuse of its schema driver under a conflicting namespace.
 
 Keep backend graph-name components such as `DEFAULT/hugegraph/g` (`graphspace/store/table`). The REST identity `DEFAULT-hugegraph` is not a metadata key.
 
diff --git a/hugegraph-commons/hugegraph-common/README.md b/hugegraph-commons/hugegraph-common/README.md
index 9598cfb61..df04380db 100644
--- a/hugegraph-commons/hugegraph-common/README.md
+++ b/hugegraph-commons/hugegraph-common/README.md
@@ -16,6 +16,15 @@ The shared REST client encodes request bodies using the charset declared in
 `Content-Type`, or UTF-8 when no charset is declared. JSON and text requests do
 not depend on the JVM's default charset. Gzip compresses those same encoded bytes.
 
+## HTTPS certificate verification
+
+A custom trust store configures which certificate authorities or certificates the
+REST client trusts. It does not disable server identity verification: the
+certificate must also contain a Subject Alternative Name matching the requested
+DNS name or IP address. Certificates with a different identity or no matching
+Subject Alternative Name are rejected, including certificates explicitly trusted
+by the configured store. This uses OkHttp's default hostname verifier.
+
 ## Components
 
 - Lock: atomic lock, key lock, lock group and lock manager
diff --git a/hugegraph-commons/hugegraph-common/src/main/java/org/apache/hugegraph/rest/AbstractRestClient.java b/hugegraph-commons/hugegraph-common/src/main/java/org/apache/hugegraph/rest/AbstractRestClient.java
index ecb7deca3..a38dcb69a 100644
--- a/hugegraph-commons/hugegraph-common/src/main/java/org/apache/hugegraph/rest/AbstractRestClient.java
+++ b/hugegraph-commons/hugegraph-common/src/main/java/org/apache/hugegraph/rest/AbstractRestClient.java
@@ -20,7 +20,6 @@ package org.apache.hugegraph.rest;
 import java.io.FileInputStream;
 import java.io.IOException;
 import java.io.UnsupportedEncodingException;
-import java.net.URI;
 import java.net.URLEncoder;
 import java.nio.charset.StandardCharsets;
 import java.security.KeyStore;
@@ -31,7 +30,6 @@ import java.util.Objects;
 import java.util.concurrent.TimeUnit;
 
 import javax.net.ssl.HostnameVerifier;
-import javax.net.ssl.HttpsURLConnection;
 import javax.net.ssl.SSLContext;
 import javax.net.ssl.SSLSession;
 import javax.net.ssl.SSLSocketFactory;
@@ -211,7 +209,7 @@ public abstract class AbstractRestClient implements RestClient {
         }
 
         // ssl
-        configSsl(builder, this.baseUrl, config.getTrustStoreFile(),
+        configSsl(builder, config.getTrustStoreFile(),
                   config.getTrustStorePassword());
 
         // Execute builder callback before builder.build() for user configs
@@ -233,7 +231,7 @@ public abstract class AbstractRestClient implements RestClient {
     }
 
     @SneakyThrows
-    private void configSsl(OkHttpClient.Builder builder, String url, String trustStoreFile,
+    private void configSsl(OkHttpClient.Builder builder, String trustStoreFile,
                            String trustStorePass) {
         if (StringUtils.isBlank(trustStoreFile) || StringUtils.isBlank(trustStorePass)) {
             return;
@@ -244,8 +242,7 @@ public abstract class AbstractRestClient implements RestClient {
         sslContext.init(null, new TrustManager[]{trustManager}, null);
         SSLSocketFactory sslSocketFactory = sslContext.getSocketFactory();
 
-        builder.sslSocketFactory(sslSocketFactory, trustManager)
-               .hostnameVerifier(new HostNameVerifier(url));
+        builder.sslSocketFactory(sslSocketFactory, trustManager);
     }
 
     @Override
@@ -465,26 +462,20 @@ public abstract class AbstractRestClient implements RestClient {
         return (X509TrustManager) trustManagers[0];
     }
 
+    /**
+     * Compatibility wrapper using the same certificate identity verification as OkHttp.
+     */
     public static class HostNameVerifier implements HostnameVerifier {
 
-        private final String url;
+        private static final HostnameVerifier DEFAULT_VERIFIER = new OkHttpClient().hostnameVerifier();
 
         public HostNameVerifier(String url) {
-            if (!url.startsWith("http://") && !url.startsWith("https://")) {
-                url = "http://" + url;
-            }
-            url = URI.create(url).getHost();
-            this.url = url;
+            // Keep the public constructor; a URL is not evidence of the peer's identity.
         }
 
         @Override
         public boolean verify(String hostname, SSLSession session) {
-            if (!this.url.isEmpty() && this.url.endsWith(hostname)) {
-                return true;
-            } else {
-                HostnameVerifier verifier = HttpsURLConnection.getDefaultHostnameVerifier();
-                return verifier.verify(hostname, session);
-            }
+            return DEFAULT_VERIFIER.verify(hostname, session);
         }
     }
 }
diff --git a/hugegraph-commons/hugegraph-common/src/test/java/org/apache/hugegraph/unit/rest/RestClientTest.java b/hugegraph-commons/hugegraph-common/src/test/java/org/apache/hugegraph/unit/rest/RestClientTest.java
index 93a69dd8e..8f61a8460 100644
--- a/hugegraph-commons/hugegraph-common/src/test/java/org/apache/hugegraph/unit/rest/RestClientTest.java
+++ b/hugegraph-commons/hugegraph-common/src/test/java/org/apache/hugegraph/unit/rest/RestClientTest.java
@@ -17,16 +17,29 @@
 
 package org.apache.hugegraph.unit.rest;
 
-import java.security.NoSuchAlgorithmException;
+import java.io.InputStream;
+import java.io.OutputStream;
+import java.net.InetSocketAddress;
+import java.nio.charset.StandardCharsets;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.nio.file.Paths;
+import java.security.KeyStore;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.Comparator;
 import java.util.HashMap;
+import java.util.List;
 import java.util.Map;
 import java.util.UUID;
 import java.util.concurrent.TimeUnit;
-import java.util.function.BiFunction;
+import java.util.stream.Stream;
 
+import javax.net.ssl.KeyManagerFactory;
 import javax.net.ssl.SSLContext;
+import javax.net.ssl.SSLHandshakeException;
+import javax.net.ssl.SSLPeerUnverifiedException;
 import javax.net.ssl.SSLSession;
-import javax.net.ssl.SSLSessionContext;
 
 import org.apache.hugegraph.rest.AbstractRestClient;
 import org.apache.hugegraph.rest.ClientException;
@@ -43,6 +56,8 @@ import org.mockito.Mockito;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
 import com.google.common.net.HttpHeaders;
+import com.sun.net.httpserver.HttpsConfigurator;
+import com.sun.net.httpserver.HttpsServer;
 
 import lombok.SneakyThrows;
 import okhttp3.HttpUrl;
@@ -145,30 +160,112 @@ public class RestClientTest {
     }
 
     @Test
-    public void testHostNameVerifier() {
-        BiFunction<String, String, Boolean> verifer = (url, hostname) -> {
-            AbstractRestClient.HostNameVerifier verifier;
-            SSLSession session;
-            try {
-                SSLSessionContext sc = SSLContext.getDefault()
-                                                 .getClientSessionContext();
-                session = sc.getSession(new byte[]{11});
-                verifier = new AbstractRestClient.HostNameVerifier(url);
-            } catch (NoSuchAlgorithmException e) {
-                throw new RuntimeException(e);
+    public void testHttpsCertificateIdentity() throws Exception {
+        Path directory = Files.createTempDirectory("rest-client-tls-");
+        try {
+            Path validStore = createServerStore(directory, "valid", "dns:localhost,ip:127.0.0.1");
+            Path wrongStore = createServerStore(directory, "wrong", "dns:other.example");
+            Path noSanStore = createServerStore(directory, "no-san", null);
+            assertHttpsRequest(validStore, validStore, "localhost", null);
+            assertHttpsRequest(validStore, validStore, "127.0.0.1", null);
+            assertHttpsRequest(wrongStore, wrongStore, "localhost", SSLPeerUnverifiedException.class);
+            assertHttpsRequest(noSanStore, noSanStore, "localhost", SSLPeerUnverifiedException.class);
+            assertHttpsRequest(validStore, wrongStore, "localhost", SSLHandshakeException.class);
+        } finally {
+            try (Stream<Path> paths = Files.walk(directory)) {
+                for (Path path : (Iterable<Path>) paths.sorted(Comparator.reverseOrder())::iterator) {
+                    Files.deleteIfExists(path);
+                }
             }
-            return verifier.verify(hostname, session);
-        };
+        }
+    }
 
-        Assert.assertTrue(verifer.apply("http://baidu.com", "baidu.com"));
-        Assert.assertTrue(verifer.apply("http://test1.baidu.com", "baidu.com"));
-        Assert.assertTrue(verifer.apply("http://test2.baidu.com", "baidu.com"));
-        Assert.assertFalse(verifer.apply("http://baidu2.com", "baidu.com"));
-        Assert.assertTrue(verifer.apply("http://baidu.com", ""));
-        Assert.assertTrue(verifer.apply("baidu.com", "baidu.com"));
-        Assert.assertTrue(verifer.apply("http://baidu.com/test", "baidu.com"));
-        Assert.assertTrue(verifer.apply("baidu.com/test/abc", "baidu.com"));
-        Assert.assertFalse(verifer.apply("baidu.com.sina.com", "baidu.com"));
+    private static Path createServerStore(Path directory, String name, String san) throws Exception {
+        Path store = directory.resolve(name + ".p12");
+        List<String> command = new ArrayList<>(Arrays.asList(
+                Paths.get(System.getProperty("java.home"), "bin", "keytool").toString(),
+                "-genkeypair", "-alias", "server", "-keyalg", "RSA", "-keysize", "2048",
+                "-validity", "2", "-dname", "CN=localhost", "-storetype", "PKCS12",
+                "-keystore", store.toString(), "-storepass", "test-password",
+                "-keypass", "test-password", "-noprompt"));
+        if (san != null) {
+            command.addAll(Arrays.asList("-ext", "SAN=" + san));
+        }
+        Path log = directory.resolve(name + ".log");
+        Process process = new ProcessBuilder(command).redirectErrorStream(true)
+                                                     .redirectOutput(log.toFile()).start();
+        try {
+            boolean finished = process.waitFor(30, TimeUnit.SECONDS);
+            Assert.assertTrue("keytool timed out", finished);
+            Assert.assertEquals(Files.readString(log, StandardCharsets.UTF_8), 0, process.exitValue());
+        } finally {
+            if (process.isAlive()) {
+                process.destroyForcibly();
+                process.waitFor(5, TimeUnit.SECONDS);
+            }
+        }
+        return store;
+    }
+
+    private static KeyStore readStore(Path path) throws Exception {
+        KeyStore store = KeyStore.getInstance("PKCS12");
+        try (InputStream input = Files.newInputStream(path)) {
+            store.load(input, "test-password".toCharArray());
+        }
+        return store;
+    }
+
+    private static void assertHttpsRequest(Path serverStore, Path trustedStore, String hostname,
+                                           Class<? extends Throwable> failure) throws Exception {
+        KeyStore keys = readStore(serverStore);
+        KeyManagerFactory keyManagers = KeyManagerFactory.getInstance(KeyManagerFactory.getDefaultAlgorithm());
+        keyManagers.init(keys, "test-password".toCharArray());
+        SSLContext context = SSLContext.getInstance("TLS");
+        context.init(keyManagers.getKeyManagers(), null, null);
+        HttpsServer server = HttpsServer.create(new InetSocketAddress("127.0.0.1", 0), 0);
+        server.setHttpsConfigurator(new HttpsConfigurator(context));
+        server.createContext("/probe", exchange -> {
+            byte[] response = "{}".getBytes(StandardCharsets.UTF_8);
+            exchange.sendResponseHeaders(200, response.length);
+            try (OutputStream output = exchange.getResponseBody()) {
+                output.write(response);
+            }
+        });
+        Path trustStore = Files.createTempFile(serverStore.getParent(), "trust-", ".jks");
+        server.start();
+        try {
+            KeyStore trusted = KeyStore.getInstance("JKS");
+            trusted.load(null, null);
+            trusted.setCertificateEntry("server", readStore(trustedStore).getCertificate("server"));
+            try (OutputStream output = Files.newOutputStream(trustStore)) {
+                trusted.store(output, "test-password".toCharArray());
+            }
+            RestClientConfig config = RestClientConfig.builder().timeout(5000)
+                    .trustStoreFile(trustStore.toString()).trustStorePassword("test-password").build();
+            String url = "https://" + hostname + ":" + server.getAddress().getPort();
+            RestClient client = new AbstractRestClient(url, config) {
+                @Override
+                protected void checkStatus(Response response, int... statuses) {
+                    Assert.assertEquals(200, response.code());
+                }
+            };
+            try {
+                if (failure == null) {
+                    Assert.assertEquals(200, client.get("probe").status());
+                } else {
+                    org.junit.Assert.assertThrows(failure, () -> client.get("probe"));
+                }
+            } finally {
+                client.close();
+            }
+            SSLSession session = Mockito.mock(SSLSession.class);
+            Mockito.when(session.getPeerCertificates()).thenReturn(keys.getCertificateChain("server"));
+            Assert.assertEquals(failure != SSLPeerUnverifiedException.class,
+                                new AbstractRestClient.HostNameVerifier(url).verify(hostname, session));
+        } finally {
+            server.stop(0);
+            Files.deleteIfExists(trustStore);
+        }
     }
 
     @Test