JiriOndrusek commented on code in PR #558:
URL:
https://github.com/apache/camel-quarkus-examples/pull/558#discussion_r3735626588
##########
http-pqc-j17/src/main/java/org/acme/http/pqc/trustmanager/HybridPqcX509TrustManager.java:
##########
@@ -18,60 +18,139 @@
import java.security.cert.CertificateException;
import java.security.cert.X509Certificate;
+import java.util.Objects;
import javax.net.ssl.X509TrustManager;
+import javax.security.auth.x500.X500Principal;
import org.acme.http.pqc.certificates.util.CertificateValidationException;
import org.acme.http.pqc.certificates.util.CertificatesUtil;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
/**
- * Custom X509TrustManager that validates hybrid PQC certificates at the TLS
layer.
+ * Custom {@link X509TrustManager} that adds hybrid PQC validation on top of
the standard checks.
*
- * This TrustManager validates both RSA and ML-DSA-65 signatures during the
TLS handshake,
- * rejecting connections with invalid or RSA-only certificates before the
application layer
- * sees the request.
+ * <p>
+ * The important part of this class is what it does <em>not</em> do itself.
Chain building, trust
+ * anchor lookup and validity-period checking are delegated to the platform
trust manager that
+ * Quarkus builds from the configured truststore; only once that has passed is
the ML-DSA-65
+ * alternative signature verified on top. Getting this ordering wrong is the
classic way to write a
+ * trust manager that accepts anything: a custom check on its own replaces the
platform checks rather
+ * than adding to them, because the JSSE handshake asks this class and nothing
else.
+ *
+ * <p>
+ * The effect is that a client certificate must chain to a trust anchor, be
inside its validity
+ * period, <em>and</em> carry a valid ML-DSA-65 alternative signature made by
its issuer. A
+ * self-signed certificate carrying well-formed PQC extensions is rejected,
because no anchor vouches
+ * for it, and a certificate issued by the trusted CA without an alternative
signature is rejected
+ * too.
+ *
+ * <p>
+ * Note that this example has no revocation checking (no CRL or OCSP), which a
production deployment
+ * would need.
*/
public class HybridPqcX509TrustManager implements X509TrustManager {
Review Comment:
**Observation: `X509TrustManager` vs `X509ExtendedTrustManager`**
This implements `X509TrustManager`, not `X509ExtendedTrustManager`. The
extended interface adds 3-arg methods (`checkServerTrusted(chain, authType,
SSLEngine)`) that carry the SSL context needed for endpoint identification
(hostname verification).
For this example it is safe:
- Server-side `checkClientTrusted` does not do hostname verification
regardless.
- JSSE's `SSLContextImpl.chooseTrustManager()` wraps a plain
`X509TrustManager` in `AbstractTrustManagerWrapper`, which performs endpoint
identification after calling the 2-arg method.
However, that safety net does not cover non-standard deployments (e.g.
Netty's OpenSSL/tcnative provider, which bypasses `SSLContext.init()`). If
someone adapts this for client-side TLS in such a setup,
`checkServerTrusted(chain, authType)` on the delegate runs without the
`SSLEngine`, so hostname verification is skipped — a valid certificate for
`evil.com` would be accepted when connecting to `good.com`.
Implementing `X509ExtendedTrustManager` instead and delegating the 3-arg
methods to `delegate.checkServerTrusted(chain, authType, engine)` before
calling `validateHybridChain` would close this gap for all deployment scenarios.
##########
http-pqc-j17/src/main/java/org/acme/http/pqc/trustmanager/HybridPqcX509TrustManager.java:
##########
@@ -18,60 +18,139 @@
import java.security.cert.CertificateException;
import java.security.cert.X509Certificate;
+import java.util.Objects;
import javax.net.ssl.X509TrustManager;
+import javax.security.auth.x500.X500Principal;
import org.acme.http.pqc.certificates.util.CertificateValidationException;
import org.acme.http.pqc.certificates.util.CertificatesUtil;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
/**
- * Custom X509TrustManager that validates hybrid PQC certificates at the TLS
layer.
+ * Custom {@link X509TrustManager} that adds hybrid PQC validation on top of
the standard checks.
*
- * This TrustManager validates both RSA and ML-DSA-65 signatures during the
TLS handshake,
- * rejecting connections with invalid or RSA-only certificates before the
application layer
- * sees the request.
+ * <p>
+ * The important part of this class is what it does <em>not</em> do itself.
Chain building, trust
+ * anchor lookup and validity-period checking are delegated to the platform
trust manager that
+ * Quarkus builds from the configured truststore; only once that has passed is
the ML-DSA-65
+ * alternative signature verified on top. Getting this ordering wrong is the
classic way to write a
+ * trust manager that accepts anything: a custom check on its own replaces the
platform checks rather
+ * than adding to them, because the JSSE handshake asks this class and nothing
else.
+ *
+ * <p>
+ * The effect is that a client certificate must chain to a trust anchor, be
inside its validity
+ * period, <em>and</em> carry a valid ML-DSA-65 alternative signature made by
its issuer. A
+ * self-signed certificate carrying well-formed PQC extensions is rejected,
because no anchor vouches
+ * for it, and a certificate issued by the trusted CA without an alternative
signature is rejected
+ * too.
+ *
+ * <p>
+ * Note that this example has no revocation checking (no CRL or OCSP), which a
production deployment
+ * would need.
*/
public class HybridPqcX509TrustManager implements X509TrustManager {
private static final Logger LOG =
LoggerFactory.getLogger(HybridPqcX509TrustManager.class);
+ private final X509TrustManager delegate;
+
+ /**
+ * @param delegate the platform trust manager to perform chain, anchor and
expiry validation
+ */
+ public HybridPqcX509TrustManager(X509TrustManager delegate) {
+ this.delegate = Objects.requireNonNull(delegate, "delegate trust
manager is required");
+ }
+
@Override
public void checkClientTrusted(X509Certificate[] chain, String authType)
throws CertificateException {
- if (chain == null || chain.length == 0) {
- throw new CertificateException("Client certificate chain is
empty");
- }
+ requireChain(chain, "Client");
- X509Certificate clientCert = chain[0];
- LOG.debug("Validating client certificate at TLS layer: {}",
clientCert.getSubjectX500Principal());
+ // Standard X.509 validation first: chain, trust anchor, validity
period
+ delegate.checkClientTrusted(chain, authType);
- try {
- // Validate hybrid certificate - throws
CertificateValidationException on failure
- CertificatesUtil.validateHybridCertificate(clientCert);
- LOG.debug("Client certificate validated successfully at TLS layer
(RSA + ML-DSA-65)");
- } catch (CertificateValidationException e) {
- LOG.error("Hybrid PQC certificate validation failed: {}",
e.getMessage());
- throw new CertificateException("Validation failed: " +
e.getMessage(), e);
- }
+ validateHybridChain(chain, "Client");
}
@Override
public void checkServerTrusted(X509Certificate[] chain, String authType)
throws CertificateException {
- // Not implemented - this example only validates client certificates
- // (server-to-client authentication, not client-to-server)
- //
- // In mutual TLS where client validates server's hybrid certificate,
- // this method would implement similar validation logic.
- LOG.debug("Server certificate validation not implemented (client-auth
only)");
+ requireChain(chain, "Server");
+
+ // Standard X.509 validation first: chain, trust anchor, validity
period, hostname material
+ delegate.checkServerTrusted(chain, authType);
+
+ validateHybridChain(chain, "Server");
}
@Override
public X509Certificate[] getAcceptedIssuers() {
- // Return empty array for self-signed certificates in this demo.
- //
- // In production with a CA hierarchy, this would return the
- // list of trusted CA certificates that can issue client certificates.
- return new X509Certificate[0];
+ // Delegate, so that the certificate authorities advertised during the
handshake are the ones
+ // in the configured truststore. Returning an empty array here tells
peers nothing about which
+ // issuers are acceptable.
+ return delegate.getAcceptedIssuers();
+ }
+
+ private static void requireChain(X509Certificate[] chain, String peer)
throws CertificateException {
+ if (chain == null || chain.length == 0) {
+ throw new CertificateException(peer + " certificate chain is
empty");
+ }
+ }
+
+ /**
+ * Verifies the ML-DSA-65 alternative signature on every certificate in
the chain, each against the
+ * ML-DSA-65 public key published by its issuer.
+ */
+ private void validateHybridChain(X509Certificate[] chain, String peer)
throws CertificateException {
+ for (X509Certificate cert : chain) {
+ LOG.debug("Validating {} certificate hybrid PQC extensions: {}",
peer, cert.getSubjectX500Principal());
+
+ X509Certificate issuer = findIssuer(cert, chain);
+ if (issuer == null) {
+ throw new CertificateException("Could not find the issuer of "
+ cert.getSubjectX500Principal()
+ + ", so its ML-DSA-65 signature cannot be verified");
+ }
+
+ try {
+ CertificatesUtil.validateHybridCertificate(cert, issuer);
+ } catch (CertificateValidationException e) {
+ LOG.error("Hybrid PQC certificate validation failed: {}",
e.getMessage());
+ throw new CertificateException("Validation failed: " +
e.getMessage(), e);
+ }
+ }
+
+ LOG.debug("{} certificate chain validated successfully (RSA chain +
ML-DSA-65)", peer);
+ }
+
+ /**
+ * Finds the certificate that issued {@code cert}, looking first in the
chain the peer presented and
+ * then among the configured trust anchors. Peers commonly send only their
own certificate and leave
+ * the anchor to the relying party, so both need checking.
+ *
+ * <p>
+ * The chain has already been validated by the delegate at this point, so
a certificate found in it
+ * is one the platform trust manager accepted as part of a path to an
anchor.
+ */
+ private X509Certificate findIssuer(X509Certificate cert, X509Certificate[]
chain) {
+ X500Principal issuerName = cert.getIssuerX500Principal();
+
+ for (X509Certificate candidate : chain) {
+ if (candidate != cert &&
candidate.getSubjectX500Principal().equals(issuerName)) {
Review Comment:
**Observation: DN-only issuer matching**
`findIssuer` matches by `X500Principal` (the distinguished name) alone. The
platform trust manager uses Authority Key Identifier (AKI) / Subject Key
Identifier (SKI) for unambiguous issuer resolution, so the PQC layer can
diverge from the chain the platform actually validated.
For this example's flat hierarchy (one CA, no intermediates) it is safe.
In a production PKI it breaks under:
- **CA key rollover** — truststore holds both old and new root CAs with the
same DN but different ML-DSA keys. `getAcceptedIssuers()` returns whichever
first; if it picks the wrong one, the PQC check rejects a valid certificate.
- **Cross-certified CAs** — two CAs sharing a DN, same problem.
- **Malicious chain injection** — an attacker sends `[leaf,
rogue-intermediate]` where the rogue has the right DN but the wrong alt public
key. The platform ignores it (AKI/SKI mismatch), but `findIssuer` picks it
because it matches the DN and appears first.
Using the AKI extension (OID 2.5.29.35) on the certificate under test to
match the issuer's SKI would resolve all three.
--
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]
