[Libreoffice-commits] core.git: Branch 'libreoffice-6-4' - cui/source uui/source xmlsecurity/inc xmlsecurity/source xmlsecurity/uiconfig

Jan-Marek Glogowski (via logerrit) logerrit at kemper.freedesktop.org
Tue Dec 17 13:37:50 UTC 2019


 cui/source/options/optinet2.cxx              |   14 ----
 uui/source/secmacrowarnings.cxx              |    4 -
 xmlsecurity/inc/macrosecurity.hxx            |    3 -
 xmlsecurity/inc/strings.hrc                  |    2 
 xmlsecurity/source/dialogs/macrosecurity.cxx |   76 ++++++++++++++++++++-------
 xmlsecurity/uiconfig/ui/securitytrustpage.ui |    4 +
 6 files changed, 68 insertions(+), 35 deletions(-)

New commits:
commit ad1a41032054991cfb8b9e821c1dd25ec21d0aec
Author:     Jan-Marek Glogowski <jan-marek.glogowski at extern.cib.de>
AuthorDate: Wed Dec 11 10:24:37 2019 +0100
Commit:     Jan-Marek Glogowski <glogow at fbihome.de>
CommitDate: Tue Dec 17 14:37:02 2019 +0100

    Fix macro security UI usability problems
    
    * Don't hide the option dialogs "Macro security" push button.
      I don't see any reason, why these settings should be hidden, if
      macros are disabled or settings locked. At least a user can now
      check, what is going on (still nothing shows disabled macros for
      a document in the UI AFAIK).
    * Don't scale the lock icons of the trusted list boxes.
      This just uses the same alignments, which the macro security
      level lock image uses, otherwise the image is scaled to fit the
      whole space of its layout cell.
    * Don't disable the trusted list boxes.
      If the setting is locked, it's sufficient to disable all the
      buttons, which allow modification (so View can stay enabled).
      This way you can still scroll the list. Correct button handling
      is already implemented and works for me.
    * Catch exceptions of broken certificate data.
      If your config contains certificates, which can't be correctly
      decoded, the NSS backend will throw an exception, which kills
      the dialog, but not the nested loop, resulting in a locked LO.
      Also show an error dialog with the broken base64-encoded data.
    
    Change-Id: I79002e0ce85cf9a9017caf858407f2f635a3a074
    Reviewed-on: https://gerrit.libreoffice.org/85056
    Tested-by: Jenkins
    Reviewed-by: Jan-Marek Glogowski <glogow at fbihome.de>
    (cherry picked from commit b3348ce498b3d54b3e5e6518954ad9d5e917b8f2)
    Reviewed-on: https://gerrit.libreoffice.org/85282

diff --git a/cui/source/options/optinet2.cxx b/cui/source/options/optinet2.cxx
index b79a1d9bbd3e..5e3c6a878ff4 100644
--- a/cui/source/options/optinet2.cxx
+++ b/cui/source/options/optinet2.cxx
@@ -734,20 +734,6 @@ IMPL_LINK_NOARG(SvxSecurityTabPage, MacroSecPBHdl, weld::Button&, void)
 
 void SvxSecurityTabPage::InitControls()
 {
-    // Hide all controls which belong to the macro security button in case the macro
-    // security settings managed by the macro security dialog opened via the button
-    // are all readonly or if the macros are disabled in general.
-    // @@@ Better would be to query the dialog whether it is 'useful' or not. Exposing
-    //     macro security dialog implementations here, which is bad.
-    if (    mpSecOptions->IsMacroDisabled()
-         || (    mpSecOptions->IsReadOnly( SvtSecurityOptions::EOption::MacroSecLevel )
-              && mpSecOptions->IsReadOnly( SvtSecurityOptions::EOption::MacroTrustedAuthors )
-              && mpSecOptions->IsReadOnly( SvtSecurityOptions::EOption::SecureUrls ) ) )
-    {
-        //Hide these
-        m_xMacroSecFrame->hide();
-    }
-
 #ifndef UNX
     m_xCertFrame->hide();
 #endif
diff --git a/uui/source/secmacrowarnings.cxx b/uui/source/secmacrowarnings.cxx
index af913b303172..6c6e343a08f0 100644
--- a/uui/source/secmacrowarnings.cxx
+++ b/uui/source/secmacrowarnings.cxx
@@ -105,7 +105,7 @@ IMPL_LINK_NOARG(MacroWarning, ViewSignsBtnHdl, weld::Button&, void)
 IMPL_LINK_NOARG(MacroWarning, EnableBtnHdl, weld::Button&, void)
 {
     if (mxAlwaysTrustCB->get_active())
-    {   // insert path into trusted path list
+    {
         uno::Reference< security::XDocumentDigitalSignatures > xD(
             security::DocumentDigitalSignatures::createWithVersion(comphelper::getProcessComponentContext(), maODFVersion));
         xD->setParentWindow(m_xDialog->GetXWindow());
@@ -136,7 +136,7 @@ void MacroWarning::InitControls()
     if (mbShowSignatures)
     {
         mxViewSignsBtn->connect_clicked(LINK(this, MacroWarning, ViewSignsBtnHdl));
-        mxViewSignsBtn->set_sensitive(false);   // default
+        mxViewSignsBtn->set_sensitive(false);
         mxAlwaysTrustCB->connect_clicked(LINK(this, MacroWarning, AlwaysTrustCheckHdl));
 
         mnActSecLevel = SvtSecurityOptions().GetMacroSecurityLevel();
diff --git a/xmlsecurity/inc/macrosecurity.hxx b/xmlsecurity/inc/macrosecurity.hxx
index 3dafc9d6d4dd..88d1c18a325d 100644
--- a/xmlsecurity/inc/macrosecurity.hxx
+++ b/xmlsecurity/inc/macrosecurity.hxx
@@ -120,8 +120,9 @@ private:
     DECL_LINK(TrustCertLBSelectHdl, weld::TreeView&, void);
     DECL_LINK(TrustFileLocLBSelectHdl, weld::TreeView&, void);
 
-    void FillCertLB();
+    void FillCertLB(const bool bShowWarnings = false);
     void ImplCheckButtons();
+    void ShowBrokenCertificateError(const OUString& rData);
 
 public:
     MacroSecurityTrustedSourcesTP(weld::Container* pParent, MacroSecurity* pDlg);
diff --git a/xmlsecurity/inc/strings.hrc b/xmlsecurity/inc/strings.hrc
index deed17e9e1a2..e16a3dd7b5db 100644
--- a/xmlsecurity/inc/strings.hrc
+++ b/xmlsecurity/inc/strings.hrc
@@ -63,6 +63,8 @@
 #define STR_SELECTSIGN                              NC_("selectcertificatedialog|str_selectsign", "Select")
 #define STR_ENCRYPT                                 NC_("selectcertificatedialog|str_encrypt", "Encrypt")
 
+#define STR_BROKEN_MACRO_CERTIFICATE_DATA           NC_("STR_BROKEN_MACRO_CERTIFICATE_DATA", "Macro security problem!\n\nBroken certificate data: %{data}")
+
 #endif
 
 /* vim:set shiftwidth=4 softtabstop=4 expandtab: */
diff --git a/xmlsecurity/source/dialogs/macrosecurity.cxx b/xmlsecurity/source/dialogs/macrosecurity.cxx
index bb1015586cf4..0fbdbcf57323 100644
--- a/xmlsecurity/source/dialogs/macrosecurity.cxx
+++ b/xmlsecurity/source/dialogs/macrosecurity.cxx
@@ -21,6 +21,8 @@
 #include <macrosecurity.hxx>
 #include <certificateviewer.hxx>
 #include <biginteger.hxx>
+#include <resourcemanager.hxx>
+#include <strings.hrc>
 
 #include <osl/file.hxx>
 #include <sal/log.hxx>
@@ -195,27 +197,52 @@ void MacroSecurityTrustedSourcesTP::ImplCheckButtons()
     m_xRemoveLocPB->set_sensitive( bLocationSelected && !mbURLsReadonly);
 }
 
+void MacroSecurityTrustedSourcesTP::ShowBrokenCertificateError(const OUString& rData)
+{
+    OUString aMsg = XsResId(STR_BROKEN_MACRO_CERTIFICATE_DATA);
+    aMsg = aMsg.replaceFirst("%{data}", rData);
+    std::unique_ptr<weld::MessageDialog> xErrorBox(Application::CreateMessageDialog(m_pDlg->getDialog(),
+        VclMessageType::Error, VclButtonsType::Ok, aMsg));
+    xErrorBox->run();
+}
 
 IMPL_LINK_NOARG(MacroSecurityTrustedSourcesTP, ViewCertPBHdl, weld::Button&, void)
 {
     int nEntry = m_xTrustCertLB->get_selected_index();
     if (nEntry != -1)
     {
-        sal_uInt16 nSelected = m_xTrustCertLB->get_id(nEntry).toUInt32();
-
-        uno::Reference< css::security::XCertificate > xCert = m_pDlg->m_xSecurityEnvironment->getCertificate( m_aTrustedAuthors[nSelected][0], xmlsecurity::numericStringToBigInteger( m_aTrustedAuthors[nSelected][1] ) );
-
-        // If we don't get it, create it from signature data:
-        if ( !xCert.is() )
-            xCert = m_pDlg->m_xSecurityEnvironment->createCertificateFromAscii( m_aTrustedAuthors[nSelected][2] ) ;
+        const sal_uInt16 nSelected = m_xTrustCertLB->get_id(nEntry).toUInt32();
+        uno::Reference< css::security::XCertificate > xCert;
+        try
+        {
+            xCert = m_pDlg->m_xSecurityEnvironment->getCertificate(m_aTrustedAuthors[nSelected][0],
+                            xmlsecurity::numericStringToBigInteger(m_aTrustedAuthors[nSelected][1]));
+        }
+        catch (...)
+        {
+            TOOLS_WARN_EXCEPTION("xmlsecurity.dialogs", "matching certificate not found for: " << m_aTrustedAuthors[nSelected][0]);
+        }
 
-        SAL_WARN_IF( !xCert.is(), "xmlsecurity.dialogs", "*MacroSecurityTrustedSourcesTP::ViewCertPBHdl(): Certificate not found and can't be created!" );
+        if (!xCert.is())
+        {
+            try
+            {
+                xCert = m_pDlg->m_xSecurityEnvironment->createCertificateFromAscii(m_aTrustedAuthors[nSelected][2]);
+            }
+            catch (...)
+            {
+                TOOLS_WARN_EXCEPTION("xmlsecurity.dialogs", "certificate data couldn't be parsed: " << m_aTrustedAuthors[nSelected][2]);
+            }
+        }
 
         if ( xCert.is() )
         {
             CertificateViewer aViewer(m_pDlg->getDialog(), m_pDlg->m_xSecurityEnvironment, xCert, false, nullptr);
             aViewer.run();
         }
+        else
+            // should never happen, as we parsed the certificate data when we added it!
+            ShowBrokenCertificateError(m_aTrustedAuthors[nSelected][2]);
     }
 }
 
@@ -297,7 +324,7 @@ IMPL_LINK_NOARG(MacroSecurityTrustedSourcesTP, TrustFileLocLBSelectHdl, weld::Tr
     ImplCheckButtons();
 }
 
-void MacroSecurityTrustedSourcesTP::FillCertLB()
+void MacroSecurityTrustedSourcesTP::FillCertLB(const bool bShowWarnings)
 {
     m_xTrustCertLB->clear();
 
@@ -309,12 +336,27 @@ void MacroSecurityTrustedSourcesTP::FillCertLB()
         {
             css::uno::Sequence< OUString >&              rEntry = m_aTrustedAuthors[ nEntry ];
 
-            // create from RawData
-            uno::Reference< css::security::XCertificate > xCert = m_pDlg->m_xSecurityEnvironment->createCertificateFromAscii( rEntry[ 2 ] );
-
-            m_xTrustCertLB->append(OUString::number(nEntry), xmlsec::GetContentPart(xCert->getSubjectName()));
-            m_xTrustCertLB->set_text(nEntry, xmlsec::GetContentPart(xCert->getIssuerName()), 1);
-            m_xTrustCertLB->set_text(nEntry, utl::GetDateTimeString(xCert->getNotValidAfter()), 2);
+            try
+            {
+                // create from RawData
+                uno::Reference< css::security::XCertificate > xCert = m_pDlg->m_xSecurityEnvironment->createCertificateFromAscii(rEntry[2]);
+                m_xTrustCertLB->append(OUString::number(nEntry), xmlsec::GetContentPart(xCert->getSubjectName()));
+                m_xTrustCertLB->set_text(nEntry, xmlsec::GetContentPart(xCert->getIssuerName()), 1);
+                m_xTrustCertLB->set_text(nEntry, utl::GetDateTimeString(xCert->getNotValidAfter()), 2);
+            }
+            catch (...)
+            {
+                if (bShowWarnings)
+                {
+                    TOOLS_WARN_EXCEPTION("xmlsecurity.dialogs", "certificate data couldn't be parsed: " << rEntry[2]);
+                    OUString sData = rEntry[2];
+                    css::uno::Any tools_warn_exception(DbgGetCaughtException());
+                    OUString sException = OStringToOUString(exceptionToString(tools_warn_exception), RTL_TEXTENCODING_UTF8);
+                    if (!sException.isEmpty())
+                        sData +=  " / " + sException;
+                    ShowBrokenCertificateError(sData);
+                }
+            }
         }
     }
 }
@@ -352,14 +394,12 @@ MacroSecurityTrustedSourcesTP::MacroSecurityTrustedSourcesTP(weld::Container* pP
     m_aTrustedAuthors = m_pDlg->m_aSecOptions.GetTrustedAuthors();
     mbAuthorsReadonly = m_pDlg->m_aSecOptions.IsReadOnly( SvtSecurityOptions::EOption::MacroTrustedAuthors );
     m_xTrustCertROFI->set_visible(mbAuthorsReadonly);
-    m_xTrustCertLB->set_sensitive(!mbAuthorsReadonly);
 
-    FillCertLB();
+    FillCertLB(true);
 
     const css::uno::Sequence< OUString > aSecureURLs = m_pDlg->m_aSecOptions.GetSecureURLs();
     mbURLsReadonly = m_pDlg->m_aSecOptions.IsReadOnly( SvtSecurityOptions::EOption::SecureUrls );
     m_xTrustFileROFI->set_visible(mbURLsReadonly);
-    m_xTrustFileLocLB->set_sensitive(!mbURLsReadonly);
     m_xAddLocPB->set_sensitive(!mbURLsReadonly);
 
     for (const auto& rSecureURL : aSecureURLs)
diff --git a/xmlsecurity/uiconfig/ui/securitytrustpage.ui b/xmlsecurity/uiconfig/ui/securitytrustpage.ui
index 88edf37e485c..b9d5ef518396 100644
--- a/xmlsecurity/uiconfig/ui/securitytrustpage.ui
+++ b/xmlsecurity/uiconfig/ui/securitytrustpage.ui
@@ -169,6 +169,8 @@
                     <property name="can_focus">False</property>
                     <property name="no_show_all">True</property>
                     <property name="icon_name">res/lock.png</property>
+                    <property name="halign">center</property>
+                    <property name="valign">center</property>
                   </object>
                   <packing>
                     <property name="left_attach">0</property>
@@ -325,6 +327,8 @@
                     <property name="can_focus">False</property>
                     <property name="no_show_all">True</property>
                     <property name="icon_name">res/lock.png</property>
+                    <property name="halign">center</property>
+                    <property name="valign">center</property>
                   </object>
                   <packing>
                     <property name="left_attach">0</property>


More information about the Libreoffice-commits mailing list