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

Unable to use iBus (input method) #675

Closed
genggoro opened this issue Mar 30, 2017 · 112 comments
Closed

Unable to use iBus (input method) #675

genggoro opened this issue Mar 30, 2017 · 112 comments
Milestone

Comments

@genggoro
Copy link

@genggoro genggoro commented Mar 30, 2017

Is this simply because the GNOME runtime currently doesn't include im-ibus.so? If so, how are Flatpak apps supposed to work with the IM on the host system in general, considering that different people use different IMs?

@matthiasclasen

This comment has been minimized.

Copy link
Collaborator

@matthiasclasen matthiasclasen commented Mar 30, 2017

There is no 'supposed' way for this.
For gnome apps and gnome on the host, we will continue to rely on ibus as the framework that is well-integrated on the host side. So we need to make sure that im-ibus is available and enabled in the runtime

@alexlarsson

This comment has been minimized.

Copy link
Member

@alexlarsson alexlarsson commented Apr 4, 2017

Not only that, we also need to expose the host ibus in a safe way. I guess the clients talk to it using dbus? Is the apis safe for sandboxed use?

@matthiasclasen

This comment has been minimized.

Copy link
Collaborator

@matthiasclasen matthiasclasen commented Apr 4, 2017

I will look at this

@derekdai

This comment has been minimized.

Copy link

@derekdai derekdai commented Apr 4, 2017

Not only iBus, but also other input method engine such as scim, fcitx, ...

@matthiasclasen

This comment has been minimized.

Copy link
Collaborator

@matthiasclasen matthiasclasen commented Apr 5, 2017

Here is the org.freedesktop.IBus interface:

https://github.com/ibus/ibus/blob/master/bus/ibusimpl.c#L180

The client-side im module calls CreateInputContext to create the object that it then works with.
We probably want to not allow anything beyond that in the sandbox

Here is the org.freedesktop.IBus.InputContext interface:

https://github.com/ibus/ibus/blob/master/bus/inputcontext.c#L216

I haven't looked through this in detail yet

@juhp

This comment has been minimized.

Copy link

@juhp juhp commented Apr 6, 2017

@matthiasclasen

This comment has been minimized.

Copy link
Collaborator

@matthiasclasen matthiasclasen commented Apr 6, 2017

There's not much of any parameter validation in the input context code, that probably needs some improvements. Eg bus_input_context_set_capabilities passes the client-provided capabilities on to the engine without filtering out any extra bits that might be set. Same for the arguments of ProcessKeyEvent and SetCursorLocation.

It is not clear to me that the InputContext.SetEngine method is called from the client-side code. If it is not needed, maybe it shouldn't be exposed.

@fujiwarat

This comment has been minimized.

Copy link

@fujiwarat fujiwarat commented Apr 6, 2017

There's not much of any parameter validation in the input context code, that probably needs some improvements.

Sounds good to me.

It is not clear to me that the InputContext.SetEngine method is called from the client-side code. If it is not needed, maybe it shouldn't be exposed.

I think an idea of a parameter likes ibus-daemon --secure to keep API.

@epico

This comment has been minimized.

Copy link

@epico epico commented Apr 6, 2017

I think an idea of a parameter likes ibus-daemon --secure to keep API.

I think maybe good to share the ibus-daemon for the sandbox and native apps.

@matthiasclasen

This comment has been minimized.

Copy link
Collaborator

@matthiasclasen matthiasclasen commented Apr 6, 2017

I think an idea of a parameter likes ibus-daemon --secure to keep API.

I don't think that makes much sense, tbh. Would we run one ibus-daemon for sandboxed apps in 'secure' mode, and another one for the rest of the desktop ? If the 'secure' mode is good enough for all of the desktop, then why have the insecure mode at all ? Just drop the apis that are not needed, or add a smaller interface that we can safely expose to sandboxed apps

@epico

This comment has been minimized.

Copy link

@epico epico commented Apr 6, 2017

IIUC, only "CreateInputContext" method of "org.freedesktop.IBus" interface is allowed in the sandbox.

Also I think "BecomeMonitor" method of "org.freedesktop.DBus" interface should be disallowed.

@matthiasclasen

This comment has been minimized.

Copy link
Collaborator

@matthiasclasen matthiasclasen commented Apr 6, 2017

What do you mean ? Are you saying ibus already has special code to handle being inside a sandbox ? where is that code ?

@epico

This comment has been minimized.

Copy link

@epico epico commented Apr 6, 2017

Sorry, I just try to summarize my understanding, and made a spelling mistake, corrected it now.

@fujiwarat

This comment has been minimized.

Copy link

@fujiwarat fujiwarat commented Apr 6, 2017

then why have the insecure mode at all ?

Probably we will keep ibus_input_context_set_engine() and InputContext.SetEngine() until the version is bumped. Currently we have no plan to update the IBus version to 1.6 for a while.

@fujiwarat

This comment has been minimized.

Copy link

@fujiwarat fujiwarat commented Apr 6, 2017

I mean --secure option would be an idea if you like to drop the API immediately since currently we have no plan to bump the version.
Seems you don't bustle up the implementation so I cancel that idea now.

What do you mean ?

I think he put his idea not to allow CreateInputContext beyond that in the sandbox since "BecomeMonitor" method is used by dbus-monitor.

@matthiasclasen

This comment has been minimized.

Copy link
Collaborator

@matthiasclasen matthiasclasen commented Apr 6, 2017

Here is a concern that alex mentioned on irc:

clients probably get a path to the context object, and can easily guess other clients context paths - what happens if they make calls there ?

@matthiasclasen

This comment has been minimized.

Copy link
Collaborator

@matthiasclasen matthiasclasen commented Apr 6, 2017

I was misstating things a bit, I think.

We actually need to use a separate unique bus name to prevent the client from talking to other, 'unsafe' interfaces. So, the suggestion is have a separate bus connection, take a different well-known name, and expose limited, safe interfaces on that name. And strictly validate that clients only make calls on 'their' objects.

@matthiasclasen

This comment has been minimized.

Copy link
Collaborator

@matthiasclasen matthiasclasen commented Apr 6, 2017

Here is how the context object paths get created:

src/ibusshare.h:#define IBUS_PATH_INPUT_CONTEXT "/org/freedesktop/IBus/InputContext_%d"

bus/inputcontext.c: gchar *path = g_strdup_printf (IBUS_PATH_INPUT_CONTEXT, ++id);

@matthiasclasen

This comment has been minimized.

Copy link
Collaborator

@matthiasclasen matthiasclasen commented Apr 6, 2017

And I can't find any validation that would check that Inputcontext calls are coming from the owner of the context.

@matthiasclasen

This comment has been minimized.

Copy link
Collaborator

@matthiasclasen matthiasclasen commented Apr 6, 2017

An extra complication here is that ibus is using not just its own bus but its own daemon implementation which may or may not cause us extra problems when proxying - d-feet certainly has issues talking to it.

@epico

This comment has been minimized.

Copy link

@epico epico commented Apr 7, 2017

For d-feet, we filed a bug before and write a draft patch for it.

URL: https://bugzilla.redhat.com/show_bug.cgi?id=1111033

@fujiwarat

This comment has been minimized.

Copy link

@fujiwarat fujiwarat commented Apr 7, 2017

CC'ing @phuang

@matthiasclasen

This comment has been minimized.

Copy link
Collaborator

@matthiasclasen matthiasclasen commented Apr 7, 2017

That d-feet patch seems wrong. Instead of making d-feet connect to unsupported addresses, why not make ibus provide a supported one ? Or, if that is hard, make gio support the address in question.

You can already get d-feet to connect to ibus by doing

DBUS_SESSION_BUS_ADDRESS=$IBUS_ADDRESS d-feet

That is how I did my initial investigation of ibus protocol.

@matthiasclasen

This comment has been minimized.

Copy link
Collaborator

@matthiasclasen matthiasclasen commented Apr 7, 2017

From irc:

alexlarsson> mclasen: honestly, the right way would probably to use a per-client connection instead of a separate bus
mclasen> ok
alexlarsson> just do the create-context on the normal bus
then pass an fd to the client for dbus connection
mclasen> there's multiple contexts in an app
alexlarsson> and make sure you only allow access to that context from that fd
mclasen> I think
alexlarsson> so, one peer connection per unique id then
and then only handle the context:es for that connection from that connection
if you know what i mean

@matthiasclasen

This comment has been minimized.

Copy link
Collaborator

@matthiasclasen matthiasclasen commented Apr 10, 2017

A longer-term perspective for wayland is that this should really move to the compositor, and use the text protocol. That would solve the sandboxing issues for good.

@matthiasclasen

This comment has been minimized.

Copy link
Collaborator

@matthiasclasen matthiasclasen commented May 9, 2017

To clarify some of the last comments:

We want: one peer-to-peer connection per client (ie per application), not one per context (it turns out that clients generally use many contexts (one per text field)).

@fujiwarat

This comment has been minimized.

Copy link

@fujiwarat fujiwarat commented Jul 5, 2017

I tried to get a dbus address per client with the following way:
https://github.com/fujiwarat/ibus/commits/dedicated-bus

  1. Get a shared dbus address from $XDG_CONFIG_HOME/ibus/bus/*
  2. Connect ibus-daemon with the shared dbus address.
  3. Get a dedicted dbus address from the shared connection.
  4. Connect ibus-daemon with the dedicated dbus address.

But the packet is still readable by dbus-monitor with the shared dbus address.

@epico

This comment has been minimized.

Copy link

@epico epico commented Jul 7, 2017

I just read the comments of dbus-proxy recently.
Will ibus also use dbus-proxy for filtering?

Is it not secure to expose non-well-known names
to ibus client through dbus-proxy,
such as "/org/freedesktop/IBus/InputContext_%d"?

@fujiwarat

This comment has been minimized.

Copy link

@fujiwarat fujiwarat commented Jul 10, 2017

Will ibus also use dbus-proxy for filtering?

I don't know how to use dbus-proxy. I guess g_dbus_connection_add_filter() does not help to avoid dbus-monitor.

Is it not secure to expose non-well-known names to ibus client through dbus-proxy,

Right. But primarily I'm thinking to access by client.

@fujiwarat

This comment has been minimized.

Copy link

@fujiwarat fujiwarat commented Jul 12, 2017

I tried flatpak run org.gnome.gedit command and understood it runs /usr/libexec/flatpak-dbus-proxy internally.
My understanding is flatpak-dbus-proxy sets the policy of "org.gnome.gedit*" to FLATPAK_POLICY_OWN but seems the dbus methods whose destination is "org.gnome.gedit*" are called three times only.
I can see most packets with dbus-monitor --address 'unix:path=/run/user/$UID/bus' but don't see any differences between flatpak run org.gnome.gedit and flatpak --socket=session-bus run org.gnome.gedit, the latter works without any filters as far as I understand.

Is it a good example to test flatpak-dbus-monitor?

Is it a expected result that dbus-monitor can observe the most packets of ibus?
If yes, I do not understand what we try to restrict in the dbus connection but maybe my patch is fine at the moment.

@fujiwarat

This comment has been minimized.

Copy link

@fujiwarat fujiwarat commented Aug 24, 2017

This is no different than the current security model, other than possibly being a bit easier to implement. And there is really no way to fix that. If you're running unconstrained on the host you can do whatever you want, like attaching to ibus with gdb or whatever. Without sandboxing there can never be any security in this.

OK, I see. Probably I stop to think about it until any other frameworks will implement the sandboxes for the engine layers. Thank you.

For testing you can build some app like the test I have above. You have to bundle the apps you want to test though.

OK, I see.

I redid the patch slightly differently, fixing some bugs, and it now supports ibus restarts.
New version at: https://github.com/alexlarsson/ibus/tree/ibus-portal-2

Who launch ibus-portal again in your patch?
My understanding is, ibus-portal is activated by running ibus_bus_new_async_client() in the client applications but once ibus-portal exits with on_name_lost(), the clients wait for portal_name_appeared() until ibus-poral restarts.

@fujiwarat

This comment has been minimized.

Copy link

@fujiwarat fujiwarat commented Aug 25, 2017

I added a code to run ibus-portal by ibus-daemon so that ibus clients can reconnect when ibus-daemon restarts
https://github.com/fujiwarat/ibus/commits/ibus-portal-2

@alexlarsson

This comment has been minimized.

Copy link
Member

@alexlarsson alexlarsson commented Aug 25, 2017

@fujiwarat Ah, yeah, that looks good

@alexlarsson

This comment has been minimized.

Copy link
Member

@alexlarsson alexlarsson commented Aug 25, 2017

(In my testing i was manually restarting it)

@fujiwarat

This comment has been minimized.

Copy link

@fujiwarat fujiwarat commented Aug 25, 2017

@alexlarsson Thank you for your quick review.
I will start the code review of the patches on behalf of you CC'ing you next week. Of course, yourself also can start it with cd ibus; git log codereview.settings.
I will adjust 80 columns for every line to follow the ibus coding style.

fujiwarat added a commit to ibus/ibus that referenced this issue Aug 30, 2017
This adds a dbus service called org.freedesktop.portal.IBus on the
session bus. It is a very limited service that only implements
CreateInputContext and the InputContext interface (and Service.Destroy
for lifetime access).

It uses gdbus code generation for demarshalling the method calls which
means it will verify that all arguments have the right type.

Additionally all method calls to the input context object have to be
from the client that created it, so each client is isolated.

BUG=flatpak/flatpak#675
R=Shawn.P.Huang@gmail.com

Review URL: https://codereview.appspot.com/326350043

Patch from Alexander Larsson <alexl@redhat.com>.
fujiwarat added a commit to ibus/ibus that referenced this issue Aug 31, 2017
This adds a new way to create an IbusBus, ibus_bus_new_async_client().
This returns an object that is not guarantee to handle any calls
that are not needed by a client, meaning CreateInputContext and
handling the input context.

If you are running in a flatpak, or if IBUS_USE_PORTAL is set, then
instead of talking to the regular ibus bus we connect to
org.freedesktop.portal.IBus on the session bus and use the
limited org.freedesktop.IBus.Portal interface instead of the
org.freedesktop.IBus interface.

This allows flatpaks (or other sandbox systems) to safely use
dbus clients (apps).

BUG=flatpak/flatpak#675

Review URL: https://codereview.appspot.com/328410043

Patch from Alexander Larsson <alexl@redhat.com>.
@alexlarsson

This comment has been minimized.

Copy link
Member

@alexlarsson alexlarsson commented Sep 1, 2017

I built the gtk3 im-module into the freedesktop runtime now. I guess we should add the gtk2 one to the gnome runtime also.

@fujiwarat

This comment has been minimized.

Copy link

@fujiwarat fujiwarat commented Sep 4, 2017

I built the gtk3 im-module into the freedesktop runtime now. I guess we should add the gtk2 one to the gnome runtime also.

Yes. Also /usr/libexec/ibus-x11 and /usr/lib64/qt5/plugins/platforminputcontexts/libibusplatforminputcontextplugin.so too.

fujiwarat added a commit to ibus/ibus that referenced this issue Sep 4, 2017
Test with:
flatpak-builder --force-clean app org.test.IBus.json
flatpak-builder --run --nofilesystem=host app org.test.IBus.json zenity --entry

BUG=flatpak/flatpak#675
R=Shawn.P.Huang@gmail.com

Review URL: https://codereview.appspot.com/329090043

Patch from Alexander Larsson <alexl@redhat.com>.
fujiwarat added a commit to ibus/ibus that referenced this issue Sep 4, 2017
When ibus-daemon restarts, ibus-portal exits with on_name_lost() and
the clients wait for portal_name_appeared() until ibus-poral restarts.
Now the clients can connect to ibus-daemon with this way and also
they don't have to activate ibus-portal.

BUG=flatpak/flatpak#675
R=Shawn.P.Huang@gmail.com

Review URL: https://codereview.appspot.com/321530043
@alexlarsson

This comment has been minimized.

Copy link
Member

@alexlarsson alexlarsson commented Sep 4, 2017

What exactly does ibus-x11 do?

@fujiwarat

This comment has been minimized.

Copy link

@fujiwarat fujiwarat commented Sep 5, 2017

ibus-x11 communicates with X clients and ibus-daemon for the key events with XIM and IBus protocols.
X application (X11 or Java) <=> X server <=> ibus-x11 <=> ibus-daemon
You have to set XMODIFIERS environment variable. E.g. env XMODIFIERS=@im=ibus xterm

@fujiwarat

This comment has been minimized.

Copy link

@fujiwarat fujiwarat commented Sep 15, 2017

Now Fedora IBus RPM includes ibus-portal.
Do Fedora and upstream Flatpak enable IBus?

@alexlarsson

This comment has been minimized.

Copy link
Member

@alexlarsson alexlarsson commented Sep 18, 2017

The very latest runtimes should have ibus support, yes.

@fujiwarat

This comment has been minimized.

Copy link

@fujiwarat fujiwarat commented Sep 21, 2017

Thank you. I forgot to copy ibusimcontext.c to client/gtk3 in fedora build.

@juhp

This comment has been minimized.

Copy link

@juhp juhp commented Nov 14, 2017

Which are the first flatpak and ibus releases that support this?

@alexlarsson

This comment has been minimized.

Copy link
Member

@alexlarsson alexlarsson commented Nov 16, 2017

@juhp Ibus 1.5.17, and it is independent on the flatpak version, but needs a recent flatpak runtime with the ibus version in it (should be ok with e.g. freedesktop.org 1.6 runtimes and gnome >= 3.24 runtimes).

@chenzhiwei

This comment has been minimized.

Copy link

@chenzhiwei chenzhiwei commented Oct 12, 2018

Hello, I still can't use ibus in Slack flatpak app, I think both ibus and Slack flat app are latest version:

  • ibus
zhiwei@deepin:~$ ibus version
IBus 1.5.18
zhiwei@deepin:~$ dpkg -l | grep ibus
ii  gir1.2-ibus-1.0:amd64                                  1.5.18-1                             amd64        Intelligent Input Bus - introspection data
ii  ibus                                                   1.5.18-1                             amd64        Intelligent Input Bus - core
ii  ibus-clutter:amd64                                     0.0+git20090728.a936bacf-5.1+b2      amd64        ibus input method framework for clutter
ii  ibus-gtk:amd64                                         1.5.18-1                             amd64        Intelligent Input Bus - GTK+2 support
ii  ibus-gtk3:amd64                                        1.5.18-1                             amd64        Intelligent Input Bus - GTK+3 support
ii  ibus-qt4                                               1.3.3-1+b1                           amd64        qt-immodule for ibus (QT4) (plugin)
ii  ibus-rime                                              1.2-1+b1                             amd64        Rime Input Method Engine for IBus
ii  libgusb2:amd64                                         0.2.11-1                             amd64        GLib wrapper around libusb1
ii  libibus-1.0-5:amd64                                    1.5.18-1                             amd64        Intelligent Input Bus - shared library
ii  libibus-qt1                                            1.3.3-1+b1                           amd64        qt-immodule for ibus (QT4) (library)
  • com.slack.Slack
zhiwei@deepin:~$ flatpak info com.slack.Slack 
Ref: app/com.slack.Slack/x86_64/stable
ID: com.slack.Slack
Arch: x86_64
Branch: stable
Origin: flathub
Date: 2018-10-08 13:27:56 +0000
Subject: Update to 3.3.3 (9dc0519c)
Commit: af79c798127613ed8f87418ee2406b1ef52c3781d8a054ce727a08cd5409b0fb
Parent: 597977f6d80c3df3c890df167b30240c6f503da12172fbf8fc2782402f91f651
Location: /var/lib/flatpak/app/com.slack.Slack/x86_64/stable/af79c798127613ed8f87418ee2406b1ef52c3781d8a054ce727a08cd5409b0fb
Installed size: 2.8 MB
Runtime: org.freedesktop.Platform/x86_64/18.08

There Slack ibus/bus directory was created .var/app/com.slack.Slack/config/ibus/bus/ but nothing inside, and I tried to cp .config/ibus/bus/312142facf6a41b9b8ea773d842a66de-unix-0 .var/app/com.slack.Slack/config/ibus/bus/, but still can't use ibus.

Update: I can use ibus in org.gnome.gedit flat app.

Can someone give me some advice about this issue?

@hadess

This comment has been minimized.

Copy link
Contributor

@hadess hadess commented Oct 12, 2018

Can someone give me some advice about this issue?

This bug has been closed for a year, and there's a link about Electron apps just above your comment. Following that link would be a good first step. File a separate bug if your issue is a separate problem.

@laichiaheng

This comment has been minimized.

Copy link

@laichiaheng laichiaheng commented Oct 15, 2018

Why is it closed, I still can't input Chinese in the flatpak version of OBS Studio, Steam, whatever else.
OS: Ubuntu 18.04.1

@hadess

This comment has been minimized.

Copy link
Contributor

@hadess hadess commented Oct 15, 2018

Why is it closed, I still can't input Chinese in the flatpak version of OBS Studio, Steam, whatever else.

It's closed because the IBus support got mrged. If you have further problems, file a bug. I'm guessing that in this case, you should file an issue against your distribution, as IBus input works on other distributions.

@jeffshee

This comment has been minimized.

Copy link

@jeffshee jeffshee commented Nov 7, 2018

Having the same issue as well, on Fedora 29.
Filed a bug to Red Hat Bugzilla already.
https://bugzilla.redhat.com/show_bug.cgi?id=1647627

@juhp

This comment has been minimized.

Copy link

@juhp juhp commented Nov 8, 2018

Let's move discussion on the Electron apps issue to #1671.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
You can’t perform that action at this time.