Bug 83939 - LibreOffice leaves invalid signature in output when creating digitally signed PDF if the signing fails
Summary: LibreOffice leaves invalid signature in output when creating digitally signed...
Status: RESOLVED FIXED
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: Printing and PDF export (show other bugs)
Version:
(earliest affected)
4.4.0.0.alpha0+ Master
Hardware: Other Linux (All)
: medium normal
Assignee: Not Assigned
QA Contact:
URL:
Whiteboard: target:4.5.0 target:4.4.0.0.beta2
Keywords:
Depends on:
Blocks: PDF-Signature
  Show dependency treegraph
 
Reported: 2014-09-16 17:02 UTC by Markus Wernig
Modified: 2014-12-05 12:19 UTC (History)
5 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 Markus Wernig 2014-09-16 17:02:15 UTC
This report is a follow up to bug 66701. See there for the steps that lead
here.

When creating digitally signed PDFs, LO sometimes creates PDF with invalid signatures. The conditions under which this happens are unclear.

This was originally reported in bug 66701. The PDF that was attached there (attachment 82188 [details]) shows that instead of the digital signature LO had embedded a long row (16394) of zeroes (line 198).

The same also happened in bug 83937, where LO was run unter valgrind when producing the signed PDF (as it would crash otherwise). The resulting PDF from that case is in attachment 106382 [details]), Same amount of zeroes, line 290.

It is unknown what caused the error in the first bug. But in the second one, the sequence that lead to the corrupt signature seems to have been the following (without having looked at the code)

...0) LO selects the key in the token from NSS list
1) LO asks for storage location
2) LO sends data to sign to the token via NSS function 
3) PKCS# library (libcvP11) gets called from NSS
4) PKCS# library starts external helper program to ask user for PIN
5) External helper program crashes (segfault)
6) Somehow that does not create an error condition that LO detects
7) Signature (all zeroes) is written into PDF

It may be worth noting that the ODF file itself can be successfully signed (see attachment 106381 [details]) with the very same key/certificate from the very same token (also in the same session) via File->Digital Signatures (if not run under valgrind)
Comment 1 Tor Lillqvist 2014-12-03 10:01:57 UTC
I wonder if the actual bug here is that if the signing fails, an error message should be presented to the user, the PDF creation should be aborted and the PDF file produced so far deleted. What now happens if signing fails inside NSS is that the PDF is left in an intermediate state with the space recerved for the signature still containing just the zeros that are written there as a placeholder first.
Comment 2 Tor Lillqvist 2014-12-03 10:06:27 UTC
So would it be a good enough fix for this bug to just make sure that if signing fails, then no PDF is produced, and an error message is displayed?
Comment 3 Markus Wernig 2014-12-03 10:41:45 UTC
The fact that the signature operation fails is a major part of the problem. Or even the very problem itself. It should not fail.

The same operation with the same certificate/token succeeds when signing the ODF. So it is highly unlikely to be a NSS problem, and the problem will not be solved by issuing an error message.

The fact that LO leaves the unfinished PDF and does not notify the user is a bug that should be fixed, though.
Comment 4 Tor Lillqvist 2014-12-03 11:23:19 UTC
I have a patch that should make it so that if the signing fails, the PDF is not generated (in an unfinished state). That was not hard. Will verify it a bit more and commit.

I could reproduce producing PDF with an unfinished all-zeroes signature only on Windows, though. On Linux, if I managed to make a certificate show up in the File:Export as PDF:Digital Signatures:Select dialog, then signing with it also worked.

(However, that signature was then not fully approved by Adobe Reader, but that is not what *this* bug is about.)
Comment 5 Tor Lillqvist 2014-12-03 11:28:09 UTC
The successful signing on Linux was when I used the only (non-hardware) not self-signed certificate I had easy access to, a code-signing certificate issued by Apple. Adobe Reader was not entirely satisfied with that, but at least it did confirm that the document has not been tampered with after signing, which I guess is the main thing.

Will next try with the hardware token. Hopefully such a signature will be good enough even for Adobe Reader.
Comment 6 Commit Notification 2014-12-03 12:45:12 UTC
Tor Lillqvist committed a patch related to this issue.
It has been pushed to "master":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=840f75065918c4584fa9159fdc90242b5374ab37

fdo#83939: Check return value from pPDFWriter->Emit()

It will be available in 4.5.0.

The patch should be included in the daily builds available at
http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
http://wiki.documentfoundation.org/Testing_Daily_Builds
Affected users are encouraged to test the fix and report feedback.
Comment 7 Commit Notification 2014-12-03 12:45:16 UTC
Tor Lillqvist committed a patch related to this issue.
It has been pushed to "master":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=1fe9ee73a758603ee0e1465931352c41ef8bd999

fdo#83939: Add new error code for failed PDF signing, and handle it

It will be available in 4.5.0.

The patch should be included in the daily builds available at
http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
http://wiki.documentfoundation.org/Testing_Daily_Builds
Affected users are encouraged to test the fix and report feedback.
Comment 8 Commit Notification 2014-12-03 12:45:19 UTC
Tor Lillqvist committed a patch related to this issue.
It has been pushed to "master":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=d8a8ff8177df628636d80c4dc1d8f597f6677dfc

fdo#83939: Set error code if signing failed

It will be available in 4.5.0.

The patch should be included in the daily builds available at
http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
http://wiki.documentfoundation.org/Testing_Daily_Builds
Affected users are encouraged to test the fix and report feedback.
Comment 9 Commit Notification 2014-12-03 13:53:05 UTC
Tor Lillqvist committed a patch related to this issue.
It has been pushed to "libreoffice-4-4":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=002cb3c4f7917ef52c1364743e52807ffbdc2fc0&h=libreoffice-4-4

fdo#83939: Check return value from pPDFWriter->Emit()

It will be available in 4.4.0.0.beta2.

The patch should be included in the daily builds available at
http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
http://wiki.documentfoundation.org/Testing_Daily_Builds
Affected users are encouraged to test the fix and report feedback.
Comment 10 Commit Notification 2014-12-03 13:53:09 UTC
Tor Lillqvist committed a patch related to this issue.
It has been pushed to "libreoffice-4-4":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=21f065e66743a0cbf0ac83cd368cd07636e638d4&h=libreoffice-4-4

fdo#83939: Add new error code for failed PDF signing, and handle it

It will be available in 4.4.0.0.beta2.

The patch should be included in the daily builds available at
http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
http://wiki.documentfoundation.org/Testing_Daily_Builds
Affected users are encouraged to test the fix and report feedback.
Comment 11 Commit Notification 2014-12-03 13:53:12 UTC
Tor Lillqvist committed a patch related to this issue.
It has been pushed to "libreoffice-4-4":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=24ffd0d942716b7652bb89d1a1a7d171b1bf7bfa&h=libreoffice-4-4

fdo#83939: Set error code if signing failed

It will be available in 4.4.0.0.beta2.

The patch should be included in the daily builds available at
http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
http://wiki.documentfoundation.org/Testing_Daily_Builds
Affected users are encouraged to test the fix and report feedback.
Comment 12 Tor Lillqvist 2014-12-04 09:35:15 UTC
The very specific issue this bug is about is now fixed, so marking this as resolved/fixed. Please don't re-open this bug for other signature-related issues, but file one bug for each specific issue.