bug#37445: 27.0.50; Permission denied after make install

bug#37445: 27.0.50; Permission denied after make install

[Tino Calancha]

X-Debbugs-Cc: Paul Eggert <eggert@cs.ucla.edu>

I get a permission denied error
after commit 9dc306b1db08196684d05a474148e16305adbad0

The following steps reproduce the error (you need 2 users to test it)

# Install Emacs w/ the first user
$ whoami
ec2-user
$ cd ~/soft/emacs-master
$ make && sudo make install
# Launch Emasc w/ the second user
$ su user_foo
$ cd
$ emacs
emacs: Reading symbolic link: Permission denied, /home/ec2-user/soft


In GNU Emacs 27.0.50 (build 8, x86_64-pc-linux-gnu)
  of 2019-09-18 built
Repository revision: 9dc306b1db08196684d05a474148e16305adbad0
Repository branch: master
System Description: Amazon Linux 2

Recent messages:
For information about GNU Emacs and the GNU system, type C-? C-a.
C-x C-g is undefined
Omitting...
(Nothing to omit)
Omitting...
Omitted 2 lines.
Collecting branch info...done
Pull from remote repository 'origin'? (y or n) y
Collecting branch info...done
git pull origin done!

Configured using:
  'configure --with-libxml2'

Configured features:
SOUND NOTIFY INOTIFY LIBSELINUX GNUTLS LIBXML2 ZLIB THREADS PDUMPER GMP

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

Major mode: Gited

Minor modes in effect:
   shell-dirtrack-mode: t
   display-time-mode: t
   winner-mode: t
   simpleclip-mode: t
   show-paren-mode: t
   minibuffer-depth-indicate-mode: t
   gited-hide-details-mode: t
   tooltip-mode: t
   global-eldoc-mode: t
   electric-indent-mode: t
   file-name-shadow-mode: t
   global-font-lock-mode: t
   font-lock-mode: t
   auto-composition-mode: t
   auto-encryption-mode: t
   auto-compression-mode: t
   buffer-read-only: t
   line-number-mode: t
   transient-mark-mode: t

Load-path shadows:
None found.

Features:
(shadow sort mail-extr emacsbug shell mule-util cl-extra term/xterm
xterm server manoj-dark-theme time smtpmail sendmail bookmark+
bookmark+-key bookmark+-1 gnus-sum url url-proxy url-privacy url-expand
url-methods url-history url-cookie url-domsuf url-util mailcap shr image
svg dom gnus-group gnus-undo gnus-start gnus-cloud nnimap nnmail
mail-source utf7 netrc nnoo parse-time iso8601 gnus-spec gnus-int
gnus-range message rmc puny rfc822 mml mml-sec epa derived epg
epg-config mm-decode mm-bodies mm-encode mail-parse rfc2231 mailabbrev
gmm-utils mailheader gnus-win bookmark+-bmu org-element avl-tree
generator help-mode org advice org-macro org-footnote org-pcomplete
pcomplete org-list org-faces org-entities noutline outline org-version
ob-emacs-lisp ob ob-tangle org-src ob-ref ob-lob ob-table ob-keys ob-exp
ob-comint ob-core ob-eval org-compat org-macs org-loaddefs format-spec
cal-menu calendar cal-loaddefs bookmark+-lit bookmark+-mac bookmark pp
misc emacs-lock winner xml git-handlers pcase ibuf-macs dired-x
dired-aux usl rect ibuffer ibuffer-loaddefs thingatpt simpleclip edmacro
kmacro cl paren mb-depth grep compile comint regexp-opt ansi-color ring
gnus nnheader gnus-util rmail rmail-loaddefs rfc2047 rfc2045 ietf-drums
text-property-search time-date mail-utils mm-util mail-prsvr wid-edit
cus-start cus-load gited find-func vc-git diff-mode easy-mmode dired
dired-loaddefs gited-ci finder-inf info tool-bar package easymenu
browse-url url-handlers url-parse auth-source cl-seq eieio eieio-core
cl-macs eieio-loaddefs password-cache json subr-x map url-vars seq
byte-opt gv bytecomp byte-compile cconv cl-loaddefs cl-lib tooltip eldoc
electric uniquify ediff-hook vc-hooks lisp-float-type tabulated-list
replace newcomment text-mode elisp-mode lisp-mode prog-mode register
page menu-bar rfn-eshadow isearch timer select mouse jit-lock font-lock
syntax facemenu font-core term/tty-colors frame 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 charscript charprop
case-table epa-hook jka-cmpr-hook help simple abbrev obarray minibuffer
cl-preloaded nadvice loaddefs button faces cus-face macroexp files
text-properties overlay sha1 md5 base64 format env code-pages mule
custom widget hashtable-print-readable backquote threads inotify
multi-tty make-network-process emacs)

Memory information:
((conses 16 290467 26391)
  (symbols 48 24391 2)
  (strings 32 75778 2373)
  (string-bytes 1 2664995)
  (vectors 16 29579)
  (vector-slots 8 317353 13828)
  (floats 8 235 339)
  (intervals 56 616 91)
  (buffers 992 15))

bug#37445: 27.0.50; Permission denied after make install

[Paul Eggert]

On 9/18/19 2:02 AM, Tino Calancha wrote:

> # Install Emacs w/ the first user
> $ whoami
> ec2-user
> $ cd ~/soft/emacs-master
> $ make && sudo make install
> # Launch Emasc w/ the second user
> $ su user_foo
> $ cd
> $ emacs
> emacs: Reading symbolic link: Permission denied, /home/ec2-user/soft

This appears to be a configuration error in how Emacs master starts up. 
Apparently if you build Emacs in (say) /tmp/foo and then install Emacs, the 
Emacs you install consults files in /tmp/foo during startup. After you remove 
/tmp/foo, someone else can create a /tmp/foo and hijack anybody who starts up 
the installed Emacs.

I papered over the problem with commit 2019-09-18T11:21:19Z!eggert@cs.ucla.edu 
(735940f4551a43f3b4381105dc074cd7d494f2f3), which suppresses the diagnostic and 
let Emacs continue to run. However, the configuration error remains and I will 
try to squeeze free time to look at it.

I should be able to reproduce the original problem by compiling with 
-DPICKY_EACCES. That is, the idea is to use -DPICKY_EACCES to debug longstanding 
bugs in Emacs that we otherwise might not have discovered.

bug#37445: 27.0.50; Permission denied after make install

[Paul Eggert]

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

I installed the attached patch, which should fix the bug even when PICKY_EACCES 
is nonzero. Boldly closing the bug report.

[-- Attachment #2: 0001-Omit-some-overenthusiastic-file-truename-calls.patch --]
[-- Type: text/x-patch, Size: 3662 bytes --]

From dff4f9c759f5cf19047719716ea5ee8ffdc3006e Mon Sep 17 00:00:00 2001
From: Paul Eggert <eggert@cs.ucla.edu>
Date: Wed, 18 Sep 2019 23:53:46 -0700
Subject: [PATCH] Omit some overenthusiastic file-truename calls

Problem reported by Tino Calancha (Bug#37445).
* src/emacs.c (init_cmdargs): Call file-truename only if
needed, i.e., if invocation-directory ends in "/i386/" on
WINDOWSNT.
* src/lread.c (readevalloop): If the sourcename is not
absolute, make it absolute.  There is no need to convert
non-absolute files into truenames, since absolute files are
not converted into truenames.
(init_lread): Do not convert source-directory into a truename
at startup.  There is no need to do so in a dumped Emacs since
an absolute file name suffices.  The source directory might
not even exist any more, or might have been replaced by an
interloper who takes advantage of the truename calculation.
(syms_of_lread): Remove Qfile_truename; no longer needed.
---
 src/emacs.c | 14 +++++++++-----
 src/lread.c | 11 +++--------
 2 files changed, 12 insertions(+), 13 deletions(-)

diff --git a/src/emacs.c b/src/emacs.c
index eb732810db..8a8d8b558e 100644
--- a/src/emacs.c
+++ b/src/emacs.c
@@ -479,9 +479,6 @@ init_cmdargs (int argc, char **argv, int skip_args, char const *original_pwd)
 
   if (!NILP (Vinvocation_directory))
     {
-      if (NILP (Vpurify_flag) && !NILP (Ffboundp (Qfile_truename)))
-        Vinvocation_directory = call1 (Qfile_truename, Vinvocation_directory);
-
       dir = Vinvocation_directory;
 #ifdef WINDOWSNT
       /* If we are running from the build directory, set DIR to the
@@ -490,8 +487,15 @@ init_cmdargs (int argc, char **argv, int skip_args, char const *original_pwd)
       if (SBYTES (dir) > sizeof ("/i386/") - 1
 	  && 0 == strcmp (SSDATA (dir) + SBYTES (dir) - sizeof ("/i386/") + 1,
 			  "/i386/"))
-	dir = Fexpand_file_name (build_string ("../.."), dir);
-#else  /* !WINDOWSNT */
+	{
+	  if (NILP (Vpurify_flag))
+	    {
+	      Lisp_Object file_truename = intern ("file-truename");
+	      if (!NILP (Ffboundp (file_truename)))
+		dir = call1 (file_truename, dir);
+	    }
+	  dir = Fexpand_file_name (build_string ("../.."), dir);
+	}
 #endif
       name = Fexpand_file_name (Vinvocation_name, dir);
       while (1)
diff --git a/src/lread.c b/src/lread.c
index 99e0ce30ba..4f3446b09d 100644
--- a/src/lread.c
+++ b/src/lread.c
@@ -1999,11 +1999,10 @@ readevalloop (Lisp_Object readcharfun,
 	    (NILP (lex_bound) || EQ (lex_bound, Qunbound)
 	     ? Qnil : list1 (Qt)));
 
-  /* Try to ensure sourcename is a truename, except whilst preloading.  */
+  /* Ensure sourcename is absolute, except whilst preloading.  */
   if (!will_dump_p ()
-      && !NILP (sourcename) && !NILP (Ffile_name_absolute_p (sourcename))
-      && !NILP (Ffboundp (Qfile_truename)))
-    sourcename = call1 (Qfile_truename, sourcename) ;
+      && !NILP (sourcename) && !NILP (Ffile_name_absolute_p (sourcename)))
+    sourcename = Fexpand_file_name (sourcename, Qnil);
 
   LOADHIST_ATTACH (sourcename);
 
@@ -4678,9 +4677,6 @@ load_path_default (void)
 void
 init_lread (void)
 {
-  if (NILP (Vpurify_flag) && !NILP (Ffboundp (Qfile_truename)))
-    Vsource_directory = call1 (Qfile_truename, Vsource_directory);
-
   /* First, set Vload_path.  */
 
   /* Ignore EMACSLOADPATH when dumping.  */
@@ -5100,7 +5096,6 @@ this variable will become obsolete.  */);
   DEFSYM (Qload, "load");
   DEFSYM (Qload_file_name, "load-file-name");
   DEFSYM (Qeval_buffer_list, "eval-buffer-list");
-  DEFSYM (Qfile_truename, "file-truename");
   DEFSYM (Qdir_ok, "dir-ok");
   DEFSYM (Qdo_after_load_evaluation, "do-after-load-evaluation");
 
-- 
2.17.1

bug#37445: 27.0.50; Permission denied after make install

[Tino Calancha]

On Wed, 18 Sep 2019, Paul Eggert wrote:

> I installed the attached patch, which should fix the bug even when 
> PICKY_EACCES is nonzero. Boldly closing the bug report.

Hi Paul,
I think the commit message mentions the opposite of what is actually done 
in the code.

commit 30026cfe666e9647aeef73e26df5ffca2fa2c662
Author: Paul Eggert <eggert@cs.ucla.edu>
Date:   Thu Sep 19 00:19:11 2019 -0700

     Default PICKY_ACCESS to false on non-MS

     * src/fileio.c (PICKY_EACCES) [!DOS_NT]: Default to false.

This is the code we have now:

#ifndef PICKY_EACCES
# ifdef DOS_NT
enum { PICKY_EACCES = false };
# else
enum { PICKY_EACCES = true };
# endif
#endif


This code starts Emacs on my Linux machine but it refuses to load my 
.emacs file:

M-: (load-file ".emacs")
Load error for /home/user_foo/.emacs:
(file-error Testing file Permission denied 
/home/ec2-user/soft/emacs-master/src)

bug#37445: 27.0.50; Permission denied after make install

[Paul Eggert]

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

On 9/19/19 4:35 AM, Tino Calancha wrote:
> 
> This code starts Emacs on my Linux machine but it refuses to load my 
> .emacs file:
> 
> M-: (load-file ".emacs")
> Load error for /home/user_foo/.emacs:
> (file-error Testing file Permission denied 
> /home/ec2-user/soft/emacs-master/src)

Hmm, I'm not seeing the problem. As user_foo, what happens if you run 
this shell command?

cat /home/user_foo/.emacs

Also, what happens when you do this as user_foo?

strace -o tr emacs -Q -batch -eval '(message "%s" (load-file ".emacs"))'

Look at your "tr" file, and compare its system calls to mine (compressed 
and attached).

[-- Attachment #2: tr.gz --]
[-- Type: application/gzip, Size: 13305 bytes --]

bug#37445: 27.0.50; Permission denied after make install

[Tino Calancha]

On Thu, 19 Sep 2019, Paul Eggert wrote:

> On 9/19/19 4:35 AM, Tino Calancha wrote:
>> 
>> This code starts Emacs on my Linux machine but it refuses to load my .emacs 
>> file:
>
> Hmm, I'm not seeing the problem. As user_foo, what happens if you run this 
> shell command?

I have reproduced the problem at 2 Amazon machines with the following 
recipe:
[A call to `require' inside .emacs seems to fire the issue]

$ whoami
ec2-user
$ cd ~/soft/emacs-master
$ make && sudo make install

$ ls /home
ec2-user

# Create a fresh new user
$ sudo useradd user_foo
$ ls /home
ec2-user  user_foo

# Change to the new user and go to its home dir
$ sudo su user_foo
$ cd
$ pwd
/home/user_foo
$ ls -a
.  ..  .bash_logout  .bash_profile  .bashrc  .emacs

$ cat .emacs
;; .emacs

(custom-set-variables
  ;; uncomment to always end a file with a newline
  ;'(require-final-newline t)
  ;; uncomment to disable loading of "default.el" at startup
  ;'(inhibit-default-init t)
  ;; default to unified diffs
  '(diff-switches "-u"))

;;; uncomment for CJK utf-8 support for non-Asian users
;; (require 'un-define)

# Add (require 'ert) at the botton: this seems to fire the issue
$ echo "(require 'ert)" >> .emacs

# Now launch Emacs: you will see at *Warnings* buffer
# File error: Testing file, Permission denied, /home/ec2-user/soft/emacs-master/src

# Now, if you want you can try:
M-: (require 'ert) RET
Testing file: Permission denied, /home/ec2-user/soft/emacs-master/src

bug#37445: 27.0.50; Permission denied after make install

[Eli Zaretskii]

> From: Tino Calancha <tino.calancha@gmail.com>
> Date: Fri, 20 Sep 2019 06:07:54 +0000 (UTC)
> Cc: 37445@debbugs.gnu.org, Tino Calancha <tino.calancha@gmail.com>
> 
> # Add (require 'ert) at the botton: this seems to fire the issue
> $ echo "(require 'ert)" >> .emacs
> 
> # Now launch Emacs: you will see at *Warnings* buffer
> # File error: Testing file, Permission denied, /home/ec2-user/soft/emacs-master/src
> 
> # Now, if you want you can try:
> M-: (require 'ert) RET
> Testing file: Permission denied, /home/ec2-user/soft/emacs-master/src

Can you show the C and Lisp backtraces for that error?  You can run
Emacs under GDB with a breakpoint on this line of fileio.c:

  static Lisp_Object
  file_metadata_errno (char const *action, Lisp_Object file, int err)
  {
    if (err == ENOENT || err == ENOTDIR || err == 0)
      return Qnil;
    report_file_errno (action, file, err);  <<<<<<<<<<<<<<<<<<<<<<<<<<
  }

Thanks.

bug#37445: 27.0.50; Permission denied after make install

[Paul Eggert]

On 9/19/19 11:07 PM, Tino Calancha wrote:
> 
> # Now launch Emacs: you will see at *Warnings* buffer
> # File error: Testing file, Permission denied, /home/ec2-user/soft/emacs-master/src

Thanks, I think I see the problem: Emacs is examining its source code, via the 
Lisp variable source-directory, a variable that is put into the dump file. But 
in your case the source code's permissions forbid access.

This glitch suggests that there are more-serious security problems in the 
default Emacs install. If source-directory is (say) "/tmp/emacs-build/whatever", 
and /tmp/emacs-build is removed after the build, an attacker can provide a bogus 
source directory in place of the real one, and this could cause real problems.

Fedora 30 solves this potential security problem by arranging for the Lisp 
variable source-directory to have a value like "/usr/share/emacs/26.2/", which 
is a place attackers shouldn't be able to overwrite.

However, the default Emacs install doesn't do that. It installs the sources into 
(say) "/usr/local/share/emacs/27.0.50", but it doesn't arrange for 
source-directory to point there; instead, source-directory points to wherever 
the sources happened to be when Emacs was built, which could be in /tmp. This 
sounds like a configuration error in the default Emacs install, and I plan to 
look into why it's unsafe whereas the Fedora Emacs install is safer.

bug#37445: 27.0.50; Permission denied after make install

[Eli Zaretskii]

> From: Paul Eggert <eggert@cs.ucla.edu>
> Date: Fri, 20 Sep 2019 02:10:10 -0700
> Cc: 37445@debbugs.gnu.org
> 
> This glitch suggests that there are more-serious security problems in the 
> default Emacs install. If source-directory is (say) "/tmp/emacs-build/whatever", 
> and /tmp/emacs-build is removed after the build, an attacker can provide a bogus 
> source directory in place of the real one, and this could cause real problems.

What kind of problems could accessing such a directory cause?

Note that there are also various EMACS* environment variables to which
Emacs heeds, they can override the likes of data-directory.

> Fedora 30 solves this potential security problem by arranging for the Lisp 
> variable source-directory to have a value like "/usr/share/emacs/26.2/", which 
> is a place attackers shouldn't be able to overwrite.
> 
> However, the default Emacs install doesn't do that. It installs the sources into 
> (say) "/usr/local/share/emacs/27.0.50", but it doesn't arrange for 
> source-directory to point there; instead, source-directory points to wherever 
> the sources happened to be when Emacs was built, which could be in /tmp. This 
> sounds like a configuration error in the default Emacs install, and I plan to 
> look into why it's unsafe whereas the Fedora Emacs install is safer.

If you point source-directory away of where the real sources are, some
Help features will cease working.  So I don't think we want the Fedora
solution.  What we want is that sources will be inaccessible in this
situation, but APIs such as 'load' and 'require' still work
regardless.

bug#37445: 27.0.50; Permission denied after make install

[Paul Eggert]

On 9/20/19 5:40 AM, Eli Zaretskii wrote:
> If you point source-directory away of where the real sources are, some
> Help features will cease working.

Which Help features are those? The files that Help uses are already installed 
into the etc or lisp subdirectories of /usr/share/emacs/27.0.50 (or whatever).

I looked through the Emacs source code, and the only use of source-directory 
that seemed relevant was find-function-C-source-directory; is that what you were 
referring to? If so, the problem can be addressed by installing the C sources 
into /usr/share/emacs/27.0.50/src/*.[chm], which is something we should be doing 
anyway since the build directory might be missing or (worse) wrong when someone 
wants to look at the sources of the installed Emacs. In the old days installing 
the C sources might have been thought too heavyweight but those days are long gone.

bug#37445: 27.0.50; Permission denied after make install

[Eli Zaretskii]

> Cc: tino.calancha@gmail.com, 37445@debbugs.gnu.org
> From: Paul Eggert <eggert@cs.ucla.edu>
> Date: Fri, 20 Sep 2019 11:17:43 -0700
> 
> I looked through the Emacs source code, and the only use of source-directory 
> that seemed relevant was find-function-C-source-directory; is that what you were 
> referring to?

Yes.  Help uses that when you activate the button in a *Help* buffer
that says a function is defined in C sources.

> If so, the problem can be addressed by installing the C sources 
> into /usr/share/emacs/27.0.50/src/*.[chm], which is something we should be doing 
> anyway since the build directory might be missing or (worse) wrong when someone 
> wants to look at the sources of the installed Emacs. In the old days installing 
> the C sources might have been thought too heavyweight but those days are long gone.

You are talking about a serious change in Emacs installation
procedure.  It should first be discussed and its various implications
understood and considered.  I'm not so sure this is a good approach,
or that it should be the only one supported.  Some people may not want
to install sources; others still build their own Emacs, and have the
sources available in accessible directories, so installing the sources
into yet another tree will be uneconomical for them.  And there might
be other use cases and other considerations.  If we want to make such
changes, we should do that in a way that caters to all the use cases
we support today.

I asked what security problems could be caused by accessing a source
tree, and you didn't answer.  From where I stand, that is a crucial
question: if the danger is not real, or non-existent, I see no good
reason to make such significant changes, certainly not soon.

bug#37445: 27.0.50; Permission denied after make install

[Paul Eggert]

On 9/20/19 11:59 AM, Eli Zaretskii wrote:
> If we want to make such
> changes, we should do that in a way that caters to all the use cases
> we support today.

Of course. Among other things we should continue to let people access the 
sources where they were originally built, if that's what they want to do. But 
the default installation should be a safe one.

If it is considered to be too much to install the C source files by default, we 
can simply make that an installation option with default off; that will still be 
safe, since find-function-C-source-directory will do the right thing when the 
source files are not installed. However, I'm mildly inclined to install the 
source files by default since they don't grow the installation size that much: 
on my platform the current default installation is 144 MiB, and the relevant 
source files are 8.6 MiB uncompressed, 2.5 MiB compressed (these counts include 
filesystem overhead).
> I asked what security problems could be caused by accessing a source tree

I'll reply separately about that.

bug#37445: 27.0.50; Permission denied after make install

[Eli Zaretskii]

> Cc: tino.calancha@gmail.com, 37445@debbugs.gnu.org
> From: Paul Eggert <eggert@cs.ucla.edu>
> Date: Fri, 20 Sep 2019 12:33:20 -0700
> 
> If it is considered to be too much to install the C source files by default, we 
> can simply make that an installation option with default off; that will still be 
> safe, since find-function-C-source-directory will do the right thing when the 
> source files are not installed. However, I'm mildly inclined to install the 
> source files by default since they don't grow the installation size that much: 

If we decide to go this way, I think we should start by making such
source installation optional, because there might be factors we will
be unable to consider or predict.  There's no rush and no particular
reason to make this the default soon, just because we can.

But let's discuss this on emacs-devel, not here.

> > I asked what security problems could be caused by accessing a source tree
> 
> I'll reply separately about that.

TIA, and please do that on emacs-devel.

bug#37445: 27.0.50; Permission denied after make install

[Paul Eggert]

On 9/20/19 11:07 PM, Eli Zaretskii wrote:
> let's discuss this on emacs-devel, not here.

OK, I'll do that. I filed Bug#37527, with a patch for the problem with 
'C-h f' on typical GNU/Linux installations, and will mention this on 
emacs-devel. I am closing Bug#37445 with this email, as that bug was 
fixed by the earlier patches already on master.