diff options
-rw-r--r-- | xmlsecurity/CppunitTest_xmlsecurity_signing.mk | 1 | ||||
-rw-r--r-- | xmlsecurity/inc/biginteger.hxx | 11 | ||||
-rw-r--r-- | xmlsecurity/qa/unit/signing/signing.cxx | 18 | ||||
-rw-r--r-- | xmlsecurity/source/component/documentdigitalsignatures.cxx | 2 | ||||
-rw-r--r-- | xmlsecurity/source/helper/xmlsignaturehelper.cxx | 6 | ||||
-rw-r--r-- | xmlsecurity/source/helper/xsecverify.cxx | 6 | ||||
-rw-r--r-- | xmlsecurity/source/xmlsec/mscrypt/x509certificate_mscryptimpl.cxx | 92 | ||||
-rw-r--r-- | xmlsecurity/source/xmlsec/nss/x509certificate_nssimpl.cxx | 95 |
8 files changed, 211 insertions, 20 deletions
diff --git a/xmlsecurity/CppunitTest_xmlsecurity_signing.mk b/xmlsecurity/CppunitTest_xmlsecurity_signing.mk index 4b3c4787ea73..030becd36a37 100644 --- a/xmlsecurity/CppunitTest_xmlsecurity_signing.mk +++ b/xmlsecurity/CppunitTest_xmlsecurity_signing.mk @@ -27,6 +27,7 @@ $(eval $(call gb_CppunitTest_use_libraries,xmlsecurity_signing, \ unotest \ utl \ xmlsecurity \ + xsec_xmlsec \ )) $(eval $(call gb_CppunitTest_use_externals,xmlsecurity_signing,\ diff --git a/xmlsecurity/inc/biginteger.hxx b/xmlsecurity/inc/biginteger.hxx index 8b4d8a9143b5..1e6b3f4a876e 100644 --- a/xmlsecurity/inc/biginteger.hxx +++ b/xmlsecurity/inc/biginteger.hxx @@ -32,8 +32,17 @@ namespace xmlsecurity XSECXMLSEC_DLLPUBLIC OUString bigIntegerToNumericString( const css::uno::Sequence< sal_Int8 >& serial ); XSECXMLSEC_DLLPUBLIC css::uno::Sequence< sal_Int8 > numericStringToBigInteger ( const OUString& serialNumber ); +// DNs read as strings from XML files may need to be mangled for compatibility +// as NSS and MS CryptoAPI have different string serialisations; if the DN is +// from an XCertificate it's "native" already and doesn't need to be mangled. +enum EqualMode +{ + NOCOMPAT, + COMPAT_2ND, + COMPAT_BOTH +}; XSECXMLSEC_DLLPUBLIC bool EqualDistinguishedNames(OUString const& rName1, - OUString const& rName2); + OUString const& rName2, EqualMode eMode); } #endif diff --git a/xmlsecurity/qa/unit/signing/signing.cxx b/xmlsecurity/qa/unit/signing/signing.cxx index 781c555fbd3f..85aacdd12cf3 100644 --- a/xmlsecurity/qa/unit/signing/signing.cxx +++ b/xmlsecurity/qa/unit/signing/signing.cxx @@ -49,6 +49,7 @@ #include <documentsignaturehelper.hxx> #include <xmlsignaturehelper.hxx> #include <documentsignaturemanager.hxx> +#include <biginteger.hxx> #include <certificate.hxx> #include <sfx2/docfile.hxx> #include <sfx2/docfilt.hxx> @@ -91,6 +92,7 @@ public: void testODFTripleX509Data(); void testODFMacroDoubleX509Data(); void testODFDoubleX509Certificate(); + void testDNCompatibility(); /// Test a typical OOXML where a number of (but not all) streams are signed. void testOOXMLPartial(); /// Test a typical broken OOXML signature where one stream is corrupted. @@ -154,6 +156,7 @@ public: CPPUNIT_TEST(testODFTripleX509Data); CPPUNIT_TEST(testODFMacroDoubleX509Data); CPPUNIT_TEST(testODFDoubleX509Certificate); + CPPUNIT_TEST(testDNCompatibility); CPPUNIT_TEST(testOOXMLPartial); CPPUNIT_TEST(testOOXMLBroken); CPPUNIT_TEST(testOOXMLDescription); @@ -752,6 +755,21 @@ void SigningTest::testODFDoubleX509Certificate() CPPUNIT_ASSERT(!infos[0].Signer.is()); } +void SigningTest::testDNCompatibility() +{ + OUString const msDN("CN=\"\"\"ABC\"\".\", O=\"Enterprise \"\"ABC\"\"\""); + OUString const nssDN("CN=\\\"ABC\\\".,O=Enterprise \\\"ABC\\\""); + // this is just the status quo, possibly either NSS or CryptoAPI might change + CPPUNIT_ASSERT(!xmlsecurity::EqualDistinguishedNames(msDN, nssDN, xmlsecurity::NOCOMPAT)); + CPPUNIT_ASSERT(!xmlsecurity::EqualDistinguishedNames(nssDN, msDN, xmlsecurity::NOCOMPAT)); + // with compat flag it should work, with the string one 2nd and the native one 1st +#ifdef _WIN32 + CPPUNIT_ASSERT(xmlsecurity::EqualDistinguishedNames(msDN, nssDN, xmlsecurity::COMPAT_2ND)); +#else + CPPUNIT_ASSERT(xmlsecurity::EqualDistinguishedNames(nssDN, msDN, xmlsecurity::COMPAT_2ND)); +#endif +} + void SigningTest::testOOXMLPartial() { createDoc(m_directories.getURLFromSrc(DATA_DIRECTORY) + "partial.docx"); diff --git a/xmlsecurity/source/component/documentdigitalsignatures.cxx b/xmlsecurity/source/component/documentdigitalsignatures.cxx index bedc07e261df..dc8d93ec8ec7 100644 --- a/xmlsecurity/source/component/documentdigitalsignatures.cxx +++ b/xmlsecurity/source/component/documentdigitalsignatures.cxx @@ -616,7 +616,7 @@ sal_Bool DocumentDigitalSignatures::isAuthorTrusted( for ( ; pAuthors != pAuthorsEnd; ++pAuthors ) { SvtSecurityOptions::Certificate aAuthor = *pAuthors; - if (xmlsecurity::EqualDistinguishedNames(aAuthor[0], xAuthor->getIssuerName()) + if (xmlsecurity::EqualDistinguishedNames(aAuthor[0], xAuthor->getIssuerName(), xmlsecurity::NOCOMPAT) && (aAuthor[1] == sSerialNum)) { bFound = true; diff --git a/xmlsecurity/source/helper/xmlsignaturehelper.cxx b/xmlsecurity/source/helper/xmlsignaturehelper.cxx index 86982f03dacf..538d4c696760 100644 --- a/xmlsecurity/source/helper/xmlsignaturehelper.cxx +++ b/xmlsecurity/source/helper/xmlsignaturehelper.cxx @@ -48,6 +48,8 @@ #include <comphelper/ofopxmlhelper.hxx> #include <comphelper/sequence.hxx> #include <tools/diagnose_ex.h> +#include <rtl/ustrbuf.hxx> +#include <sal/log.hxx> #include <boost/optional.hpp> @@ -633,7 +635,7 @@ static auto CheckX509Data( start = i; // issuer isn't in the list break; } - if (xmlsecurity::EqualDistinguishedNames(certs[i]->getIssuerName(), certs[j]->getSubjectName())) + if (xmlsecurity::EqualDistinguishedNames(certs[i]->getIssuerName(), certs[j]->getSubjectName(), xmlsecurity::NOCOMPAT)) { if (i == j) // self signed { @@ -666,7 +668,7 @@ static auto CheckX509Data( if (chain[i] != j) { if (xmlsecurity::EqualDistinguishedNames( - certs[chain[i]]->getSubjectName(), certs[j]->getIssuerName())) + certs[chain[i]]->getSubjectName(), certs[j]->getIssuerName(), xmlsecurity::NOCOMPAT)) { if (chain.size() != i + 1) // already found issuee? { diff --git a/xmlsecurity/source/helper/xsecverify.cxx b/xmlsecurity/source/helper/xsecverify.cxx index 503bae24d379..8d96433fd6cf 100644 --- a/xmlsecurity/source/helper/xsecverify.cxx +++ b/xmlsecurity/source/helper/xsecverify.cxx @@ -273,7 +273,7 @@ void XSecController::setX509Data( OUString const serialNumber(xmlsecurity::bigIntegerToNumericString(xCert->getSerialNumber())); auto const iter = std::find_if(rX509IssuerSerials.begin(), rX509IssuerSerials.end(), [&](auto const& rX509IssuerSerial) { - return xmlsecurity::EqualDistinguishedNames(issuerName, rX509IssuerSerial.first) + return xmlsecurity::EqualDistinguishedNames(issuerName, rX509IssuerSerial.first, xmlsecurity::COMPAT_2ND) && serialNumber == rX509IssuerSerial.second; }); if (iter != rX509IssuerSerials.end()) @@ -420,7 +420,7 @@ void XSecController::setX509CertDigest( { for (auto & it : rData) { - if (xmlsecurity::EqualDistinguishedNames(it.X509IssuerName, rX509IssuerName) + if (xmlsecurity::EqualDistinguishedNames(it.X509IssuerName, rX509IssuerName, xmlsecurity::COMPAT_BOTH) && it.X509SerialNumber == rX509SerialNumber) { it.CertDigest = rCertDigest; @@ -443,7 +443,7 @@ void XSecController::setX509CertDigest( { SAL_INFO("xmlsecurity.helper", "cannot parse X509Certificate"); } - else if (xmlsecurity::EqualDistinguishedNames(xCert->getIssuerName(),rX509IssuerName) + else if (xmlsecurity::EqualDistinguishedNames(xCert->getIssuerName(), rX509IssuerName, xmlsecurity::COMPAT_2ND) && xmlsecurity::bigIntegerToNumericString(xCert->getSerialNumber()) == rX509SerialNumber) { it.CertDigest = rCertDigest; diff --git a/xmlsecurity/source/xmlsec/mscrypt/x509certificate_mscryptimpl.cxx b/xmlsecurity/source/xmlsec/mscrypt/x509certificate_mscryptimpl.cxx index 062b91387cba..da0504addbc7 100644 --- a/xmlsecurity/source/xmlsec/mscrypt/x509certificate_mscryptimpl.cxx +++ b/xmlsecurity/source/xmlsec/mscrypt/x509certificate_mscryptimpl.cxx @@ -31,6 +31,7 @@ #include "oid.hxx" #include <rtl/locale.h> +#include <rtl/ustrbuf.hxx> #include <osl/nlsupport.h> #include <osl/process.h> #include <o3tl/char16_t2wchar_t.hxx> @@ -671,6 +672,67 @@ Sequence<OUString> SAL_CALL X509Certificate_MSCryptImpl::getSupportedServiceName namespace xmlsecurity { +// based on some guesswork and: +// https://datatracker.ietf.org/doc/html/rfc1485 +// https://docs.microsoft.com/en-us/windows/win32/api/wincrypt/nf-wincrypt-certnametostra#CERT_X500_NAME_STR +// the main problem appears to be that in values NSS uses \ escapes but CryptoAPI requires " quotes around value +static OUString CompatDNNSS(OUString const& rDN) +{ + OUStringBuffer buf(rDN.getLength()); + enum { DEFAULT, INVALUE, INQUOTE } state(DEFAULT); + for (sal_Int32 i = 0; i < rDN.getLength(); ++i) + { + if (state == DEFAULT) + { + buf.append(rDN[i]); + if (rDN[i] == '=') + { + if (rDN.getLength() == i+1) + { + break; // invalid? + } + else + { + buf.append('"'); + state = INVALUE; + } + } + } + else if (state == INVALUE) + { + if (rDN[i] == '+' || rDN[i] == ',' || rDN[i] == ';') + { + buf.append('"'); + buf.append(rDN[i]); + state = DEFAULT; + } + else if (rDN[i] == '\\') + { + if (rDN.getLength() == i+1) + { + break; // invalid? + } + if (rDN[i+1] == '"') + { + buf.append('"'); + } + buf.append(rDN[i+1]); + ++i; + } + else + { + buf.append(rDN[i]); + } + if (i+1 == rDN.getLength()) + { + buf.append('"'); + state = DEFAULT; + } + } + } + return buf.makeStringAndClear(); +} + static bool EncodeDistinguishedName(OUString const& rName, CERT_NAME_BLOB & rBlob) { LPCWSTR pszError; @@ -693,22 +755,38 @@ static bool EncodeDistinguishedName(OUString const& rName, CERT_NAME_BLOB & rBlo } bool EqualDistinguishedNames( - OUString const& rName1, OUString const& rName2) + OUString const& rName1, OUString const& rName2, + EqualMode const eMode) { + if (eMode == COMPAT_BOTH && !rName1.isEmpty() && rName1 == rName2) + { // handle case where both need to be converted + return true; + } CERT_NAME_BLOB blob1; if (!EncodeDistinguishedName(rName1, blob1)) { return false; } CERT_NAME_BLOB blob2; - if (!EncodeDistinguishedName(rName2, blob2)) + bool ret(false); + if (!!EncodeDistinguishedName(rName2, blob2)) { - delete[] blob1.pbData; - return false; + ret = CertCompareCertificateName(X509_ASN_ENCODING, + &blob1, &blob2) == TRUE; + delete[] blob2.pbData; + } + if (!ret && eMode == COMPAT_2ND) + { + CERT_NAME_BLOB blob2compat; + if (!EncodeDistinguishedName(CompatDNNSS(rName2), blob2compat)) + { + delete[] blob1.pbData; + return false; + } + ret = CertCompareCertificateName(X509_ASN_ENCODING, + &blob1, &blob2compat) == TRUE; + delete[] blob2compat.pbData; } - bool const ret(CertCompareCertificateName(X509_ASN_ENCODING, - &blob1, &blob2) == TRUE); - delete[] blob2.pbData; delete[] blob1.pbData; return ret; } diff --git a/xmlsecurity/source/xmlsec/nss/x509certificate_nssimpl.cxx b/xmlsecurity/source/xmlsec/nss/x509certificate_nssimpl.cxx index 332f9363f4dd..9e954fb91cca 100644 --- a/xmlsecurity/source/xmlsec/nss/x509certificate_nssimpl.cxx +++ b/xmlsecurity/source/xmlsec/nss/x509certificate_nssimpl.cxx @@ -30,6 +30,8 @@ #include <comphelper/servicehelper.hxx> #include <cppuhelper/supportsservice.hxx> #include <rtl/ref.hxx> +#include <rtl/ustrbuf.hxx> +#include <sal/log.hxx> #include "x509certificate_nssimpl.hxx" #include <biginteger.hxx> @@ -526,22 +528,103 @@ Sequence<OUString> SAL_CALL X509Certificate_NssImpl::getSupportedServiceNames() namespace xmlsecurity { +// based on some guesswork and: +// https://datatracker.ietf.org/doc/html/rfc1485 +// https://docs.microsoft.com/en-us/windows/win32/api/wincrypt/nf-wincrypt-certnametostra#CERT_X500_NAME_STR +// the main problem appears to be that in values " is escaped as "" vs. \" +static OUString CompatDNCryptoAPI(OUString const& rDN) +{ + OUStringBuffer buf(rDN.getLength()); + enum { DEFAULT, INVALUE, INQUOTE } state(DEFAULT); + for (sal_Int32 i = 0; i < rDN.getLength(); ++i) + { + if (state == DEFAULT) + { + buf.append(rDN[i]); + if (rDN[i] == '=') + { + if (rDN.getLength() == i+1) + { + break; // invalid? + } + else if (rDN[i+1] == '"') + { + buf.append(rDN[i+1]); + ++i; + state = INQUOTE; + } + else + { + state = INVALUE; + } + } + } + else if (state == INVALUE) + { + if (rDN[i] == '+' || rDN[i] == ',' || rDN[i] == ';') + { + state = DEFAULT; + } + buf.append(rDN[i]); + } + else + { + assert(state == INQUOTE); + if (rDN[i] == '"') + { + if (rDN.getLength() != i+1 && rDN[i+1] == '"') + { + buf.append('\\'); + buf.append(rDN[i+1]); + ++i; + } + else + { + buf.append(rDN[i]); + state = DEFAULT; + } + } + else + { + buf.append(rDN[i]); + } + } + } + return buf.makeStringAndClear(); +} + bool EqualDistinguishedNames( - OUString const& rName1, OUString const& rName2) + OUString const& rName1, OUString const& rName2, + EqualMode const eMode) { + if (eMode == COMPAT_BOTH && !rName1.isEmpty() && rName1 == rName2) + { // handle case where both need to be converted + return true; + } CERTName *const pName1(CERT_AsciiToName(OUStringToOString(rName1, RTL_TEXTENCODING_UTF8).getStr())); if (pName1 == nullptr) { return false; } CERTName *const pName2(CERT_AsciiToName(OUStringToOString(rName2, RTL_TEXTENCODING_UTF8).getStr())); - if (pName2 == nullptr) + bool ret(false); + if (pName2) { - CERT_DestroyName(pName1); - return false; + ret = (CERT_CompareName(pName1, pName2) == SECEqual); + CERT_DestroyName(pName2); + } + if (!ret && eMode == COMPAT_2ND) + { + CERTName *const pName2Compat(CERT_AsciiToName(OUStringToOString( + CompatDNCryptoAPI(rName2), RTL_TEXTENCODING_UTF8).getStr())); + if (pName2Compat == nullptr) + { + CERT_DestroyName(pName1); + return false; + } + ret = CERT_CompareName(pName1, pName2Compat) == SECEqual; + CERT_DestroyName(pName2Compat); } - bool const ret(CERT_CompareName(pName1, pName2) == SECEqual); - CERT_DestroyName(pName2); CERT_DestroyName(pName1); return ret; } |