Bug 161871 - Certificate Chooser -> regression 9f327102c: columns broken, users may explicitly want X.509 or GPG, usage needed
Summary: Certificate Chooser -> regression 9f327102c: columns broken, users may explic...
Status: RESOLVED FIXED
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: LibreOffice (show other bugs)
Version:
(earliest affected)
24.2.0.0 alpha1+
Hardware: All All
: medium normal
Assignee: Samuel Mehrbrodt (allotropia)
URL:
Whiteboard: target:25.2.0 target:24.2.6 target:24...
Keywords:
Depends on:
Blocks: OpenPGP
  Show dependency treegraph
 
Reported: 2024-07-02 14:17 UTC by Moritz Duge (allotropia) (a.k.a. kolAflash)
Modified: 2024-07-31 06:28 UTC (History)
6 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 Moritz Duge (allotropia) (a.k.a. kolAflash) 2024-07-02 14:17:07 UTC
Regression between 7.6 and libreoffice-24.2.0.0.alpha1 due to:
https://git.libreoffice.org/core/+/9f327102c435887bbae650b3a573f44500b6f534%5E%21/

Columns broken:
Since the commits the columns don't work at all. All but the first column miss actual content.

Discussion: Type actually useful!?
I think the Type column (X.509/GPG) is actually useful. When having X.509 and GPG set up, a user might want to explicitly use X.509 or GPG for certain documents. Usually because the document is for a person which only has X.509 or GPG.
So I would keep this column.

Discussion: Usage still needed
I agree, that the "Usage" (sign and/or encrypt) ideally should not be needed here. But currently the Certificate Chooser lets users choose encrypt-only keys for signing and the other way around. When selecting a key with a wrong "Usage" the cryptographic process fails. To make this less painful the "Usage" column should stay for the moment. But the perfect solution would be a proper preselection which only lists the keys suitable for the given use case (sign or encrypt a document).


So I'd vote to revert the commit.

And when a preselection for the "Usage" (sign/encrypt) has been implemented, the part dropping the "Usage" column can again be cherry-picked.

--

Bug moved out of this meta bug:

(OpenPGP) - [META] OpenPGP bugs and enhancements
Bug 158839 comment 1, section: CertificateChooser dialog
Comment 1 Moritz Duge (allotropia) (a.k.a. kolAflash) 2024-07-02 17:19:56 UTC
Reverting 9f327102c works well on the current master and solves the listed problems.

Tested master at:
https://git.libreoffice.org/core/+/b32aa67f17a645c1d6f2b544d315f12bad123547/
Comment 2 Heiko Tietze 2024-07-04 13:26:12 UTC
Wasn't X509 for PDF only? "All but the first column miss actual content." is hard to follow.
Comment 3 Moritz Duge (allotropia) (a.k.a. kolAflash) 2024-07-04 15:44:22 UTC
(In reply to Heiko Tietze from comment #2)
> Wasn't X509 for PDF only?

Beside PDF files you can also sign ODF files with X.509. ODF X.509 signing is just broken since LO 24.2, but it worked fine in 7.6. And I hope to have this fixed soon.

regression: ODF X.509 signing doesn't work since libxmlsec 1.2.37 -> 1.3.1
https://bugs.documentfoundation.org/show_bug.cgi?id=161872

When signing ODF files GPG and X.509 usage is possible. So the list shows X.509 and GPG keys in the same dialog.


> "All but the first column miss actual content." is hard to follow.

Only the first column "Issued to" has content.

The remaining columns "Issued by" and "Expiration date" are left empty since 9f327102c. There's simply no text in these columns.
Comment 4 Moritz Duge (allotropia) (a.k.a. kolAflash) 2024-07-04 19:58:22 UTC
(In reply to Moritz D. from comment #0)
> [...]
> And when a preselection for the "Usage" (sign/encrypt) has been implemented,
> the part dropping the "Usage" column can again be cherry-picked.

Just as a note for a follow up ticket:

I had another thought about this. And maybe we shouldn't even try to implement pre-filtering by the usage. It could irritate users when keys are not being shown. Instead there should be a proper error when the user selects a key which doesn't have the needed Usage attributes (instead of letting the cryptographic procedure crash as it currently does).
Comment 5 Heiko Tietze 2024-07-05 06:41:33 UTC
(In reply to Moritz Duge from comment #3)
> Only the first column "Issued to" has content.
> 
> The remaining columns "Issued by" and "Expiration date" are left empty since
> 9f327102c. There's simply no text in these columns.

That's weird. The mentioned patch just hides the two columns to improve usability. It was a conscious decision and just reverting sounds wrong to me. But I haven't checked the issue, so no confirmation, just ranting ;-).
Comment 6 TokieSan 2024-07-05 17:19:52 UTC
I don't agree with the prefiltering part due to the same reason you proposed which is conscious, the decision to remove the usage & type columns were mainly to appeal to the most % of the use cases leaving the less used details in the "view certificate" dialog and back then we had a couple of meetings to decide on what to keep/remove.

I think that for the usage part, usually in cases where two different keys used for encryption vs signing it gets added as a subkey (and thus in the UI both will have the same entry, but the subkey will be automatically used according to whether it's in encryption or signing) Also in cases for splitted certificates, a large margin of those usually include that usage part in the comment attribute of the certificate (since in other cert usage cases you are only seeing the comments) which is also viewed directly in the dialog

The question is what % would the case the two keys are separate of the total usage cases (and it'll be frustrating not to have the usage columns) and view certificate dialog won't be enough. I agree that the dialog in case a sign only cert was used for encryption should include more information anyways rather than just "OpenPGP key not trusted, damaged, or encryption failure".

These are my thoughts tho based on the earlier discussions we had and your comments, some details are maybe missing but maybe hossein can provide his input as well
Comment 7 Samuel Mehrbrodt (allotropia) 2024-07-15 05:41:39 UTC
(In reply to Heiko Tietze from comment #5)
> It was a conscious decision and just reverting sounds wrong to
> me. But I haven't checked the issue, so no confirmation, just ranting ;-).

Where was this decided? Can you point me to some minutes or bug # ?
Comment 8 Samuel Mehrbrodt (allotropia) 2024-07-15 07:19:05 UTC
Will revert that commit for now.

As Moritz pointed out the commit broke the columns in all backends except GTK3.

Needs more discussion, and if it's decided that Type and Usage columns are indeed to be removed, a proper fix.
Comment 9 Commit Notification 2024-07-15 11:00:59 UTC
Samuel Mehrbrodt committed a patch related to this issue.
It has been pushed to "master":

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

tdf#161871 Revert "Removed Type and Usage Columns from Certificate Chooser"

It will be available in 25.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 10 Commit Notification 2024-07-29 18:42: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/d9e065b8fada80b7c7c413ce27b744af19c407c6

tdf#161871 Revert "Removed Type and Usage Columns from Certificate Chooser"

It will be available in 24.2.6.

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 11 Commit Notification 2024-07-30 05:30:39 UTC
Samuel Mehrbrodt committed a patch related to this issue.
It has been pushed to "libreoffice-24-8":

https://git.libreoffice.org/core/commit/0b80db94d445434ab12002b9ba11c15f00accc19

tdf#161871 Revert "Removed Type and Usage Columns from Certificate Chooser"

It will be available in 24.8.1.

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 12 Commit Notification 2024-07-30 05:31:42 UTC
Samuel Mehrbrodt committed a patch related to this issue.
It has been pushed to "libreoffice-24-8-0":

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

tdf#161871 Revert "Removed Type and Usage Columns from Certificate Chooser"

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.