diff --git a/src/main/java/xades4j/production/BasicSignatureOptions.java b/src/main/java/xades4j/production/BasicSignatureOptions.java index 425f37a5..2d68759d 100644 --- a/src/main/java/xades4j/production/BasicSignatureOptions.java +++ b/src/main/java/xades4j/production/BasicSignatureOptions.java @@ -1,16 +1,16 @@ /* * 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 . */ @@ -20,9 +20,8 @@ * Configuration of basic signature options such as whether {@code ds:KeyInfo} * elements should be included. * - * @see XadesSigningProfile#withBasicSignatureOptions(BasicSignatureOptions) - * * @author luis + * @see XadesSigningProfile#withBasicSignatureOptions(BasicSignatureOptions) */ public final class BasicSignatureOptions { @@ -32,7 +31,8 @@ public final class BasicSignatureOptions private boolean includeSubjectName = false; private boolean includeIssuerSerial = false; private boolean includePublicKey = false; - private boolean signKeyInfo = false ; + private boolean signKeyInfo = false; + private boolean omitSigningCertificateProperty = false; /** * Configures whether to check that the keyUsage of the signing certificate @@ -77,6 +77,7 @@ boolean checkCertificateValidity() /** * Configures whether the signing certificate / chain should be included in {@code ds:KeyInfo}. * Defauls to {@link SigningCertificateMode#SIGNING_CERTIFICATE }. + * * @param includeSigningCertificateMode the include mode * @return the current instance */ @@ -85,7 +86,7 @@ public BasicSignatureOptions includeSigningCertificate(SigningCertificateMode in this.includeSigningCertificateMode = includeSigningCertificateMode; return this; } - + SigningCertificateMode includeSigningCertificate() { return this.includeSigningCertificateMode; @@ -94,6 +95,7 @@ SigningCertificateMode includeSigningCertificate() /** * Configures whether the subject name should be included in {@code ds:KeyInfo}. * Defaults to false. + * * @param includeSubjectName {@code true} if the subject name should be included; false otherwise * @return the current instance */ @@ -111,6 +113,7 @@ boolean includeSubjectName() /** * Configures whether the issuer/serial should be included in {@code ds:KeyInfo}. * Defaults to false. + * * @param includeIssuerSerial {@code true} if the issuer/serial should be included; false otherwise * @return the current instance */ @@ -119,7 +122,7 @@ public BasicSignatureOptions includeIssuerSerial(boolean includeIssuerSerial) this.includeIssuerSerial = includeIssuerSerial; return this; } - + boolean includeIssuerSerial() { return this.includeIssuerSerial; @@ -129,6 +132,7 @@ boolean includeIssuerSerial() * Configures whether a {@code ds:KeyValue} element containing the public key's * value should be included in {@code ds:KeyInfo}. * Defaults to false. + * * @param includePublicKey {@code true} if the public key should be included; false otherwise * @return the current instance */ @@ -137,11 +141,12 @@ public BasicSignatureOptions includePublicKey(boolean includePublicKey) this.includePublicKey = includePublicKey; return this; } - + boolean includePublicKey() { return this.includePublicKey; } + /** * Configures whether the signature should cover the {@code ds:KeyInfo} element. * Defaults to false. @@ -159,4 +164,37 @@ boolean signKeyInfo() { return this.signKeyInfo; } + + /** + * Configures whether the {@code SigningCertificate} qualifying property is omitted from the signature. + * Defaults to false, i.e. the property is included. + *

+ * Note that XAdES mandates that either the {@code SigningCertificate} property is included or the signing + * certificate is incorporated within {@code ds:KeyInfo} and at least the signing certificate is signed. + * Enabling this setting is only allowed when {@link #signKeyInfo(boolean)} is enabled and + * {@link #includeSigningCertificate(SigningCertificateMode)} or {@link #includeIssuerSerial(boolean)} are + * also enabled. + * + * @param omitSigningCertificateProperty {@code true} if the property should be omitted; false otherwise + * @return + */ + public BasicSignatureOptions omitSigningCertificateProperty(boolean omitSigningCertificateProperty) + { + this.omitSigningCertificateProperty = omitSigningCertificateProperty; + return this; + } + + boolean omitSigningCertificateProperty() + { + return this.omitSigningCertificateProperty; + } + + void ensureValid() throws KeyingDataException + { + if (this.omitSigningCertificateProperty && + (!this.signKeyInfo || !this.includeIssuerSerial && this.includeSigningCertificateMode == SigningCertificateMode.NONE)) + { + throw new KeyingDataException("Signing certificate must be included in KeyInfo and KeyInfo must be signed"); + } + } } diff --git a/src/main/java/xades4j/production/SignerBES.java b/src/main/java/xades4j/production/SignerBES.java index c622aafa..2ccc23cd 100644 --- a/src/main/java/xades4j/production/SignerBES.java +++ b/src/main/java/xades4j/production/SignerBES.java @@ -19,12 +19,14 @@ import static xades4j.utils.CanonicalizerUtils.checkC14NAlgorithm; import jakarta.inject.Inject; + import java.security.PrivateKey; import java.security.cert.X509Certificate; import java.util.ArrayList; import java.util.Collection; import java.util.List; import java.util.UUID; + import org.apache.xml.security.exceptions.XMLSecurityException; import org.apache.xml.security.signature.Manifest; import org.apache.xml.security.signature.ObjectContainer; @@ -63,6 +65,7 @@ /** * Base logic for producing XAdES signatures (XAdES-BES). + * * @author Luís */ class SignerBES implements XadesSigner @@ -72,9 +75,11 @@ class SignerBES implements XadesSigner { Init.initXMLSec(); } + /**/ private final KeyingDataProvider keyingProvider; private final SignatureAlgorithms signatureAlgorithms; + private final BasicSignatureOptions basicSignatureOptions; private final SignedDataObjectsProcessor dataObjectDescsProcessor; private final PropertiesDataObjectsGenerator propsDataObjectsGenerator; private final SignedPropertiesMarshaller signedPropsMarshaller; @@ -109,6 +114,7 @@ protected SignerBES( this.keyingProvider = keyingProvider; this.signatureAlgorithms = signatureAlgorithms; + this.basicSignatureOptions = basicSignatureOptions; this.propsDataObjectsGenerator = propsDataObjectsGenerator; this.signedPropsMarshaller = signedPropsMarshaller; this.unsignedPropsMarshaller = unsignedPropsMarshaller; @@ -145,6 +151,8 @@ public final XadesSignatureResult sign( throw new IllegalArgumentException("Data objects list is empty"); } + this.basicSignatureOptions.ensureValid(); + Document signatureDocument = DOMHelper.getOwnerDocument(referenceNode); // Generate unique identifiers for the Signature and the SignedProperties. @@ -174,7 +182,7 @@ public final XadesSignatureResult sign( SignedDataObjectsProcessor.Result signedDataObjectsResult = this.dataObjectDescsProcessor.process( signedDataObjects, signature); - + /* ds:KeyInfo */ this.keyInfoBuilder.buildKeyInfo(signingCertificateChain, signature); @@ -191,7 +199,8 @@ public final XadesSignatureResult sign( try { signature.appendObject(qPropsXmlObj); - } catch (XMLSignatureException ex) + } + catch (XMLSignatureException ex) { // -> xmlSignature.appendObject(xmlObj): not thrown when signing. throw new IllegalStateException(ex); @@ -251,7 +260,8 @@ public final XadesSignatureResult sign( Transforms transforms = TransformUtils.createTransforms(canonAlg, this.algorithmsParametersMarshaller, signatureDocument); signature.addDocument('#' + signedPropsId, transforms, digestAlgUri, null, QualifyingProperty.SIGNED_PROPS_TYPE_URI); - } catch (XMLSignatureException ex) + } + catch (XMLSignatureException ex) { // Seems to be thrown when the digest algorithm is not supported. In // this case, if it wasn't thrown when processing the data objects it @@ -296,7 +306,8 @@ public final XadesSignatureResult sign( return new XadesSignatureResult(signature, qualifProps); } - private String getDigestAlgUri() { + private String getDigestAlgUri() + { String digestAlgUri = this.signatureAlgorithms.getDigestAlgorithmForDataObjectReferences(); if (StringUtils.isNullOrEmptyString(digestAlgUri)) { @@ -320,7 +331,8 @@ private XMLSignature createSignature(Document signatureDocument, String baseUri, try { return new XMLSignature(signatureDocument, baseUri, signatureAlgElem, canonAlgElem); - } catch (XMLSecurityException ex) + } + catch (XMLSecurityException ex) { // Following the code, doesn't seem to be thrown at all. throw new XAdES4jXMLSigException(ex.getMessage(), ex); @@ -351,7 +363,8 @@ private static void digestManifests(Iterable manifests) throws XAdES4j { m.generateDigestValues(); } - } catch (XMLSignatureException ex) + } + catch (XMLSignatureException ex) { throw new XAdES4jXMLSigException("Error digesting manifest", ex); } @@ -364,8 +377,12 @@ private static void digestManifests(Iterable manifests) throws XAdES4j protected void getFormatSpecificSignatureProperties( Collection formatSpecificSignedSigProps, Collection formatSpecificUnsignedSigProps, - List signingCertificateChain) throws ValidationDataException { - SigningCertificateProperty scp = new SigningCertificateProperty(signingCertificateChain); - formatSpecificSignedSigProps.add(scp); + List signingCertificateChain) throws ValidationDataException + { + if (!this.basicSignatureOptions.omitSigningCertificateProperty()) + { + SigningCertificateProperty scp = new SigningCertificateProperty(signingCertificateChain); + formatSpecificSignedSigProps.add(scp); + } } } diff --git a/src/test/java/xades4j/production/KeyInfoBuilderTest.java b/src/test/java/xades4j/production/KeyInfoBuilderTest.java index 7e7d91d2..ea16072c 100644 --- a/src/test/java/xades4j/production/KeyInfoBuilderTest.java +++ b/src/test/java/xades4j/production/KeyInfoBuilderTest.java @@ -60,11 +60,9 @@ public static void setUpClass() throws Exception @Test void testIncludeCertAndKey() throws Exception { - KeyInfoBuilder keyInfoBuilder = new KeyInfoBuilder( - new BasicSignatureOptions().includeSigningCertificate(SigningCertificateMode.SIGNING_CERTIFICATE).includePublicKey(true), - new SignatureAlgorithms(), - new TestAlgorithmsParametersMarshallingProvider(), - new DefaultX500NameStyleProvider()); + KeyInfoBuilder keyInfoBuilder = createKeyInfoBuilder(new BasicSignatureOptions() + .includeSigningCertificate(SigningCertificateMode.SIGNING_CERTIFICATE) + .includePublicKey(true)); XMLSignature xmlSignature = getTestSignature(); keyInfoBuilder.buildKeyInfo(certificates, xmlSignature); @@ -74,6 +72,9 @@ void testIncludeCertAndKey() throws Exception KeyValue kv = xmlSignature.getKeyInfo().itemKeyValue(0); assertTrue(kv.getPublicKey().getAlgorithm().startsWith("RSA")); + assertEquals(1, xmlSignature.getKeyInfo().lengthX509Data()); + assertEquals(1, xmlSignature.getKeyInfo().itemX509Data(0).lengthCertificate()); + XMLX509Certificate x509Certificate = xmlSignature.getKeyInfo().itemX509Data(0).itemCertificate(0); assertEquals(testCertificate, x509Certificate.getX509Certificate()); } @@ -81,11 +82,8 @@ void testIncludeCertAndKey() throws Exception @Test void testIncludeCertChain() throws Exception { - KeyInfoBuilder keyInfoBuilder = new KeyInfoBuilder( - new BasicSignatureOptions().includeSigningCertificate(SigningCertificateMode.FULL_CHAIN), - new SignatureAlgorithms(), - new TestAlgorithmsParametersMarshallingProvider(), - new DefaultX500NameStyleProvider()); + KeyInfoBuilder keyInfoBuilder = createKeyInfoBuilder(new BasicSignatureOptions() + .includeSigningCertificate(SigningCertificateMode.FULL_CHAIN)); XMLSignature xmlSignature = getTestSignature(); keyInfoBuilder.buildKeyInfo(certificates, xmlSignature); @@ -105,11 +103,8 @@ void testIncludeCertChain() throws Exception @Test void testIncludeIssuerSerial() throws Exception { - KeyInfoBuilder keyInfoBuilder = new KeyInfoBuilder( - new BasicSignatureOptions().includeIssuerSerial(true), - new SignatureAlgorithms(), - new TestAlgorithmsParametersMarshallingProvider(), - new DefaultX500NameStyleProvider()); + KeyInfoBuilder keyInfoBuilder = createKeyInfoBuilder(new BasicSignatureOptions() + .includeIssuerSerial(true)); XMLSignature xmlSignature = getTestSignature(); keyInfoBuilder.buildKeyInfo(certificates, xmlSignature); @@ -121,11 +116,8 @@ void testIncludeIssuerSerial() throws Exception @Test void testIncludeSubjectName() throws Exception { - KeyInfoBuilder keyInfoBuilder = new KeyInfoBuilder( - new BasicSignatureOptions().includeSubjectName(true), - new SignatureAlgorithms(), - new TestAlgorithmsParametersMarshallingProvider(), - new DefaultX500NameStyleProvider()); + KeyInfoBuilder keyInfoBuilder = createKeyInfoBuilder(new BasicSignatureOptions() + .includeSubjectName(true)); XMLSignature xmlSignature = getTestSignature(); keyInfoBuilder.buildKeyInfo(certificates, xmlSignature); @@ -137,11 +129,8 @@ void testIncludeSubjectName() throws Exception @Test void testSignKeyInfo() throws Exception { - KeyInfoBuilder keyInfoBuilder = new KeyInfoBuilder( - new BasicSignatureOptions().signKeyInfo(true), - new SignatureAlgorithms(), - new TestAlgorithmsParametersMarshallingProvider(), - new DefaultX500NameStyleProvider()); + KeyInfoBuilder keyInfoBuilder = createKeyInfoBuilder(new BasicSignatureOptions() + .signKeyInfo(true)); XMLSignature xmlSignature = getTestSignature(); keyInfoBuilder.buildKeyInfo(certificates, xmlSignature); @@ -153,6 +142,15 @@ void testSignKeyInfo() throws Exception assertSame(xmlSignature.getKeyInfo().getElement(), refNode); } + private static KeyInfoBuilder createKeyInfoBuilder(BasicSignatureOptions bso) + { + return new KeyInfoBuilder( + bso, + new SignatureAlgorithms(), + new TestAlgorithmsParametersMarshallingProvider(), + new DefaultX500NameStyleProvider()); + } + private XMLSignature getTestSignature() throws Exception { Document doc = getNewDocument(); diff --git a/src/test/java/xades4j/production/OtherSignerTests.java b/src/test/java/xades4j/production/OtherSignerTests.java index 69e655ac..8d5139a9 100644 --- a/src/test/java/xades4j/production/OtherSignerTests.java +++ b/src/test/java/xades4j/production/OtherSignerTests.java @@ -24,18 +24,24 @@ import static org.apache.xml.security.utils.Constants.SignatureSpecNS; import static org.apache.xml.security.utils.Constants._TAG_SIGNATURE; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertThrows; import java.util.Iterator; +import java.util.stream.Stream; import javax.xml.namespace.NamespaceContext; import javax.xml.xpath.XPath; import javax.xml.xpath.XPathConstants; import javax.xml.xpath.XPathFactory; + import org.apache.xml.security.signature.XMLSignatureByteInput; import org.apache.xml.security.signature.XMLSignatureInput; import org.apache.xml.security.utils.resolver.ResourceResolverContext; import org.apache.xml.security.utils.resolver.ResourceResolverException; import org.apache.xml.security.utils.resolver.ResourceResolverSpi; import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.MethodSource; import org.w3c.dom.Attr; import org.w3c.dom.Document; import org.w3c.dom.Element; @@ -43,6 +49,7 @@ import xades4j.algorithms.ExclusiveCanonicalXMLWithoutComments; import xades4j.properties.DataObjectDesc; import xades4j.properties.QualifyingProperty; +import xades4j.properties.SigningCertificateProperty; import xades4j.providers.ValidationDataProvider; import xades4j.providers.impl.ValidationDataFromCertValidationProvider; import xades4j.verification.VerifierTestBase; @@ -184,4 +191,86 @@ public Iterator getPrefixes(String namespaceURI) return null; } } + + @ParameterizedTest + @MethodSource + void failsIfKeyInfoIsNotSignedAndSigningCertificatePropertyIsOmitted(BasicSignatureOptions options) throws Exception + { + Document doc = getTestDocument(); + Element root = doc.getDocumentElement(); + + XadesSigner signer = new XadesBesSigningProfile(keyingProviderMy) + .withBasicSignatureOptions(options) + .newSigner(); + + SignedDataObjects dataObjs = new SignedDataObjects(new AnonymousDataObjectReference("foo".getBytes())); + + KeyingDataException ex = assertThrows(KeyingDataException.class, () -> { + signer.sign(dataObjs, root); + }); + + assertEquals("Signing certificate must be included in KeyInfo and KeyInfo must be signed", ex.getMessage()); + } + + static Stream failsIfKeyInfoIsNotSignedAndSigningCertificatePropertyIsOmitted() + { + return Stream.of( + new BasicSignatureOptions() + .omitSigningCertificateProperty(true) + .signKeyInfo(false) + .includeSigningCertificate(SigningCertificateMode.NONE) + .includeIssuerSerial(false), + new BasicSignatureOptions() + .omitSigningCertificateProperty(true) + .signKeyInfo(false) + .includeSigningCertificate(SigningCertificateMode.SIGNING_CERTIFICATE) + .includeIssuerSerial(true), + new BasicSignatureOptions() + .omitSigningCertificateProperty(true) + .signKeyInfo(true) + .includeSigningCertificate(SigningCertificateMode.NONE) + .includeIssuerSerial(false) + ); + } + + @ParameterizedTest + @MethodSource + void signingCertificatePropertyCanBeOmittedIfKeyInfoIsSigned(BasicSignatureOptions options) throws Exception + { + Document doc = getTestDocument(); + Element root = doc.getDocumentElement(); + + XadesSigner signer = new XadesBesSigningProfile(keyingProviderMy) + .withBasicSignatureOptions(options) + .newSigner(); + + SignedDataObjects dataObjs = new SignedDataObjects(new AnonymousDataObjectReference("foo".getBytes())); + + XadesSignatureResult result = signer.sign(dataObjs, root); + + assertFalse( + result.getQualifyingProperties().getSignedProperties().getSigProps().stream() + .anyMatch(it -> SigningCertificateProperty.class.equals(it.getClass())) + ); + + assertEquals(0, + doc.getElementsByTagNameNS(QualifyingProperty.XADES_XMLNS, SigningCertificateProperty.PROP_NAME).getLength() + ); + } + + static Stream signingCertificatePropertyCanBeOmittedIfKeyInfoIsSigned() + { + return Stream.of( + new BasicSignatureOptions() + .omitSigningCertificateProperty(true) + .signKeyInfo(true) + .includeSigningCertificate(SigningCertificateMode.SIGNING_CERTIFICATE) + .includeIssuerSerial(false), + new BasicSignatureOptions() + .omitSigningCertificateProperty(true) + .signKeyInfo(true) + .includeSigningCertificate(SigningCertificateMode.NONE) + .includeIssuerSerial(true) + ); + } } diff --git a/src/test/java/xades4j/production/SignerBESTest.java b/src/test/java/xades4j/production/SignerBESTest.java index 997f97e0..d00922a0 100644 --- a/src/test/java/xades4j/production/SignerBESTest.java +++ b/src/test/java/xades4j/production/SignerBESTest.java @@ -31,10 +31,15 @@ import xades4j.properties.DataObjectDesc; import xades4j.properties.DataObjectFormatProperty; import xades4j.properties.IndividualDataObjsTimeStampProperty; +import xades4j.properties.QualifyingProperty; import xades4j.properties.SignerRoleProperty; +import xades4j.properties.SigningCertificateProperty; import java.io.File; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertTrue; + /** * @author Luís */ @@ -60,7 +65,15 @@ void testSignBES() throws Exception DataObjectDesc obj2 = new EnvelopedXmlObject(objectContent, "text/xml", null).withDataObjectFormat(new DataObjectFormatProperty("text/xml", "MyEncoding").withDescription("Isto é uma descrição do elemento dentro do object").withDocumentationUri("http://doc3.txt").withDocumentationUri("http://doc4.txt").withIdentifier("http://elem.in.object")).withCommitmentType(commitment).withDataObjectTimeStamp(dataObjsTimeStamp); SignedDataObjects dataObjs = new SignedDataObjects(obj1, obj2).withCommitmentType(globalCommitment).withDataObjectsTimeStamp(); - signer.sign(dataObjs, elemToSign); + XadesSignatureResult result = signer.sign(dataObjs, elemToSign); + + assertTrue( + result.getQualifyingProperties().getSignedProperties().getSigProps().stream() + .anyMatch(it -> SigningCertificateProperty.class.equals(it.getClass()))); + + assertEquals(1, + doc1.getElementsByTagNameNS(QualifyingProperty.XADES_XMLNS, SigningCertificateProperty.PROP_NAME).getLength() + ); outputDocument(doc1, "document.signed.bes.xml"); }