bug: epg send unencrypted data in trash

bug: epg send unencrypted data in trash

[Thierry Volpiatto]

Hi,
i start using recently delete-by-moving-to-trash.(i had no trash before)
So now when i decrypt a file and read it in emacs, i have an unencrypted
file world readable 'epg-output-xxx' in the trash.
That's bad.

-- 
A+ Thierry
Get my Gnupg key:
gpg --keyserver pgp.mit.edu --recv-keys 59F29997 

Re: bug: epg send unencrypted data in trash

[Thierry Volpiatto]

Thierry Volpiatto <thierry.volpiatto@gmail.com> writes:

> Hi,
> i start using recently delete-by-moving-to-trash.(i had no trash before)
> So now when i decrypt a file and read it in emacs, i have an unencrypted
> file world readable 'epg-output-xxx' in the trash.
> That's bad.

So if one use gnus with a .authinfo.gpg, and use also
delete-by-moving-to-trash, he will have the bad surprise to find his
authinfo data in clear text in the trash.

Maybe let-bind delete-by-moving-to-trash in epg-delete-output-file would
be good.
It's what i will do here anyway.


(defun epg-delete-output-file (context)
  "Delete the output file of CONTEXT."
  (let ((delete-by-moving-to-trash nil))
    (when (and (epg-context-output-file context)
             (file-exists-p (epg-context-output-file context)))
      (delete-file (epg-context-output-file context)))))

With this change, data in clear text will never go to trash.

-- 
A+ Thierry
Get my Gnupg key:
gpg --keyserver pgp.mit.edu --recv-keys 59F29997 

Re: bug: epg send unencrypted data in trash

[Ted Zlatanov]

On Fri, 11 Feb 2011 19:16:13 +0100 Thierry Volpiatto <thierry.volpiatto@gmail.com> wrote: 

TV> Thierry Volpiatto <thierry.volpiatto@gmail.com> writes:
>> i start using recently delete-by-moving-to-trash.(i had no trash before)
>> So now when i decrypt a file and read it in emacs, i have an unencrypted
>> file world readable 'epg-output-xxx' in the trash.
>> That's bad.

TV> So if one use gnus with a .authinfo.gpg, and use also
TV> delete-by-moving-to-trash, he will have the bad surprise to find his
TV> authinfo data in clear text in the trash.

TV> Maybe let-bind delete-by-moving-to-trash in epg-delete-output-file would
TV> be good.
TV> It's what i will do here anyway.

TV> (defun epg-delete-output-file (context)
TV>   "Delete the output file of CONTEXT."
TV>   (let ((delete-by-moving-to-trash nil))
TV>     (when (and (epg-context-output-file context)
TV>              (file-exists-p (epg-context-output-file context)))
TV>       (delete-file (epg-context-output-file context)))))

TV> With this change, data in clear text will never go to trash.

According to the docs for `delete-file' it shouldn't matter what
`delete-by-moving-to-trash' says because this is called
non-interactively (TRASH is nil when delete-file is called
non-interactively).  Or is this funcall interactive somehow?

Ted

Re: bug: epg send unencrypted data in trash

[Thierry Volpiatto]

Ted Zlatanov <tzz@lifelogs.com> writes:

> On Fri, 11 Feb 2011 19:16:13 +0100 Thierry Volpiatto <thierry.volpiatto@gmail.com> wrote: 
>
> TV> Thierry Volpiatto <thierry.volpiatto@gmail.com> writes:
>>> i start using recently delete-by-moving-to-trash.(i had no trash before)
>>> So now when i decrypt a file and read it in emacs, i have an unencrypted
>>> file world readable 'epg-output-xxx' in the trash.
>>> That's bad.
>
> TV> So if one use gnus with a .authinfo.gpg, and use also
> TV> delete-by-moving-to-trash, he will have the bad surprise to find his
> TV> authinfo data in clear text in the trash.
>
> TV> Maybe let-bind delete-by-moving-to-trash in epg-delete-output-file would
> TV> be good.
> TV> It's what i will do here anyway.
>
> TV> (defun epg-delete-output-file (context)
> TV>   "Delete the output file of CONTEXT."
> TV>   (let ((delete-by-moving-to-trash nil))
> TV>     (when (and (epg-context-output-file context)
> TV>              (file-exists-p (epg-context-output-file context)))
> TV>       (delete-file (epg-context-output-file context)))))
>
> TV> With this change, data in clear text will never go to trash.
>
> According to the docs for `delete-file' it shouldn't matter what
> `delete-by-moving-to-trash' says because this is called
> non-interactively (TRASH is nil when delete-file is called
> non-interactively).  Or is this funcall interactive somehow?
I don't know, but actually without delete-by-moving-to-trash let-bound
to nil, non--encrypted data goes to trash.

To reproduce:
Enable delete-by-moving-to-trash
Edit your .authinfo.gpg from dired, then kill buffer.
Go to trash you will see content of your .authinfo in an epg-output-xxx
file.

Same if you open gnus.

If you use delete-by-moving-to-trash maybe you already have
non--encrypted data in your trash.

-- 
A+ Thierry
Get my Gnupg key:
gpg --keyserver pgp.mit.edu --recv-keys 59F29997 

Re: bug: epg send unencrypted data in trash

[Ted Zlatanov]

On Fri, 11 Feb 2011 22:05:09 +0100 Thierry Volpiatto <thierry.volpiatto@gmail.com> wrote: 

TV> Ted Zlatanov <tzz@lifelogs.com> writes:
>> According to the docs for `delete-file' it shouldn't matter what
>> `delete-by-moving-to-trash' says because this is called
>> non-interactively (TRASH is nil when delete-file is called
>> non-interactively).  Or is this funcall interactive somehow?
TV> I don't know, but actually without delete-by-moving-to-trash let-bound
TV> to nil, non--encrypted data goes to trash.

I think this is because in the C source (fileio.c) TRASH is set to
"(null current-prefix-arg)" which is not equivalent to
`called-interactively-p' AFAIK.  IOW I think your fix is masking the
underlying problem.  But I don't know the C source so well so I could be
misunderstanding things.  Perhaps Chong Yidong, who created revision
100462 and introduced the TRASH parameter, can check it.

Ted

Re: bug: epg send unencrypted data in trash

[Chong Yidong]

Ted Zlatanov <tzz@lifelogs.com> writes:

> On Fri, 11 Feb 2011 22:05:09 +0100 Thierry Volpiatto
> <thierry.volpiatto@gmail.com> wrote:
>
> TV> Ted Zlatanov <tzz@lifelogs.com> writes:
>>> According to the docs for `delete-file' it shouldn't matter what
>>> `delete-by-moving-to-trash' says because this is called
>>> non-interactively (TRASH is nil when delete-file is called
>>> non-interactively).  Or is this funcall interactive somehow?
> TV> I don't know, but actually without delete-by-moving-to-trash let-bound
> TV> to nil, non--encrypted data goes to trash.
>
> I think this is because in the C source (fileio.c) TRASH is set to
> "(null current-prefix-arg)" which is not equivalent to
> `called-interactively-p' AFAIK.  IOW I think your fix is masking the
> underlying problem.  But I don't know the C source so well so I could be
> misunderstanding things.  Perhaps Chong Yidong, who created revision
> 100462 and introduced the TRASH parameter, can check it.

I think TV is using the emacs-23 branch, where the TRASH parameter is
not present.  I've changed EPA and EPG to bind delete-by-moving-to-trash
to nil there.  No fix is necessary for the trunk, IIUC.

Re: bug: epg send unencrypted data in trash

[Thierry Volpiatto]

Chong Yidong <cyd@stupidchicken.com> writes:

> Ted Zlatanov <tzz@lifelogs.com> writes:
>
>> On Fri, 11 Feb 2011 22:05:09 +0100 Thierry Volpiatto
>> <thierry.volpiatto@gmail.com> wrote:
>>
>> TV> Ted Zlatanov <tzz@lifelogs.com> writes:
>>>> According to the docs for `delete-file' it shouldn't matter what
>>>> `delete-by-moving-to-trash' says because this is called
>>>> non-interactively (TRASH is nil when delete-file is called
>>>> non-interactively).  Or is this funcall interactive somehow?
>> TV> I don't know, but actually without delete-by-moving-to-trash let-bound
>> TV> to nil, non--encrypted data goes to trash.
>>
>> I think this is because in the C source (fileio.c) TRASH is set to
>> "(null current-prefix-arg)" which is not equivalent to
>> `called-interactively-p' AFAIK.  IOW I think your fix is masking the
>> underlying problem.  But I don't know the C source so well so I could be
>> misunderstanding things.  Perhaps Chong Yidong, who created revision
>> 100462 and introduced the TRASH parameter, can check it.
>
> I think TV is using the emacs-23 branch, 
Yes.

> where the TRASH parameter is
> not present.  I've changed EPA and EPG to bind delete-by-moving-to-trash
> to nil there.  No fix is necessary for the trunk, IIUC.
>
>

-- 
A+ Thierry
Get my Gnupg key:
gpg --keyserver pgp.mit.edu --recv-keys 59F29997 

Re: bug: epg send unencrypted data in trash

[Ted Zlatanov]

On Fri, 11 Feb 2011 19:23:06 -0500 Chong Yidong <cyd@stupidchicken.com> wrote: 

CY> I think TV is using the emacs-23 branch, where the TRASH parameter is
CY> not present.  I've changed EPA and EPG to bind delete-by-moving-to-trash
CY> to nil there.  No fix is necessary for the trunk, IIUC.

A good reason to use M-x report-emacs-bug which collects the version :)

I still don't understand the parameter definition and would appreciate
some guidance.  How does TRASH get set based on called-interactively-p,
as claimed by the docstring?

DEFUN ("delete-file", Fdelete_file, Sdelete_file, 1, 2,
       "(list (read-file-name \
                (if (and delete-by-moving-to-trash (null current-prefix-arg)) \
                    \"Move file to trash: \" \"Delete file: \") \
                nil default-directory (confirm-nonexistent-file-or-buffer)) \
              (null current-prefix-arg))",
       doc: /* Delete file named FILENAME.  If it is a symlink, remove the symlink.
If file has multiple names, it continues to exist with the other names.
TRASH non-nil means to trash the file instead of deleting, provided
`delete-by-moving-to-trash' is non-nil.

When called interactively, TRASH is t if no prefix argument is given.
With a prefix argument, TRASH is nil.  */)

I don't see anything in the body of the function to set TRASH, either.
Is it claimed because we assume `current-prefix-arg' is always null
non-interactively and `universal-arg' is always called interactively?

Thanks
Ted

Re: bug: epg send unencrypted data in trash

[Chong Yidong]

Ted Zlatanov <tzz@lifelogs.com> writes:

> How does TRASH get set based on called-interactively-p, as claimed by
> the docstring?
>
> DEFUN ("delete-file", Fdelete_file, Sdelete_file, 1, 2,
>        "(list (read-file-name \
>                 (if (and delete-by-moving-to-trash (null current-prefix-arg)) \
>                     \"Move file to trash: \" \"Delete file: \") \
>                 nil default-directory (confirm-nonexistent-file-or-buffer)) \
>               (null current-prefix-arg))",
>        doc: /* Delete file named FILENAME.  If it is a symlink, remove the symlink.
> If file has multiple names, it continues to exist with the other names.
> TRASH non-nil means to trash the file instead of deleting, provided
> `delete-by-moving-to-trash' is non-nil.
>
> When called interactively, TRASH is t if no prefix argument is given.
> With a prefix argument, TRASH is nil.  */)
>
> I don't see anything in the body of the function to set TRASH, either.
> Is it claimed because we assume `current-prefix-arg' is always null
> non-interactively and `universal-arg' is always called interactively?

Er, I'm not sure I understand what the confusion is about.  The above
interactive spec is run if and only if delete-file is called
interactively.  That's what interactive specs do---provide the arguments
for an interactive call.

Re: bug: epg send unencrypted data in trash

[Ted Zlatanov]

On Sat, 12 Feb 2011 13:49:40 -0500 Chong Yidong <cyd@stupidchicken.com> wrote: 

CY> The above interactive spec is run if and only if delete-file is
CY> called interactively.  That's what interactive specs do---provide
CY> the arguments for an interactive call.

OK, I understand now.  I somehow thought the interactive spec was used
in the non-interactive case.

Ted