dbaccess/source/core/dataaccess/ModelImpl.cxx | 24 - dbaccess/source/core/inc/ModelImpl.hxx | 3 include/sfx2/docmacromode.hxx | 10 officecfg/registry/schema/org/openoffice/Office/Common.xcs | 125 ++++++++ sfx2/source/doc/docmacromode.cxx | 184 +++++++++---- sfx2/source/doc/objmisc.cxx | 26 - sfx2/source/inc/objshimp.hxx | 3 sw/CppunitTest_sw_odfimport.mk | 8 sw/qa/extras/odfimport/data/ZoneMacroTest.odt |binary sw/qa/extras/odfimport/odfimport.cxx | 131 +++++++++ 10 files changed, 426 insertions(+), 88 deletions(-)
New commits: commit 73e751fc67ea5ae52d15bea1645241ef624188a4 Author: Mike Kaganski <[email protected]> AuthorDate: Thu Nov 9 16:12:45 2023 +0300 Commit: Mike Kaganski <[email protected]> CommitDate: Thu Dec 7 19:45:11 2023 +0300 Fix USE_CONFIG_APPROVE_CONFIRMATION and USE_CONFIG_REJECT_CONFIRMATION They still showed UI in case of signed macros. Two decisions were made, to improve security of USE_CONFIG_APPROVE_CONFIRMATION: 1. In case of High macro security mode, valid but untrusted certificate will be automatically rejected (because it is not safe to automatically add trusted certificates) - so in this mode, USE_CONFIG_APPROVE_CONFIRMATION is the same as USE_CONFIG_REJECT_CONFIRMATION; 2. In case of Medium macro security mode, valid but untrusted certificate will not automatically allow macros execution, but will proceed to the following checks - which on Windows will try to check the source's Security Zone, and may disallow macros based on that. Only after Security Zone check the macros will be automatically allowed. Change-Id: I1a9c92c6b940b689599c5d106798ecfc691dad46 Reviewed-on: https://gerrit.libreoffice.org/c/core/+/159214 Tested-by: Jenkins Reviewed-by: Mike Kaganski <[email protected]> Reviewed-on: https://gerrit.libreoffice.org/c/core/+/159278 Reviewed-by: Miklos Vajna <[email protected]> diff --git a/sfx2/source/doc/docmacromode.cxx b/sfx2/source/doc/docmacromode.cxx index 3d4198662379..02f568ac6027 100644 --- a/sfx2/source/doc/docmacromode.cxx +++ b/sfx2/source/doc/docmacromode.cxx @@ -253,9 +253,12 @@ namespace sfx2 // should not ask any confirmations. FROM_LIST_AND_SIGNED_WARN should only allow // trusted signed macros at this point; so it may only ask for confirmation to add // certificates to trusted, and shouldn't show UI when trusted list is read-only. - const bool bAllowUI = nMacroExecutionMode != MacroExecMode::FROM_LIST_AND_SIGNED_NO_WARN - && (nMacroExecutionMode == MacroExecMode::ALWAYS_EXECUTE - || !SvtSecurityOptions::IsReadOnly(SvtSecurityOptions::EOption::MacroTrustedAuthors)); + const bool bAllowUI + = nMacroExecutionMode != MacroExecMode::FROM_LIST_AND_SIGNED_NO_WARN + && eAutoConfirm == eNoAutoConfirm + && (nMacroExecutionMode == MacroExecMode::ALWAYS_EXECUTE + || !SvtSecurityOptions::IsReadOnly( + SvtSecurityOptions::EOption::MacroTrustedAuthors)); const bool bHasTrustedMacroSignature = m_xData->m_rDocumentAccess.hasTrustedScriptingSignature(bAllowUI ? rxInteraction : nullptr); if (bHasTrustedMacroSignature) @@ -267,11 +270,22 @@ namespace sfx2 || nSignatureState == SignatureState::NOTVALIDATED ) { // there is valid signature, but it is not from the trusted author - // this case includes explicit reject from user in the UI in cases of - // FROM_LIST_AND_SIGNED_WARN and ALWAYS_EXECUTE - if (!bAllowUI) - lcl_showDocumentMacrosDisabledError(rxInteraction, m_xData->m_bDocMacroDisabledMessageShown); - return disallowMacroExecution(); + if (eAutoConfirm == eAutoConfirmApprove + && nMacroExecutionMode == MacroExecMode::ALWAYS_EXECUTE) + { + // For ALWAYS_EXECUTE + eAutoConfirmApprove (USE_CONFIG_APPROVE_CONFIRMATION + // in Medium security mode), do not approve it right here; let Security Zone + // check below do its job first. + } + else + { + // All other cases of valid but untrusted signatures should result in denied + // macros here. This includes explicit reject from user in the UI in cases + // of FROM_LIST_AND_SIGNED_WARN and ALWAYS_EXECUTE + if (!bAllowUI) + lcl_showDocumentMacrosDisabledError(rxInteraction, m_xData->m_bDocMacroDisabledMessageShown); + return disallowMacroExecution(); + } } // Other values of nSignatureState would result in either rejected macros // (FROM_LIST_AND_SIGNED_*), or a confirmation. commit c20eac27f116025adff8a443c7ca23f4e5c66313 Author: Mike Kaganski <[email protected]> AuthorDate: Wed Nov 8 14:19:41 2023 +0300 Commit: Mike Kaganski <[email protected]> CommitDate: Thu Dec 7 19:44:14 2023 +0300 Simplify a bit Additionally, get rid of a variable that was for system path, but used "URL" in its name :-) Change-Id: I77db0ae42677e2fef1431d45b1736d2e54ff26d9 Reviewed-on: https://gerrit.libreoffice.org/c/core/+/159156 Tested-by: Jenkins Reviewed-by: Mike Kaganski <[email protected]> Reviewed-on: https://gerrit.libreoffice.org/c/core/+/159277 Reviewed-by: Miklos Vajna <[email protected]> diff --git a/sfx2/source/doc/docmacromode.cxx b/sfx2/source/doc/docmacromode.cxx index 9be2f27badb2..3d4198662379 100644 --- a/sfx2/source/doc/docmacromode.cxx +++ b/sfx2/source/doc/docmacromode.cxx @@ -353,10 +353,7 @@ namespace sfx2 if ( eAutoConfirm == eNoAutoConfirm ) { OUString sReferrer(sURL); - - OUString aSystemFileURL; - if ( osl::FileBase::getSystemPathFromFileURL( sReferrer, aSystemFileURL ) == osl::FileBase::E_None ) - sReferrer = aSystemFileURL; + osl::FileBase::getSystemPathFromFileURL(sReferrer, sReferrer); bSecure = lcl_showMacroWarning( rxInteraction, sReferrer ); } commit bd9bd5f0a2ce66a3ea2c4e6f032014d173b91b2b Author: Mike Kaganski <[email protected]> AuthorDate: Wed Nov 8 14:15:51 2023 +0300 Commit: Mike Kaganski <[email protected]> CommitDate: Thu Dec 7 19:43:05 2023 +0300 Do not throw on IZoneIdentifier COM error Not being able to obtain Security Zone info from OS is not a fatal error here; just handle it accordingly. Change-Id: Ifb19c88f2c08e99c313aecc54044252bac50f88e Reviewed-on: https://gerrit.libreoffice.org/c/core/+/159155 Tested-by: Jenkins Reviewed-by: Mike Kaganski <[email protected]> Reviewed-on: https://gerrit.libreoffice.org/c/core/+/159276 Reviewed-by: Miklos Vajna <[email protected]> diff --git a/sfx2/source/doc/docmacromode.cxx b/sfx2/source/doc/docmacromode.cxx index 26bd105e0f1e..9be2f27badb2 100644 --- a/sfx2/source/doc/docmacromode.cxx +++ b/sfx2/source/doc/docmacromode.cxx @@ -299,9 +299,9 @@ namespace sfx2 osl::FileBase::getSystemPathFromFileURL(sURL, sFilePath); sal::systools::COMReference<IZoneIdentifier> pZoneId; pZoneId.CoCreateInstance(CLSID_PersistentZoneIdentifier); - sal::systools::COMReference<IPersistFile> pPersist(pZoneId, sal::systools::COM_QUERY_THROW); + sal::systools::COMReference<IPersistFile> pPersist(pZoneId, sal::systools::COM_QUERY); DWORD dwZone; - if (!SUCCEEDED(pPersist->Load(o3tl::toW(sFilePath.getStr()), STGM_READ)) || + if (!pPersist || !SUCCEEDED(pPersist->Load(o3tl::toW(sFilePath.getStr()), STGM_READ)) || !SUCCEEDED(pZoneId->GetId(&dwZone))) { // no Security Zone info found -> assume a local file, not commit 3aa188ba92d437e15eda13a63bafa1382d028f08 Author: Mike Kaganski <[email protected]> AuthorDate: Wed Nov 8 11:57:17 2023 +0300 Commit: Mike Kaganski <[email protected]> CommitDate: Thu Dec 7 19:42:08 2023 +0300 tdf#158090: Do not auto-reject SignatureState::BROKEN in ALWAYS_EXECUTE case It doesn't make sense to silently reject it here, but e.g., allow the confirmation dialog for SignatureState::INVALID case; also, it was only possible to get a silent execution of BROKEN-signature macros (in Low security mode) vs. silent reject (in all higher modes) - which was not good security-wise. Now it will result in the usual confirmation dialog in Medium security mode. Both BROKEN and INVALID signature states are made sure to not allow automatically depending on the Windows Security Zone. Change-Id: I41b0fc96b6bd00e960ae612e79fa1f0f1e06a069 Reviewed-on: https://gerrit.libreoffice.org/c/core/+/159153 Tested-by: Jenkins Reviewed-by: Mike Kaganski <[email protected]> Reviewed-on: https://gerrit.libreoffice.org/c/core/+/159275 Reviewed-by: Xisco Fauli <[email protected]> diff --git a/sfx2/source/doc/docmacromode.cxx b/sfx2/source/doc/docmacromode.cxx index 964a802f770f..26bd105e0f1e 100644 --- a/sfx2/source/doc/docmacromode.cxx +++ b/sfx2/source/doc/docmacromode.cxx @@ -205,6 +205,7 @@ namespace sfx2 if ( nMacroExecutionMode == MacroExecMode::ALWAYS_EXECUTE_NO_WARN ) return allowMacroExecution(); + SignatureState nSignatureState = SignatureState::UNKNOWN; const OUString sURL(m_xData->m_rDocumentAccess.getDocumentLocation()); try { @@ -230,7 +231,7 @@ namespace sfx2 // check whether the document is signed with trusted certificate if ( nMacroExecutionMode != MacroExecMode::FROM_LIST ) { - SignatureState nSignatureState = m_xData->m_rDocumentAccess.getScriptingSignatureState(); + nSignatureState = m_xData->m_rDocumentAccess.getScriptingSignatureState(); if (!bHasValidContentSignature && (nMacroExecutionMode == MacroExecMode::FROM_LIST_AND_SIGNED_NO_WARN @@ -257,13 +258,7 @@ namespace sfx2 || !SvtSecurityOptions::IsReadOnly(SvtSecurityOptions::EOption::MacroTrustedAuthors)); const bool bHasTrustedMacroSignature = m_xData->m_rDocumentAccess.hasTrustedScriptingSignature(bAllowUI ? rxInteraction : nullptr); - if ( nSignatureState == SignatureState::BROKEN ) - { - if (!bAllowUI) - lcl_showDocumentMacrosDisabledError(rxInteraction, m_xData->m_bDocMacroDisabledMessageShown); - return disallowMacroExecution(); - } - else if ( bHasTrustedMacroSignature ) + if (bHasTrustedMacroSignature) { // there is trusted macro signature, allow macro execution return allowMacroExecution(); @@ -278,6 +273,8 @@ namespace sfx2 lcl_showDocumentMacrosDisabledError(rxInteraction, m_xData->m_bDocMacroDisabledMessageShown); return disallowMacroExecution(); } + // Other values of nSignatureState would result in either rejected macros + // (FROM_LIST_AND_SIGNED_*), or a confirmation. } } catch ( const Exception& ) @@ -342,7 +339,10 @@ namespace sfx2 case 0: // Ask break; case 1: // Allow - return allowMacroExecution(); + if (nSignatureState != SignatureState::BROKEN + && nSignatureState != SignatureState::INVALID) + return allowMacroExecution(); + break; case 2: // Deny return disallowMacroExecution(); } commit b53835f93e3c29c81ad6b02bc4e2f11fcd092573 Author: Mike Kaganski <[email protected]> AuthorDate: Mon Jan 2 07:41:46 2023 +0000 Commit: Mike Kaganski <[email protected]> CommitDate: Thu Dec 7 19:41:05 2023 +0300 Only call getDocumentLocation once Change-Id: I0d611e5170b392a6f2b78fda51e48cd1a3287fa7 Reviewed-on: https://gerrit.libreoffice.org/c/core/+/144909 Tested-by: Jenkins Reviewed-by: Mike Kaganski <[email protected]> diff --git a/sfx2/source/doc/docmacromode.cxx b/sfx2/source/doc/docmacromode.cxx index 89b77f488bcc..964a802f770f 100644 --- a/sfx2/source/doc/docmacromode.cxx +++ b/sfx2/source/doc/docmacromode.cxx @@ -205,12 +205,13 @@ namespace sfx2 if ( nMacroExecutionMode == MacroExecMode::ALWAYS_EXECUTE_NO_WARN ) return allowMacroExecution(); + const OUString sURL(m_xData->m_rDocumentAccess.getDocumentLocation()); try { // get document location from medium name and check whether it is a trusted one // the service is created without document version, since it is not of interest here Reference< XDocumentDigitalSignatures > xSignatures(DocumentDigitalSignatures::createDefault(::comphelper::getProcessComponentContext())); - INetURLObject aURLReferer( m_xData->m_rDocumentAccess.getDocumentLocation() ); + INetURLObject aURLReferer(sURL); OUString aLocation = aURLReferer.GetMainURL( INetURLObject::DecodeMechanism::NONE ); @@ -297,7 +298,6 @@ namespace sfx2 #if defined(_WIN32) // Windows specific: try to decide macros loading depending on Windows Security Zones // (is the file local, or it was downloaded from internet, etc?) - OUString sURL(m_xData->m_rDocumentAccess.getDocumentLocation()); OUString sFilePath; osl::FileBase::getSystemPathFromFileURL(sURL, sFilePath); sal::systools::COMReference<IZoneIdentifier> pZoneId; @@ -352,7 +352,7 @@ namespace sfx2 if ( eAutoConfirm == eNoAutoConfirm ) { - OUString sReferrer( m_xData->m_rDocumentAccess.getDocumentLocation() ); + OUString sReferrer(sURL); OUString aSystemFileURL; if ( osl::FileBase::getSystemPathFromFileURL( sReferrer, aSystemFileURL ) == osl::FileBase::E_None ) commit ef65818aa4e4f2812e45bd749f55cc1a7a7f5882 Author: Mike Kaganski <[email protected]> AuthorDate: Wed Nov 8 11:45:17 2023 +0300 Commit: Mike Kaganski <[email protected]> CommitDate: Thu Dec 7 19:40:03 2023 +0300 Simplify a bit Change-Id: Ibf5a0800d7b9410a6191e3be7deef1edd4725640 Reviewed-on: https://gerrit.libreoffice.org/c/core/+/159152 Tested-by: Jenkins Reviewed-by: Mike Kaganski <[email protected]> Reviewed-on: https://gerrit.libreoffice.org/c/core/+/159274 Reviewed-by: Miklos Vajna <[email protected]> diff --git a/sfx2/source/doc/docmacromode.cxx b/sfx2/source/doc/docmacromode.cxx index ce103db868ab..89b77f488bcc 100644 --- a/sfx2/source/doc/docmacromode.cxx +++ b/sfx2/source/doc/docmacromode.cxx @@ -151,8 +151,6 @@ namespace sfx2 bool DocumentMacroMode::adjustMacroMode( const Reference< XInteractionHandler >& rxInteraction, bool bHasValidContentSignature ) { - sal_Int16 nMacroExecutionMode = m_xData->m_rDocumentAccess.getCurrentMacroExecMode(); - if ( SvtSecurityOptions::IsMacroDisabled() ) { // no macro should be executed at all @@ -169,6 +167,7 @@ namespace sfx2 }; AutoConfirmation eAutoConfirm( eNoAutoConfirm ); + sal_Int16 nMacroExecutionMode = m_xData->m_rDocumentAccess.getCurrentMacroExecMode(); if ( ( nMacroExecutionMode == MacroExecMode::USE_CONFIG ) || ( nMacroExecutionMode == MacroExecMode::USE_CONFIG_REJECT_CONFIRMATION ) || ( nMacroExecutionMode == MacroExecMode::USE_CONFIG_APPROVE_CONFIRMATION ) commit 6d4f0c15a74bdd411c8b0220700919d84f2078a1 Author: Mike Kaganski <[email protected]> AuthorDate: Wed Nov 8 10:31:05 2023 +0300 Commit: Mike Kaganski <[email protected]> CommitDate: Thu Dec 7 19:38:51 2023 +0300 Early shortcut for cases requiring both macro and document signatures This avoids a possible problem in High security mode, introduced in commit 1dc71daf7fa7204a98c75dac680af664ab9c8edb (Improve macro checks, 2021-01-28), where a valid but untrusted macro certificate initiates a UI asking to always allow this certificate; but no matter what user chose, macros will be disallowed when the document itself is unsigned. Now it will check the document signature state early. Change-Id: If2255be5da19f3de0090154f0b891ed9496e7bc6 Reviewed-on: https://gerrit.libreoffice.org/c/core/+/159105 Tested-by: Jenkins Reviewed-by: Mike Kaganski <[email protected]> Reviewed-on: https://gerrit.libreoffice.org/c/core/+/159273 Reviewed-by: Miklos Vajna <[email protected]> diff --git a/sfx2/source/doc/docmacromode.cxx b/sfx2/source/doc/docmacromode.cxx index 72d3f1e0be9d..ce103db868ab 100644 --- a/sfx2/source/doc/docmacromode.cxx +++ b/sfx2/source/doc/docmacromode.cxx @@ -230,6 +230,21 @@ namespace sfx2 // check whether the document is signed with trusted certificate if ( nMacroExecutionMode != MacroExecMode::FROM_LIST ) { + SignatureState nSignatureState = m_xData->m_rDocumentAccess.getScriptingSignatureState(); + + if (!bHasValidContentSignature + && (nMacroExecutionMode == MacroExecMode::FROM_LIST_AND_SIGNED_NO_WARN + || nMacroExecutionMode == MacroExecMode::FROM_LIST_AND_SIGNED_WARN) + && m_xData->m_rDocumentAccess.macroCallsSeenWhileLoading()) + { + // When macros are required to be signed, and the document has events which call + // macros, the document content needs to be signed, too. Do it here, and avoid + // possible UI asking to always trust certificates, after which the user's choice + // to allow macros would be ignored anyway. + lcl_showMacrosDisabledUnsignedContentError(rxInteraction, m_xData->m_bDocMacroDisabledMessageShown); + return disallowMacroExecution(); + } + // At this point, the possible values of nMacroExecutionMode are: ALWAYS_EXECUTE, // FROM_LIST_AND_SIGNED_WARN (the default), FROM_LIST_AND_SIGNED_NO_WARN. // ALWAYS_EXECUTE corresponds to the Medium security level; it should ask for @@ -237,27 +252,17 @@ namespace sfx2 // should not ask any confirmations. FROM_LIST_AND_SIGNED_WARN should only allow // trusted signed macros at this point; so it may only ask for confirmation to add // certificates to trusted, and shouldn't show UI when trusted list is read-only. - // the trusted macro check will also retrieve the signature state ( small optimization ) const bool bAllowUI = nMacroExecutionMode != MacroExecMode::FROM_LIST_AND_SIGNED_NO_WARN && (nMacroExecutionMode == MacroExecMode::ALWAYS_EXECUTE || !SvtSecurityOptions::IsReadOnly(SvtSecurityOptions::EOption::MacroTrustedAuthors)); const bool bHasTrustedMacroSignature = m_xData->m_rDocumentAccess.hasTrustedScriptingSignature(bAllowUI ? rxInteraction : nullptr); - SignatureState nSignatureState = m_xData->m_rDocumentAccess.getScriptingSignatureState(); if ( nSignatureState == SignatureState::BROKEN ) { if (!bAllowUI) lcl_showDocumentMacrosDisabledError(rxInteraction, m_xData->m_bDocMacroDisabledMessageShown); return disallowMacroExecution(); } - else if (nMacroExecutionMode != MacroExecMode::ALWAYS_EXECUTE - && m_xData->m_rDocumentAccess.macroCallsSeenWhileLoading() - && bHasTrustedMacroSignature && !bHasValidContentSignature) - { - // When macros are signed, and the document has events which call macros, the document content needs to be signed too. - lcl_showMacrosDisabledUnsignedContentError(rxInteraction, m_xData->m_bDocMacroDisabledMessageShown); - return disallowMacroExecution(); - } else if ( bHasTrustedMacroSignature ) { // there is trusted macro signature, allow macro execution @@ -267,6 +272,8 @@ namespace sfx2 || nSignatureState == SignatureState::NOTVALIDATED ) { // there is valid signature, but it is not from the trusted author + // this case includes explicit reject from user in the UI in cases of + // FROM_LIST_AND_SIGNED_WARN and ALWAYS_EXECUTE if (!bAllowUI) lcl_showDocumentMacrosDisabledError(rxInteraction, m_xData->m_bDocMacroDisabledMessageShown); return disallowMacroExecution(); commit 2a0b5e82a16899872192cdd3d3c7d954b24a23ea Author: Mike Kaganski <[email protected]> AuthorDate: Wed Nov 8 10:17:42 2023 +0300 Commit: Mike Kaganski <[email protected]> CommitDate: Thu Dec 7 19:31:10 2023 +0300 Simplify a bit Change-Id: I05ef5346f5aab25b208aa058658353cf71e68e87 Reviewed-on: https://gerrit.libreoffice.org/c/core/+/159103 Tested-by: Jenkins Reviewed-by: Mike Kaganski <[email protected]> Reviewed-on: https://gerrit.libreoffice.org/c/core/+/159272 Reviewed-by: Miklos Vajna <[email protected]> diff --git a/sfx2/source/doc/docmacromode.cxx b/sfx2/source/doc/docmacromode.cxx index 29243b45f764..72d3f1e0be9d 100644 --- a/sfx2/source/doc/docmacromode.cxx +++ b/sfx2/source/doc/docmacromode.cxx @@ -272,26 +272,20 @@ namespace sfx2 return disallowMacroExecution(); } } - - // at this point it is clear that the document is neither in secure location nor signed with trusted certificate - if ( ( nMacroExecutionMode == MacroExecMode::FROM_LIST_AND_SIGNED_NO_WARN ) - || ( nMacroExecutionMode == MacroExecMode::FROM_LIST_AND_SIGNED_WARN ) - ) - { - if ( nMacroExecutionMode == MacroExecMode::FROM_LIST_AND_SIGNED_WARN ) - lcl_showDocumentMacrosDisabledError( rxInteraction, m_xData->m_bDocMacroDisabledMessageShown ); - - return disallowMacroExecution(); - } } catch ( const Exception& ) { - if ( ( nMacroExecutionMode == MacroExecMode::FROM_LIST_AND_SIGNED_WARN ) - || ( nMacroExecutionMode == MacroExecMode::FROM_LIST_AND_SIGNED_NO_WARN ) - ) - { - return disallowMacroExecution(); - } + DBG_UNHANDLED_EXCEPTION("sfx.doc"); + } + + // at this point it is clear that the document is neither in secure location nor signed with trusted certificate + if ((nMacroExecutionMode == MacroExecMode::FROM_LIST_AND_SIGNED_NO_WARN) + || (nMacroExecutionMode == MacroExecMode::FROM_LIST_AND_SIGNED_WARN)) + { + if (nMacroExecutionMode == MacroExecMode::FROM_LIST_AND_SIGNED_WARN) + lcl_showDocumentMacrosDisabledError(rxInteraction, m_xData->m_bDocMacroDisabledMessageShown); + + return disallowMacroExecution(); } #if defined(_WIN32) commit bf9ed5d3e8bbc08b2a899116ab0e7d9f8c1704b0 Author: Mike Kaganski <[email protected]> AuthorDate: Wed Nov 8 10:14:13 2023 +0300 Commit: Mike Kaganski <[email protected]> CommitDate: Thu Dec 7 19:30:06 2023 +0300 FROM_LIST_NO_WARN was handled above Change-Id: I88da25f5f2819edd78fb95573c619cd8e3187469 Reviewed-on: https://gerrit.libreoffice.org/c/core/+/159102 Tested-by: Jenkins Reviewed-by: Mike Kaganski <[email protected]> Reviewed-on: https://gerrit.libreoffice.org/c/core/+/159271 Reviewed-by: Miklos Vajna <[email protected]> diff --git a/sfx2/source/doc/docmacromode.cxx b/sfx2/source/doc/docmacromode.cxx index 41b994354243..29243b45f764 100644 --- a/sfx2/source/doc/docmacromode.cxx +++ b/sfx2/source/doc/docmacromode.cxx @@ -286,8 +286,7 @@ namespace sfx2 } catch ( const Exception& ) { - if ( ( nMacroExecutionMode == MacroExecMode::FROM_LIST_NO_WARN ) - || ( nMacroExecutionMode == MacroExecMode::FROM_LIST_AND_SIGNED_WARN ) + if ( ( nMacroExecutionMode == MacroExecMode::FROM_LIST_AND_SIGNED_WARN ) || ( nMacroExecutionMode == MacroExecMode::FROM_LIST_AND_SIGNED_NO_WARN ) ) { commit 4c6e0d6c806366cf09d8a03b54f841c5c71fc5dd Author: Mike Kaganski <[email protected]> AuthorDate: Wed Nov 8 09:35:46 2023 +0300 Commit: Mike Kaganski <[email protected]> CommitDate: Thu Dec 7 19:29:01 2023 +0300 tdf#158090: Limit signed document requirement to High security level Commit 1dc71daf7fa7204a98c75dac680af664ab9c8edb (Improve macro checks, 2021-01-28) introduced a new requirement, that trusted macro signature must be accompanied by valid document signature when the document has events calling macros, otherwise macros are not allowed. But this breaks multiple workflows, where security level is set to limit users' ability to run unsigned macros, where documents aren't signed. As the first step, limit the security hardening introduced in the said commit to High security level; in Medium security level, restore the previous behavior. The plan is to fix more inconsistencies later, and then introduce a new separate configuration to require document signature to allow trusted macros (enabled by default), so that the combination of its default value and the High default security level keep the hardened default security implemented currently, while allowing users to opt to the previous documented behavior. Change-Id: I71ff0e531f3a42fbee7828982e4fd39f0e9d6ea3 Reviewed-on: https://gerrit.libreoffice.org/c/core/+/159101 Tested-by: Mike Kaganski <[email protected]> Reviewed-by: Thorsten Behrens <[email protected]> Reviewed-on: https://gerrit.libreoffice.org/c/core/+/159270 Tested-by: Jenkins Reviewed-by: Miklos Vajna <[email protected]> diff --git a/sfx2/source/doc/docmacromode.cxx b/sfx2/source/doc/docmacromode.cxx index d2096341a244..41b994354243 100644 --- a/sfx2/source/doc/docmacromode.cxx +++ b/sfx2/source/doc/docmacromode.cxx @@ -250,9 +250,9 @@ namespace sfx2 lcl_showDocumentMacrosDisabledError(rxInteraction, m_xData->m_bDocMacroDisabledMessageShown); return disallowMacroExecution(); } - else if ( m_xData->m_rDocumentAccess.macroCallsSeenWhileLoading() && - bHasTrustedMacroSignature && - !bHasValidContentSignature) + else if (nMacroExecutionMode != MacroExecMode::ALWAYS_EXECUTE + && m_xData->m_rDocumentAccess.macroCallsSeenWhileLoading() + && bHasTrustedMacroSignature && !bHasValidContentSignature) { // When macros are signed, and the document has events which call macros, the document content needs to be signed too. lcl_showMacrosDisabledUnsignedContentError(rxInteraction, m_xData->m_bDocMacroDisabledMessageShown); commit 422437e5d122259c7a9d27dde3c7de498073889e Author: Mike Kaganski <[email protected]> AuthorDate: Wed Nov 8 09:32:58 2023 +0300 Commit: Mike Kaganski <[email protected]> CommitDate: Thu Dec 7 19:28:11 2023 +0300 Add comments for the magic constants Change-Id: Ic35758357c18c9d172532819a3e8968fd66b26ea Reviewed-on: https://gerrit.libreoffice.org/c/core/+/159100 Tested-by: Jenkins Reviewed-by: Mike Kaganski <[email protected]> Reviewed-on: https://gerrit.libreoffice.org/c/core/+/159269 Reviewed-by: Miklos Vajna <[email protected]> diff --git a/sfx2/source/doc/docmacromode.cxx b/sfx2/source/doc/docmacromode.cxx index aed1afa6fc15..d2096341a244 100644 --- a/sfx2/source/doc/docmacromode.cxx +++ b/sfx2/source/doc/docmacromode.cxx @@ -182,16 +182,16 @@ namespace sfx2 switch ( SvtSecurityOptions::GetMacroSecurityLevel() ) { - case 3: + case 3: // "Very high" nMacroExecutionMode = MacroExecMode::FROM_LIST_NO_WARN; break; - case 2: + case 2: // "High" nMacroExecutionMode = MacroExecMode::FROM_LIST_AND_SIGNED_WARN; break; - case 1: + case 1: // "Medium" nMacroExecutionMode = MacroExecMode::ALWAYS_EXECUTE; break; - case 0: + case 0: // "Low" nMacroExecutionMode = MacroExecMode::ALWAYS_EXECUTE_NO_WARN; break; default: commit d8839ee71aaffd198e9a43e1255fa1ece445e4f8 Author: Mike Kaganski <[email protected]> AuthorDate: Wed Nov 8 09:19:17 2023 +0300 Commit: Mike Kaganski <[email protected]> CommitDate: Thu Dec 7 19:27:04 2023 +0300 Set document's current macro exec mode consistently in adjustMacroMode Change-Id: Id69d4d043d824b1a9e558bfc9395c4ff64da7c38 Reviewed-on: https://gerrit.libreoffice.org/c/core/+/159099 Tested-by: Jenkins Reviewed-by: Mike Kaganski <[email protected]> Reviewed-on: https://gerrit.libreoffice.org/c/core/+/159268 Reviewed-by: Miklos Vajna <[email protected]> diff --git a/sfx2/source/doc/docmacromode.cxx b/sfx2/source/doc/docmacromode.cxx index 2bb2ac0d6ae3..aed1afa6fc15 100644 --- a/sfx2/source/doc/docmacromode.cxx +++ b/sfx2/source/doc/docmacromode.cxx @@ -201,10 +201,10 @@ namespace sfx2 } if ( nMacroExecutionMode == MacroExecMode::NEVER_EXECUTE ) - return false; + return disallowMacroExecution(); if ( nMacroExecutionMode == MacroExecMode::ALWAYS_EXECUTE_NO_WARN ) - return true; + return allowMacroExecution(); try { commit 8f19cd31178bccd46bdac13d4892deb75940fb54 Author: Mike Kaganski <[email protected]> AuthorDate: Tue Nov 7 16:05:07 2023 +0300 Commit: Mike Kaganski <[email protected]> CommitDate: Thu Dec 7 19:25:34 2023 +0300 Pass XInteractionHandler to hasTrustedScriptingSignature instead of a bool This allows to use the same interaction handler there, as used in DocumentMacroMode::adjustMacroMode. hasTrustedScriptingSignature used to find its own interaction handler; and that would conflict with e.g. ODatabaseModelImpl::adjustMacroMode_AutoReject, which passes nullptr to adjustMacroMode, with intention to not show any UI; but with signed macros (see tdf#97694), the UI would still appear. Change-Id: Ia209f96bef67dccfe1da23c4d172ac47497f8eb1 Reviewed-on: https://gerrit.libreoffice.org/c/core/+/159070 Tested-by: Jenkins Reviewed-by: Mike Kaganski <[email protected]> Reviewed-on: https://gerrit.libreoffice.org/c/core/+/159267 Reviewed-by: Miklos Vajna <[email protected]> diff --git a/dbaccess/source/core/dataaccess/ModelImpl.cxx b/dbaccess/source/core/dataaccess/ModelImpl.cxx index ffa91c19924e..c116c0e12997 100644 --- a/dbaccess/source/core/dataaccess/ModelImpl.cxx +++ b/dbaccess/source/core/dataaccess/ModelImpl.cxx @@ -1359,7 +1359,8 @@ SignatureState ODatabaseModelImpl::getScriptingSignatureState() return m_nScriptingSignatureState; } -bool ODatabaseModelImpl::hasTrustedScriptingSignature(bool bAllowUIToAddAuthor) +bool ODatabaseModelImpl::hasTrustedScriptingSignature( + const css::uno::Reference<css::task::XInteractionHandler>& _rxInteraction) { bool bResult = false; @@ -1391,20 +1392,15 @@ bool ODatabaseModelImpl::hasTrustedScriptingSignature(bool bAllowUIToAddAuthor) }); } - if (!bResult && bAllowUIToAddAuthor) + if (!bResult && _rxInteraction) { - Reference<XInteractionHandler> xInteraction; - xInteraction = m_aMediaDescriptor.getOrDefault("InteractionHandler", xInteraction); - if (xInteraction.is()) - { - task::DocumentMacroConfirmationRequest aRequest; - aRequest.DocumentURL = m_sDocFileLocation; - aRequest.DocumentStorage = xStorage; - aRequest.DocumentSignatureInformation = aInfo; - aRequest.DocumentVersion = aODFVersion; - aRequest.Classification = task::InteractionClassification_QUERY; - bResult = SfxMedium::CallApproveHandler(xInteraction, uno::makeAny(aRequest), true); - } + task::DocumentMacroConfirmationRequest aRequest; + aRequest.DocumentURL = m_sDocFileLocation; + aRequest.DocumentStorage = xStorage; + aRequest.DocumentSignatureInformation = aInfo; + aRequest.DocumentVersion = aODFVersion; + aRequest.Classification = task::InteractionClassification_QUERY; + bResult = SfxMedium::CallApproveHandler(_rxInteraction, uno::Any(aRequest), true); } } catch (uno::Exception&) diff --git a/dbaccess/source/core/inc/ModelImpl.hxx b/dbaccess/source/core/inc/ModelImpl.hxx index b6e2fe729ffb..d335539caaba 100644 --- a/dbaccess/source/core/inc/ModelImpl.hxx +++ b/dbaccess/source/core/inc/ModelImpl.hxx @@ -416,7 +416,8 @@ public: virtual bool macroCallsSeenWhileLoading() const override; virtual css::uno::Reference< css::document::XEmbeddedScripts > getEmbeddedDocumentScripts() const override; virtual SignatureState getScriptingSignatureState() override; - virtual bool hasTrustedScriptingSignature( bool bAllowUIToAddAuthor ) override; + virtual bool hasTrustedScriptingSignature( + const css::uno::Reference<css::task::XInteractionHandler>& _rxInteraction) override; // IModifiableDocument virtual void storageIsModified() override; diff --git a/include/sfx2/docmacromode.hxx b/include/sfx2/docmacromode.hxx index 0acb44cbfbb1..b601a959f6f4 100644 --- a/include/sfx2/docmacromode.hxx +++ b/include/sfx2/docmacromode.hxx @@ -152,10 +152,18 @@ namespace sfx2 When this happens, this method here should be replaced by a method at this new class. + @param _rxInteraction + A handler for interactions which might become necessary to trust a correct + but not yet trusted signature, possibly also adding the author certificate to + trusted list. + + If the user needs to be asked, and if this parameter is <NULL/>, the most + defensive assumptions will be made, i.e. false will be returned. + @seealso <sfx2/signaturestate.hxx> */ virtual bool - hasTrustedScriptingSignature( bool bAllowUIToAddAuthor ) = 0; + hasTrustedScriptingSignature( const css::uno::Reference< css::task::XInteractionHandler >& _rxInteraction ) = 0; protected: ~IMacroDocumentAccess() {} diff --git a/sfx2/source/doc/docmacromode.cxx b/sfx2/source/doc/docmacromode.cxx index 923c899e8e99..2bb2ac0d6ae3 100644 --- a/sfx2/source/doc/docmacromode.cxx +++ b/sfx2/source/doc/docmacromode.cxx @@ -241,7 +241,7 @@ namespace sfx2 const bool bAllowUI = nMacroExecutionMode != MacroExecMode::FROM_LIST_AND_SIGNED_NO_WARN && (nMacroExecutionMode == MacroExecMode::ALWAYS_EXECUTE || !SvtSecurityOptions::IsReadOnly(SvtSecurityOptions::EOption::MacroTrustedAuthors)); - const bool bHasTrustedMacroSignature = m_xData->m_rDocumentAccess.hasTrustedScriptingSignature(bAllowUI); + const bool bHasTrustedMacroSignature = m_xData->m_rDocumentAccess.hasTrustedScriptingSignature(bAllowUI ? rxInteraction : nullptr); SignatureState nSignatureState = m_xData->m_rDocumentAccess.getScriptingSignatureState(); if ( nSignatureState == SignatureState::BROKEN ) diff --git a/sfx2/source/doc/objmisc.cxx b/sfx2/source/doc/objmisc.cxx index bd5153ee5c6c..c6590d1b8b41 100644 --- a/sfx2/source/doc/objmisc.cxx +++ b/sfx2/source/doc/objmisc.cxx @@ -1817,7 +1817,8 @@ SignatureState SfxObjectShell_Impl::getScriptingSignatureState() return nSignatureState; } -bool SfxObjectShell_Impl::hasTrustedScriptingSignature( bool bAllowUIToAddAuthor ) +bool SfxObjectShell_Impl::hasTrustedScriptingSignature( + const css::uno::Reference<css::task::XInteractionHandler>& _rxInteraction) { bool bResult = false; @@ -1853,22 +1854,15 @@ bool SfxObjectShell_Impl::hasTrustedScriptingSignature( bool bAllowUIToAddAuthor [&xSigner](const security::DocumentSignatureInformation& rInfo) { return xSigner->isAuthorTrusted( rInfo.Signer ); }); - if ( !bResult && bAllowUIToAddAuthor ) + if (!bResult && _rxInteraction) { - uno::Reference< task::XInteractionHandler > xInteraction; - if ( rDocShell.GetMedium() ) - xInteraction = rDocShell.GetMedium()->GetInteractionHandler(); - - if ( xInteraction.is() ) - { - task::DocumentMacroConfirmationRequest aRequest; - aRequest.DocumentURL = getDocumentLocation(); - aRequest.DocumentStorage = rDocShell.GetMedium()->GetZipStorageToSign_Impl(); - aRequest.DocumentSignatureInformation = aInfo; - aRequest.DocumentVersion = aVersion; - aRequest.Classification = task::InteractionClassification_QUERY; - bResult = SfxMedium::CallApproveHandler( xInteraction, uno::makeAny( aRequest ), true ); - } + task::DocumentMacroConfirmationRequest aRequest; + aRequest.DocumentURL = getDocumentLocation(); + aRequest.DocumentStorage = rDocShell.GetMedium()->GetZipStorageToSign_Impl(); + aRequest.DocumentSignatureInformation = aInfo; + aRequest.DocumentVersion = aVersion; + aRequest.Classification = task::InteractionClassification_QUERY; + bResult = SfxMedium::CallApproveHandler( _rxInteraction, uno::Any( aRequest ), true ); } } } diff --git a/sfx2/source/inc/objshimp.hxx b/sfx2/source/inc/objshimp.hxx index f6f84d6f40e2..1c09aa0842d9 100644 --- a/sfx2/source/inc/objshimp.hxx +++ b/sfx2/source/inc/objshimp.hxx @@ -146,7 +146,8 @@ struct SfxObjectShell_Impl final : public ::sfx2::IMacroDocumentAccess virtual css::uno::Reference< css::document::XEmbeddedScripts > getEmbeddedDocumentScripts() const override; virtual SignatureState getScriptingSignatureState() override; - virtual bool hasTrustedScriptingSignature( bool bAllowUIToAddAuthor ) override; + virtual bool hasTrustedScriptingSignature( + const css::uno::Reference<css::task::XInteractionHandler>& _rxInteraction) override; }; #endif commit 2e541a18ae2ded6c5c0b96a248b6fcfe102da92f Author: Mike Kaganski <[email protected]> AuthorDate: Tue Nov 7 13:38:33 2023 +0300 Commit: Mike Kaganski <[email protected]> CommitDate: Thu Dec 7 19:19:31 2023 +0300 Add a description comment Basically describing commit 71c6f438cecc3ce5e8060efe1df840652885701c (tdf#129311 don't allow temporary trusted certs, 2019-12-17). Change-Id: I4d947014b09412638560e9249f242cf6ff222cc2 Reviewed-on: https://gerrit.libreoffice.org/c/core/+/159069 Tested-by: Jenkins Reviewed-by: Mike Kaganski <[email protected]> Reviewed-on: https://gerrit.libreoffice.org/c/core/+/159266 Reviewed-by: Miklos Vajna <[email protected]> diff --git a/sfx2/source/doc/docmacromode.cxx b/sfx2/source/doc/docmacromode.cxx index ea9cb18fe799..923c899e8e99 100644 --- a/sfx2/source/doc/docmacromode.cxx +++ b/sfx2/source/doc/docmacromode.cxx @@ -230,6 +230,13 @@ namespace sfx2 // check whether the document is signed with trusted certificate if ( nMacroExecutionMode != MacroExecMode::FROM_LIST ) { + // At this point, the possible values of nMacroExecutionMode are: ALWAYS_EXECUTE, + // FROM_LIST_AND_SIGNED_WARN (the default), FROM_LIST_AND_SIGNED_NO_WARN. + // ALWAYS_EXECUTE corresponds to the Medium security level; it should ask for + // confirmation when macros are unsigned or untrusted. FROM_LIST_AND_SIGNED_NO_WARN + // should not ask any confirmations. FROM_LIST_AND_SIGNED_WARN should only allow + // trusted signed macros at this point; so it may only ask for confirmation to add + // certificates to trusted, and shouldn't show UI when trusted list is read-only. // the trusted macro check will also retrieve the signature state ( small optimization ) const bool bAllowUI = nMacroExecutionMode != MacroExecMode::FROM_LIST_AND_SIGNED_NO_WARN && (nMacroExecutionMode == MacroExecMode::ALWAYS_EXECUTE commit 5104424c82a2d8a1c06396f35bb2162c7cca83ca Author: Mike Kaganski <[email protected]> AuthorDate: Tue Nov 7 10:55:48 2023 +0300 Commit: Mike Kaganski <[email protected]> CommitDate: Thu Dec 7 19:17:46 2023 +0300 Rename variable: The UI is not only to "add" author (i.e., modify config) It is mainly to allow macro execution for this unknown certificate once. The UI will even disable the option to add, when the config is read-only. Change-Id: Iebc526c23572dc7c0e94fac79fafc8b402d451c3 Reviewed-on: https://gerrit.libreoffice.org/c/core/+/159051 Tested-by: Jenkins Reviewed-by: Mike Kaganski <[email protected]> Reviewed-on: https://gerrit.libreoffice.org/c/core/+/159265 Reviewed-by: Miklos Vajna <[email protected]> diff --git a/sfx2/source/doc/docmacromode.cxx b/sfx2/source/doc/docmacromode.cxx index f8283ff1a6ff..ea9cb18fe799 100644 --- a/sfx2/source/doc/docmacromode.cxx +++ b/sfx2/source/doc/docmacromode.cxx @@ -231,15 +231,15 @@ namespace sfx2 if ( nMacroExecutionMode != MacroExecMode::FROM_LIST ) { // the trusted macro check will also retrieve the signature state ( small optimization ) - const bool bAllowUIToAddAuthor = nMacroExecutionMode != MacroExecMode::FROM_LIST_AND_SIGNED_NO_WARN + const bool bAllowUI = nMacroExecutionMode != MacroExecMode::FROM_LIST_AND_SIGNED_NO_WARN && (nMacroExecutionMode == MacroExecMode::ALWAYS_EXECUTE || !SvtSecurityOptions::IsReadOnly(SvtSecurityOptions::EOption::MacroTrustedAuthors)); - const bool bHasTrustedMacroSignature = m_xData->m_rDocumentAccess.hasTrustedScriptingSignature(bAllowUIToAddAuthor); + const bool bHasTrustedMacroSignature = m_xData->m_rDocumentAccess.hasTrustedScriptingSignature(bAllowUI); SignatureState nSignatureState = m_xData->m_rDocumentAccess.getScriptingSignatureState(); if ( nSignatureState == SignatureState::BROKEN ) { - if (!bAllowUIToAddAuthor) + if (!bAllowUI) lcl_showDocumentMacrosDisabledError(rxInteraction, m_xData->m_bDocMacroDisabledMessageShown); return disallowMacroExecution(); } @@ -260,7 +260,7 @@ namespace sfx2 || nSignatureState == SignatureState::NOTVALIDATED ) { // there is valid signature, but it is not from the trusted author - if (!bAllowUIToAddAuthor) + if (!bAllowUI) lcl_showDocumentMacrosDisabledError(rxInteraction, m_xData->m_bDocMacroDisabledMessageShown); return disallowMacroExecution(); } commit 4845dd7232d8757b5cda04f1a194ad7d3788d500 Author: Mike Kaganski <[email protected]> AuthorDate: Tue Nov 7 10:09:23 2023 +0300 Commit: Mike Kaganski <[email protected]> CommitDate: Thu Dec 7 19:16:52 2023 +0300 getCurrentMacroExecMode returns sal_Int16 And that conforms the IDL definition of css::document::MacroExecMode Change-Id: I78ebfa94eb50552e7f4ecf3d64a0ac0556c56867 Reviewed-on: https://gerrit.libreoffice.org/c/core/+/159029 Tested-by: Jenkins Reviewed-by: Mike Kaganski <[email protected]> Reviewed-on: https://gerrit.libreoffice.org/c/core/+/159264 Reviewed-by: Miklos Vajna <[email protected]> diff --git a/sfx2/source/doc/docmacromode.cxx b/sfx2/source/doc/docmacromode.cxx index adf44ff19ce2..f8283ff1a6ff 100644 --- a/sfx2/source/doc/docmacromode.cxx +++ b/sfx2/source/doc/docmacromode.cxx @@ -151,7 +151,7 @@ namespace sfx2 bool DocumentMacroMode::adjustMacroMode( const Reference< XInteractionHandler >& rxInteraction, bool bHasValidContentSignature ) { - sal_uInt16 nMacroExecutionMode = m_xData->m_rDocumentAccess.getCurrentMacroExecMode(); + sal_Int16 nMacroExecutionMode = m_xData->m_rDocumentAccess.getCurrentMacroExecMode(); if ( SvtSecurityOptions::IsMacroDisabled() ) { commit e0d6943630548ffea53e09f8dce5dbbef467d4f3 Author: Mike Kaganski <[email protected]> AuthorDate: Mon Nov 6 21:32:01 2023 +0300 Commit: Mike Kaganski <[email protected]> CommitDate: Thu Dec 7 19:15:56 2023 +0300 Simplify a bit XDocumentDigitalSignatures::isLocationTrusted would remove segments itself, as needed; this change not only simplifies this code, but also potentially allows to define not only trusted directories, but also individuals trusted files (if the UI would be adjusted). Change-Id: I0b0d60946d84a52479fcce5ce49d368cf53283fd Reviewed-on: https://gerrit.libreoffice.org/c/core/+/159009 Tested-by: Jenkins Reviewed-by: Mike Kaganski <[email protected]> Reviewed-on: https://gerrit.libreoffice.org/c/core/+/159263 Reviewed-by: Miklos Vajna <[email protected]> diff --git a/sfx2/source/doc/docmacromode.cxx b/sfx2/source/doc/docmacromode.cxx index 1d64ab7960e8..adf44ff19ce2 100644 --- a/sfx2/source/doc/docmacromode.cxx +++ b/sfx2/source/doc/docmacromode.cxx @@ -213,9 +213,7 @@ namespace sfx2 Reference< XDocumentDigitalSignatures > xSignatures(DocumentDigitalSignatures::createDefault(::comphelper::getProcessComponentContext())); INetURLObject aURLReferer( m_xData->m_rDocumentAccess.getDocumentLocation() ); - OUString aLocation; - if ( aURLReferer.removeSegment() ) - aLocation = aURLReferer.GetMainURL( INetURLObject::DecodeMechanism::NONE ); + OUString aLocation = aURLReferer.GetMainURL( INetURLObject::DecodeMechanism::NONE ); if ( !aLocation.isEmpty() && xSignatures->isLocationTrusted( aLocation ) ) { commit 398cf7e30fba2cdca8de16f4c6aef4186692d15b Author: Mike Kaganski <[email protected]> AuthorDate: Mon Jan 2 07:34:56 2023 +0000 Commit: Mike Kaganski <[email protected]> CommitDate: Thu Dec 7 19:15:02 2023 +0300 Avoid reinterpret_cast Change-Id: I52b1f3d9fb0a3476ac1649ebc05c71aa8f2ce99e Reviewed-on: https://gerrit.libreoffice.org/c/core/+/144908 Tested-by: Jenkins Reviewed-by: Mike Kaganski <[email protected]> diff --git a/sfx2/source/doc/docmacromode.cxx b/sfx2/source/doc/docmacromode.cxx index 53b8c910feae..1d64ab7960e8 100644 --- a/sfx2/source/doc/docmacromode.cxx +++ b/sfx2/source/doc/docmacromode.cxx @@ -39,6 +39,7 @@ #include <tools/urlobj.hxx> #if defined(_WIN32) +#include <o3tl/char16_t2wchar_t.hxx> #include <officecfg/Office/Common.hxx> #include <systools/win32/comtools.hxx> #include <urlmon.h> @@ -299,7 +300,7 @@ namespace sfx2 pZoneId.CoCreateInstance(CLSID_PersistentZoneIdentifier); sal::systools::COMReference<IPersistFile> pPersist(pZoneId, sal::systools::COM_QUERY_THROW); DWORD dwZone; - if (!SUCCEEDED(pPersist->Load(reinterpret_cast<LPCOLESTR>(sFilePath.getStr()), STGM_READ)) || + if (!SUCCEEDED(pPersist->Load(o3tl::toW(sFilePath.getStr()), STGM_READ)) || !SUCCEEDED(pZoneId->GetId(&dwZone))) { // no Security Zone info found -> assume a local file, not commit 429700e60f5fd7155989f31ad616085d66bc107f Author: Mike Kaganski <[email protected]> AuthorDate: Sun Jan 1 14:21:34 2023 +0000 Commit: Mike Kaganski <[email protected]> CommitDate: Thu Dec 7 19:13:39 2023 +0300 Avoid unneeded initialization, and use URLZONE ids Change-Id: I8c6f31865b992fab0739fbefed5d39f21d0fa664 Reviewed-on: https://gerrit.libreoffice.org/c/core/+/144904 Tested-by: Jenkins Reviewed-by: Mike Kaganski <[email protected]> diff --git a/sfx2/source/doc/docmacromode.cxx b/sfx2/source/doc/docmacromode.cxx index 97d0001e10a4..53b8c910feae 100644 --- a/sfx2/source/doc/docmacromode.cxx +++ b/sfx2/source/doc/docmacromode.cxx @@ -304,25 +304,25 @@ namespace sfx2 { // no Security Zone info found -> assume a local file, not // from the internet - dwZone = 0; + dwZone = URLZONE_LOCAL_MACHINE; } // determine action from zone and settings - sal_Int32 nAction = 0; + sal_Int32 nAction; switch (dwZone) { - case 0: + case URLZONE_LOCAL_MACHINE: nAction = officecfg::Office::Common::Security::Scripting::WindowsSecurityZone::ZoneLocal::get(); break; - case 1: + case URLZONE_INTRANET: nAction = officecfg::Office::Common::Security::Scripting::WindowsSecurityZone::ZoneIntranet::get(); break; - case 2: + case URLZONE_TRUSTED: nAction = officecfg::Office::Common::Security::Scripting::WindowsSecurityZone::ZoneTrusted::get(); break; - case 3: + case URLZONE_INTERNET: nAction = officecfg::Office::Common::Security::Scripting::WindowsSecurityZone::ZoneInternet::get(); break; - case 4: + case URLZONE_UNTRUSTED: nAction = officecfg::Office::Common::Security::Scripting::WindowsSecurityZone::ZoneUntrusted::get(); break; default: commit fbb11be8b9281381c51d7057d57d75f41e5e6a81 Author: Vasily Melenchuk <[email protected]> AuthorDate: Mon Dec 5 20:32:41 2022 +0300 Commit: Mike Kaganski <[email protected]> CommitDate: Thu Dec 7 19:12:30 2023 +0300 Related: tdf#125093 Check Windows Security Zones for macros In Windows, files get assigned security zones (local, from intranet, from internet, etc) after download via browser or email client. This is used by MS Word to decide in which mode it is safe to open file. This patch implements basic support for similar feature: by default there are no changes in macro behavior. But it is possible to use expert configuration options to tweak default behavior, and for example disable macros automatically, if a file is downloaded from Internet or other unsafe locations. Change-Id: I0bf1ae4e54d75dd5d07cab309124a67a85ef2d4d Reviewed-on: https://gerrit.libreoffice.org/c/core/+/143680 Tested-by: Jenkins Reviewed-by: Thorsten Behrens <[email protected]> diff --git a/officecfg/registry/schema/org/openoffice/Office/Common.xcs b/officecfg/registry/schema/org/openoffice/Office/Common.xcs index 72ba024f8bc3..487c3a7066e9 100644 --- a/officecfg/registry/schema/org/openoffice/Office/Common.xcs +++ b/officecfg/registry/schema/org/openoffice/Office/Common.xcs @@ -2852,6 +2852,131 @@ <desc>List with trusted authors.</desc> </info> </set> + <group oor:name="WindowsSecurityZone"> + <info> + <desc>Contains security settings regarding Basic scripts.</desc> + </info> + <prop oor:name="ZoneLocal" oor:type="xs:int" oor:nillable="false"> + <info> + <desc>Action needed for opening document with macro with Windows zone + identifier URLZONE_LOCAL_MACHINE (0, local machine).</desc> + </info> + <constraints> + <enumeration oor:value="0"> + <info> + <desc>Ask</desc> + </info> + </enumeration> + <enumeration oor:value="1"> + <info> + <desc>Allow</desc> + </info> + </enumeration> + <enumeration oor:value="2"> + <info> + <desc>Deny</desc> + </info> + </enumeration> + </constraints> + <value>0</value> + </prop> + <prop oor:name="ZoneIntranet" oor:type="xs:int" oor:nillable="false"> + <info> + <desc>Action needed for opening document with macro with Windows zone + identifier URLZONE_INTRANET (1, local machine).</desc> + </info> + <constraints> + <enumeration oor:value="0"> + <info> + <desc>Ask</desc> + </info> + </enumeration> + <enumeration oor:value="1"> + <info> + <desc>Allow</desc> + </info> + </enumeration> + <enumeration oor:value="2"> + <info> + <desc>Deny</desc> + </info> + </enumeration> + </constraints> + <value>0</value> + </prop> + <prop oor:name="ZoneTrusted" oor:type="xs:int" oor:nillable="false"> + <info> + <desc>Action needed for opening document with macro with Windows zone + identifier URLZONE_TRUSTED (2, trusted).</desc> + </info> + <constraints> + <enumeration oor:value="0"> + <info> + <desc>Ask</desc> + </info> + </enumeration> + <enumeration oor:value="1"> + <info> + <desc>Allow</desc> + </info> + </enumeration> + <enumeration oor:value="2"> + <info> + <desc>Deny</desc> + </info> + </enumeration> + </constraints> + <value>0</value> + </prop> + <prop oor:name="ZoneInternet" oor:type="xs:int" oor:nillable="false"> + <info> + <desc>Action needed for opening document with macro with Windows zone + identifier URLZONE_INTERNET (3, internet).</desc> + </info> + <constraints> + <enumeration oor:value="0"> + <info> + <desc>Ask</desc> + </info> + </enumeration> + <enumeration oor:value="1"> + <info> + <desc>Allow</desc> + </info> + </enumeration> + <enumeration oor:value="2"> + <info> + <desc>Deny</desc> + </info> + </enumeration> + </constraints> + <value>0</value> + </prop> + <prop oor:name="ZoneUntrusted" oor:type="xs:int" oor:nillable="false"> + <info> + <desc>Action needed for opening document with macro with Windows zone + identifier URLZONE_UNTRUSTED (3, untrusted source).</desc> + </info> + <constraints> + <enumeration oor:value="0"> + <info> + <desc>Ask</desc> + </info> + </enumeration> + <enumeration oor:value="1"> + <info> + <desc>Allow</desc> + </info> + </enumeration> + <enumeration oor:value="2"> + <info> + <desc>Deny</desc> + </info> + </enumeration> + </constraints> + <value>0</value> + </prop> + </group> </group> </group> <group oor:name="View"> diff --git a/sfx2/source/doc/docmacromode.cxx b/sfx2/source/doc/docmacromode.cxx index 887c672a3ffd..97d0001e10a4 100644 --- a/sfx2/source/doc/docmacromode.cxx +++ b/sfx2/source/doc/docmacromode.cxx @@ -38,6 +38,11 @@ #include <tools/diagnose_ex.h> #include <tools/urlobj.hxx> +#if defined(_WIN32) +#include <officecfg/Office/Common.hxx> +#include <systools/win32/comtools.hxx> +#include <urlmon.h> +#endif namespace sfx2 { @@ -284,6 +289,59 @@ namespace sfx2 } } +#if defined(_WIN32) + // Windows specific: try to decide macros loading depending on Windows Security Zones + // (is the file local, or it was downloaded from internet, etc?) + OUString sURL(m_xData->m_rDocumentAccess.getDocumentLocation()); + OUString sFilePath; + osl::FileBase::getSystemPathFromFileURL(sURL, sFilePath); + sal::systools::COMReference<IZoneIdentifier> pZoneId; + pZoneId.CoCreateInstance(CLSID_PersistentZoneIdentifier); + sal::systools::COMReference<IPersistFile> pPersist(pZoneId, sal::systools::COM_QUERY_THROW); + DWORD dwZone; + if (!SUCCEEDED(pPersist->Load(reinterpret_cast<LPCOLESTR>(sFilePath.getStr()), STGM_READ)) || + !SUCCEEDED(pZoneId->GetId(&dwZone))) + { + // no Security Zone info found -> assume a local file, not + // from the internet + dwZone = 0; + } + + // determine action from zone and settings + sal_Int32 nAction = 0; + switch (dwZone) { + case 0: + nAction = officecfg::Office::Common::Security::Scripting::WindowsSecurityZone::ZoneLocal::get(); + break; + case 1: + nAction = officecfg::Office::Common::Security::Scripting::WindowsSecurityZone::ZoneIntranet::get(); + break; + case 2: + nAction = officecfg::Office::Common::Security::Scripting::WindowsSecurityZone::ZoneTrusted::get(); + break; + case 3: + nAction = officecfg::Office::Common::Security::Scripting::WindowsSecurityZone::ZoneInternet::get(); + break; + case 4: + nAction = officecfg::Office::Common::Security::Scripting::WindowsSecurityZone::ZoneUntrusted::get(); + break; + default: + // unknown zone, let's ask the user + nAction = 0; + break; + } + + // act on result + switch (nAction) + { + case 0: // Ask + break; + case 1: // Allow + return allowMacroExecution(); + case 2: // Deny + return disallowMacroExecution(); + } +#endif // confirmation is required bool bSecure = false; diff --git a/sw/CppunitTest_sw_odfimport.mk b/sw/CppunitTest_sw_odfimport.mk index 718770f090f5..2c43890a0d68 100644 --- a/sw/CppunitTest_sw_odfimport.mk +++ b/sw/CppunitTest_sw_odfimport.mk @@ -48,6 +48,10 @@ $(eval $(call gb_CppunitTest_set_include,sw_odfimport,\ $$(INCLUDE) \ )) +$(eval $(call gb_CppunitTest_use_system_win32_libs,sw_odfimport,\ + ole32 \ +)) + $(eval $(call gb_CppunitTest_use_api,sw_odfimport,\ udkapi \ offapi \ @@ -65,4 +69,8 @@ $(eval $(call gb_CppunitTest_add_arguments,sw_odfimport, \ -env:arg-env=$(gb_Helper_LIBRARY_PATH_VAR)"$$$${$(gb_Helper_LIBRARY_PATH_VAR)+=$$$$$(gb_Helper_LIBRARY_PATH_VAR)}" \ )) +$(eval $(call gb_CppunitTest_use_custom_headers,sw_odfimport,\ + officecfg/registry \ +)) + # vim: set noet sw=4 ts=4: diff --git a/sw/qa/extras/odfimport/data/ZoneMacroTest.odt b/sw/qa/extras/odfimport/data/ZoneMacroTest.odt new file mode 100644 index 000000000000..01847632c637 Binary files /dev/null and b/sw/qa/extras/odfimport/data/ZoneMacroTest.odt differ diff --git a/sw/qa/extras/odfimport/odfimport.cxx b/sw/qa/extras/odfimport/odfimport.cxx index b6f242601f5f..dc6ee2945d89 100644 --- a/sw/qa/extras/odfimport/odfimport.cxx +++ b/sw/qa/extras/odfimport/odfimport.cxx @@ -41,8 +41,10 @@ #include <com/sun/star/text/XTextFramesSupplier.hpp> #include <com/sun/star/document/XDocumentInsertable.hpp> #include <com/sun/star/style/ParagraphAdjust.hpp> +#include <com/sun/star/document/MacroExecMode.hpp> #include <comphelper/propertysequence.hxx> +#include <comphelper/propertyvalue.hxx> #include <editeng/boxitem.hxx> #include <IDocumentSettingAccess.hxx> @@ -57,6 +59,14 @@ #include <unotxdoc.hxx> #include <frmatr.hxx> +#if defined(_WIN32) +#include <officecfg/Office/Common.hxx> +#include <osl/file.hxx> +#include <unotools/securityoptions.hxx> +#include <systools/win32/comtools.hxx> +#include <urlmon.h> +#endif + typedef std::map<OUString, css::uno::Sequence< css::table::BorderLine> > AllBordersMap; typedef std::pair<OUString, css::uno::Sequence< css::table::BorderLine> > StringSequencePair; @@ -1352,5 +1362,126 @@ CPPUNIT_TEST_FIXTURE(Test, testForcepoint101) load(mpTestDocumentPath, "forcepoint101.fodt"); } +#ifdef _WIN32 +template <class T> +void runWindowsFileZoneTests(css::uno::Reference<css::frame::XDesktop2> aDesktop, + const OUString& sFileName, sal_Int32 configValue, sal_Int32 zoneId, + sal_Bool expectedResult) +{ + // Set desired configuration params + auto xChanges = comphelper::ConfigurationChanges::create(); + T::set(configValue, xChanges); + xChanges->commit(); + + // Set Windows Security Zone for temp file + sal::systools::COMReference<IZoneIdentifier> pZoneId; + pZoneId.CoCreateInstance(CLSID_PersistentZoneIdentifier); + + // ignore setting of Zone 0, since at least for Windows Server + // setups, that always leads to E_ACCESSDENIED - presumably since + // the file is already local? + // + // See below for the workaround (calling tests for ZONE_LOCAL + // first) + if( zoneId != 0 ) + { + CPPUNIT_ASSERT(SUCCEEDED(pZoneId->SetId(zoneId))); + sal::systools::COMReference<IPersistFile> pPersist(pZoneId, sal::systools::COM_QUERY_THROW); + OUString sTempFileWinPath; + osl::FileBase::getSystemPathFromFileURL(sFileName, sTempFileWinPath); + CPPUNIT_ASSERT( + SUCCEEDED(pPersist->Save(reinterpret_cast<LPCOLESTR>(sTempFileWinPath.getStr()), TRUE))); + } + + // Load doc with default for UI settings: do not suppress macro + uno::Sequence<beans::PropertyValue> aLoadArgs{ comphelper::makePropertyValue( + "MacroExecutionMode", css::document::MacroExecMode::USE_CONFIG) }; + auto aComponent = aDesktop->loadComponentFromURL(sFileName, "_default", 0, aLoadArgs); + + // Are macro enabled in doc? + SwXTextDocument* pTextDoc = dynamic_cast<SwXTextDocument*>(aComponent.get()); + CPPUNIT_ASSERT_EQUAL(expectedResult, pTextDoc->getAllowMacroExecution()); + + aComponent->dispose(); +} +#endif + +CPPUNIT_TEST_FIXTURE(Test, testWindowsFileZone) +{ +// This makes sense only for Windows +#ifdef _WIN32 + // Create a temp copy of zone test file + utl::TempFile aTempFile; + aTempFile.EnableKillingFile(); + SvStream& aStreamDst = *aTempFile.GetStream(StreamMode::WRITE); + SvFileStream aStreamSrc(m_directories.getURLFromSrc(mpTestDocumentPath) + "ZoneMacroTest.odt", StreamMode::READ); + aStreamDst.WriteStream(aStreamSrc); + aTempFile.CloseStream(); + + // Tweak macro security to 1 + SvtSecurityOptions::SetMacroSecurityLevel(1); + + // Run all tests: set for temp file security zone and then check if macro are enabled + // depending on configuration values for given zone + // There is no easy way to check default (0) variant, so macro are disabled by default in these tests. + + // run tests for ZoneLocal first, since runWindowsFileZoneTests + // ignores Zone 0 (see above) - assuming the initial file state is + // local after a copy, we're still triggering the expected + // behaviour + runWindowsFileZoneTests< + officecfg::Office::Common::Security::Scripting::WindowsSecurityZone::ZoneLocal>( + mxDesktop, aTempFile.GetURL(), 0, 0, false); + runWindowsFileZoneTests< + officecfg::Office::Common::Security::Scripting::WindowsSecurityZone::ZoneLocal>( + mxDesktop, aTempFile.GetURL(), 1, 0, true); + runWindowsFileZoneTests< + officecfg::Office::Common::Security::Scripting::WindowsSecurityZone::ZoneLocal>( + mxDesktop, aTempFile.GetURL(), 2, 0, false); + + // run tests for other zones (these actually set the Windows + // Security Zone at the file) + runWindowsFileZoneTests< + officecfg::Office::Common::Security::Scripting::WindowsSecurityZone::ZoneUntrusted>( + mxDesktop, aTempFile.GetURL(), 0, 4, false); + runWindowsFileZoneTests< + officecfg::Office::Common::Security::Scripting::WindowsSecurityZone::ZoneUntrusted>( + mxDesktop, aTempFile.GetURL(), 1, 4, true); + runWindowsFileZoneTests< + officecfg::Office::Common::Security::Scripting::WindowsSecurityZone::ZoneUntrusted>( + mxDesktop, aTempFile.GetURL(), 2, 4, false); + + runWindowsFileZoneTests< + officecfg::Office::Common::Security::Scripting::WindowsSecurityZone::ZoneInternet>( + mxDesktop, aTempFile.GetURL(), 0, 3, false); + runWindowsFileZoneTests< + officecfg::Office::Common::Security::Scripting::WindowsSecurityZone::ZoneInternet>( + mxDesktop, aTempFile.GetURL(), 1, 3, true); + runWindowsFileZoneTests< + officecfg::Office::Common::Security::Scripting::WindowsSecurityZone::ZoneInternet>( + mxDesktop, aTempFile.GetURL(), 2, 3, false); + + runWindowsFileZoneTests< + officecfg::Office::Common::Security::Scripting::WindowsSecurityZone::ZoneTrusted>( + mxDesktop, aTempFile.GetURL(), 0, 2, false); + runWindowsFileZoneTests< + officecfg::Office::Common::Security::Scripting::WindowsSecurityZone::ZoneTrusted>( + mxDesktop, aTempFile.GetURL(), 1, 2, true); + runWindowsFileZoneTests< + officecfg::Office::Common::Security::Scripting::WindowsSecurityZone::ZoneTrusted>( + mxDesktop, aTempFile.GetURL(), 2, 2, false); + + runWindowsFileZoneTests< + officecfg::Office::Common::Security::Scripting::WindowsSecurityZone::ZoneIntranet>( + mxDesktop, aTempFile.GetURL(), 0, 1, false); + runWindowsFileZoneTests< + officecfg::Office::Common::Security::Scripting::WindowsSecurityZone::ZoneIntranet>( + mxDesktop, aTempFile.GetURL(), 1, 1, true); + runWindowsFileZoneTests< + officecfg::Office::Common::Security::Scripting::WindowsSecurityZone::ZoneIntranet>( + mxDesktop, aTempFile.GetURL(), 2, 1, false); +#endif +} + CPPUNIT_PLUGIN_IMPLEMENT(); /* vim:set shiftwidth=4 softtabstop=4 expandtab: */
