[gnutls-dev] [PATCH] Fixing OpenPGP keyring import (again)

[Ludovic Courtès]

Hi,

The patch below (against) `HEAD' fixes OpenPGP keyring import.  It
should also work for ASCII-armored keyrings, although I did not test it
since I did not have ASCII-armored keyrings at hand.  It also adds a
test for this so that we can catch it earlier next time.

Two issues with the test:

  1. For some reason, `check_id ()' doesn't work with the second ID
     that's commented in `keyring.c', although it should.  Actually, I
     have the exact same test in Scheme and that one works.  So there
     must be something fishy going on, but I couldn't find out what.

  2. There's a memory leak in `cdk_keydb_get_pk ()':

     3,466 (24 direct, 3,442 indirect) bytes in 2 blocks are definitely lost in 
loss record 4 of 4
        at 0x401D4B0: malloc (vg_replace_malloc.c:149)
        by 0x420C7F3: (within /usr/lib/libgcrypt.so.11.2.2)
        by 0x420CA60: gcry_malloc (in /usr/lib/libgcrypt.so.11.2.2)
        by 0x420CCDC: gcry_calloc (in /usr/lib/libgcrypt.so.11.2.2)
        by 0x402BBA5: cdk_calloc (main.c:163)
        by 0x402E33A: cdk_kbnode_new (kbnode.c:41)
        by 0x4030758: cdk_keydb_get_keyblock (keydb.c:1715)
        by 0x4031953: cdk_keydb_search (keydb.c:938)
        by 0x40325D7: cdk_keydb_get_pk (keydb.c:1268)
        by 0x4029878: gnutls_openpgp_keyring_check_id (extras.c:103)
        by 0x8048921: doit (keyring.c:163)
        by 0x8048B44: main (utils.c:148)

     Fixing it is left as an exercise to the reader.  :-)

     Also, for some unknown reason, Valgrind doesn't show the above leak
     when just run from `make check'.

Thanks,
Ludovic.

PS: BTW, how's Git going?  :-)


ChangeLog entry:

        * libextra/openpgp/extras.c (gnutls_openpgp_keyring_import):
        Fixed again, for raw keyring import (ASCII keyring import
        untested).

        * configure.in: Added `tests/openpgp/Makefile'.
        * tests/Makefile.am (SUBDIRS): Add `openpgp' when
        `ENABLE_OPENPGP' is true.

--- orig/configure.in
+++ mod/configure.in
@@ -637,7 +637,7 @@
        tests/pkcs1-padding/Makefile tests/pkcs8-decode/Makefile \
        tests/pkcs12-decode/Makefile tests/pathlen/Makefile \
        tests/key-id/Makefile tests/sha2/Makefile \
-       tests/hostname-check/Makefile \
+       tests/hostname-check/Makefile tests/openpgp/Makefile \
        includes/Makefile includes/gnutls/gnutls.h \
        lib/Makefile lib/minitasn1/Makefile lib/x509/Makefile \
        libextra/Makefile libextra/openpgp/Makefile libextra/opencdk/Makefile \


--- orig/libextra/openpgp/extras.c
+++ mod/libextra/openpgp/extras.c
@@ -110,13 +110,14 @@
 }
 
 /**
- * gnutls_openpgp_keyring_import - This function will import a RAW or BASE64 
encoded key
+ * gnutls_openpgp_keyring_import - Import a raw- or Base64-encoded OpenPGP 
keyring
  * @keyring: The structure to store the parsed key.
  * @data: The RAW or BASE64 encoded keyring.
- * @format: One of gnutls_openpgp_keyring_fmt elements.
+ * @format: One of #gnutls_openpgp_keyring_fmt elements.
  *
- * This function will convert the given RAW or Base64 encoded keyring
- * to the native gnutls_openpgp_keyring_t format. The output will be stored in 
'keyring'.
+ * This function will convert the given RAW or Base64 encoded keyring to the
+ * native #gnutls_openpgp_keyring_t format.  The output will be stored in
+ * 'keyring'.
  *
  * Returns 0 on success.
  *
@@ -126,30 +127,53 @@
                               const gnutls_datum_t *data,
                               gnutls_openpgp_key_fmt_t format)
 {
-  int rc;
-  keybox_blob *blob = NULL;
-
+  int rc = 0;
+  cdk_error_t err;
 
-  blob = kbx_read_blob (data, 0);
-  if (!blob)
+  if (format == GNUTLS_OPENPGP_FMT_RAW)
     {
-      gnutls_assert ();
-      return GNUTLS_E_OPENPGP_KEYRING_ERROR;
+      err = cdk_keydb_new (&keyring->db, CDK_DBTYPE_DATA,
+                          data->data, data->size);
+      if (err)
+       {
+         gnutls_assert ();
+         goto leave;
+       }
     }
-
-  keyring->db = kbx_to_keydb (blob);
-  if (!keyring->db)
+  else
     {
-      gnutls_assert ();
-      rc = GNUTLS_E_OPENPGP_KEYRING_ERROR;
-      goto leave;
+      cdk_stream_t input;
+
+      err = cdk_stream_tmp_from_mem (data->data, data->size, &input);
+      if (err)
+       {
+         gnutls_assert ();
+         goto leave;
+       }
+
+      err = cdk_stream_set_armor_flag (input, CDK_ARMOR_MESSAGE);
+      if (err)
+       {
+         cdk_stream_close (input);
+         gnutls_assert ();
+         goto leave;
+       }
+
+      err = cdk_keydb_new_from_stream (&keyring->db, 0, input);
+
+      cdk_stream_close (input);
+
+      if (err)
+       {
+         gnutls_assert ();
+         goto leave;
+       }
     }
 
-  rc = 0;
 
-leave:
-  kbx_blob_release (blob);
+ leave:
+  rc = _gnutls_map_cdk_rc (err);
   return rc;
 }
 
--- orig/tests/Makefile.am
+++ mod/tests/Makefile.am
@@ -21,6 +21,9 @@
 
 SUBDIRS = rsa-md5-collision pkcs1-padding pkcs8-decode pkcs12-decode   \
        userid pathlen key-id sha2 hostname-check
+if ENABLE_OPENPGP
+SUBDIRS += openpgp
+endif
 
 AM_CPPFLAGS = -I$(top_srcdir)/lgl -I$(top_builddir)/lgl                        
\
        -I$(top_srcdir)/gl -I$(top_builddir)/gl                         \


--- /dev/null
+++ 
/home/ludo/src/laas/gnutls/,,address@hidden/new-files-archive/./tests/openpgp/Makefile.am
@@ -0,0 +1,41 @@
+## Process this file with automake to produce Makefile.in
+# Copyright (C) 2007 Free Software Foundation
+#
+# Author: Ludovic Courtès.
+#
+# This file is part of GNUTLS.
+#
+# This file is free software; you can redistribute it and/or modify it
+# under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+#
+# This file is distributed in the hope that it will be useful, but
+# WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+# General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this file; if not, write to the Free Software Foundation,
+# Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
+
+openpgp_sources = keyring.c
+
+if ENABLE_OPENPGP
+TESTS_ENVIRONMENT = $(VALGRIND)
+
+TESTS = keyring
+
+check_PROGRAMS = keyring
+
+AM_CPPFLAGS = -I$(top_srcdir)/includes -I$(top_builddir)/includes      \
+             -I$(top_srcdir)/tests
+LDADD = -L$(top_builddir)/libextra -lgnutls-extra      \
+        -L$(top_builddir)/lib      -lgnutls            \
+       -L$(top_builddir)/tests    -lutils
+
+else
+
+EXTRA_DIST = $(openpgp_sources)
+
+endif
--- /dev/null
+++ 
/home/ludo/src/laas/gnutls/,,address@hidden/new-files-archive/./tests/openpgp/keyring.c
@@ -0,0 +1,180 @@
+/*
+ * Copyright (C) 2007 Free Software Foundation
+ * Author: Ludovic Courtès
+ *
+ * This file is part of GNUTLS.
+ *
+ * GNUTLS is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * GNUTLS is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with GNUTLS; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA
+ */
+
+#ifdef HAVE_CONFIG_H
+# include "config.h"
+#endif
+
+#include <stdio.h>
+
+#include <gnutls/gnutls.h>
+#include <gnutls/extra.h>
+#include <gnutls/openpgp.h>
+
+#include "utils.h"
+
+/* A raw-encoded OpenPGP keyring.  This is a copy of (`sha1sum' output):
+   5fdce61bff528070dfabdd237d91be618c353b4e  src/openpgp/cli_ring.gpg  */
+static unsigned char raw_keyring[] =
+  "\231\1\242\4\74\147\225\215\21\4\0\200\261\145\41\213\370\50"
+  "\6\372\157\114\30\13\361\361\117\300\20\56\17\116\25\140\121\55"
+  "\13\277\270\244\32\172\220\133\7\215\104\173\115\65\44\6\303\244"
+  "\330\373\314\36\260\335\277\117\202\343\35\202\37\306\6\77\127\276"
+  "\73\107\366\310\265\244\361\113\276\222\101\165\333\50\252\155\273\303"
+  "\22\40\235\170\224\372\163\173\310\262\326\74\274\237\111\262\216\140"
+  "\374\260\174\136\10\52\363\304\173\215\161\122\336\21\376\130\56\157"
+  "\377\243\372\110\4\137\315\171\170\347\267\25\173\0\240\277\24\237"
+  "\32\311\275\230\132\54\244\235\1\335\21\262\203\223\1\321\337\3"
+  "\375\24\20\257\42\102\31\324\166\234\267\270\125\367\55\74\275\220"
+  "\4\77\365\136\33\156\156\241\33\172\326\225\77\33\54\252\262\135"
+  "\3\347\251\224\24\123\355\101\350\221\40\132\204\317\40\231\51\215"
+  "\271\52\313\16\350\317\174\113\132\62\16\230\42\100\176\52\255\25"
+  "\170\222\304\321\305\323\144\201\366\364\242\145\43\372\244\327\21\270"
+  "\53\260\372\7\107\12\150\160\277\57\200\110\240\247\20\54\234\337"
+  "\114\203\360\335\372\322\342\65\136\65\244\31\64\164\225\251\237\77"
+  "\126\143\214\3\377\153\220\333\134\161\16\21\125\337\126\114\132\7"
+  "\52\364\370\275\370\210\110\103\210\314\241\246\160\26\75\37\51\252"
+  "\354\300\234\213\171\215\173\200\203\42\151\57\146\11\343\16\122\100"
+  "\63\335\102\137\123\203\266\23\313\6\253\362\206\163\41\207\20\347"
+  "\150\71\170\66\36\66\270\363\22\257\322\104\133\142\60\240\206\305"
+  "\235\355\164\212\21\223\73\211\101\113\120\266\361\107\322\30\103\46"
+  "\377\302\101\62\334\100\215\266\62\334\26\63\122\320\214\3\346\306"
+  "\4\156\225\241\356\142\344\264\45\104\162\56\40\127\150\157\40\50"
+  "\116\157\40\143\157\155\155\145\156\164\163\51\40\74\167\150\157\100"
+  "\167\150\157\151\163\56\157\162\147\76\210\135\4\23\21\2\0\35"
+  "\5\2\74\147\225\215\5\11\3\302\147\0\5\13\7\12\3\4"
+  "\3\25\3\2\3\26\2\1\2\27\200\0\12\11\20\65\24\134"
+  "\352\247\331\74\77\226\130\0\237\170\231\313\311\366\351\114\60\173"
+  "\230\70\167\150\4\333\373\103\327\317\157\0\240\244\135\2\220\125"
+  "\63\240\155\313\353\326\311\161\372\35\361\172\145\70\376\231\1\242"
+  "\4\74\112\305\154\21\4\0\347\56\166\266\56\357\251\243\275\131"
+  "\100\223\51\44\30\5\14\2\327\2\235\154\242\6\156\374\64\310"
+  "\140\70\142\174\144\76\261\246\122\247\257\35\67\317\106\374\120\132"
+  "\301\340\306\231\263\170\225\264\274\263\345\65\101\377\332\107\146\326"
+  "\26\214\53\212\257\326\253\42\106\155\6\321\200\64\325\332\306\230"
+  "\346\231\73\245\263\120\377\202\56\34\330\160\52\165\21\116\213\163"
+  "\246\260\234\263\271\74\344\115\273\121\154\233\265\371\133\266\146\30"
+  "\206\2\240\241\104\162\66\300\145\217\0\240\217\133\136\170\330\137"
+  "\171\54\302\7\57\224\164\144\127\46\373\115\223\163\3\376\65\170"
+  "\326\211\326\140\156\221\30\351\371\247\4\53\226\74\362\77\75\217"
+  "\23\167\242\163\300\360\227\115\277\104\263\312\274\276\24\335\144\101"
+  "\45\125\206\76\71\251\306\47\146\55\167\254\66\146\52\344\111\171"
+  "\54\62\142\323\361\56\230\62\247\126\123\11\326\173\240\256\115\362"
+  "\137\136\332\11\67\5\152\325\276\211\364\6\236\275\176\307\154\344"
+  "\62\104\35\365\325\57\377\320\155\71\345\366\36\66\224\173\151\212"
+  "\167\313\142\253\201\344\244\22\53\371\5\6\161\331\224\154\206\136"
+  "\4\0\320\141\103\172\226\115\336\61\210\30\302\262\115\340\10\346"
+  "\0\226\266\15\270\246\204\270\132\203\215\21\237\311\60\61\30\211"
+  "\255\127\243\271\47\364\110\370\116\262\123\306\43\355\247\73\102\377"
+  "\170\274\346\72\152\123\35\165\246\114\350\124\5\23\200\216\237\133"
+  "\20\316\7\135\64\27\270\1\26\111\30\261\61\323\124\114\207\145"
+  "\250\354\271\227\37\141\240\237\307\75\120\230\6\20\153\131\167\322"
+  "\21\313\16\35\4\320\355\226\274\350\233\256\217\163\330\0\260\122"
+  "\23\234\277\215\264\111\117\160\145\156\103\104\113\40\164\145\163\164"
+  "\40\153\145\171\40\50\117\156\154\171\40\151\156\164\145\156\144\145"
+  "\144\40\146\157\162\40\164\145\163\164\40\160\165\162\160\157\163\145"
+  "\163\41\51\40\74\157\160\145\156\143\144\153\100\146\157\157\55\142"
+  "\141\162\56\157\162\147\76\210\142\4\23\21\2\0\32\5\2\74"
+  "\112\305\154\5\13\7\12\3\4\3\25\3\2\3\26\2\1\2"
+  "\36\1\2\27\200\0\22\11\20\275\127\54\334\314\300\174\65\7"
+  "\145\107\120\107\0\1\1\201\301\0\234\16\22\215\216\324\104\174"
+  "\155\313\316\141\120\331\315\206\342\15\204\131\245\0\237\146\201\146"
+  "\54\200\306\252\317\35\55\53\302\4\360\202\376\200\323\333\244\271"
+  "\1\15\4\74\112\305\157\20\4\0\342\1\126\122\140\151\320\147"
+  "\322\117\115\161\346\323\206\130\340\213\343\277\44\154\32\334\340\215"
+  "\266\234\330\324\131\301\355\63\127\70\101\7\230\165\132\375\267\237"
+  "\27\227\317\2\56\160\307\226\17\22\312\150\226\322\174\375\44\241"
+  "\34\323\26\335\341\373\314\36\246\25\305\303\37\354\145\156\106\160"
+  "\170\310\165\374\120\233\36\313\231\310\265\154\55\207\134\120\342\1"
+  "\213\133\17\243\170\140\156\266\102\132\45\63\203\17\125\375\41\326"
+  "\111\1\126\25\324\232\35\11\351\121\17\137\0\3\5\4\0\320"
+  "\275\255\344\4\62\165\206\165\310\175\7\60\303\140\230\24\147\272"
+  "\341\276\266\314\20\132\74\37\66\153\375\276\241\56\67\204\126\121"
+  "\62\70\270\255\101\116\122\242\251\146\35\35\361\333\153\265\363\77"
+  "\151\6\26\141\7\125\154\201\62\44\63\13\60\223\55\267\310\314"
+  "\202\45\147\55\172\342\112\362\106\227\120\345\71\266\141\352\144\165"
+  "\322\340\74\330\323\203\215\304\250\254\112\375\41\65\66\376\76\226"
+  "\354\235\12\352\145\26\113\127\156\1\263\172\215\312\211\362\262\127"
+  "\320\210\116\4\30\21\2\0\6\5\2\74\112\305\157\0\22\11"
+  "\20\275\127\54\334\314\300\174\65\7\145\107\120\107\0\1\1\165"
+  "\146\0\237\140\36\37\231\340\260\174\167\346\177\76\354\241\341\237"
+  "\224\143\323\163\147\0\237\152\306\236\264\21\232\157\373\364\111\347"
+  "\321\124\330\56\5\324\10\141\333";
+
+/* The ID of a key known to be in the above keyring.  */
+static const unsigned char id_in_keyring[8] =
+  /* "Dr. Who", first key in the keyring */
+  { 0x35, 0x14, 0x5c, 0xea,
+    0xa7, 0xd9, 0x3c, 0x3f };
+/*   { 0xbd, 0x57, 0x2c, 0xdc, */
+/*     0xcc, 0xc0, 0x7c, 0x35 }; */
+
+static const unsigned char id_not_in_keyring[8] =
+  { 0x00, 0x00, 0x00, 0x00,
+    0x00, 0x00, 0x00, 0x00 };
+
+
+void
+doit (void)
+{
+  int ret;
+  gnutls_openpgp_keyring_t keyring;
+  gnutls_datum_t data;
+
+  ret = gnutls_global_init ();
+  if (ret < 0)
+    fail ("init %d\n", ret);
+
+  ret = gnutls_global_init_extra ();
+  if (ret < 0)
+    fail ("extra-init %d\n", ret);
+
+  ret = gnutls_openpgp_keyring_init (&keyring);
+  if (ret < 0)
+    fail ("keyring-init %d\n", ret);
+
+
+  data.data = raw_keyring;
+  data.size = sizeof (raw_keyring);
+  ret = gnutls_openpgp_keyring_import (keyring, &data,
+                                      GNUTLS_OPENPGP_FMT_RAW);
+
+  if (ret < 0)
+    fail ("keyring-import %d\n", ret);
+
+  ret = gnutls_openpgp_keyring_check_id (keyring, id_not_in_keyring, 0);
+  if (ret == 0)
+    fail ("keyring-check-id (not-in-keyring) %d\n", ret);
+
+  ret = gnutls_openpgp_keyring_check_id (keyring, id_in_keyring, 0);
+  if (ret != 0)
+    fail ("keyring-check-id %d\n", ret);
+
+  success ("done\n");
+
+  gnutls_openpgp_keyring_deinit (keyring);
+  gnutls_global_deinit ();
+}
+
+/* Local Variables:
+   coding: latin-1
+   End:
+ */