<<< Date Index >>>     <<< Thread Index >>>

Re: [PATCH] Re: Security issue / bad UI design in mutt CVS (encryption options)



* Derek Martin [Sat, 07 Aug 2004 19:53:15 +0900]:

> > One frequently heared criticism of the old behavior concerned the
> > fact that it takes two key presses to come from "both" to "sign
> > only" or to "encrypt only." What do you guys think -- should the
> > "encrypt" option disable signing, and should "sign" disable
> > encryption, with "both" (or "encrypt" followed by "sign as") being
> > the only way to get both security functions?

> Yes, definitely.  This always was the point of all of the bug reports
> (both mine and the others), and I believe it is the reason behind the
> recent behavior change.  This behavior saves keystrokes, and as such
> I think it is the right way to go.

> The attached patch should solve all of the afforementioned problems,
> and should apply against the current CVS.  I can't thoroughly test the
> S/MIME stuff since I don't use it, but the PGP menu tested out ok.

  I object to the inclusion of this patch as-is, detailed rationale
  follows:

    - I strongly object to changing the default shorcut to wipe crypt
      options from (f)orget to (c)lear. The latter is probably more
      appropriate, but I know for sure that the "pfy" keystroke sequence
      is hard-coded in many mutt users' fingers. *Please* don't break
      that.

    - While the "msg->security = XXX" is ok for (e)ncrypt and (s)ign, I
      don't think it's appropriate for sign (a)s or encrypt (w)ith. The
      patch leaves the sign (a)s stuff right (i.e., untouched), but leaves
      the encrypt (w)ith thing in an inconsistent state (it's not then
      possible to do "bw" and choose key, one has to add (s)ign again
      later).

  I then propose the attached patch for inclusion in mutt CVS. I ask:

    - Derek to consider it as a compromise between no-change and his
      full patch, and to say if he agrees.

    - Thomas to commit it and put Derek's name in the changelog, since
      it's him who has raised the issue in the past and this time too.

    - everybody else reading this to comment about it.


  thanks,


-- 
Adeodato Simó
    EM: asp16 [ykwim] alu.ua.es | PK: DA6AE621
 
If you think nobody cares if you're alive, try missing a couple of car
payments.
                -- Earl Wilson
--- mutt-cvs/pgp.c      2004-07-20 15:05:21.000000000 +0200
+++ mutt-cvs/pgp.c      2004-08-07 13:38:41.000000000 +0200
@@ -1462,15 +1462,15 @@
   if (!(WithCrypto & APPLICATION_PGP))
     return msg->security;
 
-  switch (mutt_multi_choice (_("PGP (e)ncrypt, (s)ign, sign (a)s, (b)oth, 
(i)nline, or (f)orget it? "),
+  switch (mutt_multi_choice (_("PGP (e)ncrypt, (s)ign, sign (a)s, (b)oth, 
(i)nline [toggle], or (f)orget it? "),
                             _("esabif")))
   {
   case 1: /* (e)ncrypt */
-    msg->security ^= ENCRYPT;
+    msg->security = (msg->security & INLINE) | ENCRYPT;
     break;
 
   case 2: /* (s)ign */
-    msg->security ^= SIGN;
+    msg->security = (msg->security & INLINE) | SIGN;
     break;
 
   case 3: /* sign (a)s */
@@ -1496,10 +1496,7 @@
     break;
 
   case 4: /* (b)oth */
-    if ((msg->security & (ENCRYPT | SIGN)) == (ENCRYPT | SIGN))
-      msg->security = 0;
-    else
-      msg->security |= (ENCRYPT | SIGN);
+    msg->security = (msg->security & INLINE) | ENCRYPT | SIGN;
     break;
 
   case 5: /* (i)nline */
--- mutt-cvs/smime.c    2004-06-28 03:46:06.000000000 +0200
+++ mutt-cvs/smime.c    2004-08-07 13:38:17.000000000 +0200
@@ -1944,7 +1944,7 @@
                             _("eswabf")))
   {
   case 1: /* (e)ncrypt */
-    msg->security ^= ENCRYPT;
+    msg->security = ENCRYPT;
     break;
 
   case 3: /* encrypt (w)ith */
@@ -1977,7 +1977,7 @@
     if(!SmimeDefaultKey)
        mutt_message("Can\'t sign: No key specified. use sign(as).");
     else
-       msg->security ^= SIGN;
+       msg->security = SIGN;
     break;
 
   case 4: /* sign (a)s */
@@ -1998,7 +1998,7 @@
     break;
 
   case 5: /* (b)oth */
-    msg->security |= (ENCRYPT | SIGN);
+    msg->security = (ENCRYPT | SIGN);
     break;
 
   case 6: /* (f)orget it */