-
Notifications
You must be signed in to change notification settings - Fork 71
fix: Log ignored certificates during client authentication #4415
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: v3.x.x
Are you sure you want to change the base?
Changes from 11 commits
970bf43
9d14b4c
cf8e372
9258c5e
5dd882c
c8bda34
7ee33f4
b006e54
d7cfaa8
6d5b5d0
6e5656f
7571743
b69e310
1fb9a6d
7963cad
21da760
a1a1254
1b7bca2
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -25,6 +25,7 @@ | |
| import org.springframework.web.filter.OncePerRequestFilter; | ||
| import org.zowe.apiml.message.log.ApimlLogger; | ||
| import org.zowe.apiml.product.logging.annotations.InjectApimlLogger; | ||
| import org.zowe.apiml.security.common.util.CertificateLoggingUtils; | ||
| import org.zowe.apiml.security.common.verify.CertificateValidator; | ||
|
|
||
| import java.io.ByteArrayInputStream; | ||
|
|
@@ -57,6 +58,23 @@ public class CategorizeCertsFilter extends OncePerRequestFilter { | |
|
|
||
| private final CertificateValidator certificateValidator; | ||
|
|
||
| /** | ||
| * Logs information about certificates that were ignored during authentication. | ||
| * Delegates to {@link CertificateLoggingUtils} for the actual logging implementation. | ||
| * | ||
| * @param originalCerts The original array of certificates before filtering | ||
| * @param filteredCerts The array of certificates after filtering for authentication | ||
| */ | ||
| private void logIgnoredCertificates(X509Certificate[] originalCerts, X509Certificate[] filteredCerts) { | ||
| CertificateLoggingUtils.logIgnoredCertificates( | ||
| originalCerts, | ||
| filteredCerts, | ||
| publicKeyCertificatesBase64, | ||
| log, | ||
| CategorizeCertsFilter::base64EncodePublicKey | ||
|
||
| ); | ||
| } | ||
|
|
||
| /** | ||
| * Get certificates from request (if exists), separate them (to use only APIML certificate to request sign and | ||
| * other for authentication) and store again into request. | ||
|
|
@@ -78,7 +96,12 @@ private void categorizeCerts(ServletRequest request) { | |
| // add the client certificate to the certs array | ||
| String subjectDN = ((X509Certificate) clientCert.get()).getSubjectX500Principal().getName(); | ||
| log.debug("Found client certificate in header, adding it to the request. Subject DN: {}", subjectDN); | ||
| httpServletRequest.setAttribute(ATTR_NAME_CLIENT_AUTH_X509_CERTIFICATE, selectCerts(new X509Certificate[]{(X509Certificate) clientCert.get()}, certificateForClientAuth)); | ||
|
|
||
| X509Certificate[] headerCerts = new X509Certificate[]{(X509Certificate) clientCert.get()}; | ||
| X509Certificate[] clientAuthCerts = selectCerts(headerCerts, certificateForClientAuth); | ||
| logIgnoredCertificates(headerCerts, clientAuthCerts); | ||
|
|
||
| httpServletRequest.setAttribute(ATTR_NAME_CLIENT_AUTH_X509_CERTIFICATE, clientAuthCerts); | ||
| return; | ||
| } else if (isClientCertificateIgnored(httpServletRequest)) { | ||
| log.debug("Client certificate is ignored."); | ||
|
|
@@ -88,7 +111,10 @@ private void categorizeCerts(ServletRequest request) { | |
| } | ||
| } | ||
|
|
||
| httpServletRequest.setAttribute(ATTR_NAME_CLIENT_AUTH_X509_CERTIFICATE, selectCerts(certs, certificateForClientAuth)); | ||
| X509Certificate[] clientAuthCerts = selectCerts(certs, certificateForClientAuth); | ||
| logIgnoredCertificates(certs, clientAuthCerts); | ||
|
|
||
| httpServletRequest.setAttribute(ATTR_NAME_CLIENT_AUTH_X509_CERTIFICATE, clientAuthCerts); | ||
| httpServletRequest.setAttribute(ATTR_NAME_JAKARTA_SERVLET_REQUEST_X509_CERTIFICATE, selectCerts(certs, apimlCertificate)); | ||
|
|
||
| log.debug(LOG_FORMAT_FILTERING_CERTIFICATES, ATTR_NAME_CLIENT_AUTH_X509_CERTIFICATE, httpServletRequest.getAttribute(ATTR_NAME_CLIENT_AUTH_X509_CERTIFICATE)); | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,143 @@ | ||
| /* | ||
| * This program and the accompanying materials are made available under the terms of the | ||
| * Eclipse Public License v2.0 which accompanies this distribution, and is available at | ||
| * https://www.eclipse.org/legal/epl-v20.html | ||
| * | ||
| * SPDX-License-Identifier: EPL-2.0 | ||
| * | ||
| * Copyright Contributors to the Zowe Project. | ||
| */ | ||
|
|
||
| package org.zowe.apiml.security.common.util; | ||
|
|
||
| import lombok.experimental.UtilityClass; | ||
| import org.slf4j.Logger; | ||
|
|
||
| import java.security.cert.X509Certificate; | ||
| import java.util.*; | ||
| import java.util.function.Function; | ||
| import java.util.stream.Collectors; | ||
|
|
||
| /** | ||
| * Utility class for logging certificate-related operations, particularly for logging | ||
| * certificates that were ignored/filtered during client authentication. | ||
| */ | ||
| @UtilityClass | ||
|
||
| public class CertificateLoggingUtils { | ||
|
|
||
| private static final String UNKNOWN = "Unknown"; | ||
|
|
||
| /** | ||
| * Logs information about certificates that were ignored during authentication. | ||
| * Compares the original set of certificates with the filtered set to identify ignored certificates. | ||
| * Uses Base64-encoded public keys for reliable comparison instead of X509Certificate object equality. | ||
| * | ||
| * @param originalCerts The original array of certificates before filtering | ||
| * @param filteredCerts The array of certificates after filtering for authentication | ||
| * @param publicKeyCertificatesBase64 Set of Base64-encoded public keys of known APIML certificates | ||
| * @param logger The logger to use for output | ||
| * @param base64Encoder Function to encode certificate public key to Base64 | ||
| */ | ||
| public static void logIgnoredCertificates( | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Try to refactor a bit this method, it's too long, you can extract smaller blocks into private methods
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done, Thank You for pointing that out. |
||
| X509Certificate[] originalCerts, | ||
| X509Certificate[] filteredCerts, | ||
| Set<String> publicKeyCertificatesBase64, | ||
| Logger logger, | ||
| Function<X509Certificate, String> base64Encoder | ||
| ) { | ||
| if (originalCerts == null || originalCerts.length == 0) return; | ||
|
|
||
| List<X509Certificate> ignoredCerts = identifyIgnoredCertificates( | ||
| originalCerts, filteredCerts, base64Encoder | ||
| ); | ||
|
|
||
| if (!ignoredCerts.isEmpty()) { | ||
| logCertificateSummary(ignoredCerts, base64Encoder, logger); | ||
| logCertificateDetails(ignoredCerts, publicKeyCertificatesBase64, base64Encoder, logger); | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * Identifies certificates that were ignored by comparing original and filtered certificate arrays. | ||
| */ | ||
| private static List<X509Certificate> identifyIgnoredCertificates( | ||
| X509Certificate[] originalCerts, | ||
| X509Certificate[] filteredCerts, | ||
| Function<X509Certificate, String> base64Encoder | ||
| ) { | ||
| Set<String> originalKeys = Arrays.stream(originalCerts) | ||
| .map(base64Encoder) | ||
| .collect(Collectors.toSet()); | ||
|
|
||
| Set<String> filteredKeys = filteredCerts != null | ||
| ? Arrays.stream(filteredCerts) | ||
| .map(base64Encoder) | ||
| .collect(Collectors.toSet()) | ||
| : new HashSet<>(); | ||
|
|
||
| Set<String> ignoredKeys = new HashSet<>(originalKeys); | ||
| ignoredKeys.removeAll(filteredKeys); | ||
|
Comment on lines
76
to
87
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This can be one method
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done, Thank You for pointing that out. |
||
|
|
||
| return Arrays.stream(originalCerts) | ||
| .filter(cert -> ignoredKeys.contains(base64Encoder.apply(cert))) | ||
| .toList(); | ||
| } | ||
|
|
||
| /** | ||
| * Logs a summary of all ignored certificates with their key details. | ||
| */ | ||
| private static void logCertificateSummary( | ||
| List<X509Certificate> ignoredCerts, | ||
| Function<X509Certificate, String> base64Encoder, | ||
| Logger logger | ||
| ) { | ||
| logger.debug("Certificates ignored/not used for authentication: {}", | ||
| ignoredCerts.stream() | ||
| .map(cert -> formatCertificateInfo(cert, base64Encoder)) | ||
| .collect(Collectors.joining(", "))); | ||
| } | ||
|
|
||
| /** | ||
| * Formats certificate information for logging. | ||
| */ | ||
| private static String formatCertificateInfo( | ||
| X509Certificate cert, | ||
| Function<X509Certificate, String> base64Encoder | ||
| ) { | ||
| String subjectDN = cert.getSubjectX500Principal() != null | ||
| ? cert.getSubjectX500Principal().getName() | ||
| : UNKNOWN; | ||
| String issuerDN = cert.getIssuerX500Principal() != null | ||
| ? cert.getIssuerX500Principal().getName() | ||
| : UNKNOWN; | ||
| String publicKeyBase64 = base64Encoder.apply(cert); | ||
| return String.format("[Subject: %s, Issuer: %s, Public Key (first 20 chars): %s...]", | ||
| subjectDN, issuerDN, publicKeyBase64.substring(0, Math.min(20, publicKeyBase64.length()))); | ||
| } | ||
|
|
||
| /** | ||
| * Logs detailed information about each ignored certificate including the reason for ignoring. | ||
| */ | ||
| private static void logCertificateDetails( | ||
| List<X509Certificate> ignoredCerts, | ||
| Set<String> publicKeyCertificatesBase64, | ||
| Function<X509Certificate, String> base64Encoder, | ||
| Logger logger | ||
| ) { | ||
| ignoredCerts.forEach(cert -> { | ||
| String publicKeyBase64 = base64Encoder.apply(cert); | ||
| boolean isApimlCert = publicKeyCertificatesBase64.contains(publicKeyBase64); | ||
| String subjectDN = cert.getSubjectX500Principal() != null | ||
| ? cert.getSubjectX500Principal().getName() | ||
| : UNKNOWN; | ||
| if (isApimlCert) { | ||
| logger.debug("Certificate with subject '{}' was ignored because it is an APIML Gateway certificate (not used for client authentication)", | ||
| subjectDN); | ||
| } else { | ||
| logger.debug("Certificate with subject '{}' was ignored for unknown reason (not in APIML cert set, but filtered by predicate)", | ||
| subjectDN); | ||
| } | ||
| }); | ||
| } | ||
| } | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -21,6 +21,7 @@ | |
| import org.springframework.web.server.WebFilterChain; | ||
| import org.zowe.apiml.message.log.ApimlLogger; | ||
| import org.zowe.apiml.product.logging.annotations.InjectApimlLogger; | ||
| import org.zowe.apiml.security.common.util.CertificateLoggingUtils; | ||
| import org.zowe.apiml.security.common.verify.CertificateValidator; | ||
| import reactor.core.publisher.Mono; | ||
|
|
||
|
|
@@ -97,6 +98,9 @@ private ServerWebExchange categorizeCerts(ServerWebExchange exchange) { | |
| new X509Certificate[]{clientCertFromHeader.get()}, | ||
| certificateForClientAuth | ||
| ); | ||
|
|
||
| logIgnoredCertificates(new X509Certificate[]{clientCertFromHeader.get()}, clientAuthCerts); | ||
|
|
||
| exchange.getAttributes().put(ATTR_NAME_CLIENT_AUTH_X509_CERTIFICATE, clientAuthCerts); | ||
| log.debug(LOG_FORMAT_FILTERING_CERTIFICATES, ATTR_NAME_CLIENT_AUTH_X509_CERTIFICATE, Arrays.toString(clientAuthCerts)); | ||
|
|
||
|
|
@@ -108,6 +112,9 @@ private ServerWebExchange categorizeCerts(ServerWebExchange exchange) { | |
|
|
||
| } else { | ||
| X509Certificate[] clientAuthCerts = selectCerts(certsFromTls, certificateForClientAuth); | ||
|
|
||
| logIgnoredCertificates(certsFromTls, clientAuthCerts); | ||
|
|
||
| exchange.getAttributes().put(ATTR_NAME_CLIENT_AUTH_X509_CERTIFICATE, clientAuthCerts); | ||
| log.debug(LOG_FORMAT_FILTERING_CERTIFICATES, ATTR_NAME_CLIENT_AUTH_X509_CERTIFICATE, Arrays.toString(clientAuthCerts)); | ||
|
|
||
|
|
@@ -127,6 +134,23 @@ private ServerWebExchange categorizeCerts(ServerWebExchange exchange) { | |
| return exchange.mutate().request(requestBuilder.build()).build(); | ||
| } | ||
|
|
||
| /** | ||
| * Logs information about certificates that were ignored during authentication. | ||
| * Delegates to {@link CertificateLoggingUtils} for the actual logging implementation. | ||
| * | ||
| * @param originalCerts The original array of certificates before filtering | ||
| * @param filteredCerts The array of certificates after filtering for authentication | ||
| */ | ||
| private void logIgnoredCertificates(X509Certificate[] originalCerts, X509Certificate[] filteredCerts) { | ||
| CertificateLoggingUtils.logIgnoredCertificates( | ||
| originalCerts, | ||
| filteredCerts, | ||
| publicKeyCertificatesBase64, | ||
| log, | ||
| CategorizeCertsWebFilter::base64EncodePublicKey | ||
|
||
| ); | ||
| } | ||
|
|
||
| /** | ||
| * Extracts and decodes an X.509 certificate from the CLIENT_CERT_HEADER. | ||
| * | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please remove this as an argument, simply call the method or if not visible, move to a utility class that's visible in apiml-common