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

mailto: links are blocked on Chrome #1138

Closed
benoit74 opened this issue May 30, 2024 · 15 comments · Fixed by #1123
Closed

mailto: links are blocked on Chrome #1138

benoit74 opened this issue May 30, 2024 · 15 comments · Fixed by #1123
Assignees
Milestone

Comments

@benoit74
Copy link

ZIM: https://dev.library.kiwix.org/viewer#fas-military-medicine_en_2024-05/irp.fas.org/doddir/milmed/index.html
Chrome: 125
OS: MacOS Sonoma 14.5

When clicking on Steve Aftergood at the bottom of the front page, we should open a mailto: link. This does not happen.

Looks like we have a security message:

image

Is there anything which can be done or is it just Chrome who is too aggressive?

Nota: opening link outside viewer, i.e. from https://dev.library.kiwix.org/content/fas-military-medicine_en_2024-05/irp.fas.org/doddir/milmed/index.html works as expected, clearly an "iframe" issue.

@Jaifroid
Copy link
Member

Jaifroid commented Jun 2, 2024

@benoit74 see #916 and #943. A fix for the mailto: issue would just be an extension of the fixes for loading external content and PDFs in readers. External content violates the sandbox and CSP in all browsers, and PDFs are considered insecure by Chromium browsers. These were fixed (at least for Kiwix Serve) by checking the link clicked by the user and opening an external window if it's https or by serving a different CSP for PDFs: a variation of what we have discussed in openzim/warc2zim#276. I have to do the same for in KJS. I know you would prefer this to be fixed in scrapers, but not all readers have the same sandboxed CSP, and to fix at scraper level would require some rewriting of mailto: links so they are redirected to a new window before being "opened". Maybe that's easy, not sure, or it could require custom JS in each scraper.

@benoit74
Copy link
Author

benoit74 commented Jun 3, 2024

This time I would probably prefer to do it in readers, for the exact reason you mention ^^ The mailto: link is valid from a ZIM / HTML specification perspective and tweaking it for proper operation is a reader specific tweak (even it could probably be fairly identical in all readers). Contrary from openzim/warc2zim#276 where the broken links are invalid, they lead to a missing ZIM entries and scraper should hence probably take care of it on its own.

Or we have to decide to amend the ZIM specification and clearly explain that mailto: links (and probably others) have to be tweaked inside the ZIM, but this has never been discussed so far.

Glad that the way to solve the issue in kiwix-server is known. Does it means the fix is already there in KJS (because AFAIK, the mailto links are working there)?

@Jaifroid
Copy link
Member

Jaifroid commented Jun 3, 2024

@benoit74 In the browser extension filtering for mailto: is currently only working in JQuery mode (which has some limited Zimit support) -- see screenshot. This is because in that mode there is legacy code which checks whether the protocol of the article window is the same as the protocol of the clicked link, and it shows the external link dialogue box if that's the case. However, this isn't in the (mainstream) ServiceWorker code, where filters are implemented for specific cases, due to the fact that in Zimit1 the ReplayWorker handled a lot of this. In Zimit2 we no longer have ReplayWorker filtering stuff, so will have to handle edge cases like this ourselves.

image

@Jaifroid
Copy link
Member

Jaifroid commented Jun 3, 2024

I've just thought that it might be an easy fix to add mailto: to the Kiwix Serve and KJS sandbox Content Security Policy. Worth a try at least.

UPDATE: According to my tests in the PWA, this can indeed be solved by relaxing the CSP for the content of the iframe (at least, if not for the whole app as well). That should apply in Kiwix Serve too, which uses a very similar CSP.

@kelson42 Should an issue for modifying the CSP be opened on libkiwix instead of here?

@kelson42
Copy link
Collaborator

kelson42 commented Jun 3, 2024

@Jaifroid Yes please. If this change has any other impacts. Please list them

@Jaifroid
Copy link
Member

Jaifroid commented Jun 3, 2024

@Jaifroid Yes please. If this change has any other impacts. Please list them

@kelson42 I opened #1090. It should have no other impacts, as the fix is specific to mailto:.

@kelson42
Copy link
Collaborator

kelson42 commented Jun 3, 2024

Sorry, the issue does not being much.. I though you would do a PR.

@Jaifroid
Copy link
Member

Jaifroid commented Jun 3, 2024

Sorry, the issue does not being much.. I though you would do a PR.

Did you mean me? I could do a "blind" PR, but I don't have the ability to test it, as I can't compile Kiwix Serve. I'm also at work...

EDIT: I would if I could, e.g. if we had a way to build with a container (?), but I've done the next best thing which is to point out exactly where the CSP entry needs to be added AFAIK.

@Jaifroid
Copy link
Member

Jaifroid commented Jun 4, 2024

One thing that occurs to me is that nowadays there are all sorts of custom protocols. Even things like zoomus: (for a Zoom meeting), or magnet: (for a Magnet link), geo:, maps:, etc. -- anyone can define such a protocol / URI. While mailto: seems like one we should natively support via the CSP, I wonder if we need a more generic way to detect that a clicked link with such a URL needs to be opened outside the iframe / webview? In non-Zimit ZIMs, this can be achieved simply by comparing the protocol of the article's window to the protocol of the link's href, and decide its external if they don't match. But with Zimit, it's a little more complicated to do this at ZIM level due to need to add support for Wombat rewriting (and Python rewriting in Zimit2 when converting the WARC).

EDIT: I'm not sure what Wombat does with such protocols. Hopefully it leaves them alone?

@kelson42
Copy link
Collaborator

kelson42 commented Jun 4, 2024

@Jaifroid We have a shortlist in zimwriterfs of schemes we shozld not rewrite. geo: and tel: are indeed interesting cases.

@benoit74
Copy link
Author

benoit74 commented Jun 4, 2024

In Zimit it is quite simple now.

Any link with a protocol has not been rewritten, because result of rewriting is always a relative link because there is no need to specify of protocol once inside the ZIM, we just want to continue to use whatever has been used to load the HTML (starting with http if ZIM is served on http, https if it is served on https, but also the hostname, the base path, ... plus we could have other readers using other protocols).

So at reader level, any relative link is de facto already rewritten, and expected to point to a resource existing inside the ZIM. If corresponding item is missing in the ZIM (for any reason) it is always replaced by an absolute link with original http or https protocol used for crawling, including hostname and base path.

Finally FYI, warc2zim rewrites only relative links and http/https absolute links. All other protocols are simply ignored and kept as-is.

From my PoV (and this is true not only for Zimit but all ZIMs), all links with a protocol specified should be able to escape the sandbox since we know for sure they do not point to something inside the ZIM. Or at least they shouldn't. Not only mailto: and tel:, any absolute link.

@benoit74
Copy link
Author

benoit74 commented Jun 4, 2024

We have a shortlist in zimwriterfs of schemes we should not rewrite

Why only a shortlist? I don't get it

@kelson42
Copy link
Collaborator

kelson42 commented Jun 4, 2024

@benoit74 Sounds like you want ro make a C++ PR ;)

@Jaifroid
Copy link
Member

Jaifroid commented Jun 4, 2024

OK, thanks, both -- apart from mailto:, geo: and tel: links in Wikivoyage ZIMs (last two already dealt with, e.g. adding the World icon for geo: and telephone icon for tel:), then readers should just compare the protocols, and offer to open a new browser window/tab in the case of any other protocol, so the browser can deal with it however it wants. I'll reference this in #1090.

@benoit74
Copy link
Author

benoit74 commented Jun 4, 2024

Oh, of course, each reader is welcomed to handle any scheme he knows about with custom behaviors, but generally it should fallback to opening it in the browser / system and let it decide

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

Successfully merging a pull request may close this issue.

4 participants