electron / electron

:electron: Build cross-platform desktop apps with JavaScript, HTML, and CSS
https://electronjs.org
MIT License
113.53k stars 15.27k forks source link

Unable to open asar archive when having no execution permission on $HOME #3666

Closed jviotti closed 8 years ago

jviotti commented 8 years ago

I'm having a strange issue in some GNU/Linux systems with v0.35.2:

Uncaught Error: Cannot find module '/home/jviotti/Downloads/7193eec573c62867151d-4e63199c2cf83a13e4a2301c61deec2616f9187b/node_modules/electron-prebuilt/dist/resources/atom.asar/renderer/lib/init.js'

screenshot from 2015-12-02 13-35-42

The file referenced above does exist:

$ asar list node_modules/electron-prebuilt/dist/resources/atom.asar | grep renderer/lib/init.js
/renderer/lib/init.js

I've been able to build a small dummy application that reproduces the error: https://gist.github.com/jviotti/7193eec573c62867151d.

Instructions:

  1. Download the gist as a zip.
  2. npm install.
  3. Execute npm start as sudo: sudo npm start (the app runs fine without sudo).

    Some experimentation I did:

    • I can reproduce the issue on Fedora 22, but not on an Ubuntu 14.04 virtual machine.
    • The issue is gone by downgrading to v0.31.2 (any higher version fails).
    • The issue happens by using npm start as well as executing the electron binary directly.

I'm bisecting between v0.32.0 and v0.31.2 but bootstrap.py is taking forever in each step here (I don't have a good internet connection currently).

What could be causing this?

jviotti commented 8 years ago

A consequence of this issue is that window.require is not defined in the renderer process, making impossible to require any code.

jviotti commented 8 years ago

Turns out I can reproduce in Ubuntu 14.04 running in a real machine, but not in a VM.

jviotti commented 8 years ago

I'm still doing the bisect process. The first bad commit is among these ones:

53b9d61 Fix building on Windows
2c3751e Merge pull request #2696 from John-Lin/master
4a81300 Merge pull request #2695 from takashi/patch-1
73af4c0 remove -jp suffix from quick-start-jp.md
2734d67 add Traditional Chinese documents link in README to navigate
c81de98 Update brightray to Chrome 45
599e9b9 Provide task runner for the node mode
c1d7ad9 Devtools no longer uses iframes
ee0dc0d Update clang
4254eb2 Fix API changes on Linux 
262b66b Feed gin::PerIsolateData with a task runner
45491ca Fix API changes
1db8432 Upgrade to Chrome 45
992aada Can only run tests on x64 machine

I'll finish the remaining steps tomorrow.

jviotti commented 8 years ago

The bug was introduced in the following commits:

599e9b9 Provide task runner for the node mode
c1d7ad9 Devtools no longer uses iframes
ee0dc0d Update clang
4254eb2 Fix API changes on Linux 
262b66b Feed gin::PerIsolateData with a task runner
45491ca Fix API changes
1db8432 Upgrade to Chrome 45

Sadly, all these commits are completely broken on Linux (both Ubuntu and Fedora) and do not build at all (I get lots of C++ errors), so I couldn't reduce the options anymore.

Here is the compare view between those commits: https://github.com/atom/electron/compare/1db8432...599e9b9.

Hopefully someone with more experience in the C++ side of electron can review.

jviotti commented 8 years ago

As a side note, https://github.com/atom/electron/commit/992aada90f4bb53913e58d90f49b8ecce7bf02c1 is the last good commit.

jviotti commented 8 years ago

These are the errors I get on the broken commits mentioned above:

parallels@ubuntu:~/Projects/electron$ ./script/build.py -c D
ninja: Entering directory `out/D'
[18/1114] CXX obj/vendor/brightray/browser/brightray.browser_client.o
FAILED: /home/parallels/Projects/electron/vendor/llvm-build/Release+Asserts/bin/clang++ -MMD -MF obj/vendor/brightray/browser/brightray.browser_client.o.d -DNDEBUG -DV8_USE_EXTERNAL_STARTUP_DATA -DSK_SUPPORT_LEGACY_GETTOPDEVICE -DSK_SUPPORT_LEGACY_BITMAP_CONFIG -DSK_SUPPORT_LEGACY_DEVICE_VIRTUAL_ISOPAQUE -DSK_SUPPORT_LEGACY_N32_NAME -DSK_SUPPORT_LEGACY_SETCONFIG -DSK_IGNORE_ETC1_SUPPORT -DSK_IGNORE_GPU_DITHER -DTOOLKIT_VIEWS -DUSE_AURA -DUSE_X11 -DUSE_NSS_CERTS -DUSE_NSS -D_LARGEFILE_SOURCE -D_LARGEFILE64_SOURCE -D_FILE_OFFSET_BITS=64 -DDEBUG -DCOMPONENT_BUILD -DGURL_DLL -DSKIA_DLL -DUSING_V8_SHARED -DWEBKIT_DLL -I../../vendor/brightray -I/home/parallels/Projects/electron/vendor/brightray/vendor/download/libchromiumcontent/src -I/home/parallels/Projects/electron/vendor/brightray/vendor/download/libchromiumcontent/src/skia/config -I/home/parallels/Projects/electron/vendor/brightray/vendor/download/libchromiumcontent/src/third_party/skia/include/core -I/home/parallels/Projects/electron/vendor/brightray/vendor/download/libchromiumcontent/src/third_party/mojo/src -I/home/parallels/Projects/electron/vendor/brightray/vendor/download/libchromiumcontent/src/third_party/WebKit -I/home/parallels/Projects/electron/vendor/brightray/vendor/download/libchromiumcontent/shared_library/gen -pthread -I/usr/include/gtk-2.0 -I/usr/lib/x86_64-linux-gnu/gtk-2.0/include -I/usr/include/atk-1.0 -I/usr/include/cairo -I/usr/include/gdk-pixbuf-2.0 -I/usr/include/pango-1.0 -I/usr/include/gio-unix-2.0/ -I/usr/include/freetype2 -I/usr/include/glib-2.0 -I/usr/lib/x86_64-linux-gnu/glib-2.0/include -I/usr/include/pixman-1 -I/usr/include/libpng12 -I/usr/include/harfbuzz -I/usr/include/dbus-1.0 -I/usr/lib/x86_64-linux-gnu/dbus-1.0/include -I/usr/include/gconf/2 -I/usr/include/nss -I/usr/include/nspr -Wno-deprecated-register -Wno-sentinel -m64 -march=x86-64 -std=c++11 -Wno-reserved-user-defined-literal -D__STRICT_ANSI__ -fno-rtti  -c ../../vendor/brightray/browser/browser_client.cc -o obj/vendor/brightray/browser/brightray.browser_client.o
In file included from ../../vendor/brightray/browser/browser_client.cc:7:
In file included from ../../vendor/brightray/browser/browser_context.h:8:
../../vendor/brightray/browser/permission_manager.h:26:72: error: non-virtual member function marked 'override' hides virtual member function
      const base::Callback<void(content::PermissionStatus)>& callback) override;
                                                                       ^
/home/parallels/Projects/electron/vendor/brightray/vendor/download/libchromiumcontent/src/content/public/browser/permission_manager.h:29:16: note: hidden overloaded virtual function 'content::PermissionManager::RequestPermission' declared here: type mismatch at 2nd parameter ('content::RenderFrameHost *' vs 'content::WebContents *')
  virtual void RequestPermission(
               ^
In file included from ../../vendor/brightray/browser/browser_client.cc:7:
In file included from ../../vendor/brightray/browser/browser_context.h:8:
../../vendor/brightray/browser/permission_manager.h:30:63: error: non-virtual member function marked 'override' hides virtual member function
                               const GURL& requesting_origin) override;
                                                              ^
/home/parallels/Projects/electron/vendor/brightray/vendor/download/libchromiumcontent/src/content/public/browser/permission_manager.h:39:16: note: hidden overloaded virtual function 'content::PermissionManager::CancelPermissionRequest' declared here: type mismatch at 2nd parameter ('content::RenderFrameHost *' vs 'content::WebContents *')
  virtual void CancelPermissionRequest(PermissionType permission,
               ^
2 errors generated.
[18/1114] CXX obj/vendor/brightray/browser/brightray.browser_context.o
FAILED: /home/parallels/Projects/electron/vendor/llvm-build/Release+Asserts/bin/clang++ -MMD -MF obj/vendor/brightray/browser/brightray.browser_context.o.d -DNDEBUG -DV8_USE_EXTERNAL_STARTUP_DATA -DSK_SUPPORT_LEGACY_GETTOPDEVICE -DSK_SUPPORT_LEGACY_BITMAP_CONFIG -DSK_SUPPORT_LEGACY_DEVICE_VIRTUAL_ISOPAQUE -DSK_SUPPORT_LEGACY_N32_NAME -DSK_SUPPORT_LEGACY_SETCONFIG -DSK_IGNORE_ETC1_SUPPORT -DSK_IGNORE_GPU_DITHER -DTOOLKIT_VIEWS -DUSE_AURA -DUSE_X11 -DUSE_NSS_CERTS -DUSE_NSS -D_LARGEFILE_SOURCE -D_LARGEFILE64_SOURCE -D_FILE_OFFSET_BITS=64 -DDEBUG -DCOMPONENT_BUILD -DGURL_DLL -DSKIA_DLL -DUSING_V8_SHARED -DWEBKIT_DLL -I../../vendor/brightray -I/home/parallels/Projects/electron/vendor/brightray/vendor/download/libchromiumcontent/src -I/home/parallels/Projects/electron/vendor/brightray/vendor/download/libchromiumcontent/src/skia/config -I/home/parallels/Projects/electron/vendor/brightray/vendor/download/libchromiumcontent/src/third_party/skia/include/core -I/home/parallels/Projects/electron/vendor/brightray/vendor/download/libchromiumcontent/src/third_party/mojo/src -I/home/parallels/Projects/electron/vendor/brightray/vendor/download/libchromiumcontent/src/third_party/WebKit -I/home/parallels/Projects/electron/vendor/brightray/vendor/download/libchromiumcontent/shared_library/gen -pthread -I/usr/include/gtk-2.0 -I/usr/lib/x86_64-linux-gnu/gtk-2.0/include -I/usr/include/atk-1.0 -I/usr/include/cairo -I/usr/include/gdk-pixbuf-2.0 -I/usr/include/pango-1.0 -I/usr/include/gio-unix-2.0/ -I/usr/include/freetype2 -I/usr/include/glib-2.0 -I/usr/lib/x86_64-linux-gnu/glib-2.0/include -I/usr/include/pixman-1 -I/usr/include/libpng12 -I/usr/include/harfbuzz -I/usr/include/dbus-1.0 -I/usr/lib/x86_64-linux-gnu/dbus-1.0/include -I/usr/include/gconf/2 -I/usr/include/nss -I/usr/include/nspr -Wno-deprecated-register -Wno-sentinel -m64 -march=x86-64 -std=c++11 -Wno-reserved-user-defined-literal -D__STRICT_ANSI__ -fno-rtti  -c ../../vendor/brightray/browser/browser_context.cc -o obj/vendor/brightray/browser/brightray.browser_context.o
In file included from ../../vendor/brightray/browser/browser_context.cc:5:
In file included from ../../vendor/brightray/browser/browser_context.h:8:
../../vendor/brightray/browser/permission_manager.h:22:16: error: no type named 'WebContents' in namespace 'content'
      content::WebContents* web_contents,
      ~~~~~~~~~^
../../vendor/brightray/browser/permission_manager.h:28:41: error: no type named 'WebContents' in namespace 'content'
                               content::WebContents* web_contents,
                               ~~~~~~~~~^
In file included from ../../vendor/brightray/browser/browser_context.cc:8:
../../vendor/brightray/browser/inspectable_web_contents_impl.h:149:68: error: non-virtual member function marked 'override' hides virtual member function
      content::SessionStorageNamespace* session_storage_namespace) override;
                                                                   ^
/home/parallels/Projects/electron/vendor/brightray/vendor/download/libchromiumcontent/src/content/public/browser/web_contents_delegate.h:293:16: note: hidden overloaded virtual function 'content::WebContentsDelegate::ShouldCreateWebContents' declared here: type mismatch at 5th parameter ('const std::string &' (aka 'const basic_string<char> &') vs 'const base::string16 &' (aka 'const basic_string<char16, base::string16_char_traits> &'))
  virtual bool ShouldCreateWebContents(
               ^
In file included from ../../vendor/brightray/browser/browser_context.cc:9:
../../vendor/brightray/browser/network_delegate.h:64:67: error: only virtual member functions can be marked 'override'
  bool OnCanThrottleRequest(const net::URLRequest& request) const override;
                                                                  ^~~~~~~~
../../vendor/brightray/browser/browser_context.cc:188:35: error: allocating an object of abstract class type 'brightray::PermissionManager'
    permission_manager_.reset(new PermissionManager);
                                  ^
/home/parallels/Projects/electron/vendor/brightray/vendor/download/libchromiumcontent/src/content/public/browser/permission_manager.h:29:16: note: unimplemented pure virtual method 'RequestPermission' in 'PermissionManager'
  virtual void RequestPermission(
               ^
/home/parallels/Projects/electron/vendor/brightray/vendor/download/libchromiumcontent/src/content/public/browser/permission_manager.h:39:16: note: unimplemented pure virtual method 'CancelPermissionRequest' in 'PermissionManager'
  virtual void CancelPermissionRequest(PermissionType permission,
               ^
5 errors generated.
[18/1114] CXX obj/vendor/brightray/browser/brightray.browser_main_parts.o
FAILED: /home/parallels/Projects/electron/vendor/llvm-build/Release+Asserts/bin/clang++ -MMD -MF obj/vendor/brightray/browser/brightray.browser_main_parts.o.d -DNDEBUG -DV8_USE_EXTERNAL_STARTUP_DATA -DSK_SUPPORT_LEGACY_GETTOPDEVICE -DSK_SUPPORT_LEGACY_BITMAP_CONFIG -DSK_SUPPORT_LEGACY_DEVICE_VIRTUAL_ISOPAQUE -DSK_SUPPORT_LEGACY_N32_NAME -DSK_SUPPORT_LEGACY_SETCONFIG -DSK_IGNORE_ETC1_SUPPORT -DSK_IGNORE_GPU_DITHER -DTOOLKIT_VIEWS -DUSE_AURA -DUSE_X11 -DUSE_NSS_CERTS -DUSE_NSS -D_LARGEFILE_SOURCE -D_LARGEFILE64_SOURCE -D_FILE_OFFSET_BITS=64 -DDEBUG -DCOMPONENT_BUILD -DGURL_DLL -DSKIA_DLL -DUSING_V8_SHARED -DWEBKIT_DLL -I../../vendor/brightray -I/home/parallels/Projects/electron/vendor/brightray/vendor/download/libchromiumcontent/src -I/home/parallels/Projects/electron/vendor/brightray/vendor/download/libchromiumcontent/src/skia/config -I/home/parallels/Projects/electron/vendor/brightray/vendor/download/libchromiumcontent/src/third_party/skia/include/core -I/home/parallels/Projects/electron/vendor/brightray/vendor/download/libchromiumcontent/src/third_party/mojo/src -I/home/parallels/Projects/electron/vendor/brightray/vendor/download/libchromiumcontent/src/third_party/WebKit -I/home/parallels/Projects/electron/vendor/brightray/vendor/download/libchromiumcontent/shared_library/gen -pthread -I/usr/include/gtk-2.0 -I/usr/lib/x86_64-linux-gnu/gtk-2.0/include -I/usr/include/atk-1.0 -I/usr/include/cairo -I/usr/include/gdk-pixbuf-2.0 -I/usr/include/pango-1.0 -I/usr/include/gio-unix-2.0/ -I/usr/include/freetype2 -I/usr/include/glib-2.0 -I/usr/lib/x86_64-linux-gnu/glib-2.0/include -I/usr/include/pixman-1 -I/usr/include/libpng12 -I/usr/include/harfbuzz -I/usr/include/dbus-1.0 -I/usr/lib/x86_64-linux-gnu/dbus-1.0/include -I/usr/include/gconf/2 -I/usr/include/nss -I/usr/include/nspr -Wno-deprecated-register -Wno-sentinel -m64 -march=x86-64 -std=c++11 -Wno-reserved-user-defined-literal -D__STRICT_ANSI__ -fno-rtti  -c ../../vendor/brightray/browser/browser_main_parts.cc -o obj/vendor/brightray/browser/brightray.browser_main_parts.o
In file included from ../../vendor/brightray/browser/browser_main_parts.cc:7:
In file included from ../../vendor/brightray/browser/browser_context.h:8:
../../vendor/brightray/browser/permission_manager.h:22:16: error: no type named 'WebContents' in namespace 'content'
      content::WebContents* web_contents,
      ~~~~~~~~~^
../../vendor/brightray/browser/permission_manager.h:28:41: error: no type named 'WebContents' in namespace 'content'
                               content::WebContents* web_contents,
                               ~~~~~~~~~^
2 errors generated.
ninja: build stopped: subcommand failed.
jviotti commented 8 years ago

The issue seems to be gone by cherry picking c81de98 Update brightray to Chrome 45. I don't have access to a Linux machine at the moment (only a VM, but I can't reproduce the original issue there), but I'll keep bisecting later.

zcbenz commented 8 years ago

Can you try whether 0.36.0 works?

jviotti commented 8 years ago

@zcbenz It still fails with the same error.

arashpayan commented 8 years ago

I just experienced this while using v0.36.2 as well. On Ubuntu 14.04 x86_64, and on a custom Linux distro running on an ARM chip. Running as a non-root user lets me work around the issue on both systems.

jviotti commented 8 years ago

@arashpayan Sadly I was unable to find the reason of the error. It seems to start happening right after upgrading to Chrome 45, and is still present on newer Chrome versions.

jviotti commented 8 years ago

@zcbenz Any updates on this?

zcbenz commented 8 years ago

@jviotti I can not reproduce the problem on any machine, and I never saw similar issues in Atom, so you are basically on your own.

ghost commented 8 years ago

@jviotti : maybe a silly question because I'm unfamiliar with the build process, but shouldn't those compilation failures show up in the build process?

I'm think I'm having the same issue myself, and trying to track it down. My app actually requires root privs to run (or CAP_NET_ADMIN capabilities), so I'm glad you came up with a simpler test case :)

I'm currently attempting to build electron myself to see if i can reproduce those errors on Fedora 23.

ghost commented 8 years ago

definitely getting a ton of build errors and/or warnings when trying to build 0.36.4, so many that the output went past my shell buffer, and I don't have time to build again right now. I do see similar brightray issues though.

jviotti commented 8 years ago

@jrobeson Are you bisecting electron? I get the build errors on certain commits, unless I cherry pick c81de98 Update brightray to Chrome 45 on each step.

ghost commented 8 years ago

nah, I'm not bisecting, that'd take up too much machine time. I just wanted to see if i could replicate your build issue with the latest electron, since it seemed like nobody else did/could. Seems like that build issue should be verified against a newer chromium.

ghost commented 8 years ago

I wonder if it's fixed if chrome 48 updates will help.

jviotti commented 8 years ago

@zcbenz I've been debugging this issue on master and found that the origin of this issue is _file.IsValid() returning false in asar/archive.cc when running getOrCreateArchive() on atom.asar:

// https://github.com/atom/electron/blob/master/atom/common/asar/archive.cc#L139

bool Archive::Init() {
  if (!file_.IsValid())
    return false;

...

I was able to get more information about the error like this:

if (!file_.IsValid()) {

    // Include <iostream> for std::cout and std::endl
    std::cout << file_.ErrorToString(file_.error_details()) << std::endl;

    return false;
}

Which printed: FILE_ERROR_ACCESS_DENIED, which is very weird given that the process is running as root and I of course have access to the package.

What do you think?


UPDATE 1: I've tried adjusting the permissions of atom.asar and even making root the owner without luck.

UPDATE 2: Another discovery: file_.GetPlatformFile() returns -1 when running as root, while it returns 201 when being ran as a normal user. This is probably due to the file not able to being opened before.

jviotti commented 8 years ago

Could this be somehow related to new Chromium policies for accessing files? The code works fine when dealing with asar archives in the main process, but fails when loading the first file inside an asar in the renderer process.

Maybe Chromium prevents I/O to filesystem files from a WebView when the process is running as root?

I extracted resources/atom.asar to resources/atom and modified node_bindings.cc to access the extracted directory instead of the asar package:

// https://github.com/atom/electron/blob/master/atom/common/node_bindings.cc#L166

resources_path.Append(FILE_PATH_LITERAL("atom"))

But still module.js refuses to require the file, returning an 'internalModuleStat' of -13 (when the expected is 0).

arashpayan commented 8 years ago

I'd like to share my work around for this bug, in case it helps in debugging. Please take it with a grain of sodium chloride.

When I originally extracted the electron archive, it was on my desktop which has multiple user accounts, and the file owner was set to my personal user account (arash). When I transferred it (I don't remember if I tared then scped or just straight scp) to the test system where I first experienced this bug, there was only one account, root. ls showed the files as being owned by root, but I kept experiencing the bug from this thread. On a whim, I thought maybe the uid from my desktop computer was intact in the files and causing the problem.

To test my theory, I created another account on this test system named foo, and without doing anything else, running ls now showed the electron files as belonging to foo. I presume this was because the uid of the new foo user was the same as the user id of the account on my desktop. Running electron as foo worked just fine. So I chowned the files back to root and this error went away.

So maybe the invalid uid is causing some function to fail in Electron. It's still odd, because root should have access to everything.

jviotti commented 8 years ago

@arashpayan Thanks for the information.

Can you reproduce the issue without transferring files between systems, etc (by running on the same system where electron was initially downloaded)?

I've tried the chown suggestion but it sadly didn't work for me.

petrosagg commented 8 years ago

I reproduced this issue using @jviotti's gist on my machine.

node version: v5.4.1 npm version: 3.5.3 electron-prebuild version: 0.35.6 Distro: Arch linux natively AppArmor/SELinux: N/A Linux: 4.3.3-2-ARCH

I'm currently investigating two strace dumps to see the difference between running as root and running as a user.

arashpayan commented 8 years ago

@jviotti that's where it gets weird. If I run electron as root on my desktop where I originally extracted the files (Ubuntu 14.04), it still fails. That's why I was originally hesitant to present my work around.

jviotti commented 8 years ago

I found this closed issue about the exact same problem where the author came close to what we have here: https://github.com/atom/electron/issues/3528.

He mentions that the app works in his personal computer but not in the Raspberry Pi, but notice he mentions that the app is running as root on the Pi, which supports the argument that the issue is only present when running as root.

The guy mentions that Raspbian is able to run the app without any issues, but it fails with this error on Arch, which maybe indicates some dependency issue.

Maybe @szz-dvl might want to provide more information here.

jviotti commented 8 years ago

@zcbenz Could this be somehow related to Chromium's Linux SUID Sandboxing? According to https://chromium.googlesource.com/chromium/src/+/master/docs/linux_suid_sandbox.md:

With r20110, Chromium on Linux can now sandbox its renderers using a SUID helper binary. This is one of our layer-1 sandboxing solutions.

According to the Chromium docs (https://chromium.googlesource.com/chromium/src/+/master/docs/linux_suid_sandbox_development.md), there is an option to disable sandboxing:

--disable-setuid-sandbox

However this option doesn't seem to be supported as a Chromium switch by electron.

Can you instruct us on how to disable sandboxing programatically to test this?


UPDATE 1: I've tried setting CHROME_DEVEL_SANDBOX to an empty string as pointed out by the docs but this didn't make any difference.

jviotti commented 8 years ago

Building electron as root fixes the issue, but I have no idea why.

zcbenz commented 8 years ago

@jviotti The sandbox of Chromium is disabled in Electron, so it shouldn't be the cause.

Can you paste the value ofpath_.value()? Chromium does a check of whether path component contains .., and if it does the FILE_ERROR_ACCESS_DENIED error would be returned.

Refs: https://codereview.chromium.org/1469153006 https://codereview.chromium.org/1488873002

jviotti commented 8 years ago

@zcbenz The result of path_.value() is /home/jviotti/Projects/electron-current/out/D/resources/atom.asar, no ...

jviotti commented 8 years ago

I've modified Archive::Init to print the mode of path_.value() before checking for it's validity:

#include <sys/stat.h>

...

int getPathMode(std::string name) {
  struct stat buffer;   
  int ret = stat(name.c_str(), &buffer);
  if (ret < 0) return -1;
  return buffer.st_mode;
}

bool Archive::Init() {
  std::cout << getPathMode(path_.value()) << std::endl;
  if (!file_.IsValid()) {
    std::cout << "Archive is invalid: ("
              << file_.ErrorToString(file_.error_details())
              << "): "
              << path_.value()
              << std::endl;
    return false;
  }

...

This is the output when running my app as root (notice I packed the gist I linked above as resources/app.asar for convenience):

$ sudo ./out/D/electron 
33204
33204
(electron) loadUrl is deprecated. Use loadURL instead.
33204
-1
Archive is invalid: (FILE_ERROR_ACCESS_DENIED): /home/jviotti/electron-current/out/D/resources/atom.asar
-1
Archive is invalid: (FILE_ERROR_ACCESS_DENIED): /home/jviotti/electron-current/out/D/resources/atom.asar
-1
Archive is invalid: (FILE_ERROR_ACCESS_DENIED): /home/jviotti/electron-current/out/D/resources/atom.asar
-1
Archive is invalid: (FILE_ERROR_ACCESS_DENIED): /home/jviotti/electron-current/out/D/resources/atom.asar
-1
Archive is invalid: (FILE_ERROR_ACCESS_DENIED): /home/jviotti/electron-current/out/D/resources/atom.asar
-1
Archive is invalid: (FILE_ERROR_ACCESS_DENIED): /home/jviotti/electron-current/out/D/resources/atom.asar
-1
Archive is invalid: (FILE_ERROR_ACCESS_DENIED): /home/jviotti/electron-current/out/D/resources/atom.asar
-1
Archive is invalid: (FILE_ERROR_ACCESS_DENIED): /home/jviotti/electron-current/out/D/resources/atom.asar
-1
Archive is invalid: (FILE_ERROR_ACCESS_DENIED): /home/jviotti/electron-current/out/D/resources/atom.asar
-1
Archive is invalid: (FILE_ERROR_ACCESS_DENIED): /home/jviotti/electron-current/out/D/resources/atom.asar
-1
Archive is invalid: (FILE_ERROR_ACCESS_DENIED): /home/jviotti/electron-current/out/D/resources/atom.asar
-1
Archive is invalid: (FILE_ERROR_ACCESS_DENIED): /home/jviotti/electron-current/out/D/resources/atom.asar
-1
Archive is invalid: (FILE_ERROR_ACCESS_DENIED): /home/jviotti/electron-current/out/D/resources/atom.asar
-1
Archive is invalid: (FILE_ERROR_ACCESS_DENIED): /home/jviotti/electron-current/out/D/resources/atom.asar
-1
Archive is invalid: (FILE_ERROR_ACCESS_DENIED): /home/jviotti/electron-current/out/D/resources/atom.asar
-1
Archive is invalid: (FILE_ERROR_ACCESS_DENIED): /home/jviotti/electron-current/out/D/resources/atom.asar
[30652:0128/113606:ERROR:CONSOLE(340)] "Uncaught Error: Cannot find module '/home/jviotti/electron-current/out/D/resources/atom.asar/renderer/lib/init.js'", source: module.js (340)

Notice that the stat succeeds three times at the beginning, apparently before the index.html file gets loaded.

jviotti commented 8 years ago

I also noticed that the stat always succeeds on /home and /home/jviotti but not any nested directories like /home/jviotti/Projects despite root having full permissions on those of course.

As an experiment, I created a fake john account on my system, and turns out I can stat files inside /home/john without any issues. What could be going on?

jviotti commented 8 years ago

@zcbenz I found something! The newly created /home/john home directory had execution permissions, which /home/jviotti didn't. Adding executing permissions to my home directory fixes the issue:

chmod a+x /home/jviotti

Removing the execution permissions makes the issue appear again.

jviotti commented 8 years ago

I initially mentioned that this bug was not reproducible in an Ubuntu 14.04 virtual machine, which after checking, had execution permissions on the home directory by default. Under the assumption that execution permissions were the issue, I've removed execution permissions from my home directory in the virtual machine and the error didn't happen.

I also ran the same experiment in OS X and was not able to reproduce the issue.

I tried the execution permissions workaround in two real Linux boxes and that worked, so it definitely seems to be a Linux specific issue, but it doesn't happen on virtual machines. What could be the difference between running electron in both environments?

jviotti commented 8 years ago

I've made some progress regarding why the file is read successfully a couple of times before failing:

I can see that running my app in Electron forks three processes:

When running under root, the main process has a uid and gid of zero. The first three times that the file is opened is opened in original process that forks the other three. This process has full permissions over the system, and I can freely change my uid and gid with setuid and setgid correctly (as a way to check I indeed have permissions).

The rest of the times, the file is attempted to be opened in the two renderer processes. Since their parent process is the main process, they inherit the uid and gid of the parent (the main process), which inherits from the original process, but lack enough permissions for some reason (setuid returns -1, and they throw our ACCESS_DENIED error).

See more information about the three processes in this output from ps, where 23294 is the original process that forks the three ones, 23296 is the main process, and 23320 and 23321 are the renderer processes:

 PPID   PID  PGID   SID TTY      TPGID STAT   UID   TIME COMMAND
 6049 23293 23293  6049 pts/10   23293 S+       0   0:00          |   |   \_ sudo ./out/D/electron /home/jviotti/Downloads/7193eec573c62867151d-4e63199c2cf83a13e4a2301c61deec2616f9187b
23293 23294 23293  6049 pts/10   23293 Sl+      0   0:01          |   |       \_ /home/jviotti/electron-current/out/D/electron /home/jviotti/Downloads/7193eec573c62867151d-4e63199c2cf83a13e4a2301c61deec2616f9187b
23294 23296 23293  6049 pts/10   23293 S+       0   0:00          |   |           \_ /home/jviotti/electron-current/out/D/electron --type=zygote --no-sandbox
23296 23320 23293  6049 pts/10   23293 Sl+      0   0:00          |   |               \_ /home/jviotti/electron-current/out/D/electron --type=renderer --no-sandbox --lang=en-US --node-integration=true --num-raster-threads=2 --content-image-texture-target=3553,3553,3553,3553,3553,3553,3553,3553,3553,3553,3553,3553,3553 --video-image-texture-target=3553 --disable-accelerated-video-decode --disable-webrtc-hw-encoding --disable-gpu-compositing --channel=23294.0.921772104 --v8-natives-passed-by-fd --v8-snapshot-passed-by-fd
23296 23321 23293  6049 pts/10   23293 Sl+      0   0:00          |   |               \_ /home/jviotti/electron-current/out/D/electron --type=renderer --no-sandbox --lang=en-US --num-raster-threads=2 --content-image-texture-target=3553,3553,3553,3553,3553,3553,3553,3553,3553,3553,3553,3553,3553 --video-image-texture-target=3553 --disable-accelerated-video-decode --disable-webrtc-hw-encoding --disable-gpu-compositing --channel=23294.1.175272910 --v8-natives-passed-by-fd --v8-snapshot-passed-by-fd

@zcbenz Why are there two renderer process threads? Also, why is node-integration=true only enabled in one of them?


UPDATE 1: According to this SuperUser answer:

root is always uid 0, but uid 0 is not necessarily always root.

All the threads have a UID 0, and they show as ran by root under htop, so how can the access error occur?

UPDATE 2: I also tested what's the euid (effective group id) of all process, which is what programs use for checking permissions, and is zero in all cases as well.

petrosagg commented 8 years ago

After some strace digging I found the offending syscall. The threads that run the node code will do

capset({_LINUX_CAPABILITY_VERSION_3, 0}, {0, 0, 0})

which drops all the capabilities. This is ok when the process is running as a user that has access to every directory in the path leading to atom.asar, but it will fail if one directory isn't accessible. That's why doing chmod a+x on the home directory fixes the problem.

Here is an example C program that demonstrates the above

#include <sys/capability.h>
#include <stdio.h>
#include <fcntl.h>

#define FILENAME "/home/petrosagg/somefile"

int main () {
    struct __user_cap_header_struct hdr;
    struct __user_cap_data_struct data;

    hdr.version = _LINUX_CAPABILITY_VERSION;
    hdr.pid = 0;

    data.effective = 0;
    data.permitted = 0;
    data.inheritable = 0;

    if (capset(&hdr, &data) < 0)
        printf("capset failed");

    if (open(FILENAME, O_RDONLY) < 0)
        printf("open failed");

    return 0;
}

Change the FILENAME accordingly for your system and run it once with and once without sudo. Only without will work.

Now that we know what the problem is, I'm not sure what the fix should be. On one hand, it sounds too invasive to disable that syscall. On the other hand, standalone nodejs doesn't do that by default when running under root.

Some input from the maintainers would be highly appreciated.

jviotti commented 8 years ago

The sys call seems to proceed from chromium. There is a function called Credentials::DropAllCapabilitiesOnCurrentThread() that performs this:

// See https://code.google.com/p/chromium/codesearch#chromium/src/sandbox/linux/services/credentials.cc&q=DropAllCapabilitiesOnCurrentThread&sq=package:chromium&type=cs&l=173

// static
bool Credentials::DropAllCapabilitiesOnCurrentThread() {
  return SetCapabilitiesOnCurrentThread(std::vector<Capability>());
}

// static
bool Credentials::SetCapabilitiesOnCurrentThread(
    const std::vector<Capability>& caps) {
  struct cap_hdr hdr = {};
  hdr.version = _LINUX_CAPABILITY_VERSION_3;
  struct cap_data data[_LINUX_CAPABILITY_U32S_3] = {{}};

  // Initially, cap has no capability flags set. Enable the effective and
  // permitted flags only for the requested capabilities.
  for (const Capability cap : caps) {
    const int cap_num = CapabilityToKernelValue(cap);
    const size_t index = CAP_TO_INDEX(cap_num);
    const uint32_t mask = CAP_TO_MASK(cap_num);
    data[index].effective |= mask;
    data[index].permitted |= mask;
  }

  return sys_capset(&hdr, data) == 0;
}

This is itself used in a function called Credentials::ForkAndDropCapabilitiesInChild:

// See https://code.google.com/p/chromium/codesearch#chromium/src/sandbox/linux/services/credentials.cc&q=ForkAndDrop&sq=package:chromium&type=cs&l=325

pid_t Credentials::ForkAndDropCapabilitiesInChild() {
  pid_t pid = fork();
  if (pid != 0) {
    return pid;
  }

  // Since we just forked, we are single threaded.
  PCHECK(DropAllCapabilitiesOnCurrentThread());
  return 0;
}

Which is called by Zygote, which drops capabilities independently of sandbox settings:

// See https://code.google.com/p/chromium/codesearch#chromium/src/content/zygote/zygote_linux.cc&q=ForkAndDropCapabilitiesInChild&sq=package:chromium&type=cs&l=448

if (sandbox_flags_ & kSandboxLinuxPIDNS && sandbox_flags_ & kSandboxLinuxUserNS) {
   pid = sandbox::NamespaceSandbox::ForkInNewPidNamespace(/*drop_capabilities_in_child=*/true);
} else {
   pid = sandbox::Credentials::ForkAndDropCapabilitiesInChild();
}

I guess we could submit a patch to libchromiumcontent to not drop capabilities when sandbox is not enabled, although I'm not sure about the impact of such change.

zcbenz commented 8 years ago

Awesome research on this! Patching the ForkAndDropCapabilitiesInChild code looks good to me, we are not using sandbox at all so it doesn't hurt to give more capabilities to renderer process.

jviotti commented 8 years ago

@zcbenz Do you have any instructions on how to build electron with a local checkout of libchromiumcontent? I've made my changes to libchromiumcontent but having problems following the nested script/bootstrap calls, and the fact that brightray always seems to call the download script.

zcbenz commented 8 years ago

@jviotti You can find some instructions at https://github.com/atom/libchromiumcontent/issues/174#issuecomment-178409045, the key is to pass the file:// URL to the bootstrap script.

jviotti commented 8 years ago

@zcbenz Thanks for the instructions. The patch seems to be working great! See https://github.com/atom/libchromiumcontent/pull/180