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

[DISCUSSION] Allow more URI schemes #7562

Closed
PankajBhojwani opened this issue Sep 8, 2020 · 19 comments · Fixed by #14993
Closed

[DISCUSSION] Allow more URI schemes #7562

PankajBhojwani opened this issue Sep 8, 2020 · 19 comments · Fixed by #14993
Labels
Area-User Interface Issues pertaining to the user interface of the Console or Terminal In-PR This issue has a related PR Issue-Task It's a feature request, but it doesn't really need a major design. Needs-Tag-Fix Doesn't match tag requirements Product-Terminal The new Windows Terminal.
Milestone

Comments

@PankajBhojwani
Copy link
Contributor

PankajBhojwani commented Sep 8, 2020

Description of the new feature/enhancement

We recently added support for hyperlinks (but only certain schemes so far). We should add support for more URI schemes.

Discussion points:

  • Which schemes should we support?
  • Which schemes can be easily allowed without much security risks and which would require more thought?
  • Is it on us to deal with the security concerns with regards to clicking links or is that on the user?
@PankajBhojwani PankajBhojwani added the Issue-Feature Complex enough to require an in depth planning process and actual budgeted, scheduled work. label Sep 8, 2020
@ghost ghost added Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting Needs-Tag-Fix Doesn't match tag requirements labels Sep 8, 2020
@PankajBhojwani PankajBhojwani added Area-Accessibility Issues related to accessibility Area-VT Virtual Terminal sequence support Product-Terminal The new Windows Terminal. labels Sep 8, 2020
@ghost ghost removed the Needs-Tag-Fix Doesn't match tag requirements label Sep 8, 2020
@PankajBhojwani PankajBhojwani removed Area-Accessibility Issues related to accessibility Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting labels Sep 8, 2020
@DHowett DHowett added Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting and removed Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting labels Sep 8, 2020
@DHowett DHowett added this to the Terminal v2.0 milestone Sep 8, 2020
@DHowett DHowett added Issue-Task It's a feature request, but it doesn't really need a major design. Area-User Interface Issues pertaining to the user interface of the Console or Terminal and removed Issue-Feature Complex enough to require an in depth planning process and actual budgeted, scheduled work. Area-VT Virtual Terminal sequence support labels Sep 14, 2020
@dodexahedron
Copy link

What about a more generic implementation of URI parsing to match anything that fits the RFC, and leave handling actually opening it to Windows? Artificially implementing schemes, explicitly, on a one-off basis, seems limiting and more complex.

@PankajBhojwani PankajBhojwani changed the title Allow mailto URIs [DISCUSSION] Allow more URI schemes Sep 25, 2020
@PankajBhojwani
Copy link
Contributor Author

PankajBhojwani commented Sep 25, 2020

You are completely correct that this way is more complex/limiting. However there are security concerns with regards to allowing any scheme to be opened (see the discussion in #7526 for example).

That being said, I have marked this issue as a generic place to discuss adding support for more URI schemes, and further concerns regarding which schemes we can easily support and which schemes would require more thought with regards to security/viability/etc can be talked about here.

@j4james, would love to hear your input on this!

@dodexahedron
Copy link

dodexahedron commented Sep 25, 2020

Well taken.

I tend to be of a mind aligning with the third bullet point above, for a few reasons:

  • This particular feature is just a convenience to the user, and isn’t likely to be a primary means of interacting with it.
  • A user who has downloaded, installed, and is using Terminal is significantly less likely to be a novice user who doesn’t understand the repercussions of clicking random links.
  • The text, as displayed, is the link, is it not? In other words, it’s not like an HTML page, where the displayed text may not match the actual target of the hyperlink.
  • I highly doubt a malicious actor would go through the effort of having someone open up Terminal in order to carry out their dirty deeds, when there are far simpler ways of getting someone to click a nasty link.
  • In my opinion, it isn’t Terminal’s responsibility or place to protect a user from themselves any more than any other part of the shell, so, if an action is possible via a click elsewhere, it feels like it should be possible, here, too, to me.

A question/thought, though: If clicked hyperlinks are launched in such a way that UAC will be invoked, regardless of the permissions Terminal was launched with, would that not also be a fairly decent safeguard, leaving all responsibility then up to the user to confirm they want to do something potentially dangerous? Is that behavior possible?

@j4james
Copy link
Collaborator

j4james commented Sep 26, 2020

I think by default most protocols should warn the user before opening, similar to the way browsers do, with a dialog telling them exactly which application is going to be opened, and the url that triggered the event. We can whitelist a few protocols by default (http being an obvious candidate), but I think that list should be really short. Users can always choose to whitelist individual protocols, but I wouldn't make that too easy for them, otherwise they'll just blindly whitelist everything.

When it comes to the file protocol in particular, we need to be even more careful. I don't think it's good enough to have a blanket whitelist for all file urls. One possibility would be to whitelist particular file extensions in the same way we would whitelist the protocols. So we default to popping up a warning - again letting the user know exactly which application is going to be opened - and then they can always have an option to whitelist trusted file extensions if they really want to.

Another thought I had was to ShellExecute the file urls using an edit verb. So for example if you clicked on a file url for a Python script, ideally we want to allow that to open in an editor, but never allow it to execute. I don't know how feasible that is in practice, though. Does edit guarantee that nothing will every be executed? And are there any file extensions that we would want to support that couldn't be opened that way? That needs to be investigated.

Finally, I should point out that I'm not a security expert, but in my experience security people tend to be way more paranoid than me about stuff like this, and I'm terrified of this functionality. If I were you, I wouldn't want to implement anything that hasn't been signed off by someone senior in Microsoft's security team, especially if we're planning to add hyperlink support to conhost as well, or if Windows Terminals is going to be become the default Windows console one day.

@KalleOlaviNiemitalo
Copy link

I looked for ways to avoid having a list of safe URI schemes in Windows Terminal, but didn't find any.

ASSOCSTR_FRIENDLYAPPNAME and IHandlerActivationHost seem to be ways to get the name of the URL handler application, if Windows Terminal wants to display that in the warning.

@DHowett
Copy link
Member

DHowett commented Sep 28, 2020

So, in general I agree with the arguments made on behalf of security here. However, there's one issue that tips me in the other direction:

Given the basic premise that a remote application can't generate meaningful file URIs (it lacks knowledge of the layout of the remote system and any ability to drop files in known places), the concern about an application printing a link to an executable or executable document applies only to local applications.

But then... would such a local application that can produce a path to a malicious executable not itself simply do the bad thing? It's running with the user's integrity level and identity already. It can do far, far worse things than trick the user into clicking a link to another EXE file. It is, or can just directly spawn, an EXE file.

The same holds for URIs handled by applications that act immediately (like a hypothetical ms-settings:reset_system?confirmed=yes&confirm_token=aaaaaa (which doesn't exist, but illustrates the point). Those applications are already potential vectors for attack regardless of who's calling them.

So I guess my question is ... what's the attack, and how does Terminal launching file URIs amplify it?

@DHowett
Copy link
Member

DHowett commented Sep 28, 2020

If we were talking about terminal automatically fetching (or launching!) URLs as directed by an application, I'd be more concerned...

@miniksa
Copy link
Member

miniksa commented Sep 28, 2020

I've located an internal contact related to URI handling and reputation. We'll reach out to the experts at Microsoft who have already handled this for other products and get some advice.

@j4james
Copy link
Collaborator

j4james commented Sep 28, 2020

Given the basic premise that a remote application can't generate meaningful file URIs (it lacks knowledge of the layout of the remote system and any ability to drop files in known places),

If it knows you're on a Windows box, it can reasonably assume it'll be able to find a bunch of executables under c:\Windows, or /mnt/c/Windows. It doesn't have to be right 100% of the time.

Also, note that most exploits are a combination of vulnerabilities. You may be thinking this isn't a problem because an attacker can only execute files that are already on the system, but can't drop files there. But there is likely also a developer somewhere thinking they don't have a problem because their vulnerability only allows the attacker to drop files on the system, but not execute them. Combine the two and the user is screwed.

I should also mention that the original proposed hyperlink code would already have allowed a file url to run remote executables using a UNC path to a public SMB share. In my tests, the OS did at least seem to detect that as a remote executable and provide a warning, but I wouldn't want to rely on that being an insurmountable defence. Anything that lets an attacker get closer to an exploit is a bad thing, even if it doesn't seem to go all the way.

@JasonWThompson
Copy link

I would love it if the windows terminal supported URIs that point to a specific file resource like sftp. A use case might be to allow someone to script up an alternative form of ls/gci that would output sftp hyperlinks that point to the files listed. One could then ctrl+click to download said file locally rather than having to fire up WinSCP, navigating to the file, and then downloading it to the device.

For that matter if windows terminal supported URIs that pointed to file locations, a future version of Windows terminal could have an option to make the linked item draggable to the desktop. Could you imagine SSH into a host, cd to the log directory, get a listing of files, and then dragging and dropping a listing item to the desktop!? 🤤 Or maybe being able to right click and select, "Open with VS Code" 🤩

I've gone way off topic, but supporting URI/URLs that point to specific file resources would be a dream come true.

@TheXenocide
Copy link

TheXenocide commented May 10, 2021

I found this while trying to use vscode://file/[path to file]:[line] URIs using ANSI Hyperlinks. It would be very helpful to use already registered system handlers.

I don't see why it needs to be any more secure than a browser (e.g. Are you sure, Choose Application, etc.) but if it was necessary maybe the configuration file can whitelist allowed schemes? (at a minimum, maybe the configuration approach can at least allow a shorter track to allow people to take advantage of them while the rest of the UX is figured out?)

It would also be super handy to have a URI that could enter text as input on the current terminal as I'd love to make my powershell scripts more interactive with less text on the screen. Seems like this could go really well with VCS tooling, powerlines, directory listings (e.g. a hyperlink that change directory to a specific parent) and such.

One of the first things that came to mind to test this with was adding a little config gear to my powerline and making it a hyperlink to vscode://file/$PROFILE or to edit a json config file similarly.

@mkanet
Copy link

mkanet commented Dec 8, 2022

I would love to be able to create mailto: and msteams:// clickable links similar to how we open https:// hyperlinks in MS Terminal. I can't tell from the above discussion if we will see this capability or not. Hopefully, there will at least be some way to achieve this through a work-around or official feature.

@zadjii-msft
Copy link
Member

note to self: Write-Output "`e]8;;msteams://foo`e\This is a link to teams?`e]8;;`e\" doesn't work, cause we manually block that.

I don't think we ever came to a decision one way or the other. We need to find someone who's an actual expert in this area to give us a bit more feedback. I don't think we're opposed to the idea, but we just don't want to go into this blindly 😄

@Araxeus
Copy link

Araxeus commented Jan 6, 2023

Would love to see support for ::{CLSID} and shell: protocols

For example, I have an interactive directory explorer (https://github.com/Araxeus/ls-interactive) and I'm trying to make everything a link
The problem is linking to My Computer - there isn't any valid path to it, only one of the schemes mentioned above

shell:MyComputerFolder
::{20D04FE0-3AEA-1069-A2D8-08002B30309D}

@zadjii-msft I think there shouldn't be security problems with those since they are built in windows


shell: constants:
https://learn.microsoft.com/en-us/windows/win32/shell/search-protocol-crumb#constants-for-common-folders
saved here:
HKEY_LOCAL_MACHINE\SOFTWARE\Microsoft\Windows\CurrentVersion\Explorer\FolderDescriptions\

https://learn.microsoft.com/en-us/windows/win32/shell/known-folders

https://support.microsoft.com/en-us/topic/how-to-redirect-user-shell-folders-to-a-specified-path-by-using-profile-maker-ed6289ae-1f9c-b874-4e8c-20d23ea65b2e

https://docs.rainmeter.net/tips/launching-windows-special-folders/

@WasabiFan
Copy link

Big 👍 here.

If there's uncertainty over which schemes are "safe" to automatically allow, it seems to me a configuration option to enter a list of schemes (or a URL regex) would be an effective stopgap. I have scripts which output vscode:// and similar URLs as mentioned in a comment above and it's ineffective in Terminal.

@mkanet
Copy link

mkanet commented Mar 8, 2023

Maybe some URI's that aren't a security concern can get added first? Who actually decides what can get added and what can't?

@zadjii-msft
Copy link
Member

zadjii-msft commented Mar 8, 2023

I mean, ultimately we as the maintainers decide what can and what can't, though, with feedback from the community. I kinda come down to two trains of thought:

  • What schemes are safe for us to enable always? https, sure, but the rest I honestly don't know enough about the threat space here to make any sort of decision. That's where I'd love input from someone who is an expert, and that we haven't had the cycles to dig up.
  • How can we conditionally add support for additional schemes? Even if we don't allow all of them by default (and I'm sure we never will), what's the best way of enabling users to pick and choose which are enabled? Is this just a list of well-known URI schemes that we let folks opt in to? Or maybe more broadly a whole list of regexes? How do these layer (i.e. profile-specific settings on top of profiles.defaults ones)? Another question I certainly haven't had the cycles for recently.

edit: notes here so I don't ping folks:

The relevant security comments: #7526 (comment), #7526 (comment)

I finally get to come back with a bit of an answer to this.

Also, using ShellExecute on a file url seems risky from a security point of view.

We chatted with the defender/application security folks about this. At the end of the day, ShellExecute ends up using the same reputation services as the rest of Windows. Defender/ATM/"Sense" gets a chance to intervene if the executable is unsigned or untrustworthy.

Looking at it all-up:

  • this doesn't let a would-be attacker specify command-line arguments (ala "cmd.exe /s /c do_a_bad_thing") (using somebody else's reputation to cause mayhem)
  • Links require affirmative (Ctrl+click) action to activate, and I will staunchly refuse to add support for opening one without a modifier key
  • For links that the user has vetted:
    • A local application that can overwrite a linked region with another link is running at-or-above user permission and can just do_the_bad_thing() (or the reputation service kicks in)
    • A remote application that can overwrite a linked region either...
      1. can manipulate a UNC path and replace an executable (network permission required; reputation service kicks in)
      2. can make a user launch an otherwise-acquired local executable (reputation service kicks in)
  • ShellExecute de-elevates because it bounces a launch request off the shell

I'm less concerned about the security aspect of this feature.

Now- for WSL paths. "Works predictably for 15% of applications" (h/t to PhMajerus' AXSH, and other on-Windows requestors) is better in so many ways than "Works for 0% of applications", in my estimation. Incremental progress 😄 while we work on features that'll make it even more broadly applicable.

I'm inclined to call in favor of merging this.

Wouldn't it be better to use the "edit" verb here for file URLs? Perhaps having two separate conditions, like IsUriSupportedForOpen, and IsUriSupportedForEdit. Because if a user is clicking on a link to a .py file, I think it's more likely they would want the file edited rather than executed (which is what the open verb tends to do).

I don't know if there are any downsides to using the edit verb though. I did bring this up in the #7562 discussion thread, but I don't think anyone commented on the merits of the idea one way or the other.

I'm not in love with the idea of using the edit verb. I just gave it a test on a couple of the files I had laying around:

extension open behavior edit behavior
ps1 notepad powershell ISE (?!)
txt notepad notepad
cpp visual studio preview, which is not my default VS (argh) notepad
no extension "how do you want to open this?" dialog an error message from ShellExecute
.gitconfig, .wslconifg, etc same as above same as above

It's great if somebody's registered for it, but I'm bothered that the OS doesn't give it the same "do what with?" treatment.


more notes

Write-Output "`e]8;;msteams://`e\This link will open Teams`e]8;;`e\"
Write-Output "`e]8;;ms-settings://`e\This link will open the Settings app`e]8;;`e\"
Write-Output "`e]8;;shell:MyComputerFolder`e\This link will open 'My Computer'?`e]8;;`e\"
Write-Output "`e]8;;vscode://file/d:/100k.py:5:10`e\This link will open '100k.py' to line 5, col 10`e]8;;`e\"
printf "\n\x1b]8;;file://wsl$/$WSL_DISTRO_NAME$PWD\x1b\\$P
WD\x1b]8;;\x1b\\ \n\n"

those look like the sensible ones I have installed

@Seldaek
Copy link

Seldaek commented Mar 8, 2023

@zadjii-msft IMO enabling people to add schemes at least (much simpler than allowing complete URI regexes, and I don't really see the benefit as the URI regex should match all of them anyway) would already be a good 80-90% solution.

I'd add phpstorm or vscode to an array of allowed schemes in my config (augmenting the defaults of https, http, file) and would become able to click those links. By doing this I'm also taking the responsibility for possibly adding threat vectors, but I doubt there are many ways URIs can be abused, as links are so prevalent online.

Given the utility this brings for developers opening files directly in editors from CLI output, I think it's well worth the addition and the minimal risk this carries.

I fully understand you don't want to spend too much time on a 100% solution here, but it seems like adding the config option for schemes and supporting it in

bool TerminalPage::_IsUriSupported(const winrt::Windows::Foundation::Uri& parsedUri)
and wherever the highlighting-on-hover/parsing happens (I could not find this, grepping for http or file is a nightmare 😄) is reasonably low effort and would resolve the issue for most I'm sure.

@zadjii-msft
Copy link
Member

You know, there's a good point that I think has maybe been lost in the last two years. I think we long ago conflated "autodetected URIs" with "manually emitted" ones.

I think there's a lot more minutia that would go into adding more URIs to auto-detect, and how that plays with some other regex asks.

But I think I've lost the part of the discussion where we only allowed OSC8 links to https:, http: and file:. I'll need to dig for why that was. Seems like from the other discussion notes there's no reason to filter out the other URI schemes, we just only ever supported those to start with...

@microsoft-github-policy-service microsoft-github-policy-service bot added the In-PR This issue has a related PR label Mar 14, 2023
carlos-zamora pushed a commit that referenced this issue Mar 17, 2023
Does two things related to URLs emitted via OSC8. 

* Allows `wsl$` and `wsl.localhost` as the hostname in `file://` URIs
* Generally allows _all_ URIs that parse as a URI. 

The relevant security comments: #7526 (comment)
> this doesn't let a would-be attacker specify command-line arguments (ala "cmd.exe /s /c do_a_bad_thing") (using somebody else's reputation to cause mayhem)
> 
> `ShellExecute` de-elevates because it bounces a launch request off the shell
> 
> "Works predictably for 15% of applications" (h/t to PhMajerus' AXSH, and other on-Windows requestors) is better in so many ways than "Works for 0% of applications", in my estimation. Incremental progress 😄 while we work on features that'll make it even more broadly applicable.

Closes #10188
Closes #7562
@microsoft-github-policy-service microsoft-github-policy-service bot added the Needs-Tag-Fix Doesn't match tag requirements label Mar 17, 2023
DHowett pushed a commit that referenced this issue Mar 31, 2023
Does two things related to URLs emitted via OSC8.

* Allows `wsl$` and `wsl.localhost` as the hostname in `file://` URIs
* Generally allows _all_ URIs that parse as a URI.

The relevant security comments: #7526 (comment)
> this doesn't let a would-be attacker specify command-line arguments (ala "cmd.exe /s /c do_a_bad_thing") (using somebody else's reputation to cause mayhem)
>
> `ShellExecute` de-elevates because it bounces a launch request off the shell
>
> "Works predictably for 15% of applications" (h/t to PhMajerus' AXSH, and other on-Windows requestors) is better in so many ways than "Works for 0% of applications", in my estimation. Incremental progress 😄 while we work on features that'll make it even more broadly applicable.

Closes #10188
Closes #7562

(cherry picked from commit 65640f6)
Service-Card-Id: 88719284
Service-Version: 1.17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-User Interface Issues pertaining to the user interface of the Console or Terminal In-PR This issue has a related PR Issue-Task It's a feature request, but it doesn't really need a major design. Needs-Tag-Fix Doesn't match tag requirements Product-Terminal The new Windows Terminal.
Projects
None yet
Development

Successfully merging a pull request may close this issue.