cool#9992 lok doc sign: async DocumentDigitalSignatures::ImplViewSignatures()

Currently SfxObjectShell::CheckIsReadonly() has a hack for the LOK case
to show the signatures dialog read-only, as only that is async.

The next step is to make DocumentDigitalSignatures::ImplViewSignatures()
async, though that requires all callers of the function to be async, so
make DocumentDigitalSignatures::signScriptingContent() async as well.
There is also DocumentDigitalSignatures::signPackage(), but turns out
that's dead code, so just remove it.

Once this is in place, we had a problem that the callbacks tried to
interact with libxmlsec, but the dialog was still alive in
DocumentDigitalSignatures::ImplViewSignatures() by the time the callback
was running, so there were two DocumentSignatureManager instances at the
same time, and both assumes it should call the global libxmlsec
init/uninit, which resulted in failing to verify the just created
signature.

Fix this similar to how Tomaz fixed the same problem around pdfium in
commit 067a8a954c (pdfium: only init
pdfium library one and destroy on LO exit, 2020-06-03).

Change-Id: I3fb63c06195564732e1576dbd755157e676fb762
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/173117
Tested-by: Jenkins
Reviewed-by: Miklos Vajna <vmiklos@collabora.com>

include/sfx2/digitalsignatures.hxx

@@ -38,6 +38,13 @@ public:
                                           const std::function<void(bool)>& rCallback)
         = 0;
 
+    /// Async replacement for signScriptingContent().
+    virtual void
+    SignScriptingContentAsync(const css::uno::Reference<css::embed::XStorage>& rxStorage,
+                              const css::uno::Reference<css::io::XStream>& xSignStream,
+                              const std::function<void(bool)>& rCallback)
+        = 0;
+
 protected:
     ~DigitalSignatures() noexcept = default;
 };

offapi/com/sun/star/security/XDocumentDigitalSignatures.idl

@@ -78,6 +78,8 @@ interface XDocumentDigitalSignatures : com::sun::star::uno::XInterface
     /** signs the content of the Scripting including macros and basic dialogs
 
         <p>The rest of document content will not be signed.</p>
+
+       Deprecated, this synchronous version would block the UI till signing is in progress.
      */
     boolean signScriptingContent( [in] ::com::sun::star::embed::XStorage xStorage,
                                   [in] ::com::sun::star::io::XStream xSignStream);
@@ -102,6 +104,8 @@ interface XDocumentDigitalSignatures : com::sun::star::uno::XInterface
     string getScriptingContentSignatureDefaultStreamName();
 
     /** signs the full Package, which means everything in the storage except the content of META-INF
+
+       Deprecated, this synchronous version would block the UI till signing is in progress.
      */
     boolean signPackage( [in] ::com::sun::star::embed::XStorage Storage,
                          [in] ::com::sun::star::io::XStream xSignStream);

sfx2/source/doc/docfile.cxx

@@ -4388,6 +4388,8 @@ void SfxMedium::SignContents_Impl(weld::Window* pDialogParent,
                 throw uno::RuntimeException();
         }
 
+        auto xModelSigner = dynamic_cast<sfx2::DigitalSignatures*>(xSigner.get());
+        assert(xModelSigner);
         if ( bSignScriptingContent )
         {
             // If the signature has already the document signature it will be removed
@@ -4401,8 +4403,10 @@ void SfxMedium::SignContents_Impl(weld::Window* pDialogParent,
             // xWriteableZipStor because a writable storage can't have 2
             // instances of sub-storage for the same directory open, but with
             // independent storages it somehow works
-            if (xSigner->signScriptingContent(GetScriptingStorageToSign_Impl(), xStream))
-            {
+            xModelSigner->SignScriptingContentAsync(
+                GetScriptingStorageToSign_Impl(), xStream,
+                [this, xSigner, xMetaInf, xWriteableZipStor,
+                 onSignDocumentContentFinished](bool bRet) {
                 // remove the document signature if any
                 OUString aDocSigName = xSigner->getDocumentContentSignatureDefaultStreamName();
                 if ( !aDocSigName.isEmpty() && xMetaInf->hasByName( aDocSigName ) )
@@ -4429,13 +4433,12 @@ void SfxMedium::SignContents_Impl(weld::Window* pDialogParent,
                     || !uno::Reference<util::XModifiable>(pImpl->xStorage, uno::UNO_QUERY_THROW)->isModified());
                 // the temporary file has been written, commit it to the original file
                 Commit();
-                bChanges = true;
-            }
+                onSignDocumentContentFinished(bRet);
+            });
+            return;
         }
         else
         {
-            auto xModelSigner = dynamic_cast<sfx2::DigitalSignatures*>(xSigner.get());
-            assert(xModelSigner);
             if (xMetaInf.is())
             {
                 // ODF.

xmlsecurity/inc/documentsignaturemanager.hxx

@@ -54,6 +54,7 @@ class XComponentContext;
 }
 }
 class PDFSignatureHelper;
+class Xmlsec;
 
 /// Manages signatures (addition, removal), used by DigitalSignaturesDialog.
 class XMLSECURITY_DLLPUBLIC DocumentSignatureManager
@@ -75,6 +76,7 @@ private:
     css::uno::Reference<css::xml::crypto::XXMLSecurityContext> mxSecurityContext;
     css::uno::Reference<css::xml::crypto::XSEInitializer> mxGpgSEInitializer;
     css::uno::Reference<css::xml::crypto::XXMLSecurityContext> mxGpgSecurityContext;
+    std::shared_ptr<Xmlsec> mpXmlsecLibrary;
 
 public:
     DocumentSignatureManager(const css::uno::Reference<css::uno::XComponentContext>& xContext,

xmlsecurity/source/component/documentdigitalsignatures.cxx

@@ -81,9 +81,10 @@ private:
     bool m_bHasDocumentSignature;
 
     /// @throws css::uno::RuntimeException
-    bool ImplViewSignatures(const css::uno::Reference<css::embed::XStorage>& rxStorage,
+    void ImplViewSignatures(const css::uno::Reference<css::embed::XStorage>& rxStorage,
                             const css::uno::Reference<css::io::XStream>& xSignStream,
-                            DocumentSignatureMode eMode, bool bReadOnly);
+                            DocumentSignatureMode eMode, bool bReadOnly,
+                            const std::function<void(bool)>& rCallback);
     /// @throws css::uno::RuntimeException
     void ImplViewSignatures(const css::uno::Reference<css::embed::XStorage>& rxStorage,
                             const css::uno::Reference<css::io::XInputStream>& xSignStream,
@@ -216,6 +217,10 @@ public:
     void SignDocumentContentAsync(const css::uno::Reference<css::embed::XStorage>& xStorage,
                                   const css::uno::Reference<css::io::XStream>& xSignStream,
                                   const std::function<void(bool)>& rCallback) override;
+    /// See sfx2::DigitalSignatures::SignScriptingContentAsync().
+    void SignScriptingContentAsync(const css::uno::Reference<css::embed::XStorage>& xStorage,
+                                   const css::uno::Reference<css::io::XStream>& xSignStream,
+                                   const std::function<void(bool)>& rCallback) override;
 };
 
 }
@@ -354,12 +359,10 @@ OUString DocumentDigitalSignatures::getDocumentContentSignatureDefaultStreamName
 }
 
 sal_Bool DocumentDigitalSignatures::signScriptingContent(
-    const Reference< css::embed::XStorage >& rxStorage,
-    const Reference< css::io::XStream >& xSignStream )
+    const Reference< css::embed::XStorage >& /*rxStorage*/,
+    const Reference< css::io::XStream >& /*xSignStream*/ )
 {
-    OSL_ENSURE(!m_sODFVersion.isEmpty(),"DocumentDigitalSignatures: ODF Version not set, assuming minimum 1.2");
-    OSL_ENSURE(m_nArgumentsCount == 2, "DocumentDigitalSignatures: Service was not initialized properly");
-    return ImplViewSignatures( rxStorage, xSignStream, DocumentSignatureMode::Macros, false );
+    for (;;) { std::abort(); } // avoid "must return a value" warnings
 }
 
 Sequence< css::security::DocumentSignatureInformation >
@@ -386,11 +389,10 @@ OUString DocumentDigitalSignatures::getScriptingContentSignatureDefaultStreamNam
 
 
 sal_Bool DocumentDigitalSignatures::signPackage(
-    const Reference< css::embed::XStorage >& rxStorage,
-    const Reference< css::io::XStream >& xSignStream  )
+    const Reference< css::embed::XStorage >& /*rxStorage*/,
+    const Reference< css::io::XStream >& /*xSignStream*/  )
 {
-    OSL_ENSURE(!m_sODFVersion.isEmpty(),"DocumentDigitalSignatures: ODF Version not set, assuming minimum 1.2");
-    return ImplViewSignatures( rxStorage, xSignStream, DocumentSignatureMode::Package, false );
+    for (;;) { std::abort(); } // avoid "must return a value" warnings
 }
 
 Sequence< css::security::DocumentSignatureInformation >
@@ -424,12 +426,12 @@ void DocumentDigitalSignatures::ImplViewSignatures(
     Reference< io::XStream > xStream;
     if ( xSignStream.is() )
         xStream.set( xSignStream, UNO_QUERY );
-    ImplViewSignatures( rxStorage, xStream, eMode, bReadOnly );
+    ImplViewSignatures( rxStorage, xStream, eMode, bReadOnly, [](bool /*bSuccess*/){} );
 }
 
-bool DocumentDigitalSignatures::ImplViewSignatures(
+void DocumentDigitalSignatures::ImplViewSignatures(
     const Reference< css::embed::XStorage >& rxStorage, const Reference< css::io::XStream >& xSignStream,
-    DocumentSignatureMode eMode, bool bReadOnly )
+    DocumentSignatureMode eMode, bool bReadOnly, const std::function<void(bool)>& rCallback )
 {
     bool bChanges = false;
     auto xSignaturesDialog = std::make_shared<DigitalSignaturesDialog>(
@@ -447,7 +449,8 @@ bool DocumentDigitalSignatures::ImplViewSignatures(
         {
             xSignaturesDialog->beforeRun();
             weld::DialogController::runAsync(xSignaturesDialog, [] (sal_Int32) {});
-            return false;
+            rCallback(false);
+            return;
         }
         else if (xSignaturesDialog->run() == RET_OK)
         {
@@ -461,6 +464,8 @@ bool DocumentDigitalSignatures::ImplViewSignatures(
                     xTrans->commit();
                 }
             }
+            rCallback(bChanges);
+            return;
         }
     }
     else
@@ -471,7 +476,7 @@ bool DocumentDigitalSignatures::ImplViewSignatures(
         xBox->run();
     }
 
-    return bChanges;
+    rCallback(bChanges);
 }
 
 Sequence< css::security::DocumentSignatureInformation >
@@ -835,8 +840,16 @@ void DocumentDigitalSignatures::SignDocumentContentAsync(const css::uno::Referen
                               const std::function<void(bool)>& rCallback)
 {
     OSL_ENSURE(!m_sODFVersion.isEmpty(), "DocumentDigitalSignatures: ODF Version not set, assuming minimum 1.2");
-    bool bRet = ImplViewSignatures( rxStorage, xSignStream, DocumentSignatureMode::Content, false );
-    rCallback(bRet);
+    ImplViewSignatures( rxStorage, xSignStream, DocumentSignatureMode::Content, false, rCallback );
+}
+
+void DocumentDigitalSignatures::SignScriptingContentAsync(
+    const Reference<css::embed::XStorage>& rxStorage,
+    const Reference<css::io::XStream>& xSignStream, const std::function<void(bool)>& rCallback)
+{
+    OSL_ENSURE(!m_sODFVersion.isEmpty(),"DocumentDigitalSignatures: ODF Version not set, assuming minimum 1.2");
+    OSL_ENSURE(m_nArgumentsCount == 2, "DocumentDigitalSignatures: Service was not initialized properly");
+    ImplViewSignatures( rxStorage, xSignStream, DocumentSignatureMode::Macros, false, rCallback );
 }
 
 sal_Bool DocumentDigitalSignatures::signPackageWithCertificate(

xmlsecurity/source/helper/documentsignaturemanager.cxx

@@ -56,6 +56,33 @@ using namespace css;
 using namespace css::graphic;
 using namespace css::uno;
 
+/// RAII class to init / shut down libxmlsec.
+class Xmlsec
+{
+public:
+    Xmlsec();
+    ~Xmlsec();
+};
+
+Xmlsec::Xmlsec() { initXmlSec(); }
+
+Xmlsec::~Xmlsec() { deInitXmlSec(); }
+
+namespace
+{
+/// Shared access to libxmlsec, to avoid double init.
+struct XmlsecLibrary
+{
+    static std::shared_ptr<Xmlsec>& get();
+};
+
+std::shared_ptr<Xmlsec>& XmlsecLibrary::get()
+{
+    static std::shared_ptr<Xmlsec> pInstance = std::make_shared<Xmlsec>();
+    return pInstance;
+}
+}
+
 DocumentSignatureManager::DocumentSignatureManager(
     const uno::Reference<uno::XComponentContext>& xContext, DocumentSignatureMode eMode)
     : mxContext(xContext)
@@ -64,7 +91,7 @@ DocumentSignatureManager::DocumentSignatureManager(
 {
 }
 
-DocumentSignatureManager::~DocumentSignatureManager() { deInitXmlSec(); }
+DocumentSignatureManager::~DocumentSignatureManager() { mpXmlsecLibrary.reset(); }
 
 bool DocumentSignatureManager::init()
 {
@@ -76,7 +103,7 @@ bool DocumentSignatureManager::init()
                 "DocumentSignatureManager::Init - mxGpgSEInitializer already set!");
 
     // xmlsec is needed by both services, so init before those
-    initXmlSec();
+    mpXmlsecLibrary = XmlsecLibrary::get();
 
     mxSEInitializer = xml::crypto::SEInitializer::create(mxContext);
 #if HAVE_FEATURE_GPGME