1629307 - Shouldn't prompt for http auth if the XFO (X-Frame-Options header) will block loading the page anyway

Status: RESOLVED FIXED

(Core :: Networking: HTTP, defect, P3)

Description

[:Gijs (he/him)]

STR:

1. load data:text/html,<iframe src="https://login.mozilla.com/">

ER:
frame with error page

AR:
first get an http auth prompt, then an error page.

I believe we send the xfo header in the response that causes the http auth prompt, so we might as well not prompt. I don't think this is a security bug given that there is no way the framing page gets any data (the password, after all, will only go to the XFO page) or can clickjack anything, as we'll refuse to load the page after the password anyway.

Comment 1

[Andy Grover [:grover]]

hey Christoph, do you know if the code popping the prompt is in security code or Necko code? Also CCing dragana.

Comment 2

[Christoph Kerschbaumer [:ckerschb}]

(In reply to Andy Grover [:grover] from comment #1)

hey Christoph, do you know if the code popping the prompt is in security code or Necko code? Also CCing dragana.

I don't know for sure actually, but I would imagine it's somewhere in Necko code because I have never encountered it. Hopefully Dragana knows.

Comment 3

[Dragana Damjanovic [:dragana]]

auth prompts are in necko. I could not reproduce the problem at first. I only got a frame with the error page.
but than I have updated nightly and there it is a prompt.
so this is a recent regression. Christoph, do you know what has changed? Necko auth prompt is the same for a long time, maybe it is documentchannel problem.

Comment 4

[:Gijs (he/him)]

(In reply to Dragana Damjanovic [:dragana] from comment #3)

auth prompts are in necko. I could not reproduce the problem at first. I only got a frame with the error page.
but than I have updated nightly and there it is a prompt.
so this is a recent regression. Christoph, do you know what has changed? Necko auth prompt is the same for a long time, maybe it is documentchannel problem.

I'm surprised; I tested in beta (though it may have been 76 beta?) and could reproduce there too. I just tested with mozregression --launch 75 and the data URI from comment #0 and can reproduce there, too. Same for mozregression --launch 70, and a build from 2019-01-01. I don't think this is a recent regression. Perhaps there's some other reason this didn't initially reproduce for you?

Comment 5

[:Gijs (he/him)]

FWIW, I expect the checks for XFO simply happen later than the auth prompt, and that this has always been broken. Fixing it would involve either moving the checks to happen sooner or duplicating them / checking for this specifically before prompting. We may need to do the same for the CSP frame-ancestors directives.

Comment 6

[Christoph Kerschbaumer [:ckerschb}]

(In reply to Dragana Damjanovic [:dragana] from comment #3)

auth prompts are in necko. I could not reproduce the problem at first. I only got a frame with the error page.
but than I have updated nightly and there it is a prompt.
so this is a recent regression. Christoph, do you know what has changed? Necko auth prompt is the same for a long time, maybe it is documentchannel problem.

I don't think this is a regression. Can you guide me to the auth-prompt code? Then we can verify when things happen.

FWIW, XFO checks happen in the Parent within DocumentLoadListener::OnStartRequest().

Comment 7

[Dragana Damjanovic [:dragana]]

A prompt is happening while processing 40x response that will lead to another load.
A prompt is trigger by this call:
https://searchfox.org/mozilla-central/rev/567b68b8ff4b6d607ba34a6f1926873d21a7b4d7/netwerk/protocol/http/nsHttpChannel.cpp#2822

Comment 8

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

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

For more information, please visit auto_nag documentation.

Comment 9

[Dragana Damjanovic [:dragana]]

Christoph, please look at comment 6, if you have missed it.

Comment 10

[Christoph Kerschbaumer [:ckerschb}]

Andy, so I looked at this in a little more detail, and while we now know how things get processed chronologically, I don't see an easy fix for it:

1. load data:text/html,<iframe src="https://login.mozilla.com/">
2. auth happens in nsHttpChannel::ContinueProcessResponse
3. XFO blocks the load in DocumentLoadListener::OnStartRequest

At the time we process (2) we don't know whether XFO response headers will cause the load to be blocked. Please note that we can't really know at this point because XFO should only apply to documents and not e.g. to downloads. And at (2) we don't know whether a load will result in a download or not.

Do you think there Is an more or less easy way to move (2) after (3) somehow?

Comment 11

[Dragana Damjanovic [:dragana]]

This is not really possible to fix.

Comment 12

[Valentin Gosu [:valentin]]

I think we can reconsider this.
Technically it should be possible to (also) call nsContentSecurityUtils::PerformCSPFrameAncestorAndXFOCheck(this); from ContinueProcessResponse1 and that should cancel the channel before we process authentication.

Comment 13

[Sunil Mayya]

I was able to reproduce the issue with the latest nightly. I also tried the fix suggested by Valentin and it works.

Comment 14

[Sunil Mayya]

Attachment: Bug 1629307 - prevent auth prompts (status 401) if XFO checks fails. r=#necko

Comment 15

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/5f5c6f101a19
prevent auth prompts if XFO checks fails. r=necko-reviewers,valentin,ckerschb

Comment 16

[Serban Stanca [:SerbanS]]

https://hg.mozilla.org/mozilla-central/rev/5f5c6f101a19

Comment 17

[edgul]

Attachment: Backed out changeset 5f5c6f101a19 (Bug 1629307) for causing failure bug 1809151: corporate web proxy no kerberos auth for iframe content since 108.0.1 r=#necko-reviewers

Comment 18

[Phabricator Automation]

Comment on attachment 9313813 [details]
Backed out changeset 5f5c6f101a19 (Bug 1629307) for causing failure bug 1809151: corporate web proxy no kerberos auth for iframe content since 108.0.1 r=#necko-reviewers

Revision D167691 was moved to bug 1809151. Setting attachment 9313813 [details] to obsolete.

Comment 19

[edgul]

Patch is being backed out as per bug 1809151. May want to revisit the solution here.

Comment 20

[Ryan VanderMeulen [:RyanVM]]

This has now been reverted from all Firefox releases starting from 109.0.1 and later.

Comment 21

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/c827092ae59a
prevent auth prompts (status 401) if XFO checks fails. r=necko-reviewers,valentin,ckerschb

Comment 22

[Sandor Molnar[:smolnar]]

https://hg.mozilla.org/mozilla-central/rev/c827092ae59a