diff --git a/pom.xml b/pom.xml index aaaa8110..c7b585d3 100644 --- a/pom.xml +++ b/pom.xml @@ -12,6 +12,8 @@ UTF-8 1.6 1.6 + 1.7 + 1.7 1.56 diff --git a/src/main/java/xades4j/production/XadesSigningProfile.java b/src/main/java/xades4j/production/XadesSigningProfile.java index 0d29e264..e0a1461d 100644 --- 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); } + /** + * Experimental API. It may be changed or removed in future releases. + */ public XadesSigningProfile withX500NameStyleProvider( X500NameStyleProvider x500NameStyleProvider) { return withBinding(X500NameStyleProvider.class, x500NameStyleProvider); } + /** + * Experimental API. It may be changed or removed in future releases. + */ public XadesSigningProfile withX500NameStyleProvider( Class x500NameStyleProviderClass) { diff --git a/src/main/java/xades4j/providers/X500NameStyleProvider.java b/src/main/java/xades4j/providers/X500NameStyleProvider.java index a16045a2..1bdb9945 100644 --- 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; - /** + * Experimental API. 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 index 6873e87d..b7f442fe 100644 --- 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; /** + * Experimental API. 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 index 969b54d9..d1afb82d 100644 --- 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; - /** + * Experimental API. 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 index d0ebf9c8..667f2ecb 100644 --- a/src/main/java/xades4j/utils/X500ExtensibleNameStyle.java +++ b/src/main/java/xades4j/utils/X500ExtensibleNameStyle.java @@ -5,6 +5,8 @@ import java.util.Map; /** + * Experimental API. 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 index f125e815..f2f9e5a9 100644 --- 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 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 index 1acb68bf..966ed476 100644 --- 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 { 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 index cce1effb..b6cedece 100644 --- 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 { 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 index 5e92c0d8..0f8cfeb8 100644 --- 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 new file mode 100644 index 00000000..3e3f4fab --- /dev/null +++ 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 . + */ +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; + +/** + * Experimental API. 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 index 5834bbed..7fd473f7 100644 --- 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 { 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 (...)" diff --git a/src/test/java/xades4j/verification/DistinguishedNameComparerTest.java b/src/test/java/xades4j/verification/DistinguishedNameComparerTest.java new file mode 100644 index 00000000..329c5d64 --- /dev/null +++ b/src/test/java/xades4j/verification/DistinguishedNameComparerTest.java @@ -0,0 +1,97 @@ +package xades4j.verification; + +import java.io.FileInputStream; +import java.io.IOException; +import java.io.InputStream; +import java.security.cert.CertificateFactory; +import java.security.cert.X509Certificate; +import java.util.Arrays; +import java.util.Collection; +import javax.security.auth.x500.X500Principal; +import static org.junit.Assert.*; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.Parameterized; +import org.junit.runners.Parameterized.Parameters; +import xades4j.providers.X500NameStyleProvider; +import xades4j.providers.impl.DefaultX500NameStyleProvider; +import xades4j.utils.RFC4519ExtensibleStyle; +import xades4j.utils.SignatureServicesTestBase; + +/** + * @author luis + */ +@RunWith(Parameterized.class) +public class DistinguishedNameComparerTest extends SignatureServicesTestBase +{ + @Parameters + public static Collection data() throws Exception + { + return Arrays.asList(new Object[][] + { + // #1 + // Certificate includes the value of OID.2.5.4.97 as UTF8String + { + "2.5.4.97=#0c0f56415445532d413636373231343939,CN=UANATACA CA1 2016,OU=TSP-UANATACA,O=UANATACA S.A.,L=Barcelona (see current address at www.uanataca.com/address),C=ES", + certFromResource("issue166/EMPUBqscdA.cer") + }, + { + "2.5.4.97=#130f56415445532d413636373231343939,CN=UANATACA CA1 2016,OU=TSP-UANATACA,O=UANATACA S.A.,L=Barcelona (see current address at www.uanataca.com/address),C=ES", + certFromResource("issue166/EMPUBqscdA.cer") + }, + { + "OID.2.5.4.97=VATES-A66721499, CN=UANATACA CA1 2016, OU=TSP-UANATACA, O=UANATACA S.A., L=Barcelona (see current address at www.uanataca.com/address), C=ES", + certFromResource("issue166/EMPUBqscdA.cer") + }, + // #2 + { + "CN = Itermediate, OU = CC, O = ISEL, C = PT", + certFromFile("my/LG.cer") + }, + // #3 + { + "C = PT, O = SCEE - Sistema de Certificação Electrónica do Estado, OU = ECEstado, CN = Cartão de Cidadão 001", + certFromFile("pt/ECQualifSigCC0001.cer") + } + }); + } + + private static X509Certificate certFromResource(String resourcePath) throws Exception + { + CertificateFactory certFactory = CertificateFactory.getInstance("X.509"); + try (InputStream is = DistinguishedNameComparerTest.class.getResourceAsStream(resourcePath)) + { + return (X509Certificate) certFactory.generateCertificate(is); + } + } + + private static X509Certificate certFromFile(String filePath) throws Exception + { + CertificateFactory certFactory = CertificateFactory.getInstance("X.509"); + try (InputStream is = new FileInputStream(toPlatformSpecificCertDirFilePath(filePath))) + { + return (X509Certificate) certFactory.generateCertificate(is); + } + } + private final String issuerDn; + private final X509Certificate cert; + private final RFC4519ExtensibleStyle nameStyle; + private final X500NameStyleProvider x500NameStyleProvider; + + public DistinguishedNameComparerTest(String issuerDn, X509Certificate cert) throws IOException + { + this.issuerDn = issuerDn; + this.cert = cert; + this.nameStyle = new RFC4519ExtensibleStyle(); + this.x500NameStyleProvider = new DefaultX500NameStyleProvider(this.nameStyle); + } + + @Test + public void canCompare() + { + X500Principal principal = cert.getIssuerX500Principal(); + DistinguishedNameComparer comparer = new DistinguishedNameComparer(this.nameStyle, this.x500NameStyleProvider); + + assertTrue(comparer.areEqual(principal, issuerDn)); + } +} diff --git a/src/test/java/xades4j/verification/Issue166Test.java b/src/test/java/xades4j/verification/Issue166Test.java new file mode 100644 index 00000000..11a8121e --- /dev/null +++ b/src/test/java/xades4j/verification/Issue166Test.java @@ -0,0 +1,121 @@ +package xades4j.verification; + +import java.io.InputStream; +import java.security.cert.CertificateFactory; +import java.security.cert.X509Certificate; +import javax.security.auth.x500.X500Principal; +import org.bouncycastle.asn1.x500.X500Name; +import org.junit.Assert; +import org.junit.Before; +import org.junit.Test; +import xades4j.utils.SignatureServicesTestBase; + +/** + * Investigation for https://github.com/luisgoncalves/xades4j/issues/166. + * @author luis + */ +public class Issue166Test extends SignatureServicesTestBase +{ + private final String dnUtf8 = "2.5.4.97=#0c0f56415445532d413636373231343939,CN=UANATACA CA1 2016,OU=TSP-UANATACA,O=UANATACA S.A.,L=Barcelona (see current address at www.uanataca.com/address),C=ES"; + private final String dnPrintable = "2.5.4.97=#130f56415445532d413636373231343939,CN=UANATACA CA1 2016,OU=TSP-UANATACA,O=UANATACA S.A.,L=Barcelona (see current address at www.uanataca.com/address),C=ES"; + private final String dnPlain = "OID.2.5.4.97=VATES-A66721499, CN=UANATACA CA1 2016, OU=TSP-UANATACA, O=UANATACA S.A., L=Barcelona (see current address at www.uanataca.com/address), C=ES"; + + private X509Certificate cert; + + @Before + public void setUp() throws Exception + { + CertificateFactory certFactory = CertificateFactory.getInstance("X.509"); + // Certificate includes the value of OID.2.5.4.97 as UTF8String + try(InputStream is = getClass().getResourceAsStream("issue166/EMPUBqscdA.cer")) + { + cert = (X509Certificate) certFactory.generateCertificate(is); + } + } + + @Test + public void javaCannotCompareStrings() throws Exception + { + X500Principal principal1 = new X500Principal(dnUtf8); + X500Principal principal2 = new X500Principal(dnPrintable); + X500Principal principal3 = new X500Principal(dnPlain); + + Assert.assertFalse(principal1.equals(principal2)); + Assert.assertFalse(principal1.equals(principal3)); + } + + @Test + public void javaCanComparePrintableAndPlainStrings() throws Exception + { + X500Principal principal1 = new X500Principal(dnPrintable); + X500Principal principal2 = new X500Principal(dnPlain); + + Assert.assertTrue(principal1.equals(principal2)); + } + + @Test + public void javaCannotCompareCertAndPrintableString() throws Exception + { + X500Principal principal1 = cert.getIssuerX500Principal(); + X500Principal principal2 = new X500Principal(dnPrintable); + + Assert.assertFalse(principal1.equals(principal2)); + } + + @Test + public void javaCanCompareCertAndUtf8String() throws Exception + { + X500Principal principal1 = cert.getIssuerX500Principal(); + X500Principal principal2 = new X500Principal(dnUtf8); + + Assert.assertTrue(principal1.equals(principal2)); + } + + @Test + public void javaCannotCompareCertAndPlainString() throws Exception + { + X500Principal principal1 = cert.getIssuerX500Principal(); + X500Principal principal2 = new X500Principal(dnPlain); + + Assert.assertFalse(principal1.equals(principal2)); + } + + @Test + public void bcCanCompareStrings() throws Exception + { + X500Name principal1 = new X500Name(dnUtf8); + X500Name principal2 = new X500Name(dnPrintable); + X500Name principal3 = new X500Name(dnPlain); + + Assert.assertTrue(principal1.equals(principal2)); + Assert.assertTrue(principal1.equals(principal3)); + Assert.assertTrue(principal2.equals(principal3)); + } + + @Test + public void bcCanCompareCertAndPrintableString() throws Exception + { + X500Name principal1 = X500Name.getInstance(cert.getIssuerX500Principal().getEncoded()); + X500Name principal2 = new X500Name(dnPrintable); + + Assert.assertTrue(principal1.equals(principal2)); + } + + @Test + public void bcCanCompareCertAndUtf8String() throws Exception + { + X500Name principal1 = X500Name.getInstance(cert.getIssuerX500Principal().getEncoded()); + X500Name principal2 = new X500Name(dnUtf8); + + Assert.assertTrue(principal1.equals(principal2)); + } + + @Test + public void bcCanCompareCertAndPlainString() throws Exception + { + X500Name principal1 = X500Name.getInstance(cert.getIssuerX500Principal().getEncoded()); + X500Name principal2 = new X500Name(dnPlain); + + Assert.assertTrue(principal1.equals(principal2)); + } +} diff --git a/src/test/resources/xades4j/verification/issue166/EMPUBqscdA.cer b/src/test/resources/xades4j/verification/issue166/EMPUBqscdA.cer new file mode 100644 index 00000000..852000fb Binary files /dev/null and b/src/test/resources/xades4j/verification/issue166/EMPUBqscdA.cer differ