This is an automated email from the ASF dual-hosted git repository.
mridulpathak pushed a commit to branch trunk
in repository https://gitbox.apache.org/repos/asf/ofbiz-framework.git
The following commit(s) were added to refs/heads/trunk by this push:
new 5781035741 Improved: Remove the accept-any-certificate TrustManager
from SSLUtil (OFBIZ-13638)
5781035741 is described below
commit 57810357416c833fe051729e42e4e8ff2ab6c34b
Author: Mridul Pathak <[email protected]>
AuthorDate: Sat Oct 3 20:12:03 2026 +0530
Improved: Remove the accept-any-certificate TrustManager from SSLUtil
(OFBIZ-13638)
SSLUtil could create an SSLContext that trusts any server certificate,
through TrustAnyManager and the trustAny parameter of getSSLContext and
getSSLSocketFactory. Nothing in the production code enabled it: it was only
reachable through HttpClient.setAllowUntrusted and
URLConnector.openUntrustedConnection, and the only callers were two tests that
call the local server over HTTPS with its self-signed certificate. HttpClient
also retried a request without certificate validation after a [...]
TrustAnyManager, getTrustAnyManagers, the trustAny parameters, the
openUntrustedConnection methods, setAllowUntrusted and the retry are removed,
so server certificates are always validated. HttpClient and URLConnector
instead accept an optional extra truststore, which the two tests use, through
the new HttpsTestSupport.trustLocalServer, to trust the certificate of the
local server and nothing else.
---
.../org/apache/ofbiz/base/util/HttpClient.java | 33 ++++------
.../java/org/apache/ofbiz/base/util/SSLUtil.java | 73 +++++++---------------
.../org/apache/ofbiz/base/util/URLConnector.java | 36 +++++------
.../ofbiz/ws/rs/test/RestTestHttpRequest.java | 11 +++-
.../apache/ofbiz/testtools/HttpsTestSupport.java | 53 ++++++++++++++++
.../ofbiz/widget/test/WidgetMacroLibraryTests.java | 11 +++-
6 files changed, 118 insertions(+), 99 deletions(-)
diff --git
a/framework/base/src/main/java/org/apache/ofbiz/base/util/HttpClient.java
b/framework/base/src/main/java/org/apache/ofbiz/base/util/HttpClient.java
index 5f3883ff9e..6c5f37d219 100644
--- a/framework/base/src/main/java/org/apache/ofbiz/base/util/HttpClient.java
+++ b/framework/base/src/main/java/org/apache/ofbiz/base/util/HttpClient.java
@@ -27,7 +27,7 @@ import java.net.HttpURLConnection;
import java.net.URL;
import java.net.URLConnection;
import java.nio.charset.StandardCharsets;
-import java.security.cert.CertificateException;
+import java.security.KeyStore;
import java.util.Base64;
import java.util.HashMap;
import java.util.List;
@@ -49,7 +49,7 @@ public class HttpClient {
private int timeout = 30000;
private boolean debug = false;
private boolean lineFeed = true;
- private boolean trustAny = false;
+ private KeyStore trustStore = null;
private boolean followRedirects = true;
private boolean keepAlive = false;
@@ -232,14 +232,17 @@ public class HttpClient {
return this.hostnameVerifier;
}
- /** Allow untrusted server certificates */
- public void setAllowUntrusted(boolean trustAny) {
- this.trustAny = trustAny;
+ /**
+ * Sets an extra truststore whose certificates are trusted, in addition to
the system and the configured truststores.
+ * Server certificates are always validated; {@code null} (the default)
means no extra certificates are trusted.
+ */
+ public void setTrustStore(KeyStore trustStore) {
+ this.trustStore = trustStore;
}
- /** Do we trust any certificate */
- public boolean getAllowUntrusted() {
- return this.trustAny;
+ /** Returns the extra truststore whose certificates are trusted, or {@code
null} for none */
+ public KeyStore getTrustStore() {
+ return this.trustStore;
}
/**
@@ -437,10 +440,6 @@ public class HttpClient {
}
private InputStream sendHttpRequestStream(String method) throws
HttpClientException {
- return sendHttpRequestStream(method, false);
- }
-
- private InputStream sendHttpRequestStream(String method, boolean
overrideTrust) throws HttpClientException {
// setup some SSL variables
SSLUtil.loadJsseProperties(this.debug);
@@ -470,11 +469,7 @@ public class HttpClient {
// Create the URL and open the connection.
try {
requestUrl = UtilURL.fromUrlString(localUrl);
- if (overrideTrust) {
- con = URLConnector.openUntrustedConnection(requestUrl,
timeout, clientCertAlias, hostnameVerifier);
- } else {
- con = URLConnector.openConnection(requestUrl, timeout,
clientCertAlias, hostnameVerifier);
- }
+ con = URLConnector.openConnection(requestUrl, timeout,
clientCertAlias, hostnameVerifier, trustStore);
if (Debug.verboseOn() || debug) {
Debug.logVerbose("Connection opened to : " +
requestUrl.toExternalForm(), MODULE);
}
@@ -556,10 +551,6 @@ public class HttpClient {
in = con.getInputStream();
} catch (IOException ioe) {
- if ((trustAny && !overrideTrust) && (ioe.getCause() instanceof
CertificateException)) {
- Debug.logWarning(ioe.getCause(), MODULE);
- return sendHttpRequestStream(method, true);
- }
if ((con instanceof HttpURLConnection)) {
try {
in = ((HttpURLConnection) con).getErrorStream();
diff --git
a/framework/base/src/main/java/org/apache/ofbiz/base/util/SSLUtil.java
b/framework/base/src/main/java/org/apache/ofbiz/base/util/SSLUtil.java
index dd5fd662d2..daa438f26f 100644
--- a/framework/base/src/main/java/org/apache/ofbiz/base/util/SSLUtil.java
+++ b/framework/base/src/main/java/org/apache/ofbiz/base/util/SSLUtil.java
@@ -58,32 +58,6 @@ public final class SSLUtil {
SSLUtil.loadJsseProperties();
}
- private static final class TrustAnyManager implements X509TrustManager {
-
- @Override
- public void checkClientTrusted(X509Certificate[] certs, String string)
throws CertificateException {
- Debug.logImportant("Trusting (un-trusted) client certificate
chain:", MODULE);
- for (X509Certificate cert: certs) {
- Debug.logImportant("---- " +
cert.getSubjectX500Principal().getName() + " valid: " + cert.getNotAfter(),
MODULE);
-
- }
- }
-
- @Override
- public void checkServerTrusted(X509Certificate[] certs, String string)
throws CertificateException {
- Debug.logImportant("Trusting (un-trusted) server certificate
chain:", MODULE);
- for (X509Certificate cert: certs) {
- Debug.logImportant("---- " +
cert.getSubjectX500Principal().getName() + " valid: " + cert.getNotAfter(),
MODULE);
- }
- }
-
- @Override
- public X509Certificate[] getAcceptedIssuers() {
- return new X509Certificate[0];
- }
- }
-
-
public static boolean isClientTrusted(X509Certificate[] chain, String
authType) {
TrustManager[] mgrs = new TrustManager[0];
try {
@@ -134,8 +108,14 @@ public final class SSLUtil {
}
private static TrustManager[] getTrustManagers() throws IOException,
GeneralSecurityException, GenericConfigException {
+ return getTrustManagersIncluding(null);
+ }
+
+ private static TrustManager[] getTrustManagersIncluding(KeyStore
additionalTrustStore)
+ throws IOException, GeneralSecurityException,
GenericConfigException {
MultiTrustManager tm = new MultiTrustManager();
tm.add(KeyStoreUtil.getSystemTrustStore());
+ tm.add(additionalTrustStore);
if (tm.getNumberOfKeyStores() < 1) {
Debug.logWarning("System truststore not found!", MODULE);
}
@@ -154,10 +134,6 @@ public final class SSLUtil {
return new TrustManager[] {tm };
}
- public static TrustManager[] getTrustAnyManagers() {
- return new TrustManager[] {new TrustAnyManager() };
- }
-
public static KeyManager[] getKeyManagers(KeyStore ks, String password,
String alias) throws GeneralSecurityException {
KeyManagerFactory factory = KeyManagerFactory.getInstance("SunX509");
factory.init(ks, password.toCharArray());
@@ -178,37 +154,34 @@ public final class SSLUtil {
public static SSLSocketFactory getSSLSocketFactory(KeyStore ks, String
password, String alias)
throws IOException, GeneralSecurityException,
GenericConfigException {
- return getSSLContext(ks, password, alias, false).getSocketFactory();
+ return getSSLContext(ks, password, alias).getSocketFactory();
}
- public static SSLContext getSSLContext(KeyStore ks, String password,
String alias, boolean trustAny)
+ public static SSLContext getSSLContext(KeyStore ks, String password,
String alias)
throws IOException, GeneralSecurityException,
GenericConfigException {
KeyManager[] km = SSLUtil.getKeyManagers(ks, password, alias);
- TrustManager[] tm;
- if (trustAny) {
- tm = SSLUtil.getTrustAnyManagers();
- } else {
- tm = SSLUtil.getTrustManagers();
- }
+ TrustManager[] tm = SSLUtil.getTrustManagers();
SSLContext context = SSLContext.getInstance("SSL");
context.init(km, tm, SECURE_RANDOM);
return context;
}
- public static SSLSocketFactory getSSLSocketFactory(String alias, boolean
trustAny)
+ /**
+ * Gets a socket factory that validates server certificates against the
system truststore and the configured
+ * component truststores, and additionally against the given truststore.
+ * @param alias the alias of the client certificate to use, or {@code null}
+ * @param additionalTrustStore an extra truststore to trust, or {@code
null} for none
+ */
+ public static SSLSocketFactory getSSLSocketFactory(String alias, KeyStore
additionalTrustStore)
throws IOException, GeneralSecurityException,
GenericConfigException {
- return getSSLContext(alias, trustAny).getSocketFactory();
+ return getSSLContext(alias, additionalTrustStore).getSocketFactory();
}
- public static SSLContext getSSLContext(String alias, boolean trustAny)
throws IOException, GeneralSecurityException, GenericConfigException {
+ public static SSLContext getSSLContext(String alias, KeyStore
additionalTrustStore)
+ throws IOException, GeneralSecurityException,
GenericConfigException {
KeyManager[] km = SSLUtil.getKeyManagers(alias);
- TrustManager[] tm;
- if (trustAny) {
- tm = SSLUtil.getTrustAnyManagers();
- } else {
- tm = SSLUtil.getTrustManagers();
- }
+ TrustManager[] tm =
SSLUtil.getTrustManagersIncluding(additionalTrustStore);
SSLContext context = SSLContext.getInstance("SSL");
context.init(km, tm, SECURE_RANDOM);
@@ -216,7 +189,7 @@ public final class SSLUtil {
}
private static SSLSocketFactory getSSLSocketFactory(String alias) throws
IOException, GeneralSecurityException, GenericConfigException {
- return getSSLSocketFactory(alias, false);
+ return getSSLSocketFactory(alias, null);
}
public static SSLSocketFactory getSSLSocketFactory() throws IOException,
GeneralSecurityException, GenericConfigException {
@@ -225,12 +198,12 @@ public final class SSLUtil {
public static SSLServerSocketFactory getSSLServerSocketFactory(KeyStore
ks, String password, String alias)
throws IOException, GeneralSecurityException,
GenericConfigException {
- return getSSLContext(ks, password, alias,
false).getServerSocketFactory();
+ return getSSLContext(ks, password, alias).getServerSocketFactory();
}
public static SSLServerSocketFactory getSSLServerSocketFactory(String
alias)
throws IOException, GeneralSecurityException,
GenericConfigException {
- return getSSLContext(alias, false).getServerSocketFactory();
+ return getSSLContext(alias, null).getServerSocketFactory();
}
public static void loadJsseProperties() {
diff --git
a/framework/base/src/main/java/org/apache/ofbiz/base/util/URLConnector.java
b/framework/base/src/main/java/org/apache/ofbiz/base/util/URLConnector.java
index 79035ee47f..dab315f783 100644
--- a/framework/base/src/main/java/org/apache/ofbiz/base/util/URLConnector.java
+++ b/framework/base/src/main/java/org/apache/ofbiz/base/util/URLConnector.java
@@ -23,6 +23,7 @@ import java.net.HttpURLConnection;
import java.net.URL;
import java.net.URLConnection;
import java.security.GeneralSecurityException;
+import java.security.KeyStore;
import javax.net.ssl.HostnameVerifier;
import javax.net.ssl.HttpsURLConnection;
@@ -41,14 +42,14 @@ public class URLConnector {
private URL url = null;
private String clientCertAlias = null;
private boolean timedOut = false;
- private boolean trustAnyCert = false;
+ private KeyStore trustStore = null;
private HostnameVerifier hostnameVerifier = null;
protected URLConnector() { }
- private URLConnector(URL url, String clientCertAlias, HostnameVerifier
hostnameVerifier, boolean trustAnyCert) {
+ private URLConnector(URL url, String clientCertAlias, HostnameVerifier
hostnameVerifier, KeyStore trustStore) {
this.clientCertAlias = clientCertAlias;
this.url = url;
- this.trustAnyCert = trustAnyCert;
+ this.trustStore = trustStore;
this.hostnameVerifier = hostnameVerifier;
}
@@ -99,26 +100,17 @@ public class URLConnector {
*/
public static URLConnection openConnection(URL url, int timeout, String
clientCertAlias, HostnameVerifier hostnameVerifier)
throws IOException {
- URLConnector uc = new URLConnector(url, clientCertAlias,
hostnameVerifier, false);
- return uc.openConnection(timeout);
- }
-
- // allow untrusted certs
- public static URLConnection openUntrustedConnection(URL url) throws
IOException {
- return openUntrustedConnection(url, 30000);
+ return openConnection(url, timeout, clientCertAlias, hostnameVerifier,
null);
}
- public static URLConnection openUntrustedConnection(URL url, int timeout)
throws IOException {
- return openUntrustedConnection(url, timeout, null, null);
- }
-
- public static URLConnection openUntrustedConnection(URL url, String
clientCertAlias) throws IOException {
- return openUntrustedConnection(url, 30000, clientCertAlias, null);
- }
-
- static URLConnection openUntrustedConnection(URL url, int timeout, String
clientCertAlias, HostnameVerifier hostnameVerifier)
- throws IOException {
- URLConnector uc = new URLConnector(url, clientCertAlias,
hostnameVerifier, true);
+ /**
+ * Open a connection, trusting only valid certificates and the
certificates of the given truststore.
+ * @param hostnameVerifier the host name verifier to use for HTTPS, or
{@code null} for the default verification
+ * @param trustStore an extra truststore whose certificates are trusted,
or {@code null} for none
+ */
+ public static URLConnection openConnection(URL url, int timeout, String
clientCertAlias, HostnameVerifier hostnameVerifier,
+ KeyStore trustStore) throws IOException {
+ URLConnector uc = new URLConnector(url, clientCertAlias,
hostnameVerifier, trustStore);
return uc.openConnection(timeout);
}
@@ -133,7 +125,7 @@ public class URLConnector {
if ("HTTPS".equalsIgnoreCase(url.getProtocol())) {
HttpsURLConnection scon = (HttpsURLConnection) con;
try {
-
scon.setSSLSocketFactory(SSLUtil.getSSLSocketFactory(clientCertAlias,
trustAnyCert));
+
scon.setSSLSocketFactory(SSLUtil.getSSLSocketFactory(clientCertAlias,
trustStore));
if (hostnameVerifier != null) {
scon.setHostnameVerifier(hostnameVerifier);
}
diff --git
a/framework/rest-api/src/test/java/org/apache/ofbiz/ws/rs/test/RestTestHttpRequest.java
b/framework/rest-api/src/test/java/org/apache/ofbiz/ws/rs/test/RestTestHttpRequest.java
index a776a30e3c..a0e6a23210 100644
---
a/framework/rest-api/src/test/java/org/apache/ofbiz/ws/rs/test/RestTestHttpRequest.java
+++
b/framework/rest-api/src/test/java/org/apache/ofbiz/ws/rs/test/RestTestHttpRequest.java
@@ -25,8 +25,10 @@ import static org.junit.jupiter.api.Assertions.assertTrue;
import static org.junit.jupiter.api.Assertions.fail;
import java.io.ByteArrayInputStream;
+import java.io.IOException;
import java.io.InputStream;
import java.nio.charset.StandardCharsets;
+import java.security.GeneralSecurityException;
import java.util.Base64;
import java.util.zip.GZIPInputStream;
import java.util.zip.InflaterInputStream;
@@ -36,6 +38,7 @@ import org.apache.ofbiz.base.start.Start;
import org.apache.ofbiz.base.util.Debug;
import org.apache.ofbiz.base.util.HttpClient;
import org.apache.ofbiz.base.util.HttpClientException;
+import org.apache.ofbiz.testtools.HttpsTestSupport;
import org.apache.ofbiz.testtools.JunitJupiterTest;
import org.apache.ofbiz.testtools.JupiterTestHelper;
import org.junit.jupiter.api.BeforeAll;
@@ -64,9 +67,11 @@ class RestTestHttpRequest implements JupiterTestHelper {
private static HttpClient initHttpClient() {
HttpClient http = new HttpClient();
http.followRedirects(true);
- http.setAllowUntrusted(true);
- // the test certificate is not issued for localhost
- http.setHostnameVerifier((hostname, session) ->
"localhost".equals(hostname));
+ try {
+ HttpsTestSupport.trustLocalServer(http);
+ } catch (IOException | GeneralSecurityException e) {
+ throw new IllegalStateException("Cannot trust the certificate of
the local server", e);
+ }
return http;
}
diff --git
a/framework/testtools/src/main/java/org/apache/ofbiz/testtools/HttpsTestSupport.java
b/framework/testtools/src/main/java/org/apache/ofbiz/testtools/HttpsTestSupport.java
new file mode 100644
index 0000000000..fa6d2c6627
--- /dev/null
+++
b/framework/testtools/src/main/java/org/apache/ofbiz/testtools/HttpsTestSupport.java
@@ -0,0 +1,53 @@
+/*******************************************************************************
+ * 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.ofbiz.testtools;
+
+import java.io.File;
+import java.io.IOException;
+import java.security.GeneralSecurityException;
+import java.security.KeyStore;
+
+import org.apache.ofbiz.base.util.HttpClient;
+import org.apache.ofbiz.base.util.KeyStoreUtil;
+
+/**
+ * Helper for tests that call the HTTPS port of the OFBiz instance they run in.
+ */
+public final class HttpsTestSupport {
+
+ private static final String SERVER_KEYSTORE =
"framework/base/config/ofbizssl.jks";
+ private static final String SERVER_KEYSTORE_PASSWORD = "changeit";
+
+ private HttpsTestSupport() { }
+
+ /**
+ * Makes the client accept the certificate of the local OFBiz server,
which is self-signed and not issued
+ * for localhost, and nothing else: the server certificate is still
validated, against the keystore the
+ * server uses, and only the host name localhost is accepted.
+ * @param client the client to set up
+ * @throws IOException if the server keystore cannot be read
+ * @throws GeneralSecurityException if the server keystore cannot be loaded
+ */
+ public static void trustLocalServer(HttpClient client) throws IOException,
GeneralSecurityException {
+ File keystoreFile = new File(System.getProperty("ofbiz.home"),
SERVER_KEYSTORE);
+ KeyStore serverKeyStore =
KeyStoreUtil.getStore(keystoreFile.toURI().toURL(), SERVER_KEYSTORE_PASSWORD);
+ client.setTrustStore(serverKeyStore);
+ client.setHostnameVerifier((hostname, session) ->
"localhost".equals(hostname));
+ }
+}
diff --git
a/framework/widget/src/test/java/org/apache/ofbiz/widget/test/WidgetMacroLibraryTests.java
b/framework/widget/src/test/java/org/apache/ofbiz/widget/test/WidgetMacroLibraryTests.java
index 253447b8a5..2ee029742a 100644
---
a/framework/widget/src/test/java/org/apache/ofbiz/widget/test/WidgetMacroLibraryTests.java
+++
b/framework/widget/src/test/java/org/apache/ofbiz/widget/test/WidgetMacroLibraryTests.java
@@ -23,11 +23,14 @@ import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.api.Assertions.assertNotNull;
+import java.io.IOException;
import java.io.InputStream;
+import java.security.GeneralSecurityException;
import org.apache.ofbiz.base.start.Start;
import org.apache.ofbiz.base.util.HttpClient;
import org.apache.ofbiz.base.util.HttpClientException;
+import org.apache.ofbiz.testtools.HttpsTestSupport;
import org.apache.ofbiz.testtools.JunitJupiterTest;
import org.apache.ofbiz.testtools.JupiterTestHelper;
import org.apache.tika.metadata.Metadata;
@@ -50,9 +53,11 @@ public class WidgetMacroLibraryTests implements
JupiterTestHelper {
public HttpClient initHttpClient() throws HttpClientException {
HttpClient http = new HttpClient();
http.followRedirects(true);
- http.setAllowUntrusted(true);
- // the test certificate is not issued for localhost
- http.setHostnameVerifier((hostname, session) ->
"localhost".equals(hostname));
+ try {
+ HttpsTestSupport.trustLocalServer(http);
+ } catch (IOException | GeneralSecurityException e) {
+ throw new HttpClientException("Cannot trust the certificate of the
local server", e);
+ }
return http;
}
