GNU extensions to read_s2k for 2.5.x [was: Re: more on read_s2k() for GnuTLS ...]

[Daniel Kahn Gillmor]

On Tue 2008-08-19 06:02:04 -0400, Nikos Mavrogiannopoulos wrote:

> It looks nice. Expect me to apply it soon (later today or tomorrow).

I see that it's applied in git already.  Thank you very much, Nikos!

But ack!  i've got a frustrating request (but one that i figure is
better done now than later): I've done a bit more reading, and found a
reference to one other GNU S2K extension used by GPG.  In DETAILS from
the GnuPG sources [0], it says:

   GNU extensions to the S2K algorithm
   ===================================
   S2K mode 101 is used to identify these extensions.
   After the hash algorithm the 3 bytes "GNU" are used to make
   clear that these are extensions for GNU, the next bytes gives the
   GNU protection mode - 1000.  Defined modes are:
     1001 - do not store the secret part at all
     1002 - a stub to access smartcards (not used in 1.2.x)

I'm not proposing that we handle mode 1002 yet (i haven't encountered
it and don't know how we'd talk to the smartcard anyway), but
semantically, the code i asked you to commit now seems slightly wrong.
In particular, it treats S2K mode 101 as GNU-Dummy, when in fact it
should be "GNU Extensions", and it should just test the data after the
hash to find out whether it's the gnu-dummy extension or not.

The attached patch (against the current git head) doesn't change any
functionality in the code, but it makes the semantics more congruent
with the extension strategy outlined by GPG.  It should also make it
easier for any of us to implement/adopt other GNU S2K extensions in
the future.  Sorry for the confusion.  Please let me know if there's
any trouble with the patch.

Regards,

        --dkg

[0] http://cvs.gnupg.org/cgi-bin/viewcvs.cgi/trunk/doc/DETAILS?root=GnuPG

diff --git a/lib/opencdk/opencdk.h b/lib/opencdk/opencdk.h
index d4717b1..af2b6d6 100644
--- a/lib/opencdk/opencdk.h
+++ b/lib/opencdk/opencdk.h
@@ -186,7 +186,10 @@ enum cdk_s2k_type_t {
     CDK_S2K_SIMPLE     = 0,
     CDK_S2K_SALTED     = 1,
     CDK_S2K_ITERSALTED = 3,
-    CDK_S2K_GNU_DUMMY = 101 /* look for --export-secret-subkeys in gpg(1) */
+    CDK_S2K_GNU_EXT = 101
+ /* GNU  extensions: refer to DETAILS from GnuPG: 
+  http://cvs.gnupg.org/cgi-bin/viewcvs.cgi/trunk/doc/DETAILS?root=GnuPG
+ */
 };
 
 /* The different kind of user ID preferences. */
diff --git a/lib/opencdk/read-packet.c b/lib/opencdk/read-packet.c
index 78e5605..bc3c58b 100644
--- a/lib/opencdk/read-packet.c
+++ b/lib/opencdk/read-packet.c
@@ -98,9 +98,9 @@ read_s2k (cdk_stream_t inp, cdk_s2k_t s2k)
       if (s2k->mode == CDK_S2K_ITERSALTED)
        s2k->count = cdk_stream_getc (inp);
     }
-  else if (s2k->mode == CDK_S2K_GNU_DUMMY)
+  else if (s2k->mode == CDK_S2K_GNU_EXT)
     {
-      /* look for --export-secret-subkeys in gpg(1) */
+      /* GNU extensions to the S2K : read DETAILS from gnupg */
       return 0;
     }
   else
@@ -356,7 +356,7 @@ read_secret_key (cdk_stream_t inp, size_t pktlen, 
cdk_pkt_seckey_t sk)
       if (rc)
        return rc;
        /* refer to --export-secret-subkeys in gpg(1) */
-      if (sk->protect.s2k->mode == CDK_S2K_GNU_DUMMY) 
+      if (sk->protect.s2k->mode == CDK_S2K_GNU_EXT) 
        sk->protect.ivlen = 0;
       else {
        sk->protect.ivlen = gcry_cipher_get_algo_blklen (sk->protect.algo);
@@ -421,11 +421,22 @@ read_secret_key (cdk_stream_t inp, size_t pktlen, 
cdk_pkt_seckey_t sk)
        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;
+      /* Handle the GNU S2K extensions we know (just gnu-dummy right now): */
+      if (sk->protect.s2k->mode == CDK_S2K_GNU_EXT) {
+       unsigned char gnumode;
+       if ((sk->enclen < strlen("GNU") + 1) ||
+           (0 != memcmp("GNU", sk->encdata, strlen("GNU"))))
+         return CDK_Inv_Packet;
+       gnumode = sk->encdata[strlen("GNU")];
+       /* we only handle gnu-dummy (mode 1).
+          mode 2 should refer to external smart cards.
+       */
+       if (gnumode != 1)
+         return CDK_Inv_Packet;
+       /* gnu-dummy should have no more data */
+       if (sk->enclen != strlen("GNU") + 1)
+         return CDK_Inv_Packet;
+      }
       nskey = cdk_pk_get_nskey (sk->pk->pubkey_algo);
       if (!nskey)
        {

pgpIMh4YcjXcG.pgp
Description: PGP signature