ubuntu pdf doc viewer will not let me sign a document
Affects | Status | Importance | Assigned to | Milestone | |
---|---|---|---|---|---|
Poppler |
Fix Released
|
Wishlist
|
|||
evince (Ubuntu) |
Triaged
|
Wishlist
|
Unassigned |
Bug Description
Just updated in last few weeks, i think ubuntu 12.4
To sign the document i have to send it to my neighbors windows computer, open it, sign it, then send it, then I get a note from echo sign that the document was sent with my signature.
ProblemType: Bug
DistroRelease: Ubuntu 11.10
Package: evince 3.2.1-0ubuntu2.3
ProcVersionSign
Uname: Linux 3.0.0-27-generic i686
ApportVersion: 1.23-0ubuntu4
Architecture: i386
Date: Fri Nov 30 18:13:25 2012
ExecutablePath: /usr/bin/evince
InstallationMedia: Ubuntu 11.04 "Natty Narwhal" - Release i386 (20110427.1)
ProcEnviron:
PATH=(custom, no user)
LANG=en_US.UTF-8
SHELL=/bin/bash
SourcePackage: evince
UpgradeStatus: Upgraded to oneiric on 2012-11-18 (12 days ago)
In freedesktop.org Bugzilla #16770, Carlos Garcia Campos (carlosgc) wrote : | #5 |
*** Bug 19120 has been marked as a duplicate of this bug. ***
In freedesktop.org Bugzilla #16770, Advax (advax) wrote : | #6 |
http://
I hacked Xpdf to tell me of the existence of SigFlags bits, but lack the skill to implement this properly in finite time
In freedesktop.org Bugzilla #16770, Markus Kilås (markuskilas) wrote : | #7 |
Created attachment 37425
Initial patch for parsing digitally signed PDFs
I have started to look at support for verifying signed PDF documents.
The attached patched gives very basic support by providing methods for getting the signature data (/Contents), the signature type (/SubFilter i.e. PKCS7) and the ByteRanges that the verifier needs to calculate the digest over. Then the actual signature and certificate chain verification is not specific to PDF and could be implemented by the applications using any crypto library.
// Markus Kilås
<email address hidden>
In freedesktop.org Bugzilla #16770, Markus Kilås (markuskilas) wrote : | #8 |
Sample signed document:
http://
In freedesktop.org Bugzilla #16770, Albert Astals Cid (aacid) wrote : | #9 |
Why force the applications to implement it? After all they are all going to do the same, so it makes sense to have it at the poppler level too.
Code related i don't see why you store contents as a GooString and the others as Objects, what's the reason?
Also before doing getArray() and getName() you need to check with isArray and isName, otherwise if the file is broken we will crash.
In freedesktop.org Bugzilla #16770, Brad Hards (bradh) wrote : | #10 |
I do kind-of agree with Markus that the verification operations can be done externally. There is an application level dependency in that the certificate store could depend on the desktop / user environment.
In freedesktop.org Bugzilla #16770, Brad Hards (bradh) wrote : | #11 |
It would be very useful to have example code that actually does the validation operations (e.g. in the glib or qt examples). Perhaps gnutls (LGPLv2+) may be suitable.
In freedesktop.org Bugzilla #16770, Markus Kilås (markuskilas) wrote : | #12 |
I can see you point that the verification should be included if all applications were to use it.
However, I was not just sure if it is good to add a dependency to a particular crypto library. There are Gnutls, openssl and NSS and possibly other? I have not used any of them for this purpose (I am mainly a Java developer now days and normally use the Bouncy Castle API). And as Brad mentions the trusted root certificates might be fetched from some keystore integrated with the desktop.
I think my initial idea was to have support in poppler to get only that is needed and then an application could implement the rest and later some of that could be refactored and moved back into poppler, but that's just and idea you know better how poppler works.
Regarding the code related question: I have not been using poppler before and I noticed while looking at this that there was at least two ways of doing it. I did not really understand the implications of choosing one over the other. What is recommended - storing the Object in the class or copy the string? I wasn't also sure I was freeing the memory correctly...
In freedesktop.org Bugzilla #16770, Albert Astals Cid (aacid) wrote : | #13 |
Brad, you are the "expert" here, you think it makes sense commiting the patch (maybe fixing the style?) ?
In freedesktop.org Bugzilla #16770, Albert Astals Cid (aacid) wrote : | #14 |
I've spoken with Brad off-line and i think i agree with him that we should have a working test/example showing how things work before commiting anything, otherwise it will just bitrot, no-one will be able to figure out how to use it, etc...
So if you could hack a quick test/example using the library of your choose it would be great
In freedesktop.org Bugzilla #16770, Cmorve69-w (cmorve69-w) wrote : | #15 |
I'm far from understanding all of this. But notice it seems NSS will be *the* Linux crypto library.
Fedora is pursuing it: http://
And it's in Linux Standard Base 4: http://
In freedesktop.org Bugzilla #16770, Jelle de Jong (jelledejong) wrote : | #16 |
Any progress or possible ETA for digital signature support for PDF documents?
In freedesktop.org Bugzilla #16770, iroli (roland-lezuo) wrote : | #17 |
bump
In freedesktop.org Bugzilla #16770, nodata (ubuntu-nodata) wrote : | #18 |
Since there hasn't been any progress for a couple of years, would it be possible to close this bug?
In freedesktop.org Bugzilla #16770, sjjh (simon-harhues) wrote : | #19 |
In favor of closing it, I would like to see some progress here. :)
I believe the importance for digital signing will increase in future. Maybe the poppler team could ask the developers/
In freedesktop.org Bugzilla #16770, André Guerreiro (aguerreiro) wrote : | #20 |
Created attachment 66786
PDF Signature verification support
Here's an initial attempt at solving this issue.
This patch adds signature verification support to poppler core.
It uses OpenSSL PKCS7 API for the crypto operations (signature and certificate Validations).
4 new functions were added at the glib wrapper level:
poppler_
poppler_
poppler_
poppler_
I've coordinated with Vasco Dias to expose this feature in Evince and his work is in the latest patches attached to this bug: https:/
As the additional dependency on OpenSSL couldn't possibly satisfy everyone I made it optional at build-time with --enable-openssl for Autotools and -DENABLE_OPENSSL=ON for cmake
Current limitations:
- Timestamps contained in the PKCS7 signature are not verified
- the new functionality is not yet exposed in the qt4 wrapper as I prioritized the glib wrapper to support Evince.
In freedesktop.org Bugzilla #16770, Albert Astals Cid (aacid) wrote : | #21 |
Haven't looked at the code yet, but thanks anyway for trying to solve this.
openssl is not compatible with GPLv2 code (or at least its compatibility is discussed by some) any change you guys can use GNU TLS or Mozilla NSS which seem to be more in the clear?
In freedesktop.org Bugzilla #16770, Josh Triplett (joshtriplett) wrote : | #22 |
Since the patch provided supports verifying signatures, any thoughts on adding support for creating signatures, ideally in a manner compatible with Adobe's implementation?
In freedesktop.org Bugzilla #16770, André Guerreiro (aguerreiro) wrote : | #23 |
@Albert
OK, I can see the problem for poppler in terms of licensing.
A quick evaluation of the alternatives:
gnutls seems to be unsuited for this because it doesn't have a decent PKCS7 API that would allow me to parse the signature and access each component.
I've only found this in the docs: http://
NSS seems to be more promising as I've found example code for PKCS#7 validation in its source tarball: mozilla/
The disadvantage I see with nss is that we won't be able to reuse the system certificate store usually in /etc/ssl/certs because it will need to use a particular Berkeley DB cert store as you can find in your Firefox/Thunderbird Profile. So we'd have an implicit dependency on .mozilla/... being present or worse we'll need to introduce our own cert store.
I have no experience with gnutls or nss so if anyone can correct me or add something, feel free.
In freedesktop.org Bugzilla #16770, André Guerreiro (aguerreiro) wrote : | #24 |
Also I can see merit in Fedora's effort of consolidation around NSS but I think it's a really herculean effort to port over so many packages.
In freedesktop.org Bugzilla #16770, Markus Kilås (markuskilas) wrote : | #25 |
(In reply to comment #19)
I know that LibreOffice uses NSS as when I look at digital signatures my certificates from Firefox is availble. However, I don't think the LibreOffice Ubuntu packages require the whole Firefox to be installed.
In freedesktop.org Bugzilla #16770, André Guerreiro (aguerreiro) wrote : | #26 |
(In reply to comment #21)
> (In reply to comment #19)
>
> I know that LibreOffice uses NSS as when I look at digital signatures my
> certificates from Firefox is availble. However, I don't think the LibreOffice
> Ubuntu packages require the whole Firefox to be installed.
Yes it doesn't require Firefox or Thunderbird but if you didn't have any of them you wouldn't have any CA certs in LO.
In freedesktop.org Bugzilla #16770, Albert Astals Cid (aacid) wrote : | #27 |
Andre: what's missing regarding PCKS#7 in http://
From what i see you can go gnutls_pkcs7_export + gnutls_
If that does not work for you, what big projects do with openssl (i.e. Qt) is dlopen it on runtime, that seems to avoid the gpl incompatibiltiy which if you ask me is a bit lame, but if it works for them i guess it should work for us.
In freedesktop.org Bugzilla #16770, Albert Astals Cid (aacid) wrote : | #28 |
What is missing in gnutls? I think that you have to use the pcks7 export and pass the data to the x509 import and then you have all those useful functions.
If none of the options is enough, we can do what big projects do (e.g. Qt) and dlopen openssl, if it works for them, it ought to work for us.
In freedesktop.org Bugzilla #16770, Albert Astals Cid (aacid) wrote : | #29 |
Doh, there was a glitch in bugzilla and it created both my yesterday comment that i got an error when pasting and today's one :-S
In freedesktop.org Bugzilla #16770, André Guerreiro (aguerreiro) wrote : | #30 |
What's missing in gnutls is a way to parse all the relevant components of the PKCS#7 object as present in a PDF signature.
It seems that in gnutls they assume those objects can only contain certificates and CRLs as you can confirm if you go through the functions that take gnutls_pkcs7_t as argument.
With openssl you can get the certificates, signature, and the digest of the signed content (these are the essential parts for detached signatures as used in PDF) as well as any optional timestamps or CRLs.
In freedesktop.org Bugzilla #16770, André Guerreiro (aguerreiro) wrote : | #31 |
Regarding the dlopen workaround I'll take a look at it this week.
I'll try to minimize the pitfall of possible missing/different symbols by targeting only the last major version of openssl (1.0).
In freedesktop.org Bugzilla #16770, Chpe (chpe) wrote : | #32 |
qt can link to openssl because its lgpl licence has an openssl exception, and not because dlopening would make it not 'link' to openssl. poppler has no such openssl exception; and even if it did, that wouldn't help the programmes using poppler which are gpl (e.g. evince) without such an exception.
In freedesktop.org Bugzilla #16770, André Guerreiro (aguerreiro) wrote : | #33 |
So if I understood correctly Qt is only using the dlopen approach to overcome restrictions to crypto exports but not (L)GPL incompatibilities, like stated here:
http://
Getting back to our point I'll need the definitive opinion from Poppler maintainers on using dlopen'ed openssl or replacing it with NSS.
Both options are extra work but I'm willing to do the extra mile to get this accepted.
In freedesktop.org Bugzilla #16770, Carlos Garcia Campos (carlosgc) wrote : | #34 |
(In reply to comment #26)
> What's missing in gnutls is a way to parse all the relevant components of the
> PKCS#7 object as present in a PDF signature.
>
> It seems that in gnutls they assume those objects can only contain certificates
> and CRLs as you can confirm if you go through the functions that take
> gnutls_pkcs7_t as argument.
>
> With openssl you can get the certificates, signature, and the digest of the
> signed content (these are the essential parts for detached signatures as used
> in PDF) as well as any optional timestamps or CRLs.
Would it be a lot of work to add support for that to gnutls?
In freedesktop.org Bugzilla #16770, Albert Astals Cid (aacid) wrote : | #35 |
Maintainer hat on: I think it'd be easier for all if you can use NSS (now that gnutls has been ruled out because of lack of functionality). https:/
What do you think?
In freedesktop.org Bugzilla #16770, Carlos Garcia Campos (carlosgc) wrote : | #36 |
(In reply to comment #30)
> (In reply to comment #26)
> > What's missing in gnutls is a way to parse all the relevant components of the
> > PKCS#7 object as present in a PDF signature.
> >
> > It seems that in gnutls they assume those objects can only contain certificates
> > and CRLs as you can confirm if you go through the functions that take
> > gnutls_pkcs7_t as argument.
> >
> > With openssl you can get the certificates, signature, and the digest of the
> > signed content (these are the essential parts for detached signatures as used
> > in PDF) as well as any optional timestamps or CRLs.
>
> Would it be a lot of work to add support for that to gnutls?
Replying to myself:
<KaL> I wonder if it could be useful for glib-networking to implement the missing things in gnutls, or if we don't need that at all
<danw> reading...
<danw> chpe, KaL_out: both gnutls and glib-networking intentionally only do TLS, not crypto in general, so I don't think it makes sense to add the extra PKCS#7 functionality to either of them
<danw> NSS would be better than OpenSSL, and once all the p11-kit / NSS-shared-DB stuff gets figured out fully, then NSS-based apps will be able to access your gnome-keyring certificates via PKCS#11
In freedesktop.org Bugzilla #16770, André Guerreiro (aguerreiro) wrote : | #37 |
OK, NSS with shared DB is what I'll pursue from now on.
Thanks everyone for the input.
In freedesktop.org Bugzilla #16770, Weibo3721 (weibo3721) wrote : | #38 |
Comment on attachment 37425
Initial patch for parsing digitally signed PDFs
>diff --git a/poppler/Form.cc b/poppler/Form.cc
>index 4df8a7d..1da9776 100644
>--- a/poppler/Form.cc
>+++ b/poppler/Form.cc
>@@ -1107,11 +1107,28 @@ void FormFieldChoice
> FormFieldSignat
> : FormField(xrefA, dict, ref, formSignature)
> {
>+ Object v, contents;
>+
>+ if (dict->
>+ if (v.dictLookup(
>+ this->contents = contents.
>+ v.dictLookup(
>+ v.dictLookup(
>+ v.dictLookup(
>+ }
>+ contents.free();
>+ }
>+ v.free();
> }
>
> FormFieldSignat
> {
>-
>+ if(contents) {
>+ delete contents;
>+}
>+ byteRange.free();
>+ filter.free();
>+ subFilter.free();
> }
>
> //-----
>diff --git a/poppler/Form.h b/poppler/Form.h
>index 35d66af..e5d1a1f 100644
>--- a/poppler/Form.h
>+++ b/poppler/Form.h
>@@ -418,6 +418,18 @@ public:
> FormFieldSignat
>
> virtual ~FormFieldSigna
>+
>+ Array* GetByteRange() { return byteRange.
>+ GooString* GetContents() const { return contents; }
>+ char* GetFilter() { return filter.getName(); }
>+ char* GetSubFilter() { return subFilter.
>+
>+protected:
>+ GooString* contents;
>+ Object byteRange;
>+ Object filter;
>+ Object subFilter;
>+
> };
>
> //-----
>diff --git a/poppler/PDFDoc.cc b/poppler/PDFDoc.cc
>index ade5fe4..80b5f6c 100644
>--- a/poppler/PDFDoc.cc
>+++ b/poppler/PDFDoc.cc
>@@ -358,6 +358,25 @@ GBool PDFDoc:
> return ret;
> }
>
>+#define SIGFLAGS_
>+#define SIGFLAGS_
>+
>+GBool PDFDoc::isSigned() {
>+ GBool ret;
>+ Object sigFlags;
>+
>+ getCatalog(
>+
>+ if (sigFlags.isInt()) {
>+ ret = sigFlags.getInt() & SIGFLAGS_
>+ } else {
>+ ret = gFalse;
>+ }
>+ sigFlags.free();
>+
>+ return ret;
>+}
>+
> void PDFDoc:
> double hDPI, double vDPI, int rotate,
> GBool useMediaBox, GBool crop, GBool printing,
>diff --git a/poppler/PDFDoc.h b/poppler/PDFDoc.h
>index 6d7dea2..22fdb40 100644
>--- a/poppler/PDFDoc.h
>+++ b/poppler/PDFDoc.h
>@@ -176,6 +176,9 @@ public:
> // Is the file encrypted?
> GBool isEncrypted() { return xref->isEncrypt
>
>+ // Is the file signed?
>+ GBool isSigned();
>+
> // Check various permissions.
> GBool okToPrint(GBool ignoreOwnerPW = gFalse)
> { return xref->okToPrint
>diff --git a/utils/pdfinfo.cc b/utils/pdfinfo.cc
>index 2abe8b4..b4b2233 100644
>--- a/utils/pdfinfo.cc
>+++ b/utils/pdfinfo.cc
>@@ -49,6 +49,7 @@
> #include "PDFDocEncoding...
In freedesktop.org Bugzilla #16770, Albert Astals Cid (aacid) wrote : | #39 |
weibo?
In freedesktop.org Bugzilla #16770, José Aliste (jose-aliste) wrote : | #40 |
(In reply to comment #33)
> OK, NSS with shared DB is what I'll pursue from now on.
> Thanks everyone for the input.
Andre, any update on the status of this? Thanks
38 comments hidden Loading more comments | view all 130 comments |
leonard janetzke (y2skiii) wrote : | #1 |
- Dependencies.txt Edit (3.6 KiB, text/plain; charset="utf-8")
- KernLog.txt Edit (5.0 KiB, text/plain; charset="utf-8")
- ProcMaps.txt Edit (30.9 KiB, text/plain; charset="utf-8")
- ProcStatus.txt Edit (771 bytes, text/plain; charset="utf-8")
- RelatedPackageVersions.txt Edit (233 bytes, text/plain; charset="utf-8")
- XsessionErrors.txt Edit (17.8 KiB, text/plain; charset="utf-8")
Sebastien Bacher (seb128) wrote : | #2 |
Thank you for your bug report, that's a known issue upstream in poppler and evince and work is ongoing to add signature support:
https:/
https:/
Changed in evince (Ubuntu): | |
importance: | Undecided → Wishlist |
status: | New → Triaged |
Sebastien Bacher (seb128) wrote : | #3 |
see also bug #740506 about being able to check a signature
Changed in poppler: | |
importance: | Unknown → Wishlist |
status: | Unknown → Confirmed |
tags: | removed: apparmor |
87 comments hidden Loading more comments | view all 130 comments |
In freedesktop.org Bugzilla #16770, Markus Kilås (markuskilas) wrote : | #91 |
I'm trying to build the branch (on Fedora 21) but obviously I am missing some dependency or not having the right versions etc. Anyone having an idea?
Build output:
---
[user@dev-21 poppler]$ sudo yum install nspr-devel
Loaded plugins: langpacks, post-transactio
Package nspr-devel-
Nothing to do
[user@dev-21 poppler]$ make
make all-recursive
make[1]: Entering directory '/home/
Making all in goo
make[2]: Entering directory '/home/
make[2]: Nothing to be done for 'all'.
make[2]: Leaving directory '/home/
Making all in fofi
make[2]: Entering directory '/home/
make[2]: Nothing to be done for 'all'.
make[2]: Leaving directory '/home/
Making all in splash
make[2]: Entering directory '/home/
make[2]: Nothing to be done for 'all'.
make[2]: Leaving directory '/home/
Making all in poppler
make[2]: Entering directory '/home/
make all-am
make[3]: Entering directory '/home/
CXX libpoppler_
In file included from SignatureHandle
SignatureHandle
#include <nspr/prprf.h>
compilation terminated.
Makefile:1084: recipe for target 'libpoppler_
make[3]: *** [libpoppler_
make[3]: Leaving directory '/home/
Makefile:840: recipe for target 'all' failed
make[2]: *** [all] Error 2
make[2]: Leaving directory '/home/
Makefile:642: recipe for target 'all-recursive' failed
make[1]: *** [all-recursive] Error 1
make[1]: Leaving directory '/home/
Makefile:524: recipe for target 'all' failed
make: *** [all] Error 2
---
In freedesktop.org Bugzilla #16770, Markus Kilås (markuskilas) wrote : | #92 |
(In reply to Markus Kilås from comment #87)
> I'm trying to build the branch (on Fedora 21) but obviously I am missing
> some dependency or not having the right versions etc. Anyone having an idea?
>
> Build output:
> ---
> [user@dev-21 poppler]$ sudo yum install nspr-devel
> Loaded plugins: langpacks, post-transactio
> Package nspr-devel-
> Nothing to do
> [user@dev-21 poppler]$ make
> make all-recursive
> make[1]: Entering directory '/home/
> Making all in goo
> make[2]: Entering directory '/home/
> make[2]: Nothing to be done for 'all'.
> make[2]: Leaving directory '/home/
> Making all in fofi
> make[2]: Entering directory '/home/
> make[2]: Nothing to be done for 'all'.
> make[2]: Leaving directory '/home/
> Making all in splash
> make[2]: Entering directory '/home/
> make[2]: Nothing to be done for 'all'.
> make[2]: Leaving directory '/home/
> Making all in poppler
> make[2]: Entering directory '/home/
> make all-am
> make[3]: Entering directory '/home/
> CXX libpoppler_
> In file included from SignatureHandle
> SignatureHandle
> directory
> #include <nspr/prprf.h>
> ^
> compilation terminated.
> Makefile:1084: recipe for target 'libpoppler_
> make[3]: *** [libpoppler_
> make[3]: Leaving directory '/home/
> Makefile:840: recipe for target 'all' failed
> make[2]: *** [all] Error 2
> make[2]: Leaving directory '/home/
> Makefile:642: recipe for target 'all-recursive' failed
> make[1]: *** [all-recursive] Error 1
> make[1]: Leaving directory '/home/
> Makefile:524: recipe for target 'all' failed
> make: *** [all] Error 2
> ---
I found a workaround for my build issue by creating symlinks:
/usr/include/nspr -> /usr/include/nspr4
/usr/inclyde/nss -> /usr/include/nss3
In freedesktop.org Bugzilla #16770, Adrian Johnson (ajohnson-redneon) wrote : | #93 |
(In reply to Markus Kilås from comment #88)
> I found a workaround for my build issue by creating symlinks:
> /usr/include/nspr -> /usr/include/nspr4
> /usr/inclyde/nss -> /usr/include/nss3
Are you using configure or cmake? The nss pkgconfig file should contain the correct include path.
In freedesktop.org Bugzilla #16770, Markus Kilås (markuskilas) wrote : | #94 |
(In reply to Adrian Johnson from comment #89)
> (In reply to Markus Kilås from comment #88)
> > I found a workaround for my build issue by creating symlinks:
> > /usr/include/nspr -> /usr/include/nspr4
> > /usr/inclyde/nss -> /usr/include/nss3
>
> Are you using configure or cmake? The nss pkgconfig file should contain the
> correct include path.
After checking out the branch I read INSTALL but did not find any ./configure so I run ./autogen.sh. It first failed for many different reasons which eventually got resolved by installing missing dependencies. After that I run the now generated ./configure and then make.
In freedesktop.org Bugzilla #16770, Adrian Johnson (ajohnson-redneon) wrote : | #95 |
What is the output of "pkg-config --cflags nss"?
In freedesktop.org Bugzilla #16770, Markus Kilås (markuskilas) wrote : | #96 |
(In reply to Adrian Johnson from comment #91)
> What is the output of "pkg-config --cflags nss"?
[user@dev-21 poppler]$ pkg-config --cflags nss
-I/usr/include/nss3 -I/usr/
In freedesktop.org Bugzilla #16770, Markus Kilås (markuskilas) wrote : | #97 |
Created attachment 119174
Handle SEC_ERROR_
When verifying a PDF signed by a certificate issued by a CA not in the trust store I would expect to get an error "Certificate isn't Trusted" however currently the error message actually is the more generic "Unknown issue with Certificate or corrupted data".
I tested with http://
[user@dev-21 utils]$ ./pdfsigverify ~/Downloads/
Digital Signature Info of: /home/user/
Signature #1:
- Signer Certificate Common Name: Signer 2
- Signing Time: Nov 23 2011 17:09:42
- Signature Validation: Signature is Valid.
- Certificate Validation: Unknown issue with Certificate or corrupted data.
Then after adding http://
[user@dev-21 utils]$ ./pdfsigverify ~/Downloads/
Digital Signature Info of: /home/user/
Signature #1:
- Signer Certificate Common Name: Signer 2
- Signing Time: Nov 23 2011 17:09:42
- Signature Validation: Signature is Valid.
- Certificate Validation: Certificate is Trusted.
The attached patch adds the NSS error code for untrusted issuer so the error will be "Certificate isn't trusted".
In freedesktop.org Bugzilla #16770, Adrian Johnson (ajohnson-redneon) wrote : | #98 |
I suggest we change the pdfsig "-c" switch to "-nocert". We can easily add a single character option if we find it is needed. But we can't get rid of it if we later regret it.
I'm still waiting for an answer to comment 79.
In freedesktop.org Bugzilla #16770, Albert Astals Cid (aacid) wrote : | #99 |
(In reply to Markus Kilås from comment #93)
> Created attachment 119174 [details]
> Handle SEC_ERROR_
I pushed this and also some code to differentiate between an unknown and an untrusted issuer (no idea what's the difference but if nss has this difference i feel we also have to)
In freedesktop.org Bugzilla #16770, Albert Astals Cid (aacid) wrote : | #100 |
(In reply to Adrian Johnson from comment #94)
> I suggest we change the pdfsig "-c" switch to "-nocert". We can easily add a
> single character option if we find it is needed. But we can't get rid of it
> if we later regret it.
>
> I'm still waiting for an answer to comment 79.
As said please let's not spend too much time on arguing over switch names, if you really really want -nocert, just push a patch to the signatureHandling branch that switches to it.
In freedesktop.org Bugzilla #16770, André Guerreiro (aguerreiro) wrote : | #101 |
(In reply to Adrian Johnson from comment #79)
> + r_values[0] = r2.isInt64() ? r2.getInt64() : r2.getInt();
> + r_values[1] = r3.isInt64() ? r3.getInt64() : r3.getInt();
> + r_values[2] = r4.isInt64() ? r4.getInt64() : r4.getInt();
>
> According the PDF Reference, the ByteRange array contains pairs of
> (offset,length).
>
> Why do we ignore the first offset and later assume it is 0? Why do we assume
> there are exactly two pairs.
>
> I only skimmed over the digital signatures section so maybe I missed
> something.
Actually the PDF spec allows for more than 2 pairs of values in /ByteRange but it would mean that there is more than one gap in the signed data apart from the signature itself. Quoting from ISO 32000-1 section 12.8.1:
"This range should be the entire file, including the signature dictionary but excluding the signature value itself (the Contents entry). Other ranges may be used but since they do not check for all changes to the document, their use is not recommended."
Obviously in a file with multiple signatures each signature should cover the latest revision present in the file when the signature was appended.
In freedesktop.org Bugzilla #16770, Adrian Johnson (ajohnson-redneon) wrote : | #102 |
(In reply to Albert Astals Cid from comment #96)
> As said please let's not spend too much time on arguing over switch names,
> if you really really want -nocert, just push a patch to the
> signatureHandling branch that switches to it.
Done. I've also added a man page. If any further information that should be in the man page can be posted here I can format it into the man page format and push it out.
In freedesktop.org Bugzilla #16770, Adrian Johnson (ajohnson-redneon) wrote : | #103 |
(In reply to Andre Guerreiro from comment #97)
> Quoting from ISO 32000-1 section 12.8.1:
> "This range should be the entire file, including the signature dictionary
> but excluding the signature value itself (the Contents entry). Other ranges
> may be used but since they do not check for all changes to the document,
> their use is not recommended."
This advice is for PDF producers. As a PDF consumer we should accept PDFs that don't follow this advice.
In freedesktop.org Bugzilla #16770, André Guerreiro (aguerreiro) wrote : | #104 |
Created attachment 119283
Manpage improvement
Here's an improvement to the manpage.
Corrected a typo and added some missing context
In freedesktop.org Bugzilla #16770, Adrian Johnson (ajohnson-redneon) wrote : | #105 |
(In reply to Andre Guerreiro from comment #100)
> Created attachment 119283 [details] [review]
> Manpage improvement
Pushed
In freedesktop.org Bugzilla #16770, Albert Astals Cid (aacid) wrote : | #106 |
So we're stuck on "need to use the offset" part, right?
Could someone try to do make the code use it even if we don't have any pdf that needs it?
In freedesktop.org Bugzilla #16770, Markus Kilås (markuskilas) wrote : | #107 |
(In reply to Albert Astals Cid from comment #102)
> So we're stuck on "need to use the offset" part, right?
>
> Could someone try to do make the code use it even if we don't have any pdf
> that needs it?
I am not sure if it is good to apply the robustness principle on security functions. In those cases it might be better to be defensive and reject signatures not following the recommendation.
In this case if the ByteRange does not cover the whole document there could be parts of the document that can be modified without invalidating the signature. Would it then be good to tell the user that the signature has been validated and the document is not modified even though in fact there are parts of the document for which we don't know?
In freedesktop.org Bugzilla #16770, xormar (public-wernig) wrote : | #108 |
I think the correct statement to issue in this case would be that a part ("revision") of the PDF has been signed and to show the validation result for that revision, ideally giving the user a chance to view only the signed part.
This might even be indispensable when there are multiple signatures applied to the document.
In freedesktop.org Bugzilla #16770, André Guerreiro (aguerreiro) wrote : | #109 |
Created attachment 120434
Improve robustness of SignatureHandle
This patch adds additional NULL-checking in SignatureHandle
It also removes a useless branch in validateCertifi
In freedesktop.org Bugzilla #16770, Markus Kilås (markuskilas) wrote : | #110 |
Created attachment 120758
Fix printf for unsigned int
In pdfsig.cc the NetBeans IDE gave warning:
Mismatching the argument type "unsigned int" and conversion specifier "d"
The attached patch changes from %d to %u in the printf.
In freedesktop.org Bugzilla #16770, Markus Kilås (markuskilas) wrote : | #111 |
(In reply to Markus Wernig from comment #104)
> I think the correct statement to issue in this case would be that a part
> ("revision") of the PDF has been signed and to show the validation result
> for that revision, ideally giving the user a chance to view only the signed
> part.
>
> This might even be indispensable when there are multiple signatures applied
> to the document.
So one idea could be to also use the first offset when checking the signature and in case it is not zero instead of return the status as SIGNATURE_VALID say something like "SIGNATURE_
The pdfsig tool already implements support for multiple signatures/
In freedesktop.org Bugzilla #16770, Markus Kilås (markuskilas) wrote : | #112 |
Created attachment 120760
Considering offset 0 and signature only covering part of PDF
Note that this patch has not been tested with a non-zero first offset document.
In freedesktop.org Bugzilla #16770, Albert Astals Cid (aacid) wrote : | #113 |
(In reply to Markus Kilås from comment #106)
> Created attachment 120758 [details] [review]
> Fix printf for unsigned int
>
> In pdfsig.cc the NetBeans IDE gave warning:
> Mismatching the argument type "unsigned int" and conversion specifier "d"
>
> The attached patch changes from %d to %u in the printf.
Pushed
In freedesktop.org Bugzilla #16770, Albert Astals Cid (aacid) wrote : | #114 |
(In reply to Markus Kilås from comment #108)
> Created attachment 120760 [details] [review]
> Considering offset 0 and signature only covering part of PDF
>
> Note that this patch has not been tested with a non-zero first offset
> document.
Adrian what do you think of this patch?
In freedesktop.org Bugzilla #16770, Adrian Johnson (ajohnson-redneon) wrote : | #115 |
I don't think we need to check if the byte range covers the entire document. Our job, when verifying the signature, is to use the byte ranges provided in the signature dictionary. It is up to the pdf producer to ensure the byte range covers the entire document (excluding the signature value).
All we need to do is ensure we check all bytes ranges in the ByteRange array. We should also check that each byte range is within the file. eg check that each offset is >= 0 and offset + length <= file size.
While it would be nice to check if the byte range covers the entire document, poppler does not provide any easy way to determine the file offsets of a dictionary value. This makes it difficult to check if the excluded range only covers the signature value.
In freedesktop.org Bugzilla #16770, Adrian Johnson (ajohnson-redneon) wrote : | #116 |
Created attachment 120889
Check in ranges in ByteRange array
This is an alternative to the patch in comment 108. It checks all ranges in the ByteRange array.
In freedesktop.org Bugzilla #16770, Adrian Johnson (ajohnson-redneon) wrote : | #117 |
(In reply to Andre Guerreiro from comment #105)
> Created attachment 120434 [details] [review]
> Improve robustness of SignatureHandle
>
> This patch adds additional NULL-checking in
> SignatureHandle
> signatures like the one contained here:
> http://
>
> It also removes a useless branch in validateCertifi
Pushed
In freedesktop.org Bugzilla #16770, Albert Astals Cid (aacid) wrote : | #118 |
(In reply to Adrian Johnson from comment #112)
> Created attachment 120889 [details] [review]
> Check in ranges in ByteRange array
>
> This is an alternative to the patch in comment 108. It checks all ranges in
> the ByteRange array.
I'm not very convinced by the naming of
GBool isInteger() { return type == objInt || type == objInt64; }
what do you think of
isIntOrInt64() ?
In freedesktop.org Bugzilla #16770, Adrian Johnson (ajohnson-redneon) wrote : | #119 |
(In reply to Albert Astals Cid from comment #114)
> I'm not very convinced by the naming of
> GBool isInteger() { return type == objInt || type == objInt64; }
> what do you think of
> isIntOrInt64() ?
I prefer a single word instead of isXXXorYYY() but I can appreciate that isInteger() could easily be confused with isInt(). I'll change it to isIntOrInt64().
I'll also add a corresponding getIntOrInt64() and replace the
"xxx.isInt64() ? xxx.getInt64() : xxx.getInt()" expressions with a single getter.
In freedesktop.org Bugzilla #16770, Adrian Johnson (ajohnson-redneon) wrote : | #120 |
Created attachment 120924
Check in ranges in ByteRange array v2
- renamed isInteger() to isIntOrInt64()
- added and use getIntOrInt64()
In freedesktop.org Bugzilla #16770, André Guerreiro (aguerreiro) wrote : | #121 |
Created attachment 120992
Load NSS root certs module
This change is needed to actually do certificate validation, because as it is NSS is trying to load the module which contains all the builtin root certs from the Firefox profile directory where it is usually missing. This way it will load the module from a system library directory.
In freedesktop.org Bugzilla #16770, Albert Astals Cid (aacid) wrote : | #122 |
Andre and Andre what do you think of Adrian's patch?
In freedesktop.org Bugzilla #16770, André Guerreiro (aguerreiro) wrote : | #123 |
I'm in favour of Adrian's patch. It's an improvement with additional sanity checks on the ByteRange values.
Indeed I tried to see if you could check if a given ByteRange covers the whole document and also found no easy way to do it with existing poppler functions/APIs.
In freedesktop.org Bugzilla #16770, Albert Astals Cid (aacid) wrote : | #124 |
Ok, good, so Adrian can you commit your patch?
After that we have attachment 120992 "Load NSS root certs module" that honestly I don't understand at all but some googleing seems to confirm it's needed.
And that would be it to try to merge it into master?
In freedesktop.org Bugzilla #16770, Adrian Johnson (ajohnson-redneon) wrote : | #125 |
(In reply to Albert Astals Cid from comment #120)
> Ok, good, so Adrian can you commit your patch?
Pushed
In freedesktop.org Bugzilla #16770, Albert Astals Cid (aacid) wrote : | #126 |
Anyone against merging attachment 120992 and then merging the branch to master?
I'll do it next week if noone complains.
In freedesktop.org Bugzilla #16770, Albert Astals Cid (aacid) wrote : | #127 |
Pushed \o/
Now, this is not "all the work".
We still need to do the glib/qt/cpp frontend work, and that will probably mean some changes to the new code, but getting this merged is an important step.
Wonder how to proceed, this bug has 122 comments.
Should we close this one and open different ones for the frontends api?
In freedesktop.org Bugzilla #16770, André Guerreiro (aguerreiro) wrote : | #128 |
Thanks Albert for merging it.
Yes it's not finished and I'm intending to pick up last summer's work on the glib frontend part.
I agree that we should close this bug and open specific ones to track the frontend development or any other issue we find with the core code.
In freedesktop.org Bugzilla #16770, Carlos Garcia Campos (carlosgc) wrote : | #129 |
\o/ Great job guys!
In freedesktop.org Bugzilla #16770, Albert Astals Cid (aacid) wrote : | #130 |
I've opened these bugs
https:/
https:/
https:/
Closing this one
Changed in poppler: | |
status: | Confirmed → Fix Released |
Since poppler is the basis for most pdf-processing software on Linux it would be great if it provided some functionality to access digital signatures embedded in PDF documents, so that the applications can display details of signing certificate and verify the validity of signature.
An example of such signatures can be seen on www.aloaha. com/cache/ multiplesignatu res.pdf
http://
Look at the objects along the right border of the page. On Windows the signatures can be checked using Adobe Acrobat 8.x