Merge pull request #174 from luisgoncalves/dn-comparison (closes #166)

Improve comparison of DNs
luisgoncalves · Nov 20, 2018 · 7978e91 · 7978e91
2 parents 0d4034f + a15d530
commit 7978e91
Show file tree

Hide file tree

Showing 15 changed files with 323 additions and 32 deletions.
diff --git a/pom.xml b/pom.xml
@@ -12,6 +12,8 @@
         <project.build.sourceEncoding>UTF-8</project.build.sourceEncoding>
         <maven.compiler.source>1.6</maven.compiler.source>
         <maven.compiler.target>1.6</maven.compiler.target>
+        <maven.compiler.testSource>1.7</maven.compiler.testSource>
+        <maven.compiler.testTarget>1.7</maven.compiler.testTarget>
         <bouncycastle.version>1.56</bouncycastle.version>
     </properties>
 

diff --git a/src/main/java/xades4j/production/XadesSigningProfile.java b/src/main/java/xades4j/production/XadesSigningProfile.java
@@ -210,12 +210,18 @@ public XadesSigningProfile withDigestEngineProvider(
         return withBinding(MessageDigestEngineProvider.class, digestProviderClass);
     }
 
+    /**
+     * <b>Experimental API</b>. It may be changed or removed in future releases.
+     */
     public XadesSigningProfile withX500NameStyleProvider(
             X500NameStyleProvider x500NameStyleProvider)
     {
         return withBinding(X500NameStyleProvider.class, x500NameStyleProvider);
     }
 
+    /**
+     * <b>Experimental API</b>. It may be changed or removed in future releases.
+     */
     public XadesSigningProfile withX500NameStyleProvider(
             Class<? extends X500NameStyleProvider> x500NameStyleProviderClass)
     {

diff --git a/src/main/java/xades4j/providers/X500NameStyleProvider.java b/src/main/java/xades4j/providers/X500NameStyleProvider.java
@@ -2,13 +2,26 @@
 
 import javax.security.auth.x500.X500Principal;
 
-
 /**
+ * <b>Experimental API</b>. It may be changed or removed in future releases.
+ * 
  * @author Artem R. Romanenko
  * @version 06.08.18
  */
 public interface X500NameStyleProvider
 {
+    /**
+     * Parse a DN string.
+     * @param dn
+     * @return the parsed DN
+     * @exception IllegalArgumentException if the name is invalid
+     */
     X500Principal fromString(String dn);
+
+    /**
+     * Get a DN string.
+     * @param dn
+     * @return the DN string
+     */
     String toString(X500Principal dn);
 }
diff --git a/src/main/java/xades4j/providers/impl/DefaultX500NameStyleProvider.java b/src/main/java/xades4j/providers/impl/DefaultX500NameStyleProvider.java
@@ -9,6 +9,8 @@
 import javax.security.auth.x500.X500Principal;
 
 /**
+ * <b>Experimental API</b>. It may be changed or removed in future releases.
+ * 
  * @author Artem R. Romanenko
  * @version 06.08.18
  */
@@ -31,7 +33,6 @@ public X500Principal fromString(String dn)
     {
         return new X500Principal(dn, x500ExtensibleNameStyle.getKeywordMap());
     }
-
     @Override
     public String toString(X500Principal x500Principal)
     {

diff --git a/src/main/java/xades4j/utils/RFC4519ExtensibleStyle.java b/src/main/java/xades4j/utils/RFC4519ExtensibleStyle.java
@@ -11,8 +11,9 @@
 import java.util.Map;
 import java.util.Set;
 
-
 /**
+ * <b>Experimental API</b>. It may be changed or removed in future releases.
+ * 
  * @author Artem R. Romanenko
  * @version 30.07.18
  */

diff --git a/src/main/java/xades4j/utils/X500ExtensibleNameStyle.java b/src/main/java/xades4j/utils/X500ExtensibleNameStyle.java
@@ -5,6 +5,8 @@
 import java.util.Map;
 
 /**
+ * <b>Experimental API</b>. It may be changed or removed in future releases.
+ * 
  * @author Artem R. Romanenko
  * @version 06.08.18
  */

diff --git a/src/main/java/xades4j/verification/CertRefUtils.java b/src/main/java/xades4j/verification/CertRefUtils.java
@@ -21,12 +21,10 @@
 import java.security.cert.X509Certificate;
 import java.util.Arrays;
 import java.util.Collection;
-import javax.security.auth.x500.X500Principal;
 import xades4j.UnsupportedAlgorithmException;
 import xades4j.XAdES4jException;
 import xades4j.properties.data.CertRef;
 import xades4j.providers.MessageDigestEngineProvider;
-import xades4j.providers.X500NameStyleProvider;
 
 /**
  *
@@ -37,17 +35,19 @@ class CertRefUtils
     static CertRef findCertRef(
             X509Certificate cert,
             Collection<CertRef> certRefs,
-            X500NameStyleProvider x500NameStyleProvider) throws SigningCertificateVerificationException
+            DistinguishedNameComparer dnComparer) throws SigningCertificateVerificationException
     {
         for (final CertRef certRef : certRefs)
         {
-            // Need to use a X500Principal because the DN strings can have different
-            // spaces and so on.
-            X500Principal certRefIssuerPrincipal;
             try
             {
-                certRefIssuerPrincipal = x500NameStyleProvider.fromString(certRef.issuerDN);
-            } catch (IllegalArgumentException ex)
+                if (dnComparer.areEqual(cert.getIssuerX500Principal(), certRef.issuerDN) &&
+                    certRef.serialNumber.equals(cert.getSerialNumber()))
+                {
+                    return certRef;
+                }
+            }
+            catch (IllegalArgumentException ex)
             {
                 throw new SigningCertificateVerificationException(ex)
                 {
@@ -58,9 +58,6 @@ protected String getVerificationMessage()
                     }
                 };
             }
-            if (cert.getIssuerX500Principal().equals(certRefIssuerPrincipal) &&
-                    certRef.serialNumber.equals(cert.getSerialNumber()))
-                return certRef;
         }
         return null;
     }

diff --git a/src/main/java/xades4j/verification/CompleteCertRefsVerifier.java b/src/main/java/xades4j/verification/CompleteCertRefsVerifier.java
@@ -26,7 +26,6 @@
 import xades4j.properties.data.CertRef;
 import xades4j.properties.data.CompleteCertificateRefsData;
 import xades4j.providers.MessageDigestEngineProvider;
-import xades4j.providers.X500NameStyleProvider;
 
 /**
  * XAdES G.2.2.12
@@ -35,16 +34,15 @@
 class CompleteCertRefsVerifier implements QualifyingPropertyVerifier<CompleteCertificateRefsData>
 {
     private final MessageDigestEngineProvider messageDigestProvider;
-    private final X500NameStyleProvider x500NameStyleProvider;
-
+    private final DistinguishedNameComparer dnComparer;
 
     @Inject
     public CompleteCertRefsVerifier(
             MessageDigestEngineProvider messageDigestProvider,
-            X500NameStyleProvider x500NameStyleProvider)
+            DistinguishedNameComparer dnComparer)
     {
         this.messageDigestProvider = messageDigestProvider;
-        this.x500NameStyleProvider = x500NameStyleProvider;
+        this.dnComparer = dnComparer;
     }
 
     @Override
@@ -61,7 +59,7 @@ public QualifyingProperty verify(
 
         for (X509Certificate caCert : caCerts)
         {
-            CertRef caRef = CertRefUtils.findCertRef(caCert, caCertRefs, this.x500NameStyleProvider);
+            CertRef caRef = CertRefUtils.findCertRef(caCert, caCertRefs, this.dnComparer);
             if (null == caRef)
                 throw new CompleteCertRefsCertNotFoundException(caCert);
             try

diff --git a/src/main/java/xades4j/verification/CompleteRevocRefsVerifier.java b/src/main/java/xades4j/verification/CompleteRevocRefsVerifier.java
@@ -32,7 +32,6 @@
 import xades4j.properties.data.CRLRef;
 import xades4j.properties.data.CompleteRevocationRefsData;
 import xades4j.providers.MessageDigestEngineProvider;
-import xades4j.providers.X500NameStyleProvider;
 import xades4j.utils.CrlExtensionsUtils;
 
 /**
@@ -42,15 +41,15 @@
 class CompleteRevocRefsVerifier implements QualifyingPropertyVerifier<CompleteRevocationRefsData>
 {
     private final MessageDigestEngineProvider digestEngineProvider;
-    private final X500NameStyleProvider x500NameStyleProvider;
+    private final DistinguishedNameComparer dnComparer;
 
     @Inject
     public CompleteRevocRefsVerifier(
             MessageDigestEngineProvider digestEngineProvider,
-            X500NameStyleProvider x500NameStyleProvider)
+            DistinguishedNameComparer dnComparer)
     {
         this.digestEngineProvider = digestEngineProvider;
-        this.x500NameStyleProvider = x500NameStyleProvider;
+        this.dnComparer = dnComparer;
     }
 
     @Override
@@ -75,7 +74,8 @@ public QualifyingProperty verify(
                 // should treat the signature as invalid."
 
                 // Check issuer and issue time.
-                if (!crl.getIssuerX500Principal().equals(this.x500NameStyleProvider.fromString(crlRef.issuerDN)) ||
+
+                if (!this.dnComparer.areEqual(crl.getIssuerX500Principal(), crlRef.issuerDN) ||
                         !crl.getThisUpdate().equals(crlRef.issueTime.getTime()))
                     continue;
 

diff --git a/src/main/java/xades4j/verification/DefaultVerificationBindingsModule.java b/src/main/java/xades4j/verification/DefaultVerificationBindingsModule.java
@@ -50,6 +50,8 @@
 import xades4j.providers.TimeStampVerificationProvider;
 import xades4j.providers.impl.DefaultX500NameStyleProvider;
 import xades4j.utils.BuiltIn;
+import xades4j.utils.RFC4519ExtensibleStyle;
+import xades4j.utils.X500ExtensibleNameStyle;
 
 /**
  * Contains the Guice bindings for the default components and the bindings for the
@@ -90,6 +92,7 @@ public InputStream getSignaturePolicyDocumentStream(
         bind(QualifyingPropertiesVerifier.class).to(QualifyingPropertiesVerifierImpl.class);
         bind(QualifyingPropertyVerifiersMapper.class).to(QualifyingPropertyVerifiersMapperImpl.class);
         bind(X500NameStyleProvider.class).to(DefaultX500NameStyleProvider.class);
+        bind(X500ExtensibleNameStyle.class).to(RFC4519ExtensibleStyle.class);
 //        customGlobalStructureVerifiers.add(new CustomPropertiesDataObjsStructureVerifier()
 //        {
 //            @Override

diff --git a/src/main/java/xades4j/verification/DistinguishedNameComparer.java b/src/main/java/xades4j/verification/DistinguishedNameComparer.java
@@ -0,0 +1,51 @@
+/*
+ * XAdES4j - A Java library for generation and verification of XAdES signatures.
+ * Copyright (C) 2018 Luis Goncalves.
+ *
+ * XAdES4j is free software; you can redistribute it and/or modify it under
+ * the terms of the GNU Lesser General Public License as published by the Free
+ * Software Foundation; either version 3 of the License, or any later version.
+ *
+ * XAdES4j is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS
+ * FOR A PARTICULAR PURPOSE. See the GNU Lesser General Public License for more
+ * details.
+ *
+ * You should have received a copy of the GNU Lesser General Public License along
+ * with XAdES4j. If not, see <http://www.gnu.org/licenses/>.
+ */
+package xades4j.verification;
+
+import com.google.inject.Inject;
+import javax.security.auth.x500.X500Principal;
+import org.bouncycastle.asn1.x500.X500Name;
+import xades4j.providers.X500NameStyleProvider;
+import xades4j.utils.X500ExtensibleNameStyle;
+
+/**
+ * <b>Experimental API</b>. It may be changed or removed in future releases.
+ *
+ * @author luis
+ */
+class DistinguishedNameComparer
+{
+    private final X500ExtensibleNameStyle x500NameStyle;
+    private final X500NameStyleProvider x500NameStyleProvider;
+
+    @Inject
+    DistinguishedNameComparer(X500ExtensibleNameStyle x500NameStyle, X500NameStyleProvider x500NameStyleProvider)
+    {
+        this.x500NameStyle = x500NameStyle;
+        this.x500NameStyleProvider = x500NameStyleProvider;
+    }
+
+    /**
+     * @exception IllegalArgumentException if the DN string is invalid 
+     */
+    boolean areEqual(X500Principal parsedDn, String stringDn)
+    {
+        X500Name first = X500Name.getInstance(parsedDn.getEncoded());
+        X500Name second = X500Name.getInstance(this.x500NameStyle, this.x500NameStyleProvider.fromString(stringDn).getEncoded());
+        return first.equals(second);
+    }
+}
diff --git a/src/main/java/xades4j/verification/SigningCertificateVerifier.java b/src/main/java/xades4j/verification/SigningCertificateVerifier.java
@@ -26,7 +26,6 @@
 import xades4j.properties.data.CertRef;
 import xades4j.providers.MessageDigestEngineProvider;
 import xades4j.properties.data.SigningCertificateData;
-import xades4j.providers.X500NameStyleProvider;
 import xades4j.verification.QualifyingPropertyVerificationContext.CertificationChainData;
 
 /**
@@ -36,15 +35,15 @@
 class SigningCertificateVerifier implements QualifyingPropertyVerifier<SigningCertificateData>
 {
     private final MessageDigestEngineProvider messageDigestProvider;
-    private final X500NameStyleProvider x500NameStyleProvider;
+    private final DistinguishedNameComparer dnComparer;
 
     @Inject
     public SigningCertificateVerifier(
             MessageDigestEngineProvider messageDigestProvider,
-            X500NameStyleProvider x500NameStyleProvider)
+            DistinguishedNameComparer dnComparer)
     {
         this.messageDigestProvider = messageDigestProvider;
-        this.x500NameStyleProvider = x500NameStyleProvider;
+        this.dnComparer = dnComparer;
     }
 
     @Override
@@ -62,7 +61,7 @@ public QualifyingProperty verify(
         // "If the verifier does not find any reference matching the signing certificate,
         // the validation of this property should be taken as failed."
         X509Certificate signingCert = certPathIter.next();
-        CertRef signingCertRef = CertRefUtils.findCertRef(signingCert, certRefs, this.x500NameStyleProvider);
+        CertRef signingCertRef = CertRefUtils.findCertRef(signingCert, certRefs, this.dnComparer);
         if (null == signingCertRef)
             throw new SigningCertificateReferenceNotFoundException(signingCert);
 
@@ -71,7 +70,7 @@ public QualifyingProperty verify(
         // from SigningCertificate, are the same."
         X500Principal keyInfoIssuer = certChainData.getValidationCertIssuer();
         if (keyInfoIssuer != null &&
-                (!this.x500NameStyleProvider.fromString(signingCertRef.issuerDN).equals(keyInfoIssuer) ||
+                (!this.dnComparer.areEqual(keyInfoIssuer, signingCertRef.issuerDN) ||
                 !signingCertRef.serialNumber.equals(certChainData.getValidationCertSerialNumber())))
             throw new SigningCertificateIssuerSerialMismatchException(
                     signingCertRef.issuerDN,
@@ -94,7 +93,7 @@ public QualifyingProperty verify(
         while (certPathIter.hasNext())
         {
             X509Certificate cert = certPathIter.next();
-            CertRef certRef = CertRefUtils.findCertRef(cert, certRefs, this.x500NameStyleProvider);
+            CertRef certRef = CertRefUtils.findCertRef(cert, certRefs, this.dnComparer);
             // "Should one or more certificates in the certification path not be
             // referenced by this property, the verifier should assume that the
             // verification is successful (...)"