auth-source patch for secure logging

auth-source patch for secure logging

[Ted Zlatanov]

Attached is a patch to auth-source.el that:

- introduces auth-source-debug

- sets it to nil by default, so no logging is done (this changes the
  previous behavior where we always log to *Messages*)

- allows t (relay to 'message) and any function as options for that
  variable

It's against the Gnus CVS, but Emacs CVS has the same contents.

If it's possible to include this in the upcoming release, I think it
would improve security for Emacs users.  It's not a critical fix,
however, so I will defer to the maintainers to decide.

If it's accepted, please commit it to the Emacs CVS and then Miles can
sync it back to the Gnus CVS.

Ted

Re: auth-source patch for secure logging

[Ted Zlatanov]

[-- Attachment #1: Type: text/plain, Size: 198 bytes --]

On Thu, 16 Jul 2009 09:23:29 -0500 Ted Zlatanov <tzz@lifelogs.com> wrote: 

TZ> Attached is a patch to auth-source.el that:

Of course, the actual patch file is optional on the first message.

Ted


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: auth-source.patch --]
[-- Type: text/x-diff, Size: 2802 bytes --]

Index: auth-source.el
===================================================================
RCS file: /usr/local/cvsroot/gnus/lisp/auth-source.el,v
retrieving revision 7.16
diff -r7.16 auth-source.el
102a77,100
> (defcustom auth-source-debug nil
>   "Whether auth-source should log debug messages.  
> Also see `auth-source-hide-passwords'.
> 
> Can be t, which means to use `message'.  Be careful, your
> authentication data will be in the clear (except for passwords,
> which are always cleared)..
> 
> Can also be a function, in which case the function should take
> the same parameters as `message'."
>   :group 'auth-source
>   :version "23.1" ;; No Gnus
>   :type	`(choice 
> 	  :tag "auth-source debugging mode"
> 	  (const :tag "Log using `message' to the *Messages* buffer" t)
> 	  (function :tag "Function that takes arguments like `message'")
> 	  (const :tag "Don't log anything" nil)))
> 
> (defcustom auth-source-hide-passwords t
>   "Whether auth-source should hide passwords in log messages."
>   :group 'auth-source
>   :version "23.1" ;; No Gnus
>   :type `boolean)
> 
139a138,150
> ;; (let ((auth-source-debug 'debug)) (auth-source-debug "hello"))
> ;; (let ((auth-source-debug t)) (auth-source-debug "hello"))
> ;; (let ((auth-source-debug nil)) (auth-source-debug "hello"))
> (defun auth-source-do-debug (&rest msg)
>   ;; set logger to either the function in auth-source-debug or 'message
>   ;; note that it will be 'message if auth-source-debug is nil, so
>   ;; we also check the value
>   (let ((logger (if (functionp auth-source-debug) 
> 		    auth-source-debug 
> 		  'message)))
>     (when auth-source-debug
>       (apply logger msg))))
> 
174,176c185,187
<   (gnus-message 9
< 		"auth-source-user-or-password: get %s for %s (%s)"
< 		mode host protocol)
---
>   (auth-source-do-debug
>    "auth-source-user-or-password: get %s for %s (%s)"
>    mode host protocol)
183,188c194,199
< 	  (gnus-message 9
< 			"auth-source-user-or-password: cached %s=%s for %s (%s)"
< 			mode
< 			;; don't show the password
< 			(if (member "password" mode) "SECRET" found)
< 			host protocol)
---
> 	  (auth-source-do-debug
> 	   "auth-source-user-or-password: cached %s=%s for %s (%s)"
> 	   mode
> 	   ;; don't show the password
> 	   (if (and (member "password" mode) auth-source-hide-passwords) "SECRET" found)
> 	   host protocol)
198,203c209,214
< 	  (gnus-message 9
< 			"auth-source-user-or-password: found %s=%s for %s (%s)"
< 			mode
< 			;; don't show the password
< 			(if (member "password" mode) "SECRET" found)
< 			host protocol)
---
> 	  (auth-source-do-debug
> 	   "auth-source-user-or-password: found %s=%s for %s (%s)"
> 	   mode
> 	   ;; don't show the password
> 	   (if (and (member "password" mode) auth-source-hide-passwords) "SECRET" found)
> 	   host protocol)

Re: auth-source patch for secure logging

[Chong Yidong]

Ted Zlatanov <tzz@lifelogs.com> writes:

> Attached is a patch to auth-source.el that:
>
> - introduces auth-source-debug
> - sets it to nil by default, so no logging is done (this changes the
>   previous behavior where we always log to *Messages*)
> - allows t (relay to 'message) and any function as options for that
>   variable
>
> It's against the Gnus CVS, but Emacs CVS has the same contents.
>
> If it's possible to include this in the upcoming release, I think it
> would improve security for Emacs users.  It's not a critical fix,
> however, so I will defer to the maintainers to decide.
>
> If it's accepted, please commit it to the Emacs CVS and then Miles can
> sync it back to the Gnus CVS.

Let's not check this into the branch.  These messages are not logged by
default anyway, since gnus-verbose defaults to 7.

It may be checked into the trunk, with the following caveats:

> (defcustom auth-source-debug nil
>   "Whether auth-source should log debug messages.
> Also see `auth-source-hide-passwords'.
>
> Can be t, which means to use `message'.  Be careful, your
> authentication data will be in the clear (except for passwords,
> which are always cleared)..
>
> Can also be a function, in which case the function should take
> the same parameters as `message'."

This docstring could be improved.  I suggest:

  "Whether auth-source should log debug messages.
Also see `auth-source-hide-passwords'.

If the value is nil, debug messages are not logged.
If the value is t, debug messages are logged with `message'.
 In that case, your authentication data will be in the
 clear (except for passwords, which are always stripped out).
If the value is a function, debug messages are logged by calling
 that function using the same arguments as `message'."

The docstring of auth-source-hide-passwords should also mention that
it's only relevant if auth-source-debug is non-nil.

> (defun auth-source-do-debug (&rest msg)
>   ;; set logger to either the function in auth-source-debug or 'message
>   ;; note that it will be 'message if auth-source-debug is nil, so
>   ;; we also check the value
>   (let ((logger (if (functionp auth-source-debug)
> 		    auth-source-debug
> 		  'message)))
>     (when auth-source-debug
>       (apply logger msg))))

You should put the `when' check on the outside.

Re: auth-source patch for secure logging

[Davis Herring]

> TZ> Attached is a patch to auth-source.el that:
>
> Of course, the actual patch file is optional on the first message.
>
> Ted

In fact, the grammar in RFC 2045 explicitly forbids a MIME boundary within
any message whose body contains the words "attached", "attachment", or
"attaching" (in any case) on a line that does not begin with a >, so as to
avoid confusing pre-MIME MTAs that would mistake it for the beginning of a
new, non-textual message in the DECnet style (much like the common problem
of ">From" in mbox files).  Some modern MUAs can be configured to ignore
this restriction since it's so rarely relevant now, but very few users
know about those (non-compliant) features.

So it's really not your fault.

Davis

PS - I hear that Google is actively flouting the RFC with their GMail
service by explicitly asking the user if they meant to attach files when
they request to send a compliant message.  Fortunately it's still an
experimental ("Labs") feature, and the IETF has been notified.

PPS - If replying to this message, be sure to use the > quotation style if
you desire to attach any files; the body of this message (and this very
postpostscript) include the words called out in the RFC.

-- 
This product is sold by volume, not by mass.  If it appears too dense or
too sparse, it is because mass-energy conversion has occurred during
shipping.

Re: auth-source patch for secure logging

[Ted Zlatanov]

[-- Attachment #1: Type: text/plain, Size: 469 bytes --]

On Thu, 16 Jul 2009 11:32:48 -0400 Chong Yidong <cyd@stupidchicken.com> wrote: 

CY> It may be checked into the trunk, with the following caveats:

CY> This docstring could be improved.  I suggest:
...
CY> The docstring of auth-source-hide-passwords should also mention that
CY> it's only relevant if auth-source-debug is non-nil.
...
CY> You should put the `when' check on the outside.

Thanks.  Another revision of the patch is attached for your consideration.

Ted


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: auth-source.patch --]
[-- Type: text/x-diff, Size: 2953 bytes --]

Index: auth-source.el
===================================================================
RCS file: /usr/local/cvsroot/gnus/lisp/auth-source.el,v
retrieving revision 7.16
diff -r7.16 auth-source.el
102a77,101
> (defcustom auth-source-debug nil
>   "Whether auth-source should log debug messages.
> Also see `auth-source-hide-passwords'.
> 
> If the value is nil, debug messages are not logged.
> If the value is t, debug messages are logged with `message'.
>  In that case, your authentication data will be in the
>  clear (except for passwords, which are always stripped out).
> If the value is a function, debug messages are logged by calling
>  that function using the same arguments as `message'."
>   :group 'auth-source
>   :version "23.1" ;; No Gnus
>   :type	`(choice 
> 	  :tag "auth-source debugging mode"
> 	  (const :tag "Log using `message' to the *Messages* buffer" t)
> 	  (function :tag "Function that takes arguments like `message'")
> 	  (const :tag "Don't log anything" nil)))
> 
> (defcustom auth-source-hide-passwords t
>   "Whether auth-source should hide passwords in log messages.
> Only relevant if auth-source-debug is not nil."
>   :group 'auth-source
>   :version "23.1" ;; No Gnus
>   :type `boolean)
> 
139a139,151
> ;; (let ((auth-source-debug 'debug)) (auth-source-debug "hello"))
> ;; (let ((auth-source-debug t)) (auth-source-debug "hello"))
> ;; (let ((auth-source-debug nil)) (auth-source-debug "hello"))
> (defun auth-source-do-debug (&rest msg)
>   ;; set logger to either the function in auth-source-debug or 'message
>   ;; note that it will be 'message if auth-source-debug is nil, so
>   ;; we also check the value
>   (when auth-source-debug
>     (let ((logger (if (functionp auth-source-debug)
> 		      auth-source-debug 
> 		    'message)))
>       (apply logger msg))))
> 
174,176c186,188
<   (gnus-message 9
< 		"auth-source-user-or-password: get %s for %s (%s)"
< 		mode host protocol)
---
>   (auth-source-do-debug
>    "auth-source-user-or-password: get %s for %s (%s)"
>    mode host protocol)
183,188c195,200
< 	  (gnus-message 9
< 			"auth-source-user-or-password: cached %s=%s for %s (%s)"
< 			mode
< 			;; don't show the password
< 			(if (member "password" mode) "SECRET" found)
< 			host protocol)
---
> 	  (auth-source-do-debug
> 	   "auth-source-user-or-password: cached %s=%s for %s (%s)"
> 	   mode
> 	   ;; don't show the password
> 	   (if (and (member "password" mode) auth-source-hide-passwords) "SECRET" found)
> 	   host protocol)
198,203c210,215
< 	  (gnus-message 9
< 			"auth-source-user-or-password: found %s=%s for %s (%s)"
< 			mode
< 			;; don't show the password
< 			(if (member "password" mode) "SECRET" found)
< 			host protocol)
---
> 	  (auth-source-do-debug
> 	   "auth-source-user-or-password: found %s=%s for %s (%s)"
> 	   mode
> 	   ;; don't show the password
> 	   (if (and (member "password" mode) auth-source-hide-passwords) "SECRET" found)
> 	   host protocol)

Re: auth-source patch for secure logging

[Ted Zlatanov]

On Thu, 16 Jul 2009 08:55:23 -0700 (PDT) "Davis Herring" <herring@lanl.gov> wrote: 

TZ> Attached is a patch to auth-source.el that:
>> 
>> Of course, the actual patch file is optional on the first message.

DH> In fact, the grammar in RFC 2045 explicitly forbids a MIME boundary within
DH> any message whose body contains the words "attached", "attachment", or
DH> "attaching" (in any case) on a line that does not begin with a >, so as to
DH> avoid confusing pre-MIME MTAs that would mistake it for the beginning of a
DH> new, non-textual message in the DECnet style (much like the common problem
DH> of ">From" in mbox files).  Some modern MUAs can be configured to ignore
DH> this restriction since it's so rarely relevant now, but very few users
DH> know about those (non-compliant) features.

DH> So it's really not your fault.

DH> Davis

DH> PS - I hear that Google is actively flouting the RFC with their GMail
DH> service by explicitly asking the user if they meant to attach files when
DH> they request to send a compliant message.  Fortunately it's still an
DH> experimental ("Labs") feature, and the IETF has been notified.

DH> PPS - If replying to this message, be sure to use the > quotation style if
DH> you desire to attach any files; the body of this message (and this very
DH> postpostscript) include the words called out in the RFC.

This made my day, thanks Davis.

Ted

Re: auth-source patch for secure logging

[Ted Zlatanov]

I've comitted the updated auth-source.el into the Gnus CVS repository
rather than directly to Emacs, since this is going to the Emacs trunk so
it's not urgent.  Miles will synchronize it eventually :)

Thanks
Ted

Re: auth-source patch for secure logging

[Glenn Morris]

Chong Yidong wrote:

> Let's not check this into the branch.

It seems it got checked into the branch.

Re: auth-source patch for secure logging

[Ted Zlatanov]

On Thu, 16 Jul 2009 11:32:48 -0400 Chong Yidong <cyd@stupidchicken.com> wrote: 

CY> Let's not check this into the branch.
...
CY> It may be checked into the trunk, with the following caveats:

On Fri, 17 Jul 2009 13:31:43 -0400 Glenn Morris <rgm@gnu.org> wrote: 

GM> It seems it got checked into the branch.

I checked it into the Gnus CVS trunk.  It should have been synchronized
with the Emacs CVS trunk only.  What branch are you referring to?

Ted

Re: auth-source patch for secure logging

[Glenn Morris]

Ted Zlatanov wrote:

> GM> It seems it got checked into the branch.
>
> I checked it into the Gnus CVS trunk.  It should have been synchronized
> with the Emacs CVS trunk only.  What branch are you referring to?

The EMACS_23_1_RC release branch in Emacs CVS.

http://lists.gnu.org/archive/html/emacs-diffs/2009-07/msg00480.html

Re: auth-source patch for secure logging

[Ted Zlatanov]

On Fri, 17 Jul 2009 14:01:21 -0400 Glenn Morris <rgm@gnu.org> wrote: 

GM> Ted Zlatanov wrote:
GM> It seems it got checked into the branch.
>> 
>> I checked it into the Gnus CVS trunk.  It should have been synchronized
>> with the Emacs CVS trunk only.  What branch are you referring to?

GM> The EMACS_23_1_RC release branch in Emacs CVS.

GM> http://lists.gnu.org/archive/html/emacs-diffs/2009-07/msg00480.html

Right, I know about this branch, I was just making sure we're discussing
the same thing.  I don't know how it got into the 23.1 RC branch, sorry.
As I mentioned, it was not my intention that it would.  Maybe Miles'
synchronization script misfired.

Ted

Re: auth-source patch for secure logging

[Chong Yidong]

Ted Zlatanov <tzz@lifelogs.com> writes:

> On Fri, 17 Jul 2009 14:01:21 -0400 Glenn Morris <rgm@gnu.org> wrote:
>
> GM> Ted Zlatanov wrote:
> GM> It seems it got checked into the branch.
>>>
>>> I checked it into the Gnus CVS trunk.  It should have been synchronized
>>> with the Emacs CVS trunk only.  What branch are you referring to?
>
> GM> The EMACS_23_1_RC release branch in Emacs CVS.
>
> GM> http://lists.gnu.org/archive/html/emacs-diffs/2009-07/msg00480.html
>
> Right, I know about this branch, I was just making sure we're discussing
> the same thing.  I don't know how it got into the 23.1 RC branch, sorry.
> As I mentioned, it was not my intention that it would.  Maybe Miles'
> synchronization script misfired.

Could someone who knows what they're doing remove the patch from the
branch?  I'd do it myself, but I'm not sure if that will end up
propagating to the Gnus trunk.

Re: auth-source patch for secure logging

[Sven Joachim]

On 2009-07-17 20:09 +0200, Ted Zlatanov wrote:

> On Fri, 17 Jul 2009 14:01:21 -0400 Glenn Morris <rgm@gnu.org> wrote: 
>
> GM> Ted Zlatanov wrote:
> GM> It seems it got checked into the branch.
>>> 
>>> I checked it into the Gnus CVS trunk.  It should have been synchronized
>>> with the Emacs CVS trunk only.  What branch are you referring to?
>
> GM> The EMACS_23_1_RC release branch in Emacs CVS.
>
> GM> http://lists.gnu.org/archive/html/emacs-diffs/2009-07/msg00480.html
>
> Right, I know about this branch, I was just making sure we're discussing
> the same thing.  I don't know how it got into the 23.1 RC branch, sorry.

Relating trunks and branches, should not Gnus branch as well for the
5.12 release?  I thought that this release was planned to happen at
about the same time as the Emacs 23.1 release.

> As I mentioned, it was not my intention that it would.  Maybe Miles'
> synchronization script misfired.

BTW, is it agreed that Gnus trunk should continue to be synced with the
Emacs trunk?

Cheers,
       Sven

Re: auth-source patch for secure logging

[Chong Yidong]

Chong Yidong <cyd@stupidchicken.com> writes:

>> Right, I know about this branch, I was just making sure we're discussing
>> the same thing.  I don't know how it got into the 23.1 RC branch, sorry.
>> As I mentioned, it was not my intention that it would.  Maybe Miles'
>> synchronization script misfired.
>
> Could someone who knows what they're doing remove the patch from the
> branch?  I'd do it myself, but I'm not sure if that will end up
> propagating to the Gnus trunk.

Since no one spoke up, I went ahead and reverted the changes in the
Emacs CVS branch.