[Help-gnutls] Re: Failure to import an OpenPGP private key

[Ludovic Courtès]

Hi,

Simon Josefsson <address@hidden> writes:

> Installed in CVS now.  Could you suggest the patch to fix your
> original problem, using this new scheme?

Thanks for doing it!  Attached is the updated patch.

> I just realized a thing: I'm not sure we are really breaking the
> API/ABI here though.  No public API/ABI is modified, only internal
> _gnutls_* APIs.  The same holds for your first patch.

Indeed, no _public_ ABI/API is modified.  That said, the ABI _is_
modified: one cannot use an older `libgnutls-extra' with a newer
`libgnutls' (or vice versa).  But that would have been an issue only if
one had been allowed to use different versions of `libgnutls-extra' and
`libgnutls' together.

> Changing
> _gnutls_* APIs without bumping the shared library version should be
> ok, right?  gnutls-extra should be the only user of those _gnutls*
> symbols, and libgnutls-extra is only ever guaranteed to work with the
> same version of libgnutls (and gnutls_global_init_extra already checks
> this).

Ok, so there's not problem.  ;-)

Changing the SO version should be ok.

> Anyway, I think the installed patch is cleaner.  For one, it removed
> including GPL'd gnutls-extra header files in the LGPL'd libgnutls,
> which seems like a good step.  There are still some variables which
> are used between gnutls and gnutls-extra, but I'll see if they cause
> any real problems (e.g., on mingw32) before working on moving those to
> a function-based API.

Good.

In my original message [0], I had in mind something that would implement
a slightly higher abstraction level over certificate types, such that no
X509/OpenPGP-specific code and no `switch (certtype)' need to appear in
`auth_cert.c' et al.  For instance, we'd move the
`proc_{x509,openpgp}_server_certificate ()' functions to specific files,
and instead just call
`_gnutls_cert_vtable[certtype].process_server_certificate ()', and so on.

But maybe it's a bit cosmetic.

Thanks,
Ludovic.

[0] http://lists.gnu.org/archive/html/help-gnutls/2007-01/msg00008.html


ChangeLog entry:

        * lib/gnutls_cert.c (_gnutls_raw_privkey_to_gkey): Pass KEY_ENC to
        `_E_gnutls_openpgp_raw_privkey_to_gkey ()'.

        * lib/gnutls_extra_hooks.h
        (_gnutls_openpgp_raw_privkey_to_gkey_func): Added a
        `gnutls_openpgp_key_fmt_t' argument.

        * libextra/gnutls_openpgp.c
        (_gnutls_openpgp_raw_privkey_to_gkey): Take a new FORMAT
        argument.  When FORMAT is `BASE64', set the armor flag on OUT.
        (gnutls_certificate_set_openpgp_key_mem): Pass
        `GNUTLS_OPENPGP_FMT_RAW' as the last argument to
        `_gnutls_openpgp_raw_privkey_to_gkey ()'.

        * libextra/openpgp/gnutls_openpgp.h
        (_gnutls_openpgp_raw_privkey_to_gkey): Updated accordingly.

        * libextra/openpgp/privkey.c (gnutls_openpgp_privkey_import):
        Pass FORMAT to `_gnutls_openpgp_raw_privkey_to_gkey ()'.


--- orig/lib/gnutls_cert.c
+++ mod/lib/gnutls_cert.c
@@ -718,7 +718,9 @@
          gnutls_assert ();
          return GNUTLS_E_INIT_LIBEXTRA;
        }
-      return _E_gnutls_openpgp_raw_privkey_to_gkey (key, raw_key);
+      return _E_gnutls_openpgp_raw_privkey_to_gkey (key, raw_key,
+                                                   (gnutls_openpgp_key_fmt_t)
+                                                   key_enc);
     default:
       gnutls_assert ();
       return GNUTLS_E_INTERNAL_ERROR;


--- orig/lib/gnutls_extra_hooks.h
+++ mod/lib/gnutls_extra_hooks.h
@@ -53,7 +53,8 @@
  const gnutls_datum_t *);
 typedef int (*_gnutls_openpgp_raw_privkey_to_gkey_func)
 (gnutls_privkey *,
- const gnutls_datum_t *);
+ const gnutls_datum_t *,
+ gnutls_openpgp_key_fmt_t);
 
 typedef int (*_gnutls_openpgp_key_to_gcert_func)
 (gnutls_cert *, gnutls_openpgp_key_t);


--- orig/libextra/gnutls_openpgp.c
+++ mod/libextra/gnutls_openpgp.c
@@ -309,6 +309,7 @@
  * _gnutls_openpgp_raw_privkey_to_gkey - Converts an OpenPGP secret key to 
GnuTLS
  * @pkey: the GnuTLS private key context to store the key.
  * @raw_key: the raw data which contains the whole key packets.
+ * @format: the format of the key packets.
  *
  * The RFC2440 (OpenPGP Message Format) data is converted into the
  * GnuTLS specific data which is need to perform secret key operations.
@@ -317,9 +318,10 @@
  -*/
 int
 _gnutls_openpgp_raw_privkey_to_gkey (gnutls_privkey * pkey,
-                                    const gnutls_datum_t * raw_key)
+                                    const gnutls_datum_t * raw_key,
+                                    gnutls_openpgp_key_fmt_t format)
 {
-  cdk_kbnode_t snode;
+  cdk_kbnode_t snode = NULL;
   cdk_packet_t pkt;
   cdk_stream_t out;
   cdk_pkt_seckey_t sk = NULL;
@@ -338,6 +340,17 @@
   if (!out)
     return GNUTLS_E_CERTIFICATE_ERROR;
 
+  if (format == GNUTLS_OPENPGP_FMT_BASE64)
+    {
+      rc = cdk_stream_set_armor_flag (out, 0);
+      if (rc)
+       {
+         rc = _gnutls_map_cdk_rc (rc);
+         gnutls_assert ();
+         goto leave;
+       }
+    }
+
   cdk_stream_write (out, raw_key->data, raw_key->size);
   cdk_stream_seek (out, 0);
 
@@ -559,7 +572,7 @@
  * @key: the datum that contains the secret key.
  *
  * This funtion is used to load OpenPGP keys into the GnuTLS credential 
structure.
- * It doesn't matter whether the keys are armored or but, but the files
+ * It doesn't matter whether the keys are armored or not, but the files
  * should only contain one key which should not be encrypted.
  **/
 int
@@ -695,7 +708,8 @@
   cdk_stream_close (inp);
 
   rc = _gnutls_openpgp_raw_privkey_to_gkey (&res->pkey[res->ncerts - 1],
-                                           &raw);
+                                           &raw,
+                                           GNUTLS_OPENPGP_FMT_RAW);
   if (rc)
     {
       gnutls_assert ();
@@ -717,7 +731,7 @@
  * @keyfile: the file that contains the secret key.
  *
  * This funtion is used to load OpenPGP keys into the GnuTLS credentials 
structure.
- * It doesn't matter whether the keys are armored or but, but the files
+ * It doesn't matter whether the keys are armored or not, but the files
  * should only contain one key which should not be encrypted.
  **/
 int


--- orig/libextra/openpgp/gnutls_openpgp.h
+++ mod/libextra/openpgp/gnutls_openpgp.h
@@ -66,7 +66,8 @@
 
 int
 _gnutls_openpgp_raw_privkey_to_gkey (gnutls_privkey * pkey,
-                                    const gnutls_datum_t * raw_key);
+                                    const gnutls_datum_t * raw_key,
+                                    gnutls_openpgp_key_fmt_t format);
 
 int
 _gnutls_openpgp_request_key (gnutls_session_t,


--- orig/libextra/openpgp/privkey.c
+++ mod/libextra/openpgp/privkey.c
@@ -94,7 +94,8 @@
 {
   int rc;
 
-  rc = _gnutls_openpgp_raw_privkey_to_gkey (&key->pkey, data);
+  rc = _gnutls_openpgp_raw_privkey_to_gkey (&key->pkey, data,
+                                           format);
   if (rc)
     {
       gnutls_assert ();