xmlsecurity/source/helper/xmlsignaturehelper.cxx | 68 ++++++++++++++++++++++-
1 file changed, 67 insertions(+), 1 deletion(-)
New commits:
commit 3e9a700091872480dd085f0928d1d30b7d74cfd7
Author: Michael Stahl <michael.st...@allotropia.de>
AuthorDate: Mon Jan 15 20:55:07 2024 +0100
Commit: Michael Stahl <michael.st...@allotropia.de>
CommitDate: Tue Jan 16 19:10:57 2024 +0100
tdf#105844 xmlsecurity: fix test failure on WNT
Commit 4d6e9d5e155da1dde05233eb87691e2a454162f6 added 2 tests that
always fail on WNT, unfortunately Jenkins doesn't actually run the
tests.
There are 3 certificates involved:
"Xmlsecurity RSA Test Root CA"
"Xmlsecurity Intermediate Root CA"
"Xmlsecurity RSA Test example Alice"
In the signature XML, there are 3 elements that contain or reference
certificates:
1. X509Data - xmlsecurity produces only the signing certificate here
2. xd:SigningCertificate (XAdES) - again only the signing certificate
3. xd:EncapsulatedX509Certificate (XAdES) - xmlsecurity produces the
full certificate chain here
All of these elements *could* contain the full certificate chain, but in
LO-produced XML signatures only 3. does.
The problem is that the function CheckUnitTestStore() that looks up
a certificate in a unit-test-specific CA store via
$LIBO_TEST_CRYPTOAPI_PKCS7 can only handle a root certificate, it does
not recursively retrieve and check a certificate chain.
The SecurityEnvironment_MSCryptImpl::verifyCertificate() already has a
parameter "seqCerts" to pass in the full certificate chain, but due to
the way the data from the XML is processed, it gets passed only the
content of the X509Data element(s), which, for LO-produced signatures,
do not contain the full certificate chain.
Instead of improving the unit-test-specific function, let's try to get
all the certificates out of the XML signature, and then pass them to
verifyCertificate().
Of course this requires some consistency checks so that the verification
can't be fooled by different certificates in different XML elements.
Change-Id: I8ca541887ceac2dfb6af5d96a5565cfa58d7f682
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/162170
Tested-by: Jenkins
Reviewed-by: Michael Stahl <michael.st...@allotropia.de>
diff --git a/xmlsecurity/source/helper/xmlsignaturehelper.cxx
b/xmlsecurity/source/helper/xmlsignaturehelper.cxx
index 0b5825b125a1..3b13f79f33f1 100644
--- a/xmlsecurity/source/helper/xmlsignaturehelper.cxx
+++ b/xmlsecurity/source/helper/xmlsignaturehelper.cxx
@@ -22,6 +22,7 @@
#include <documentsignaturehelper.hxx>
#include <xsecctl.hxx>
#include <biginteger.hxx>
+#include <certificate.hxx>
#include <UriBindingHelper.hxx>
@@ -702,7 +703,72 @@ XMLSignatureHelper::CheckAndUpdateSignatureInformation(
}
if (CheckX509Data(xSecEnv, temp, certs, tempResult))
{
- datas.emplace_back(tempResult);
+ if (rInfo.maEncapsulatedX509Certificates.empty()) // optional, XAdES
+ {
+ datas.emplace_back(tempResult);
+ }
+ else
+ {
+ // check for consistency between X509Data and
EncapsulatedX509Certificate
+ // (LO produces just the signing certificate in X509Data and
+ // the entire chain in EncapsulatedX509Certificate so in this case
+ // using EncapsulatedX509Certificate yields additional intermediate
+ // certificates that may help in verifying)
+ std::vector<SignatureInformation::X509CertInfo>
encapsulatedCertInfos;
+ for (OUString const& it : rInfo.maEncapsulatedX509Certificates)
+ {
+ encapsulatedCertInfos.emplace_back();
+ encapsulatedCertInfos.back().X509Certificate = it;
+ }
+ std::vector<uno::Reference<security::XCertificate>>
encapsulatedCerts;
+ SignatureInformation::X509Data encapsulatedResult;
+ if (CheckX509Data(xSecEnv, encapsulatedCertInfos,
encapsulatedCerts, encapsulatedResult))
+ {
+ auto const
pXCertificate(dynamic_cast<xmlsecurity::Certificate*>(certs.back().get()));
+ auto const
pECertificate(dynamic_cast<xmlsecurity::Certificate*>(encapsulatedCerts.back().get()));
+ assert(pXCertificate && pECertificate); // was just created by
CheckX509Data
+ if (pXCertificate->getSHA256Thumbprint() ==
pECertificate->getSHA256Thumbprint())
+ {
+ // both are chains - take the longer one
+ if (encapsulatedCerts.size() < certs.size())
+ {
+ datas.emplace_back(tempResult);
+ }
+ else
+ {
+#if 0
+ // extra info needed in testSigningMultipleTimes_ODT
+ // ... but with it, it fails with BROKEN signature?
+ // fails even on the first signature, because somehow
+ // the xd:SigningCertificate element was signed
+ // containing only one certificate, but in the final
+ // file it contains all 3 certificates due to this
here.
+ for (size_t i = 0; i < encapsulatedResult.size(); ++i)
+ {
+ encapsulatedResult[i].X509IssuerName =
encapsulatedCerts[i]->getIssuerName();
+ encapsulatedResult[i].X509SerialNumber =
xmlsecurity::bigIntegerToNumericString(encapsulatedCerts[i]->getSerialNumber());
+ encapsulatedResult[i].X509Subject =
encapsulatedCerts[i]->getSubjectName();
+ auto const
pCertificate(dynamic_cast<xmlsecurity::Certificate*>(encapsulatedCerts[i].get()));
+ assert(pCertificate); // this was just created by
CheckX509Data
+ OUStringBuffer aBuffer;
+ comphelper::Base64::encode(aBuffer,
pCertificate->getSHA256Thumbprint());
+ encapsulatedResult[i].CertDigest =
aBuffer.makeStringAndClear();
+ }
+ datas.emplace_back(encapsulatedResult);
+#else
+ // keep the X509Data stuff in datas but return the
+ // longer EncapsulatedX509Certificate chain
+ datas.emplace_back(tempResult);
+#endif
+ certs = encapsulatedCerts; // overwrite this seems
easier
+ }
+ }
+ else
+ {
+ SAL_WARN("xmlsecurity.comp", "X509Data and
EncapsulatedX509Certificate contain different certificates");
+ }
+ }
+ }
}
// rInfo is a copy, update the original
