Closed Bug 266828 Opened 20 years ago Closed 15 years ago

Subscribing to a "write only" newsgroup causes Thunderbirds password manager to fail.

Categories

(Thunderbird :: Security, defect)

defect
Not set
major

Tracking

(Not tracked)

RESOLVED DUPLICATE of bug 435306

People

(Reporter: awazabell, Unassigned)

Details

(Whiteboard: [patchlove][has patch needs owner])

Attachments

(4 files, 1 obsolete file)

User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.1; rv:1.7.3) Gecko/20040913 Firefox/0.10.1 Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; rv:1.7.3) Gecko/20040913 Firefox/0.10.1 The password manager breaks and TBird hangs when I subscribe to a "write only" newsgroup. I'm subscribed to several newsgroups (all on the same NNTP server) in which one of the subscriptions has "write only" properties. If I'm not subscribed to the "write only" group, all is well, but when I subscribe to it I get the standard "A News (NNTP) error occurred: no permission" both in 1.7b, 0.5+ and Tbird 0.8 (expected behavior) and when I try and access the other subscriptions it always asks for a user name and password whether I check the "save password" dialog box or not (obviously when not). Reproducible: Always Steps to Reproduce: 1.Subscribe to various newsgroups including the "write only" group on the same NNTP server. 2.Check dialog to remember password when prompted. 3.Access the "write only" newsgroup. Expected Results: Thunderbird should remember the password saved in the password manager. There is only one known workaround that I've been able to implement. Create a seperate profile for the sole purpose of accessing the "write only" subscription.
See comments at the beginning of the log file.
See comments at the beginning of the log file.
See comments at the beginning of the log file.
See comments at the beginning of the log file.
(In reply to comment #1) > can we get an NNTP protocol log: > > http://www.mozilla.org/quality/mailnews/mail-troubleshoot.html#imap Hi David, Thanks for responding so quickly. I hope these logs help. Aaron
Thanks for all the logs, Aaron. What happens is simple: if (m_responseCode == 502) { /* forget the password & username, since login failed */ rv = m_newsFolder->ForgetGroupUsername(); rv = m_newsFolder->ForgetGroupPassword(); } And that's ok since according to RFC 2980 502 means the user/password combination is not valid. In my opinion trying to determine a wrong login and discarding the password is a weakness in the way Mozilla works. It's well meant but it always causes trouble in Mail and also here in News. If we don't change the fundamental way it works or at least stop forgetting passwords at all, there's only one thing I can think of to help here. We could remove the above code from NewsResponse() at let solely the authentication functions AuthorizationResponse() and PasswordResponse() react with forgetting credentials after a 502 response. Secondly reaction on MK_NNTP_RESPONSE_PERMISSION_DENIED in NewsResponse() needs to be modified. Currently it only leaves and directly returns to NewsResponse() since m_nextState doesn't get updated. So it waits for new bytes forever.
Status: UNCONFIRMED → NEW
Ever confirmed: true
OS: Windows XP → All
Hardware: PC → All
Thanks for your help Christian, Hmm, I think I'm following you on this. If the lines of code were adjusted to what you mentioned it would eliminate the problem with the password manager but TBird would still be "broken" for this particular kind of situation. (Please bear with me as I've only taken introductory programming classes, so my comments may be oversimplified or incorrect or both) Now, to correct the "real" problem this would entail some major overhaul of TBirds source code correct? Or is it potentially a rewrite of a few lines of code? Regardless, I'd be happy to take a look at the source and make a feeble attempt to correct the problem, just point me in the right direction as far as where I should start looking in the source code, I'll give it my best shot. Aaron
(In reply to comment #8) > If the lines of code were adjusted to what you mentioned it would eliminate > the problem with the password manager but TBird would still be "broken" for > this particular kind of situation. Since I'm not that familar with the NNTP code I can't say if what I suggested doesn't create a problem somewhere else. But I think it should correct this issue. > Now, to correct the "real" problem this would entail some major overhaul of > TBirds source code correct? Or is it potentially a rewrite of a few lines of > code? IMO the "real problem" is that we try to detect when it's right to forget the password to be nice to the user. But in reality there are numerous situations (answers unexpected by the coder due to not fully reading the spec or real faulty servers a.s.o.) where this leads to problems. To help here some deeper changes and rewrites would be needed, yes. But a) such changes are always dangerous too and b) if what I wrote about works, it's unnecessary regarding your issue. > Regardless, I'd be happy to take a look at the source and make a feeble > attempt to correct the problem, just point me in the right direction as far as > where I should start looking in the source code, I'll give it my best shot. We don't expect you do to it. But to get an idea of the code, visit http://lxr.mozilla.org/seamonkey/source/mailnews/news/src/nsNNTPProtocol.cpp and search for the functions I mentioned. I can develope a patch based on what I proposed. But what would be needed before applying or even reviewing it are some tests on a real account with a write-only newsgroup. CC'ing David - maybe you can say something about password forgetting.
(In reply to comment #9) > Since I'm not that familar with the NNTP code I can't say if what I suggested > doesn't create a problem somewhere else. But I think it should correct this issue. Right, even with the limited experience I have in programming, changing a few lines of code can cause everyone to have a real bad day. > I can develope a patch based on what I proposed. But what would be needed before > applying or even reviewing it are some tests on a real account with a write-only > newsgroup. I'd be more than happy to beta test. :)
(In reply to comment #9) In regards to, > http://lxr.mozilla.org/seamonkey/source/mailnews/news/src/nsNNTPProtocol.cpp Is it possible that lines 1562-8 or 1567-8 are causing the loop problem? 1562 } 1563 else { 1564 m_nextState = m_nextStateAfterResponse; 1565 } 1566 1567 PR_FREEIF(line); 1568 return(0); /* everything ok */ http://lxr.mozilla.org/seamonkey/source/mailnews/news/src/nsNNTPProtocol.cpp#1562 Esp. 1567-8 since these are a duplicate of what happens immediately after 1558 --------------- 1558 else if (MK_NNTP_RESPONSE_PERMISSION_DENIED == m_responseCode) 1559 { 1560 PR_FREEIF(line); 1561 return (0); 1562 } --------------- Or possibly 1560-1 being part of the issue. I dunno, I'm probably barking up the wrong tree. I should probably just build TBird and play with the code a little and see what happens, rather than asking questions that potentially take up unnecessary space.
Attached patch example patch (obsolete) (deleted) — Splinter Review
This patch is what I wrote about in comment #7. It removes the NewsResponse()'s own 502-handling and makes it just throw an alert. To prevend Mozilla from waiting for further data from the server after the 502 (while the server waits for another command from us), m_nextState must get changed to something else than it currently is (NNTP_RESPONSE). The question is into what. Technically what my patch does (just continue on our plan) works (at least here on my simulated news server) for the specific problem since DisplayNewsRCResponse() (the function at the next state) can handle unexpected responses. But a) What happens if the response commes on more groups or while marking articles read (for whatever reason). This could result in hundred alerts and no way for the user to cancel. b) What if the next function isn't able to handle an unexpected response? Additionally it would be great to not only quote the server response (e.g. "no permission") but also inform the user, what command wasn't permitted. But IMO this would require a facility to save our last command.
(In reply to comment #12) > This patch is what I wrote about in comment #7. > It removes the NewsResponse()'s own 502-handling and makes it just throw an > alert. Thanks, I'll give it a try. Might be a few days though. Sorry I've not responded lately, was on vacation.
I am seeing far too many *502* errors while working the authenticated staff work groups on news.annexcafe.com. If I can't type a reply and post it within 35 seconds of opening a post, my send is rebuffed by the server with a 502 error. This is not happening to any of the OE using staffers. MS on this issue is very user friendly. Only guess is that OE is auto resending the ID/Pass as a packet with the NNTP posting. No proxies, direct access to the host server..
(In reply to comment #14) > I am seeing far too many *502* errors while working the authenticated staff work > groups on news.annexcafe.com. If I can't type a reply and post it within 35 > seconds of opening a post, my send is rebuffed by the server with a 502 error. > > This is not happening to any of the OE using staffers. MS on this issue is very > user friendly. Only guess is that OE is auto resending the ID/Pass as a packet > with the NNTP posting. > > No proxies, direct access to the host server.. The *502* errors are happening with a valid user id/pass combo with very high frequency during TB startup as it auto gets msgs from all accounts. Two servers are requiring id/pass, news.annexcafe.com and news.mozdev.org where the annexcafe requirement is for staff access to staff limited groups while the mozdev purpose is controlled general server access. I have yet to have a problem with the mozdev NNTP server. My concern is for cases where there is a mixed subscription of public and controlled access groups on a single specific NNTP server. While the specs say a *502* is an id/pass failure the reality is this error will happen during authentication collisions on the NNTP server. Additionaly if the NNTP server has an invalid id/pass limit of 3 per 30 minutes it can block all access's even by valid id/pass access requests. This is the case for annexcafe.com were the rule is set to block hacker intrusions and spam assults. The assumtion that all *502* errors are UA caused is invalid and TB should have a rule that permits a three strikes and your out before deleting id/pass. Thus permitting the user to execute two retries. Also would be nice to get a warning with the error alert that TB will delete the id/pass so users know what is happening.
This appears to be another manifestation of bug 286628. *** This bug has been marked as a duplicate of 286628 ***
Status: NEW → RESOLVED
Closed: 18 years ago
Resolution: --- → DUPLICATE
This bug is still not resolved in v.1.5.0.5. and is unrelated to bug 286628. That bug is a periodic, seemingly at random, forgetting of the user ID and password. This bug is always repeatable, always occurs, so long as the "write only" newsgroup is subscribed to. I'm not sure what else to do here. Bug 278318 is a duplicate of this one.
Status: RESOLVED → REOPENED
Resolution: DUPLICATE → ---
Version: unspecified → 1.5
(In reply to comment #17) > This bug is still not resolved in v.1.5.0.5. and is unrelated to bug 286628. > That bug is a periodic, seemingly at random, forgetting of the user ID and > password. This bug is always repeatable, always occurs, so long as the "write > only" newsgroup is subscribed to. I'm not sure what else to do here. Bug 278318 > is a duplicate of this one. > I am wondering if this is a problem with UOP newsgroups. I too have issues with the write-only access newsgroup folder. TB stalls when checking the write-only newsgroup folder. There seems to be a line missing that says to finish checking the rest of the groups after acknowledgment of the "News (NNTP error: no permission" box comes up. I had seen this activity when I reported bug #359989.
(In reply to comment #18) > I am wondering if this is a problem with UOP newsgroups. I too have issues > with the write-only access newsgroup folder. TB stalls when checking the > write-only newsgroup folder. There seems to be a line missing that says to > finish checking the rest of the groups after acknowledgment of the "News (NNTP > error: no permission" box comes up. I had seen this activity when I reported > bug #359989. > Yes, that is correct. This is the primary reason for opening the bug. Though, obviously, it may not be limited only to UOP "write only" newsgroups.
QA Contact: general
Christian, could you please come up with an updated patch? (Patch no longer applies on my tree)
Assignee: mscott → nobody
Status: REOPENED → NEW
joshua wnat to take?
Component: General → Security
QA Contact: general → thunderbird
Whiteboard: [patchlove][has patch needs owner]
Comment on attachment 164731 [details] [diff] [review] example patch Patch has bitrotted. $ patch -p0 --dry-run < ~/Desktop/attachment-1.cgi.txt patching file mailnews/news/src/nsNNTPProtocol.cpp Hunk #1 FAILED at 1601. Hunk #2 FAILED at 1609. 2 out of 2 hunks FAILED -- saving rejects to file mailnews/news/src/nsNNTPProtocol.cpp.rej
Attachment #164731 - Attachment is obsolete: true
Gary, do you really think that, after his patch was ignored for 4.5 years, Christian will now JUMP at the chance to update his patch? Maybe Momo's response should be "We're sorry that your patch was ignored for 4.5 years, and now we'll try to unbitrot it so that your effort won't have been a total waste."
> Christian will now JUMP at the chance to update his patch? No, I can't and I won't. Besides I'm out of practice with this code, the path never was more than an example, call it request for comments. But everbody's surely free to unbitrot and play with it.
Looking at the patch, it's merely that Mozilla
Status: NEW → RESOLVED
Closed: 18 years ago15 years ago
Resolution: --- → DUPLICATE
Whoops, got cut off. I meant to say: Looking at the patch, it's merely that Mozilla is forgetting the password on the first authentication error, which is what bug 435306 would aim to fix.
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: