more on read_s2k() for GnuTLS 2.4.1 (including "GNU dummy S2K")

[Daniel Kahn Gillmor]

After reading Nikos' assessment of the state of OpenCDK in handling
enciphered subpackets, i've backed off from my original goal of
actually decrypting secret keys.  I'd like GnuTLS to be able to do
that at some point, but i don't understand OpenCDK's filter model well
enough yet to actually implement the cipher needed to decrypt locked
secret keys.

However, there is a reason to prefer to be able to identify and parse
locked secret keys, even if we can't unlock them yet.  In particular,
RFC4880 allows for some secret subkeys (or the primary key) in a
keyset to be unlocked while others are locked.

For example, you might want to provide a TLS service with the
Authentication subkey for the host's key, but not provide it with the
unlocked primary key itself.  Presented with a keyset like this
(locked primary key, unlocked subkey), and asked to use the subkey,
GnuTLS will currently fail to operate despite having access to all the
information needed.

My earlier read_s2k implementation [0] would have allowed GnuTLS to
handle this case properly.

gpg itself is capable of generating such a keyset, using the flag:

 --export-secret-subkeys

But it "locks" the primary key by removing the secret MPIs entirely
for the primary key packet, using an experimental S2K identifier
(101), which GPG classifies as "gnu-dummy".

gpg(1) says of --export-secret-subkeys:

   the command has the special property to render the secret part of
   the primary key useless; this is a GNU extension to OpenPGP and
   other implementations can not be expected to successfully import
   such a key.

However, it isn't difficult to actually import such a key (and to
ensure that such a packet conforms to the flavor emitted by GPG).  The
attached patch is an update to my earlier read_s2k implementation and
is capable of interpreting the gnu-dummy S2K extension.  As a GNU
project, this seems like a worthwhile step to me.

Is there any objection to including this GNU extension in the event
that a 2.4.2 release is made?

I'd be happy to implement the same thing for the 2.5.x branch as well,
if there are no objections.

As always, i welcome feedback!

    --dkg
‐

[0] http://lists.gnu.org/archive/html/gnutls-devel/2008-06/msg00092.html

--- gnutls26-2.4.1/lib/opencdk/read-packet.c    2008-06-30 16:45:51.000000000 
-0400
+++ gnutls26-2.4.1.dkg/lib/opencdk/read-packet.c        2008-08-13 
21:19:47.000000000 -0400
@@ -78,10 +78,35 @@
 }
 
 
-static int
+/* read about S2K at http://tools.ietf.org/html/rfc4880#section-3.7.1 */
+static cdk_error_t
 read_s2k (cdk_stream_t inp, cdk_s2k_t s2k)
 {
-  return CDK_Not_Implemented;
+  size_t nread;
+
+  s2k->mode = cdk_stream_getc (inp);
+  s2k->hash_algo = cdk_stream_getc (inp);
+  if (s2k->mode == CDK_S2K_SIMPLE) 
+      return 0;
+  else if (s2k->mode == CDK_S2K_SALTED || s2k->mode == CDK_S2K_ITERSALTED)
+    {
+      if (stream_read (inp, s2k->salt, DIM (s2k->salt), &nread))
+       return CDK_Inv_Packet;
+      if (nread != DIM (s2k->salt))
+       return CDK_Inv_Packet;
+      
+      if (s2k->mode == CDK_S2K_ITERSALTED)
+       s2k->count = cdk_stream_getc (inp);
+    }
+  else if (s2k->mode == CDK_S2K_GNU_DUMMY)
+    {
+      /* look for --export-secret-subkeys in gpg(1) */
+      return 0;
+    }
+  else
+    return CDK_Not_Implemented;
+
+  return 0;
 }
 
 
@@ -194,6 +219,7 @@
 static cdk_error_t
 read_symkey_enc (cdk_stream_t inp, size_t pktlen, cdk_pkt_symkey_enc_t ske)
 {
+  cdk_error_t ret;
   cdk_s2k_t s2k;
   size_t minlen;
   size_t nread, nleft;
@@ -213,7 +239,9 @@
     return CDK_Out_Of_Core;
   
   ske->cipher_algo = cdk_stream_getc (inp);
-  s2k->mode = cdk_stream_getc (inp);
+  ret = read_s2k(inp, s2k);
+  if (ret != 0)
+    return ret;
   switch (s2k->mode)
     {
     case CDK_S2K_SIMPLE    : minlen = 0; break;
@@ -225,18 +253,6 @@
       return CDK_Inv_Packet;
     }
   
-  s2k->hash_algo = cdk_stream_getc (inp);
-  if (s2k->mode == CDK_S2K_SALTED || s2k->mode == CDK_S2K_ITERSALTED)
-    {
-      if (stream_read (inp, s2k->salt, DIM (s2k->salt), &nread))
-       return CDK_Inv_Packet;
-      if (nread != DIM (s2k->salt))
-       return CDK_Inv_Packet;
-      
-      if (s2k->mode == CDK_S2K_ITERSALTED)
-       s2k->count = cdk_stream_getc (inp);
-    }
-  
   ske->seskeylen = pktlen - 4 - minlen;
   /* We check if there is an encrypted session key and if it fits into
      the buffer. The maximal key length is 256-bit. */
@@ -421,14 +437,19 @@
       rc = read_s2k (inp, sk->protect.s2k);
       if (rc) 
        return rc;
-      sk->protect.ivlen = gcry_cipher_get_algo_blklen (sk->protect.algo);
-      if (!sk->protect.ivlen)
-       return CDK_Inv_Packet;
-      rc = stream_read (inp, sk->protect.iv, sk->protect.ivlen, &nread);
-      if (rc)
-       return rc;
-      if (nread != sk->protect.ivlen)
-       return CDK_Inv_Packet;
+      /* refer to --export-secret-subkeys in gpg(1) */
+      if (sk->protect.s2k->mode == CDK_S2K_GNU_DUMMY) 
+       sk->protect.ivlen = 0;
+      else {
+       sk->protect.ivlen = gcry_cipher_get_algo_blklen (sk->protect.algo);
+       if (!sk->protect.ivlen)
+         return CDK_Inv_Packet;
+       rc = stream_read (inp, sk->protect.iv, sk->protect.ivlen, &nread);
+       if (rc)
+         return rc;
+       if (nread != sk->protect.ivlen)
+         return CDK_Inv_Packet;
+      }
     }
   else
     sk->protect.algo = sk->s2k_usage;
@@ -476,6 +497,11 @@
        return CDK_Out_Of_Core;
       if (stream_read (inp, sk->encdata, sk->enclen, &nread))
        return CDK_Inv_Packet;
+      /* checking that this is supposed to be a GNU Dummy S2K, which we know: 
*/
+      if ((sk->protect.s2k->mode == CDK_S2K_GNU_DUMMY) && 
+         ((sk->enclen != strlen("GNU\01")) ||
+          (0 != memcmp("GNU\01", sk->encdata, strlen("GNU\01")))))
+       return CDK_Inv_Packet;
       nskey = cdk_pk_get_nskey (sk->pk->pubkey_algo);
       if (!nskey)
        return CDK_Inv_Algo;
--- gnutls26-2.4.1/lib/opencdk/opencdk.h        2008-06-30 16:45:51.000000000 
-0400
+++ gnutls26-2.4.1.dkg/lib/opencdk/opencdk.h    2008-08-13 21:08:20.000000000 
-0400
@@ -214,7 +214,8 @@
 enum cdk_s2k_type_t {
     CDK_S2K_SIMPLE     = 0,
     CDK_S2K_SALTED     = 1,
-    CDK_S2K_ITERSALTED = 3
+    CDK_S2K_ITERSALTED = 3,
+    CDK_S2K_GNU_DUMMY = 101 /* look for --export-secret-subkeys in gpg(1) */
 };

pgp7bQSerTH29.pgp
Description: PGP signature