Last Comment Bug 762301 - Don't retry after TLS-intolerance if TLS is the only enabled protocol
: Don't retry after TLS-intolerance if TLS is the only enabled protocol
Product: Core
Classification: Components
Component: Security: PSM (show other bugs)
: Trunk
: All All
: -- normal (vote)
: mozilla16
Assigned To: Kai Engert (:kaie) (on vacation)
: David Keeler [:keeler] (use needinfo?)
Depends on:
  Show dependency treegraph
Reported: 2012-06-06 16:23 PDT by Kai Engert (:kaie) (on vacation)
Modified: 2012-06-13 13:29 PDT (History)
4 users (show)
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---

Patch v1 for ESR10 branch (1.45 KB, patch)
2012-06-06 16:33 PDT, Kai Engert (:kaie) (on vacation)
no flags Details | Diff | Splinter Review
Patch v1 (mozilla-central) (1.44 KB, patch)
2012-06-06 16:54 PDT, Kai Engert (:kaie) (on vacation)
honzab.moz: review+
Details | Diff | Splinter Review

Description Kai Engert (:kaie) (on vacation) 2012-06-06 16:23:45 PDT
If a user disables SSL 3 (only TLS enabled),
and the user connects to a site that supports SSL 3, only,
as of today, the following will happen:
- NSS reports error code SSL_ERROR_NO_CYPHER_OVERLAP
- PSM maps this into category "TLS intolerance, can retry"
  (see function: isTLSIntoleranceError)
- we will retry
- after a couple of retries, Necko gives up and reports "connection reset"
  (which isn't a helpful message)

This bug suggests that a more helpful error is shown to the user.

Actually, I believe our code is already intended to handle this situation,
but contains a bug.

In PSM's "checkHandshake", we call "rememberPossibleTLSProblemSite", 
and use it's return value as boolean flag "wantRetry".

As of today, the function returns
  "true if TLS was enabled"

I think the function should return true, only if any older protocol is enabled,
and it therefore makes sense to retry.

Actually, the function already has such a smartness - only if an older protocol is enabled will it actually add the problematic site to an internal list.

See also
Comment 1 Kai Engert (:kaie) (on vacation) 2012-06-06 16:33:19 PDT
Created attachment 630765 [details] [diff] [review]
Patch v1 for ESR10 branch
Comment 2 Kai Engert (:kaie) (on vacation) 2012-06-06 16:54:59 PDT
Created attachment 630774 [details] [diff] [review]
Patch v1 (mozilla-central)
Comment 3 Honza Bambas (:mayhemer) 2012-06-12 06:18:43 PDT
Comment on attachment 630774 [details] [diff] [review]
Patch v1 (mozilla-central)

Review of attachment 630774 [details] [diff] [review]:

Comment 4 Kai Engert (:kaie) (on vacation) 2012-06-13 04:40:08 PDT
Comment on attachment 630774 [details] [diff] [review]
Patch v1 (mozilla-central)
Comment 5 Matt Brubeck (:mbrubeck) 2012-06-13 13:29:32 PDT

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