unofficial mirror of bug-gnu-emacs@gnu.org 
 help / color / mirror / code / Atom feed
* bug#57856: 28.2; bookmark context strings in encrypted files
@ 2022-09-16 11:08 Gustavo Barros
  2022-09-16 12:07 ` Lars Ingebrigtsen
  0 siblings, 1 reply; 20+ messages in thread
From: Gustavo Barros @ 2022-09-16 11:08 UTC (permalink / raw)
  To: 57856

Hi All,

I guess this one is midway between a bug report and a feature request. 
I don't see that this is anyway against expected/documented behavior of 
involved libraries (bookmark.el, epg.el), but it is arguably a bad 
corner case of interaction between the two, which represents a (small) 
potential security issue.

Currently (Emacs 28.2), when setting a bookmark in a gpg encrypted file, 
part of the buffer is stored unencrypted as `front-context-string' and 
`rear-context-string' in the `bookmark-default-file' whenever 
`bookmark-search-size' is larger than 0, which by default is 16.

It could be argued that it is unwise to set a bookmark in this context. 
But, well, users do all kind of stuff.  Besides, Emacs provides no hint 
that this may be risky (as far as I can tell).  So it would be nice if 
Emacs would be a little more conservative here, and locally set 
`bookmark-search-size' to 0 in buffers visiting encrypted files.

I think it'd be overkill to provide a full reproduction recipe, since 
most of it would just be to set up environment (key etc.) for GPG.  But 
anyone who already has a setup and an encrypted file can reproduce the 
following simple steps (which I have tested in an .org.gpg file with 
`emacs -Q'):

Visit the encrypted file.  Set a bookmark with `bookmark-set' ("C-x r 
m") somewhere near a non-empty part of the buffer.  Save bookmarks with 
`bookmark-save'.  Inspect `bookmark-default-file' (by default 
"~/.emacs.d/bookmarks"), particularly `front-context-string' and 
`rear-context-string' of the pertinent bookmark, to find part of the 
original encrypted file stored there unencrypted.

Best regards,
Gustavo.


In GNU Emacs 28.2 (build 2, x86_64-pc-linux-gnu, GTK+ Version 3.24.20, 
cairo version 1.16.0)
 of 2022-09-12 built on gusbrs-laptop
Windowing system distributor 'The X.Org Foundation', version 
11.0.12013000
System Description: Linux Mint 20.3

Configured using:
 'configure --with-mailutils --with-xwidgets --with-native-compilation
 --without-compress-install'

Configured features:
ACL CAIRO DBUS FREETYPE GIF GLIB GMP GNUTLS GPM GSETTINGS HARFBUZZ JPEG
JSON LCMS2 LIBOTF LIBSELINUX LIBSYSTEMD LIBXML2 M17N_FLT MODULES
NATIVE_COMP NOTIFY INOTIFY PDUMPER PNG RSVG SECCOMP SOUND THREADS TIFF
TOOLKIT_SCROLL_BARS X11 XDBE XIM XPM XWIDGETS GTK3 ZLIB

Important settings:
  value of $LC_MONETARY: pt_BR.UTF-8
  value of $LC_NUMERIC: pt_BR.UTF-8
  value of $LANG: en_US.UTF-8
  locale-coding-system: utf-8-unix

Major mode: Lisp Interaction

Minor modes in effect:
  tooltip-mode: t
  global-eldoc-mode: t
  eldoc-mode: t
  show-paren-mode: t
  electric-indent-mode: t
  mouse-wheel-mode: t
  tool-bar-mode: t
  menu-bar-mode: t
  file-name-shadow-mode: t
  global-font-lock-mode: t
  font-lock-mode: t
  blink-cursor-mode: t
  auto-composition-mode: t
  auto-encryption-mode: t
  auto-compression-mode: t
  line-number-mode: t
  indent-tabs-mode: t
  transient-mark-mode: t

Load-path shadows:
None found.

Features:
(shadow sort mail-extr emacsbug sendmail bookmark pp vc-git diff-mode
vc-dispatcher org-element avl-tree generator ol-eww eww xdg url-queue
thingatpt mm-url ol-rmail ol-mhe ol-irc ol-info ol-gnus nnselect
gnus-search eieio-opt cl-extra help-mode speedbar ezimage dframe
gnus-art mm-uu mml2015 mm-view mml-smime smime dig gnus-sum shr kinsoku
svg dom browse-url url url-proxy url-privacy url-expand url-methods
url-history url-cookie url-domsuf url-util url-parse url-vars mailcap
gnus-group gnus-undo gnus-start gnus-dbus dbus xml gnus-cloud nnimap
nnmail mail-source utf7 netrc nnoo parse-time gnus-spec gnus-int
gnus-range message rmc puny rfc822 mml mml-sec mm-decode mm-bodies
mm-encode mail-parse rfc2231 mailabbrev gmm-utils mailheader gnus-win
gnus nnheader gnus-util rmail rmail-loaddefs auth-source cl-seq eieio
eieio-core cl-macs eieio-loaddefs password-cache rfc2047 rfc2045
ietf-drums text-property-search mail-utils mm-util mail-prsvr wid-edit
ol-docview doc-view jka-compr image-mode exif dired dired-loaddefs
ol-bibtex ol-bbdb ol-w3m ol-doi org-link-doi org ob ob-tangle ob-ref
ob-lob ob-table ob-exp org-macro org-footnote org-src ob-comint
org-pcomplete pcomplete comint ansi-color ring org-list org-faces
org-entities noutline outline easy-mmode org-version ob-emacs-lisp
ob-core ob-eval org-table oc-basic json map bibtex iso8601 time-date
subr-x ol rx org-keys oc org-compat advice org-macs org-loaddefs
format-spec find-func cal-menu calendar cal-loaddefs cl-loaddefs cl-lib
seq byte-opt gv bytecomp byte-compile cconv epa-file epa derived epg
rfc6068 epg-config iso-transl tooltip eldoc paren electric uniquify
ediff-hook vc-hooks lisp-float-type elisp-mode mwheel term/x-win x-win
term/common-win x-dnd tool-bar dnd fontset image regexp-opt fringe
tabulated-list replace newcomment text-mode lisp-mode prog-mode register
page tab-bar menu-bar rfn-eshadow isearch easymenu timer select
scroll-bar mouse jit-lock font-lock syntax font-core term/tty-colors
frame minibuffer cl-generic cham georgian utf-8-lang misc-lang
vietnamese tibetan thai tai-viet lao korean japanese eucjp-ms cp51932
hebrew greek romanian slovak czech european ethiopic indian cyrillic
chinese composite emoji-zwj charscript charprop case-table epa-hook
jka-cmpr-hook help simple abbrev obarray cl-preloaded nadvice button
loaddefs faces cus-face macroexp files window text-properties overlay
sha1 md5 base64 format env code-pages mule custom widget
hashtable-print-readable backquote threads xwidget-internal dbusbind
inotify lcms2 dynamic-setting system-font-setting font-render-setting
cairo move-toolbar gtk x-toolkit x multi-tty make-network-process
native-compile emacs)

Memory information:
((conses 16 238516 14883)
 (symbols 48 20306 0)
 (strings 32 72413 2731)
 (string-bytes 1 2383288)
 (vectors 16 36730)
 (vector-slots 8 659339 39456)
 (floats 8 313 89)
 (intervals 56 312 0)
 (buffers 992 11))





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

* bug#57856: 28.2; bookmark context strings in encrypted files
  2022-09-16 11:08 bug#57856: 28.2; bookmark context strings in encrypted files Gustavo Barros
@ 2022-09-16 12:07 ` Lars Ingebrigtsen
  2022-09-16 12:30   ` Gustavo Barros
  0 siblings, 1 reply; 20+ messages in thread
From: Lars Ingebrigtsen @ 2022-09-16 12:07 UTC (permalink / raw)
  To: Gustavo Barros; +Cc: 57856

Gustavo Barros <gusbrs.2016@gmail.com> writes:

> It could be argued that it is unwise to set a bookmark in this
> context. But, well, users do all kind of stuff.  Besides, Emacs
> provides no hint that this may be risky (as far as I can tell).  So it
> would be nice if Emacs would be a little more conservative here, and
> locally set `bookmark-search-size' to 0 in buffers visiting encrypted
> files.

I think that makes sense.  Alternatively, we could prompt the user for
what to do in these files?

Anybody have any opinions?





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

* bug#57856: 28.2; bookmark context strings in encrypted files
  2022-09-16 12:07 ` Lars Ingebrigtsen
@ 2022-09-16 12:30   ` Gustavo Barros
  2022-09-16 13:01     ` Lars Ingebrigtsen
  0 siblings, 1 reply; 20+ messages in thread
From: Gustavo Barros @ 2022-09-16 12:30 UTC (permalink / raw)
  To: Lars Ingebrigtsen; +Cc: 57856

Hi Lars,

Thanks again for the very prompt response.

On Fri, 16 Sep 2022 at 14:07, Lars Ingebrigtsen <larsi@gnus.org> wrote:

> Gustavo Barros <gusbrs.2016@gmail.com> writes:
>
>> It could be argued that it is unwise to set a bookmark in this
>> context. But, well, users do all kind of stuff.  Besides, Emacs
>> provides no hint that this may be risky (as far as I can tell).  So 
>> it
>> would be nice if Emacs would be a little more conservative here, and
>> locally set `bookmark-search-size' to 0 in buffers visiting encrypted
>> files.
>
> I think that makes sense.  Alternatively, we could prompt the user for
> what to do in these files?
>
> Anybody have any opinions?

If this were to be left to user discretion, I'd suggest a variable 
rather than a prompt.  What would we ask?  "This file is encrypted. Set 
bookmark, yes or no?"  "Why wouldn't I? Yes."  For the value of 
`bookmark-search-size'?  "What's that? Default. RET"  The general point 
is, how many users should we expect to know "at prompt time" that 
`bookmark.el' stores part of the buffer in the bookmark file and what it 
represents for an encrypted file?

But, to be honest, I'm not even sure if this being optional is 
particularly meaningful.  Do you see any use case where one would 
actually want to store unencrypted context of an encrypted file?

Best regards,
Gustavo.





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

* bug#57856: 28.2; bookmark context strings in encrypted files
  2022-09-16 12:30   ` Gustavo Barros
@ 2022-09-16 13:01     ` Lars Ingebrigtsen
  2022-09-16 13:18       ` Michael Albinus
  0 siblings, 1 reply; 20+ messages in thread
From: Lars Ingebrigtsen @ 2022-09-16 13:01 UTC (permalink / raw)
  To: Gustavo Barros; +Cc: 57856

Gustavo Barros <gusbrs.2016@gmail.com> writes:

> But, to be honest, I'm not even sure if this being optional is
> particularly meaningful.  Do you see any use case where one would
> actually want to store unencrypted context of an encrypted file?

I could imagine somebody just .gpg-ing most of their files on principle
without them really being "secret", and wanting the bookmark feature to
work properly (and for that you have to have the context).

But that's probably a very unusual setup, so perhaps just setting
bookmark-search-size to 0 in these buffers is the best (and least
annoying) way to go.  And if somebody wants to override this, they can
do it from some hook...





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

* bug#57856: 28.2; bookmark context strings in encrypted files
  2022-09-16 13:01     ` Lars Ingebrigtsen
@ 2022-09-16 13:18       ` Michael Albinus
  2022-09-18 10:19         ` Lars Ingebrigtsen
  0 siblings, 1 reply; 20+ messages in thread
From: Michael Albinus @ 2022-09-16 13:18 UTC (permalink / raw)
  To: Lars Ingebrigtsen; +Cc: Gustavo Barros, 57856

Lars Ingebrigtsen <larsi@gnus.org> writes:

>> But, to be honest, I'm not even sure if this being optional is
>> particularly meaningful.  Do you see any use case where one would
>> actually want to store unencrypted context of an encrypted file?
>
> I could imagine somebody just .gpg-ing most of their files on principle
> without them really being "secret", and wanting the bookmark feature to
> work properly (and for that you have to have the context).
>
> But that's probably a very unusual setup, so perhaps just setting
> bookmark-search-size to 0 in these buffers is the best (and least
> annoying) way to go.  And if somebody wants to override this, they can
> do it from some hook...

JFTR, there's not only gpg encryption. We have also tramp-crypt.el,
which let users encrypt all their files on a remote host. Those files
shall be handled similar, when it comes to implementation.

See (info "(tramp) Keeping files encrypted")

Best regards, Michael.





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

* bug#57856: 28.2; bookmark context strings in encrypted files
  2022-09-16 13:18       ` Michael Albinus
@ 2022-09-18 10:19         ` Lars Ingebrigtsen
  2022-09-18 10:43           ` Michael Albinus
  0 siblings, 1 reply; 20+ messages in thread
From: Lars Ingebrigtsen @ 2022-09-18 10:19 UTC (permalink / raw)
  To: Michael Albinus; +Cc: Gustavo Barros, 57856

Michael Albinus <michael.albinus@gmx.de> writes:

> JFTR, there's not only gpg encryption. We have also tramp-crypt.el,
> which let users encrypt all their files on a remote host. Those files
> shall be handled similar, when it comes to implementation.
>
> See (info "(tramp) Keeping files encrypted")

Right...  switching off bookmark context lines would go into
`epa-file-insert-file-contents' in the gpg case, I think -- where would
we do that in the Tramp case?





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

* bug#57856: 28.2; bookmark context strings in encrypted files
  2022-09-18 10:19         ` Lars Ingebrigtsen
@ 2022-09-18 10:43           ` Michael Albinus
  2022-09-19  7:42             ` Lars Ingebrigtsen
  0 siblings, 1 reply; 20+ messages in thread
From: Michael Albinus @ 2022-09-18 10:43 UTC (permalink / raw)
  To: Lars Ingebrigtsen; +Cc: Gustavo Barros, 57856

Lars Ingebrigtsen <larsi@gnus.org> writes:

Hi Lars,

>> JFTR, there's not only gpg encryption. We have also tramp-crypt.el,
>> which let users encrypt all their files on a remote host. Those files
>> shall be handled similar, when it comes to implementation.
>>
>> See (info "(tramp) Keeping files encrypted")
>
> Right...  switching off bookmark context lines would go into
> `epa-file-insert-file-contents' in the gpg case, I think -- where would
> we do that in the Tramp case?

There is `tramp-handle-insert-file-contents'. Since this is used for
several Tramp backends, we need a check, whether Tramp encryption is
active. Something like

--8<---------------cut here---------------start------------->8---
(when (tramp-crypt-file-name-p filename)
   ...)
--8<---------------cut here---------------end--------------->8---

OTOH, this would expose the "tramp-crypt-" namespace to tramp.el. Hmm,
maybe I will adapt the solution, but you could start with this.

Best regards, Michael.





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

* bug#57856: 28.2; bookmark context strings in encrypted files
  2022-09-18 10:43           ` Michael Albinus
@ 2022-09-19  7:42             ` Lars Ingebrigtsen
  2022-09-19  9:00               ` Michael Albinus
  2022-09-19 11:13               ` Gustavo Barros
  0 siblings, 2 replies; 20+ messages in thread
From: Lars Ingebrigtsen @ 2022-09-19  7:42 UTC (permalink / raw)
  To: Michael Albinus; +Cc: Gustavo Barros, 57856

Michael Albinus <michael.albinus@gmx.de> writes:

>> Right...  switching off bookmark context lines would go into
>> `epa-file-insert-file-contents' in the gpg case, I think -- where would
>> we do that in the Tramp case?
>
> There is `tramp-handle-insert-file-contents'. Since this is used for
> several Tramp backends, we need a check, whether Tramp encryption is
> active. Something like
>
> (when (tramp-crypt-file-name-p filename)
>    ...)
>
> OTOH, this would expose the "tramp-crypt-" namespace to tramp.el. Hmm,
> maybe I will adapt the solution, but you could start with this.

Looking at how to integrate this (which is always iffy when we're
dealing with several packages), I think this has to be done the other
way.  That is, `bookmark-make-record' has to check whether the file is
encrypted or not, becaues `*insert-file-contents' might not have been
run at all (if this is a new encrypted file, for instance).

So `tramp-crypt-file-name-p' is perfect for that, but there doesn't seem
to be any `epa-file-name-p' (or something equivalent)?  Hm...  OK, that
was trivial to add, so I've now done it this way in Emacs 29.







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

* bug#57856: 28.2; bookmark context strings in encrypted files
  2022-09-19  7:42             ` Lars Ingebrigtsen
@ 2022-09-19  9:00               ` Michael Albinus
  2022-09-19 12:03                 ` Lars Ingebrigtsen
  2022-09-19 11:13               ` Gustavo Barros
  1 sibling, 1 reply; 20+ messages in thread
From: Michael Albinus @ 2022-09-19  9:00 UTC (permalink / raw)
  To: Lars Ingebrigtsen; +Cc: Gustavo Barros, 57856

Lars Ingebrigtsen <larsi@gnus.org> writes:

Hi Lars,

> Looking at how to integrate this (which is always iffy when we're
> dealing with several packages), I think this has to be done the other
> way.  That is, `bookmark-make-record' has to check whether the file is
> encrypted or not, becaues `*insert-file-contents' might not have been
> run at all (if this is a new encrypted file, for instance).
>
> So `tramp-crypt-file-name-p' is perfect for that, but there doesn't seem
> to be any `epa-file-name-p' (or something equivalent)?  Hm...  OK, that
> was trivial to add, so I've now done it this way in Emacs 29.

Thanks.

Thinking further about, there are also other files which shouldn't
expose snippets to bookmarks. Think about .authinfo / .netrc (more
general, all files used as auth-sources backend). And perhaps other
files.

Wouldn't it be better, if packages like Tramp, epa, auth-sources, you
name it, could mark files to be excluded from bookmark-make-record? For
example, a hook a package could contribute to. In Tramp, we would run

(add-hook 'bookmark-inhibit-bookmark-hook #'tramp-crypt-file-name-p)

Similar for other packages.

Best regards, Michael.





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

* bug#57856: 28.2; bookmark context strings in encrypted files
  2022-09-19  7:42             ` Lars Ingebrigtsen
  2022-09-19  9:00               ` Michael Albinus
@ 2022-09-19 11:13               ` Gustavo Barros
  1 sibling, 0 replies; 20+ messages in thread
From: Gustavo Barros @ 2022-09-19 11:13 UTC (permalink / raw)
  To: Lars Ingebrigtsen; +Cc: Michael Albinus, 57856


On Mon, 19 Sep 2022 at 09:42, Lars Ingebrigtsen <larsi@gnus.org> wrote:

> Looking at how to integrate this (which is always iffy when we're
> dealing with several packages), I think this has to be done the other
> way.  That is, `bookmark-make-record' has to check whether the file is
> encrypted or not, becaues `*insert-file-contents' might not have been
> run at all (if this is a new encrypted file, for instance).
>
> So `tramp-crypt-file-name-p' is perfect for that, but there doesn't 
> seem
> to be any `epa-file-name-p' (or something equivalent)?  Hm...  OK, 
> that
> was trivial to add, so I've now done it this way in Emacs 29.

Thank you very much.

Best regards,
Gustavo.





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

* bug#57856: 28.2; bookmark context strings in encrypted files
  2022-09-19  9:00               ` Michael Albinus
@ 2022-09-19 12:03                 ` Lars Ingebrigtsen
  2022-09-19 12:16                   ` Michael Albinus
  0 siblings, 1 reply; 20+ messages in thread
From: Lars Ingebrigtsen @ 2022-09-19 12:03 UTC (permalink / raw)
  To: Michael Albinus; +Cc: Gustavo Barros, 57856

Michael Albinus <michael.albinus@gmx.de> writes:

> Thinking further about, there are also other files which shouldn't
> expose snippets to bookmarks. Think about .authinfo / .netrc (more
> general, all files used as auth-sources backend). And perhaps other
> files.
>
> Wouldn't it be better, if packages like Tramp, epa, auth-sources, you
> name it, could mark files to be excluded from bookmark-make-record? For
> example, a hook a package could contribute to. In Tramp, we would run
>
> (add-hook 'bookmark-inhibit-bookmark-hook #'tramp-crypt-file-name-p)

Yes, that's true, but we don't really have a way to define
auth-source-file-p except by heuristics -- a netrc file may be called
anything.

So I'm not sure how that'd look...





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

* bug#57856: 28.2; bookmark context strings in encrypted files
  2022-09-19 12:03                 ` Lars Ingebrigtsen
@ 2022-09-19 12:16                   ` Michael Albinus
  2022-09-19 12:34                     ` Lars Ingebrigtsen
  0 siblings, 1 reply; 20+ messages in thread
From: Michael Albinus @ 2022-09-19 12:16 UTC (permalink / raw)
  To: Lars Ingebrigtsen; +Cc: Gustavo Barros, 57856

Lars Ingebrigtsen <larsi@gnus.org> writes:

Hi Lars,

> Michael Albinus <michael.albinus@gmx.de> writes:
>
>> Thinking further about, there are also other files which shouldn't
>> expose snippets to bookmarks. Think about .authinfo / .netrc (more
>> general, all files used as auth-sources backend). And perhaps other
>> files.
>>
>> Wouldn't it be better, if packages like Tramp, epa, auth-sources, you
>> name it, could mark files to be excluded from bookmark-make-record? For
>> example, a hook a package could contribute to. In Tramp, we would run
>>
>> (add-hook 'bookmark-inhibit-bookmark-hook #'tramp-crypt-file-name-p)
>
> Yes, that's true, but we don't really have a way to define
> auth-source-file-p except by heuristics -- a netrc file may be called
> anything.
>
> So I'm not sure how that'd look...

auth-source-file-name-p could scan the auth-sources declarations for a
file based backend, and return t for such files. Shouldn't be too hard
to implement (oops, do I offer to volunteer?)

Let's start with bookmark-inhibit-bookmark-hook, adding
tramp-crypt-file-name-p and epa-file-name-p, and see, who else wants to
be added.

Best regards, Michael.





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

* bug#57856: 28.2; bookmark context strings in encrypted files
  2022-09-19 12:16                   ` Michael Albinus
@ 2022-09-19 12:34                     ` Lars Ingebrigtsen
  2022-09-19 13:03                       ` Michael Albinus
  0 siblings, 1 reply; 20+ messages in thread
From: Lars Ingebrigtsen @ 2022-09-19 12:34 UTC (permalink / raw)
  To: Michael Albinus; +Cc: Gustavo Barros, 57856

Michael Albinus <michael.albinus@gmx.de> writes:

> auth-source-file-name-p could scan the auth-sources declarations for a
> file based backend, and return t for such files. Shouldn't be too hard
> to implement (oops, do I offer to volunteer?)

Yes, those were the heuristics I were alluding to.  😀  But I was
thinking about packages that say

(let ((auth-sources ...))
  (auth-source-search ...))

and we'd have no idea how to identify those files.






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

* bug#57856: 28.2; bookmark context strings in encrypted files
  2022-09-19 12:34                     ` Lars Ingebrigtsen
@ 2022-09-19 13:03                       ` Michael Albinus
  2022-09-19 18:44                         ` Lars Ingebrigtsen
  0 siblings, 1 reply; 20+ messages in thread
From: Michael Albinus @ 2022-09-19 13:03 UTC (permalink / raw)
  To: Lars Ingebrigtsen; +Cc: Gustavo Barros, 57856

Lars Ingebrigtsen <larsi@gnus.org> writes:

Hi Lars,

> (let ((auth-sources ...))
>   (auth-source-search ...))
>
> and we'd have no idea how to identify those files.

Yes. Packages which hide to-be-protected files in a let-form cannot
expect that we identify such files, except inside that let-form. This
isn't worse than today.

But we will protect users who do in their init file

(setq auth-sources '("~/.authinfo" "~/my-own-netrc-file-name"))

Maybe, additionally or instead of, all files with enabled reveal-mode in
the visiting buffer shall be protected as well. Another check function.

(defun reveal-mode-p (filename)
  (when-let ((buffer (get-file-buffer filename)))
    (buffer-local-value reveal-mode buffer)))

Best regards, Michael.





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

* bug#57856: 28.2; bookmark context strings in encrypted files
  2022-09-19 13:03                       ` Michael Albinus
@ 2022-09-19 18:44                         ` Lars Ingebrigtsen
  2022-09-20 14:49                           ` Michael Albinus
  0 siblings, 1 reply; 20+ messages in thread
From: Lars Ingebrigtsen @ 2022-09-19 18:44 UTC (permalink / raw)
  To: Michael Albinus; +Cc: Gustavo Barros, 57856

Michael Albinus <michael.albinus@gmx.de> writes:

> But we will protect users who do in their init file
>
> (setq auth-sources '("~/.authinfo" "~/my-own-netrc-file-name"))

Yes, that's a good point.

> Maybe, additionally or instead of, all files with enabled reveal-mode in
> the visiting buffer shall be protected as well. Another check function.
>
> (defun reveal-mode-p (filename)
>   (when-let ((buffer (get-file-buffer filename)))
>     (buffer-local-value reveal-mode buffer)))

Hm...  I'm not sure about that.  I can see people wanting to use that
mode in buffers that have no secrets -- just because they don't want to
display that stuff.  And the context strings do add real value in
bookmarks when finding the correct location.





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

* bug#57856: 28.2; bookmark context strings in encrypted files
  2022-09-19 18:44                         ` Lars Ingebrigtsen
@ 2022-09-20 14:49                           ` Michael Albinus
  2022-09-20 14:53                             ` Lars Ingebrigtsen
  2022-09-20 15:03                             ` Gustavo Barros
  0 siblings, 2 replies; 20+ messages in thread
From: Michael Albinus @ 2022-09-20 14:49 UTC (permalink / raw)
  To: Lars Ingebrigtsen; +Cc: Gustavo Barros, 57856

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

Lars Ingebrigtsen <larsi@gnus.org> writes:

Hi Lars,

> Michael Albinus <michael.albinus@gmx.de> writes:
>
>> But we will protect users who do in their init file
>>
>> (setq auth-sources '("~/.authinfo" "~/my-own-netrc-file-name"))
>
> Yes, that's a good point.

I gave it a try. WDYT?
(Documentation must be added, of course)

Best regards, Michael.


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

diff --git a/lisp/auth-source-pass.el b/lisp/auth-source-pass.el
index 86e0b48a79..4519705e14 100644
--- a/lisp/auth-source-pass.el
+++ b/lisp/auth-source-pass.el
@@ -319,6 +319,16 @@ auth-source-pass--name-port-user-suffixes
     (list
      (format "%s" name)))))

+(defun auth-source-pass-file-name-p (file)
+  "Say whether FILE is used by `auth-source-pass'."
+  (and (stringp file) (stringp auth-source-pass-filename)
+       (string-equal
+        (expand-file-name file) (expand-file-name auth-source-pass-filename))))
+
+(with-eval-after-load 'bookmark
+  (add-hook 'bookmark-inhibit-bookmark-functions
+	    #'auth-source-pass-file-name-p))
+
 (provide 'auth-source-pass)
 ;;; auth-source-pass.el ends here

diff --git a/lisp/auth-source.el b/lisp/auth-source.el
index c79e5b81f7..13ebd80106 100644
--- a/lisp/auth-source.el
+++ b/lisp/auth-source.el
@@ -522,6 +522,21 @@ auth-source-backend-parse-parameters

 ;; (mapcar #'auth-source-backend-parse auth-sources)

+(defun auth-source-file-name-p (file)
+  "Say whether FILE is used by `auth-sources'."
+  (let* ((backends (mapcar #'auth-source-backend-parse auth-sources))
+         (files
+          (mapcar (lambda (x)
+                    (when (member (slot-value x 'type) '(json netrc plstore))
+                      (slot-value x 'source)))
+                  backends)))
+    (member (expand-file-name file)
+            (mapcar #'expand-file-name (remq nil files)))))
+
+(with-eval-after-load 'bookmark
+  (add-hook 'bookmark-inhibit-bookmark-functions
+	    #'auth-source-file-name-p))
+
 (cl-defun auth-source-search (&rest spec
                               &key max require create delete
                               &allow-other-keys)
diff --git a/lisp/bookmark.el b/lisp/bookmark.el
index f150a24bbf..941cf7932c 100644
--- a/lisp/bookmark.el
+++ b/lisp/bookmark.el
@@ -592,6 +592,14 @@ bookmark-make-record-function
 in which case a default heuristic will be used.  The function can also
 equivalently just return ALIST without NAME.")

+(defcustom bookmark-inhibit-bookmark-functions nil
+  "List of functions to call before make a bookmark record.
+The functions take `buffer-file-name' as argument.  If any of
+these functions returns non-nil, the current buffer is not used
+for a bookmark."
+  :type 'hook
+  :version "29.1")
+
 (defun bookmark-make-record ()
   "Return a new bookmark record (NAME . ALIST) for the current location."
   (let* ((bookmark-search-size
@@ -599,10 +607,8 @@ bookmark-make-record
           ;; don't include any context in the bookmark file, because
           ;; that would leak (possibly secret) data.
           (if (and buffer-file-name
-                   (or (and (fboundp 'epa-file-name-p)
-                            (epa-file-name-p buffer-file-name))
-                       (and (fboundp 'tramp-crypt-file-name-p)
-                            (tramp-crypt-file-name-p buffer-file-name))))
+                   (not (run-hook-with-args-until-success
+                         'bookmark-inhibit-bookmark-functions buffer-file-name)))
               0
             bookmark-search-size))
          (record (funcall bookmark-make-record-function)))
diff --git a/lisp/epa-hook.el b/lisp/epa-hook.el
index 70c3030881..f4616b3b91 100644
--- a/lisp/epa-hook.el
+++ b/lisp/epa-hook.el
@@ -92,6 +92,10 @@ epa-file-name-p
   "Say whether FILE is handled by `epa-file'."
   (and auto-encryption-mode (string-match-p epa-file-name-regexp file)))

+(with-eval-after-load 'bookmark
+  (add-hook 'bookmark-inhibit-bookmark-functions
+	    #'epa-file-name-p))
+
 (define-minor-mode auto-encryption-mode
   "Toggle automatic file encryption/decryption (Auto Encryption mode)."
   :global t :init-value t :group 'epa-file :version "23.1"
diff --git a/lisp/net/tramp-crypt.el b/lisp/net/tramp-crypt.el
index d556c87606..60a1c37c9c 100644
--- a/lisp/net/tramp-crypt.el
+++ b/lisp/net/tramp-crypt.el
@@ -852,6 +852,14 @@ tramp-crypt-handle-unlock-file
     (tramp-compat-funcall
      'unlock-file (tramp-crypt-encrypt-file-name filename))))

+(with-eval-after-load 'bookmark
+  (add-hook 'bookmark-inhibit-bookmark-functions
+	    #'tramp-crypt-file-name-p)
+  (add-hook 'tramp-crypt-unload-hook
+	    (lambda ()
+	      (remove-hook 'bookmark-inhibit-bookmark-functions
+			   #'tramp-crypt-file-name-p))))
+
 (add-hook 'tramp-unload-hook
 	  (lambda ()
 	    (unload-feature 'tramp-crypt 'force)))

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

* bug#57856: 28.2; bookmark context strings in encrypted files
  2022-09-20 14:49                           ` Michael Albinus
@ 2022-09-20 14:53                             ` Lars Ingebrigtsen
  2022-09-20 15:00                               ` Michael Albinus
  2022-09-20 15:03                             ` Gustavo Barros
  1 sibling, 1 reply; 20+ messages in thread
From: Lars Ingebrigtsen @ 2022-09-20 14:53 UTC (permalink / raw)
  To: Michael Albinus; +Cc: Gustavo Barros, 57856

Michael Albinus <michael.albinus@gmx.de> writes:

> I gave it a try. WDYT?
> (Documentation must be added, of course)

Looks good to me.






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

* bug#57856: 28.2; bookmark context strings in encrypted files
  2022-09-20 14:53                             ` Lars Ingebrigtsen
@ 2022-09-20 15:00                               ` Michael Albinus
  0 siblings, 0 replies; 20+ messages in thread
From: Michael Albinus @ 2022-09-20 15:00 UTC (permalink / raw)
  To: Lars Ingebrigtsen; +Cc: Gustavo Barros, 57856

Lars Ingebrigtsen <larsi@gnus.org> writes:

Hi Lars,

>> I gave it a try. WDYT?
>> (Documentation must be added, of course)
>
> Looks good to me.

Thanks. I'll wait a day or two, whether somebody hollers. If not, I'll push.

Best regards, Michael.





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

* bug#57856: 28.2; bookmark context strings in encrypted files
  2022-09-20 14:49                           ` Michael Albinus
  2022-09-20 14:53                             ` Lars Ingebrigtsen
@ 2022-09-20 15:03                             ` Gustavo Barros
  2022-09-20 16:19                               ` Michael Albinus
  1 sibling, 1 reply; 20+ messages in thread
From: Gustavo Barros @ 2022-09-20 15:03 UTC (permalink / raw)
  To: Michael Albinus; +Cc: Lars Ingebrigtsen, 57856

Hi Michael,

On Tue, 20 Sep 2022 at 16:49, Michael Albinus <michael.albinus@gmx.de> 
wrote:

> I gave it a try. WDYT?
> (Documentation must be added, of course)

If I may, I do have a comment about the defcustom.  I think 
`bookmark-inhibit-bookmark-functions` is not really a precise name, 
since it is not about inhibiting the bookmark itself, but only the 
context strings.  How about `bookmark-inhibit-context-functions`?  On 
the same vein, the docstring could be clearer about it.  Say:

"List of functions to call before making a bookmark record.
The functions take `buffer-file-name' as argument.  If any of
these functions returns non-nil, the bookmark does not record
context strings from the current buffer."

WDYT?

(Either way, thank you!)

Best regards,
Gustavo.





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

* bug#57856: 28.2; bookmark context strings in encrypted files
  2022-09-20 15:03                             ` Gustavo Barros
@ 2022-09-20 16:19                               ` Michael Albinus
  0 siblings, 0 replies; 20+ messages in thread
From: Michael Albinus @ 2022-09-20 16:19 UTC (permalink / raw)
  To: Gustavo Barros; +Cc: Lars Ingebrigtsen, 57856

Gustavo Barros <gusbrs.2016@gmail.com> writes:

> Hi Michael,

Hi Gustavo,

> If I may, I do have a comment about the defcustom.  I think
> `bookmark-inhibit-bookmark-functions` is not really a precise name,
> since it is not about inhibiting the bookmark itself, but only the
> context strings.  How about `bookmark-inhibit-context-functions`?  On
> the same vein, the docstring could be clearer about it.  Say:
>
> "List of functions to call before making a bookmark record.
> The functions take `buffer-file-name' as argument.  If any of
> these functions returns non-nil, the bookmark does not record
> context strings from the current buffer."
>
> WDYT?

Thanks, will apply both. I'm notoriously bad in giving names and writing
documentation, so everything will help me!

> Best regards,
> Gustavo.

Best regards, Michael.





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

end of thread, other threads:[~2022-09-20 16:19 UTC | newest]

Thread overview: 20+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-09-16 11:08 bug#57856: 28.2; bookmark context strings in encrypted files Gustavo Barros
2022-09-16 12:07 ` Lars Ingebrigtsen
2022-09-16 12:30   ` Gustavo Barros
2022-09-16 13:01     ` Lars Ingebrigtsen
2022-09-16 13:18       ` Michael Albinus
2022-09-18 10:19         ` Lars Ingebrigtsen
2022-09-18 10:43           ` Michael Albinus
2022-09-19  7:42             ` Lars Ingebrigtsen
2022-09-19  9:00               ` Michael Albinus
2022-09-19 12:03                 ` Lars Ingebrigtsen
2022-09-19 12:16                   ` Michael Albinus
2022-09-19 12:34                     ` Lars Ingebrigtsen
2022-09-19 13:03                       ` Michael Albinus
2022-09-19 18:44                         ` Lars Ingebrigtsen
2022-09-20 14:49                           ` Michael Albinus
2022-09-20 14:53                             ` Lars Ingebrigtsen
2022-09-20 15:00                               ` Michael Albinus
2022-09-20 15:03                             ` Gustavo Barros
2022-09-20 16:19                               ` Michael Albinus
2022-09-19 11:13               ` Gustavo Barros

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).