From fd1bc178b02e05cd12ec784ff87f5c97069bc5f5 Mon Sep 17 00:00:00 2001 From: Miklos Vajna Date: Tue, 15 May 2018 22:16:42 +0200 Subject: tdf#109180 xmlsecurity nss: fix signing with ECDSA key Using an ECDSA key but writing RSA URIs would fail later in libxmlsec. Also fix up CppunitTest_xmlsecurity_signing (env vars were set too late), so that the new testcase actually fails without the fix. Change-Id: I9e584844d5cd046952b2f19130aeaa5a765bfc0a Reviewed-on: https://gerrit.libreoffice.org/54400 Tested-by: Jenkins Reviewed-by: Miklos Vajna --- xmlsecurity/qa/unit/signing/data/cert8.db | Bin 65536 -> 65536 bytes xmlsecurity/qa/unit/signing/data/key3.db | Bin 16384 -> 16384 bytes xmlsecurity/qa/unit/signing/signing.cxx | 79 +++++++++++++++++++++++------- 3 files changed, 61 insertions(+), 18 deletions(-) (limited to 'xmlsecurity/qa') diff --git a/xmlsecurity/qa/unit/signing/data/cert8.db b/xmlsecurity/qa/unit/signing/data/cert8.db index 8354fd309e3a..07afe1566989 100644 Binary files a/xmlsecurity/qa/unit/signing/data/cert8.db and b/xmlsecurity/qa/unit/signing/data/cert8.db differ diff --git a/xmlsecurity/qa/unit/signing/data/key3.db b/xmlsecurity/qa/unit/signing/data/key3.db index 8ab32c28d584..fac36c06870a 100644 Binary files a/xmlsecurity/qa/unit/signing/data/key3.db and b/xmlsecurity/qa/unit/signing/data/key3.db differ diff --git a/xmlsecurity/qa/unit/signing/signing.cxx b/xmlsecurity/qa/unit/signing/signing.cxx index 9d1f9240caef..31382925a092 100644 --- a/xmlsecurity/qa/unit/signing/signing.cxx +++ b/xmlsecurity/qa/unit/signing/signing.cxx @@ -41,6 +41,7 @@ #include #include #include +#include using namespace com::sun::star; @@ -64,6 +65,7 @@ public: void registerNamespaces(xmlXPathContextPtr& pXmlXpathCtx) override; void testDescription(); + void testECDSA(); /// Test a typical ODF where all streams are signed. void testODFGood(); /// Test a typical broken ODF signature where one stream is corrupted. @@ -113,6 +115,7 @@ public: #endif CPPUNIT_TEST_SUITE(SigningTest); CPPUNIT_TEST(testDescription); + CPPUNIT_TEST(testECDSA); CPPUNIT_TEST(testODFGood); CPPUNIT_TEST(testODFBroken); CPPUNIT_TEST(testODFNo); @@ -147,7 +150,7 @@ public: private: void createDoc(const OUString& rURL); void createCalc(const OUString& rURL); - uno::Reference getCertificate(DocumentSignatureManager& rSignatureManager); + uno::Reference getCertificate(DocumentSignatureManager& rSignatureManager, svl::crypto::SignatureMethodAlgorithm eAlgo); }; SigningTest::SigningTest() @@ -158,11 +161,6 @@ void SigningTest::setUp() { test::BootstrapFixture::setUp(); - mxComponentContext.set(comphelper::getComponentContext(getMultiServiceFactory())); - mxDesktop.set(frame::Desktop::create(mxComponentContext)); - mxSEInitializer = xml::crypto::SEInitializer::create(mxComponentContext); - mxSecurityContext = mxSEInitializer->createSecurityContext(OUString()); - OUString aSourceDir = m_directories.getURLFromSrc(DATA_DIRECTORY); OUString aTargetDir = m_directories.getURLFromWorkdir( "/CppunitTest/xmlsecurity_signing.test.user/"); @@ -184,6 +182,12 @@ void SigningTest::setUp() osl_setEnvironment(mozCertVar.pData, aTargetPath.pData); OUString gpgHomeVar("GNUPGHOME"); osl_setEnvironment(gpgHomeVar.pData, aTargetPath.pData); + + // Initialize crypto after setting up the environment variables. + mxComponentContext.set(comphelper::getComponentContext(getMultiServiceFactory())); + mxDesktop.set(frame::Desktop::create(mxComponentContext)); + mxSEInitializer = xml::crypto::SEInitializer::create(mxComponentContext); + mxSecurityContext = mxSEInitializer->createSecurityContext(OUString()); } void SigningTest::tearDown() @@ -214,16 +218,19 @@ void SigningTest::createCalc(const OUString& rURL) mxComponent = loadFromDesktop(rURL, "com.sun.star.sheet.SpreadsheetDocument"); } -uno::Reference SigningTest::getCertificate(DocumentSignatureManager& rSignatureManager) +uno::Reference SigningTest::getCertificate(DocumentSignatureManager& rSignatureManager, svl::crypto::SignatureMethodAlgorithm eAlgo) { - uno::Reference xCertificate; - uno::Reference xSecurityEnvironment = rSignatureManager.getSecurityEnvironment(); uno::Sequence> aCertificates = xSecurityEnvironment->getPersonalCertificates(); - if (!aCertificates.hasElements()) - return xCertificate; - return aCertificates[0]; + for (const auto& xCertificate : aCertificates) + { + auto pCertificate = dynamic_cast(xCertificate.get()); + CPPUNIT_ASSERT(pCertificate); + if (pCertificate->getSignatureMethodAlgorithm() == eAlgo) + return xCertificate; + } + return uno::Reference(); } void SigningTest::testDescription() @@ -246,7 +253,7 @@ void SigningTest::testDescription() aManager.maSignatureHelper.SetStorage(xStorage, "1.2"); // Then add a signature document. - uno::Reference xCertificate = getCertificate(aManager); + uno::Reference xCertificate = getCertificate(aManager, svl::crypto::SignatureMethodAlgorithm::RSA); if (!xCertificate.is()) return; OUString aDescription("SigningTest::testDescription"); @@ -260,6 +267,42 @@ void SigningTest::testDescription() CPPUNIT_ASSERT_EQUAL(aDescription, rInformations[0].ouDescription); } +void SigningTest::testECDSA() +{ + // Create an empty document and store it to a tempfile, finally load it as a storage. + createDoc(""); + + utl::TempFile aTempFile; + aTempFile.EnableKillingFile(); + uno::Reference xStorable(mxComponent, uno::UNO_QUERY); + utl::MediaDescriptor aMediaDescriptor; + aMediaDescriptor["FilterName"] <<= OUString("writer8"); + xStorable->storeAsURL(aTempFile.GetURL(), aMediaDescriptor.getAsConstPropertyValueList()); + + DocumentSignatureManager aManager(mxComponentContext, DocumentSignatureMode::Content); + CPPUNIT_ASSERT(aManager.init()); + uno::Reference xStorage = comphelper::OStorageHelper::GetStorageOfFormatFromURL(ZIP_STORAGE_FORMAT_STRING, aTempFile.GetURL(), embed::ElementModes::READWRITE); + CPPUNIT_ASSERT(xStorage.is()); + aManager.mxStore = xStorage; + aManager.maSignatureHelper.SetStorage(xStorage, "1.2"); + + // Then add a signature. + uno::Reference xCertificate = getCertificate(aManager, svl::crypto::SignatureMethodAlgorithm::ECDSA); + if (!xCertificate.is()) + return; + OUString aDescription; + sal_Int32 nSecurityId; + aManager.add(xCertificate, mxSecurityContext, aDescription, nSecurityId, false); + + // Read back the signature and make sure that it's valid. + aManager.read(/*bUseTempStream=*/true); + std::vector& rInformations = aManager.maCurrentSignatureInformations; + CPPUNIT_ASSERT_EQUAL(static_cast(1), rInformations.size()); + // This was SecurityOperationStatus_UNKNOWN, signing with an ECDSA key was + // broken. + CPPUNIT_ASSERT_EQUAL(css::xml::crypto::SecurityOperationStatus_OPERATION_SUCCEEDED, rInformations[0].nStatus); +} + void SigningTest::testOOXMLDescription() { // Create an empty document and store it to a tempfile, finally load it as a storage. @@ -280,7 +323,7 @@ void SigningTest::testOOXMLDescription() aManager.maSignatureHelper.SetStorage(xStorage, "1.2"); // Then add a document signature. - uno::Reference xCertificate = getCertificate(aManager); + uno::Reference xCertificate = getCertificate(aManager, svl::crypto::SignatureMethodAlgorithm::RSA); if (!xCertificate.is()) return; OUString aDescription("SigningTest::testDescription"); @@ -314,7 +357,7 @@ void SigningTest::testOOXMLAppend() CPPUNIT_ASSERT_EQUAL(static_cast(1), rInformations.size()); // Then add a second document signature. - uno::Reference xCertificate = getCertificate(aManager); + uno::Reference xCertificate = getCertificate(aManager, svl::crypto::SignatureMethodAlgorithm::RSA); if (!xCertificate.is()) return; sal_Int32 nSecurityId; @@ -346,7 +389,7 @@ void SigningTest::testOOXMLRemove() CPPUNIT_ASSERT_EQUAL(static_cast(2), rInformations.size()); // Then remove the last added signature. - uno::Reference xCertificate = getCertificate(aManager); + uno::Reference xCertificate = getCertificate(aManager, svl::crypto::SignatureMethodAlgorithm::RSA); if (!xCertificate.is()) return; aManager.remove(0); @@ -377,7 +420,7 @@ void SigningTest::testOOXMLRemoveAll() CPPUNIT_ASSERT_EQUAL(static_cast(1), rInformations.size()); // Then remove the only signature in the document. - uno::Reference xCertificate = getCertificate(aManager); + uno::Reference xCertificate = getCertificate(aManager, svl::crypto::SignatureMethodAlgorithm::RSA); if (!xCertificate.is()) return; aManager.remove(0); @@ -625,7 +668,7 @@ void SigningTest::testXAdES() aManager.maSignatureHelper.SetStorage(xStorage, "1.2"); // Create a signature. - uno::Reference xCertificate = getCertificate(aManager); + uno::Reference xCertificate = getCertificate(aManager, svl::crypto::SignatureMethodAlgorithm::RSA); if (!xCertificate.is()) return; sal_Int32 nSecurityId; -- cgit