583908 - Enable TLS false start in Mozilla.

Status: RESOLVED FIXED

(Core :: Security: PSM, defect, P2)

Description

[Wan-Teh Chang]

Attachment: Proposed patch

The attached patch enables TLS false start in Mozilla.

It adds a new preference, security.ssl.enable_false_start,
with a default value of true.

To review this patch, look at this MXR query for a similar
preference security.enable_tls_session_tickets:
http://mxr.mozilla.org/mozilla-central/search?string=security.enable_tls_session_tickets

This patch similar changes for security.ssl.enable_false_start.

Note: there are servers that are incompatible with TLS
false start.  So this patch has a low but nonzero
compatibility risk.  We can eliminate the risk completely
by changing the default value of security.ssl.enable_false_start
to false, but then very few users will be testing this
feature, making it harder to discover and fix such
incompatible servers.

Comment 1

[Robert Sayre]

Comment on attachment 462252 [details] [diff] [review]
Proposed patch

this doesn't need SR. Looks good as long as kaie is ok with it.

Comment 2

[Wan-Teh Chang]

Comment on attachment 462252 [details] [diff] [review]
Proposed patch

I pushed the patch to mozilla-central in changeset 5a649ed4ab65:
http://hg.mozilla.org/mozilla-central/rev/5a649ed4ab65

Comment 3

[Wan-Teh Chang]

Attachment: Change the default to false

TLS false start broke a Mozilla mochitest named test_bug329869.html.
So I have to change the default to false (disabled) until I find
out what's wrong.

Comment 4

[Wan-Teh Chang]

The test failed with these messages:

314 ERROR TEST-UNEXPECTED-FAIL | /tests/security/ssl/mixedcontent/test_bug329869.html | FAILURE: , expected secure got broken
315 ERROR TEST-UNEXPECTED-FAIL | /tests/security/ssl/mixedcontent/test_bug329869.html | FAILURE: for 'secure' expected flags [0,0,0],

Comment 5

[Wan-Teh Chang]

Comment on attachment 462332 [details] [diff] [review]
Change the default to false

This patch was pushed to mozilla-central in changeset b28c5262fd9e:
http://hg.mozilla.org/mozilla-central/rev/b28c5262fd9e

Comment 6

[Wan-Teh Chang]

I verified both inside a debugger and with the NSS ssltap tool
that Firefox is sending application data early when TLS false
start is enabled.

Here is a sample ssltap trace:

<-- [
[server_hello and certificate handshake messages omitted for brevity]
(1063 bytes of 4)
SSLRecord { [Tue Aug  3 12:57:23 2010]
   0: 16 03 01 00  04                                     | .....
   type    = 22 (handshake)
   version = { 3,1 }
   length  = 4 (0x4)
   handshake {
   0: 0e 00 00 00                                         | ....
      type = 14 (server_hello_done)
      length = 0 (0x000000)
   }
}
]
--> [
(314 bytes of 262, with 47 left over)
SSLRecord { [Tue Aug  3 12:57:23 2010]
   0: 16 03 01 01  06                                     | .....
   type    = 22 (handshake)
   version = { 3,1 }
   length  = 262 (0x106)
   handshake {
   0: 10 00 01 02                                         | ....
      type = 16 (client_key_exchange)
      length = 258 (0x000102)
         ClientKeyExchange {
            message = {...}
         }
   }
}
(314 bytes of 1, with 41 left over)
SSLRecord { [Tue Aug  3 12:57:23 2010]
   0: 14 03 01 00  01                                     | .....
   type    = 20 (change_cipher_spec)
   version = { 3,1 }
   length  = 1 (0x1)
   0: 01                                                  | .
}
(314 bytes of 36)
SSLRecord { [Tue Aug  3 12:57:23 2010]
   0: 16 03 01 00  24                                     | ....$
   type    = 22 (handshake)
   version = { 3,1 }
   length  = 36 (0x24)
            < encrypted >
}
]
--> [
(378 bytes of 373)
SSLRecord { [Tue Aug  3 12:57:23 2010]
   0: 17 03 01 01  75                                     | ....u
   type    = 23 (application_data)
   version = { 3,1 }
   length  = 373 (0x175)
            < encrypted >
}
]
<-- [
(47 bytes of 1, with 41 left over)
SSLRecord { [Tue Aug  3 12:57:23 2010]
   0: 14 03 01 00  01                                     | .....
   type    = 20 (change_cipher_spec)
   version = { 3,1 }
   length  = 1 (0x1)
   0: 01                                                  | .
}
(47 bytes of 36)
SSLRecord { [Tue Aug  3 12:57:23 2010]
   0: 16 03 01 00  24                                     | ....$
   type    = 22 (handshake)
   version = { 3,1 }
   length  = 36 (0x24)
            < encrypted >
}
]
<-- [
(503 bytes of 498)
SSLRecord { [Tue Aug  3 12:57:23 2010]
   0: 17 03 01 01  f2                                     | .....
   type    = 23 (application_data)
   version = { 3,1 }
   length  = 498 (0x1f2)
            < encrypted >
}
]

Comment 7

[Wan-Teh Chang]

Attachment: Change the default back to true (rollup patch)

This patch changes the default of security.ssl.enable_false_start
back to true (TLS false start enabled).  The patch includes the
necessary NSS changes, reviewed in bug 525092 comment 28.

The benefit of this patch is to reduce the connection setup
latency due to SSL full handshakes, and to test this SSL
protocol change as early as possible in the Firefox 4.0 beta
cycle.

See the Note at the end of comment 0 for the compatibility
risk of this patch.  The regression risk of this patch is
now lower than before after fixing the bug uncovered by
the test_bug329869.html mochitest failure.

Comment 8

[Robert Sayre]

http://hg.mozilla.org/mozilla-central/rev/79aa28daf1f4

Comment 9

[Kai Engert (:KaiE:)]

Comment on attachment 462252 [details] [diff] [review]
Proposed patch

r=kaie

Comment 10

[Kai Engert (:KaiE:)]

In bug 585061 comment 13 Wan-Teh said he wants to address that bug in this bug, so I'm reopening this one.

Comment 11

[Wan-Teh Chang]

Attachment: Test the SSL_ENABLE_FALSE_START macro

I leave the security.ssl.enable_false_start preference
alone.  It is harmless to have a hidden preference that
does nothing.

Comment 12

[Wan-Teh Chang]

Attachment: Test the SSL_ENABLE_FALSE_START macro (correct patch)

I attached the wrong patch.  Sorry about the confusion.

Comment 13

[Kai Engert (:KaiE:)]

Comment on attachment 463653 [details] [diff] [review]
Test the SSL_ENABLE_FALSE_START macro (correct patch)

r=kaie

Comment 14

[Kai Engert (:KaiE:)]

the latest patch fixes the compilation time problem.

but what happens if:
- mozilla is compiled against 3.12.8
- user has multiple copies of NSS installed on the system
- for some reason at runtime an older version of NSS gets chosen?

Comment 15

[Wan-Teh Chang]

Comment on attachment 463653 [details] [diff] [review]
Test the SSL_ENABLE_FALSE_START macro (correct patch)

Requesting approval2.0.

The benefit of the patch is to allow building
mozilla-central against a released version of
NSS.  (TLS false start is only in NSS 3.12.8
Beta.)

The patch has no compatibility or regression
risk.

Comment 16

[Wan-Teh Chang]

(In reply to comment #14)
The SSL_OptionSetDefault calls will do nothing and return
SECFailure.  Since the code ignores the return value of
SSL_OptionSetDefault, it does no harm.

Comment 17

[Kai Engert (:KaiE:)]

(In reply to comment #16)
> it does no harm.

good, thanks for verifying

Comment 18

[Wan-Teh Chang]

Comment on attachment 463653 [details] [diff] [review]
Test the SSL_ENABLE_FALSE_START macro (correct patch)

http://hg.mozilla.org/mozilla-central/rev/1907ad5a3f9a