fix off-by-one in building attribute subpackets

change default compression to 1
add ask-sig-expire and ask-cert-expire (--expert was getting absurdly
overloaded)
permit v3 subkeys
use --expert to protect adding multiple photo ids and adding photos to a
v3 key

8 files changed, 106 insertions, 19 deletions

diff --git a/g10/ChangeLog b/g10/ChangeLog
index 69982aa77..91eabf8ec 100644
--- a/g10/ChangeLog
+++ b/g10/ChangeLog
@@ -1,3 +1,29 @@
+2002-01-05  David Shaw  <[email protected]>
+
+	* keyedit.c (menu_adduid): Require --expert before adding a photo
+	ID to a v3 key, and before adding a second photo ID to any key.
+
+	* keyedit.c (keyedit_menu): Don't allow adding photo IDs in
+	rfc1991 or pgp2 mode.
+
+	* getkey.c (merge_selfsigs_subkey): Permit v3 subkeys.  Believe it
+	or not, this is allowed by rfc 2440, and both PGP 6 and PGP 7 work
+	fine with them.
+
+	* g10.c, options.h, keyedit.c, sign.c: Move the "ask for
+	expiration" switch off of --expert, which was getting quite
+	overloaded, and onto ask-sig-expire and ask-cert-expire.  Both
+	default to off.
+
+	* g10.c (main): Change the default compression algo to 1, to be
+	more OpenPGP compliant (PGP also uses this, so it'll help with
+	interoperability problems as well).
+
+	* encode.c (encode_crypt): Handle compression algo 2, since the
+	default is now 1.
+
+	* build-packet.c (build_attribute_subpkt): Fix off-by-one error.
+
 2002-01-05  Werner Koch  <[email protected]>
 
 	* g10.c (main): Do not register the secret keyrings for certain
diff --git a/g10/build-packet.c b/g10/build-packet.c
index af1b5515b..55c923f15 100644
--- a/g10/build-packet.c
+++ b/g10/build-packet.c
@@ -928,7 +928,7 @@ build_attribute_subpkt(PKT_user_id *uid,byte type,
   /* realloc uid->attrib_data to the right size */
 
   uid->attrib_data=m_realloc(uid->attrib_data,
-			     uid->attrib_len+idx+headerlen+buflen);
+			     uid->attrib_len+idx+1+headerlen+buflen);
 
   attrib=&uid->attrib_data[uid->attrib_len];
 
diff --git a/g10/encode.c b/g10/encode.c
index 89093df01..3a90c3712 100644
--- a/g10/encode.c
+++ b/g10/encode.c
@@ -397,7 +397,11 @@ encode_crypt( const char *filename, STRLIST remusr )
 	    ; /* don't use compression */
 	else {
 	    if( compr_algo == 1 )
-		zfx.algo = 1; /* default is 2 */
+		zfx.algo = 1;
+	    if( compr_algo == 2 )
+		zfx.algo = 2;
+	    /* Any other compr_algo will fall back to
+               opt.def_compress_algo in the compress_filter. */
 	    iobuf_push_filter( out, compress_filter, &zfx );
 	}
     }
diff --git a/g10/g10.c b/g10/g10.c
index 553047c15..1c64516a7 100644
--- a/g10/g10.c
+++ b/g10/g10.c
@@ -123,6 +123,10 @@ enum cmd_and_opt_values { aNull = 0,
     oTextmode,
     oExpert,
     oNoExpert,
+    oAskSigExpire,
+    oNoAskSigExpire,
+    oAskCertExpire,
+    oNoAskCertExpire,
     oFingerprint,
     oWithFingerprint,
     oAnswerYes,
@@ -336,6 +340,10 @@ static ARGPARSE_OPTS opts[] = {
     { oTextmode, "textmode",  0, N_("use canonical text mode")},
     { oExpert, "expert",   0, "@"},
     { oNoExpert, "no-expert",   0, "@"},
+    { oAskSigExpire, "ask-sig-expire",   0, "@"},
+    { oNoAskSigExpire, "no-ask-sig-expire",   0, "@"},
+    { oAskCertExpire, "ask-cert-expire",   0, "@"},
+    { oNoAskCertExpire, "no-ask-cert-expire",   0, "@"},
     { oOutput, "output",    2, N_("use as output file")},
     { oVerbose, "verbose",   0, N_("verbose") },
     { oQuiet,	"quiet",   0, N_("be somewhat more quiet") },
@@ -747,7 +755,7 @@ main( int argc, char **argv )
     /* note: if you change these lines, look at oOpenPGP */
     opt.def_cipher_algo = 0;
     opt.def_digest_algo = 0;
-    opt.def_compress_algo = 2;
+    opt.def_compress_algo = 1;
     opt.s2k_mode = 3; /* iterated+salted */
     opt.s2k_digest_algo = DIGEST_ALGO_SHA1;
     opt.s2k_cipher_algo = CIPHER_ALGO_CAST5;
@@ -1100,6 +1108,10 @@ main( int argc, char **argv )
 	  case oTextmode: opt.textmode=1;  break;
 	  case oExpert: opt.expert = 1; break;
 	  case oNoExpert: opt.expert = 0; break;
+	  case oAskSigExpire: opt.ask_sig_expire = 1; break;
+	  case oNoAskSigExpire: opt.ask_sig_expire = 0; break;
+	  case oAskCertExpire: opt.ask_cert_expire = 1; break;
+	  case oNoAskCertExpire: opt.ask_cert_expire = 0; break;
 	  case oUser: /* store the local users */
 	    add_to_strlist2( &locusr, pargs.r.ret_str, utf8_strings );
 	    break;
diff --git a/g10/getkey.c b/g10/getkey.c
index bf4e50ed1..298c117ae 100644
--- a/g10/getkey.c
+++ b/g10/getkey.c
@@ -1442,8 +1442,6 @@ merge_selfsigs_subkey( KBNODE keyblock, KBNODE subnode )
     subpk->is_valid = 0;
     subpk->main_keyid[0] = mainpk->main_keyid[0];
     subpk->main_keyid[1] = mainpk->main_keyid[1];
-    if ( subpk->version < 4 )
-        return; /* there are no v3 subkeys */
 
     /* find the latest key binding self-signature. */
     signode = NULL;
diff --git a/g10/keyedit.c b/g10/keyedit.c
index 72e4a65d0..1b7260d0c 100644
--- a/g10/keyedit.c
+++ b/g10/keyedit.c
@@ -435,7 +435,7 @@ sign_uids( KBNODE keyblock, STRLIST locusr, int *ret_modified,
 
 	/* Only ask for duration if we haven't already set it to match
            the expiration of the pk */
-	if(opt.expert && !duration)
+	if(opt.ask_cert_expire && !duration)
 	  duration=ask_expire_interval(1);
 
 	if(duration)
@@ -1029,10 +1029,11 @@ keyedit_menu( const char *username, STRLIST locusr, STRLIST commands,
 	    break;
 
 	  case cmdADDPHOTO:
-            if (opt.rfc2440)
+            if (opt.rfc2440 || opt.rfc1991 || opt.pgp2)
               {
                 tty_printf(
-                   _("This command is not allowed while in OpenPGP mode.\n"));
+                   _("This command is not allowed while in %s mode.\n"),
+		   opt.rfc2440?"OpenPGP":opt.pgp2?"PGP2":"RFC-1991");
                 break;
               }
 	    photo=1;
@@ -1543,19 +1544,63 @@ menu_adduid( KBNODE pub_keyblock, KBNODE sec_keyblock, int photo)
     assert(pk && sk);
 
     if(photo) {
-      /* PGP allows only one photo ID per key? */
+      int hasphoto=0;
+
+      /* PGP allows only one photo ID per key?  This is a good
+         question.  While there is no way to add more than one photo
+         ID using PGP, nevertheless PGP (7) still works properly with
+         more than one photo ID (presenting them in a nice little
+         scrolling window, no less).  GnuPG can work with any number
+         of photos. -dms */
       for( node = pub_keyblock; node; node = node->next )
 	if( node->pkt->pkttype == PKT_USER_ID &&
-	    node->pkt->pkt.user_id->attrib_data!=NULL) {
-	  log_error("You may only have one photo ID on a key.\n");
-	  return 0;
+	    node->pkt->pkt.user_id->attrib_data!=NULL)
+	  {
+	    hasphoto=1;
+
+	    if(opt.expert)
+	      {
+		tty_printf(_("WARNING: This key already has a photo ID.\n"
+			     "         Adding another photo ID may confuse "
+			     "some versions of PGP.\n"));
+		if(!cpr_get_answer_is_yes("keyedit.multi_photo.okay",
+					  _("Are you sure you still want "
+					    "to add it? (y/n) ")))
+		  return 0;
+		else
+		  break;
+	      }
+	    else
+	      {
+		tty_printf(_("You may only have one photo ID on a key.\n"));
+		return 0;
+	      }
 	}
 
-      if(pk->version==3)
+      /* Here's another one - PGP6/7 does not allow adding a photo ID
+         to a v3 key.  Still, if one is present, it will work.  Of
+         course, it does mean that PGP2 will not be able to use that
+         key anymore.  Don't bother to ask this if the key already has
+         a photo - any damage has already been done at that point. */
+      if(pk->version==3 && !hasphoto)
 	{
-	  tty_printf(_("\nWARNING: This is a PGP2-style key\n"));
-	  tty_printf(_("         Adding a photo ID may cause some versions "
-		       "of PGP to not accept this key\n"));
+	  if(opt.expert)
+	    {
+	      tty_printf(_("WARNING: This is a PGP2-style key.  "
+			   "Adding a photo ID may cause some versions\n"
+			   "         of PGP to reject this key.\n"));
+
+	      if(!cpr_get_answer_is_yes("keyedit.v3_photo.okay",
+					_("Are you sure you still want "
+					  "to add it? (y/n) ")))
+		return 0;
+	    }
+	  else
+	    {
+	      tty_printf(_("You may not add a photo ID to "
+			   "a PGP2-style key.\n"));
+	      return 0;
+	    }
 	}
 
       uid = generate_photo_id(pk);
diff --git a/g10/options.h b/g10/options.h
index a9c14c1fa..820cf280d 100644
--- a/g10/options.h
+++ b/g10/options.h
@@ -45,6 +45,8 @@ struct {
     int list_only;
     int textmode;
     int expert;
+    int ask_sig_expire;
+    int ask_cert_expire;
     int batch;	    /* run in batch mode */
     int answer_yes; /* answer yes on most questions */
     int answer_no;  /* answer no on most questions */
diff --git a/g10/sign.c b/g10/sign.c
index 740302494..f2f1ec45c 100644
--- a/g10/sign.c
+++ b/g10/sign.c
@@ -564,7 +564,7 @@ sign_file( STRLIST filenames, int detached, STRLIST locusr,
     if( fname && filenames->next && (!detached || encryptflag) )
 	log_bug("multiple files can only be detached signed");
 
-    if(opt.expert && !opt.pgp2 && !opt.batch &&
+    if(opt.ask_sig_expire && !opt.pgp2 && !opt.batch &&
        !opt.force_v3_sigs && !old_style)
       duration=ask_expire_interval(1);
 
@@ -742,7 +742,7 @@ clearsign_file( const char *fname, STRLIST locusr, const char *outfile )
     memset( &afx, 0, sizeof afx);
     init_packet( &pkt );
 
-    if(opt.expert && !opt.pgp2 && !opt.batch &&
+    if(opt.ask_sig_expire && !opt.pgp2 && !opt.batch &&
        !opt.force_v3_sigs && !old_style)
       duration=ask_expire_interval(1);
 
@@ -886,7 +886,7 @@ sign_symencrypt_file (const char *fname, STRLIST locusr)
     memset( &cfx, 0, sizeof cfx);
     init_packet( &pkt );
 
-    if(opt.expert && !opt.batch && !opt.force_v3_sigs && !old_style)
+    if(opt.ask_sig_expire && !opt.batch && !opt.force_v3_sigs && !old_style)
       duration=ask_expire_interval(1);
 
     rc = build_sk_list (locusr, &sk_list, 1, PUBKEY_USAGE_SIG);