Last Comment Bug 205947 - Proxy: MailNews does not display proxy auth dialog
: Proxy: MailNews does not display proxy auth dialog
Status: RESOLVED FIXED
:
Product: SeaMonkey
Classification: Client Software
Component: MailNews: Message Display (show other bugs)
: Trunk
: x86 Windows 2000
: -- normal with 2 votes (vote)
: ---
Assigned To: Scott MacGregor
: benc
Mentors:
: 217799 221772 232856 241846 248617 257636 (view as bug list)
Depends on:
Blocks: 221772 267367
  Show dependency treegraph
 
Reported: 2003-05-16 08:58 PDT by Grant Fengstad
Modified: 2005-04-05 23:31 PDT (History)
16 users (show)
asa: blocking1.7-
chofmann: blocking1.8a2+
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
mailnews-proxy-auth-0.patch (8.36 KB, patch)
2004-08-28 04:08 PDT, Vladimir Vukicevic [:vlad] [:vladv]
darin.moz: review-
Details | Diff | Splinter Review
force proxyAuth to false if no proxy server configured (1.07 KB, patch)
2004-08-29 10:52 PDT, Vladimir Vukicevic [:vlad] [:vladv]
no flags Details | Diff | Splinter Review
v2 patch for nsHttpChannel::ProcessAuthentication (2.60 KB, patch)
2004-08-30 17:16 PDT, Darin Fisher
cbiesinger: review+
shaver: superreview+
Details | Diff | Splinter Review
mailnews-proxy-auth-1.patch (12.29 KB, patch)
2004-08-31 15:29 PDT, Vladimir Vukicevic [:vlad] [:vladv]
darin.moz: review+
shaver: superreview+
shaver: approval‑aviary+
Details | Diff | Splinter Review
mailnews-proxy-auth-2.patch (aviary) (12.48 KB, patch)
2004-09-01 21:54 PDT, Vladimir Vukicevic [:vlad] [:vladv]
vladimir: review+
vladimir: superreview+
vladimir: approval‑aviary+
Details | Diff | Splinter Review
mailnews-proxy-auth-2.patch (aviary) (12.51 KB, patch)
2004-09-01 22:04 PDT, Vladimir Vukicevic [:vlad] [:vladv]
vladimir: review+
vladimir: superreview+
vladimir: approval‑aviary+
roc: approval1.7.5+
Details | Diff | Splinter Review
mailnews-proxy-patch-2.patch (trunk) (12.44 KB, patch)
2004-09-01 22:12 PDT, Vladimir Vukicevic [:vlad] [:vladv]
no flags Details | Diff | Splinter Review

Description Grant Fengstad 2003-05-16 08:58:25 PDT
User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.4b) Gecko/20030514
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.4b) Gecko/20030514

I am configured to use a proxy in order to access Internet based content and
services.  This proxy requires authentication upon first use.

When I open up the mail client and open an email that contains Internet based
content (ie: images, etc.), they never load.  The mail/news client does not
invoke the proxy authentication routine when attempting to load Internet based
content.

If I go to the browser and then direct a session to the Internet, I am prompted
for authentication within the browser.  Once authenticated, the mailnews client
works like a charm.

Reproducible: Always

Steps to Reproduce:
1.Open Mozilla (MailNews Client)
2.Open email that references internet based content
3.Content is not displayed
Comment 1 wbayer 2003-06-26 19:19:43 PDT
We can confirm this behaviour. We also noticed and reported the identical
behaviour in current Thunderbird builds. Unfortunately, with Thunderbird being a
standalone mail program, there is no similar workaround. 
Comment 2 wbayer 2003-07-03 22:16:01 PDT
I can also confirm that this is a regression introduced after Mozilla 1.4RC1, as
it works properly in that build. Please elevate the priority of this bug as it
disables Thunderbird from displaying HTML mail with embedded links to external
web sites. We would be pleased to assist in testing a patch for this issue
through our proxy servers.
Comment 3 Scott MacGregor 2003-07-23 12:43:14 PDT
Flagging this bug for thunderbird. 

Unfortunately I don't have access to proxy servers to try to debug this so that
makes it extremely hard! 

Any java script errors showing up in the JS console when you try this?



Comment 4 Sam Kamens 2003-07-23 13:21:06 PDT
No, no errors in the JS console or anywhere else.  It just never tries to do the
proxy authentication.
Comment 5 Jo Hermans 2003-08-30 03:54:46 PDT
*** Bug 217799 has been marked as a duplicate of this bug. ***
Comment 6 wbayer 2004-01-13 22:17:00 PST
Mozilla Thunderbird 0.5a (20040105) still exhibits this behaviour. Clicking 
the Thunderbird icon (top right) launches a proxy challenge. The default 
browser (Firebird 0.7) is launched, but returning to Thunderbird and refresing 
the message, it still doesn't display.
Comment 7 wbayer 2004-02-06 15:06:02 PST
As expected, this MailNews client issue was not resolved when we switched from
the Sun Proxy server to Squid proxy on Linux. The Mozilla MailNews client still
does not issue a proxy challenge when e-mail with external links is received.

At the bottom of this post is a sample html message exhibiting this behaviour.

Using Mozilla 1.6, a proxy challenge can be generated by asking the web browser
to go to an external web site. Once this has been done, a refresh of the mail
message allows everything to display as expected.

==============================================================================
Sample mail message illustrating this bug:
==============================================================================

Return-path: <M8jAMwIBAQAGZLIACAAAAAAEL6/q8w@cnl08.b.tep1.com>
Received: from proxy2.komatsu.ca (proxy2.komatsu.ca [XXX.XXX.XXX.XXX])
 by messenger.komatsu.ca
 (iPlanet Messaging Server 5.2 HotFix 1.12 (built Feb 13 2003))
 with SMTP id <JS4L08PH7AKS4N02YW@messenger.komatsu.ca> for
 wbayer@ims-ms-daemon; Fri, 06 Feb 2004 11:28:44 -0500 (Eastern Standard Time)
Received: From out005.toptp.com ([38.113.202.25])
 by proxy2.komatsu.ca (WebShield SMTP v4.5 MR1a P0803.345)
 ;	id 1076084922979; Fri, 06 Feb 2004 11:28:42 -0500
Date: Fri, 06 Feb 2004 08:13:31 -0800
From: FurnitureFan <Newsletter@FurnitureFan.com>
Subject: Furniture Newsletter
To: wbayer@komatsu.ca
Errors-to: M8jAMwIBAQAGZLIACAAAAAAEL6/q8w@cnl08.b.tep1.com
Reply-to: Newsletter@FurnitureFan.com
Message-id: <1911963319-1463792638-1076084043@cnl08.b.tep1.com>
MIME-version: 1.0
Content-type: multipart/alternative; boundary=1249438995.1463793406.1076084022
Original-recipient: rfc822;wbayer@komatsu.ca

--1249438995.1463793406.1076084022
Content-Type: text/plain

FurnitureFan.com
The Furniture Directory

If you cannot view this message, please visit:
http://cnl08.c.tep1.com/maabU98aa4aGXa6Xxhne/

Romancing Your bedroom
This Valentines Day rekindle the magic and escape your every day life. Create a
seductive environment by transforming your space with any of these elegant
hand-crafted items from Oakwood Interiors.

See more bedroom
http://cnl08.c.tep1.com/maabU98aa4aGYa6Xxhne/

Oakwood Interiors - Alder Rose Mansion Bed
http://cnl08.c.tep1.com/maabU98aa4aGZa6Xxhne/

Build your dreams piece by piece 
with the Alder Rose Mansion Collection from Oakwood Interiors. American made of
solid wood, this collection will stand the test of time with its quality and
exceptional design. 

Alder Rose Vanity
http://cnl08.c.tep1.com/maabU98aa4aG0a6Xxhne/

Alder Rose Night Stand
http://cnl08.c.tep1.com/maabU98aa4aG1a6Xxhne/

====================================================================
Update your profile here:
http://cnl08.u.tep1.com/survey/?a2i7O3.a6Xxhn.d2JheWVy

Unsubscribe here:
http://cnl08.u.tep1.com/survey/?a2i7O3.a6Xxhn.d2JheWVy.u

Delivered by Topica Email Publisher, http://www.email-publisher.com/

--1249438995.1463793406.1076084022
Content-Type: text/html
MIME-Version: 1.0
Content-Transfer-Encoding: quoted-printable

<!DOCTYPE HTML PUBLIC "-//W3C//DTD HTML 4.01 Transitional//EN">
<html>
<head>
<title>FurnitureFan Consumer Newsletter</title>
<meta http-equiv=3D"Content-Type" content=3D"text/html; charset=3Diso-8859-=
1">
<style type=3D"text/css">
<!--
.mnav {


	font-family: Arial, Helvetica, sans-serif;
	font-size: 12px;
	font-weight: bold;
	font-variant: normal;
	color: #333333;
	padding: 8px;
	text-align: left;
	font-style: normal;
	line-height: normal;
	word-spacing: 7px;
	border-top: 1px;
	border-right: 1px;
	border-bottom: 1px;
	border-left: 1px;
	background-color: #F3F1E4;
}
a:link {
	color: #666666;
	text-decoration: none;
}
a:visited {
	color: #666666;
	text-decoration: none;
}
a:hover {
	color: #666666;
	text-decoration: underline;
}
a:active {
	color: #666666;
	text-decoration: none;
}
.greenbox {
	border: 1px solid #D1CCA4;
}
.T1 {
	font-family: Arial, Helvetica, sans-serif;
	font-size: 10pt;
	color: #333333;
	line-height: 20px;
}
.t2 {
	font-family: Arial, Helvetica, sans-serif;
	font-size: 9.5pt;
	color: #333333;
	width: 220px;
	text-align: left;
	padding: 3px;
}
.t3 {
	font-family: Arial, Helvetica, sans-serif;
	font-size: 9pt;
	color: #333333;
}
.t4 {
	font-family: Arial, Helvetica, sans-serif;
	font-size: 8.6pt;
	color: #303030;
	padding: 5px;
	font-weight: bold;
	text-align: center;
}
.white-top {
	border-top-width: 1px;
	border-top-style: solid;
	border-top-color: #FFFFFF;
}
-->
</style>
</head>

<body leftmargin=3D"0" topmargin=3D"0">
<table width=3D"630" border=3D"0" align=3D"center" cellpadding=3D"0" cellsp=
acing=3D"0">
  <tr>=20
    <td><FONT face=3D"Arial, Helvetica, sans-serif" size=3D1><a href=3D"htt=
p://cnl08.c.tep1.com/maabU98aa4aGy=
a6Xxhne/">If=20
      you cannot view this message, please visit: http://www.furniturefan.c=
om/ECP/jumppage.aspx?linkid=3D3608</a><br>
      To continue receiving your FurnitureFan Newsletter without interrupti=
on,=20
      please add =93Newsletter@FurnitureFan.com=94 to <br>
      your email Address Book.</FONT></td>
  </tr>
  <tr>=20
    <td><a href=3D"http://cnl08.c.tep1.com/maabU98aa4aGz=
a6Xxhne/"><img src=3D"http://www2.furniturefan.com/ecp/cn/0013/images/Furni=
tureFan.gif" width=3D"180" height=3D"34" border=3D"0"></a></td>
  </tr>
  <tr>=20
    <td class=3D"mnav"><div align=3D"center"><a href=3D"http://cnl08.c.tep1=
.com/maabU98aa4aGA=
a6Xxhne/">Living=20
        Room</a> <font color=3D"#972D38">|</font> <a href=3D"http://cnl08.c=
.tep1.com/maabU98aa4aGB=
a6Xxhne/">Dining=20
        Room</a> <font color=3D"#972D38">|</font> <a href=3D"http://cnl08.c=
.tep1.com/maabU98aa4aGC=
a6Xxhne/">Bedroom</a>=20
        <font color=3D"#972D38">|</font> <a href=3D"http://cnl08.c.tep1.com=
/maabU98aa4aGD=
a6Xxhne/">Entertainment</a>=20
        <font color=3D"#972D38">|</font> <a href=3D"http://cnl08.c.tep1.com=
/maabU98aa4aGE=
a6Xxhne/">Kids</a>=20
        <font color=3D"#972D38">|</font> <a href=3D"http://cnl08.c.tep1.com=
/maabU98aa4aGF=
a6Xxhne/">Office</a>=20
        <font color=3D"#972D38">|</font> <a href=3D"http://cnl08.c.tep1.com=
/maabU98aa4aGG=
a6Xxhne/">Accent=20
        Furniture</a></div></td>
  </tr>
  <tr>=20
    <td height=3D"5" bgcolor=3D"#FFFFFF">&nbsp;</td>
  </tr>
  <tr>=20
    <td class=3D"greenbox"><table width=3D"630" border=3D"0" cellspacing=
=3D"3" cellpadding=3D"0">
        <tr>=20
          <td width=3D"276" valign=3D"top" bgcolor=3D"E7E2C8"> <table width=
=3D"276" border=3D"0" cellspacing=3D"0" cellpadding=3D"0">
              <tr>=20
                <td><img src=3D"http://www.furniturefan.com/ecp/cn/0016/ima=
ges/bnr.gif" width=3D"276" height=3D"94"></td>
              </tr>
              <tr>=20
                <td style=3D"padding-left: 15px; padding-right:15px; paddin=
g-top:4px; padding-bottom:15px"><div class=3D"t1">=20
                    <p>This Valentines Day rekindle the magic and escape yo=
ur=20
                      every day life. Create a seductive environment by tra=
nsforming=20
                      your space with any of these elegant hand-crafted ite=
ms=20
                      from <strong>Oakwood Interiors</strong>.</p>
                  </div></td>
              </tr>
              <tr>=20
                <td style=3D"padding-top:4px; padding-bottom:15px; padding-=
left:5px;"><a href=3D"http://cnl08.c.tep1.com/maabU98aa4aGH=
a6Xxhne/"><img src=3D"http://www.furniturefan.com/ecp/cn/0016/images/furnit=
ure.gif" width=3D"259" height=3D"59" border=3D"0"></a></td>
              </tr>
            </table></td>
          <td align=3D"left" valign=3D"top"> <div align=3D"left">=20
              <table width=3D"350" height=3D"100%" border=3D"0" cellpadding=
=3D"0" cellspacing=3D"2">
                <tr>=20
                  <td><a href=3D"http://cnl08.c.tep1.com/maabU98aa4aGJ=
a6Xxhne/"><img src=3D"http://www.furniturefan.com/ecp/cn/0016/images/oakwoo=
d_bed.jpg" width=3D"350" height=3D"340" border=3D"0"></a></td>
                </tr>
              </table>
            </div></td>
        </tr>
      </table></td>
  </tr>
  <tr>=20
    <td height=3D"5"></td>
  </tr>
  <tr>=20
    <td bgcolor=3D"972D38" class=3D"greenbox"><table width=3D"630" border=
=3D"0" cellspacing=3D"0" cellpadding=3D"0">
        <tr>=20
          <td width=3D"117"><a href=3D"http://cnl08.c.tep1.com/maabU98aa4aG=
L=
a6Xxhne/"><img src=3D"http://www.furniturefan.com/ecp/cn/0016/images/oakwoo=
d_vanity2.jpg" width=3D"117" height=3D"150" border=3D"0"></a></td>
          <td width=3D"125" align=3D"right"><a href=3D"http://cnl08.c.tep1.=
com/maabU98aa4aGO=
a6Xxhne/"><img src=3D"http://www.furniturefan.com/ecp/cn/0016/images/oakwoo=
d_chest3.jpg" width=3D"124" height=3D"150" border=3D"0"></a></td>
          <td class=3D"T1" style=3D"padding-left:15px;"><font color=3D"E7E2=
C8"><img src=3D"http://www.furniturefan.com/ecp/cn/0016/images/furniture_fa=
n7.gif" width=3D"335" height=3D"30"><br>
            with the Alder Rose Mansion Collection from Oakwood Interiors. =
American=20
            made of solid wood, this collection will stand the test of time=
 with=20
            its quality and exceptional design.</font></td>
        </tr>
      </table></td>
  </tr>
  <tr>
    <td><div align=3D"center"><font color=3D"#333333" size=3D"1" face=3D"Ar=
ial, Helvetica, sans-serif">FurnitureFan.com=20
        142 North Rd., Sudbury, MA 01776</font></div></td>
  </tr>

</table>
<img src=3D"http://www.furniturefan.com/ECP/jumppage.aspx?linkid=3D3593">=
=20
<!-- 0x0fa9bafd8b87adf4ab0a121bfdabe341000 -->
<img src=3D"http://cnl08.c.tep1.com/naabU98aa4aGQ=
a6Xxhne/" width=3D1 height=3D1>
<!-- 0x012390fa6570193887fff02398eeaa99283 -->


<!-- Begin Footer -->
<P>
<CENTER>
        <FONT STYLE=3D"font: 10px Verdana, Sans-Serif;" FACE=3D"Verdana, Sa=
ns-Serif" SIZE=3D"-1">
         <A HREF=3D"http://cnl08.u.tep1.com/survey/?a2i7O3.=
a6Xxhn.=
d2JheWVy">Update your profile</A> or
         <A HREF=3D"http://cnl08.u.tep1.com/survey/?a2i7O3.=
a6Xxhn.=
d2JheWVy.u">unsubscribe</A> here.
        </FONT>
</CENTER>

<CENTER><FONT STYLE=3D"font: 10px verdana, sans-serif;" FACE=3D"Verdana, Sa=
ns-Serif" SIZE=3D"-1">
Delivered by <A HREF=3D"http://www.email-publisher.com/">Topica Email Publi=
sher</a>

</font></center>
</body>
</html>

--1249438995.1463793406.1076084022--

==============================================================================

We are willing to test a patch once available.

Regards,

Bill


Comment 8 Scott MacGregor 2004-02-06 15:14:02 PST
Darin do you remember any changes to our proxy code going in after 1.4 RC1?

I'll start searching the 1.4 branch checkins since we should have branched
before RC1 was released.
Comment 9 benc 2004-03-15 23:38:10 PST
Scott: the proxy servers I used to use got RIF'd, so if you want me to look at
this, I can scheme about configuring something for mozilla.org.

Grant: can you see if password manager functions as a workaround? This might
solve your problem, it also might help understand what is going on.
Comment 10 Darin Fisher 2004-03-15 23:42:08 PST
Yikes!  Is this bug still a problem?  Sounds pretty major.
Comment 11 wbayer 2004-03-19 11:36:39 PST
(In reply to comment #10)
> Yikes!  Is this bug still a problem?  Sounds pretty major.

I confirmed this problem persists using the Windows Mozilla Thunderbird 0.5+
(20040312) build on Windows 2000, both with the "Automatic proxy configurarion
URL" selected under Advanced Options or using manual proxy preferences.

We have both the Linux Squid and Netscape 3.6 proxy servers. 

Also seen on occasion (however difficult to replicate): The first time an
external URL link in an e-mail message is clicked, Firefox 0.8 (also configured
with the "Automatic proxy configurarion URL" option) is started, however no
proxy challenge appears. Clicking the link a second time does result in the
proxy challenge being initiated.

We would like to assist in resolving this bug. Please advise how we can help.

Comment 12 benc 2004-04-18 20:45:50 PDT
Darin: I think you are going to need to decide if this needs to be fixed. In the
context of 1.7, integrated products like Mozilla have a build in work-around.

Scott: I do not closely track Thunderbird bugs, but I would strongly recommend
fixing this before your next Thunderbird release.
Comment 13 Scott MacGregor 2004-04-22 15:47:17 PDT
this bug was caused by the security drill:

http://bugzilla.mozilla.org/show_bug.cgi?id=51631

auth dialogs are disabled in all mail message windows because spammers would use
it to force up a user/pwd dialog and get the user's name and password.
Comment 14 Scott MacGregor 2004-04-22 15:50:26 PDT
Here's where the docshell refuses to return an auth prompter if auth is turned
off for this docshell instance:

http://lxr.mozilla.org/seamonkey/source/docshell/base/nsDocShell.cpp#386

as you can see at this point in time we have no idea what kind of auth request
we are being asked for (legit proxy or other). 

Comment 15 Scott MacGregor 2004-04-22 22:29:05 PDT
*** Bug 221772 has been marked as a duplicate of this bug. ***
Comment 16 Scott MacGregor 2004-04-22 22:38:00 PDT
Question for Darin :)

In fixing the big security bug, an attribute was added to nsIDocShell for
blocking secure auth prompts. Instead of fixing all of the callers that try to
get a nsIAuthPrompter from the docshell to bring up auth UI, it looks like we
hide this in nsIDocShell::GetInterface. Failing to return the auth prompt if
this flag has been set on the docshell. 

Of course GetInterface has no way of knowing what the auth prompt is going to be
used for. In the case of a legitimate proxy request, we may want to allow it. 

Seems to me we could solve this a couple of ways:

1) Always return a nsIAuthPrompter, put the burden on the caller to check and
see if they should be allowed to use the returned auth prompter. My fear here is
that we make it too easy for the security hole to show back up for new code.

2) I wonder if we could have a special way for the http channel to still get the
auth prompter even if the flag is set on the docshell object and the http
channel (and just this channel) could check to see if we are doing proxy auth
before actually using the auth prompt. 

3) Remove nsIAuthPrompter from nsIDocShell::GetInterface. Make an explicit
method for getting the auth prompt from a docshell. Force all callers to pass in
a boolean flag for proxy which determines if we return an auth prompter or not.

Any other ideas? 
Comment 17 Scott MacGregor 2004-04-27 08:52:20 PDT
*** Bug 241846 has been marked as a duplicate of this bug. ***
Comment 18 wbayer 2004-05-01 11:20:06 PDT
Echoing comment #12, I would strongly recommend fixing this before your next
Thunderbird release.
Comment 19 benc 2004-05-10 09:16:11 PDT
Scott: as we discussed before, you could consider supporting proxy via a fixed
pref format. Take a look at Instant Messenger module of Netscape 7 in the prefs.

Also, this bug seems to require the same changes as bug 112179, so maybe the
combined advantages could make a case of implementing this.

Comment 20 Tristan Nitot 2004-05-16 09:05:56 PDT
This is a blocker for a major French account on Thunderbird. Mozilla Europe
would *love* to see this bug fixed.
Comment 21 Malin 2004-05-17 00:32:21 PDT
It is also blocking Glocalnet in Sweden.
Comment 22 Asa Dotzler [:asa] 2004-05-27 10:46:14 PDT
Doesn't look like this is gonna happen for 1.7. 
Comment 23 chris hofmann 2004-05-27 10:54:14 PDT
looks like this is going to need some redesign to avoid security problems and
still provide what it needed.  we are going to have to put this off for 1.7 and
try for 1.8, with the hope that a patch might be able to be back ported for
1.7.1 and tbird 1.0...
Comment 24 Darin Fisher 2004-06-01 07:40:41 PDT
A major problem is that the nsIAuthPrompt implementation does not have
sufficient context to determine whether or not to show a prompt.  I think the
solution to this bug lies in improving the nsIAuthPrompt API so that it has
sufficient context.  In this case, that means telling the prompt whether or not
it is being invoked for proxy auth.  (I can think of other bits of information
that would be valuable for other bugs too... like the degree of password
protection.)

If nsIAuthPrompt were modified so that necko could pass more context to the
prompter, then we would just need to modify the prompter implementation that is
used by mailnews.
Comment 25 Peter Van der Beken [:peterv] 2004-06-01 11:09:57 PDT
Hmm, changing nsIAuthPrompt sounds like it wouldn't be possible to backport to
the long-lived stable branch.
Comment 26 Darin Fisher 2004-06-01 11:59:11 PDT
well, we could also invent nsIAuthPrompt2.  necko could try to use
nsIAuthPrompt2 and fallback to using nsIAuthPrompt if it cannot get a
nsIAuthPrompt2 implementation.  we could continue disabling support for
nsIAuthPrompt in mailnews.
Comment 27 Hendrik de Goede 2004-08-11 00:51:44 PDT
Seems to me this is also blocking RSS support in Thunderbird.
When adding a rss feed, Thunderbird will try to validate the provided URL, it
never finishes.
Comment 28 Vladimir Vukicevic [:vlad] [:vladv] 2004-08-28 04:08:23 PDT
Created attachment 157248 [details] [diff] [review]
mailnews-proxy-auth-0.patch

How's this for an attempt?

I added a nsIAuthPromptRequestor interface that takes flags indicating what
type of auth request is wanted (well, just 0 or 1 for now, other flags in the
future as necessary).  nsHttpChannel then tries to get this interface first and
use it to get an nsIAuthPrompt with flags; otherwise it falls back to just
doing a GetInterface.

The only problem I see is I don't see how this approach (or any other)
maintains the lockdown which caused this problem in the first place --
nsHttpChannel decides whether to tell AuthPromptRequester it wants a proxy auth
dialog based on its is proxyAuth flag, which is set if the HTTP response code
is 407 as opposed to 401.  This means that all a malicious site would have to
do would be to return a 407 instead of a 401, and HttpChannel would have no way
of knowing whether it came from a legitimate proxy or not.  Not sure how to fix
this or even if we can; we can maybe ensure that there was a proxy configured
and that the auth request is coming from there, but that'll fail in the
(admittedly slightly contrived) case of a transparent http proxy that wants
auth.
Comment 29 Vladimir Vukicevic [:vlad] [:vladv] 2004-08-28 13:30:35 PDT
Comment on attachment 157248 [details] [diff] [review]
mailnews-proxy-auth-0.patch

Darin, could you take a look at this?  Or please suggest another reviewer if
you're not the best person :)  See comments above for patch notes.
Comment 30 Darin Fisher 2004-08-29 10:22:52 PDT
Vlad: this patch makes sense to me.  I haven't done a thorough review yet, but I
too am concerned about the 407 from origin server case.  Currently, we prevent
exposing cached proxy credentials when a 407 occurs over a non-proxy connection.
 We just treat the 407 as coming from the origin server instead of from the
proxy server.  That affects the password manager key used, which affects what is
prefilled in the auth prompt.  But, we preserve the fact that this is a proxy
auth attempt.  We should probably set proxyAuth to false inside
nsHttpChannel::GetCredentialsForChallenge at the right place to be consistent.

I'll need to read the RFCs again to be sure, but I think we can probably assume
that 407 can only rightfully be generated by an explicit proxy server. 
Transparent proxies are probably not permitted to generate that error code,
although folks may be abusing that restriction in the real world.
Comment 31 Vladimir Vukicevic [:vlad] [:vladv] 2004-08-29 10:52:50 PDT
Created attachment 157339 [details] [diff] [review]
force proxyAuth to false if no proxy server configured

Something like this?  As long as any legitimate proxy eats any 407's that it
receives itself from a site, this should be fine -- does the spec require that?
Comment 32 Christian :Biesinger (don't email me, ping me on IRC) 2004-08-30 12:14:45 PDT
Comment on attachment 157248 [details] [diff] [review]
mailnews-proxy-auth-0.patch

if you do want to go down a route like this, I would instead change
nsIAuthPrompt to add a "boolean isProxyAuth" param, or something like that.
that interface is suboptimal anyway, with requiring to pass the entire dialog
text to it...
Comment 33 Darin Fisher 2004-08-30 16:18:34 PDT
If we add the isProxyAuth param to nsIAuthPrompt then perhaps we don't need
nsIAuthPromptRequestor.  I assumed the point of nsIAuthPromptRequestor was to
avoid having to change nsIAuthPrompt.  Is that not right?
Comment 34 Vladimir Vukicevic [:vlad] [:vladv] 2004-08-30 16:31:38 PDT
(In reply to comment #33)
> If we add the isProxyAuth param to nsIAuthPrompt then perhaps we don't need
> nsIAuthPromptRequestor.  I assumed the point of nsIAuthPromptRequestor was to
> avoid having to change nsIAuthPrompt.  Is that not right?

That's correct (I actually typed up a reply earlier this morning, but just now
notice that the bugzilla tab is sitting at the password prompt..)

It seems cleaner to follow the current practice of not giving out a
nsIAuthPrompt at all if the user shouldn't be prompted; hence the Requestor,
with added flags.  Otherwise, the decision whether to display a prompt or not
has to be moved into a nsIAuthPrompt implementation.
Comment 35 Darin Fisher 2004-08-30 16:42:02 PDT
At some point, I do wish to provide a better nsIAuthPrompt.  We should give the
prompt the knowledge that it is prompting for proxy auth vs. normal auth, etc. 
But, there are many other things that should be included in a reworking of such
an interface.  For now, I'm happy with nsIAuthPromptRequestor.  It could be a
low risk addition to the stable branches too.
Comment 36 Scott MacGregor 2004-08-30 16:50:47 PDT
this is going to be a very welcome fix for the aviary branch!
Comment 37 Darin Fisher 2004-08-30 17:16:40 PDT
Created attachment 157455 [details] [diff] [review]
v2 patch for nsHttpChannel::ProcessAuthentication

Take a hard line and reject 407 when proxy server not configured.  This is
consistent with IE.
Comment 38 Darin Fisher 2004-08-30 17:29:31 PDT
Comment on attachment 157248 [details] [diff] [review]
mailnews-proxy-auth-0.patch

>+[scriptable, uuid(129d3bd5-8a26-4b0b-b8a0-19fdea029196)]
>+interface nsIAuthPromptRequestor : nsISupports
>+{
>+    /* Normal (non-proxy) prompt request. */
>+    const PRUint32 PROMPT_NORMAL        = 0;
>+
>+    /* Proxy auth request. */
>+    const PRUint32 PROMPT_PROXY         = 1;
>+
>+    /**
>+     * Request a nsIAuthPrompt interface for the given flags;
>+     * throws NS_ERROR_FAILURE if no prompt is allowed or
>+     * available for the given reason.
>+     */
>+    nsIAuthPrompt requestAuthPrompt(in PRUint32 aPromptReason);
>+};

please create a new IDL file for this interface, and provide a
summary comment of some sort.  use doxygen style commenting for
params and exceptions.

should this be named nsIAuthPromptProvider instead?  oh, i guess
nsIAuthPromptRequestor corresponds to nsIInterfaceRequestor, so
maybe getAuthPrompt to correspond with getInterface?  whatever ;-)


>Index: docshell/base/nsDocShell.cpp


>+nsDocShell::RequestAuthPrompt (PRUint32 aPromptReason, nsIAuthPrompt **aResult)

GNU whitespace alert!  "when in rome, stay in rome" :-)


>+{
>+    // a priority prompt request will override a false mAllowAuth setting
>+    PRBool priorityPrompt = PR_FALSE;
>+
>+    if (aPromptReason & nsIAuthPromptRequestor::PROMPT_PROXY)
>+        priorityPrompt = PR_TRUE;

maybe this:

  priorityPrompt = aPromptReason & nsIAuthPromptRequestor::PROMPT_PROXY;


>Index: netwerk/protocol/http/src/nsHttpChannel.cpp

>@@ -2304,22 +2304,37 @@ nsHttpChannel::PromptForIdentity(const c

>+        if (proxyAuth)
>+            rv = authPromptRequestor->RequestAuthPrompt (nsIAuthPromptRequestor::PROMPT_PROXY, getter_AddRefs(authPrompt));
>+        else
>+            rv = authPromptRequestor->RequestAuthPrompt (nsIAuthPromptRequestor::PROMPT_NORMAL, getter_AddRefs(authPrompt));

nit: please fix whitespace and put a linebreak somewhere in those long lines.


>+
>+        if (NS_FAILED(rv))
>+            return NS_ERROR_NO_INTERFACE;

no need to assign or check rv really.  the !authPrompt check down below is good
enough, no?  also, how about making only one call to RequestAuthPrompt?  store
the prompt type in a variable, and then make only one call.  help the compiler
optimize a bit ;-)


>+    } else {
>+        GetCallback(NS_GET_IID(nsIAuthPrompt), getter_AddRefs(authPrompt));
>+    }

please keep braces consistent with the rest of this file, which means:

    }
    else
	GetCallback(NS_GET_IID(nsIAuthPrompt), getter_AddRefs(authPrompt));
Comment 39 Christian :Biesinger (don't email me, ping me on IRC) 2004-08-31 01:29:50 PDT
(In reply to comment #33)
> I assumed the point of nsIAuthPromptRequestor was to
> avoid having to change nsIAuthPrompt.  Is that not right?
(In reply to comment #34)
> It seems cleaner to follow the current practice of not giving out a
> nsIAuthPrompt at all if the user shouldn't be prompted; 

those are good points. so, nevermind comment 32. :)

Index: netwerk/base/public/nsIAuthPrompt.idl
+     * throws NS_ERROR_FAILURE if no prompt is allowed or
+     * available for the given reason.

if no prompt is available, maybe NS_ERROR_NOT_AVAILABLE would be better? :)

Index: docshell/base/nsDocShell.cpp
+    if (aPromptReason & nsIAuthPromptRequestor::PROMPT_PROXY)

hm... wouldn't a reason be more like an enum, so that == would be a better
comparison?

+nsDocShell::RequestAuthPrompt (PRUint32 aPromptReason, nsIAuthPrompt **aResult)
...
+        *aResult = nsnull;
+        return NS_ERROR_FAILURE;

shouldn't need to null out out params in failure cases
Comment 40 Mike Shaver (:shaver -- probably not reading bugmail closely) 2004-08-31 08:54:16 PDT
nsIInterfaceRequestor is not an interface to emulate in form, function or
spirit.  (The requestor is the _caller_, not the object on which you're calling
getInterface; likewise here.  Nobody wants nsIAuthPromptRequestee, though -- we
really care that it provides it, not that we can ask for it.)
Comment 41 Vladimir Vukicevic [:vlad] [:vladv] 2004-08-31 09:41:22 PDT
(In reply to comment #40)
> nsIInterfaceRequestor is not an interface to emulate in form, function or
> spirit.  (The requestor is the _caller_, not the object on which you're calling
> getInterface; likewise here.  Nobody wants nsIAuthPromptRequestee, though -- we
> really care that it provides it, not that we can ask for it.)

So, nsIAuthPromptProvider?  Maybe nsIAuthMeHarder?  I'm open to suggestions. :)
Comment 42 Scott MacGregor 2004-08-31 09:48:30 PDT
*** Bug 221772 has been marked as a duplicate of this bug. ***
Comment 43 Mike Shaver (:shaver -- probably not reading bugmail closely) 2004-08-31 12:34:56 PDT
Comment on attachment 157455 [details] [diff] [review]
v2 patch for nsHttpChannel::ProcessAuthentication


>-    if (proxyAuth)
>+    if (proxyAuth) {
>+        // only allow a proxy challenge if we have a proxy server configured.
>+        // otherwise, we could inadvertantly expose the user's proxy
>+        // credentials to an origin server.  We could attempt to proceed as
>+        // if we had received a 401 from the server, but why risk flirting
>+        // with trouble?  IE similarly rejects 407s when a proxy server is
>+        // not configured, so there's no reason not to do the same.
>+        if (!mConnectionInfo->UsingHttpProxy()) {
>+            LOG(("rejecting 407 when proxy server not configured!\n"));
>+            return NS_ERROR_UNEXPECTED;
>+        }
>         challenges = mResponseHead->PeekHeader(nsHttp::Proxy_Authenticate);
>+    }
>     else
>         challenges = mResponseHead->PeekHeader(nsHttp::WWW_Authenticate);

Brace the else to match the then?  (Or is that not this Rome's style?)

>-    if (proxyAuth && mConnectionInfo->UsingHttpProxy()) {
>+    if (proxyAuth) {
>         host = mConnectionInfo->ProxyHost();
>         port = mConnectionInfo->ProxyPort();
>         ident = &mProxyIdent;
>         scheme.AssignLiteral("http");
>     }

How would you feel about an assertion there?  I'll see your belt, and raise you
some suspenders!
Comment 44 Vladimir Vukicevic [:vlad] [:vladv] 2004-08-31 12:46:51 PDT
(In reply to comment #43)
> (From update of attachment 157455 [details] [diff] [review])
> 
> >-    if (proxyAuth)
> >+    if (proxyAuth) {
> >+        // only allow a proxy challenge if we have a proxy server configured.
> >+        // otherwise, we could inadvertantly expose the user's proxy
> >+        // credentials to an origin server.  We could attempt to proceed as
> >+        // if we had received a 401 from the server, but why risk flirting
> >+        // with trouble?  IE similarly rejects 407s when a proxy server is
> >+        // not configured, so there's no reason not to do the same.
> >+        if (!mConnectionInfo->UsingHttpProxy()) {
> >+            LOG(("rejecting 407 when proxy server not configured!\n"));
> >+            return NS_ERROR_UNEXPECTED;
> >+        }
> >         challenges = mResponseHead->PeekHeader(nsHttp::Proxy_Authenticate);
> >+    }
> >     else
> >         challenges = mResponseHead->PeekHeader(nsHttp::WWW_Authenticate);
> 
> Brace the else to match the then?  (Or is that not this Rome's style?)

No; the Romans are filthy heathens, it looks like.

> >-    if (proxyAuth && mConnectionInfo->UsingHttpProxy()) {
> >+    if (proxyAuth) {
> >         host = mConnectionInfo->ProxyHost();
> >         port = mConnectionInfo->ProxyPort();
> >         ident = &mProxyIdent;
> >         scheme.AssignLiteral("http");
> >     }
> 
> How would you feel about an assertion there?  I'll see your belt, and raise you
> some suspenders!

Sounds good to me; I've rolled in darin's patch into the next one I'll post and
can make this change (or I can unroll it, if you'd prefer to r them separately,
but..)
Comment 45 Christian :Biesinger (don't email me, ping me on IRC) 2004-08-31 14:04:13 PDT
Comment on attachment 157455 [details] [diff] [review]
v2 patch for nsHttpChannel::ProcessAuthentication

r=me, but doesn't the return NS_ERROR_UNEXPECTED; possibly trigger the
superfluous auth warning, which would not really be the right thing here?
Comment 46 Darin Fisher 2004-08-31 14:09:32 PDT
> r=me, but doesn't the return NS_ERROR_UNEXPECTED; possibly trigger the
> superfluous auth warning, which would not really be the right thing here?

of course, that only applies if the URL contains a userpass.
Comment 47 Darin Fisher 2004-08-31 14:11:53 PDT
vlad: combining the patches sounds good to me (less to manage when it comes time
to land this on the branches).  shaver's assertion "suspender" sounds reasonable
too.
Comment 48 Vladimir Vukicevic [:vlad] [:vladv] 2004-08-31 15:29:21 PDT
Created attachment 157543 [details] [diff] [review]
mailnews-proxy-auth-1.patch

Updated patch, with comments incorporated and darin's 407 patch rolled in.
Comment 49 Darin Fisher 2004-08-31 18:56:39 PDT
Comment on attachment 157543 [details] [diff] [review]
mailnews-proxy-auth-1.patch

>Index: netwerk/base/public/nsIAuthPromptProvider.idl

nit: how about a descriptive comment at the top of this file?


>+     * throws NS_ERROR_NOT_AVAILABLE if no prompt is allowed or
>+     * available for the given reason.

nit: use @throws instead?


>Index: netwerk/protocol/http/src/nsHttpChannel.cpp

>+        PRUint32 promptReason = nsIAuthPromptProvider::PROMPT_NORMAL;
>+        if (proxyAuth)
>+            promptReason = nsIAuthPromptProvider::PROMPT_PROXY;

nit: use |if (a) x=y; else x=z;| instead?


r=darin
Comment 50 Brendan Eich [:brendan] 2004-08-31 20:05:30 PDT
> nit: use |if (a) x=y; else x=z;| instead?

Nit on nit: use x = (a) ? y : z; instead, instead? ;-)

/be
Comment 51 Jo Hermans 2004-09-01 04:46:15 PDT
*** Bug 248617 has been marked as a duplicate of this bug. ***
Comment 52 Jo Hermans 2004-09-01 04:46:25 PDT
*** Bug 257636 has been marked as a duplicate of this bug. ***
Comment 53 Oleg 2004-09-01 04:56:40 PDT
Easy workaround for those who suffer - is to press reply on that HTML message, 
Composer window does display proxy auth dialog.
After that all gets to normall behavior untill mail client restart.
Maybe this info would help you solve this?
Comment 54 Mike Shaver (:shaver -- probably not reading bugmail closely) 2004-09-01 15:21:50 PDT
Comment on attachment 157543 [details] [diff] [review]
mailnews-proxy-auth-1.patch

I still want those suspenders, punk.

sr=shaver

aviary+ on the basis of mscott's excitement above!
Comment 55 Vladimir Vukicevic [:vlad] [:vladv] 2004-09-01 21:54:20 PDT
Created attachment 157679 [details] [diff] [review]
mailnews-proxy-auth-2.patch (aviary)

suspenders added, patch for aviary
Comment 56 Vladimir Vukicevic [:vlad] [:vladv] 2004-09-01 21:56:51 PDT
Comment on attachment 157679 [details] [diff] [review]
mailnews-proxy-auth-2.patch (aviary)

Carrying flags forward.. r=darin, sr=shaver, a=shaver/mscott
Comment 57 Vladimir Vukicevic [:vlad] [:vladv] 2004-09-01 22:04:07 PDT
Created attachment 157680 [details] [diff] [review]
mailnews-proxy-auth-2.patch (aviary)

I meant this patch, really (didn't fix the nits in the last one!)
Comment 58 Vladimir Vukicevic [:vlad] [:vladv] 2004-09-01 22:05:57 PDT
Comment on attachment 157680 [details] [diff] [review]
mailnews-proxy-auth-2.patch (aviary)

(Real) patch for aviary; carrying flags forward
r=darin,sr=shaver,a=shaver/mscott
Comment 59 Vladimir Vukicevic [:vlad] [:vladv] 2004-09-01 22:12:45 PDT
Created attachment 157681 [details] [diff] [review]
mailnews-proxy-patch-2.patch (trunk)

same patch as above, in trunk flavour
Comment 60 Vladimir Vukicevic [:vlad] [:vladv] 2004-09-01 22:15:35 PDT
in on trunk and aviary; let me know if it's wanted for 1.7 branch.
Comment 61 Darin Fisher 2004-09-01 22:57:11 PDT
> in on trunk and aviary; let me know if it's wanted for 1.7 branch.

gecko changes so yeah.. probably something that should be ported to the 1.7
branch for consistency with the aviary branch.  moreover, the necko patch
tightens up security a bit, which is probably reason enough to backport this patch.
Comment 62 Vladimir Vukicevic [:vlad] [:vladv] 2004-11-03 13:51:33 PST
landed on 1.7 branch
Comment 63 Ferran Casarramona 2005-04-05 23:31:00 PDT
*** Bug 232856 has been marked as a duplicate of this bug. ***

Note You need to log in before you can comment on or make changes to this bug.