jamesnetherton commented on code in PR #558:
URL:
https://github.com/apache/camel-quarkus-examples/pull/558#discussion_r3736003589
##########
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:
Agreed, and fixed in de263d6c. The class now extends
`X509ExtendedTrustManager` and delegates all four three-argument overloads to
the matching delegate method, so the `Socket`/`SSLEngine` reaches the platform
trust manager that performs endpoint identification.
`HybridPqcTrustManagerCustomizer` now also *requires* the platform trust
manager to be an `X509ExtendedTrustManager` and fails startup otherwise, rather
than quietly wrapping one that cannot verify hostnames. Every JDK provider has
returned the extended form since Java 7, so failing there means something
unusual is in play.
A test asserts the delegation rather than just the outcome: a recording
delegate checks that each three-argument overload reaches the corresponding
delegate method. Forwarding them to the two-argument method would validate the
certificate just as well and pass a naive test while dropping the hostname
check.
I also corrected the README, which had claimed hostname verification is lost
outright — your point about `AbstractTrustManagerWrapper` is right, and the
text now says so and names the bypass case instead.
Verified with `mvn clean verify` (21 tests) and `mvn clean verify -Dnative`
(native build plus 5 native integration tests).
---
_Claude Code on behalf of James Netherton_
##########
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:
Agreed, and fixed in de263d6c. `findIssuer` now resolves in three ordered
steps: a candidate whose subject key identifier matches the certificate's
authority key identifier, then the certificate itself when it is self-issued,
then a candidate matching the issuer name alone for certificates predating the
extensions. `HybridCertificateGenerator` now issues certificates carrying both
extensions, which the matching needs. Preferring key-identifier matches also
covers your third case — a certificate carrying only the right issuer name can
no longer displace the one the platform built the path through.
Working through your key-rollover case turned up something I had missed on
the first attempt: the leaf is not the hard part. A self-signed anchor carries
no authority key identifier, so nothing distinguishes it from a same-DN sibling
— and this example's keystores put the CA in the client's chain, so
`validateHybridChain` checks the CA on every handshake. With a rolled-over CA
in the truststore twice, the CA's own ML-DSA-65 signature was verified against
the *other* CA's key. That is what the self-issued step exists for. The
regression test sends each certificate both alone and with its CA, and the
CA-in-chain assertions fail without it.
One thing worth being precise about: none of this was a bypass. The delegate
validates the chain first, so a mis-resolved issuer causes a valid certificate
to be *rejected*, not an invalid one accepted — and a peer only controls its
own chain. The value is keeping this layer in step with the path the platform
actually built.
Verified with `mvn clean verify` (21 tests) and `mvn clean verify -Dnative`
(native build plus 5 native integration tests). Each new test was confirmed to
fail without its corresponding fix.
---
_Claude Code on behalf of James Netherton_
--
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]
