After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 614929 - Verifying digital signatures
Verifying digital signatures
Status: RESOLVED OBSOLETE
Product: evince
Classification: Core
Component: PDF
2.28.x
Other Linux
: Normal enhancement
: ---
Assigned To: Evince Maintainers
Evince Maintainers
Depends on:
Blocks:
 
 
Reported: 2010-04-06 08:04 UTC by Guido Günther
Modified: 2018-05-22 13:50 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Create new interface in libdocument (16.61 KB, patch)
2012-09-07 11:40 UTC, Vasco Dias
reviewed Details | Review
New job for getting signatures info (5.94 KB, patch)
2012-09-07 11:42 UTC, Vasco Dias
reviewed Details | Review
Create new sidebar to contain the info (21.64 KB, patch)
2012-09-07 11:42 UTC, Vasco Dias
reviewed Details | Review
Changes to the window to include the sidebar and new message at the top (7.56 KB, patch)
2012-09-07 11:43 UTC, Vasco Dias
reviewed Details | Review
Implement the new interface in the pdf backend (6.38 KB, patch)
2012-09-07 11:44 UTC, Vasco Dias
reviewed Details | Review
Little change to evince-document.h (753 bytes, patch)
2012-09-07 11:45 UTC, Vasco Dias
reviewed Details | Review
Create new interface in libdocument v2 (17.90 KB, patch)
2012-09-10 14:47 UTC, Vasco Dias
none Details | Review
New job for getting signatures info v2 (6.12 KB, patch)
2012-09-10 14:47 UTC, Vasco Dias
none Details | Review
Create new sidebar to contain the info v2 (20.68 KB, patch)
2012-09-10 14:48 UTC, Vasco Dias
none Details | Review
Changes to the window to include the sidebar and new message at the top v2 (7.57 KB, patch)
2012-09-10 14:49 UTC, Vasco Dias
none Details | Review
Implement the new interface in the pdf backend v2 (6.33 KB, patch)
2012-09-10 14:49 UTC, Vasco Dias
none Details | Review
Changes to the window to include the sidebar and new message at the top v2.1 (7.78 KB, patch)
2012-09-10 15:04 UTC, Vasco Dias
none Details | Review
Create new interface in libdocument v2.1 (18.10 KB, patch)
2012-09-11 13:03 UTC, Vasco Dias
none Details | Review
Implement the new interface in the pdf backend v2.1 (18.10 KB, patch)
2012-09-11 13:05 UTC, Vasco Dias
none Details | Review
Create new sidebar to contain the info v2.1 (21.95 KB, patch)
2012-09-11 13:07 UTC, Vasco Dias
none Details | Review
Implement the new interface in the pdf backend v2.1 (6.61 KB, patch)
2012-09-11 13:18 UTC, Vasco Dias
none Details | Review

Description Guido Günther 2010-04-06 08:04:46 UTC
PDF documents can be digitally signed, see:

http://www.adobe.com/devnet/acrobat/pdfs/PDF32000_2008.pdf

Section 12.8. Having Verification of these signatures supported in evince would be great since many bills are digitally signed this way nowadays.
Comment 1 Carlos Garcia Campos 2010-04-06 08:27:08 UTC
Yes, the main problem is that it's not supported by poppler yet:

https://bugs.freedesktop.org/show_bug.cgi?id=16770
Comment 2 Jelle de Jong 2011-04-15 08:16:57 UTC
Just a bump to indicate there are users waiting for this feature.
Comment 3 Josh Triplett 2012-04-09 21:46:23 UTC
Does this bug also cover adding signatures to documents, or should I file a separate bug for that?
Comment 4 lsof 2012-04-09 21:48:31 UTC
Best to ask under: https://bugs.freedesktop.org/show_bug.cgi?id=16770
Comment 5 Vasco Dias 2012-09-07 11:40:48 UTC
Created attachment 223749 [details] [review]
Create new interface in libdocument
Comment 6 Vasco Dias 2012-09-07 11:42:09 UTC
Created attachment 223751 [details] [review]
New job for getting signatures info
Comment 7 Vasco Dias 2012-09-07 11:42:40 UTC
Created attachment 223752 [details] [review]
Create new sidebar to contain the info
Comment 8 Vasco Dias 2012-09-07 11:43:45 UTC
Created attachment 223753 [details] [review]
Changes to the window to include the sidebar and new message at the top
Comment 9 Vasco Dias 2012-09-07 11:44:13 UTC
Created attachment 223754 [details] [review]
Implement the new interface in the pdf backend
Comment 10 Vasco Dias 2012-09-07 11:45:03 UTC
Created attachment 223755 [details] [review]
Little change to evince-document.h
Comment 11 Vasco Dias 2012-09-07 11:55:55 UTC
Here they are, 6 patches that add support for digital signatures in pdf files.

In backend/pdf/ev-poppler.cc there are some functions called not yet in upstream poppler, but they will land soon.

That's the reason I didn't delete the test function there. This way you can quickly review the changes to evince not caring about poppler changes.

Things to look for... Probably objects references and stuff like that I didn't really got my head around.
Comment 12 Christian Persch 2012-09-07 12:15:19 UTC
Review of attachment 223749 [details] [review]:

New public API needs to have gtk-doc comments (including introspection annotations where appropriate), and added to libevdocument-sections.txt. And new classes need gtk-doc SECTION:… etc, and be added to libevdocument-docs.xml (in help/reference/libdocument/).

::: libdocument/ev-document-signatures.c
@@ +18,3 @@
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
+ */
+

Missing include for config.h

@@ +27,3 @@
+{
+}
+

Missing gtk-doc comment.

@@ +35,3 @@
+  return iface->has_signatures (document_signatures);
+}
+

Missing gtk-doc comment (and introspection annotation about the returned list type).

::: libdocument/ev-document-signatures.h
@@ +2,3 @@
+ *  this file is part of evince, a gnome document viewer
+ *
+ * Copyright (C) 2012 Vasco Dias  <contact@vascodias.me>

Make that © (also in all the other files).

::: libdocument/ev-signature.c
@@ +19,3 @@
+ */
+
+#include <config.h>

"config.h" not <config.h>

@@ +50,3 @@
+  EvSignature *signature = EV_SIGNATURE (object);
+
+  // clean all the private fields

No C++ comments; use /* … */

@@ +52,3 @@
+  // clean all the private fields
+  if (signature->priv->signer_name) {
+    g_free (signature->priv->signer_name);

g_free is NULL safe, so no need to do if (foo) { g_free (foo); }

@@ +152,3 @@
+  g_object_class_install_property (g_object_class,
+                                   PROP_DOC_VALID,
+                                   g_param_spec_boolean ("is-doc-valid",

Shouldn't this be named "valid-signature" or sth like that then?

@@ +156,3 @@
+                                                        "If the document signed with the signature is valid",
+                                                        FALSE,
+                                                        G_PARAM_READWRITE |

Really writable?

@@ +162,3 @@
+  g_object_class_install_property (g_object_class,
+                                   PROP_ID_KNOWN,
+                                   g_param_spec_boolean ("is-id-known",

"signer-identity-known" or so?

@@ +172,3 @@
+  g_object_class_install_property (g_object_class,
+                                   PROP_SIGN_TIME,
+                                   g_param_spec_string ("sign-time",

"signature-time" or so? There's really no need to use abbreviations here.

Also, why string and not a GDateTime (param_spec_boxed with G_TYPE_DATE_TIME) ?

::: libdocument/ev-signature.h
@@ +50,3 @@
+
+struct _EvSignatureClass {
+  GObjectClass base_class;

Add some padding here:

/*< private >*/
gpointer padding[8];
Comment 13 Christian Persch 2012-09-07 12:16:41 UTC
Review of attachment 223751 [details] [review]:

Need gtk-doc stuff (see previous comments).

::: libview/ev-jobs.c
@@ +595,3 @@
+    g_list_free (job->signatures);
+    job->signatures = NULL;
+  }

g_list_free_full is simpler.
Comment 14 Christian Persch 2012-09-07 12:19:35 UTC
Review of attachment 223752 [details] [review]:

::: shell/ev-sidebar-signatures.c
@@ +22,3 @@
+
+#include <glib/gi18n.h>             // ----- support for translation
+

No C++ comments.

@@ +435,3 @@
+                                      COL_HAS_ICON, TRUE,
+                                      COL_MAKE_BOLD, TRUE,
+                                      -1);

Use gtk_tree_store_insert_with_values instead of append + set.
Comment 15 Christian Persch 2012-09-07 12:22:00 UTC
Review of attachment 223753 [details] [review]:

::: shell/ev-window.c
@@ +789,3 @@
+                                   EvWindow      *window)
+{
+  // force the sidebar to show and open the signatures

No C++ comments.

@@ +868,3 @@
+                                   const gchar *format,
+                                   ...)
+{

Add a prototype for this, with a G_GNUC_PRINTF attribute, for type safety.

@@ +882,3 @@
+  area = ev_message_area_new (GTK_MESSAGE_INFO,
+                              msg,
+                              _("See detailed information..."),

… instead of . . .
Comment 16 Christian Persch 2012-09-07 12:24:02 UTC
Review of attachment 223754 [details] [review]:

::: backend/pdf/ev-poppler.cc
@@ +3364,3 @@
+          break;
+        default:
+          g_warning ("error trying to get signature information");

Is this a runtime or programming error? In the former case, don't g_warning; in the latter case, just fix it. If it's for forward-compatibility, just don't warn (spew on console is bad).
Comment 17 Christian Persch 2012-09-07 12:24:39 UTC
Review of attachment 223755 [details] [review]:

Squash this commit to the one that introduces this new header.
Comment 18 Vasco Dias 2012-09-10 11:51:25 UTC
Review of attachment 223754 [details] [review]:

::: backend/pdf/ev-poppler.cc
@@ +3364,3 @@
+          break;
+        default:
+          g_warning ("error trying to get signature information");

When we call poppler to verify the signatures in the file errors can occur.

This case is, for example, when the signature info in the pdf file is broken, mal-formed or any other strange errors appear when the openssl is trying to read that info.

We have no way to get this information beforehand.
Comment 19 Vasco Dias 2012-09-10 11:51:56 UTC
Review of attachment 223749 [details] [review]:

::: libdocument/ev-document-signatures.c
@@ +27,3 @@
+{
+}
+

Sorry, but could you give an example of what I should do with gtk-doc?

I'm new to this and none of the other files in the project have an example regarding this.

The same for the other gtk-doc notes.

::: libdocument/ev-document-signatures.h
@@ +2,3 @@
+ *  this file is part of evince, a gnome document viewer
+ *
+ * Copyright (C) 2012 Vasco Dias  <contact@vascodias.me>

All the other files have this with (C), are you sure you want me to change it?

::: libdocument/ev-signature.c
@@ +156,3 @@
+                                                        "If the document signed with the signature is valid",
+                                                        FALSE,
+                                                        G_PARAM_READWRITE |

I was told that to create an object that only accepts the properties at construct time it would require the writable flag. Am I wrong?

@@ +172,3 @@
+  g_object_class_install_property (g_object_class,
+                                   PROP_SIGN_TIME,
+                                   g_param_spec_string ("sign-time",

That info comes directly from the pdf signature's metadata, it already comes as a string.

::: libdocument/ev-signature.h
@@ +50,3 @@
+
+struct _EvSignatureClass {
+  GObjectClass base_class;

I don't think I should work aroung compiler or memory implementation details this way. All the other code in the project have it this way.
But if you are really sure I should do it I will.
Comment 20 Vasco Dias 2012-09-10 11:52:13 UTC
Review of attachment 223753 [details] [review]:

::: shell/ev-window.c
@@ +868,3 @@
+                                   const gchar *format,
+                                   ...)
+{

Sorry, I don't get what I should do with this...
Comment 21 Christian Persch 2012-09-10 12:02:42 UTC
(In reply to comment #18)
> Review of attachment 223754 [details] [review]:
> 
> ::: backend/pdf/ev-poppler.cc
> @@ +3364,3 @@
> +          break;
> +        default:
> +          g_warning ("error trying to get signature information");
> 
> When we call poppler to verify the signatures in the file errors can occur.
> 
> This case is, for example, when the signature info in the pdf file is broken,
> mal-formed or any other strange errors appear when the openssl is trying to
> read that info.
> 
> We have no way to get this information beforehand.

Then that should be a message to the user (using a GError to propagate the info back to the ev-window code), *not* a message on console (that no one reads).

(In reply to comment #19)
> Review of attachment 223749 [details] [review]:
> 
> ::: libdocument/ev-document-signatures.c
> @@ +27,3 @@
> +{
> +}
> +
> 
> Sorry, but could you give an example of what I should do with gtk-doc?

/**
 * SECTION:ev-foobar
 * @short_description: Foor bar
 *
 * Some utility functions to do foo with bar.
 *
 * Since: 3.6
 */

/**
 * ev_foo_bar:
 * @foo: a #EvFoo
 *
 * Does foo with bar.
 *
 * Since: 3.6
 */
void
ev_foo_bar (EvFoo *foo)
{
...
}

> I'm new to this and none of the other files in the project have an example
> regarding this.

There are a *few*, e.g. ev-macros.c has SECTION , and most .c files have gtk-doc comments for the individual functions.

And the format of libevdocument-sections.txt and libevdocument-docs.xml should be easy to understand.

https://live.gnome.org/DocumentationProject/GtkDoc might help.

> The same for the other gtk-doc notes.
> 
> ::: libdocument/ev-document-signatures.h
> @@ +2,3 @@
> + *  this file is part of evince, a gnome document viewer
> + *
> + * Copyright (C) 2012 Vasco Dias  <contact@vascodias.me>
> 
> All the other files have this with (C), are you sure you want me to change it?

Well I use © for new files, but ok.
 
> ::: libdocument/ev-signature.c
> @@ +156,3 @@
> +                                                        "If the document
> signed with the signature is valid",
> +                                                        FALSE,
> +                                                        G_PARAM_READWRITE |
> 
> I was told that to create an object that only accepts the properties at
> construct time it would require the writable flag. Am I wrong?

Oh, you're right. I had overlooked that. 
> @@ +172,3 @@
> +  g_object_class_install_property (g_object_class,
> +                                   PROP_SIGN_TIME,
> +                                   g_param_spec_string ("sign-time",
> 
> That info comes directly from the pdf signature's metadata, it already comes as
> a string.

I see that the poppler patch exposes this as a string in its API, but IMHO that's a bug. It should just construct a GDateTime from the string and expose that.

> 
> ::: libdocument/ev-signature.h
> @@ +50,3 @@
> +
> +struct _EvSignatureClass {
> +  GObjectClass base_class;
> 
> I don't think I should work aroung compiler or memory implementation details
> this way. All the other code in the project have it this way.
> But if you are really sure I should do it I will.

We don't currently have this in many classes, but IMO no reason not to do it for new classes. And it's not a 'workaround', it's standard gobject practice.

(In reply to comment #20)
> Review of attachment 223753 [details] [review]:
> 
> ::: shell/ev-window.c
> @@ +868,3 @@
> +                                   const gchar *format,
> +                                   ...)
> +{
> 
> Sorry, I don't get what I should do with this...

Add a prototype for it like this:

static void
ev_window_info_message_signatures (EvWindow    *window,
                                   const gchar *format,
                                   ...) G_GNUC_PRINTF(2,3);
Comment 22 Vasco Dias 2012-09-10 14:26:06 UTC
(In reply to comment #21)
> (In reply to comment #18)
> > Review of attachment 223754 [details] [review] [details]:
> > 
> > ::: backend/pdf/ev-poppler.cc
> > @@ +3364,3 @@
> > +          break;
> > +        default:
> > +          g_warning ("error trying to get signature information");
> > 
> > When we call poppler to verify the signatures in the file errors can occur.
> > 
> > This case is, for example, when the signature info in the pdf file is broken,
> > mal-formed or any other strange errors appear when the openssl is trying to
> > read that info.
> > 
> > We have no way to get this information beforehand.
> 
> Then that should be a message to the user (using a GError to propagate the info
> back to the ev-window code), *not* a message on console (that no one reads).
> 

I think I'll just delete the g_warning message then.
Currently when this happens the signatures sidebar shows up without any content. If I wanted to use GError to handle this how should I go about it?

> Oh, you're right. I had overlooked that. 
> > @@ +172,3 @@
> > +  g_object_class_install_property (g_object_class,
> > +                                   PROP_SIGN_TIME,
> > +                                   g_param_spec_string ("sign-time",
> > 
> > That info comes directly from the pdf signature's metadata, it already comes as
> > a string.
> 
> I see that the poppler patch exposes this as a string in its API, but IMHO
> that's a bug. It should just construct a GDateTime from the string and expose
> that.
> 

Well, poppler just gives me what the openssl gets from the signature fields. Is there any easy way to convert from a string with that format to a GDateTime object?

> > 
> > ::: libdocument/ev-signature.h
> > @@ +50,3 @@
> > +
> > +struct _EvSignatureClass {
> > +  GObjectClass base_class;
> > 
> > I don't think I should work aroung compiler or memory implementation details
> > this way. All the other code in the project have it this way.
> > But if you are really sure I should do it I will.
> 
> We don't currently have this in many classes, but IMO no reason not to do it
> for new classes. And it's not a 'workaround', it's standard gobject practice.
> 
I'll add it. I didn't find many examples or explanation of what it does... Care to ellaborate? Thanks

> (In reply to comment #20)
> > Review of attachment 223753 [details] [review] [details]:
> > 
> > ::: shell/ev-window.c
> > @@ +868,3 @@
> > +                                   const gchar *format,
> > +                                   ...)
> > +{
> > 
> > Sorry, I don't get what I should do with this...
> 
> Add a prototype for it like this:
> 
> static void
> ev_window_info_message_signatures (EvWindow    *window,
>                                    const gchar *format,
>                                    ...) G_GNUC_PRINTF(2,3);

Ok, I'll add it. I don't really understand the documentation that I read about it, can you explain me what it does?
Comment 23 Vasco Dias 2012-09-10 14:47:05 UTC
Created attachment 223918 [details] [review]
Create new interface in libdocument v2
Comment 24 Vasco Dias 2012-09-10 14:47:50 UTC
Created attachment 223919 [details] [review]
New job for getting signatures info v2
Comment 25 Vasco Dias 2012-09-10 14:48:23 UTC
Created attachment 223920 [details] [review]
Create new sidebar to contain the info v2
Comment 26 Vasco Dias 2012-09-10 14:49:09 UTC
Created attachment 223921 [details] [review]
Changes to the window to include the sidebar and new message at the top v2
Comment 27 Vasco Dias 2012-09-10 14:49:30 UTC
Created attachment 223922 [details] [review]
Implement the new interface in the pdf backend v2
Comment 28 Vasco Dias 2012-09-10 15:04:59 UTC
Created attachment 223923 [details] [review]
Changes to the window to include the sidebar and new message at the top v2.1

Replacing this patch, I added the G_GNUC_PRINTF in the wrong place and didn't test it.
Comment 29 Vasco Dias 2012-09-11 13:03:04 UTC
Created attachment 224018 [details] [review]
Create new interface in libdocument v2.1

Changed the way the GENERIC_ERROR is handled. This way we can show information to the user.
Comment 30 Vasco Dias 2012-09-11 13:05:46 UTC
Created attachment 224019 [details] [review]
Implement the new interface in the pdf backend v2.1

Changed the way the GENERIC_ERROR is handled. This way we can show information to the user.
Comment 31 Vasco Dias 2012-09-11 13:07:08 UTC
Created attachment 224020 [details] [review]
Create new sidebar to contain the info v2.1

Handle the case when the signature comes without info.
Comment 32 Vasco Dias 2012-09-11 13:18:25 UTC
Created attachment 224021 [details] [review]
Implement the new interface in the pdf backend v2.1

Changed the way the GENERIC_ERROR is handled. This way we can show information to the user.
Comment 33 Simon Harhues 2012-11-05 10:58:18 UTC
Another bug relating to digital signatures, maybe it would be possible to share some code to solve it to: https://bugzilla.gnome.org/show_bug.cgi?id=640185
Comment 34 Markus Kilås 2012-12-21 21:14:05 UTC
Anything ready to be tested here yet?
Comment 35 Markus Kilås 2016-03-04 21:22:48 UTC
Basic signature verification support has now been merged in Poppler:
https://bugs.freedesktop.org/show_bug.cgi?id=16770
Comment 36 Germán Poo-Caamaño 2016-07-07 19:29:42 UTC
The support for poppler-glib frontend is not ready yet:
https://bugs.freedesktop.org/show_bug.cgi?id=94376
Comment 37 GNOME Infrastructure Team 2018-05-22 13:50:52 UTC
-- GitLab Migration Automatic Message --

This bug has been migrated to GNOME's GitLab instance and has been closed from further activity.

You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.gnome.org/GNOME/evince/issues/143.