tauri icon indicating copy to clipboard operation
tauri copied to clipboard

[feat] Inject window.__TAURI__ in allowed remote URLs

Open Sytten opened this issue 3 years ago • 18 comments

Describe the problem

Currently it is not really possible to communicate directly with tauri from an external website. There is a workaround if you have a static external URL you want to load, but it does not work if you need to inject in arbitrary URLs.

In our case, we have a first window where the user can manage their connection. They can then connect to a remote server which provides the UI. That UI needs to communicate with tauri to store credentials on the user machine, access notifications, etc.

Describe the solution you'd like

Basically I would like a setting in the WindowBuilder to allow injection of the window.__TAURI__ in any website loaded in that window. I do understand that it can be security risk if people are not careful so that could be gated with a feature flag or a general configuration option that needs to be enabled (in the context for example) before the setting has any effect.

Alternatives considered

We are currently investigating our options:

  • Create a local server so the remote url can talk to it and in turn execute stuff in the rust context
  • Use a static wrapper page with an iframe that loads the remote URL so that the static page can relay the messages from the iframe to tauri

Neither are very elegant solutions and I would prefer to be able to communicate directly with tauri.

Additional context

No response

Sytten avatar Aug 29 '22 15:08 Sytten

It looks like __TAURI_POST_MESSAGE__ is always defined and can be used to send messages. This should probably be documented? It feels like a security loophole.

Sytten avatar Aug 29 '22 18:08 Sytten

Yeah it's not supposed to be defined. We just forget to remove it again and again x)

It's not really a security loophole tho because the webview's ipc channel is always defined and tauri_post_message is nothing more than a really slim wrapper around that https://github.com/tauri-apps/tauri/blob/dev/core/tauri/src/app.rs#L954

FabianLars avatar Aug 29 '22 18:08 FabianLars

Yeah that is what I saw, I think my thing works because I start the window as a "local" window so the IPC handler is injected https://github.com/tauri-apps/tauri/blob/2901145c497299f033ba7120af5f2e7ead16c75a/core/tauri/src/manager.rs#L1125-L1135 and then I redirect to the external URL so the IPC handler is still valid. This is probably a "bug" but I am happy to exploit it until I can get a real __TAURI__.

Sytten avatar Aug 29 '22 18:08 Sytten

For reference we are doing:

 let url = WindowUrl::App("/invalid".into());
 WindowBuilder::new(manager, "label", url).visible(false).maximized(true)
...
let main_window = window.get_window("label").unwrap()
let url = format!("http://{}:{}/", &host, port);
main_window.redirect(&url);

Sytten avatar Aug 29 '22 18:08 Sytten

Interesting, that was indeed a bug in beta which we fixed in a release candidate version (proabably the first one as part of the audit, not sure) 🤔

FabianLars avatar Aug 29 '22 19:08 FabianLars

If you are going to fix it, I would very much like to be involved since we need the escape hatch for our application. Happy to contribute coding wise for it. Feel free to ping me on discord.

Sytten avatar Aug 29 '22 19:08 Sytten

Just to be clear, the __TAURI__ property is removed on redirect, but the underlining window.ipc is still present and the handler is still the same so you can send commands/events to the tauri application. This works because the is_local variable was true at the time of window creation but is not checked again on direct so the ipc handler is not changed. This is problematic if the application has an open redirect as the attacker could talk with tauri directly. In our case we designed our commands to be safe and we benefit rom this behaviour but I don't think it's correct.

Sytten avatar Aug 29 '22 20:08 Sytten

Yeah it's correct that this is an security issue, which will get priority for a fix. We will probably (completely/temporarily) prevent such behavior in the next release/security patch, unless there is a feature addition available to optionally allow third party API access. We are completely open to this change but will prioritize the security of all users.

As you suggested to help out with contributions on this issue and to keep your application working in a similar fashion, it would be great if you could contribute a (draft) PR for this added functionality. If you have questions, comments, remarks etc. you can always ask in the contributor channels in discord.

For your use case would it be sufficient to have your 'trusted' external URLs in an allowlist addition in the security configuration and then toggle a configuration flag like dangerousExternalCommandAccess to enable this access for the configured sites in the scope? Obviously this requires changes in several places in the core functionality.

Example of the addition:

"dangerousExternalCommandAccess": {
         "scope": [
             {
               "name": "mytrustedcompany",
               "url": "https://www.mytrustedcompany.com",
              },
         ]
 }

This could be used to determine if the current host of the webview window is allowed to interact with the API.

On another note: Would you mind sharing how you currently workaround this?

tweidinger avatar Sep 05 '22 03:09 tweidinger

On another note: Would you mind sharing how you currently workaround this?

Idk about their case, but the easiest thing, if you don't wanna reconstruct tauri's request style, is to just inject the core.js file manually.

FabianLars avatar Sep 05 '22 09:09 FabianLars

Thanks for the reply. Currently we didn't ship the feature yet so this fix won't impact us but it will prevent us from moving forward. I also think security is more important.

We do not know the urls at build time as they are entered by the user at runtime. So I don't think the proposal laid out will work for us. I think the idea of a flag in the configuration is good, but the whitelist need to be configurable per window at runtime and must allow a wildcard ideally. I will draft a PR for what we would need and we can discuss from there I think.

Note that must injecting the core.js script wont be enough once the patch for IPC will be released. Really the JS part is super basic, what we need is the window.ipc.

Sytten avatar Sep 05 '22 13:09 Sytten

I think this scope config would be a good fit for tauri v1, because we said time and time again that the ipc doesn't work on external pages (even tho it does) and that we don't support browser-like use-cases yet.

So this way we can fix v1 to match the behavior we always presented, and remove the need for weird workarounds for most use-cases to not make it a super harsh breaking change. And then we can tackle more loose scopes for v2, like planned.

FabianLars avatar Sep 05 '22 13:09 FabianLars

Agreed, I will move toward that solution. But I will accept wildcards in the urls so it is easy for us to just accept every external website. I think there could be a better implementation for v2 to allow users to add dynamic checks at runtime but this will be good enough for now.

Sytten avatar Sep 05 '22 14:09 Sytten

But I will accept wildcards in the urls so it is easy for us to just accept every external website

I should have made that clearer in my previous comment, but the problem i see here is that the v1 audit was building on top of the "no apis on external sites" premise. The actual fix for this would be to disable the ipc for external sites without an opt-in config. But because quite a few people depend on this (even though we said again and again that it "doesn't work"), the scope @tweidinger prooposed is already quite generous (sounds super entitled but i don't know a better word) in the context of a v1 feature release, and i think we can't completely open it up (via wildcards) in a non-major release because this really looks like something that needs a large external audit (as it was planned all along). Btw v2 is not that far in the future, we're not talking about years of waiting for the wildcard.

I guess this is more of a question for @tweidinger, @lucasfernog and @chippers, but i wanted to voice my point of view anyway 🤷

p.s. sorry for the triple ping, didn't want to talk about you guys without giving you a chance to react :v:

FabianLars avatar Sep 05 '22 15:09 FabianLars

I understand and it makes total sense, but this won't work for us and waiting is not really an option for a startup. So I will most likely fork tauri or stay on a "broken" version until v2 is released. That being said I am working on my solution and I will push a draft PR today.

Sytten avatar Sep 05 '22 15:09 Sytten

Sorry for the double ping, but I realized the IPC is used both for modules and for (developer defined) commands. I am wondering if it would be more acceptable to the team if only commands were available vs all the modules (which include some pretty intense stuff). That would be enough for us and would really limit the scope of external attacks. That means only part of TAURI would be accessible too.

Sytten avatar Sep 05 '22 15:09 Sytten

the modules and user commands are basically the same thing before the call reaches the code where the request is actually read. Preferably it wouldn't even get this far, for example by disabling webview.ipc altogether using something like this for example https://docs.microsoft.com/en-us/dotnet/api/microsoft.web.webview2.core.corewebview2settings.iswebmessageenabled?view=webview2-dotnet-1.0.1293.44#microsoft-web-webview2-core-corewebview2settings-iswebmessageenabled - idk if the other platforms have a similar setting.

That would be my ideal solution. The only drawback would be that the user wouldn't receive nicer custom errors but only whatever the webview would give them. Also i don't have the everything about the ipc in my head right now, so maybeee this is not even compatible with it. Also there will be an ipc rewrite in v2 which won't rely much on the message ipc anymore so we'd have to extend that anyway 🤔

FabianLars avatar Sep 05 '22 16:09 FabianLars

I did a kind dirty PoC for the access to commands (doesn't support isolation). We will most likely maintain a fork with this patchset for a while. One thing I was not sure was what to do with plugins, for now I disabled access. I think my idea of IPCAccess has potential for v2. One thing that is currently annoying is that all the code assumes local == full access and external == no access. I think having a modular approach for v2 would be nicer/more flexible.

Sytten avatar Sep 05 '22 20:09 Sytten

This sounds like something the window-specific state and commands idea would fix long-term.

@tweidinger and I had been talking about this. Basically instead of having one global store for commands and state, we would have potentially many and developers could decide which Store and IPC router to associate to a given window if any.

But as fabian already said, sorry this is not something we can quickly fix as anything IPC-related touches the very core of Tauri, but as long as maintaining a fork is acceptable you can rest assured improvements to the IPC system will be coming in v2 and v3 and beyond

JonasKruckenberg avatar Sep 05 '22 23:09 JonasKruckenberg