From 62205d07101fe36609f6f38d6d8422e3ef71f748 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lu=C3=ADs=20Gon=C3=A7alves?= Date: Sat, 19 Oct 2024 13:01:07 +0100 Subject: [PATCH] Support verification of signature protecting the signing certificate in KeyInfo only --- .../java/xades4j/production/Enveloped.java | 8 +- .../CounterSignatureVerifier.java | 30 ++-- .../verification/KeyInfoProcessor.java | 70 +++++---- .../xades4j/verification/SignatureUtils.java | 32 ++++- .../verification/XAdESFormChecker.java | 28 ++-- .../XadesVerificationProfile.java | 23 +++ .../verification/XadesVerifierImpl.java | 134 +++++++++++++----- .../verification/OtherVerifierTests.java | 33 +++++ .../verification/XAdESFormCheckerTest.java | 11 +- ...hout-sign-cert-prop-keyinfo-not-signed.xml | 71 ++++++++++ .../document.bes.without-sign-cert-prop.xml | 78 ++++++++++ 11 files changed, 398 insertions(+), 120 deletions(-) create mode 100644 src/test/xml/bad/document.bes.without-sign-cert-prop-keyinfo-not-signed.xml create mode 100644 src/test/xml/document.bes.without-sign-cert-prop.xml diff --git a/src/main/java/xades4j/production/Enveloped.java b/src/main/java/xades4j/production/Enveloped.java index 3276ad1d..695fc0da 100644 --- a/src/main/java/xades4j/production/Enveloped.java +++ b/src/main/java/xades4j/production/Enveloped.java @@ -48,11 +48,11 @@ public Enveloped(XadesSigner signer) * reference is used. * * @param elementToSign the element that will be signed and will be the signature's parent - * - * @throws XAdES4jException see {@link XadesSigner#sign(xades4j.production.SignedDataObjects, org.w3c.dom.Node)} + * @return the signature result + * @throws XAdES4jException see {@link XadesSigner#sign(xades4j.production.SignedDataObjects, org.w3c.dom.Node)} * @throws IllegalArgumentException if {@code elementToSign} doesn't have an Id and isn't the document root */ - public void sign(Element elementToSign) throws XAdES4jException + public XadesSignatureResult sign(Element elementToSign) throws XAdES4jException { String refUri; if (elementToSign.hasAttribute("Id")) @@ -65,6 +65,6 @@ public void sign(Element elementToSign) throws XAdES4jException } DataObjectDesc dataObjRef = new DataObjectReference(refUri).withTransform(new EnvelopedSignatureTransform()); - signer.sign(new SignedDataObjects(dataObjRef), elementToSign); + return signer.sign(new SignedDataObjects(dataObjRef), elementToSign); } } diff --git a/src/main/java/xades4j/verification/CounterSignatureVerifier.java b/src/main/java/xades4j/verification/CounterSignatureVerifier.java index da1e81eb..4e93cf3d 100644 --- a/src/main/java/xades4j/verification/CounterSignatureVerifier.java +++ b/src/main/java/xades4j/verification/CounterSignatureVerifier.java @@ -18,8 +18,6 @@ import jakarta.inject.Inject; import org.apache.xml.security.exceptions.XMLSecurityException; -import org.apache.xml.security.signature.Reference; -import org.apache.xml.security.signature.SignedInfo; import org.apache.xml.security.utils.Constants; import org.w3c.dom.Element; import org.w3c.dom.Node; @@ -27,11 +25,13 @@ import xades4j.properties.CounterSignatureProperty; import xades4j.properties.QualifyingProperty; import xades4j.properties.data.GenericDOMData; -import xades4j.utils.CanonicalizerUtils; import xades4j.utils.DOMHelper; +import static xades4j.verification.SignatureUtils.signatureReferencesElement; + /** * XAdES section G.2.2.7 + * * @author Luís */ class CounterSignatureVerifier implements QualifyingPropertyVerifier @@ -56,7 +56,8 @@ public QualifyingProperty verify( { Element sigElem = DOMHelper.getFirstChildElement(propData.getPropertyElement()); res = verifier.verify(sigElem, null); - } catch (XAdES4jException ex) + } + catch (XAdES4jException ex) { throw new CounterSignatureXadesVerificationException(ex); } @@ -69,25 +70,14 @@ public QualifyingProperty verify( try { - SignedInfo si = res.getXmlSignature().getSignedInfo(); - for (int i = 0; i < si.getLength(); i++) + if (signatureReferencesElement(res.getXmlSignature(), (Element) targetSigValueElem)) { - Reference r = si.item(i); - if (r.getContentsAfterTransformation().getSubNode() == targetSigValueElem) - { - // The signature references the SignatureValue element. - return new CounterSignatureProperty(res); - } - else if (r.getContentsBeforeTransformation().getSubNode() == targetSigValueElem && CanonicalizerUtils.allTransformsAreC14N(r)) - { - // The signature references the SignatureValue element with - // C14N transforms only. - // TODO: same return value as before - return new CounterSignatureProperty(res); - } + return new CounterSignatureProperty(res); } + throw new CounterSignatureSigValueRefException(); - } catch (XMLSecurityException e) + } + catch (XMLSecurityException e) { // Shouldn't happen because the signature was already verified. throw new CounterSignatureVerificationException(e); diff --git a/src/main/java/xades4j/verification/KeyInfoProcessor.java b/src/main/java/xades4j/verification/KeyInfoProcessor.java index 15f54bc7..b9e262c3 100644 --- a/src/main/java/xades4j/verification/KeyInfoProcessor.java +++ b/src/main/java/xades4j/verification/KeyInfoProcessor.java @@ -16,11 +16,6 @@ */ package xades4j.verification; -import java.security.cert.X509CertSelector; -import java.security.cert.X509Certificate; -import java.util.ArrayList; -import java.util.Collections; -import java.util.List; import org.apache.xml.security.exceptions.XMLSecurityException; import org.apache.xml.security.keys.KeyInfo; import org.apache.xml.security.keys.content.X509Data; @@ -29,8 +24,14 @@ import xades4j.providers.CertificateValidationException; import xades4j.providers.X500NameStyleProvider; +import javax.annotation.Nullable; +import java.security.cert.X509CertSelector; +import java.security.cert.X509Certificate; +import java.util.ArrayList; +import java.util.Collections; +import java.util.List; + /** - * * @author Luís */ class KeyInfoProcessor @@ -42,39 +43,36 @@ private KeyInfoProcessor() static class KeyInfoRes { - final X509CertSelector certSelector; - final List keyInfoCerts; + final X509CertSelector signingCertSelector; + final boolean signingCertSelectorFromKeyInfo; + final List certs; + @Nullable final XMLX509IssuerSerial issuerSerial; - KeyInfoRes( - X509CertSelector certSelector, - List keyInfoCerts, - XMLX509IssuerSerial issuerSerial) + private KeyInfoRes( + X509CertSelector signingCertSelector, + boolean signingCertSelectorFromKeyInfo, + List certs, + @Nullable XMLX509IssuerSerial issuerSerial) { - this.keyInfoCerts = keyInfoCerts; - this.certSelector = certSelector; + this.signingCertSelector = signingCertSelector; + this.signingCertSelectorFromKeyInfo = signingCertSelectorFromKeyInfo; + this.certs = certs; this.issuerSerial = issuerSerial; } - - KeyInfoRes(X509CertSelector certSelector) - { - this.certSelector = certSelector; - this.keyInfoCerts = Collections.emptyList(); - this.issuerSerial = null; - } } static KeyInfoRes process( - KeyInfo keyInfo, CertRef signingCertRef, X500NameStyleProvider x500NameStyleProvider) throws CertificateValidationException + KeyInfo keyInfo, @Nullable CertRef signingCertRef, X500NameStyleProvider x500NameStyleProvider) throws CertificateValidationException { if (null == keyInfo || !keyInfo.containsX509Data()) { return tryUseSigningCertificateReference(signingCertRef, x500NameStyleProvider); } - + List keyInfoCerts = new ArrayList<>(1); XMLX509IssuerSerial issuerSerial = null; - X509CertSelector certSelector = new X509CertSelector(); + X509CertSelector signingCertSelector = new X509CertSelector(); // XML-DSIG 4.4.4: "Any X509IssuerSerial, X509SKI, and X509SubjectName elements // that appear MUST refer to the certificate or certificates containing the @@ -100,13 +98,13 @@ static KeyInfoRes process( if (x509Data.containsIssuerSerial()) { issuerSerial = x509Data.itemIssuerSerial(0); - certSelector.setIssuer(x500NameStyleProvider.fromString(issuerSerial.getIssuerName())); - certSelector.setSerialNumber(issuerSerial.getSerialNumber()); + signingCertSelector.setIssuer(x500NameStyleProvider.fromString(issuerSerial.getIssuerName())); + signingCertSelector.setSerialNumber(issuerSerial.getSerialNumber()); hasSelectionCriteria = true; } else if (x509Data.containsSubjectName()) { - certSelector.setSubject(x500NameStyleProvider.fromString(x509Data.itemSubjectName(0).getSubjectName())); + signingCertSelector.setSubject(x500NameStyleProvider.fromString(x509Data.itemSubjectName(0).getSubjectName())); hasSelectionCriteria = true; } } @@ -121,14 +119,10 @@ else if (x509Data.containsSubjectName()) } } - if (!hasSelectionCriteria) + if (!hasSelectionCriteria && !keyInfoCerts.isEmpty()) { - if (keyInfoCerts.isEmpty()) - { - return tryUseSigningCertificateReference(signingCertRef, x500NameStyleProvider); - } - - certSelector.setCertificate(keyInfoCerts.get(0)); + signingCertSelector.setCertificate(keyInfoCerts.get(0)); + hasSelectionCriteria = true; } } catch (XMLSecurityException ex) @@ -136,7 +130,9 @@ else if (x509Data.containsSubjectName()) throw new InvalidKeyInfoDataException("Cannot process X509Data", ex); } - return new KeyInfoRes(certSelector, keyInfoCerts, issuerSerial); + return hasSelectionCriteria + ? new KeyInfoRes(signingCertSelector, true, keyInfoCerts, issuerSerial) + : tryUseSigningCertificateReference(signingCertRef, x500NameStyleProvider); } private static KeyInfoRes tryUseSigningCertificateReference(CertRef signingCertRef, X500NameStyleProvider x500NameStyleProvider) throws CertificateValidationException @@ -149,7 +145,7 @@ private static KeyInfoRes tryUseSigningCertificateReference(CertRef signingCertR X509CertSelector certSelector = new X509CertSelector(); certSelector.setIssuer(x500NameStyleProvider.fromString(signingCertRef.getIssuerDN())); certSelector.setSerialNumber(signingCertRef.getSerialNumber()); - - return new KeyInfoRes(certSelector); + + return new KeyInfoRes(certSelector, false, Collections.emptyList(), null); } } diff --git a/src/main/java/xades4j/verification/SignatureUtils.java b/src/main/java/xades4j/verification/SignatureUtils.java index f9aa442d..fbf00b08 100644 --- a/src/main/java/xades4j/verification/SignatureUtils.java +++ b/src/main/java/xades4j/verification/SignatureUtils.java @@ -26,6 +26,7 @@ import org.apache.xml.security.signature.XMLSignature; import org.apache.xml.security.signature.XMLSignatureException; import org.apache.xml.security.transforms.Transforms; +import org.apache.xml.security.utils.ElementProxy; import org.w3c.dom.Element; import org.w3c.dom.Node; import xades4j.XAdES4jXMLSigException; @@ -33,21 +34,20 @@ import xades4j.properties.QualifyingProperty; import xades4j.utils.DOMHelper; +import static xades4j.utils.CanonicalizerUtils.allTransformsAreC14N; + /** * * @author Luís */ class SignatureUtils { - private SignatureUtils() { } - /**/ static class ReferencesRes { - /** * In signature order. */ @@ -98,10 +98,9 @@ static ReferencesRes processReferences( } if (null == signedPropsRef) - // !!! - // Still may be a XAdES signature, if the signing certificate is - // protected. For now, that scenario is not supported. { + // TODO Still may be a XAdES signature, if the signing certificate is + // protected. For now, that scenario is not supported. throw new QualifyingPropertiesIncorporationException("SignedProperties reference not found"); } @@ -258,4 +257,25 @@ private static Element getReferencedSignedPropsElem(Reference signedPropsRef) th // The referenced signed properties element must be the child of qualifying properties. return (Element) sPropsNode; } + + static boolean signatureReferencesElement(XMLSignature signature, ElementProxy elementProxy) throws XMLSecurityException + { + return signatureReferencesElement(signature, elementProxy.getElement()); + } + + static boolean signatureReferencesElement(XMLSignature signature, Element element) throws XMLSecurityException + { + SignedInfo si = signature.getSignedInfo(); + for (int i = 0; i < si.getLength(); i++) + { + Reference r = si.item(i); + if (r.getContentsAfterTransformation().getSubNode() == element || + r.getContentsBeforeTransformation().getSubNode() == element && allTransformsAreC14N(r)) + { + return true; + } + } + + return false; + } } diff --git a/src/main/java/xades4j/verification/XAdESFormChecker.java b/src/main/java/xades4j/verification/XAdESFormChecker.java index ed68a21a..d88d8be3 100644 --- a/src/main/java/xades4j/verification/XAdESFormChecker.java +++ b/src/main/java/xades4j/verification/XAdESFormChecker.java @@ -40,7 +40,7 @@ private XAdESFormChecker() { } - static XAdESForm checkForm(Collection props) throws InvalidXAdESFormException + static XAdESForm checkForm(Collection props, boolean requiresSigningCertificateProperty) throws InvalidXAdESFormException { Set availablePropsNames = new HashSet<>(); for (PropertyInfo propInfo : props) @@ -51,7 +51,7 @@ static XAdESForm checkForm(Collection props) throws InvalidXAdESFo XAdESFormDesc formDesc = XADES_C_DESC; do { - if (formDesc.check(availablePropsNames)) + if (formDesc.check(availablePropsNames, requiresSigningCertificateProperty)) return formDesc.getForm(); } while ((formDesc = formDesc.getPrevious()) != null); @@ -84,10 +84,10 @@ public XAdESFormDesc(XAdESFormDesc... baseForms) this.baseForms = baseForms; } - boolean check(Set availablePropsNames) throws InvalidXAdESFormException + boolean check(Set availablePropsNames, boolean requiresSigningCertificateProperty) throws InvalidXAdESFormException { // Check the properties for the current form. - if (!checkProps(availablePropsNames)) + if (!checkProps(availablePropsNames, requiresSigningCertificateProperty)) return false; // If the properties of the current form are available, at least one @@ -97,7 +97,7 @@ boolean check(Set availablePropsNames) throws InvalidXAdESFormException return true; for (XAdESFormDesc baseForm : baseForms) { - if (baseForm.check(availablePropsNames)) + if (baseForm.check(availablePropsNames, requiresSigningCertificateProperty)) return true; } @@ -114,7 +114,7 @@ XAdESFormDesc getPrevious() * when the form is malformed. * @return true if the format specific properties are available; false otherwise */ - protected abstract boolean checkProps(Set availablePropsNames) throws InvalidXAdESFormException; + protected abstract boolean checkProps(Set availablePropsNames, boolean requiresSigningCertificateProperty) throws InvalidXAdESFormException; abstract XAdESForm getForm(); } @@ -123,9 +123,9 @@ XAdESFormDesc getPrevious() static class XAdES_BES_Desc extends XAdESFormDesc { @Override - protected boolean checkProps(Set availablePropsNames) + protected boolean checkProps(Set availablePropsNames, boolean requiresSigningCertificateProperty) { - return availablePropsNames.contains(SigningCertificateProperty.PROP_NAME); + return availablePropsNames.contains(SigningCertificateProperty.PROP_NAME) || !requiresSigningCertificateProperty; } @Override @@ -144,7 +144,7 @@ public XAdES_EPES_Desc() } @Override - protected boolean checkProps(Set availablePropsNames) + protected boolean checkProps(Set availablePropsNames, boolean requiresSigningCertificateProperty) { return availablePropsNames.contains(SignaturePolicyBase.PROP_NAME); } @@ -165,7 +165,7 @@ public XAdES_T_Desc() } @Override - protected boolean checkProps(Set availablePropsNames) + protected boolean checkProps(Set availablePropsNames, boolean requiresSigningCertificateProperty) { return availablePropsNames.contains(SignatureTimeStampProperty.PROP_NAME); } @@ -186,7 +186,7 @@ public XAdES_C_Desc() } @Override - protected boolean checkProps(Set availablePropsNames) throws InvalidXAdESFormException + protected boolean checkProps(Set availablePropsNames, boolean requiresSigningCertificateProperty) throws InvalidXAdESFormException { boolean hasCompCertRefs = availablePropsNames.contains(CompleteCertificateRefsProperty.PROP_NAME); boolean hasCompRevocRefs = availablePropsNames.contains(CompleteRevocationRefsProperty.PROP_NAME); @@ -225,7 +225,7 @@ public XAdES_X_Desc() } @Override - protected boolean checkProps(Set availablePropsNames) { + protected boolean checkProps(Set availablePropsNames, boolean requiresSigningCertificateProperty) { return availablePropsNames.contains(SigAndRefsTimeStampProperty.PROP_NAME) || availablePropsNames.contains("RefsOnlyTimeStamp"); } @@ -246,7 +246,7 @@ public XAdES_X_L_Desc() } @Override - protected boolean checkProps(Set availablePropsNames) throws InvalidXAdESFormException + protected boolean checkProps(Set availablePropsNames, boolean requiresSigningCertificateProperty) throws InvalidXAdESFormException { boolean hasCompCert = availablePropsNames.contains(CertificateValuesProperty.PROP_NAME); boolean hasCompRevoc = availablePropsNames.contains(RevocationValuesProperty.PROP_NAME); @@ -277,7 +277,7 @@ public XAdES_A_Desc() } @Override - protected boolean checkProps(Set availablePropsNames) { + protected boolean checkProps(Set availablePropsNames, boolean requiresSigningCertificateProperty) { return availablePropsNames.contains(ArchiveTimeStampProperty.PROP_NAME); } diff --git a/src/main/java/xades4j/verification/XadesVerificationProfile.java b/src/main/java/xades4j/verification/XadesVerificationProfile.java index bfe34d04..7f35060e 100644 --- a/src/main/java/xades4j/verification/XadesVerificationProfile.java +++ b/src/main/java/xades4j/verification/XadesVerificationProfile.java @@ -67,12 +67,14 @@ public final class XadesVerificationProfile /**/ private boolean acceptUnknownProperties; private boolean secureValidation; + private boolean requireSigningCertificateProperty; private XadesVerificationProfile() { this.profileCore = new XadesProfileCore(); this.acceptUnknownProperties = false; this.secureValidation = false; + this.requireSigningCertificateProperty = true; withBinding(XadesVerifier.class, XadesVerifierImpl.class); } @@ -149,6 +151,7 @@ public XadesVerifier newVerifier() throws XadesProfileResolutionException XadesVerifierImpl v = profileCore.getInstance(XadesVerifierImpl.class, overridableModules, sealedModules); v.setAcceptUnknownProperties(acceptUnknownProperties); v.setSecureValidation(secureValidation); + v.requireSigningCertificateProperty(requireSigningCertificateProperty); return v; } @@ -256,6 +259,26 @@ public XadesVerificationProfile withSecureValidation(boolean secureValidation) return this; } + /** + * Indicates whether the signing certificate must be protected by incorporating the {@code SigningCertificate} + * signed property. Defaults to true. + *

+ * If set to false, then the signing certificate can be protected by incorporating it in the {@code KeyInfo} element + * and signing that element. In this case the signature doesn't need to include the {@code SigningCertificate} + * property (but it may). + *

+ * Either the {@code SigningCertificate} property must be present of the signing certificate must be included and + * protected in the {@code KeyInfo} element for signature verification to succeed. + * + * @param requireSigningCertificateProperty whether the {@code SigningCertificate} property is required + * @return the current instance + */ + public XadesVerificationProfile requireSigningCertificateProperty(boolean requireSigningCertificateProperty) + { + this.requireSigningCertificateProperty = requireSigningCertificateProperty; + return this; + } + /* ******************************************** */ /* *********** Custom verification ************ */ /* ******************************************** */ diff --git a/src/main/java/xades4j/verification/XadesVerifierImpl.java b/src/main/java/xades4j/verification/XadesVerifierImpl.java index ec2fa0ee..3758f2c7 100644 --- a/src/main/java/xades4j/verification/XadesVerifierImpl.java +++ b/src/main/java/xades4j/verification/XadesVerifierImpl.java @@ -17,13 +17,6 @@ package xades4j.verification; import jakarta.inject.Inject; -import java.io.InputStream; -import java.security.cert.X509Certificate; -import java.util.ArrayList; -import java.util.Collection; -import java.util.Date; -import java.util.List; -import java.util.Set; import org.apache.xml.security.exceptions.XMLSecurityException; import org.apache.xml.security.signature.Reference; import org.apache.xml.security.signature.SignedInfo; @@ -56,8 +49,18 @@ import xades4j.xml.unmarshalling.QualifyingPropertiesUnmarshaller; import xades4j.xml.unmarshalling.UnmarshalException; +import javax.annotation.Nullable; +import java.io.InputStream; +import java.security.cert.X509Certificate; +import java.util.ArrayList; +import java.util.Collection; +import java.util.Date; +import java.util.List; +import java.util.Set; + +import static xades4j.verification.SignatureUtils.signatureReferencesElement; + /** - * * @author Luís */ class XadesVerifierImpl implements XadesVerifier @@ -68,6 +71,7 @@ class XadesVerifierImpl implements XadesVerifier org.apache.xml.security.Init.init(); initFormExtension(); } + /**/ private final CertificateValidationProvider certificateValidator; private final QualifyingPropertiesVerifier qualifyingPropertiesVerifier; @@ -76,6 +80,7 @@ class XadesVerifierImpl implements XadesVerifier private final Set customSigVerifiers; private final X500NameStyleProvider x500NameStyleProvider; private boolean secureValidation; + private boolean requireSigningCertificateProperty; @Inject protected XadesVerifierImpl( @@ -99,6 +104,7 @@ protected XadesVerifierImpl( this.customSigVerifiers = customSigVerifiers; this.x500NameStyleProvider = x500NameStyleProvider; this.secureValidation = false; + this.requireSigningCertificateProperty = true; } void setAcceptUnknownProperties(boolean accept) @@ -111,6 +117,11 @@ void setSecureValidation(boolean secureValidation) this.secureValidation = secureValidation; } + void requireSigningCertificateProperty(boolean requireSigningCertificateProperty) + { + this.requireSigningCertificateProperty = requireSigningCertificateProperty; + } + @Override public XAdESVerificationResult verify(Element signatureElem, SignatureSpecificVerificationOptions verificationOptions) throws XAdES4jException { @@ -130,7 +141,8 @@ public XAdESVerificationResult verify(Element signatureElem, SignatureSpecificVe try { signature = new XMLSignature(signatureElem, verificationOptions.getBaseUri(), this.secureValidation); - } catch (XMLSecurityException ex) + } + catch (XMLSecurityException ex) { throw new UnmarshalException("Bad XML signature", ex); } @@ -177,26 +189,16 @@ public XAdESVerificationResult verify(Element signatureElem, SignatureSpecificVe CertRef signingCertRefAttempt = tryGetSigningCertificateRef(qualifPropsData); KeyInfoRes keyInfoRes = KeyInfoProcessor.process(signature.getKeyInfo(), signingCertRefAttempt, this.x500NameStyleProvider); ValidationData certValidationRes = this.certificateValidator.validate( - keyInfoRes.certSelector, + keyInfoRes.signingCertSelector, validationDate, - keyInfoRes.keyInfoCerts); + keyInfoRes.certs); if (null == certValidationRes || certValidationRes.getCerts().isEmpty()) { throw new NullPointerException("Certificate validator returned null or empty data"); } X509Certificate validationCert = certValidationRes.getCerts().get(0); - if (verificationOptions.checkKeyUsage()) - { - // Check key usage. - // - KeyUsage[0] = digitalSignature - // - KeyUsage[1] = nonRepudiation - boolean[] keyUsage = validationCert.getKeyUsage(); - if (keyUsage != null && !keyUsage[0] && !keyUsage[1]) - { - throw new SigningCertificateKeyUsageException(); - } - } + checkKeyUsage(verificationOptions, validationCert); /* Signature verification */ @@ -207,20 +209,22 @@ public XAdESVerificationResult verify(Element signatureElem, SignatureSpecificVe QualifyingPropertyVerificationContext qPropsCtx = new QualifyingPropertyVerificationContext( signature, new QualifyingPropertyVerificationContext.CertificationChainData( - certValidationRes.getCerts(), - certValidationRes.getCrls(), - keyInfoRes.issuerSerial, - this.x500NameStyleProvider), + certValidationRes.getCerts(), + certValidationRes.getCrls(), + keyInfoRes.issuerSerial, + this.x500NameStyleProvider), /**/ new QualifyingPropertyVerificationContext.SignedObjectsData( - referencesRes.dataObjsReferences, - signature)); + referencesRes.dataObjsReferences, + signature)); // Verify the properties. Data structure verification is included. Collection props = this.qualifyingPropertiesVerifier.verifyProperties(qualifPropsData, qPropsCtx); + // Determine the XAdES form by checking which (needed) properties are present. + XAdESForm signatureForm = XAdESFormChecker.checkForm(props, requiresSigningCertificateProperty(signature, keyInfoRes)); XAdESVerificationResult res = new XAdESVerificationResult( - XAdESFormChecker.checkForm(props), + signatureForm, signature, certValidationRes, props, @@ -235,7 +239,8 @@ public XAdESVerificationResult verify(Element signatureElem, SignatureSpecificVe return res; } - private static String getTargetValue(Element qualifyingPropsElem) throws QualifyingPropertiesIncorporationException { + private static String getTargetValue(Element qualifyingPropsElem) throws QualifyingPropertiesIncorporationException + { Node targetAttr = qualifyingPropsElem.getAttributeNodeNS(null, QualifyingProperty.TARGET_ATTR); if (null == targetAttr) { @@ -250,7 +255,9 @@ private static String getTargetValue(Element qualifyingPropsElem) throws Qualify /*************************************************************************************/ - private CertRef tryGetSigningCertificateRef(Collection qualifPropsData){ + @Nullable + private CertRef tryGetSigningCertificateRef(Collection qualifPropsData) + { // If the SigningCertificate property contains a single reference, it likely represents the signer's certificate. // In that case, it can be used as a fallback to identify the signing certificate when processing KeyInfo. // This is a temporary solution. @@ -268,6 +275,21 @@ private CertRef tryGetSigningCertificateRef(Collection quali return null; } + private static void checkKeyUsage(SignatureSpecificVerificationOptions verificationOptions, X509Certificate validationCert) throws SigningCertificateKeyUsageException + { + if (verificationOptions.checkKeyUsage()) + { + // Check key usage. + // - KeyUsage[0] = digitalSignature + // - KeyUsage[1] = nonRepudiation + boolean[] keyUsage = validationCert.getKeyUsage(); + if (keyUsage != null && !keyUsage[0] && !keyUsage[1]) + { + throw new SigningCertificateKeyUsageException(); + } + } + } + private Date getValidationDate( Collection qualifPropsData, XMLSignature signature, @@ -290,25 +312,58 @@ private Date getValidationDate( new QualifyingPropertyVerificationContext.CertificationChainData( new ArrayList<>(0), new ArrayList<>(0), - null, - this.x500NameStyleProvider), + null, + this.x500NameStyleProvider), /**/ new QualifyingPropertyVerificationContext.SignedObjectsData( new ArrayList<>(0), - signature)); + signature)); Collection props = this.qualifyingPropertiesVerifier.verifyProperties((Collection) sigTsData, ctx); QualifyingProperty sigTs = props.iterator().next().getProperty(); return ((SignatureTimeStampProperty) sigTs).getTime(); } + /** + * Determines if the SigningCertificate qualifying property must be in the signature. This depends on whether the + * signing certificate is protected in KeyInfo and what the profile allows. + * Note that the property is not required but is present, it's still validatated as any other property. + */ + private boolean requiresSigningCertificateProperty(XMLSignature signature, KeyInfoRes keyInfoRes) throws XAdES4jException + { + if (this.requireSigningCertificateProperty) + { + // Profile requires protecting the signing certificate using the SigningCertificate property. + return true; + } + + if (!keyInfoRes.signingCertSelectorFromKeyInfo) + { + // KeyInfo does not contain elements that identify the signing certificate, which means the + // SigningCertificate property must be used. This was already checked when processing KeyInfo, anyway. + return true; + } + + try + { + // KeyInfo contains elements that identify the signing certificate. If these elements are not protected by + // the signature, then the SigningCertificate property is required. + // Note that this only supports References to the KeyInfo element itself, containing only C14N transforms. + return !signatureReferencesElement(signature, signature.getKeyInfo()); + } + catch (XMLSecurityException e) + { + throw new XAdES4jXMLSigException("Error looking for KeyInfo reference", e); + } + } + private static void doCoreVerification( XMLSignature signature, SignatureSpecificVerificationOptions verificationOptions, X509Certificate validationCert) throws XAdES4jXMLSigException, InvalidSignatureException { List resolvers = verificationOptions.getResolvers(); - if(!CollectionUtils.nullOrEmpty(resolvers)) + if (!CollectionUtils.nullOrEmpty(resolvers)) { for (ResourceResolverSpi resolver : resolvers) { @@ -345,7 +400,8 @@ private static void doCoreVerification( // itself. { throw new SignatureValueException(signature); - } else + } + else { // References are NOT OK; get the first invalid Reference. SignedInfo si = signature.getSignedInfo(); @@ -370,8 +426,9 @@ private interface FormExtensionPropsCollector { void addProps(Collection usp, - XAdESVerificationResult res); + XAdESVerificationResult res); } + private static FormExtensionPropsCollector[][] formsExtensionTransitions; private static void initFormExtension() @@ -451,7 +508,8 @@ public XAdESVerificationResult verify( return res; } - private static FormExtensionPropsCollector getFinalFormPropsColector(XAdESForm finalForm, XAdESForm actualForm) throws InvalidFormExtensionException { + private static FormExtensionPropsCollector getFinalFormPropsColector(XAdESForm finalForm, XAdESForm actualForm) throws InvalidFormExtensionException + { FormExtensionPropsCollector finalFormPropsCollector = formsExtensionTransitions[actualForm.ordinal()][finalForm.ordinal()]; if (null == finalFormPropsCollector) diff --git a/src/test/java/xades4j/verification/OtherVerifierTests.java b/src/test/java/xades4j/verification/OtherVerifierTests.java index bb75d568..a99dde94 100644 --- a/src/test/java/xades4j/verification/OtherVerifierTests.java +++ b/src/test/java/xades4j/verification/OtherVerifierTests.java @@ -19,6 +19,9 @@ import jakarta.inject.Inject; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.CsvSource; +import org.junit.jupiter.params.provider.ValueSource; import xades4j.properties.QualifyingProperty; import xades4j.properties.data.SigningTimeData; import xades4j.utils.BuiltIn; @@ -88,4 +91,34 @@ void testVerifyWithManifest() throws Exception .followManifests(true); verifySignature("document.signed.bes.manifest.xml", mySigsVerificationProfile, options); } + + @ParameterizedTest + @ValueSource(strings = { + "document.bes.without-sign-cert-prop.xml", + "document.signed.bes.xml" + }) + void verifyWhenSigningCertificatePropertyIsNotRequired(String file) throws Exception + { + mySigsVerificationProfile.requireSigningCertificateProperty(false); + + XAdESVerificationResult result = verifySignature(file, mySigsVerificationProfile); + + assertEquals(XAdESForm.BES, result.getSignatureForm()); + } + + @ParameterizedTest + @CsvSource({ + // Profile requires the SigningCertificate property + "document.bes.without-sign-cert-prop.xml, true", + // Profile requires the SigningCertificate property + "bad/document.bes.without-sign-cert-prop-keyinfo-not-signed.xml, true", + // Profile does not require the SigningCertificate property but KeyInfo is not signed + "bad/document.bes.without-sign-cert-prop-keyinfo-not-signed.xml, false", + }) + void verifyFailsWhenCertificateIsNotProtectedAsRequired(String file, boolean requireSigningCertificateProperty) throws Exception + { + mySigsVerificationProfile.requireSigningCertificateProperty(requireSigningCertificateProperty); + + assertThrows(InvalidXAdESFormException.class, () -> verifySignature(file, mySigsVerificationProfile)); + } } diff --git a/src/test/java/xades4j/verification/XAdESFormCheckerTest.java b/src/test/java/xades4j/verification/XAdESFormCheckerTest.java index 36772361..a948b850 100644 --- a/src/test/java/xades4j/verification/XAdESFormCheckerTest.java +++ b/src/test/java/xades4j/verification/XAdESFormCheckerTest.java @@ -20,6 +20,7 @@ import java.util.ArrayList; +import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertThrows; /** @@ -30,6 +31,14 @@ class XAdESFormCheckerTest @Test void checkFormThrowsIfMinimumPropertiesAreNotPresent() throws InvalidXAdESFormException { - assertThrows(InvalidXAdESFormException.class, () -> XAdESFormChecker.checkForm(new ArrayList(0))); + assertThrows(InvalidXAdESFormException.class, () -> XAdESFormChecker.checkForm(new ArrayList(0), true)); + } + + @Test + void identifiesXadesBesWhenSigningCertificateIsNotPresentAndNotRequired() throws InvalidXAdESFormException + { + XAdESForm form = XAdESFormChecker.checkForm(new ArrayList(0), false); + + assertEquals(XAdESForm.BES, form); } } diff --git a/src/test/xml/bad/document.bes.without-sign-cert-prop-keyinfo-not-signed.xml b/src/test/xml/bad/document.bes.without-sign-cert-prop-keyinfo-not-signed.xml new file mode 100644 index 00000000..ef127a6e --- /dev/null +++ b/src/test/xml/bad/document.bes.without-sign-cert-prop-keyinfo-not-signed.xml @@ -0,0 +1,71 @@ + + + Questions, unanswered + Steve and the flubberblubs + 1989 + + + What do you know? + Steve and the flubberblubs + 2006-10-17-08:31 + + + Who do you know? + Steve and the flubberblubs + 2006-10-17-08:35 + + + When do you know? + Steve and the flubberblubs + 2006-10-17-08:39 + + + Do you know? + Steve and the flubberblubs + 2006-10-17-08:44 + + + + + + + + + + + + +rD/g8soqKz8EiPUBhEWfcQacS0ta4ULHX3dKMEH6ZoQ= + + + + + + +Ajv7C7LU/k3A3RXrR/xPuOTJdUuqGk3InJbEUjW90sA= + + + +lkE6QqT0x6dFSUj7pI04R+lxWEGqcstD4nXftjLw2oEkg2kw7HiOcOYw4Dw+Fo+i32H3BqfkpcWv +FGEJj9mpapdBF7xfW8fEexAV8dnNwwzDAvDIQ6Hg7utxptGMTDHAIeirMgeDXYdGqR0tNe64SPbg +RzEQeDcRFx7FI2vdZ0Q= + + + + +MIICbTCCAdqgAwIBAgIQpkK0uals+ItHxBlpJuypOTAJBgUrDgMCHQUAMD8xCzAJBgNVBAYTAlBU +MQ0wCwYDVQQKEwRJU0VMMQswCQYDVQQLEwJDQzEUMBIGA1UEAxMLSXRlcm1lZGlhdGUwHhcNMTAw +NjI1MTc1ODQ5WhcNMzkxMjMxMjM1OTU5WjBCMQswCQYDVQQGEwJQVDENMAsGA1UEChMESVNFTDEL +MAkGA1UECxMCQ0MxFzAVBgNVBAMTDkx1aXMgR29uY2FsdmVzMIGfMA0GCSqGSIb3DQEBAQUAA4GN +ADCBiQKBgQCpP9acMX69Dbg9ciMLFc5dm1tlpTY9OTNZ/EaCYoGVhh/3+DFgyIbEer6SA24hpREm +AhNG9+Ca0AurDPPgb3aKWFY9pj1WcOctis0VsR0YvzqP+2IGFqKDCd7bXFvv2tI0dEvpdc0oO6PF +Q02xvJG0kxQf44XljOCjUBU43jkJawIDAQABo28wbTBrBgNVHQEEZDBigBBdbbL4pDKLT56PpOpA +/56toTwwOjELMAkGA1UEBhMCUFQxDTALBgNVBAoTBElTRUwxCzAJBgNVBAsTAkNDMQ8wDQYDVQQD +EwZUZXN0Q0GCEN00x9qe7SuWQvpLK0/oay8wCQYFKw4DAh0FAAOBgQBSma8g9dQjiQo4WUljRRuG +yMUVRyCqW/9oRz8+0EoLNR/AhrIlGqdNbqQ1BkncgNNdqMAus5VD34v/EhgrkgWN5fZajMpYsmcR +Ahu4PzJ6hggAlWWMy245JwIYuV0s1Oi39GVTxVNOBIX//AONZlGWO4S2Psb1mqdZ99b/MugsaA== + + + +2024-10-19T12:51:10.089+01:00 + \ No newline at end of file diff --git a/src/test/xml/document.bes.without-sign-cert-prop.xml b/src/test/xml/document.bes.without-sign-cert-prop.xml new file mode 100644 index 00000000..2c630be3 --- /dev/null +++ b/src/test/xml/document.bes.without-sign-cert-prop.xml @@ -0,0 +1,78 @@ + + + Questions, unanswered + Steve and the flubberblubs + 1989 + + + What do you know? + Steve and the flubberblubs + 2006-10-17-08:31 + + + Who do you know? + Steve and the flubberblubs + 2006-10-17-08:35 + + + When do you know? + Steve and the flubberblubs + 2006-10-17-08:39 + + + Do you know? + Steve and the flubberblubs + 2006-10-17-08:44 + + + + + + + + + + + + +rD/g8soqKz8EiPUBhEWfcQacS0ta4ULHX3dKMEH6ZoQ= + + + + + + +xw2XsEQsRxvXQpvJKPdx9mg8MQqeQNgrPxi3Xob9oF0= + + + + + + +QmkQH32FZugiINJieh8DGpeOtEZXL2ySqvGw+YH0ens= + + + +cGcA50YNLtx2u/BxiSlhuLF/UTFngV4K2rPn4/70VK84Hx5FhHox89STU4wm7tfOtMTppaf9t71w +YuMEns9vtQYKu0UXXzmcYXiDnMUbbZ59L7VShcDBKy8U79BZLyuWSpcTztef/3j/ziZ5KAa6l2no +a7Ra5ys98W8nn4F55NE= + + + + +MIICbTCCAdqgAwIBAgIQpkK0uals+ItHxBlpJuypOTAJBgUrDgMCHQUAMD8xCzAJBgNVBAYTAlBU +MQ0wCwYDVQQKEwRJU0VMMQswCQYDVQQLEwJDQzEUMBIGA1UEAxMLSXRlcm1lZGlhdGUwHhcNMTAw +NjI1MTc1ODQ5WhcNMzkxMjMxMjM1OTU5WjBCMQswCQYDVQQGEwJQVDENMAsGA1UEChMESVNFTDEL +MAkGA1UECxMCQ0MxFzAVBgNVBAMTDkx1aXMgR29uY2FsdmVzMIGfMA0GCSqGSIb3DQEBAQUAA4GN +ADCBiQKBgQCpP9acMX69Dbg9ciMLFc5dm1tlpTY9OTNZ/EaCYoGVhh/3+DFgyIbEer6SA24hpREm +AhNG9+Ca0AurDPPgb3aKWFY9pj1WcOctis0VsR0YvzqP+2IGFqKDCd7bXFvv2tI0dEvpdc0oO6PF +Q02xvJG0kxQf44XljOCjUBU43jkJawIDAQABo28wbTBrBgNVHQEEZDBigBBdbbL4pDKLT56PpOpA +/56toTwwOjELMAkGA1UEBhMCUFQxDTALBgNVBAoTBElTRUwxCzAJBgNVBAsTAkNDMQ8wDQYDVQQD +EwZUZXN0Q0GCEN00x9qe7SuWQvpLK0/oay8wCQYFKw4DAh0FAAOBgQBSma8g9dQjiQo4WUljRRuG +yMUVRyCqW/9oRz8+0EoLNR/AhrIlGqdNbqQ1BkncgNNdqMAus5VD34v/EhgrkgWN5fZajMpYsmcR +Ahu4PzJ6hggAlWWMy245JwIYuV0s1Oi39GVTxVNOBIX//AONZlGWO4S2Psb1mqdZ99b/MugsaA== + + + +2024-10-07T12:13:43.366+01:00 + \ No newline at end of file