Bug 158576 - Improve macro signature confirmation dialog
Summary: Improve macro signature confirmation dialog
Status: RESOLVED FIXED
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: UI (show other bugs)
Version:
(earliest affected)
24.2.0.0 alpha1+
Hardware: All All
: medium enhancement
Assignee: Samuel Mehrbrodt (allotropia)
URL:
Whiteboard: target:24.2.0 target:24.8.0 target:24...
Keywords:
Depends on:
Blocks: Digital-Signatures
  Show dependency treegraph
 
Reported: 2023-12-07 09:52 UTC by Samuel Mehrbrodt (allotropia)
Modified: 2023-12-18 12:32 UTC (History)
2 users (show)

See Also:
Crash report or crash signature:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Samuel Mehrbrodt (allotropia) 2023-12-07 09:52:41 UTC
Macro Security: High (default)

Open this file: https://cgit.freedesktop.org/libreoffice/core/plain/xmlsecurity/test_docs/documents/valid_ooo3_2_doc_macro.odt

When opening this file (a document with a signed macro), you need to confirm that you trust the author of the certificate (because only signed macros from trusted sources/locations are allowed to run).
There is only a small hurdle to add the certificate to the list of trusted certificates (Just check the "Always trust macros from this source" checkbox.

The user is not required to check the certificate for validity.

This should be improved. At the very least, the user should be required to actually view the certificate before the checkbox is enabled.

Maybe relevant instructions/information about the certificate can also be added to the dialog directly, instead of hiding it behind the "View certificate" button.
Comment 1 Heiko Tietze 2023-12-07 10:01:58 UTC
Does it help if one can enable the macro/s without always trusting the certificate? In other words to have the button "Enable Macros" always enabled.
Comment 2 Samuel Mehrbrodt (allotropia) 2023-12-07 10:15:17 UTC
(In reply to Heiko Tietze from comment #1)
> Does it help if one can enable the macro/s without always trusting the
> certificate? In other words to have the button "Enable Macros" always
> enabled.

That wouldn't help with Macro security mode "High" where only macros from trusted sources are allowed to run.
Comment 3 Heiko Tietze 2023-12-07 10:21:49 UTC
(In reply to Samuel Mehrbrodt (allotropia) from comment #2)
> That wouldn't help with Macro security mode "High" where only macros from
> trusted sources are allowed to run.

High or Very High... you need to _always_ trust the certificate and my proposal was to "trust" it only once.

Showing the certificate in the dialog is a bit odd. We could change the "Enable" action into "Verify certificate" which would open the respective dialog.
Comment 4 Samuel Mehrbrodt (allotropia) 2023-12-07 10:43:46 UTC
(In reply to Heiko Tietze from comment #3)
> (In reply to Samuel Mehrbrodt (allotropia) from comment #2)
> > That wouldn't help with Macro security mode "High" where only macros from
> > trusted sources are allowed to run.
> 
> High or Very High... you need to _always_ trust the certificate and my
> proposal was to "trust" it only once.

Please read the description of the macro security level in the options dialog. What you describe only works in Medium level.

> Showing the certificate in the dialog is a bit odd. We could change the
> "Enable" action into "Verify certificate" which would open the respective
> dialog.

Yeah, or just show the relevant information in the dialog (and highlight if something looks weird if that's possible). Most users don't really know how to interpret the ceritificate dialog.
Comment 5 Commit Notification 2023-12-07 18:00:11 UTC
Samuel Mehrbrodt committed a patch related to this issue.
It has been pushed to "master":

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

tdf#158576 Require viewing the certificate

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.
Comment 6 Heiko Tietze 2023-12-08 08:44:56 UTC
(In reply to Commit Notification from comment #5)
> Samuel Mehrbrodt committed a patch related to this issue.

Guess you plan adding another patch later, right?
Comment 7 Samuel Mehrbrodt (allotropia) 2023-12-11 07:00:00 UTC
(In reply to Heiko Tietze from comment #6)
> (In reply to Commit Notification from comment #5)
> > Samuel Mehrbrodt committed a patch related to this issue.
> 
> Guess you plan adding another patch later, right?

Probably... still some things can be improved.
Comment 8 Heiko Tietze 2023-12-11 07:55:05 UTC
The patch makes LibreOffice bypass the confirmation dialog and gives the relevant feedback via infobar/s (like when "Disable Macros" has been pressed). That works but is a bit odd since two if's are combined (macro, certificate) and behave differently.

Anyway, feel free to close the dialog as resolved.
Comment 9 Samuel Mehrbrodt (allotropia) 2023-12-11 07:58:19 UTC
(In reply to Heiko Tietze from comment #8)
> The patch makes LibreOffice bypass the confirmation dialog and gives the
> relevant feedback via infobar/s (like when "Disable Macros" has been
> pressed). That works but is a bit odd since two if's are combined (macro,
> certificate) and behave differently.

Which patch? My patch above does not do that.
Comment 10 Samuel Mehrbrodt (allotropia) 2023-12-11 07:59:06 UTC
One more step to further improve security:

When the certificate is not yet or no longer valid, issue a warning in the dialog

In this case, don't allow to enable macros (by adding the certificate to the list of when the cerfificate author is not in the list of trusted authors.
Comment 11 Samuel Mehrbrodt (allotropia) 2023-12-11 11:14:17 UTC
(In reply to Samuel Mehrbrodt (allotropia) from comment #4)
> (In reply to Heiko Tietze from comment #3)
> > (In reply to Samuel Mehrbrodt (allotropia) from comment #2)
> > > That wouldn't help with Macro security mode "High" where only macros from
> > > trusted sources are allowed to run.
> > 
> > High or Very High... you need to _always_ trust the certificate and my
> > proposal was to "trust" it only once.
> 
> Please read the description of the macro security level in the options
> dialog. What you describe only works in Medium level.

So after consideration we probably can do what you suggest. I was thinking that there would be no difference left to the Medium level if we have the "Allow macros once thing". But the difference is, that in High level, this would only work for signed macros.

So I will rework the dialog to include a possibility to trust the cert only once.
That will still adhere to the High level which says that only macros from trusted sources are allowed to run (the user has to decide adhoc if the source is trusted). 

This would not be possible if the list of trusted authors is read-only.
Comment 12 Heiko Tietze 2023-12-11 13:17:51 UTC
Forgot to reset the security level. In fact, the patch enables the button so a macro can be loaded/executed without trusting the certification.

And if users want to trust, they have to view the certificate first - until then the checkbox is disabled. Reasonable but maybe hard to follow since there is no connection between the checkbox enabled state and the "view certificate" button.

(In reply to Samuel Mehrbrodt (allotropia) from comment #11)
> So I will rework the dialog to include a possibility to trust the cert only
> once.
Perhaps note that in the infobar? Not reading it myself, though ;-)
Comment 13 Commit Notification 2023-12-14 07:10:34 UTC
Samuel Mehrbrodt committed a patch related to this issue.
It has been pushed to "master":

https://git.libreoffice.org/core/commit/69814fc26fe21a258cf32fca021945f5bc69f59c

tdf#158576 Show warning when cert is not yet/no longer valid

It will be available in 24.8.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 14 Commit Notification 2023-12-14 07:11:37 UTC
Samuel Mehrbrodt committed a patch related to this issue.
It has been pushed to "master":

https://git.libreoffice.org/core/commit/37b381e2877c60c84547b22117663a72ab02fadc

tdf#158576 Allow trusting certificates only once

It will be available in 24.8.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 15 Commit Notification 2023-12-14 07:19:40 UTC
Samuel Mehrbrodt committed a patch related to this issue.
It has been pushed to "master":

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

tdf#158576 This button shows a certificate when only one signature found

It will be available in 24.8.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 2023-12-14 11:15:22 UTC
Samuel Mehrbrodt committed a patch related to this issue.
It has been pushed to "master":

https://git.libreoffice.org/core/commit/2ee0500b6033e7dbfef221e28042f23da361869e

tdf#158576 Explain why OK btn is disabled

It will be available in 24.8.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 Samuel Mehrbrodt (allotropia) 2023-12-14 11:25:03 UTC
(In reply to Heiko Tietze from comment #12)
> Forgot to reset the security level. In fact, the patch enables the button so
> a macro can be loaded/executed without trusting the certification.
> 
> And if users want to trust, they have to view the certificate first - until
> then the checkbox is disabled. Reasonable but maybe hard to follow since
> there is no connection between the checkbox enabled state and the "view
> certificate" button.

I added an explanatory tooltip for this (see comment 16).

> (In reply to Samuel Mehrbrodt (allotropia) from comment #11)
> > So I will rework the dialog to include a possibility to trust the cert only
> > once.
> Perhaps note that in the infobar? Not reading it myself, though ;-)

Which infobar?

I've done a number of improvements now with above commits. Please check if the UX makes sense or if there are more improvements needed.
Comment 18 Commit Notification 2023-12-14 15:11:06 UTC
Samuel Mehrbrodt committed a patch related to this issue.
It has been pushed to "libreoffice-24-2":

https://git.libreoffice.org/core/commit/9b56334615d9cdb42542aba746caaa8985566001

tdf#158576 Show warning when cert is not yet/no longer valid

It will be available in 24.2.0.0.beta2.

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 Commit Notification 2023-12-14 15:12:09 UTC
Samuel Mehrbrodt committed a patch related to this issue.
It has been pushed to "libreoffice-24-2":

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

tdf#158576 Allow trusting certificates only once

It will be available in 24.2.0.0.beta2.

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 20 Commit Notification 2023-12-14 15:13:12 UTC
Samuel Mehrbrodt committed a patch related to this issue.
It has been pushed to "libreoffice-24-2":

https://git.libreoffice.org/core/commit/7bf02b035033c87ebd61742dbd3f71da0229114f

tdf#158576 This button shows a certificate when only one signature found

It will be available in 24.2.0.0.beta2.

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 2023-12-14 15:13:14 UTC
Samuel Mehrbrodt committed a patch related to this issue.
It has been pushed to "libreoffice-24-2":

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

tdf#158576 Explain why OK btn is disabled

It will be available in 24.2.0.0.beta2.

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.