Never prefer SASL PLAIN for JS-XMPP auth even over encrypted connections

RESOLVED FIXED in Instantbird 43

Status

defect
RESOLVED FIXED
4 years ago
3 years ago

People

(Reporter: aleth, Assigned: aleth)

Tracking

(Depends on 1 bug)

trunk
Instantbird 43
Dependency tree / graph

Thunderbird Tracking Flags

(thunderbird47+ fixed, thunderbird48 fixed, thunderbird49 fixed, thunderbird_esr45 fixed)

Details

Attachments

(2 attachments)

Assignee

Description

4 years ago
Currently if the server sends the supported auth mechanisms in an order such that the first one we support is PLAIN, we use that even if there are better mechanisms available, as long as the connection is encrypted.

This isn't really a problem, but we might as well pick a better method.
Assignee

Comment 1

4 years ago
Attachment #8646552 - Flags: review?(clokep)
Comment on attachment 8646552 [details] [diff] [review]
neverplain.diff

Review of attachment 8646552 [details] [diff] [review]:
-----------------------------------------------------------------

How difficult would it be to have tests that just respond with the mechanisms and ensure that we choose the proper one with different sets of options?
Attachment #8646552 - Flags: review?(clokep) → review+
Assignee

Updated

4 years ago
Keywords: checkin-needed
Assignee

Comment 3

4 years ago
(In reply to Patrick Cloke [:clokep] from comment #2)
> How difficult would it be to have tests that just respond with the
> mechanisms and ensure that we choose the proper one with different sets of
> options?

Filed as a separate bug as it will require separating out some of this code into a helper function that can be tested without a fakeserver.
Assignee

Updated

4 years ago
Depends on: 1195097
Assignee

Comment 4

4 years ago
https://hg.mozilla.org/comm-central/rev/c37d201e97fa9f2baa9fe0f8edbbb41bf59c6498
Bug 1193494 - Never prefer SASL PLAIN for JS-XMPP auth even over encrypted connections. r=clokep
Assignee

Updated

4 years ago
Assignee: nobody → aleth
Status: NEW → RESOLVED
Closed: 4 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → Instantbird 43
Assignee

Updated

3 years ago
Depends on: 1267410
Assignee

Comment 5

3 years ago
https://hg.mozilla.org/comm-central/rev/5c96590fcd6cd2fb3d6e64a1a249c06e41aaac8f
Backed out bug 1193494 (changeset c37d201e97fa) as DIGEST-MD5 caused a regression for some users. rs=bustage-fix
Assignee

Comment 6

3 years ago
This backout should be uplifted to esr45. (As stated in the bug description, this patch is not essential, so there is no reason to keep it while it is causing regressions.)
Flags: needinfo?(rkent)
Assignee

Updated

3 years ago
Depends on: 1267649

Updated

3 years ago
Blocks: 1268081
Backout done for TB 45
Flags: needinfo?(rkent)

Comment 9

3 years ago
Reminder for Kent.
Attachment #8746727 - Flags: approval-comm-beta?

Updated

3 years ago
Flags: needinfo?(rkent)
The approval-comm-beta? is sufficient to get my attention when I do landings for beta.
Flags: needinfo?(rkent)
Comment on attachment 8746727 [details]
KENT DONT FORGET THE BACKOUT on Beta

Interesting use of a patch to send a message. But it works!

http://hg.mozilla.org/releases/comm-beta/rev/5ea3c40618e7
Attachment #8746727 - Flags: approval-comm-beta? → approval-comm-beta+
BTW I am not currently monitoring tracking flags other than for major releases. I'm not saying that is the right thing to do, but it is the current practice.
You need to log in before you can comment on or make changes to this bug.