omkreddy commented on code in PR #14130:
URL: https://github.com/apache/kafka/pull/14130#discussion_r1282124322
##########
clients/src/main/java/org/apache/kafka/common/security/ssl/DefaultSslEngineFactory.java:
##########
@@ -16,47 +16,48 @@
*/
package org.apache.kafka.common.security.ssl;
-import org.apache.kafka.common.KafkaException;
Review Comment:
Can we avoid changing import order and revert all unwanted code formating
changes in all the files.
##########
clients/src/main/java/org/apache/kafka/common/security/ssl/DefaultSslEngineFactory.java:
##########
@@ -578,4 +594,335 @@ private List<byte[]> pemEntries(String pem) {
return entries;
}
}
+
+ /**
+ * A wrapper around the original trust manager factory for creating common
name logging trust managers.
+ * These trust managers log the common name of an expired but otherwise
valid (client) certificate before rejecting the connection attempt.
+ * This allows to identify misconfigured clients in complex network
environments, where the IP address is not sufficient.
+ */
+ static class CommonNameLoggingTrustManagerFactoryWrapper {
Review Comment:
Can we move this these classes to new file.
##########
clients/src/main/java/org/apache/kafka/common/security/ssl/DefaultSslEngineFactory.java:
##########
@@ -255,7 +271,7 @@ private SSLContext createSSLContext(SecurityStore keystore,
SecurityStore trusts
}
String tmfAlgorithm = this.tmfAlgorithm != null ?
this.tmfAlgorithm : TrustManagerFactory.getDefaultAlgorithm();
- TrustManagerFactory tmf =
TrustManagerFactory.getInstance(tmfAlgorithm);
+ CommonNameLoggingTrustManagerFactoryWrapper tmf =
CommonNameLoggingTrustManagerFactoryWrapper.getInstance(tmfAlgorithm);
Review Comment:
I think we should add a config to enable this logging.
cc @rajinisivaram @ijuma
##########
clients/src/main/java/org/apache/kafka/common/security/ssl/DefaultSslEngineFactory.java:
##########
@@ -578,4 +594,335 @@ private List<byte[]> pemEntries(String pem) {
return entries;
}
}
+
+ /**
+ * A wrapper around the original trust manager factory for creating common
name logging trust managers.
+ * These trust managers log the common name of an expired but otherwise
valid (client) certificate before rejecting the connection attempt.
+ * This allows to identify misconfigured clients in complex network
environments, where the IP address is not sufficient.
+ */
+ static class CommonNameLoggingTrustManagerFactoryWrapper {
+
+ private TrustManagerFactory origTmf;
+
+ /**
+ * Create a wrapped trust manager factory
+ * @param kmfAlgorithm the algorithm
+ * @return A wrapped trust manager factory
+ * @throws NoSuchAlgorithmException
+ */
+ protected CommonNameLoggingTrustManagerFactoryWrapper(String
kmfAlgorithm) throws NoSuchAlgorithmException {
+ this.origTmf = TrustManagerFactory.getInstance(kmfAlgorithm);
+ }
+ /**
+ * Factory for creating a wrapped trust manager factory
+ * @param kmfAlgorithm the algorithm
+ * @return A wrapped trust manager factory
+ * @throws NoSuchAlgorithmException
+ */
+ public static CommonNameLoggingTrustManagerFactoryWrapper
getInstance(String kmfAlgorithm) throws NoSuchAlgorithmException {
+ return new
CommonNameLoggingTrustManagerFactoryWrapper(kmfAlgorithm);
+ }
+
+ public TrustManagerFactory getOriginalTrustManagerFactory() {
+ return this.origTmf;
+ }
+
+ public String getAlgorithm() {
+ return this.origTmf.getAlgorithm();
+ }
+
+ public void init(KeyStore ts) throws KeyStoreException {
+ this.origTmf.init(ts);
+ }
+
+ public TrustManager[] getTrustManagers() {
+ TrustManager[] origTrustManagers = this.origTmf.getTrustManagers();
+ TrustManager[] wrappedTrustManagers = new
TrustManager[origTrustManagers.length];
+ for (int i = 0; i < origTrustManagers.length; i++) {
+ TrustManager tm = origTrustManagers[i];
+ if (tm instanceof X509TrustManager) {
+ // Wrap only X509 trust managers
+ wrappedTrustManagers[i] = new
CommonNameLoggingTrustManager((X509TrustManager) tm);
+ } else {
+ wrappedTrustManagers[i] = tm;
+ }
+ }
+ return wrappedTrustManagers;
+ }
+ }
+
+ /**
+ * A trust manager which logs the common name of an expired but otherwise
valid (client) certificate before rejecting the connection attempt.
+ * This allows to identify misconfigured clients in complex network
environments, where the IP address is not sufficient.
+ * this class wraps a standard trust manager and delegates almost all
requests to it, except for cases where an invalid certificate is reported by the
+ * standard trust manager. In this cases this manager checks whether the
provided certificate is invalid only due to being expired and logs the common
+ * name if that is the case. This trust manager will always return the
results of the wrapped standard trust manager, i.e. return if the certificate
is valid
+ * or rethrow the original exception if it is not.
+ */
+ static class CommonNameLoggingTrustManager implements X509TrustManager {
+
+ private X509TrustManager origTm;
+
+ public CommonNameLoggingTrustManager(X509TrustManager
originalTrustManager) {
+ this.origTm = originalTrustManager;
+ }
+
+ public X509TrustManager getOriginalTrustManager() {
+ return this.origTm;
+ }
+
+ @Override
+ public void checkClientTrusted(X509Certificate[] chain, String
authType)
+ throws CertificateException {
+ CertificateException origException = null;
+ try {
+ this.origTm.checkClientTrusted(chain, authType);
+ // If the last line did not throw, the chain is valid
(including that none of the certificates is expired)
+ } catch (CertificateException e) {
+ origException = e;
+ try {
+ X509Certificate[] wrappedChain =
sortChainAnWrapEndCertificate(chain);
+ this.origTm.checkClientTrusted(wrappedChain, authType);
+ // No exception occurred this time. The certificate is
invalid due to being expired, but otherwise valid.
+ String commonName =
wrappedChain[0].getSubjectX500Principal().toString();
+ String notValidAfter =
wrappedChain[0].getNotAfter().toString();
+ log.info("Certificate with common name \"" + commonName +
"\" expired on " + notValidAfter);
Review Comment:
`log.info` => `log.warn`
Certificate => `Client Certificate`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
