1609451 - links with credentials aren't loaded

Status: RESOLVED FIXED

(Core :: Widget: Win32, defect, P2)

Description

[Leonid Rozenblyum]

User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/79.0.3945.117 Safari/537.36

Steps to reproduce:

Receive an email with a link like
https://hello:[e-mail]/

Click the link

[Content]

[ОК]

Actual results:

An error

'Не удается найти "https://hello:[e-mail]/". Проверьте, правильно ли указано имя и повторите попытку.'

which means
'Unable to find ... . Please check if the name is correct and try again'

Expected results:

The link should have been opened. Such links opening worked in Thunderbird before and has been broken recently.

Comment 1

[Andre Klapper]

Does the link work when you manually copy it and paste it manually in the web browser? Which web browser is this about?

Comment 2

[Jorg K (CEST = GMT+2)]

Attachment: 1609451.png

Clearly reproducible in TB 68.4.1 but not TB 60.x. Pasting the link into FF 68.4.1 gives no problem.

Could be caused by bug 1601905 which isn't in FF 68.4.1 but is in TB 68.4.1.

Alice, can you please check this in earlier versions and find the regression if it worked before. Can you also check a FF Nightly please. Just visit this bug and click on the link in comment #0.

Toshi, it's funny that clicking on the link doesn't work in TB, but works in FF.

Comment 3

[Leonid Rozenblyum]

(In reply to Andre Klapper from comment #1)

Does the link work when you manually copy it and paste it manually in the web browser? Which web browser is this about?

Yes the link works fine after copy/pasting it to Firefox 72.
It also works when opening the mail directly in GMail web interface of Firefox/Chrome.
It used to work for years in Thunderbird (I'm a long time use of Thunderbird and often receive email with such links).
It has been broken in some of recent releases.

Comment 4

[Jorg K (CEST = GMT+2)]

Yes the link works fine after copy/pasting it to Firefox 72.

Yes, also working in FF 74 Daily. So looks like it's TB-specific after all. Maybe we messed something up in mail/base/content/contentAreaClick.js.

Comment 5

[Alice0775 White]

I can reproduce the issue on Daily74.0a1(20200112082826).

Regression window(comm-central):
https://hg.mozilla.org/comm-central/pushloghtml?fromchange=166ccd934db34e30db0588952e598b410840cab6&tochange=b56bdad8ff021b74dc1c6cc64ecaf955a2f6f04f
https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=5b35e2ff7c15cc2f4d1eb419a18d899294560243&tochange=5805cd9ae2947386263069e1a3b2d832384bd45f

Regression winodow:(comm-esr68):
https://hg.mozilla.org/releases/comm-esr68/pushloghtml?fromchange=b48e36a58d60405b4b2d75cc9fd2d3c9c21a8ed1&tochange=d788b4098371711c639632d5e3cfd2cb628e7f89
https://hg.mozilla.org/releases/mozilla-esr68/pushloghtml?fromchange=2f459603370d1d35a56ca4e9cee21f7766bbb832&tochange=a2b4204edd360face0b27e0b7d8b06827ff5f2bd

Comment 6

[Jorg K (CEST = GMT+2)]

Thanks, Alice. Yes, I can reproduce it in TB 74 Daily, but not FF 74 Nightly.

So it looks like this is more fallout from bug 1567614 which landed in the M-C range. From the other ranges we see that this broke in TB 68.1, that's when I started a new THUNDERBIRD_68_VERBRANCH branch based on FF 68.1 ESR where bug 1567614 had landed. I landed bug 1570845 to undo the damage, but obviously not all of it.

Alice, I need more of your help. Can you track the behaviour in FF. Did FF also break in the range
https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=5b35e2ff7c15cc2f4d1eb419a18d899294560243&tochange=5805cd9ae2947386263069e1a3b2d832384bd45f

And did it get fixed with bug 1570845 landing, or how did it get fixed?
EDIT: Obsoleted questions answered in comment #8.

Comment 7

[Jorg K (CEST = GMT+2)]

Magnus, does this happen on Linux?

Comment 8

[Toshihito Kikuchi [:toshi]]

Yeah, this is another regression from the change switching from ShellExecute to IShellDispatch2.ShellExecute. TB needs to launch an app to open a URL, but FF does not. That's why this does not happen in FF. This should impact only Windows.

I confirmed invoking ShellExecute with a url containing auth info in explorer.exe failed with INET_E_SECURITY_PROBLEM (= 0x800c000e). It seems that this is because FCK (= Feature Control Key) FEATURE_HTTP_USERNAME_PASSWORD_DISABLE is turned on for explorer.exe by default.

This issue did not happen after I created the following registry to disable the FCK and logoff/re-logon, but this workaround is NOT recommended because the FCK is undocumented and it makes the system less secure.

Key: HKCU\SOFTWARE\Microsoft\Internet Explorer\Main\FeatureControl\FEATURE_HTTP_USERNAME_PASSWORD_DISABLE
Name: explorer.exe
Value: 0
Type: REG_DWORD

The fix would be not to use ShellExecuteByExplorer if a target path is URL.

Comment 9

[Jorg K (CEST = GMT+2)]

Thanks for that, Toshi. This needs an M-C fix, right?

Comment 10

[Toshihito Kikuchi [:toshi]]

Yes. I confirmed the repro on TB Daily 74, too.

We introduced ShellExecuteByExplorer for a URL because Skype launched via lync15: scheme had the issue of Bug 1567614. However, TB didn't have that issue from the beginning because TB does not use the PreferSystem32Images mitigation policy . We can skip some schemes such as http/https/ftp, or maybe we can stop using ShellExecuteByExplorer in TB.

Comment 11

[Jorg K (CEST = GMT+2)]

Attachment: 1609451.reg

Registry setting as per comment #8. This works, indeed.

Comment 12

[Masatoshi Kimura [:emk]]

(In reply to Toshihito Kikuchi [:toshi] from comment #8)

The fix would be not to use ShellExecuteByExplorer if a target path is URL.

Will Skype work correctly when it's invoked with skype: URL protocol?

Comment 13

[Masatoshi Kimura [:emk]]

(In reply to Toshihito Kikuchi [:toshi] from comment #10)

We can skip some schemes such as http/https/ftp

What happens if the default web browser or ftp client is not compatible with PreferSystem32Images mitigation policy? Even if the browser itself works, what happens if the browser launches Skype? Those browsers might not use IShellDispatch2.ShellExecute because usually they are not invoked with PreferSystem32Images mitigation policy enabled.

Comment 14

[Masatoshi Kimura [:emk]]

(In reply to Toshihito Kikuchi [:toshi] from comment #10)

We can skip some schemes such as http/https/ftp

I have second-thought that this will work because:

1. Thunderbird does not use PreferSystem32Images.
2. Firefox will open http/https internally regardless of the default browser.

But ftp should not be skipped because:

1. The default ftp client may be other applications.
2. Firefox will drop support for ftp in the future.
3. Apparently, explorer does not refuse ftp URLs with username:password unlike http(s).

Comment 15

[Masatoshi Kimura [:emk]]

(In reply to Toshihito Kikuchi [:toshi] from comment #10)

or maybe we can stop using ShellExecuteByExplorer in TB.

I think we should stop using IShellDispatch2.ShellExecute only if the PreferSystem32Images mitigation policy is not enabled (and the scheme is http or https). The enabled mitigation policies are available using GetProcessMitigationPolicy.

Comment 16

[Magnus Melin [:mkmelin]]

What is the right component for this bug?

Comment 17

[BugBot [:suhaib / :marco/ :calixte]]

The priority flag is not set for this bug.
:jimm, could you have a look please?

For more information, please visit auto_nag documentation.

Comment 18

[Toshihito Kikuchi [:toshi]]

Attachment: Bug 1609451 - Skip ShellExecuteByExplorer to open a uri in Thunderbird. r=froydnj

We use ShellExecuteByExplorer to open a uri because applications may not
support the mitigation policies inherited from our process.

Unlike Firefox, Thunderbird runs ShellExecuteByExplorer to launch a browser
to open an HTTP/HTTPS uri. If a uri includes credentials like
http://user:[e-mail]/, ShellExecuteByExplorer succceds, but
explorer.exe refuses to open the uri by design. In this case the fallback to
ShellExecuteExW does not happen because the API is async and no error is
returned

The proposed fix is to skip ShellExecuteByExplorer if it's Thunderbird.
Because Thunderbird does not apply additional mitigation policies which may
cause compat issues, we don't need to run ShellExecuteByExplorer in the
first place.

Comment 19

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/470fe22118ba
Skip ShellExecuteByExplorer to open a uri in Thunderbird.  r=froydnj

Comment 20

[Jorg K (CEST = GMT+2)]

Thanks, Toshi. TB release management needs to get this shipped in TB 68.x at some stage.

Comment 21

[Cosmin Sabou [:CosminS]]

https://hg.mozilla.org/mozilla-central/rev/470fe22118ba

Comment 22

[abodenlosz]

The issue is still reproducible with the latest build of TB(68.7.0).

Comment 23

[Magnus Melin [:mkmelin]]

I think we should uplift this to the Thunderbird 68 ver bransch.

Comment 24

[Wayne Mery (:wsmwk)]

(In reply to Magnus Melin [:mkmelin] from comment #23)

I think we should uplift this to the Thunderbird 68 ver bransch.

agreed. This has been on beta 76, so it's good to uplift.

Comment 25

[Rob Lemley [:rjl]]

I took a shot at this today. The commit in comment 21 does not merge nicely on mozilla-esr68.

I did get something to compile, but I've not had a chance to verify if it even starts up much less fixes the problem.
https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=cfc93706462f069d3cf0c0268de8305e179a97fb

Comment 26

[Toshihito Kikuchi [:toshi]]

(In reply to Rob Lemley [:rjl] from comment #25)

I took a shot at this today. The commit in comment 21 does not merge nicely on mozilla-esr68.

I did get something to compile, but I've not had a chance to verify if it even starts up much less fixes the problem.
https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=cfc93706462f069d3cf0c0268de8305e179a97fb

The patch does not nicely fit because ESR does not have 08488953cff81039d248a949a1701dcd2d600f2c. I'll prepare a patch for ESR and do sanity check next week.

I didn't know the technique to build TB with modifying .gecko_rev.yml. It looks really handy!

Comment 27

[Toshihito Kikuchi [:toshi]]

Here's a change for mozilla-esr68.
https://treeherder.mozilla.org/#/jobs?repo=try&revision=5e20a11e3b8e5a1af04aef9ad895a85e8ac7e811

Rob, can you kick a build of TB ESR68? I have some troubles to build mozilla-esr68/comm-esr68 locally..

Comment 28

[Rob Lemley [:rjl]]

Try build with the patch from comment 27 applied. Thanks Toshihito! Are you verifying functionality? If it does work as expected it's probably easiest to attach it to this bug to make it easier to get it applied to our release branch.

https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=7fb329260cab209f3161b681ffa010929140eb4d&selectedTaskRun=ZlIzUT6NRA6---lPuQQm8A-0

Comment 29

[Toshihito Kikuchi [:toshi]]

Attachment: Bug 1609451 - Skip ShellExecuteByExplorer to open a uri in Thunderbird.

We use ShellExecuteByExplorer to open a uri because applications may not
support the mitigation policies inherited from our process.

Unlike Firefox, Thunderbird runs ShellExecuteByExplorer to launch a browser
to open an HTTP/HTTPS uri. If a uri includes credentials like
http://user:[e-mail]/, ShellExecuteByExplorer succceds, but
explorer.exe refuses to open the uri by design. In this case the fallback to
ShellExecuteExW does not happen because the API is async and no error is
returned

The proposed fix is to skip ShellExecuteByExplorer if it's Thunderbird.
Because Thunderbird does not apply additional mitigation policies which may
cause compat issues, we don't need to run ShellExecuteByExplorer in the
first place.

To uplift the change to ESR68, this patch includes the patch of Bug 1615370
as well as the small change not to use Result::inspect() which does not
exist in ESR68.

Comment 30

[Toshihito Kikuchi [:toshi]]

(In reply to Rob Lemley [:rjl] from comment #28)

Try build with the patch from comment 27 applied. Thanks Toshihito! Are you verifying functionality? If it does work as expected it's probably easiest to attach it to this bug to make it easier to get it applied to our release branch.

https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=7fb329260cab209f3161b681ffa010929140eb4d&selectedTaskRun=ZlIzUT6NRA6---lPuQQm8A-0

Thanks! I've verified that Try build and the impacted scenario, opening a uri with a credential from Thunderbird, worked nicely. I uploaded the change as D72856.

Comment 31

[Rob Lemley [:rjl]]

D72856 is on THUNDERBIRD_68_VERBRANCH on m-esr68, for Thunderbird 68.8.0.
https://hg.mozilla.org/releases/mozilla-esr68/rev/de088581d1528bab485eaf443cac763927cd4703

Comment 32

[abodenlosz]

Verified the fix on Windows 10 with TB 76.0b3. Updating the flag to verified.

Comment 33

[Leonid Rozenblyum]

I confirm the bug is fixed on TB 68.8.0. Thanks to everybody for the efforts to make it happen!