From 7d695c5da3d3411672a3fb339e506da52be690a2 Mon Sep 17 00:00:00 2001 From: Arturo Bernal Date: Fri, 20 Sep 2024 13:48:25 +0200 Subject: [PATCH] HTTPCLIENT-2337: Add sanitizeX500Principal method to escape control characters in X500Principal. Escapes ISO control characters in X500Principal using hexadecimal representation. --- .../http/ssl/AbstractClientTlsStrategy.java | 34 +++++- .../ssl/AbstractClientTlsStrategyTest.java | 114 ++++++++++++++++++ 2 files changed, 146 insertions(+), 2 deletions(-) create mode 100644 httpclient5/src/test/java/org/apache/hc/client5/http/ssl/AbstractClientTlsStrategyTest.java 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..cb0a335bc 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 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 @@ void verifySession( final X509Certificate x509 = (X509Certificate) cert; final X500Principal peer = x509.getSubjectX500Principal(); - LOG.debug(" peer principal: {}", peer); + LOG.debug("Peer principal: {}", toEscapedString(peer)); final Collection> altNames1 = x509.getSubjectAlternativeNames(); if (altNames1 != null) { final List altNames = new ArrayList<>(); @@ -284,7 +285,7 @@ void verifySession( } final X500Principal issuer = x509.getIssuerX500Principal(); - LOG.debug(" issuer principal: {}", issuer); + LOG.debug("Escaped issuer principal: {}", toEscapedString(issuer)); final Collection> altNames2 = x509.getIssuerAlternativeNames(); if (altNames2 != null) { final List altNames = new ArrayList<>(); @@ -322,4 +323,33 @@ void verifySession( } } + /** + * Converts an X500Principal to a cleaned string by escaping control characters. + *

+ * 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. + *

+ * + *

Note: 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.

+ * + * @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("\\x").append(String.format("%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 + * . + * + */ + +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