Closed Bug 847032 Opened 12 years ago Closed 11 years ago

[email] IMAP only supports initially-SSL encrypted connections, STARTTLS upgrade not supported from unencrypted cleartext

Categories

(Firefox OS Graveyard :: Gaia::E-Mail, defect)

defect
Not set
normal

Tracking

(blocking-b2g:leo+, b2g18 fixed, b2g-v1.1hd fixed)

RESOLVED FIXED
1.2 FC (16sep)
blocking-b2g leo+
Tracking Status
b2g18 --- fixed
b2g-v1.1hd --- fixed

People

(Reporter: robert, Assigned: mcav)

References

Details

(Keywords: late-l10n)

Attachments

(7 files)

User Agent: Mozilla/5.0 (X11; Ubuntu; Linux x86_64; rv:19.0) Gecko/20100101 Firefox/19.0 Build ID: 20130218103317 Steps to reproduce: tried autoconfig with STARTTLS Actual results: autoconfig with STARTTLS didnt work cause of code only SSL is accepted, but STARTTLS is lookuped too before { * type: 'imap+smtp', * incoming: { * hostname: <imap hostname>, * port: <imap port number>, * socketType: <one of 'plain', 'SSL', 'STARTTLS'>, * username: <imap username>, * }, * outgoing: { * hostname: <smtp hostname>, * port: <smtp port>, * socketType: <one of 'plain', 'SSL', 'STARTTLS'>, * username: <smtp username>, * }, * } // We do not support unencrypted connections outside of unit tests. if (config.incoming.socketType !== 'SSL' || config.outgoing.socketType !== 'SSL') { callback('no-config-info', null, { status: 'unsafe' }); return; Expected results: STARTTLS is secure ,dont forbid it
try with testaccount decribed at http://automx.org/en/
The problem is that mozTCPSocket doesn't support STARTTLS at this time. We had a bug on that, but no bug here in the e-mail components. I've marked this bug as dependent on the mozTCPSocket bug. Thanks for filing this bug!
Status: UNCONFIRMED → NEW
Depends on: 784816
Ever confirmed: true
Summary: autoconfig Gaia::E-Mail only SSL no STARTTLS → [email] IMAP only supports initially-SSL encrypted connections, STARTTLS upgrade not supported from unencrypted cleartext
Nom'd for blocking 1.2 (Koi).
blocking-b2g: --- → koi?
Triage: This bug has been added to the v1.2 backlog but is not blocking release
blocking-b2g: koi? → -
Since bug 784816 has landed, this bug is now actionable. I've moved it into the e-mail app's Pivotal Tracker backlog from the icebox (our pivotal tracker dashboard is at https://www.pivotaltracker.com/s/projects/867311) This primarily entails: * UI ** un-collapse the 'select' combo-boxes in setup_manual_config.html and add startTLS options back. This includes adding a new string for startTLS and an l10n note that indicates the string, like SSL above it, probably should not be translated because it's technical. ** Probably add some logic to the combo-box so that we change the port to 587 when STARTTLS is selected (if previously set to 465) and to 465 when SSL is selected (if previously set to 587). It might make sense to move the select boxes above the port selection so that the user can avoid manually having to type the port-number (and potentially typo it) if they follow the order we put them in. * Backend ** Modify mailapi/worker-support/configparser-main.js to allow socketType values of 'STARTTLS'. Right now we intentionally explode on things that aren't 'SSL'. 'plain' is still an illegal value, as are all other values. ** Uncomment the commented out logic in imap.js in ImapConnection.prototype.connect that has us try and perform the upgrade and fix it up. From https://bugzilla.mozilla.org/show_bug.cgi?id=784816#c56 and friends I think what we basically want is to send our 'STARTTLS' command over the wire, wait for the OK so we know the receiving pipline is clear and there's no chance of TLS upgrade getting confused, then trigger the socket upgrade via startTLS() then immediately call fnInit() so we get the CAPABILITY command in the pipe to trigger the upgrade and get those results back. I think the only reason we'd want to wait for the server to ACK our STARTTLS command with an OK ** Update node-net.js and net-main.js to relay the startTLS upgrade call across. Note that the semantics of upgrade failure as documented in bug 784816 are that the connection will error out and close if we can't upgrade, so bug 911426 on having readyState indicate our security status is not required. Automated testing of this is somewhat annoying, and indeed for that reason we currently don't actually use SSL in our tests. We'll want to add cases as follows, I think 1) in test_autoconfig.js, create a variation on goodImapXML and its uses to make sure we're okay with STARTTLS. but otherwise I think we need to do manual verification for now. Namely: 1) Verify that we can setup a startTLS account with a server serving legit certificates on port 587. Ensure we're really using port 587 and that the connection is actually encrypted by using wireshark or other packet sniffer. 2) Verify that we fail to setup a startTLS account with a server with bad certificates with the appropriate error message. (Localhost dovecot can work for this.) 3) Verify that we fail to setup a startTLS account with a server that doesn't advertise STARTTLS. Ideally quickly. It would suck a bit if we did 'A01 STARTTLS' and it said 'NO' and then we still tried to perform a TLS upgrade but we timed out with the server in the ensuing confusion.
Assignee: nobody → mcav
Status: NEW → ASSIGNED
Attached file SimpleSMTP pull request (deleted) —
Attachment #799627 - Flags: review?(bugmail)
Attached file gelam-pull-request.html (deleted) —
Attachment #799628 - Flags: review?(bugmail)
Attached file Gaia Pull Request (deleted) —
There are three patches here: SimpleSMTP, GELAM, and Gaia. The Gaia PR currently includes all of the other changes (via "make install-into-gaia") because it's probably easier for you to test that way, but I'd be happy to pull that out and only show the UI changes if you'd prefer that, :asuth. This adds support for STARTTLS for both IMAP and SMTP servers. I tested STARTTLS support using an aol.com email account -- while the AOL documentation formally says you're supposed to use SSL ports, they also support STARTTLS for both IMAP and SMTP on the other ports. I verified using Wireshark that the encryption handshake occurs properly and that you can read and send e-mail using STARTTLS connections. Using a local dovecot server with an invalid cert results in an error, as expected ("bad-security"). Connecting to a server that says NO to STARTTLS (tested using netcat) results in immediate connection failure, as expected.
Attachment #799659 - Flags: review?(bugmail)
Target Milestone: --- → 1.2 FC (16sep)
I took a look at this when it went up last week but didn't want to formally review it since I thought the mozTCPSocket startTLS stuff might have regressed us. Which it did, so I backed it out. I'll review this for real once the fixed mozTCPSocket lands again. This looked good in general, but I'll do my test runs, etc. then.
Comment on attachment 799627 [details] SimpleSMTP pull request Looking at the simplesmtp code that we're using and on trunk, its TLS upgrade is opportunistic. If the server does not advertise STARTTLS support then an upgrade will not be initiated. We need to force the TLS upgrade and/or generate a connection error if support is not advertised more directly.
Attachment #799627 - Flags: review?(bugmail) → review-
Comment on attachment 799628 [details] gelam-pull-request.html As noted in the pull request, there's an incoming/outgoing typo. Let's add another autoconfig entry that has STARTTLS not match SSL (only 1 of the 2 variants required). I think we may also want tests in test_imap_prober and test_smtp_prober after all that verify that if the server does not advertise STARTTLS that we get angry or at least we try and initiate a STARTTLS upgrade regardless. This worked for me with the incoming/outgoing change.
Attachment #799628 - Flags: review?(bugmail) → review-
Comment on attachment 799659 [details] Gaia Pull Request Let's add a gaia unit test for the select box change-o magic. The logic looks good and works right when I test it by hand, but this is exactly what the unit test infrastructure is for. (I do *not* believe we need integration test coverage for it, however.) Otherwise good!
Attachment #799659 - Flags: review?(bugmail) → review-
Okay, have another look. I believe I addressed all of your comments: - simplesmtp now takes just a "crypto" flag instead of "secureConnection" and "ignoreTLS", so that it can clearly distinguish between requests for a plain socket vs STARTTLS vs SSL. I was going to just keep the old flags and always force STARTTLS, but the tests require support for "plain", so rather than adding another flag I just passed along our existing ".crypto" flag. Seems clearer to me. - in "starttls" mode, simplesmtp will always try to establish a STARTTLS connection whether the server says it supports it or not. If the server fails to establish the connection, it fails out. - Fixed the glitches in GELAM as requested, and added tests for test_imap_prober and test_smtp_prober. - Added a gaia unit test for port selection change logic. The tests pass for me (with the checked-out simplesmtp) and I verified that I can still log in using a STARTTLS account. Let me know if I missed anything else. Thanks!
Attachment #799659 - Flags: review- → review?(bugmail)
Attachment #799628 - Flags: review- → review?(bugmail)
Attachment #799627 - Flags: review- → review?(bugmail)
Oh, and one more thing: Gaia was leaking the global 'hookupInputAreaResetButtons' from input_areas.js; I see that there's a bug about widening its scope to all apps, but since it's only used in email for now I made it a proper module so that mocha doesn't complain about leaking globals.
Comment on attachment 799627 [details] SimpleSMTP pull request There was still the potential for startTLS bypass if the server claimed to not support EHLO. I tacked on a simple fix for this based on your existing "hey, I want startTLS!" logic. r=asuth conditional on that, which I'm landing for time reasons. I definitely like the cleanup of just passing the crypto in!
Attachment #799627 - Flags: review?(bugmail) → review+
Comment on attachment 799628 [details] gelam-pull-request.html Thanks for the extra unit test! I added another variant on it for the HELO bypass case noted in the previous review comment. I also did the console.log hookup so we can see the SMTP traffic in the pretty logs. I've landed that as an extra commit and pulling in the simplesmtp changes.
Attachment #799628 - Flags: review?(bugmail) → review+
Attachment #799659 - Flags: review?(bugmail) → review+
triage, please leo+ this bug as i suspect it blocks bug 914520, which was reported by partner. ni? asuth to confirm.
blocking-b2g: - → leo?
Flags: needinfo?(bugmail)
tested and landed on v1-train. b2g18 fix just landed on bug 784816 so the new stuff won't work until tomorrow's builds https://github.com/mozilla-b2g/gaia/pull/12229 https://github.com/mozilla-b2g/gaia/commit/8b20ff956f8e366a17a7ef5f64935a6a6a8cb0c0
Flags: needinfo?(bugmail)
Keywords: late-l10n
triage: leo+, raised as critical during work week
blocking-b2g: leo? → leo+
Attached file log for adding t-online.de account (deleted) —
I/Gecko ( 675): TCPSocket: Host info: securesmtp.t-online.de:587 I/Gecko ( 675): I/Gecko ( 675): TCPSocket: SSL: false I/Gecko ( 675): I/Gecko ( 143): TCPSocket: content process: false I/Gecko ( 143): I/Gecko ( 143): TCPSocket: window init: undefined I/Gecko ( 143): TCPSocket: startup called I/Gecko ( 143): I/Gecko ( 143): TCPSocket: Host info: securesmtp.t-online.de:587 I/Gecko ( 143): I/Gecko ( 143): TCPSocket: SSL: false I/Gecko ( 143): I/Gecko ( 675): WWAR: PROBE:SMTP sad. error: | server-problem | | I/Gecko ( 675): WWAR: PROBE:IMAP sad. Error | BAD | Error while executing request: Invalid login | Invalid login
Only "inbox" folder. I/Gecko ( 1139): WERR: Error: This server doesn't support the command FolderSync I/Gecko ( 1139): WLOG: runOp(check: {"type":"syncFolderList","longtermId":"1/0","lifecycle":"do","localStatus":"done","serverStatus":"checking","tryCount":1,"humanOp":"syncFolderList"}) I/Gecko ( 1139): WLOG: runOp(check: {"type":"syncFolderList","longtermId":"1/0","lifecycle":"do","localStatus":"done","serverStatus":"checking","tryCount":6,"humanOp":"syncFolderList"})
Attached image auto setup t-online.de account.png (deleted) —
Attached image munual setup t-online.de account.png (deleted) —
v1.1.0hd: 8b20ff956f8e366a17a7ef5f64935a6a6a8cb0c0 v1.1.0hd: 4231e3f6d72badd3c3aa3394f37e0fa4cb5ce002
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: