You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@hc.apache.org by gg...@apache.org on 2024/09/22 15:08:20 UTC
(httpcomponents-client) branch master updated: HTTPCLIENT-2337: Sanitize X500Principal Logging in ClientTlsStrategy classes (#581)
This is an automated email from the ASF dual-hosted git repository.
ggregory pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/httpcomponents-client.git
The following commit(s) were added to refs/heads/master by this push:
new e9560a4ae HTTPCLIENT-2337: Sanitize X500Principal Logging in ClientTlsStrategy classes (#581)
e9560a4ae is described below
commit e9560a4ae6518a042d460594e61c0220aee72fb3
Author: Arturo Bernal <ab...@apache.org>
AuthorDate: Sun Sep 22 17:08:15 2024 +0200
HTTPCLIENT-2337: Sanitize X500Principal Logging in ClientTlsStrategy classes (#581)
* HTTPCLIENT-2337: Add sanitizeX500Principal method to escape control characters in X500Principal. Escapes ISO control characters in X500Principal using hexadecimal representation.
* Remove "Escaped" from debug log message
* Use a single call to append() for each character in toEscapedString()
---------
Co-authored-by: Gary Gregory <ga...@users.noreply.github.com>
---
.../http/ssl/AbstractClientTlsStrategy.java | 34 +++++-
.../http/ssl/AbstractClientTlsStrategyTest.java | 114 +++++++++++++++++++++
2 files changed, 146 insertions(+), 2 deletions(-)
diff --git a/httpclient5/src/main/java/org/apache/hc/client5/http/ssl/AbstractClientTlsStrategy.java b/httpclient5/src/main/java/org/apache/hc/client5/http/ssl/AbstractClientTlsStrategy.java
index c79d8394d..c18a144e4 100644
--- a/httpclient5/src/main/java/org/apache/hc/client5/http/ssl/AbstractClientTlsStrategy.java
+++ b/httpclient5/src/main/java/org/apache/hc/client5/http/ssl/AbstractClientTlsStrategy.java
@@ -51,6 +51,7 @@ import javax.security.auth.x500.X500Principal;
import org.apache.hc.client5.http.config.TlsConfig;
import org.apache.hc.core5.annotation.Contract;
+import org.apache.hc.core5.annotation.Internal;
import org.apache.hc.core5.annotation.ThreadingBehavior;
import org.apache.hc.core5.concurrent.FutureCallback;
import org.apache.hc.core5.http.HttpHost;
@@ -271,7 +272,7 @@ abstract class AbstractClientTlsStrategy implements TlsStrategy, TlsSocketStrate
final X509Certificate x509 = (X509Certificate) cert;
final X500Principal peer = x509.getSubjectX500Principal();
- LOG.debug(" peer principal: {}", peer);
+ LOG.debug("Peer principal: {}", toEscapedString(peer));
final Collection<List<?>> altNames1 = x509.getSubjectAlternativeNames();
if (altNames1 != null) {
final List<String> altNames = new ArrayList<>();
@@ -284,7 +285,7 @@ abstract class AbstractClientTlsStrategy implements TlsStrategy, TlsSocketStrate
}
final X500Principal issuer = x509.getIssuerX500Principal();
- LOG.debug(" issuer principal: {}", issuer);
+ LOG.debug("Issuer principal: {}", toEscapedString(issuer));
final Collection<List<?>> altNames2 = x509.getIssuerAlternativeNames();
if (altNames2 != null) {
final List<String> altNames = new ArrayList<>();
@@ -322,4 +323,33 @@ abstract class AbstractClientTlsStrategy implements TlsStrategy, TlsSocketStrate
}
}
+ /**
+ * Converts an X500Principal to a cleaned string by escaping control characters.
+ * <p>
+ * This method processes the RFC2253 format of the X500Principal and escapes
+ * any ISO control characters to avoid issues in logging or other outputs.
+ * Control characters are replaced with their escaped hexadecimal representation.
+ * </p>
+ *
+ * <p><strong>Note:</strong> For testing purposes, this method is package-private
+ * to allow access within the same package. This allows tests to verify the correct
+ * behavior of the escaping process.</p>
+ *
+ * @param principal the X500Principal to escape
+ * @return the escaped string representation of the X500Principal
+ */
+ @Internal
+ String toEscapedString(final X500Principal principal) {
+ final String principalValue = principal.getName(X500Principal.RFC2253);
+ final StringBuilder sanitizedPrincipal = new StringBuilder(principalValue.length());
+ for (final char c : principalValue.toCharArray()) {
+ if (Character.isISOControl(c)) {
+ sanitizedPrincipal.append(String.format("\\x%02x", (int) c));
+ } else {
+ sanitizedPrincipal.append(c);
+ }
+ }
+ return sanitizedPrincipal.toString();
+ }
+
}
diff --git a/httpclient5/src/test/java/org/apache/hc/client5/http/ssl/AbstractClientTlsStrategyTest.java b/httpclient5/src/test/java/org/apache/hc/client5/http/ssl/AbstractClientTlsStrategyTest.java
new file mode 100644
index 000000000..b121feaa7
--- /dev/null
+++ b/httpclient5/src/test/java/org/apache/hc/client5/http/ssl/AbstractClientTlsStrategyTest.java
@@ -0,0 +1,114 @@
+/*
+ * ====================================================================
+ * 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.
+ * ====================================================================
+ *
+ * This software consists of voluntary contributions made by many
+ * individuals on behalf of the Apache Software Foundation. For more
+ * information on the Apache Software Foundation, please see
+ * <http://www.apache.org/>.
+ *
+ */
+
+package org.apache.hc.client5.http.ssl;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
+
+import java.security.cert.X509Certificate;
+
+import javax.net.ssl.SSLEngine;
+import javax.net.ssl.SSLParameters;
+import javax.net.ssl.SSLSession;
+import javax.security.auth.x500.X500Principal;
+
+import org.apache.hc.core5.reactor.ssl.SSLBufferMode;
+import org.apache.hc.core5.reactor.ssl.TlsDetails;
+import org.apache.hc.core5.ssl.SSLContexts;
+import org.junit.jupiter.api.Test;
+
+public class AbstractClientTlsStrategyTest {
+
+ @Test
+ public void testToEscapedString_withControlCharacters() {
+ // Create a X500Principal with control characters
+ final X500Principal principal = new X500Principal("CN=Test\b\bName\n,O=TestOrg");
+
+ // Create a mock subclass of AbstractClientTlsStrategy
+ final AbstractClientTlsStrategy tlsStrategy = new AbstractClientTlsStrategy(
+ SSLContexts.createDefault(),
+ null, null, SSLBufferMode.STATIC,
+ HostnameVerificationPolicy.BUILTIN,
+ HttpsSupport.getDefaultHostnameVerifier()) {
+ @Override
+ void applyParameters(final SSLEngine sslEngine, final SSLParameters sslParameters, final String[] appProtocols) {
+ // No-op for test
+ }
+
+ @Override
+ TlsDetails createTlsDetails(final SSLEngine sslEngine) {
+ return null; // No-op for test
+ }
+ };
+
+ // Call the toEscapedString method
+ final String escaped = tlsStrategy.toEscapedString(principal);
+
+ // Assert that control characters are properly escaped
+ assertEquals("CN=Test\\x08\\x08Name\\x0a,O=TestOrg", escaped);
+ }
+
+ @Test
+ public void testVerifySession_escapedPeerAndIssuer() throws Exception {
+ // Mock SSLSession and X509Certificate
+ final SSLSession mockSession = mock(SSLSession.class);
+ final X509Certificate mockCert = mock(X509Certificate.class);
+
+ // Create a mock X500Principal with control characters
+ final X500Principal peerPrincipal = new X500Principal("CN=Peer\bName,O=PeerOrg");
+ final X500Principal issuerPrincipal = new X500Principal("CN=Issuer\bName,O=IssuerOrg");
+
+ when(mockSession.getPeerCertificates()).thenReturn(new X509Certificate[]{mockCert});
+ when(mockCert.getSubjectX500Principal()).thenReturn(peerPrincipal);
+ when(mockCert.getIssuerX500Principal()).thenReturn(issuerPrincipal);
+
+ // Create a mock subclass of AbstractClientTlsStrategy
+ final AbstractClientTlsStrategy tlsStrategy = new AbstractClientTlsStrategy(
+ SSLContexts.createDefault(),
+ null, null, SSLBufferMode.STATIC,
+ HostnameVerificationPolicy.BUILTIN,
+ HttpsSupport.getDefaultHostnameVerifier()) {
+ @Override
+ void applyParameters(final SSLEngine sslEngine, final SSLParameters sslParameters, final String[] appProtocols) {
+ // No-op for test
+ }
+
+ @Override
+ TlsDetails createTlsDetails(final SSLEngine sslEngine) {
+ return null; // No-op for test
+ }
+ };
+
+ // Test the verifySession method
+ tlsStrategy.verifySession("localhost", mockSession, null);
+
+ }
+
+
+}
\ No newline at end of file