From 0836895a1e985ecd6d1a89b1a3e2a379449110bd Mon Sep 17 00:00:00 2001 From: cyb3r4nt <104218001+cyb3r4nt@users.noreply.github.com> Date: Sat, 28 Dec 2024 00:05:38 +0200 Subject: [PATCH 1/2] Improve search of trusted CA certificate in AuthenticationResponseValidator.isCertificateTrusted() Adds search of trusted CA certificates by their distinguished name. Reduces overall calculations of signatures during certificate validation. Distinguished names of signer's certificate issuer and trusted CA certificate subject must be same, because these identify the CA certificate, and are signed within the CA certificate chain. Also adds additional logging when trusted CA certificate is missing (#75) Signed-off-by: cyb3r4nt <104218001+cyb3r4nt@users.noreply.github.com> --- .../AuthenticationResponseValidator.java | 70 ++++++++++++++++++- 1 file changed, 69 insertions(+), 1 deletion(-) diff --git a/src/main/java/ee/sk/smartid/AuthenticationResponseValidator.java b/src/main/java/ee/sk/smartid/AuthenticationResponseValidator.java index e295e8e..05c08a7 100644 --- a/src/main/java/ee/sk/smartid/AuthenticationResponseValidator.java +++ b/src/main/java/ee/sk/smartid/AuthenticationResponseValidator.java @@ -38,6 +38,7 @@ import javax.naming.InvalidNameException; import javax.naming.ldap.LdapName; import javax.naming.ldap.Rdn; +import javax.security.auth.x500.X500Principal; import java.io.ByteArrayInputStream; import java.io.File; import java.io.IOException; @@ -59,7 +60,7 @@ public class AuthenticationResponseValidator { private static final Logger logger = LoggerFactory.getLogger(AuthenticationResponseValidator.class); - private List trustedCACertificates = new ArrayList<>(); + private final List trustedCACertificates = new ArrayList<>(); /** * Constructs a new {@code AuthenticationResponseValidator}. *

@@ -243,8 +244,68 @@ private boolean verifyCertificateExpiry(X509Certificate certificate) { return !certificate.getNotAfter().before(new Date()); } + private static final class CertDnDetails { + private final String country; + private final String organization; + private final String commonName; + + private CertDnDetails(String country, String organization, String commonName) { + this.country = country; + this.organization = organization; + this.commonName = commonName; + } + + private static CertDnDetails from(X500Principal principal) { + String country = null; + String organization = null; + String commonName = null; + LdapName ldapName; + try { + ldapName = new LdapName(principal.getName()); + } catch (InvalidNameException e) { + String errorMessage = "Error getting certificate common name details"; + logger.error(errorMessage, e); + throw new SmartIdClientException(errorMessage, e); + } + for (Rdn rdn : ldapName.getRdns()) { + if (rdn.getType().equalsIgnoreCase("C")) { + country = rdn.getValue().toString(); + } else if (rdn.getType().equalsIgnoreCase("O")) { + organization = rdn.getValue().toString(); + } else if (rdn.getType().equalsIgnoreCase("CN")) { + commonName = rdn.getValue().toString(); + } + } + return new CertDnDetails(country, organization, commonName); + } + + private static boolean equal(CertDnDetails first, CertDnDetails second) { + return Objects.equals(first.country, second.country) + && Objects.equals(first.organization, second.organization) + && Objects.equals(first.commonName, second.commonName); + } + } + private boolean isCertificateTrusted(X509Certificate certificate) { + CertDnDetails issuerDN = CertDnDetails.from(certificate.getIssuerX500Principal()); + for (X509Certificate trustedCACertificate : trustedCACertificates) { + logger.debug( + "Verifying signer's certificate '{}' against CA certificate '{}'", + certificate.getSubjectDN(), + trustedCACertificate.getSubjectDN() + ); + + CertDnDetails caCertDN = CertDnDetails.from(trustedCACertificate.getSubjectX500Principal()); + if (!CertDnDetails.equal(issuerDN, caCertDN)) { + logger.debug( + "Skipped trusted CA certificate '{}', no match with signer's certificate issuer '{}'", + trustedCACertificate.getSubjectDN(), + certificate.getIssuerX500Principal().toString() + ); + continue; + } + try { certificate.verify(trustedCACertificate.getPublicKey()); logger.info("Certificate verification passed for '{}' against CA certificate '{}' ", certificate.getSubjectDN() ,trustedCACertificate.getSubjectDN() ); @@ -254,6 +315,13 @@ private boolean isCertificateTrusted(X509Certificate certificate) { logger.debug("Error verifying signer's certificate: " + certificate.getSubjectDN() + " against CA certificate: " + trustedCACertificate.getSubjectDN(), e); } } + + logger.error( + "No suitable trusted CA certificate found: '{}'." + + " Ensure that this CA certificate is present in the trusted CA certificate list", + certificate.getIssuerX500Principal().toString() + ); + return false; } From 124b75a63d7020e085e24cf09e4720bf391145d1 Mon Sep 17 00:00:00 2001 From: cyb3r4nt <104218001+cyb3r4nt@users.noreply.github.com> Date: Sat, 28 Dec 2024 02:44:23 +0200 Subject: [PATCH 2/2] Fix error message in AuthenticationResponseValidator.CertDnDetails.from() Signed-off-by: cyb3r4nt <104218001+cyb3r4nt@users.noreply.github.com> --- .../java/ee/sk/smartid/AuthenticationResponseValidator.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/ee/sk/smartid/AuthenticationResponseValidator.java b/src/main/java/ee/sk/smartid/AuthenticationResponseValidator.java index 05c08a7..c67b502 100644 --- a/src/main/java/ee/sk/smartid/AuthenticationResponseValidator.java +++ b/src/main/java/ee/sk/smartid/AuthenticationResponseValidator.java @@ -263,7 +263,7 @@ private static CertDnDetails from(X500Principal principal) { try { ldapName = new LdapName(principal.getName()); } catch (InvalidNameException e) { - String errorMessage = "Error getting certificate common name details"; + String errorMessage = "Error getting certificate distinguished name"; logger.error(errorMessage, e); throw new SmartIdClientException(errorMessage, e); }