Skip to content

Commit

Permalink
HTTPCLIENT-2337: Sanitize X500Principal Logging in ClientTlsStrategy …
Browse files Browse the repository at this point in the history
…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 <[email protected]>
  • Loading branch information
arturobernalg and garydgregory authored Sep 22, 2024
1 parent aad0e9a commit e9560a4
Show file tree
Hide file tree
Showing 2 changed files with 146 additions and 2 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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<List<?>> altNames1 = x509.getSubjectAlternativeNames();
if (altNames1 != null) {
final List<String> altNames = new ArrayList<>();
Expand All @@ -284,7 +285,7 @@ void verifySession(
}

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<>();
Expand Down Expand Up @@ -322,4 +323,33 @@ void verifySession(
}
}

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

}
Original file line number Diff line number Diff line change
@@ -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);

}


}

0 comments on commit e9560a4

Please sign in to comment.