github-actions[bot] commented on code in PR #64697:
URL: https://github.com/apache/doris/pull/64697#discussion_r3534493863
##########
fe/fe-core/src/main/java/org/apache/doris/plugin/audit/AuditStreamLoader.java:
##########
@@ -34,30 +37,35 @@
import java.net.URL;
import java.util.Calendar;
import java.util.stream.Collectors;
+import javax.net.ssl.HttpsURLConnection;
public class AuditStreamLoader {
private static final Logger LOG =
LogManager.getLogger(AuditStreamLoader.class);
- private static String loadUrlPattern = "http://%s/api/%s/%s/_stream_load?";;
// timeout for both connection and read. 10 seconds is long enough.
private static final int HTTP_TIMEOUT_MS = 10000;
- private String hostPort;
private String db;
private String auditLogTbl;
private String auditLogLoadUrlStr;
private String feIdentity;
public AuditStreamLoader() {
- this.hostPort = "127.0.0.1:" + Config.http_port;
this.db = FeConstants.INTERNAL_DB_NAME;
this.auditLogTbl = AuditLoader.AUDIT_LOG_TABLE;
- this.auditLogLoadUrlStr = String.format(loadUrlPattern, hostPort, db,
auditLogTbl);
+ String scheme = Config.enable_https ? "https" : "http";
Review Comment:
This still breaks the HTTPS audit-loader path after the first hop. With this
URL, `LoadAction.streamLoad()` handles the token request and
`RestBaseController.buildRedirectUrl()` builds the BE `Location` using
`request.getScheme()`, while the selected address is `backend.getHttpPort()`.
BE heartbeats report `config::webserver_port` as that port, and the BE
stream-load `EvHttpServer` is plain HTTP, so the redirect becomes something
like `https://be:8040/api/.../_stream_load`. `loadBatch()` then opens that
`Location` as an `HttpsURLConnection`, attempting TLS against the BE HTTP port
and failing every audit batch. Please keep the FE hop HTTPS if needed, but
normalize the BE redirect hop to the BE HTTP scheme or adjust the stream-load
redirect for this internal audit path before following it.
##########
fe/fe-core/src/main/java/org/apache/doris/common/util/InternalHttpsUtils.java:
##########
@@ -0,0 +1,83 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you 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 org.apache.doris.common.util;
+
+import org.apache.doris.common.Config;
+
+import org.apache.logging.log4j.LogManager;
+import org.apache.logging.log4j.Logger;
+
+import java.io.InputStream;
+import java.nio.file.Files;
+import java.nio.file.Paths;
+import java.security.KeyStore;
+import javax.net.ssl.SSLContext;
+import javax.net.ssl.TrustManagerFactory;
+
+/**
+ * Utility for creating SSL-aware HTTP clients for internal FE communication.
+ *
+ * <p>Builds an {@link SSLContext} from the configured CA truststore once and
caches it.
+ * Hostname verification is disabled for IP-based intra-cluster connections.
+ * Certificate rotation requires a FE restart.
+ */
+public class InternalHttpsUtils {
+ private static volatile SSLContext cachedSslContext = null;
+
+ private static final Logger LOG =
LogManager.getLogger(InternalHttpsUtils.class);
+
+ /**
+ * Returns the cached SSLContext, building it from the configured
truststore on first call.
+ */
+ public static SSLContext getSslContext() {
+ if (cachedSslContext == null) {
+ synchronized (InternalHttpsUtils.class) {
+ if (cachedSslContext == null) {
+ cachedSslContext = buildSslContext();
+ }
+ }
+ }
+ return cachedSslContext;
+ }
+
+ private static SSLContext buildSslContext() {
+ try {
+ // The same CA signs all Doris TLS certs (FE HTTPS + MySQL SSL),
so mysql_ssl_default_ca_certificate
+ // is the correct trust anchor for FE HTTPS. Hostname verification
is skipped for IP-based comms.
+ KeyStore trustStore =
KeyStore.getInstance(Config.ssl_trust_store_type);
Review Comment:
This trusts a different configuration surface from the one that actually
serves FE HTTPS. The server certificate comes from
`key_store_path`/`key_store_type`/`key_store_alias` in `DorisFE` and
`HttpServer`, while this client only loads `mysql_ssl_default_ca_certificate`
as a PKCS12 trust store. For a normal HTTPS deployment that uses a custom FE
keystore signed by a CA that is not also imported into the MySQL SSL CA file,
`NoopHostnameVerifier` will skip hostname checks but certificate-chain
validation will still reject the FE certificate, so audit loading keeps
failing. Please either use/introduce FE HTTPS trust material for this internal
client, or enforce and test the requirement that the FE HTTPS certificate chain
is present in this truststore.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
