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 <garydgregory@users.noreply.github.com>

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();
+    }
+
 }

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);
+
+    }
+
+
+}