Open Bug 1812192 Opened 2 years ago Updated 1 year ago

Identify URL bar input without a scheme and allow upgrading those to HTTPS with a fallback

Categories

(Core :: DOM: Security, enhancement, P3)

enhancement

Tracking

()

ASSIGNED

People

(Reporter: daisuke, Assigned: mjurgens)

References

(Blocks 1 open bug)

Details

(Whiteboard: [domsecurity-active])

Attachments

(4 files, 1 obsolete file)

When we pass a URL string without the protocol, the function returns URL with http protocol.
https://searchfox.org/mozilla-central/rev/cf3af6bb6657278880f8baf38435eeb8f2d5d86c/docshell/base/URIFixup.sys.mjs#960
However, we'd better use the value of browser.fixup.alternate.protocol instead of it.
If the pref is empty, perhaps, we'd better use https instead of http.

We already have a pref for "alternate" uri fixup that is browser.fixup.alternate.protocol that is used by the URIFixup module to generate, starting from mozilla.com, something like https://www.mozilla.com/.
Currently fixupURIProtocol is hardcoding http:// in https://searchfox.org/mozilla-central/rev/3a13c5ad9c1dbd065e484a399af75eb98c235def/docshell/base/URIFixup.sys.mjs#957,960
The idea is to make it use whatever is defined in browser.fixup.alternate.protocol (with a fallback to https if the pref is invalid).

Open questions:

  1. There is a browser.fixup.fallback-to-https that is used by the docshell if it tries to load http but that fails. If we change fixup to do https first, this path will be hit less often, and there's no fallback to http. Is this a problem?
  2. URIFixup won't set alternateURI for https, only for http. I think the main reason behind this was privacy/security (changing the domain may inadvertantly send data to a different one). Defaulting to https will make this path hit less often. Is this a problem?
  3. Overall, while before we were trying http, and fallback to https, this will bring us to directly try https without a fallback, is this a problem?

Actually,
Freddy and I wanted to introduce something similar.

Maybe our ideas help for the open questions above:

  1. We thought also to upgrade in fixupURIProtocol but @Gijs had some second thoughts about it. Since it gets called by more than only the URLbar functions it might lead to problems (@Gijs I hope I "recited" you correctly).

  2. To ensure that the fallback to http is guaranteed we thought to introduce a flag in fixupURIProtocol that we would check here and use the HTTPSFirst fallback mechanism for that request.

But since we were unsure if starting in fixupURIProtocol is the correct call and also how to introduce a flag that reaches to MaybeHandleLoadErrorWithURIFixup we did not take further steps.

Flags: needinfo?(gijskruitbosch+bugs)

There's a point I want to make sure is emphasized clearly: If the address bar is able to transfer and hand down a flag that ends up in a loadState (and eventually loadInfo), we could tie this into the existing mechanisms for fallbacks.

Specifically, "HTTPS First", does two racing requests and falls back to HTTP if the page does not respond timely (already enabled in private browsing mode).

I think Marco can do a better job of answering the questions here than I can. :-)

Flags: needinfo?(gijskruitbosch+bugs) → needinfo?(mak)

(In reply to Tomer Yavor from comment #2)

  1. We thought also to upgrade in fixupURIProtocol but @Gijs had some second thoughts about it. Since it gets called by more than only the URLbar functions it might lead to problems (@Gijs I hope I "recited" you correctly).

Long term it's the right thing to do, but I concur with the fact UriFixup is used by a lot of consumers, sometimes even inappropriately. The work Gijs is doing in Bug 1810141 and its dependencies would help creating a list of those consumers that still rely on fixup and see how they'd react to this change. Plus it will remove some of the inappropriate uses of it, making potentially easier to migrate.
The main consumer remains the urlbar of course, and my concerns are the ones I expressed in comment 1, related to not having a fallback if https doesn't effectively work for a certain origin.

  1. To ensure that the fallback to http is guaranteed we thought to introduce a flag in fixupURIProtocol that we would check here and use the HTTPSFirst fallback mechanism for that request.

Is the plan to allow consumers to selectively opt-in to fixup to https, so you can examine one consumer at a time?
Long term we may reach a point where all consumers are working fine with it, and the added argument could be removed.

But since we were unsure if starting in fixupURIProtocol is the correct call

I think so, the alternative would be to either:

  • change default but allow the user to revert, that was our initial suggestion to just rely on browser.fixup.alternate.protocol. This of course may break users expectations without a proper fallback
  • intercept the load, that I assume is what https-first does, but this will still show http in the urlbar and it's not the ideal long term target

and also how to introduce a flag that reaches to MaybeHandleLoadErrorWithURIFixup we did not take further steps.

(In reply to Frederik Braun [:freddy] from comment #3)

There's a point I want to make sure is emphasized clearly: If the address bar is able to transfer and hand down a flag that ends up in a loadState (and eventually loadInfo), we could tie this into the existing mechanisms for fallbacks.

All the urlbar loads go through a _loadURL method that calls into openTrustedLinkIn and finally to openLinkIn
https://searchfox.org/mozilla-central/rev/9de332d5c8faac58dc1232b8a6383ce6cb1400f4/browser/components/urlbar/UrlbarInput.sys.mjs#2613,2719
https://searchfox.org/mozilla-central/rev/9de332d5c8faac58dc1232b8a6383ce6cb1400f4/browser/base/content/utilityOverlay.js#276
There's already multiple params there.
There it may either open a window or a load in a tab, so you may have to handle both cases.
The browser then goes to nsiWebNavigation.LoadURI
https://searchfox.org/mozilla-central/rev/9de332d5c8faac58dc1232b8a6383ce6cb1400f4/browser/base/content/tabbrowser.js#7059,7061
That ends in a nsDocShellLoadState::CreateFromLoadURIOptions, so there should be no problem adding additional flags to reach it.

Flags: needinfo?(mak)
Assignee: nobody → fbraun
Status: NEW → ASSIGNED
Attachment #9324182 - Attachment is obsolete: true

Hey Adam, as a triage owner - who would be a good person to give first feedback?
I'm not entirely seeking a patch review yet, as I am going through test failures and making sure that this is 100% the intended behavior, but I hope it's somewhat. close.

Flags: needinfo?(avandolder)
Duplicate of this bug: 1628831
Severity: -- → S3
Component: DOM: Navigation → DOM: Security
Flags: needinfo?(avandolder)
Priority: -- → P3
Summary: Use browser.fixup.alternate.protocol instead of hardcoding http in URIFixup::fixupURIProtocol. → Identify URL bar input without a scheme and allow upgrading those to HTTPS with a fallback
Attachment #9337195 - Attachment description: WIP: Bug 1812192 - address bar should mark searches that were schemeless -- wip → Bug 1812192 - address bar should mark navigations that were schemeless r=mak,gijs
Attachment #9337196 - Attachment description: WIP: Bug 1812192 - store and upgrade schemeless address bar loads -- wip → Bug 1812192 - store schemeless address bar loads in loadinfo and upgrade to https with fallback r=ckerschb,gijs
Attachment #9337195 - Attachment description: Bug 1812192 - address bar should mark navigations that were schemeless r=mak,gijs → WIP: Bug 1812192 - address bar should mark navigations that were schemeless r=mak,gijs
Attachment #9337196 - Attachment description: Bug 1812192 - store schemeless address bar loads in loadinfo and upgrade to https with fallback r=ckerschb,gijs → WIP: Bug 1812192 - store schemeless address bar loads in loadinfo and upgrade to https with fallback r=ckerschb,gijs
Whiteboard: [domsecurity-active]

Depends on D181097

Assignee: fbraun → mjurgens
Duplicate of this bug: 1060546
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: