Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Handle XDG paths and WebKit sandboxing the right way. #1781

Closed
aartaka opened this issue Sep 9, 2021 · 19 comments
Closed

Handle XDG paths and WebKit sandboxing the right way. #1781

aartaka opened this issue Sep 9, 2021 · 19 comments
Labels
3-series Related to releases whose major version is 3. high security

Comments

@aartaka
Copy link
Contributor

aartaka commented Sep 9, 2021

In #1505, we moved the path for WebKit extensions from .config/nyxt/extensions/ to .local/share/nyxt/extensions/. After this, a warning emerged on Nyxt launch (text copied from #1505 (comment)):

** (nyxt:2571): WARNING **: 15:52:49.141: GApplication is required for xdg-desktop-portal access in the WebKit sandbox. Actions that require xdg-desktop-portal will be broken.

After a brief Internet search, I've followed the approach of setting WEBKIT_FORCE_SANDBOX to 0 in 48ac0d8. While not exactly a fine move, it made extensions to work again.

I have two concerns there:

  1. If we don't force sandboxing, it can cause harm to our users.
  2. Forcing sandboxing breaks extension-loading vital for WebExtensions API support #1762 and usage of third-party WebKit extensions, though.

But did extension loading work in the first place? It seemed to work on simple examples and .config/nyxt/extensions/ directory, but it was a long time ago.

@Ambrevar
Copy link
Member

I suggest we add an "extension loading" test, e.g. with a dummy "hello world" extension. This is prone to break again in the future and I'm guessing from your work that extension support will soon be critical.

@aartaka
Copy link
Contributor Author

aartaka commented Sep 10, 2021

Yes, that seems to be the way to go. cl-webkit has a skeleton extension to start from.

@aartaka
Copy link
Contributor Author

aartaka commented Oct 13, 2021

@aartaka
Copy link
Contributor Author

aartaka commented Oct 13, 2021

K, creating GApplication from Lisp is actually possible, but I haven't succeeded in making it work for sandboxing, even with lots of GTK/GNOME tutorials at hand. @Ambrevar, @jmercouris, maybe you'll have better luck. I'll move to other issues for some time.

@Ambrevar
Copy link
Member

Do you have some code snippet to share?

@aartaka
Copy link
Contributor Author

aartaka commented Oct 18, 2021

This is how I've rewritten ffi-initialize to try registering GApplication:

(defmethod ffi-initialize ((browser gtk-browser) urls startup-timestamp)
  "gtk:within-main-loop handles all the GTK initialization. On
   GNU/Linux, Nyxt could hang after 10 minutes if it's not
   used. Conversely, on Darwin, if gtk:within-main-loop is used, no
   drawing happens. Drawing operations on Darwin MUST originate from
   the main thread, which the GTK main loop is not guaranteed to be
   on."
  (log:debug "Initializing GTK Interface")
  ;; (setf (uiop:getenv "WEBKIT_FORCE_SANDBOX") "0")
  ;; 40 = 32 + 8
  ;; 32 is the flag for G_APPLICATION_NON_UNIQUE, allowing several instances to
  ;; run simultaneously.
  ;; 8 is the flag for G_APPLICATION_HANDLES_COMMAND_LINE.
  (let ((app (gio:g-application-new "engineer.Atlas.Nyxt" 40)))
    (gio:g-application-register app (cffi:null-pointer))
    ;; the rest of the function body, unchanged
    (gio:g-application-run app 0 (cffi:null-pointer))))

This was not sufficient. Even though there were no more warnings on the command line, the extensions didn't load. I digged into the GApplication docs and found these three examples that hint at the fact that GApplication is to be created at the program start and to handle all the CLI args. While this is possible (even though painful) to do in Lisp, it breaks the renderer-agnosticism, as we need to start the browser in a totally renderer-dependent fashion.

I thought that GtkApplication can help there, but it's not much further from GApplication in what it provides, and is as restrictive and GApplication.

They somewhy advice to not enable sandboxing in wyebadblock (an adblock extension for WebKit browsers) README. Maybe that's exactly because it complicates extension loading by a magnintude 🤔

@aartaka
Copy link
Contributor Author

aartaka commented Oct 20, 2021

Adding to the wyebadblock info: most WebKit browsers use /usr/lib/browsername/ as the extension directory. This is outside XDG paths, so should work better. However, this is a directory that's out of non-sudo user reach.

Maybe GTK actually has a way to claim rights for /usr/lib/ subdirs? If so, this should be significantly easier to change one directory than rewriting the core logic.

@Ambrevar
Copy link
Member

Ambrevar commented Oct 20, 2021 via email

@aartaka
Copy link
Contributor Author

aartaka commented Oct 20, 2021

How would that work on non-FHS distributions like Nix and Guix?

No idea 0_o

@jian-lin
Copy link

On nixos, we still have to disable sandbox WEBKIT_FORCE_SANDBOX=0 to open a url. See NixOS/nixpkgs#158005. I think it's related to this issue.

I am not familiar with webkit, but generally disabling sandbox for a browser is not a good idea. Is there any workarounds like disabling extensions?

@jmercouris
Copy link
Member

That's fascinating. Nyxt works just fine on my NixOS installation. Then again, I compiled from source and am using a nix-shell. Perhaps there is something else awry here. Maybe it /will/ be fixed by this issue.

@kenranunderscore
Copy link
Contributor

For the record, I'm on NixOS (recent unstable version) as well, and I sadly cannot use nyxt the way I'd like either yet, that is, by getting it as pkgs.nyxt and not having to force-disable the sandboxing. I'll check a compiled Nyxt now as per @jmercouris's comment (it should just build using the shell.nix)!

@OliverEvans96
Copy link

I'm having the same issue as the other commenters on NixOS.

@kenranunderscore
Copy link
Contributor

Ah, I forgot updating here. I've chosen to build from source as well and as John said above this works just fine.

So this really seems to be an issue of the nixified build/install in nixpkgs and not an issue with Nyxt per se.

@jmercouris
Copy link
Member

I think to resolve this reliably, we'll have to use the feature macro.

@aadcg aadcg added the high label May 31, 2023
@aartaka
Copy link
Contributor Author

aartaka commented Jun 30, 2023

Note that for the systems that resolve extension directory to somewhere inside homedir, this snippet forces extension search outside it:

(defmethod files:resolve ((profile nyxt-profile) (file nyxt/renderer/gtk::gtk-extensions-directory))
  "The path to look for extensions at.
If it's beyond /home/username/, then you don't have to disable
sandboxing."
  #p"/usr/lib/nyxt/")

@magackame
Copy link

magackame commented Jun 30, 2023

Note that for the systems that resolve extension directory to somewhere inside homedir, this snippet forces extension search outside it:

(defmethod files:resolve ((profile nyxt-profile) (file nyxt/renderer/gtk::gtk-extensions-directory))
  "The path to look for extensions at.
If it's beyond /home/username/, then you don't have to disable
sandboxing."
  #p"/usr/lib/nyxt/")

This snippet goes into ~/.config/nyxt/config.lisp, right?

I tried it and get:

Nyxt version 3.3.0
<INFO> [02:07:45] Source location: #P"/usr/share/nyxt/"
<INFO> [02:07:45] Loading Lisp file #P"/home/mako/.config/nyxt/config.lisp".
WARNING:
   redefining FILES:RESOLVE (#<STANDARD-CLASS NYXT:NYXT-PROFILE>
                             #<STANDARD-CLASS NYXT/RENDERER/GTK:GTK-EXTENSIONS-DIRECTORY>) in DEFMETHOD
<INFO> [02:07:45] Listening to socket: #P"/home/mako/tmp/nyxt/nyxt.socket"

** (nyxt:9914): ERROR **: 02:07:46.005: Unable to configure xdg-desktop-portal access in the WebKit sandbox: GApplication is required.
<WARN> [02:07:46] Restarting with ("nyxt" "--failsafe").
<INFO> [02:07:46] Deleting socket #P"/home/mako/tmp/nyxt/nyxt.socket".
mako% <INFO> [02:07:46] Source location: #P"/usr/share/nyxt/"
<INFO> [02:07:46] Profile: "nofile"
<WARN> [02:07:46] Warning: Error in s-exp evaluation: There is no applicable method for the generic function
  #&lt;STANDARD-GENERIC-FUNCTION NYXT:AFTER-INIT-HOOK (1)&gt;
when called with arguments
  (NIL).
See also:
  The ANSI Standard, Section 7.6.6
mako% Backtrace for: #<SB-THREAD:THREAD tid=10127 "Nyxt finalize-buffer" RUNNING {1008828613}>
0: ((LAMBDA NIL :IN UIOP/IMAGE:PRINT-BACKTRACE))
1: ((FLET "THUNK" :IN UIOP/STREAM:CALL-WITH-SAFE-IO-SYNTAX))
2: (SB-IMPL::%WITH-STANDARD-IO-SYNTAX #<FUNCTION (FLET "THUNK" :IN UIOP/STREAM:CALL-WITH-SAFE-IO-SYNTAX) {7F31508855BB}>)
3: (UIOP/STREAM:CALL-WITH-SAFE-IO-SYNTAX #<FUNCTION (LAMBDA NIL :IN UIOP/IMAGE:PRINT-BACKTRACE) {100DDB399B}> :PACKAGE :CL)
4: ((:METHOD LOG4CL-IMPL:APPENDER-DO-APPEND (NYXT::MESSAGES-APPENDER T T T)) #<NYXT::MESSAGES-APPENDER {10071749E3}> #<LOG4CL-IMPL::FILE-LOGGER NYXT.ECHO-WARNING.FORM-FUN-5 /builddir/nyxt-3.3.0/source/message.lisp {1002E8EE03}> 3 #<FUNCTION (FLET "log-stmt8" :IN NYXT:ECHO-WARNING) {7F3150885A1B}>) [fast-method]
5: ((LABELS LOG4CL-IMPL::LOG-TO-LOGGER-APPENDERS :IN LOG4CL-IMPL::LOG-WITH-LOGGER) #<LOGGER +ROOT+ {1001564113}> #<LOG4CL-IMPL::FILE-LOGGER NYXT.ECHO-WARNING.FORM-FUN-5 /builddir/nyxt-3.3.0/source/message.lisp {1002E8EE03}> 3 #<FUNCTION (FLET "log-stmt8" :IN NYXT:ECHO-WARNING) {7F3150885A1B}>)
6: ((LABELS LOG4CL-IMPL::LOG-TO-LOGGER-APPENDERS :IN LOG4CL-IMPL::LOG-WITH-LOGGER) #<LOG4CL-IMPL::FILE-LOGGER NYXT NIL {10039CAEF3}> #<LOG4CL-IMPL::FILE-LOGGER NYXT.ECHO-WARNING.FORM-FUN-5 /builddir/nyxt-3.3.0/source/message.lisp {1002E8EE03}> 3 #<FUNCTION (FLET "log-stmt8" :IN NYXT:ECHO-WARNING) {7F3150885A1B}>)
7: ((LABELS LOG4CL-IMPL::LOG-TO-LOGGER-APPENDERS :IN LOG4CL-IMPL::LOG-WITH-LOGGER) #<LOG4CL-IMPL::FILE-LOGGER NYXT.ECHO-WARNING /builddir/nyxt-3.3.0/source/message.lisp {10039DC913}> #<LOG4CL-IMPL::FILE-LOGGER NYXT.ECHO-WARNING.FORM-FUN-5 /builddir/nyxt-3.3.0/source/message.lisp {1002E8EE03}> 3 #<FUNCTION (FLET "log-stmt8" :IN NYXT:ECHO-WARNING) {7F3150885A1B}>)
8: ((LABELS LOG4CL-IMPL::LOG-TO-LOGGER-APPENDERS :IN LOG4CL-IMPL::LOG-WITH-LOGGER) #<LOG4CL-IMPL::FILE-LOGGER NYXT.ECHO-WARNING.FORM-FUN-5 /builddir/nyxt-3.3.0/source/message.lisp {1002E8EE03}> #<LOG4CL-IMPL::FILE-LOGGER NYXT.ECHO-WARNING.FORM-FUN-5 /builddir/nyxt-3.3.0/source/message.lisp {1002E8EE03}> 3 #<FUNCTION (FLET "log-stmt8" :IN NYXT:ECHO-WARNING) {7F3150885A1B}>)
9: (LOG4CL-IMPL::LOG-WITH-LOGGER #<LOG4CL-IMPL::FILE-LOGGER NYXT.ECHO-WARNING.FORM-FUN-5 /builddir/nyxt-3.3.0/source/message.lisp {1002E8EE03}> 3 #<FUNCTION (FLET "log-stmt8" :IN NYXT:ECHO-WARNING) {7F3150885A1B}> #<PACKAGE "NYXT">)
10: (NYXT:ECHO-WARNING "Error in FFI method: ~a" "The value
  :INVALID-CODE-OBJECT-AT-PC
is not of type
  (SIMPLE-ARRAY (SIGNED-BYTE 64) (*))")
11: ((LAMBDA (#:C270) :IN NYXT:FFI-PRINT-STATUS) #<TYPE-ERROR expected-type: (SIMPLE-ARRAY (SIGNED-BYTE 64) (*)) datum: :INVALID-CODE-OBJECT-AT-PC>)
12: (SB-KERNEL::%SIGNAL #<TYPE-ERROR expected-type: (SIMPLE-ARRAY (SIGNED-BYTE 64) (*)) datum: :INVALID-CODE-OBJECT-AT-PC>)
13: (ERROR #<TYPE-ERROR expected-type: (SIMPLE-ARRAY (SIGNED-BYTE 64) (*)) datum: :INVALID-CODE-OBJECT-AT-PC>)
14: ((LAMBDA (CONDITION) :IN NYXT:FFI-PRINT-STATUS) #<TYPE-ERROR expected-type: (SIMPLE-ARRAY (SIGNED-BYTE 64) (*)) datum: :INVALID-CODE-OBJECT-AT-PC>)
15: ((LAMBDA (NYXT:BROWSER) :IN NYXT:CUSTOMIZE-INSTANCE) #<unused argument>)
16: ((:METHOD (SETF NYXT:BUFFERS) :AFTER (T NYXT:BROWSER)) #<unused argument> #<NYXT:BROWSER {1006513573}>) [fast-method]
17: ((SB-PCL::EMF (SETF NYXT:BUFFERS)) #<unused argument> #<unused argument> #<HASH-TABLE :TEST EQL :COUNT 1 {10067342F3}> #<NYXT:BROWSER {1006513573}>)
18: ((:METHOD NYXT:CUSTOMIZE-INSTANCE :AFTER (NYXT:CONTEXT-BUFFER)) #<NYXT:WEB-BUFFER 6907 {1008A1E1A3}> :PARENT-BUFFER NIL :NO-HISTORY-P T) [fast-method]
19: ((SB-PCL::EMF NYXT:CUSTOMIZE-INSTANCE) #<unused argument> #<unused argument> #<NYXT:WEB-BUFFER 6907 {1008A1E1A3}> :TITLE "" :EXTRA-MODES NIL :PARENT-BUFFER NIL :NO-HISTORY-P T :HISTORY-FILE #<NYXT:HISTORY-FILE {1008996223}> :URL #<QURI.URI:URI nyxt:new>)
20: ((:METHOD MAKE-INSTANCE :AROUND (NYXT::USER-MIXIN-CLASS)) #<NYXT:USER-CLASS NYXT:WEB-BUFFER> :TITLE "" :EXTRA-MODES NIL :PARENT-BUFFER NIL :NO-HISTORY-P T :HISTORY-FILE #<NYXT:HISTORY-FILE {1008996223}> :URL #<QURI.URI:URI nyxt:new>) [fast-method]
21: ((:METHOD NYXT:MAKE-BUFFER NIL) :URL #<QURI.URI:URI nyxt:new> :NO-HISTORY-P T) [fast-method]
22: ((:METHOD NYXT::FINALIZE-FIRST-BUFFER (NYXT:BROWSER T)) #<NYXT:BROWSER {1006513573}> NIL) [fast-method]
23: ((LAMBDA NIL :IN NYXT::FINALIZE-WINDOW))
24: ((LAMBDA NIL :IN BORDEAUX-THREADS::BINDING-DEFAULT-SPECIALS))
25: ((FLET SB-UNIX::BODY :IN SB-THREAD::RUN))
26: ((FLET "WITHOUT-INTERRUPTS-BODY-156" :IN SB-THREAD::RUN))
27: ((FLET SB-UNIX::BODY :IN SB-THREAD::RUN))
28: ((FLET "WITHOUT-INTERRUPTS-BODY-149" :IN SB-THREAD::RUN))
29: (SB-THREAD::RUN)
30: ("foreign function: call_into_lisp_")
31: ("foreign function: funcall1")
<WARN> [02:07:46] Warning: Error in FFI method: The value
  :INVALID-CODE-OBJECT-AT-PC
is not of type
  (SIMPLE-ARRAY (SIGNED-BYTE 64) (*))

The Nyxt window reopens twice and that's it.

Or it won't help on WebKitGTK 2.40.0 as said here?

@aadcg
Copy link
Member

aadcg commented Jul 1, 2023

it won't help on WebKitGTK 2.40.0 as said here?

Won't help. This issue deals with a different topic.

@aadcg
Copy link
Member

aadcg commented Dec 18, 2023

Sandboxing is enabled by default in WebKitGTK and disabling it is dangerous. See WebKit/WebKit#8591.

The experimental support for WebExtension has been dropped.

Nyxt enables sandboxing and the claim can be checked via webkit:webkit-web-context-get-sandbox-enabled.

@aadcg aadcg closed this as completed Dec 18, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3-series Related to releases whose major version is 3. high security
Development

No branches or pull requests

8 participants