Created attachment 190684 [details] Unsigned document with signed macros Since commit 1dc71daf7fa7204a98c75dac680af664ab9c8edb (Improve macro checks, 2021-01-28) it is impossible to run signed macros inside unsigned document in Medium security level. 0. Use Medium macro security level 1. Use attachment 144453 [details] from bug 119507, which is a template with signed macros, to create a new document (allow macros); 2. See that it is possible to run the macro by clicking the button; 3. Save the document, and reload (allow macros!); => See that there's a "Macros are signed, but the document (containing document events) is not signed." infobar. Clicking the button does not run the macro; manually launching the macro from IDE gives "For security reasons, you cannot run this macro." error box. It does not make sense to disallow signed macros is a situation when unsigned macros were allowed - given everything else equal. The information about the fact that the document is unsigned could be provided as an additional data point in the initial warning dialog, where the user made their choice; but as soon as they decided to allow macros, they must not be blocked. The attachment is a document created using steps 1-3. Note also, that this change seems to contradict with the underlying principles, that show e.g. in help [1] about Medium security level: > Confirmation required before executing macros from unknown sources. > > Trusted sources can be set on the Trusted Sources tab page. Signed macros from > a trusted source are allowed to run. In addition, any macro from a trusted > file location is allowed to run. All other macros require your confirmation. The ability to separately sign macros is there to allow users to see that the code that is running comes from a trusted source. Indeed, the document may be modified to change the behavior; but that's always the case when you have documents with macros - if you don't know their source, you have a risk; and it is no greater when the macros are signed. [1] https://help.libreoffice.org/24.2/en-US/text/shared/optionen/macrosecurity_sl.html
Current implementation of DocumentMacroMode::adjustMacroMode results in this (as an example, take Medium macro security level, and a document with macro bound to events, not in trusted location): > Document is NOT signed, macro is unsigned: ask and follow the choice (OK) > Document is NOT signed, macro is signed trusted: deny silently unconditionally (?) > Document is NOT signed, macro is signed untrusted: ask, then deny unconditionally (???) > Document is NOT signed, macro is signed broken: deny silently unconditionally (?) > Document is NOT signed, macro is signed invalid: ask and follow the choice (! IMO OK) > > Document is signed, macro is unsigned: ask and follow the choice (OK) > Document is signed, macro is signed trusted: allow silently unconditionally (OK) > Document is signed, macro is signed untrusted: ask and follow the choice (OK) > Document is signed, macro is signed broken: deny silently unconditionally (?) > Document is signed, macro is signed invalid: ask and follow the choice (! IMO OK) And here questions arise: why unsigned document and unsigned macro is less dangerous than unsigned document and a macro signed with a valid trusted signature? The latter is so much dangerous, that user can't use that, unless they set their security to the lowest level possible? Why, at the same time, unsigned document with a macro having an INVALID signature allows user to make their choice? Why the explicit choice made when the macro is valid but untrusted, is ignored when document is unsigned? I think, that the status of document signature should not matter here. In fact, current implementation disallows most reasonable use of macro signing in organizations, where administrators might want to restrict to use of signed macros only; but requiring every document having these macros be signed would be overkill; and in case of databases, that would be simply impossible (note that databases allow macros signed using API).
https://gerrit.libreoffice.org/c/core/+/159101 This change fixed "Document is NOT signed, macro is signed trusted" and "Document is NOT signed, macro is signed untrusted" cases from comment 1. Specifically, with it: * Document is NOT signed, macro is signed trusted: allow silently unconditionally * Document is NOT signed, macro is signed untrusted: ask and follow the choice
Mike Kaganski committed a patch related to this issue. It has been pushed to "master": https://git.libreoffice.org/core/commit/85a8c29e26f0bf48906312103e57246685d32c7e tdf#158090: Limit signed document requirement to High security level It will be available in 24.2.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.
Mike Kaganski committed a patch related to this issue. It has been pushed to "master": https://git.libreoffice.org/core/commit/ad6a8cf99b627d13f998ffcd7452a45386c649f5 tdf#158090: Do not auto-reject SignatureState::BROKEN in ALWAYS_EXECUTE case It will be available in 24.2.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.
The commit mentioned in comment 4 addresses "macro is signed broken" cases (both signed and unsigned documents) - now it will ask in Medium security mode.
As written, Comment 2 now allows signed macros in Medium security. And in High security, ew have a warning "Macros disabled". Seems good to me. Any reason this is still open?
With the above fixes, yes, it is fixed. And I see no reason to hurry with closing so much that asking for reasons the same day the last commit was merged - is it putting that much load on something, that I close it a day later, say, when I made sure?
It was not a hurry, wanted to ask if there is expected something else, as I already tested this. Thanks.
Mike Kaganski committed a patch related to this issue. It has been pushed to "libreoffice-7-6": https://git.libreoffice.org/core/commit/1042dda7d2e68b1a4da8fc321c9415c9cad2ca21 tdf#158090: Limit signed document requirement to High security level It will be available in 7.6.4. 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.
Mike Kaganski committed a patch related to this issue. It has been pushed to "libreoffice-7-6": https://git.libreoffice.org/core/commit/5aed270403d4818c9c5a6b9879e799185759b347 tdf#158090: Do not auto-reject SignatureState::BROKEN in ALWAYS_EXECUTE case It will be available in 7.6.4. 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.