Bug 97694 - Base macros cannot be digitally signed
Summary: Base macros cannot be digitally signed
Status: NEW
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: Base (show other bugs)
Version:
(earliest affected)
4.4.7.2 release
Hardware: All All
: medium enhancement
Assignee: Not Assigned
URL:
Whiteboard: target:7.0.0 target:6.4.5 target:7.1.0
Keywords:
: 117766 (view as bug list)
Depends on:
Blocks: Digital-Signatures Macro
  Show dependency treegraph
 
Reported: 2016-02-09 16:27 UTC by Manuel Fombuena
Modified: 2021-07-16 20:31 UTC (History)
7 users (show)

See Also:
Crash report or crash signature:


Attachments
Split sfx::SignaturesHandler from SfxObjectShell (85.27 KB, patch)
2020-03-13 16:53 UTC, Jan-Marek Glogowski
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Manuel Fombuena 2016-02-09 16:27:29 UTC
Issue: according to LibreOffice's help, it is possible to sign a macro (https://help.libreoffice.org/Common/Applying_Digital_Signatures). However, in Base, if you go to "Tools" > "Macros" > "Organise Macros" > (select the macro you want to edit/sign) > "Edit" and then select "File" > "Digital Signatures", the window to sign the macro won't come up. On the other hand, these steps in Writer would work to sign a macro. To make things more interesting, if you have Base and Writer open at the same time, then the macro editor will show macros of both Base and Writer but "File" > "Digital Signatures" won't open the window.

I have tested this in LO 4.4.7.2 (Windows 10 and Fedora 22), 5.0.3.2 (Windows 10), 5.0.4.2 (Windows 10 and Fedora 23) and 5.0.5.1 rc (Windows 10). I have also tried Windows XP but I cannot remember which version of LO.

I suppose that the expected behaviour is that the window "Digital Signatures" would come up when "File" > "Digital Signatures" while editing a macro in Base.
Comment 1 Buovjaga 2016-02-12 08:54:08 UTC
Repro.

Win 7 Pro 64-bit Version: 5.2.0.0.alpha0+
Build ID: a6f876d45bd4e41a7143594a6cb11b6893a0f620
CPU Threads: 4; OS Version: Windows 6.1; UI Render: default; 
TinderBox: Win-x86@39, Branch:master, Time: 2016-02-11_00:07:38
Locale: fi-FI (fi_FI)
Comment 2 Manuel Fombuena 2016-04-20 08:56:54 UTC
Is this bug going to be addressed?
Comment 3 Manuel Fombuena 2016-04-20 08:57:08 UTC Comment hidden (obsolete)
Comment 4 Buovjaga 2016-04-20 11:11:44 UTC
(In reply to Manuel Fombuena from comment #3)
> Is this bug going to be addressed?

Probably? We have about 8000 open bug reports: https://bugs.documentfoundation.org/page.cgi?id=weekly-bug-summary.html

Adding needsDevEval in case this turns out to be easyhack material.
Comment 5 Gerhard Schaber 2017-01-12 11:52:36 UTC
Does it make sense to have macro security set to High (only signed macros can be executed) by default, if it is not possible to sign macros at all?
Comment 6 QA Administrators 2018-01-13 03:30:01 UTC Comment hidden (obsolete)
Comment 7 Manuel Fombuena 2018-01-15 18:21:42 UTC
Version: 5.4.4.2 (x64)
Build ID: 2524958677847fb3bb44820e40380acbe820f960
CPU threads: 4; OS: Windows 6.19; UI render: default; 
Locale: en-GB (en_GB); Calc: group

No changes, the bug is unresolved
Comment 8 Drew Jensen 2018-05-30 02:35:32 UTC
*** Bug 117766 has been marked as a duplicate of this bug. ***
Comment 9 Oliver Brinzing 2019-07-26 14:48:27 UTC
reproducible with:

Version: 6.2.5.2 (x64)
Build ID: 1ec314fa52f458adc18c4f025c545a4e8b22c159
CPU threads: 4; OS: Windows 10.0; UI render: default; VCL: win; 
Locale: de-DE (de_DE); UI-Language: en-US
Calc:
Comment 10 Jan-Marek Glogowski 2020-03-11 11:22:30 UTC
There is also "Tools >> Macros >> Digital Signatures...", which is grayed out for me in Base, but works in Writer. Macros in ODT and ODB are stored in in the same XML files, so I see no reason, why they should not be signed technically.

Same goes for the ODF spec 1.2 "Part 3: Packages", "3.5 Digital Signatures", which basically states, that all files of an ODF can have a signature stored in some "META-INF/*signatures*" file.
Comment 11 Jan-Marek Glogowski 2020-03-11 14:37:59 UTC
So just some update. This is no easy hack! While looking into the code, I found Base has not much common ground with the macro signing implementation in sfx2, which is used by all other modules. The result is, that it gets more complicated to implement this properly. I'm still hoping for not too much code duplication. I was told, converting Base to "proper" sfx2 would be a multi-week endeavor. I still hope for some acceptable Base-specific solution.
Comment 12 Jan-Marek Glogowski 2020-03-12 12:58:25 UTC
I spend a day to try to tackle the problem a bit and analyze what a possible solution could look like. The main problem is Base not using SfxObjectShell at all, so all the signature handling is simply not available to it.

Making it worse, Base has generally its own way to do "stuff", like UNO dispatching. I tried to copy some relevant code from SfxObjectShell(_Impl) into Base, as this code is normally handled internally and not exposed via UNO. And Base internally just passes XModel around, so you don't even have access to the Macro testing functions of the implementation class (could be solved with UNOTunnel). But at some point of a few hundred lines of non-compiling code mess, I gave up on this approach. This is security code and the result would simply be unmaintainable and error prone, where one code side is patched and the other not.

My current approach is to factor out the signature handling of SfxObjectShell into a separate class, which then could also be used by Base, without rewriting Base to use SfxObjectShell. My initial approach also included the ::sfx2::DocumentMacroMode in that class, but that turned out to make it a lot larger. But luckily, in the signature code there is just a single call to it, so this became an other abstract call to be implemented by the users of that class. In the end I settled down on just handling the dialogs, the m_nDocumentSignatureState and the m_nScriptingSignatureState.

That factored out code already exists and still seem to work for the default SfxObjectShell use case (xmlsecurity tests still pass). The Base class currently just has stub code for the missing interface classes, so it compiles.

But there is an other potential problem. It seems Base can have various sources of Macro code. I don't know, why they seem to be handled separately. From ModelImpl.hxx:

    enum EmbeddedMacros
    {
        // the database document (storage) itself contains macros
        eDocumentWideMacros,
        // there are sub document( storage)s containing macros
        eSubDocumentMacros,
        // there are no known macro( storage)s
        eNoMacros
    };

IMHO the only thing reasonable is to have all or no signed macros, so this might not result in a large problem.

What I didn't yet look into is the whole DocumentMacroMode handling in Base, which is the real code checking the signatures and allowing macro execution. Eventually this would move all the DocumentMacroMode from the SfxObjectShell into the new class or add an other one. There is already some DocumentMacroMode code in Base, but since you can't sign anything, at least the signature handling was probably never even tested.
Comment 13 Drew Jensen 2020-03-12 17:08:01 UTC
(In reply to Jan-Marek Glogowski from comment #12)
> 
> But there is an other potential problem. It seems Base can have various
> sources of Macro code. I don't know, why they seem to be handled separately.

Some history here:

When Base was first introduced embedded script libraries where only supported for embedded forms and reports but not at the overall .odb container level.

That was change (don't recall which version) to only being at the .odb container level. When that happened code was introduced to convert the old style file layout to the new, moving any libraries from the embedded forms and reports to the database document container.
Comment 14 Jan-Marek Glogowski 2020-03-13 16:53:49 UTC
Created attachment 158670 [details]
Split sfx::SignaturesHandler from SfxObjectShell

I don't have time to finish the implementation, so here is my current state. Might be useful for someone in the future.
Comment 15 Commit Notification 2020-03-23 15:15:10 UTC
Samuel Mehrbrodt committed a patch related to this issue.
It has been pushed to "master":

https://git.libreoffice.org/core/commit/f2f93434f4795646255e5d8edd31fa08b8b2ffab

Related tdf#97694 Check Base macro signatures on load

It will be available in 7.0.0.

The patch should be included in the daily builds available at
https://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
https://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.
Comment 16 Commit Notification 2020-03-31 07:08:00 UTC
Samuel Mehrbrodt committed a patch related to this issue.
It has been pushed to "master":

https://git.libreoffice.org/core/commit/6c89715b7d1e78a17b6c53cdb89f2f734500fd44

Related tdf#97694 Show dialog when signed macros are blocked

It will be available in 7.0.0.

The patch should be included in the daily builds available at
https://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
https://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.
Comment 17 Commit Notification 2020-04-01 10:32:41 UTC
Samuel Mehrbrodt committed a patch related to this issue.
It has been pushed to "master":

https://git.libreoffice.org/core/commit/8d27d495a733cfc29753e2e78000b199632695f7

Related tdf#97694 Preserve macro signatures in Base

It will be available in 7.0.0.

The patch should be included in the daily builds available at
https://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
https://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.
Comment 18 Commit Notification 2020-04-08 10:12:11 UTC
Samuel Mehrbrodt committed a patch related to this issue.
It has been pushed to "master":

https://git.libreoffice.org/core/commit/792c730f3cda4f5c312921a62ecae3d322ad9b72

Related tdf#97694 Fix macro preservation on Windows

It will be available in 7.0.0.

The patch should be included in the daily builds available at
https://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
https://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.
Comment 19 Samuel Mehrbrodt (allotropia) 2020-04-08 10:17:46 UTC
So the current state is that, if a ODB already has signed macros, those signatures are preserved on save (as long as the macro hasn't been edited).

Still missing is the actual signing of macros via GUI. Signing them via API works btw.
Comment 20 Commit Notification 2020-05-01 15:49:50 UTC
Samuel Mehrbrodt committed a patch related to this issue.
It has been pushed to "libreoffice-6-4":

https://git.libreoffice.org/core/commit/53006b7618b02b6744feddaea051effc72a17bfa

Related tdf#97694 Check Base macro signatures on load

It will be available in 6.4.5.

The patch should be included in the daily builds available at
https://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
https://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.
Comment 21 Commit Notification 2020-05-01 15:51:11 UTC
Samuel Mehrbrodt committed a patch related to this issue.
It has been pushed to "libreoffice-6-4":

https://git.libreoffice.org/core/commit/172d4990635d46355ac432b89ab520776909e48c

Related tdf#97694 Show dialog when signed macros are blocked

It will be available in 6.4.5.

The patch should be included in the daily builds available at
https://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
https://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.
Comment 22 Commit Notification 2020-05-01 15:51:19 UTC
Samuel Mehrbrodt committed a patch related to this issue.
It has been pushed to "libreoffice-6-4":

https://git.libreoffice.org/core/commit/c3e021363a72930e9cbaeffc47cd49074929e7cb

Related tdf#97694 Preserve macro signatures in Base

It will be available in 6.4.5.

The patch should be included in the daily builds available at
https://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
https://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.
Comment 23 Commit Notification 2020-05-01 15:55:02 UTC
Samuel Mehrbrodt committed a patch related to this issue.
It has been pushed to "libreoffice-6-4":

https://git.libreoffice.org/core/commit/8aa2a3fdeec9007e6d175d16e4cfe43220f6d6a6

Related tdf#97694 Fix macro preservation on Windows

It will be available in 6.4.5.

The patch should be included in the daily builds available at
https://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
https://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.
Comment 24 Stephan 2020-05-22 12:32:32 UTC
I can confirm that bug, but I do not understand the limitation to Base macros, and especially I do not understand the commit f2f93434f4795646255e5d8edd31fa08b8b2ffab, since I never saw a LibreOffice version on Windows that was able to sign a macro, not in Base and not in any other application. 

All Libreoffice versions I saw since 2014 just have the Tools - Macros - Digital Signature option greyed out and deactivated.

I also would like to know what the meaning of this help file is:
https://help.libreoffice.org/6.4/en-US/text/shared/guide/digitalsign_send.html#par_idN106E0

So when Libreoffice cannot sign macros, why should it be able to evaluate macro signatures?

If there is a way to enable that, please let me know.
Comment 25 Samuel Mehrbrodt (allotropia) 2020-05-26 06:16:17 UTC
(In reply to Freischreiber from comment #24)
> I can confirm that bug, but I do not understand the limitation to Base
> macros, and especially I do not understand the commit
> f2f93434f4795646255e5d8edd31fa08b8b2ffab, since I never saw a LibreOffice
> version on Windows that was able to sign a macro, not in Base and not in any
> other application. 
> 
> All Libreoffice versions I saw since 2014 just have the Tools - Macros -
> Digital Signature option greyed out and deactivated.
> 
> I also would like to know what the meaning of this help file is:
> https://help.libreoffice.org/6.4/en-US/text/shared/guide/digitalsign_send.
> html#par_idN106E0
> 
> So when Libreoffice cannot sign macros, why should it be able to evaluate
> macro signatures?
> 
> If there is a way to enable that, please let me know.

It is possible to sign macros in most LO apps since a long time.
In Base it's currently not possible via the UI, but people can still have their own method to sign the macros in the .odb files (for example it's possible to write a script to sign macros using Basic API).

So there exist odb files with signed macros, and the commit was to evaluate those on load.
Comment 26 Stephan 2020-05-26 16:29:45 UTC
Oh I see, the macros must be part of the document, so that LO can sign them, not part of the My Macros library container. (!)

I was convinced LO cannot do that, since all my macros are in the library container...

A little bit off-topic, but: Why does LO forget the signature of macros in a template file when a document is produced from it? The macros are exported to the document, but the signature is lost...

Thank you.
Comment 27 Oliver Brinzing 2020-05-29 04:46:53 UTC
(In reply to Stephan from comment #26)

> A little bit off-topic, but: Why does LO forget the signature of macros in a
> template file when a document is produced from it? The macros are exported
> to the document, but the signature is lost...

this should be fixed with:
Bug 42316 - Creating a new document from a template removes the signature from all Macros
Comment 28 Commit Notification 2020-06-23 14:08:00 UTC
Samuel Mehrbrodt committed a patch related to this issue.
It has been pushed to "master":

https://git.libreoffice.org/core/commit/29cb36cbee9c3ff5e73bc7a6d6a2f365c5c62da7

tdf#97694 Add test for macro signature preservation in Base

It will be available in 7.1.0.

The patch should be included in the daily builds available at
https://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
https://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.
Comment 29 Julien Nabet 2021-07-16 20:31:27 UTC
During tests on https://bugs.documentfoundation.org/show_bug.cgi?id=143400 "Query with most functions could not be created in GUI", I noticed this log:
warn:dbaccess:29276:29276:dbaccess/source/core/dataaccess/ModelImpl.cxx:932:  com.sun.star.container.NoSuchElementException message: /home/julien/lo/libreoffice/package/source/xstor/xstorage.cxx:2978:  /home/julien/lo/libreoffice/package/source/xstor/xstorage.cxx:2978

Debugging a bit, I noticed it throwed in dbaccess/source/core/dataaccess/ModelImpl.cxx:
901 xMetaInf->copyElementTo(aScriptSignName, xTargetMetaInf, aScriptSignName);
(see https://opengrok.libreoffice.org/xref/core/dbaccess/source/core/dataaccess/ModelImpl.cxx?r=24a3d4ee#901)
gdb indicates "aScriptSignName" contains "macrosignatures.xml".


But I don't understand why LO goes on this part.
Indeed, bTryToPreserveScriptSignature is false by default but then:
    850     bool bIsEmbedded = sLocation.startsWith("vnd.sun.star.pkg:") && sLocation.endsWith("/EmbeddedDatabase");
    851     if (!bIsEmbedded && !sLocation.isEmpty()
    852         && (aSignatureState == SignatureState::OK || aSignatureState == SignatureState::NOTVALIDATED
    853             || aSignatureState == SignatureState::INVALID
    854             || aSignatureState == SignatureState::UNKNOWN))
    855     {
    856         bTryToPreserveScriptSignature = true;
...

The odb file from tdf#143400 uses embedded HSQLDB so it seems this line:
bool bIsEmbedded = sLocation.startsWith("vnd.sun.star.pkg:") && sLocation.endsWith("/EmbeddedDatabase");
is wrong.

On gdb, I got:
(gdb) p sLocation
$3 = "file:///tmp/DefaultDate.odb"
so bIsEmbedded can't be true in my case.

Unless we don't talk about the "embeddedness" of the database but the "embeddedness" about something else?