unofficial mirror of emacs-devel@gnu.org 
 help / color / mirror / code / Atom feed
* smtpmail.el security flaw in selecting authentication mechanism
@ 2009-03-03 16:30 Simon Josefsson
  2009-03-03 19:20 ` Stefan Monnier
  2009-03-04 22:01 ` Richard M Stallman
  0 siblings, 2 replies; 5+ messages in thread
From: Simon Josefsson @ 2009-03-03 16:30 UTC (permalink / raw)
  To: emacs-devel

I just noticed that smtpmail.el chose to use the LOGIN mechanism against
gmail.com which was surprising because they support PLAIN, which should
be preferred.  Debugging this I noticed this is the code that is
responsible for selecting the authentication mechanism to use:

(defun smtpmail-try-auth-methods (process supported-extensions host port)
  (let* ((mechs (cdr-safe (assoc 'auth supported-extensions)))
	 (mech (car (smtpmail-intersection smtpmail-auth-supported mechs)))

Some experiments with this:

(smtpmail-intersection smtpmail-auth-supported '(login plain cram-md5))
(login plain cram-md5)

Thus the code choses the first supported mechanism in the _servers_
order.  It should use the local list instead.  Compare:

(smtpmail-intersection '(login plain cram-md5) smtpmail-auth-supported)
(cram-md5 plain login)

The patch below fixes this.  I have committed it on the trunk.  Maybe it
it should be backported in case you make releases from another branch?

This can be a security problem, since it allows the server to control
whether for example LOGIN or PLAIN (that sends the password in
plaintext) is used instead of CRAM-MD5 (which does not).  Of course,
security aware people use STARTTLS anyway, which should mitigate this.

/Simon

Index: lisp/ChangeLog
===================================================================
RCS file: /sources/emacs/emacs/lisp/ChangeLog,v
retrieving revision 1.15426
diff -u -p -r1.15426 ChangeLog
--- lisp/ChangeLog	3 Mar 2009 16:12:02 -0000	1.15426
+++ lisp/ChangeLog	3 Mar 2009 16:22:18 -0000
@@ -1,3 +1,11 @@
+2009-03-03  Simon Josefsson  <simon@josefsson.org>
+
+	* mail/smtpmail.el (smtpmail-auth-supported): Mention that list is
+	in preference order.
+	(smtpmail-try-auth-methods): Improve which authentication
+	mechanism to use, so that the locally most preferred and mutually
+	supported mechanism is used.
+
 2009-03-03  Stefan Monnier  <monnier@iro.umontreal.ca>
 
 	* emacs-lisp/lisp.el (end-of-defun-function): Make it more clear that
Index: lisp/mail/smtpmail.el
===================================================================
RCS file: /sources/emacs/emacs/lisp/mail/smtpmail.el,v
retrieving revision 1.108
diff -u -p -r1.108 smtpmail.el
--- lisp/mail/smtpmail.el	5 Jan 2009 03:22:37 -0000	1.108
+++ lisp/mail/smtpmail.el	3 Mar 2009 16:22:18 -0000
@@ -218,7 +218,8 @@ This is relative to `smtpmail-queue-dir'
 (defvar smtpmail-read-point)
 
 (defconst smtpmail-auth-supported '(cram-md5 plain login)
-  "List of supported SMTP AUTH mechanisms.")
+  "List of supported SMTP AUTH mechanisms.
+The list is in preference order.")
 
 (defvar smtpmail-mail-address nil
   "Value to use for envelope-from address for mail from ambient buffer.")
@@ -534,7 +535,7 @@ This is relative to `smtpmail-queue-dir'
 
 (defun smtpmail-try-auth-methods (process supported-extensions host port)
   (let* ((mechs (cdr-safe (assoc 'auth supported-extensions)))
-	 (mech (car (smtpmail-intersection smtpmail-auth-supported mechs)))
+	 (mech (car (smtpmail-intersection mechs smtpmail-auth-supported)))
 	 (auth-user (auth-source-user-or-password
 		     "login" host (or port "smtp")))
 	 (auth-pass (auth-source-user-or-password




^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: smtpmail.el security flaw in selecting authentication mechanism
  2009-03-03 16:30 smtpmail.el security flaw in selecting authentication mechanism Simon Josefsson
@ 2009-03-03 19:20 ` Stefan Monnier
  2009-03-04 22:01 ` Richard M Stallman
  1 sibling, 0 replies; 5+ messages in thread
From: Stefan Monnier @ 2009-03-03 19:20 UTC (permalink / raw)
  To: Simon Josefsson; +Cc: emacs-devel

> The patch below fixes this.  I have committed it on the trunk.  Maybe it
> it should be backported in case you make releases from another branch?

We haven't cut a branch for the release yet.  Thank you for installing
the patch.


        Stefan




^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: smtpmail.el security flaw in selecting authentication mechanism
  2009-03-03 16:30 smtpmail.el security flaw in selecting authentication mechanism Simon Josefsson
  2009-03-03 19:20 ` Stefan Monnier
@ 2009-03-04 22:01 ` Richard M Stallman
  2009-03-04 22:12   ` Simon Josefsson
  2009-03-04 23:27   ` Stefan Monnier
  1 sibling, 2 replies; 5+ messages in thread
From: Richard M Stallman @ 2009-03-04 22:01 UTC (permalink / raw)
  To: Simon Josefsson; +Cc: emacs-devel

I am pretty sure we don't plan to make another release of Emacs 22.
Do you think we should make one just on account of this?




^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: smtpmail.el security flaw in selecting authentication mechanism
  2009-03-04 22:01 ` Richard M Stallman
@ 2009-03-04 22:12   ` Simon Josefsson
  2009-03-04 23:27   ` Stefan Monnier
  1 sibling, 0 replies; 5+ messages in thread
From: Simon Josefsson @ 2009-03-04 22:12 UTC (permalink / raw)
  To: rms; +Cc: emacs-devel

Richard M Stallman <rms@gnu.org> writes:

> I am pretty sure we don't plan to make another release of Emacs 22.
> Do you think we should make one just on account of this?

No.

/Simon




^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: smtpmail.el security flaw in selecting authentication mechanism
  2009-03-04 22:01 ` Richard M Stallman
  2009-03-04 22:12   ` Simon Josefsson
@ 2009-03-04 23:27   ` Stefan Monnier
  1 sibling, 0 replies; 5+ messages in thread
From: Stefan Monnier @ 2009-03-04 23:27 UTC (permalink / raw)
  To: rms; +Cc: Simon Josefsson, emacs-devel

> I am pretty sure we don't plan to make another release of Emacs 22.
> Do you think we should make one just on account of this?

No, it's not a serious security flaw: it still only uses one of the
protocols that we accept to use.  It just might choose a less
desirable one.


        Stefan




^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2009-03-04 23:27 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2009-03-03 16:30 smtpmail.el security flaw in selecting authentication mechanism Simon Josefsson
2009-03-03 19:20 ` Stefan Monnier
2009-03-04 22:01 ` Richard M Stallman
2009-03-04 22:12   ` Simon Josefsson
2009-03-04 23:27   ` Stefan Monnier

Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).