[Poppler-bugs] [Bug 99416] Sign PDF with digital signature

[bugzilla-daemon at freedesktop.org]

https://bugs.freedesktop.org/show_bug.cgi?id=99416

--- Comment #41 from Hans-Ulrich Jüttner <huj at froreich-bioscientia.de> ---
(In reply to Albert Astals Cid from comment #40)
> (In reply to Adrian Johnson from comment #37)
> > (In reply to Albert Astals Cid from comment #33)
> > > (In reply to Adrian Johnson from comment #30)
> > > > Created attachment 134043 [details] [review] [review] [review] [review]
> > > > Return timezone in timeToDateString
> > > > 
> > > > Here's the fix to make timeToDateString include the time zone in a portable
> > > > way.
> > > 
> > > Adrian, does this mean you're relatively happy with the patch?
> > 
> > I'm not sure which patch you are referring to.
> 
> All of them :D
> 
> > Current patch status is:
> > 
> > 1) Added signing of PDF documents via Qt5 interface and with pdfsig 
> > 
> > I have a number of major issues with this patch. I have fixed these issues
> > in (3).
> 
> So you're "happy with conditions" :D
> 
> > 
> > 2) pdfsig: add -nssdi option v2
> > 
> > Updated documentation based on your feedback.
> > 
> > 3) write document then update byte offsets and sig on disk v2
> >  
> > This fixes all the issues I had with (1).
> > 
> > 4) Return timezone in timeToDateString
> > 
> > In comment 15 I requested that the patch use the timeToDateString(). I noted
> > that this function currently does not include the timezone. This patch adds
> > the time zone. Patch (3) has already changed the signing code to use
> > timeToDateString().
> 
> This looks good to me.  Hans-Ulrich do patches 2) 3) and 4) also look good
> for you?

Patch (2), attachment #134080, looks good to me except for the help message
for option -nssdir:
+  {"-nssdir", argGooString, &nssDir,     0,
+   "don't perform certificate validation"},
should be
+  {"-nssdir", argGooString, &nssDir,     0,
+   "database directory containing the certificate and key database files"},

Patch (4), attachment #134043, is fine.

I have a little conceptual problem with patch (3), attachment #134081.
Calling method sign() from qt5 interface now writes directly to disk with
the file name as new first parameter of that method. But this leaves the
document in memory with an invalid signature and invalid ByteRange parameters.
Poppler::PDFConverter::convert() called afterwards would write this invalid
document to disk and Poppler::FormFieldSignature::validate() called after
signing would tell us that the signature is invalid.

This behaviour can be argued as signing should always be the last thing to do
before writing the signed document to disk. But I think that should be clearly
documented in the header file qt5/src/poppler-form.h saying that the document
has to be reread from disk before doing anything with it after signing.

Moreover, the new parameter saveFilename of method sign() should be added to
the documentation of that method with an @param line just as it was done for
the other parameters.

-- 
You are receiving this mail because:
You are the assignee for the bug.
You are on the CC list for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/poppler-bugs/attachments/20170913/2fca57d9/attachment.html>