bug#67323: 30.0.50; [PATCH] Set a new desktop file to mode 0600

bug#67323: 30.0.50; [PATCH] Set a new desktop file to mode 0600

[Manuel Giraud via Bug reports for GNU Emacs, the Swiss army knife of text editors]

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


Hi,

As a desktop file can contain some "secret" data, I think it is better
to make it read/write only to the user by default.  This does not
prevent the user to later change the mode of this desktop file if he
wants to "share" it.

Best regards,

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Set-a-new-desktop-file-to-mode-0600.patch --]
[-- Type: text/x-patch, Size: 1040 bytes --]

From 50605fe88c7c777592a4a785c92004d757809428 Mon Sep 17 00:00:00 2001
From: Manuel Giraud <manuel@ledu-giraud.fr>
Date: Tue, 21 Nov 2023 11:15:45 +0100
Subject: [PATCH] Set a new desktop file to mode 0600

* lisp/desktop.el (desktop-save): Set a new desktop file to mode 0600
by default.
---
 lisp/desktop.el | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/lisp/desktop.el b/lisp/desktop.el
index f096f13ab80..dc6b48f8844 100644
--- a/lisp/desktop.el
+++ b/lisp/desktop.el
@@ -1158,6 +1158,13 @@ desktop-save
 	    (desktop-release-lock)
 	  (unless (and new-modtime (desktop-owner)) (desktop-claim-lock)))
 
+        ;; If the desktop file does not exist, create one only
+        ;; read/writable by user.
+        (let ((full-name (desktop-full-file-name)))
+          (unless (file-exists-p full-name)
+            (make-empty-file full-name)
+            (set-file-modes full-name #o600)))
+
         ;; What format are we going to write the file in?
         (setq desktop-io-file-version
               (cond
-- 
2.42.1


[-- Attachment #3: Type: text/plain, Size: 7268 bytes --]




In GNU Emacs 30.0.50 (build 11, x86_64-unknown-openbsd7.4) of 2023-11-21
 built on computer
Repository revision: 04200f58f09f05f668ce7354851d488de11ccff6
Repository branch: mgi/gnus-modeline
Windowing system distributor 'The X.Org Foundation', version 11.0.12101009
System Description: OpenBSD computer 7.4 GENERIC.MP#1453 amd64

Configured using:
 'configure CC=egcc MAKEINFO=gmakeinfo --prefix=/home/manuel/emacs
 --exec-prefix=/home/manuel --with-x-toolkit=no --without-cairo
 --without-dbus --without-gconf --without-gsettings --without-sound
 --without-compress-install'

Configured features:
FREETYPE GIF GLIB GNUTLS HARFBUZZ JPEG JSON LCMS2 LIBOTF LIBXML2 MODULES
NOTIFY KQUEUE OLDXMENU PDUMPER PNG RSVG SQLITE3 THREADS TIFF TREE_SITTER
WEBP X11 XDBE XFT XIM XINPUT2 XPM ZLIB

Important settings:
  value of $LC_CTYPE: en_US.UTF-8
  locale-coding-system: utf-8-unix

Major mode: Group

Minor modes in effect:
  gnus-topic-mode: t
  display-time-mode: t
  display-battery-mode: t
  server-mode: t
  gnus-undo-mode: t
  override-global-mode: t
  repeat-mode: t
  desktop-save-mode: t
  global-eldoc-mode: t
  show-paren-mode: t
  electric-indent-mode: t
  mouse-wheel-mode: t
  menu-bar-mode: t
  file-name-shadow-mode: t
  global-font-lock-mode: t
  font-lock-mode: t
  blink-cursor-mode: t
  minibuffer-regexp-mode: t
  buffer-read-only: t
  line-number-mode: t
  indent-tabs-mode: t
  transient-mark-mode: t
  auto-composition-mode: t
  auto-encryption-mode: t
  auto-compression-mode: t

Load-path shadows:
/home/manuel/.emacs.d/elpa/ef-themes-1.4.0/theme-loaddefs hides /home/manuel/emacs/share/emacs/30.0.50/lisp/theme-loaddefs

Features:
(qp gnus-async gnus-bcklg gnus-ml magit-utils dash shortdoc comp-common
dabbrev cl-print help-fns radix-tree misearch multi-isearch
display-line-numbers proced smtpmail textsec uni-scripts idna-mapping
ucs-normalize uni-confusable textsec-check mailalias shadow sort
gnus-cite mail-extr emacsbug pulse log-edit add-log smerge-mode diff
whitespace vc-bzr vc-src vc-sccs vc-svn gnus-topic mm-archive url-cache
utf-7 imap rfc2104 nndoc nndraft nnmh network-stream nnfolder nnml
gnus-agent gnus-srvr gnus-score score-mode nnvirtual nntp gnus-cache
nnrss org-agenda mule-util on-screen pascal conf-mode vc-hg org-indent
oc-basic org-element org-persist org-id avl-tree ol-eww ol-rmail ol-mhe
ol-irc ol-info ol-gnus nnselect ol-docview doc-view jka-compr image-mode
exif ol-bibtex bibtex ol-bbdb ol-w3m ol-doi org-link-doi gnus-icalendar
org-capture org-refile org ob ob-tangle ob-ref ob-lob ob-table ob-exp
org-macro org-src ob-comint org-pcomplete org-list org-footnote
org-faces org-entities ob-emacs-lisp ob-core ob-eval org-cycle org-table
ol org-fold org-fold-core org-keys oc org-loaddefs org-version
org-compat org-macs make-mode css-mode sgml-mode facemenu imenu eww
url-queue mm-url view sh-script smie treesit executable vc-cvs vc-rcs
log-view pcvs-util vc-dir ewoc vc autorevert filenotify vc-git diff-mode
vc-dispatcher bug-reference paredit gnus-dired time battery cus-load
exwm-randr xcb-randr exwm-config ido exwm exwm-input xcb-keysyms xcb-xkb
exwm-manage exwm-floating xcb-cursor xcb-render exwm-layout
exwm-workspace exwm-core xcb-ewmh xcb-icccm xcb xcb-xproto xcb-types
xcb-debug server modus-operandi-theme modus-themes zone speed-type
url-http url-auth url-gw nsm compat ytdious mingus libmpdee reporter
edebug debug backtrace transmission color calc-bin calc-ext calc
calc-loaddefs rect calc-macs supercite regi ebdb-message ebdb-gnus
gnus-msg gnus-art mm-uu mml2015 mm-view mml-smime smime gnutls dig
gnus-sum shr pixel-fill kinsoku url-file svg dom gnus-group gnus-undo
gnus-start gnus-dbus gnus-cloud nnimap nnmail mail-source utf7 nnoo
gnus-spec gnus-int gnus-range message sendmail yank-media puny rfc822
mml mml-sec epa epg rfc6068 epg-config mm-decode mm-bodies mm-encode
mail-parse rfc2231 rfc2047 rfc2045 ietf-drums gmm-utils mailheader
gnus-win ebdb-mua ebdb-com crm ebdb-format ebdb mailabbrev eieio-opt
speedbar ezimage dframe find-func eieio-base timezone icalendar gnus
nnheader gnus-util mail-utils range mm-util mail-prsvr wid-edit web-mode
derived disp-table erlang-start skeleton cc-mode cc-fonts cc-guess
cc-menus cc-cmds cc-styles cc-align cc-engine cc-vars cc-defs slime-asdf
grep slime-tramp tramp rx trampver tramp-integration files-x
tramp-message tramp-compat xdg shell pcomplete parse-time iso8601
time-date format-spec tramp-loaddefs slime-fancy slime-indentation
slime-cl-indent cl-indent slime-trace-dialog slime-fontifying-fu
slime-package-fu slime-references slime-compiler-notes-tree advice
slime-scratch slime-presentations bridge slime-macrostep macrostep
slime-mdot-fu slime-enclosing-context slime-fuzzy slime-fancy-trace
slime-fancy-inspector slime-c-p-c slime-editing-commands slime-autodoc
slime-repl slime-parse slime apropos compile text-property-search etags
fileloop generator xref project arc-mode archive-mode noutline outline
icons pp comint ansi-osc ansi-color ring hyperspec thingatpt
slime-autoloads edmacro kmacro use-package-bind-key bind-key appt
diary-lib diary-loaddefs cal-menu calendar cal-loaddefs pcase dired-x
dired-aux dired dired-loaddefs notifications dbus xml cl-extra help-mode
use-package-core repeat easy-mmode desktop frameset debbugs-autoloads
ebdb-autoloads ef-themes-autoloads exwm-autoloads hyperbole-autoloads
magit-autoloads git-commit-autoloads finder-inf magit-section-autoloads
dash-autoloads on-screen-autoloads osm-autoloads paredit-autoloads
rust-mode-autoloads speed-type-autoloads transmission-autoloads
with-editor-autoloads info compat-autoloads ytdious-autoloads package
browse-url url url-proxy url-privacy url-expand url-methods url-history
url-cookie generate-lisp-file url-domsuf url-util mailcap url-handlers
url-parse auth-source cl-seq eieio eieio-core cl-macs password-cache
json subr-x map byte-opt gv bytecomp byte-compile url-vars cl-loaddefs
cl-lib rmc iso-transl tooltip cconv eldoc paren electric uniquify
ediff-hook vc-hooks lisp-float-type elisp-mode mwheel term/x-win x-win
term/common-win x-dnd touch-screen 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 nadvice seq simple cl-generic indonesian philippine
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 abbrev obarray oclosure
cl-preloaded button loaddefs theme-loaddefs faces cus-face macroexp
files window text-properties overlay sha1 md5 base64 format env
code-pages mule custom widget keymap hashtable-print-readable backquote
threads kqueue lcms2 dynamic-setting font-render-setting xinput2 x
multi-tty move-toolbar make-network-process emacs)

Memory information:
((conses 16 1120120 482852) (symbols 48 57677 39)
 (strings 32 296390 30784) (string-bytes 1 9719758)
 (vectors 16 181383) (vector-slots 8 3111778 142788)
 (floats 8 690 1106) (intervals 56 52158 2695) (buffers 992 125))

-- 
Manuel Giraud

bug#67323: 30.0.50; [PATCH] Set a new desktop file to mode 0600

[Eli Zaretskii]

> Date: Tue, 21 Nov 2023 11:23:56 +0100
> From:  Manuel Giraud via "Bug reports for GNU Emacs,
>  the Swiss army knife of text editors" <bug-gnu-emacs@gnu.org>
> 
> As a desktop file can contain some "secret" data, I think it is better
> to make it read/write only to the user by default.  This does not
> prevent the user to later change the mode of this desktop file if he
> wants to "share" it.

We don't do this in other cases, AFAICT, so why do it here?

The users can make this file unreadable by others if they want.

It's a backward-incompatible change in any case.

bug#67323: 30.0.50; [PATCH] Set a new desktop file to mode 0600

[Manuel Giraud via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Eli Zaretskii <eliz@gnu.org> writes:

>> Date: Tue, 21 Nov 2023 11:23:56 +0100
>> From:  Manuel Giraud via "Bug reports for GNU Emacs,
>>  the Swiss army knife of text editors" <bug-gnu-emacs@gnu.org>
>> 
>> As a desktop file can contain some "secret" data, I think it is better
>> to make it read/write only to the user by default.  This does not
>> prevent the user to later change the mode of this desktop file if he
>> wants to "share" it.
>
> We don't do this in other cases, AFAICT, so why do it here?

Hi Eli,

I had this idea while browsing savehist.el.  It have
'savehist-file-modes' set to #o600 by default.  Since desktop.el could
also contain histories or others "secrets", I thought that it may a good
idea to have more strict default.

> The users can make this file unreadable by others if they want.

Yes and it is what I have done previously for my own desktop file.  The
idea here is to have saner default.  And as I said, it also works the
other way around ;-)

> It's a backward-incompatible change in any case.

You are saying that it might surprise users who rely on the "readable
for all" nature of one desktop file by default?  I'd have a hard time to
figure out such a scenario…  But anyway, if you think this patch does
not worth it, say it and I'll close this report.

Thanks,
-- 
Manuel Giraud

bug#67323: 30.0.50; [PATCH] Set a new desktop file to mode 0600

[Eli Zaretskii]

> From: Manuel Giraud <manuel@ledu-giraud.fr>
> Cc: 67323@debbugs.gnu.org
> Date: Tue, 21 Nov 2023 14:00:28 +0100
> 
> I had this idea while browsing savehist.el.  It have
> 'savehist-file-modes' set to #o600 by default.  Since desktop.el could
> also contain histories or others "secrets", I thought that it may a good
> idea to have more strict default.
> 
> > The users can make this file unreadable by others if they want.
> 
> Yes and it is what I have done previously for my own desktop file.  The
> idea here is to have saner default.  And as I said, it also works the
> other way around ;-)
> 
> > It's a backward-incompatible change in any case.
> 
> You are saying that it might surprise users who rely on the "readable
> for all" nature of one desktop file by default?  I'd have a hard time to
> figure out such a scenario…  But anyway, if you think this patch does
> not worth it, say it and I'll close this report.

I'll wait a bit for others to chime in, if anyone has an opinion.

Thanks.

bug#67323: 30.0.50; [PATCH] Set a new desktop file to mode 0600

[Stefan Kangas]

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Manuel Giraud <manuel@ledu-giraud.fr>
>> Cc: 67323@debbugs.gnu.org
>> Date: Tue, 21 Nov 2023 14:00:28 +0100
>>
>> I had this idea while browsing savehist.el.  It have
>> 'savehist-file-modes' set to #o600 by default.  Since desktop.el could
>> also contain histories or others "secrets", I thought that it may a good
>> idea to have more strict default.
>>
>> > The users can make this file unreadable by others if they want.
>>
>> Yes and it is what I have done previously for my own desktop file.  The
>> idea here is to have saner default.  And as I said, it also works the
>> other way around ;-)
>>
>> > It's a backward-incompatible change in any case.
>>
>> You are saying that it might surprise users who rely on the "readable
>> for all" nature of one desktop file by default?  I'd have a hard time to
>> figure out such a scenario…  But anyway, if you think this patch does
>> not worth it, say it and I'll close this report.
>
> I'll wait a bit for others to chime in, if anyone has an opinion.

I think the patch makes sense.

Having defaults that protect users security and privacy better, even if
only slightly, is not a bad thing, not unless there are cases where it
hurts.  And I can't think of any such cases here.

bug#67323: 30.0.50; [PATCH] Set a new desktop file to mode 0600

[Eli Zaretskii]

> From: Stefan Kangas <stefankangas@gmail.com>
> Date: Thu, 14 Dec 2023 17:17:36 -0800
> Cc: 67323@debbugs.gnu.org
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> >> From: Manuel Giraud <manuel@ledu-giraud.fr>
> >> Cc: 67323@debbugs.gnu.org
> >> Date: Tue, 21 Nov 2023 14:00:28 +0100
> >>
> >> I had this idea while browsing savehist.el.  It have
> >> 'savehist-file-modes' set to #o600 by default.  Since desktop.el could
> >> also contain histories or others "secrets", I thought that it may a good
> >> idea to have more strict default.
> >>
> >> > The users can make this file unreadable by others if they want.
> >>
> >> Yes and it is what I have done previously for my own desktop file.  The
> >> idea here is to have saner default.  And as I said, it also works the
> >> other way around ;-)
> >>
> >> > It's a backward-incompatible change in any case.
> >>
> >> You are saying that it might surprise users who rely on the "readable
> >> for all" nature of one desktop file by default?  I'd have a hard time to
> >> figure out such a scenario…  But anyway, if you think this patch does
> >> not worth it, say it and I'll close this report.
> >
> > I'll wait a bit for others to chime in, if anyone has an opinion.
> 
> I think the patch makes sense.
> 
> Having defaults that protect users security and privacy better, even if
> only slightly, is not a bad thing, not unless there are cases where it
> hurts.  And I can't think of any such cases here.

desktop.el can create desktop files in any directory, not just under
the user's HOME directory.  (In fact, I use this feature a lot: I have
different desktop files in different directories, which allows me to
restore the last session on a per-project basis.)  While making the
desktop file unreadable/unwritable by others is probably okay under
HOME, doing that in other directories is not necessarily TRT,
especially if those desktop files can later be used from other users'
sessions.

So, if we install this, I think we need:

  . have a defcustom to control this behavior
  . the default is changed, possibly limit the new behavior to desktop
    files under the HOME directory, leaving the behavior in other
    directories as it is now
  . call out the change in NEWS; if the default changes, we should
    call it out in "Incompatible changes" section

There's also a (probably rare) scenario where the fact that the
desktop file doesn't exist does not necessarily mean it didn't exist
in that same directory.  Consider the following sequence:

  . start Emacs and restore desktop from an existing file
  . delete the desktop file
  . save desktop in the same directory

This could happen, for example, if the original desktop file was
faulty or incorrect in some way, and the user wants to "make it
right".  Completely legitimate (I think I did it myself a few times),
though probably rare.  In this case, the user won't expect the desktop
file to be treated as "new", and will certainly not expect to see its
access bits change in such a drastic way.

Bottom line: I think we should install this as optional behavior, by
default off, if at all.  If many people turn it on in their
customizations, we can later revisit the default value.

Thanks.

bug#67323: 30.0.50; [PATCH] Set a new desktop file to mode 0600

[Manuel Giraud via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Eli Zaretskii <eliz@gnu.org> writes:

[...]

> Bottom line: I think we should install this as optional behavior, by
> default off, if at all.  If many people turn it on in their
> customizations, we can later revisit the default value.

So with all these conditions (a custom, off by default), I then think
that we should not install this.  My idea was to have better privacy by
*default*.  If one user have to tweak a custom somewhere to do this, I
think he'd better set the Unix rights on its desktop file by "hand" as
I'm doing now.  And there is still umask also.
-- 
Manuel Giraud