Last Comment Bug 480514 - Implement TLS 1.2 (RFC 5246)
: Implement TLS 1.2 (RFC 5246)
Status: RESOLVED FIXED
Read comment 32 before posting any ne...
:
Product: NSS
Classification: Components
Component: Libraries (show other bugs)
: unspecified
: All All
: P1 enhancement with 245 votes (vote)
: 3.15.1
Assigned To: Adam Langley
:
:
Mentors:
http://www.ietf.org/rfc/rfc5246.txt
Depends on: RFC4346 577019 692686
Blocks: NSA-Suite-B-TLS 422232 861266 942152
  Show dependency treegraph
 
Reported: 2009-02-27 02:18 PST by Jean-Yves Perrier [:teoli]
Modified: 2015-01-09 15:56 PST (History)
138 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
TLS 1.2 PRF implementation patch for freebl (4.26 KB, patch)
2010-07-05 10:56 PDT, Kuat Eshengazin
no flags Details | Diff | Splinter Review
TLS12_PRF_blapitest_v1.patch (17.09 KB, patch)
2010-07-07 03:06 PDT, Kuat Eshengazin
no flags Details | Diff | Splinter Review
TLS12_PRF_v2.patch (4.37 KB, patch)
2010-07-10 09:56 PDT, Kuat Eshengazin
no flags Details | Diff | Splinter Review
TLS_1_2_PRF_freebl_v3.patch (4.27 KB, patch)
2010-08-04 04:27 PDT, Kuat Eshengazin
wtc: review+
Details | Diff | Splinter Review
TLS_1_2_PRF_blapitest_v2.patch (17.00 KB, patch)
2010-08-04 04:29 PDT, Kuat Eshengazin
no flags Details | Diff | Splinter Review
TLS_P_hash_freebl_v4.patch (4.19 KB, patch)
2010-08-04 09:18 PDT, Kuat Eshengazin
rrelyea: review+
Details | Diff | Splinter Review
TLS_P_hash_blapitest_v1.patch (17.02 KB, patch)
2010-08-04 09:20 PDT, Kuat Eshengazin
no flags Details | Diff | Splinter Review
work-in-progress-aug-09.patch (72.66 KB, patch)
2010-08-09 10:46 PDT, Kuat Eshengazin
no flags Details | Diff | Splinter Review
Fix comment nit about P_hash (1.35 KB, patch)
2010-08-10 20:20 PDT, Wan-Teh Chang
no flags Details | Diff | Splinter Review
(incomplete, sketch) patch (55.51 KB, patch)
2013-01-30 14:15 PST, Adam Langley
no flags Details | Diff | Splinter Review
TLS 1.2 support (134.27 KB, patch)
2013-02-13 09:52 PST, Adam Langley
ryan.sleevi: feedback-
Details | Diff | Splinter Review
Inter-diff of the changes due to #66 (25.99 KB, patch)
2013-02-15 10:57 PST, Adam Langley
no flags Details | Diff | Splinter Review
TLS 1.2 support (133.60 KB, patch)
2013-02-15 10:57 PST, Adam Langley
no flags Details | Diff | Splinter Review
[CVS patch] TLS 1.2 support, by Adam Langley (89.07 KB, patch)
2013-03-01 13:03 PST, Wan-Teh Chang
no flags Details | Diff | Splinter Review
TLS 1.2 support, passed all NSS tests, by Adam Langley (89.10 KB, patch)
2013-03-01 15:13 PST, Wan-Teh Chang
no flags Details | Diff | Splinter Review
TLS 1.2 support, v4, by Adam Langley (87.20 KB, patch)
2013-03-01 18:27 PST, Wan-Teh Chang
no flags Details | Diff | Splinter Review
Patch to add tests for renegotitation (9.37 KB, patch)
2013-03-04 13:52 PST, Adam Langley
agl: review? (wtc)
Details | Diff | Splinter Review
Add NSS vendor-defined PKCS #11 mechanisms for TLS 1.2, by Adam Langley (18.84 KB, patch)
2013-03-20 13:50 PDT, Wan-Teh Chang
wtc: checked‑in+
Details | Diff | Splinter Review
TLS 1.2 support, v5, by Adam Langley (98.54 KB, patch)
2013-05-29 17:17 PDT, Wan-Teh Chang
wtc: checked‑in+
Details | Diff | Splinter Review
Encode and decode the new supported_signature_algorithms field of the CertificateRequest message (11.33 KB, patch)
2013-05-30 16:59 PDT, Wan-Teh Chang
no flags Details | Diff | Splinter Review
Encode and decode the new supported_signature_algorithms field of the CertificateRequest message, v2 (11.33 KB, patch)
2013-05-31 15:36 PDT, Wan-Teh Chang
wtc: checked‑in+
Details | Diff | Splinter Review
Implement the new HMAC-SHA256 cipher suites (40.08 KB, patch)
2013-06-04 17:45 PDT, Wan-Teh Chang
no flags Details | Diff | Splinter Review
Implement the new HMAC-SHA256 cipher suites, v2 (40.47 KB, patch)
2013-06-05 12:44 PDT, Wan-Teh Chang
wtc: checked‑in+
Details | Diff | Splinter Review
Support the PKCS #11 bypass mode with TLS 1.2 (20.10 KB, patch)
2013-06-05 18:04 PDT, Wan-Teh Chang
no flags Details | Diff | Splinter Review
Prune the supported_signature_algorithms field of CertificateRequest to match what we can verify (7.48 KB, patch)
2013-06-07 12:09 PDT, Wan-Teh Chang
agl: review+
wtc: checked‑in+
Details | Diff | Splinter Review
Support the PKCS #11 bypass mode with TLS 1.2, v2 (32.99 KB, patch)
2013-06-14 18:27 PDT, Wan-Teh Chang
no flags Details | Diff | Splinter Review
Support the PKCS #11 bypass mode with TLS 1.2, clean up the hashing of handshake messages, v3 (41.32 KB, patch)
2013-06-15 12:47 PDT, Wan-Teh Chang
no flags Details | Diff | Splinter Review
Support the PKCS #11 bypass mode with TLS 1.2, clean up the hashing of handshake messages, v4 (41.23 KB, patch)
2013-06-17 14:15 PDT, Wan-Teh Chang
wtc: checked‑in+
Details | Diff | Splinter Review

Description Jean-Yves Perrier [:teoli] 2009-02-27 02:18:26 PST
User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.0.7) Gecko/2009021910 Firefox/3.0.7
Build Identifier: 

NSS currently support SSL 3.0 & TLS 1.0.

Even if TLS 1.1., there weren't much market demand for it as TLS 1.2 was on the work.

TLS 1.2 is out now (August 2008) and Opera is on the way to implement it. Hence it would be interesting to implement it during the next development cycle.

Reproducible: Always
Comment 1 Franklin Tse 2009-04-05 04:52:26 PDT
The Internet Explorer 8 in Windows 7 and Windows Server 2008 R2 also support TLS 1.2.
Comment 2 Jean-Marc Desperrier 2010-03-17 02:08:29 PDT
A few details about what implementing this entails :
- TLS 1.1 did not implement any actual new functionnality, only enhancement
- But as TLS 1.2 builds on TLS 1.1, TLS 1.1 MUST be implemented before implementing TLS 1.2
- List of change needed for TLS 1.1 :
    - implicit Initialization Vector (IV) is replaced with an explicit IV
    - padding errors must rise a bad_record_mac alert, not decryption_failed
    - session should be resumable after a premature close
- Now to the change in TLS 1.2 :
    - when TLS 1.2 is negociated, the MD5/SHA-1 PRF is replaced with a suite specified hash function. The hash used is SHA-256 for all TLS 1.2 suite, but in the future suite using another algorithm can be defined.
    - the digitally-signed element include a field defining the hash alg it uses. Will be SHA-256 in TLS 1.2
    - Verify_data length is no longer fixed length
    - This allows TLS 1.2 to define several new, SHA-256 based, cipher suites.
    - better check of EncryptedPreMasterSecret version numbers, as well as of some other requirements
    - sending an alert is now a MUST in many case
    - clients MUST send an empty certificate list if a cert client is requested and none is available
    - some constraints on signature and hash algorithms are relaxed
    - IDEA and DES cipher suites are now deprecated so it's better be not propose them when negociating TLS 1.2

All in one, the big change in TLS 1.2 is that SHA-256 becomes possible, but there is a number of other enhancements to take into account for a complete implementation.
Comment 3 Matic 2010-04-12 03:18:41 PDT
When will this be done? It seems like a very important security feature and TLS 1.1 was released 4 years ago.
Comment 4 Kuat Eshengazin 2010-07-05 10:56:03 PDT
Created attachment 456069 [details] [diff] [review]
TLS 1.2 PRF implementation patch for freebl

TLS 1.2 PRF implementation.
Comment 5 Kuat Eshengazin 2010-07-07 03:06:09 PDT
Created attachment 456253 [details] [diff] [review]
TLS12_PRF_blapitest_v1.patch

Tests for TLS 1.2 PRF-SHA256, PRF-SHA384,PRF-SHA512 functions.
Usage examples:

To produce 100 bytes using TLS 1.2 PRF with SHA-256 invoke:
bltest -R -m prf_sha256 -k tests/prf_sha256/key0 -t
tests/prf_sha256/seed0 -h -g 100

To produce 146 bytes using TLS 1.2 PRF with SHA-384 invoke (-x means
isFIPS = false):
bltest -R -m prf_sha384 -k tests/prf_sha384/key0 -t
tests/prf_sha384/seed0 -h -g 148 -x

To produce 196 bytes using TLS 1.2 PRF with SHA-512 run (-x means
isFIPS = false):
bltest -R -m prf_sha512 -k tests/prf_sha512/key0 -t
tests/prf_sha512/seed0 -h -g 196 -x

To run self-tests invoke:
bltest -T -m prf_sha256,prf_sha384,prf_sha512

Test vectors are taken from here
http://www.ietf.org/mail-archive/web/tls/current/msg03416.html,
If anyone knows a better source please LMK.
Comment 6 Brian Smith (:briansmith, :bsmith, use NEEDINFO?) 2010-07-08 08:18:50 PDT
Kuat,

I am working on the softoken mechanisms for TLS 1.2 now. I filed bug 577019 to track that work. 

The TLS_1_2_PRF function isn't really needed. Instead, sftk_P_hash could just be renamed TLS_P_hash and be made public. It would be handy to have a TLS_PRF_SHA256 macro that defaults the hash algorithm parameter to SHA-256.

Are you working on any other parts of TLS 1.2 support? Are you working on support for the signature_algorithms extension? Reading & writing the new SignatureAndHash syntax?
Comment 7 Kuat Eshengazin 2010-07-08 09:58:36 PDT
(In reply to comment #6)
Yes, I do Brian. My intention is to implement TLS1.2 support fully(probably except AEAD for now). If you have already done some parts, I think it would be
logical to coordinate our efforts. Currently I've finished the proposal for PKCS #11 (https://wiki.mozilla.org/images/f/f9/PKCS11_TLS12_proposal.pdf) and nearly done with it's practical implementation.
Comment 8 Kuat Eshengazin 2010-07-10 09:56:32 PDT
Created attachment 456684 [details] [diff] [review]
TLS12_PRF_v2.patch

TLS 1.2 PRF implementation. FREEBL_VERSION increased.
Comment 9 Wan-Teh Chang 2010-08-03 07:43:56 PDT
Comment on attachment 456684 [details] [diff] [review]
TLS12_PRF_v2.patch

Nit: I also prefer the TLS_1_2_PRF name because
TLS12_PRF looks like TLS 12.

You can hardcode isFIPS to PR_TRUE for TLS 1.2
PRF.
Comment 10 Kuat Eshengazin 2010-08-04 04:27:09 PDT
Created attachment 462746 [details] [diff] [review]
TLS_1_2_PRF_freebl_v3.patch

According Wan-Teh's note TLS12_PRF renamed to TLS_1_2_PRF
Comment 11 Kuat Eshengazin 2010-08-04 04:29:21 PDT
Created attachment 462747 [details] [diff] [review]
TLS_1_2_PRF_blapitest_v2.patch
Comment 12 Wan-Teh Chang 2010-08-04 07:21:42 PDT
Comment on attachment 462746 [details] [diff] [review]
TLS_1_2_PRF_freebl_v3.patch

r=wtc.  Please make the following changes.

I'm still wondering if we should export
sftk_P_hash instead (renamed to TLS_P_hash).

In blapi.h:

>- * implement TLS Pseudo Random Function (PRF)
>+ * implement TLS and TLS 1.2 Pseudo Random Function (PRF)

This should say "TLS 1.0 and TLS 1.2".

>+extern SECStatus
>+TLS_1_2_PRF(const SECItem *secret, const char *label, SECItem *seed, 
>+            SECItem *result, HASH_HashType hash);

Pleae make "HASH_HashType hash" the first parameter.

In loader.c:

Move TLS_1_2_PRF to the end of the file, matching
the order in which these functions are listed in
loader.h.  That's the convention is in this file.

In tlsprfalg.c:

>-    unsigned char outbuf[PHASH_STATE_MAX_LEN];
>+    unsigned char outbuf[PHASH_STATE_MAX_LEN]; 

Remove the space you added at the end of the line
by accident.
Comment 13 Kuat Eshengazin 2010-08-04 09:18:59 PDT
Created attachment 462792 [details] [diff] [review]
TLS_P_hash_freebl_v4.patch

sftk_P_hash renamed and exported as TLS_P_hash
Comment 14 Kuat Eshengazin 2010-08-04 09:20:22 PDT
Created attachment 462793 [details] [diff] [review]
TLS_P_hash_blapitest_v1.patch
Comment 15 Kuat Eshengazin 2010-08-09 10:46:42 PDT
Created attachment 464094 [details] [diff] [review]
work-in-progress-aug-09.patch

Work is in progress(client handshake works against gnutls-serv). Please do not review.
Comment 16 Robert Relyea 2010-08-10 10:35:57 PDT
Comment on attachment 462792 [details] [diff] [review]
TLS_P_hash_freebl_v4.patch

This addresses WTC's comments.
Comment 17 Robert Relyea 2010-08-10 15:04:15 PDT
V4 patch committed.

Checking in lib/freebl/blapi.h;
/cvsroot/mozilla/security/nss/lib/freebl/blapi.h,v  <--  blapi.h
new revision: 1.36; previous revision: 1.35
done
Checking in lib/freebl/ldvector.c;
/cvsroot/mozilla/security/nss/lib/freebl/ldvector.c,v  <--  ldvector.c
new revision: 1.24; previous revision: 1.23
done
Checking in lib/freebl/loader.c;
/cvsroot/mozilla/security/nss/lib/freebl/loader.c,v  <--  loader.c
new revision: 1.48; previous revision: 1.47
done
Checking in lib/freebl/loader.h;
/cvsroot/mozilla/security/nss/lib/freebl/loader.h,v  <--  loader.h
new revision: 1.29; previous revision: 1.28
done
Checking in lib/freebl/tlsprfalg.c;
/cvsroot/mozilla/security/nss/lib/freebl/tlsprfalg.c,v  <--  tlsprfalg.c
new revision: 1.7; previous revision: 1.6
done
Comment 18 Wan-Teh Chang 2010-08-10 20:20:40 PDT
Created attachment 464688 [details] [diff] [review]
Fix comment nit about P_hash

Kuat, I think this comment is more accurate.  What do you think?
Comment 19 Kuat Eshengazin 2010-08-12 00:18:36 PDT
(In reply to comment #18)
> Created attachment 464688 [details] [diff] [review]
> Fix comment nit about P_hash
> 
> Kuat, I think this comment is more accurate.  What do you think?
r=eskuat
Wan-Teh, I'm ok with this.
Comment 20 Wan-Teh Chang 2010-08-12 18:02:27 PDT
Comment on attachment 464688 [details] [diff] [review]
Fix comment nit about P_hash

Thank you.  Checked in on the NSS trunk (NSS 3.13).

Checking in blapi.h;
/cvsroot/mozilla/security/nss/lib/freebl/blapi.h,v  <--  blapi.h
new revision: 1.37; previous revision: 1.36
done
Comment 21 Kuat Eshengazin 2010-08-13 10:59:27 PDT
Comment on attachment 464094 [details] [diff] [review]
work-in-progress-aug-09.patch

This is quick & dirty prototype.
Bob Relyea asked to make up a list of things that are "done" and needs to be done.

What is "done" (in prototype):
1. TLS 1.2 PRF (committed)
2. PKCS #11 proposal doc (https://bugzilla.mozilla.org/attachment.cgi?id=457908)
3. Key derivation functions: PKCS11 and bypassPKCS11 mode
    a. Derive MS from PMS
    b. Derive all session keys
    c. Finished message computation
4. Digitally signed element in Certificate verify on client side 
5. SignatureAndHashAlgorithm field on client side only


What NEEDS to be done to complete the work
0. Signature algorithms extension
1. Digitally signed element
    a. in Certificate verify on server side
    b. Server Key exchange
2. Varying length verify_data - for standard TLS 1.2 
    it's not strictly necessary, since default is still 12
3. SignatureAndHashAlgorithm field handling
4. Advertising HMAC-SHA256 ciphersuites
5. TLS 1.2 should not advertise DES CiphersSuites
6. AEAD
Comment 22 Brian Smith (:briansmith, :bsmith, use NEEDINFO?) 2010-08-13 11:49:04 PDT
For #4, IMO you shouldn't advertise HMAC-SHA2+ cipher suites without having the AES_GCM (AEAD) cipher suites also available. Otherwise, TLS 1.2 will probably cause performance problems. Also, some important AES-256 cipher suites require use the P_SHA384 PRF.

For #5, export cipher suites and some other algorithms are disallowed. I don't remember if this restriction started in TLS 1.1 or TLS 1.2.

The trickiest part of the SignatureAndHashAlgorithm handling is client certificates. In particular, the server may require that the client sign the handshake messages with a hash that is different than the PRF hash (if the PRF is even based on a hash). That means that both sides either have to cache all the handshake messages up to CertificateVerify or maintain hashes for every possible signature algorithm that the server may request (for a client: at least SHA-1, SHA-256, and SHA-384; for a server: at least SHA-1 and the PRF hash function).

The syntax of digital signatures changed in TLS 1.2 to include the hash algorithm identifier. AFAICT, this means that different PKCS#11 mechanisms need to be used for TLS 1.2. Thus, there may be interoperability issues with PKCS#11 implementations that do not support those mechanisms, but do support the mechanism needed for earlier TLS versions. And, some needed mechanisms haven't been defined yet. For example, there is CKM_ECDSA_SHA1 but no CKM_ECDSA_SHA256/384/512; there is CKM_DSA_SHA1 but no CKM_DSA_SHA256/384/512. See bug 583664. I'm not quite sure what the workaround is for that yet.

It is probably important to implement extended DSA (bug 475578) at the same time as TLS 1.2 as well.
Comment 23 Andy Bentley 2010-08-21 17:22:24 PDT
Will there be a simple/easy checkbox on the FF Config of TLS 1.2 to enable be FIPS 140 compliance ?
Comment 24 Brian Smith (:briansmith, :bsmith, use NEEDINFO?) 2010-08-28 13:31:22 PDT
It looks like right now TLS_P_hash is going to be exported in the 3.012 release. If 3.012 ships before the rest of TLS 1.2 is ready, then let's not export TLS_P_hash in that version. The signature of TLS_P_hash may still change, especially if NSS will support RFC 5705. We may also interested in optimizing it with the goal of giving the TLS 1.2 handshake approximately the same speed as the TLS 1.0/1.1 handshake, and that may also involve changes to the signature.

Andy, I imagine that TLS 1.2 options will look very similar to the TLS 1.0 options. This bug is about lower-level stuff than that. Even after NSS implements TLS 1.2, there will be a significant amount of work to do in the layers above NSS in Firefox and that will all be handled in separate bugs.
Comment 26 theosophe74 2011-09-20 04:55:47 PDT
In a matter of days, the ability to decrypt TLS 1.0 traffic will be publicly demonstrated using JavaScript code that can get the job done in around 10 minutes.  TLS 1.2 is not subject to this exploit.  IE 8 / 9 on Windows 7 supports TLS 1.2.  Opera 10 supports TLS 1.2.  Is it the recommendation of Mozilla, then, in the interest of security, to use an alternate browser like the above until the newest gaping TLS security hole can be addressed?  Of course, the server with which you're communicating needs to support TLS 1.2 as well.  And Chrome is equally vulnerable as it also uses NSS.
Comment 27 stodgycat 2011-09-20 06:43:57 PDT
Once the TLS 1.0 decryption expolit is demonstrated the severity of this bug should be increased from "enhancement" to "normal" or even "major".  The confidentiality of https sessions is a major function of the browser.
Comment 28 dziastinux 2011-09-21 00:10:47 PDT
"The vulnerability resides in versions 1.0 and earlier of TLS, or transport layer security, the successor to the secure sockets layer technology that serves as the internet's foundation of trust."
Secure connection can be decrypted in 10 min. Is it enough to make this bug "major"??

http://www.theregister.co.uk/2011/09/19/beast_exploits_paypal_ssl/
Comment 29 Leen Besselink 2011-09-21 03:54:26 PDT
If I understand the issue correctly, I would suggest use seperate SSL/TLS connection for each browser tab.

That way you prevent someone from inserting a known plaintext into an existing SSL/TLS connection.

You should do the use seperate SSL/TLS connections between pages changes in the same tab if the the previous mainpage was not HTTPS and the same hostname.
Comment 30 Leen Besselink 2011-09-21 04:16:14 PDT
Man, I most be sleepy. :-)

The short term solution I suggested above was not to re-use SSL/TLS connections, that prevent the use of a choosen plaintext by a man-in-the-middle-attacker.

So:
- each browser tab should not be using the same TLS/SSL connection
- when a user navigates from one mainpage to the next mainpage and the previous mainpage is not HTTPS and the same hostname a new TLS/SSL connection should be used
Comment 31 Matthew Elvey 2011-09-21 10:40:12 PDT
<Is it enough to make this bug "major"??>  I think clearly, yes, so I'm changing it.  (Though the bug is probably already on at least most of the relevant folks' radars.)

This bug depends on enhancement Bug 577019 which hasn't been touched for a year.

Here's the equivalent Chromium bug report, FYI: https://code.google.com/p/chromium/issues/detail?id=90392 (And it refers back to this bug and NSS.)

(Agh!  Mid-air collision detected.)
Comment 32 Brian Smith (:briansmith, :bsmith, use NEEDINFO?) 2011-09-21 14:02:04 PDT
TLS 1.1 is enough to prevent the problems that people are referring to above. TLS 1.2 is farther off. TLS 1.1 support is bug 565047.
Comment 33 Thomas Reardon 2011-09-22 11:30:39 PDT
TLS 1.2 needs to be implemented in Mozilla ASAP
Comment 34 jay garcia 2011-09-23 05:58:36 PDT
Important article from the ISC:

http://www.dshield.org/diary.html?storyid=11629
Comment 35 Peter Stendahl-Juvonen 2011-09-23 13:06:29 PDT
(In reply to Thomas Reardon from comment #33)
> TLS 1.2 needs to be implemented in Mozilla ASAP

Ditto
Comment 36 Nelson Bolyard (seldom reads bugmail) 2011-09-23 13:28:47 PDT
Read comment 32 before posting any new comment.

Bugzilla bugs are not a discussion forum.  This is NOT the place for everyone 
to pile on with "I think this is important, too" comments.  The place for those
comments is the mozilla.dev.tech.crypto newsgroup.
Comment 37 Rebecca Menessec 2012-04-29 13:04:47 PDT
Is there a separate bug tracking when TLS 1.1 + 1.2 will ship in Firefox / Thunderbird / other? If I understand correctly, this is a libnss tracking bug only.

Thanks!
Comment 38 Joshua Bowman 2012-04-29 19:20:32 PDT
Rebecca, TLS 1.1 (bug #565047) has already landed in the Moz trunk, being tracked as bug #738458. Since TLS 1.2 is indefinitely on hold, being much less backward compatible currently, there's no point in creating a separate bug to track its landing.
Comment 39 Rebecca Menessec 2012-04-29 21:46:15 PDT
(In reply to Foxyshadis from comment #38)
> Rebecca, TLS 1.1 (bug #565047) has already landed in the Moz trunk, being
> tracked as bug #738458. Since TLS 1.2 is indefinitely on hold, being much
> less backward compatible currently, there's no point in creating a separate
> bug to track its landing.

Many of the last comments on #738458 are very recent, but I see a mention of 14. I'm presently using 14.0a2 (Aurora). Should I see a prefs.js / about:config option, or something in Options -> Advanced -> Encryption? Or did it land for Fox 15?

Thanks for the fast response!
Comment 40 Kurt Roeckx 2012-04-30 03:17:32 PDT
(In reply to Foxyshadis from comment #38)
> Since TLS 1.2 is [...] being much less backward compatible currently

Do you have some more information about this?  Since openssl added support for TLS 1.1 and 1.2 we ran into various sites with problems, but I see no indication that TLS 1.2 is having more problems than 1.1.
Comment 41 Joshua Bowman 2012-04-30 09:32:54 PDT
Whoops, that was based on a misreading of the comments, Kurt, sorry.

Since Aurora 14 already branched, just days before the commit, Aurora 15 should get it unless there's a big push to get it integrated early. I don't know how one would go about that though.
Comment 42 Rebecca Menessec 2012-04-30 11:51:41 PDT
One could point out that it would make Firefox 14 the only major browser with full TLS support. Major new security features are usually a good talking point. And intrinsically useful, of course.

(Yes, technically, you can get it in a couple versions of IE. ...if you're willing to go mess with Group Policy or Registry edits to switch on the SChannel bits... assuming you're running NT 6... ..._and_ if you turn it on in IE. Most bizarre.)
Comment 43 Kurt Roeckx 2012-04-30 12:04:27 PDT
IE 8 and Opera 10 have support for TLS 1.1 and 1.2, but at least IE 9 still has them disabled by default.
Comment 44 Rebecca Menessec 2012-04-30 13:09:11 PDT
Right, there's no way to get TLS > 1.0 in IE without a massive hassle. I did go through the hassle, and found a useful testing site in the process: https://www.mikestoolbox.net/ and https://www.mikestoolbox.org/

I'm actually using Opera 12 amd64 on Windows, but:

* TLS 1.1 and 1.2 are also disabled by default in Opera, at least as of v12. Not helpful.
* Opera is really no longer a major browser by any measure of "market share" / usage statistics...
Comment 45 Florian Bender 2012-05-04 01:55:03 PDT
iOS v5 implements TLS v1.2. There is a document describing technical implications by Apple: http://developer.apple.com/library/ios/#technotes/tn2287/_index.html
Comment 46 Annubis 2012-09-09 08:18:30 PDT
The Researchers who brought us BEAST have been working to make their attack more broad based and have created a MiM session cookie stealing and decrypting javascript that breaks all versions of TLS (yes even 1.2).  http://www.ekoparty.org/2012/juliano-rizzo.php
Comment 47 Brian Smith (:briansmith, :bsmith, use NEEDINFO?) 2012-09-09 14:41:51 PDT
(In reply to Annubis from comment #46)
> The Researchers who brought us BEAST have been working to make their attack
> more broad based and have created a MiM session cookie stealing and
> decrypting javascript that breaks all versions of TLS (yes even 1.2). 
> http://www.ekoparty.org/2012/juliano-rizzo.php

Thanks for pointing that out. We are aware of their research. This bug is not the best place to discuss it, and we can't really discuss it beyond saying that, yet. We need to wait until after the researchers have given their presentation at Ekoparty and then we'll be happy to discuss it.
Comment 48 davids 2012-11-29 03:18:38 PST
Any chance, some developer might give us firefox fans the overview of current delta between tls1.2 in firefox and present state in firefox?
To give user story, it is causing everyone a major pain when implementing tls 1.2 only (to save precious code size) web servers in embedded devices.

I know moving from 1.0 to 1.2 is roughly twice the Manhattan-project endeavor with all the different kdf, IVs and all, but it is already three years old request and it's not like people don't care or ask for it.
Comment 49 Andy 2012-12-12 08:17:18 PST
+1 to all. The lack of 1.1 and 1.2 support forces my entire team to switch over to Internet Exploder for all Adobe Connect sessions -sometimes requiring a switch of machines. bleh.
Comment 50 Ramón García 2013-01-04 03:25:04 PST
Hello,

To illustrate the serious need for TLS 1.2 here is a lecture in Coursera which explains quite well the attack https://class.coursera.org/crypto-004/lecture/index (a login is needed, but you can register a free account).

The problem with TLS less that 1.2 is that it breaks user expectation of a secure channel. People act based on expectations. If one sees a HTTPS sign, one expects that nobody in the middle can eavesdrop a connection, and then operates with bank accounts, buy books and so on.

With TLS lower than 1.2, it is posible by an active attacker (that modifies the content, not one that just listens) to break the encryption and make fake transactions.

Best regards.
Comment 51 Ramón García 2013-01-04 03:30:04 PST
Sorry, the link was not correct, this https://class.coursera.org/crypto-004/lecture/38 is the correct one.
Comment 52 Martin Schöttler 2013-01-10 08:33:28 PST
(In reply to Brian Smith (:bsmith) from comment #47)
> We need to wait until after the researchers have given
> their presentation at Ekoparty and then we'll be happy to discuss it.
The CRIME attack may be prevented at the server site by disabling the compression of transfered data. Nevertheless the support of at least TLS 1.1 at the browser site is urgently necessary to prevent the BEAST attack.

We are developing an application requiring a highly secure base. Deploying is expected in roughly four months from now. We would like to recommend our prefered browser, Firefox, but would be forced to switch to a browser supporting TLS 1.1 or higher.

Martin Schöttler, member of the board of "matique UG (haftungsbeschränkt)", Germany
Comment 53 David Carlos Manuelda 2013-01-30 13:40:07 PST
(In reply to Brian Smith (:bsmith) from comment #32)
> TLS 1.1 is enough to prevent the problems that people are referring to
> above. TLS 1.2 is farther off. TLS 1.1 support is bug 565047.

That's not true.
There have been found already collissions in SHA1 MAC...and the only way to avoid this suite is using TLS 1.2

Also, most of minor browsers already implemented that (konqueror, xombrero...), and some others major (opera).

Only Internet Explorer, Firefox, and Google Chrome seem not to react nor implement it.

So the more this is delayed to be implemented, the less servers will actually use it, and the more probably is that more attacks are designed with success.
Comment 55 Adam Langley 2013-01-30 14:15:13 PST
Created attachment 708324 [details] [diff] [review]
(incomplete, sketch) patch

This patch gets NSS talking TLS 1.2 in Chrome and as a server. The SSL tests are broken for me even without any changes to NSS so I'd need to investigate that first. Also I still need to add support for the signature_algorithms extension.

Also, PKCS#11 bypass mode is almost certainly broken by this. I'm sure whether I should also implement TLS 1.2 for that mode, or whether it's ok to disable TLS 1.2 when bypass is active.
Comment 56 Robert Relyea 2013-01-30 16:24:31 PST
> Also, PKCS#11 bypass mode is almost certainly broken by this. I'm sure whether I should also
> implement TLS 1.2 for that mode, or whether it's ok to disable TLS 1.2 when bypass is active.

Or vice versa (disable bypass if TLS 1.2 is enabled)... or ideally both. (If I explicitly turn on TLS 1.2, bypass should go off if it's on, if I explicitly enable bypass, TLS 1.2 should go of if it's on).

I think in practice only oracle servers turn on bypass, so it probably makes sense for TLS 1.2 to go off if bypass goes on. At redhat we compile bypass out.

bob
Comment 57 Rebecca Menessec 2013-01-30 23:54:37 PST
(In reply to David from comment #53)

> Only Internet Explorer, Firefox, and Google Chrome seem not to react nor
> implement it.

Internet Explorer from at least 9 (8?) supports TLS 1.1 and 1.2. Unfortunately, both IE and Opera disable TLS > 1.0 by default. Oracle Java (as of 1.6?) also implements TLS 1.1 and 1.2, and (I believe) also disables it by default.

Chrome on Windows appears to use Windows cipher APIs exclusively, so it should be able to support TLS. I have no idea whether it does.

http://peter.sh/experiments/chromium-command-line-switches/ certainly suggests that Chrom[ium] can and/or does support TLS up through 1.2. (Search for "--ssl-version-max")
Comment 58 Brian Smith (:briansmith, :bsmith, use NEEDINFO?) 2013-01-31 00:08:11 PST
(In reply to Robert Relyea from comment #56)
> > Also, PKCS#11 bypass mode is almost certainly broken by this. I'm sure whether > > I should also implement TLS 1.2 for that mode, or whether it's ok to disable 
> > TLS 1.2 when bypass is active.
> 
> Or vice versa (disable bypass if TLS 1.2 is enabled)... or ideally both. (If
> I explicitly turn on TLS 1.2, bypass should go off if it's on, if I
> explicitly enable bypass, TLS 1.2 should go of if it's on).
> 
> I think in practice only oracle servers turn on bypass, so it probably makes
> sense for TLS 1.2 to go off if bypass goes on. At redhat we compile bypass
> out.

If the application enables TLS 1.2, then TLS 1.2 should stay enabled even if it tries to enable bypass mode. It would be OK for the bypass mode to be enabled and for TLS 1.2 to be enabled at the same time. In that case, if the connection negotiates TLS 1.2+ then the non-bypass mode would be used, and if the connection negotiates TLS 1.1 or older then bypass mode would be used. Somebody that wants bypass mode for TLS 1.2 can file a separate bug for it.
Comment 59 David Carlos Manuelda 2013-01-31 02:25:35 PST
(In reply to Rebecca Menessec from comment #57)
> (In reply to David from comment #53)
> 
> > Only Internet Explorer, Firefox, and Google Chrome seem not to react nor
> > implement it.
> 
> Internet Explorer from at least 9 (8?) supports TLS 1.1 and 1.2.
> Unfortunately, both IE and Opera disable TLS > 1.0 by default. Oracle Java
> (as of 1.6?) also implements TLS 1.1 and 1.2, and (I believe) also disables
> it by default.
> 
> Chrome on Windows appears to use Windows cipher APIs exclusively, so it
> should be able to support TLS. I have no idea whether it does.
> 
> http://peter.sh/experiments/chromium-command-line-switches/ certainly
> suggests that Chrom[ium] can and/or does support TLS up through 1.2. (Search
> for "--ssl-version-max")
Oh yes, you are right on that, I did not notice IE has it already despite disabled.
Anyways, I don't really see the point in disabling it by default just because there are servers that don't implement it yet.
It is a bad idea to sacrifice security to gain some performance (so I agree with comment #50).
And also, reacting prematurelly could make firefox reposition amongst competitors and a good feel for their users.
Finally, as a little side note, this can be compared with circular dependencies when installing packages:
- servers don't implement it as many clients don't and/or have it disabled by default
AND
- clients don't implement it/enable by default, as many servers don't implement it yet
Comment 60 Christoph von Wittich 2013-01-31 02:41:26 PST
-TLS 1.2 server support is getting much better since some months as it is enabled by default in all Ubuntu 12.04 and higher and Debian Wheezy installations.
-IE supports TLS 1.2 since Version 9, but only on Windows 7 and higher

It is disabled by default in IE because it still causes trouble for some websites due to faulty ssl server implementations.

I enabled TLS 1.2 company wide on about 300 machines since some months now and since then I try to get companies with broken servers to fix their ssl config.

The problem is that it is impossible to find someone responsible for the webservers in bigger companies. 

TLS 1.2 connections to DHL.de fail: "Page can not be displayed" - but I did not find a way to contact the server administrator.
Comment 61 Zoltán Halassy 2013-01-31 02:46:17 PST
(In reply to David Carlos Manuelda from comment #59)
> circular dependencies

No. Just no. Security is like medicine. Say, everyone has 1% chance to get cancer, but there is a medicine which no one used yet, but it has 100% chance to prevent cancer. But there is also some unkown chance to cause sudden death. You don't have cancer yet. Would you take it?

Microsoft's choice to include the future but disable it by default yet is prevent broken TLS 1.2 server implementations make IE non-working, and pull the cheap "IE is **** yet again" card again agaist them (I don't like IE, but I do think IE 9 is decent).

Servers already implement TLS 1.2, they just needed time for it. Apache 2.4 and OpenSSL 1.0.1 for example, both are stable now (one can even make Apache 2.2 use TLS 1.2 with OpenSSL 1.0.1; even without support in configuration).

NSS will implement TLS 1.2 eventually, then Chrome, Firefox and Thunderbird will use it, again, eventually. They're not saying, they're not implementing it, they're want to be damn sure it is implemented _right_ .

If the problem would be so burning, I wouldn't think PayPal, Bank of America or the like would let people to connect them with TLS/1.0 or FIPS/PCI would let use it still.

But tell me if I'm wrong.
Comment 62 Christoph von Wittich 2013-01-31 02:49:12 PST
Interesting statistics about TLS 1.2 adoption:

January 2013: 11,4% of sites surveyed support the TLS 1.2 protocol

https://www.trustworthyinternet.org/ssl-pulse/
Comment 63 Andy bentley 2013-01-31 06:50:58 PST
Zoltan :  With respect, your wrong.   I have worked for one of those orgs and many other financial firms.  They have insurance in the form of an insurance policy to cover financial losses due to crime and regulatory fines when they get burned. They are made whole financially via insurance policy, they repay customers, and try to "fix-up" damaged customer credit ratings "sometimes".   Any org can do that too.   I presented some proposed security mediation fixes for a couple financial organizations. They wanted estimates of the cost of implementation.  I gave them estimates.  They asked if these fixes could "guarantee" that they would not get hacked. I said no.  They said so you want us to continue to pay 1mil/qtr for insurance and invest another 1.5 mil to upgrade these systems with no guarantees ?  I said, you ins. premiums wont rise as fast in the coming yrs.  They asked to quantify that with provable numbers, which was impossible, I could guess.  They declined to pay the $ to upgrade their systems. I hear they currently pay 4 mil/qtr for ins.   They continue to consider it a cost of doing business.  That's their culture.

Other orgs (Law enforcement, emergency responders, national security) have more serious security concerns than just money, ins. policies don't bring people back from the dead. "We" need this !
Comment 64 Adam Langley 2013-02-13 09:52:14 PST
Created attachment 713467 [details] [diff] [review]
TLS 1.2 support

This patch is a version that is not completely bogus. It might be r? if I don't think of anything wrong with it soon.

wtc's patches in 838767, 838769 and 838778 are a pre-req for this patch.
Comment 65 Wan-Teh Chang 2013-02-13 11:45:26 PST
Comment on attachment 713467 [details] [diff] [review]
TLS 1.2 support

Thanks a lot, Adam. I will take a look and play with the patch.
Comment 66 Ryan Sleevi 2013-02-13 16:08:49 PST
Comment on attachment 713467 [details] [diff] [review]
TLS 1.2 support

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

Some initial feedback from a quick scan through, looking for SSL<->softoken issues.

::: mozilla/security/nss/cmd/lib/pk11table.c
@@ +381,2 @@
>  	mkEntry(CKM_TLS_MASTER_KEY_DERIVE_DH, Mechanism),
> +	mkEntry(CKM_TLS_MASTER_KEY_DERIVE_DH_SHA256, Mechanism),

I think all of these mechanisms should be following the CKM_NSS_ naming, since they're not specified in PKCS#11 v2.30 or later.

This will hopefully be something that can be resolved quickly in the PKCS#11 TC in OASIS ( https://www.oasis-open.org/committees/tc_home.php?wg_abbrev=pkcs11 ).

See also https://bugzilla.mozilla.org/show_bug.cgi?id=577019

::: mozilla/security/nss/lib/ssl/ssl3ext.c
@@ +1879,5 @@
> +{
> +    SECStatus rv;
> +    SECItem algorithms;
> +    const unsigned char *b;
> +    size_t i;

Wouldn't a PRUint16 suffice here, given line 1893 is only reading two bytes, and algorithms.len/2 < algorithms.len < max(PRUint16)

@@ +1905,5 @@
> +	return SECSuccess;
> +    }
> +
> +    ss->ssl3.hs.clientSigAndHash = PORT_Alloc(
> +	    sizeof(SSL3SignatureAndHashAlgorithm) * algorithms.len/2);

Should there be a sanity check here for potential PORT_Alloc overflow (by setting a large .len value)

Should there also be a check that algorithms.len is valid for data->len, to make sure it's a consistent value?

@@ +1912,5 @@
> +    }
> +    ss->ssl3.hs.numClientSigAndHash = 0;
> +
> +    b = algorithms.data;
> +    for (i = 0; i < algorithms.len/2; i++) {

Should you precompute algorithms.len/2 (used here inside the for loop, but also on line 1909), and use that variable.

@@ +1950,5 @@
> +	tls_hash_sha384, tls_sig_rsa,
> +	tls_hash_sha1,   tls_sig_rsa,
> +	tls_hash_sha256, tls_sig_dsa,
> +	tls_hash_sha384, tls_sig_dsa,
> +	tls_hash_sha1,   tls_sig_dsa,

I don't think this is always true (eg: when not using softoken). Seems like it may need to inquire against the token(s) used for supported methods.

::: mozilla/security/nss/lib/ssl/sslsock.c
@@ +1859,5 @@
>      ss->vrange = *vrange;
> +    /* If we don't have a sufficiently up-to-date library then we cannot do TLS
> +     * 1.2. */
> +    if (ss->vrange.max >= SSL_LIBRARY_VERSION_TLS_1_2 &&
> +        !PK11_TokenExists(CKM_TLS_MASTER_KEY_DERIVE_DH_SHA256)) {

It's possible that a different token (eg: not softoken) supports this mechanism.

There's also the risk of collision in the vendor-ID space (if adopted).

::: mozilla/security/nss/lib/util/pkcs11n.h
@@ +212,5 @@
>  #define CKM_NETSCAPE_PBE_MD5_HMAC_KEY_GEN       0x8000000aUL
>  #define CKM_NETSCAPE_PBE_MD2_HMAC_KEY_GEN       0x8000000bUL
>  
>  #define CKM_TLS_PRF_GENERAL                     0x80000373UL
> +#define CKM_TLS_PRF_GENERAL_SHA256              0x80000374UL

see comments about vendor-space.

::: mozilla/security/nss/lib/util/pkcs11t.h
@@ +784,5 @@
>  
> +#define CKM_TLS_MASTER_KEY_DERIVE_SHA256  0x00000379
> +#define CKM_TLS_KEY_AND_MAC_DERIVE_SHA256 0x0000037a
> +#define CKM_TLS_MASTER_KEY_DERIVE_DH_SHA256 0x0000037b
> +#define CKM_TLS_PRF_SHA256                0x00000378

Same here. These aren't defined in 2.20
Comment 67 Adam Langley 2013-02-15 10:52:50 PST
(In reply to Ryan Sleevi from comment #66)
> I think all of these mechanisms should be following the CKM_NSS_ naming,
> since they're not specified in PKCS#11 v2.30 or later.

Done.

> Wouldn't a PRUint16 suffice here, given line 1893 is only reading two bytes,
> and algorithms.len/2 < algorithms.len < max(PRUint16)

I think a PRUint16 would suffice, but I don't think there's any reason to use it. Any compiler is going to use a register for this, which is as large as a size_t anyway.

> Should there be a sanity check here for potential PORT_Alloc overflow (by
> setting a large .len value)

A client could cause 64KB to be allocated. Is that unreasonable? I suspect they can already do worse by sending a large handshake message.

But I've sanity limited it to 512 algorithms now.

> Should there also be a check that algorithms.len is valid for data->len, to
> make sure it's a consistent value?

ssl3_ConsumeHandshakeVariable will catch the case where the length of the algorithms is larger than the extension itself and the test for data->len == 0 will catch the case where there's trailing garbage.

> Should you precompute algorithms.len/2 (used here inside the for loop, but
> also on line 1909), and use that variable.

Done.

> I don't think this is always true (eg: when not using softoken). Seems like
> it may need to inquire against the token(s) used for supported methods.

(See below.)

> > +        !PK11_TokenExists(CKM_TLS_MASTER_KEY_DERIVE_DH_SHA256)) {
> 
> It's possible that a different token (eg: not softoken) supports this
> mechanism.

PK11_TokenExists searches all the tokens, I think. If not, any idea what it should use? If PK11_TokenExists is ok, should it be called when advertising signature algorithm support? Since libssl hashes itself, it looks like it would only need to test for RSA, ECDSA, etc support and then for CKM_SHA256, etc support.
 
> There's also the risk of collision in the vendor-ID space (if adopted).

Have moved them all into the NSS space (CKM_NSS + x)
Comment 68 Adam Langley 2013-02-15 10:57:01 PST
Created attachment 714497 [details] [diff] [review]
Inter-diff of the changes due to #66
Comment 69 Adam Langley 2013-02-15 10:57:51 PST
Created attachment 714499 [details] [diff] [review]
TLS 1.2 support
Comment 70 David Carlos Manuelda 2013-02-16 02:44:42 PST
Just as a note:

This website is very useful to test while implementing it: https://www.mikestoolbox.org/

It will say lots of useful things about current browser TLS version, cipher suites and so on.
Comment 71 Wan-Teh Chang 2013-03-01 13:03:03 PST
Created attachment 720105 [details] [diff] [review]
[CVS patch] TLS 1.2 support, by Adam Langley

I applied Adam's "TLS 1.2 support" patch to the current CVS trunk.
I fixed only merging conflicts. It compiles, but there are test
failures (with core dumps). I will look at the test failures after
lunch.
Comment 72 Wan-Teh Chang 2013-03-01 15:10:27 PST
Comment on attachment 720105 [details] [diff] [review]
[CVS patch] TLS 1.2 support, by Adam Langley

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

::: mozilla/security/nss/lib/ssl/ssl3con.c
@@ +3688,5 @@
> +
> +    if (ss->ssl3.hs.messages.buf && !ss->opt.bypassPKCS11) {
> +	PORT_Free(ss->ssl3.hs.messages.buf);
> +	ss->ssl3.hs.messages.buf = NULL;
> +	ss->ssl3.hs.messages.len = 0;

The test failures and crashes (of tstclnt) occurred during renegotiation.
It is caused by this bug here. After freeing ss->ssl3.hs.messages.buf, we
also need to set ss->ssl3.hs.messages.space to 0. In the core dump, I see
ss->ssl3.hs.messages.buf is NULL, ss->ssl3.hs.messages.len is 0, but
ss->ssl3.hs.messages.space is 0x4800.
Comment 73 Wan-Teh Chang 2013-03-01 15:13:21 PST
Created attachment 720155 [details] [diff] [review]
TLS 1.2 support, passed all NSS tests, by Adam Langley

This is Adam's patch with the absolutely necessary changes
to fix merging conflicts and test failures/crashes.  It
passes the NSS test suite on my Linux computer.
Comment 74 Wan-Teh Chang 2013-03-01 18:17:29 PST
Comment on attachment 720155 [details] [diff] [review]
TLS 1.2 support, passed all NSS tests, by Adam Langley

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

I have reviewed all the changes outside lib/ssl and tests/ssl. They
look good. I will check them in first when the NSS tree reopens.

::: mozilla/security/nss/lib/softoken/pkcs11c.c
@@ +5579,5 @@
>       */
> +    case CKM_NSS_TLS_MASTER_KEY_DERIVE_SHA256:
> +    case CKM_NSS_TLS_MASTER_KEY_DERIVE_DH_SHA256:
> +	isSHA256 = PR_TRUE;
> +	/* fall thru */

This fall through won't work when we support using P_Hash with SHA-384.
We can deal with that when we cross that bridge.

@@ +6008,5 @@
> +					"client write key", &crsr, &keyblk,
> +					isFIPS);
> +		} else {
> +		    status = TLS_PRF(&secret, "client write key", &crsr, &keyblk,
> +				     isFIPS);

Calling TLS_PRF with "client write key", "server write key", and "IV block"
is for export ciphers, which are dropped in TLS 1.1. So we don't need these
three changes for TLS 1.2.

::: mozilla/security/nss/lib/ssl/sslimpl.h
@@ +802,4 @@
>      PRUint16              finishedBytes; /* size of single finished below */
>      union {
>  	TLSFinished       tFinished[2]; /* client, then server */
> +	unsigned char     sFinished[36][2];

This two-dimensional array seems wrong. I think it should
be declared as 
    unsigned char sFinished[2][36];

I don't know why the SSL 3.0 tests did not fail. Maybe this
means we are not testing secure renegotiation with SSL 3.0?

Why did you change the definition of sFinished?  Is
'sFinished' still used for SSL 3.0 ("s" stands for "SSL3")?
Comment 75 Wan-Teh Chang 2013-03-01 18:27:50 PST
Created attachment 720213 [details] [diff] [review]
TLS 1.2 support, v4, by Adam Langley

I attached a snapshot of the patch as a check point
at the end of this week.
Comment 76 Adam Langley 2013-03-01 19:55:25 PST
(In reply to Wan-Teh Chang from comment #74)
> This two-dimensional array seems wrong. I think it should
> be declared as 
>     unsigned char sFinished[2][36];

You're correct.
 
> I don't know why the SSL 3.0 tests did not fail. Maybe this
> means we are not testing secure renegotiation with SSL 3.0?

Indeed and this is troubling. I cannot see that there are any renegotiation tests in the test suite. I'll write a patch to add them before anything else.

> Why did you change the definition of sFinished?  Is
> 'sFinished' still used for SSL 3.0 ("s" stands for "SSL3")?

sFinished was previously a pair of SSL3Hashes and SSL3Hashes was a simple, 36 byte structure that held the MD5 and SHA1 hashes.

With this patch, SSH3Hashes has become more complex and the code in ssl3ext.c, which assumes that the saved finished values are just the finished contents, would have broken.
Comment 77 Adam Langley 2013-03-04 13:52:29 PST
Created attachment 720877 [details] [diff] [review]
Patch to add tests for renegotitation

This patch adds tests for renegotiation support. It's against 3.14.2, not on top of the TLS 1.2 patch.

Changes to tstclnt were needed because it previously called SSL_ReHandshake inside the handshake callback. This violated the locking order and hit an assert. tstclnt now performs the renegotiations outside of any callback, but this does require code to wait for a handshake to complete. If we don't wait then tstclnt sends the HTTP request and selfserv closes the connection before the renegotiations can complete.

Changes to ssl3con.c were needed to fix several memory leaks triggered by renegotiation.
Comment 78 Adam Langley 2013-03-05 11:37:44 PST
I have fixed the sFinished definition in the header. Sadly the renegotiation tests in #77 pass both before and after because both the server and client were broken in exactly the same way. I have tested the new code against OpenSSL to confirm the renegotiation functions correctly.

I have also reverted the changes around the export cipher suites.

(No new patch because I think wtc's version of the patch works better with bugzilla and also I'm working from 3.14.2 while some bits of the patch have landed in CVS already.)
Comment 79 Julien Pierre 2013-03-14 19:39:51 PDT
Re: the bypass vs TLS 1.2 discussion, it is likely that Oracle will pick up NSS 3.14.x in legacy products to get the current TLS 1.1 support, and TLS 1.2 when available. Several Oracle products enable bypass.

I understand that you don't want to write the code for PKCS#11 bypass for TLS 1.2. 

I am wondering about the behavior of a single socket that has all versions of the protocols enabled, as well as PKCS#11 bypass. In that case, is it possible that PKCS#11 bypass could continue to work for SSL 3.0 through TLS 1.1, but if TLS 1.2 is negotiated, PKCS#11 bypass would get transparently disabled locally ?
Comment 80 Adam Langley 2013-03-15 05:39:57 PDT
(In reply to Julien Pierre from comment #79)
> I am wondering about the behavior of a single socket that has all versions
> of the protocols enabled, as well as PKCS#11 bypass. In that case, is it
> possible that PKCS#11 bypass could continue to work for SSL 3.0 through TLS
> 1.1, but if TLS 1.2 is negotiated, PKCS#11 bypass would get transparently
> disabled locally ?

Currently, if you configure bypass mode on the socket and then try to configure a version range of (SSLv3,TLS1.2), you'll actually get (SSLv3,TLS1.1). If you try to configure (TLS1.2,TLS1.2), you'll get an error.

Is that acceptable? I'm not sure whether the situation that you describe is a worry ("Oh no! Bypass mode will be silently disabled!") or a wish ("It would be nice to have TLS 1.2 - we could just disable bypass mode for it.")
Comment 81 Julien Pierre 2013-03-15 17:16:20 PDT
Adam,

Thanks for explaining the behavior in the patch.

I would prefer if TLS 1.2 was never silently disabled when configuring a range that includes TLS 1.2, and also if configuring (TLS1.2, TLS1.2) never failed, regardless of the state of the bypass flag. As you know, the order that these things get set is not predictable, bypass could be set before or after the protocol range gets set.

I am fine with bypass being silently disabled for TLS 1.2. My wish would be that bypass continues to work if the range configured has been configured as(SSLv3, TLS1.2), SSL bypass is turned on, but the negotiated protocol version is somewhere in (SSLv3, TLS1.1). Does the current patch allow that ?
Comment 82 Wan-Teh Chang 2013-03-20 13:50:04 PDT
Created attachment 727350 [details] [diff] [review]
Add NSS vendor-defined PKCS #11 mechanisms for TLS 1.2, by Adam Langley

I will review and check in Adam's patch in installments.

This patch is the PKCS #11/softoken part. I moved it to bug 577019
and checked it in. See bug 577019 comment 11.
Comment 83 tu 2013-03-30 03:59:50 PDT
With RC4 being so insecure supporting TLS 1.2 is really important now.
http://blog.cryptographyengineering.com/2013/03/attack-of-week-rc4-is-kind-of-broken-in.html
Comment 84 wind 2013-04-10 04:37:10 PDT Comment hidden (off-topic)
Comment 85 Kurt Roeckx 2013-05-10 06:20:59 PDT
Can someone comment on the current status of this?
Comment 86 STUNGMEBAS 2013-05-24 21:14:19 PDT
(In reply to Kurt Roeckx from comment #85)
> Can someone comment on the current status of this?

Seems abandoned.
Sad really.
Comment 87 Kurt Roeckx 2013-05-25 09:39:55 PDT
(In reply to STUNGMEBAS from comment #86)
> Seems abandoned.
> Sad really.

I really hope it's not, since it's the only protocol that supports ciphers that aren't known to be broken.
Comment 88 Neil Turner 2013-05-25 10:25:48 PDT
TLS 1.1 will ship in Firefox 22 but it's pref'd off because there are still a few minor issues with it. Hopefully once these have been alleviated the focus will shift to TLS 1.2.
Comment 89 Wan-Teh Chang 2013-05-29 17:17:36 PDT
Created attachment 755696 [details] [diff] [review]
TLS 1.2 support, v5, by Adam Langley

https://hg.mozilla.org/projects/nss/rev/5a9fa031aca5

This patch is based heavily on Adam's patch v4 (attachment 720213 [details] [diff] [review]).
I fixed some bugs and coding style issues, and rewrote a very small
part of it. Thank you very much, Adam!

The lib/ssl part of this patch was reviewed at
https://codereview.chromium.org/14772023/

To be implemented:

1. The new supported_signature_algorithms field of the
CertificateRequest message.

This patch makes some shortcuts.

1. It assumes the PRF of all TLS 1.2 cipher suites is P_SHA256.

To remove this limitation, we will need to wait until the cipher
suite has been chosen to call ssl3_InitTLS12HandshakeHash. Right
now we call ssl3_InitTLS12HandshakeHash as soon as the protocol
version has been negotiated.

2. For TLS 1.2 client authentication, it requires the signature of
handshake_messages in the CertificateVerify message to use the same
hash function that the PRF uses. Both the client and server sides
make this assumption.

To remove this limitation, we can either buffer the handshake messages
until we have sent or handled the CertificateVerify message (or
are sure that it's not necessary), or compute the most common hashes
(such as SHA-1 and SHA-256) of the handshake messages until that point.
Comment 90 Wan-Teh Chang 2013-05-30 16:59:19 PDT
Created attachment 756263 [details] [diff] [review]
Encode and decode the new supported_signature_algorithms field of the CertificateRequest message

Ryan, Brian: please review this patch. Thanks.

1. Prefer ECDSA client certs over DSA client certs.

2. The main change: In TLS 1.2, the CertificateRequest message
changed from

  struct {
      ClientCertificateType certificate_types<1..2^8-1>;
      DistinguishedName certificate_authorities<0..2^16-1>;
  } CertificateRequest;

to

  struct {
      ClientCertificateType certificate_types<1..2^8-1>;
      SignatureAndHashAlgorithm
        supported_signature_algorithms<2..2^16-2>;
      DistinguishedName certificate_authorities<0..2^16-1>;
  } CertificateRequest;

where SignatureAndHashAlgorithm is defined as

      enum {
          none(0), md5(1), sha1(2), sha224(3), sha256(4), sha384(5),
          sha512(6), (255)
      } HashAlgorithm;

      enum { anonymous(0), rsa(1), dsa(2), ecdsa(3), (255) }
        SignatureAlgorithm;

      struct {
            HashAlgorithm hash;
            SignatureAlgorithm signature;
      } SignatureAndHashAlgorithm;

So ssl3_SendCertificateRequest and ssl3_HandleCertificateRequest need
to encode and decode the new supported_signature_algorithms field.

supported_signature_algorithms is also used in the signature_algorithms
extension, so I added helper functions ssl3_SizeOfSupportedSignatureAlgorithms
and ssl3_AppendSupportedSignatureAlgorithms to allow ssl3con.c and ssl3ext.c
to share code.

3. Disallow an empty supported_signature_algorithms value because the
minimum size is specified to be 2 bytes. Please let me know if you
think this is a bad idea.
Comment 91 Wan-Teh Chang 2013-05-30 18:40:16 PDT
For reference: Apache 2 with mod_ssl sends the following
supported_signature_algorithms list in the CertificateRequest message:

{ sha512, rsa }, { sha512, dsa }, { sha512, ecdsa },
{ sha384, rsa }, { sha384, dsa }, { sha384, ecdsa },
{ sha256, rsa }, { sha256, dsa }, { sha256, ecdsa },
{ sha224, rsa }, { sha224, dsa }, { sha224, ecdsa },
{ sha1, rsa }, { sha1, dsa }, { sha1, ecdsa },
{ md5, rsa }
Comment 92 Aaron Jones 2013-05-31 06:43:27 PDT
I do hope that the configuration option 'security.enable_md5_signatures' in Firefox will somehow relay to NSS that the RSAwithMD5 signature algorithm will be ignored. It would be very bad to continue to support this by default, given that public second-preimage attacks on X.509 certificates with MD5 signatures have already been demonstrated. Therefore I request that if a server should reply with a supported signature algorithms and that it only contains { md5, rsa } the connection should be aborted with the same error that results in the "The certificate uses an insecure signature algorithm" page, or words to that effect that I can't quite remember.
Comment 93 David Carlos Manuelda 2013-05-31 07:25:39 PDT
(In reply to Aaron Jones from comment #92)
> I do hope that the configuration option 'security.enable_md5_signatures' in
> Firefox will somehow relay to NSS that the RSAwithMD5 signature algorithm
> will be ignored. It would be very bad to continue to support this by
> default, given that public second-preimage attacks on X.509 certificates
> with MD5 signatures have already been demonstrated. Therefore I request that
> if a server should reply with a supported signature algorithms and that it
> only contains { md5, rsa } the connection should be aborted with the same
> error that results in the "The certificate uses an insecure signature
> algorithm" page, or words to that effect that I can't quite remember.

+1 for cancelling connection on insecure algorithms. I know that ff wants to be compatible, but also, when displaying a green bar or a lock, users should trust what navigator tells, so I simply prefer giving an error, or a warning rather than to connect silently and force server admins somehow to fix their security.
Comment 94 Kurt Roeckx 2013-05-31 09:30:19 PDT
(In reply to David Carlos Manuelda from comment #93)
> +1 for cancelling connection on insecure algorithms. I know that ff wants to
> be compatible, but also, when displaying a green bar or a lock, users should
> trust what navigator tells, so I simply prefer giving an error, or a warning
> rather than to connect silently and force server admins somehow to fix their
> security.

This bug is about the implementation of TLS 1.2 is NSS, and has nothing to do with what FF does with it.  See bug #861266 for the FF part, or other bugs that might be relevant.  Or create a new one if you think there isn't an existing bug for your concern.
Comment 95 Kurt Roeckx 2013-05-31 13:08:01 PDT
(In reply to Aaron Jones from comment #92)
> I do hope that the configuration option 'security.enable_md5_signatures' in
> Firefox will somehow relay to NSS that the RSAwithMD5 signature algorithm
> will be ignored. It would be very bad to continue to support this by
> default, given that public second-preimage attacks on X.509 certificates
> with MD5 signatures have already been demonstrated. Therefore I request that
> if a server should reply with a supported signature algorithms and that it
> only contains { md5, rsa } the connection should be aborted with the same
> error that results in the "The certificate uses an insecure signature
> algorithm" page, or words to that effect that I can't quite remember.

A CertificateRequest is a server asking a client a certificate, and that it lists which signatures it's able to verify in descending order of preference.

If you think think that certificates with md5 should not be send, I think firefox should just refuse to offer such certificates.
Comment 96 Eldorel 2013-05-31 13:17:15 PDT
>If you think think that certificates with md5 should not be send, I think firefox should just refuse to offer such certificates.

I disagree. Treating them in the same manner as invalid or expired certs still gives the end user the ability to override the error and continue.

Refusing to send the cert at all would break compatibility with older embedded devices that can't be updated.
Comment 97 Wan-Teh Chang 2013-05-31 15:36:53 PDT
Created attachment 756827 [details] [diff] [review]
Encode and decode the new supported_signature_algorithms field of the CertificateRequest message, v2

Adam reviewed this patch at https://codereview.chromium.org/16195008/.

Patch checked in: https://hg.mozilla.org/projects/nss/rev/13089af66082
Comment 98 Aaron Jones 2013-06-01 03:41:38 PDT
(In reply to Kurt Roeckx from comment #95)
> A CertificateRequest is a server asking a client a certificate, and that it
> lists which signatures it's able to verify in descending order of preference.

I know that. I'm saying that if a certificate is requested, and the server only supports verifying MD5 signatures, abort the connection (by default) unless NSS is explicitly instructed otherwise (by e.g. Firefox).

> If you think think that certificates with md5 should not be send, I think
> firefox should just refuse to offer such certificates.

That would mask the fact that the server is horribly insecure.
Comment 99 Wan-Teh Chang 2013-06-04 17:45:47 PDT
Created attachment 758326 [details] [diff] [review]
Implement the new HMAC-SHA256 cipher suites

You can review the nss/lib/ssl changes at
https://codereview.chromium.org/16394004/

The nss/tests/ssl changes are only available
here.

Note that the two new DHE_RSA cipher suites are only
implemented on the client side, like the rest of the
DHE cipher suites, so we can't test them in sslcov.txt.
Comment 100 Wan-Teh Chang 2013-06-05 12:44:14 PDT
Created attachment 758752 [details] [diff] [review]
Implement the new HMAC-SHA256 cipher suites, v2

Adam reviewed the nss/lib/ssl changes in this patch at
https://codereview.chromium.org/16394004/.  I adjusted the cipher suite
order and added a comment based on his review.

https://hg.mozilla.org/projects/nss/rev/67471ffe04fb
Comment 101 Wan-Teh Chang 2013-06-05 18:04:57 PDT
Created attachment 758914 [details] [diff] [review]
Support the PKCS #11 bypass mode with TLS 1.2

This patch eliminates the restriction that the PKCS #11
bypass mode and TLS 1.2 can't be enabled at the same time.
It still needs polishing, but it passes the NSS test suite.

I can simplify some logic if I remove the NSS_SURVIVE_DOUBLE_BYPASS_FAILURE
build option:
http://mxr.mozilla.org/nss/search?string=NSS_SURVIVE_DOUBLE_BYPASS_FAILURE
Comment 102 Wan-Teh Chang 2013-06-07 12:09:29 PDT
Created attachment 759878 [details] [diff] [review]
Prune the supported_signature_algorithms field of CertificateRequest to match what we can verify

Since ssl3_HandleCertificateVerify only supports TLS 1.2 CertificateVerify
messages that use the handshake hash (which is always SHA256 right now),
this patch prunes the supported_signature_algorithms field of our TLS 1.2
CertificateRequest message to reflect that.

This means the signature_algorithms extension in our TLS 1.2 ClientHello
cannot share the same supported_signature_algorithms list. So I reverted
the two helper functions I added.
Comment 103 Wan-Teh Chang 2013-06-07 12:36:20 PDT
Comment on attachment 759878 [details] [diff] [review]
Prune the supported_signature_algorithms field of CertificateRequest to match what we can verify

https://hg.mozilla.org/projects/nss/rev/f6a0a66c6037
Comment 104 Wan-Teh Chang 2013-06-14 18:27:07 PDT
Created attachment 763033 [details] [diff] [review]
Support the PKCS #11 bypass mode with TLS 1.2, v2

In this patch I also delay the creation of the handshake hash contexts
until we know which hash functions to use. I changed ssl3_NewHandshakeHashes
to not call ssl3_RestartHandshakeHashes, so that we never call
ssl3_RestartHandshakeHashes twice.

This patch still needs polishing. I attached it as a checkpoint.
Comment 105 Wan-Teh Chang 2013-06-15 12:47:01 PDT
Created attachment 763135 [details] [diff] [review]
Support the PKCS #11 bypass mode with TLS 1.2, clean up the hashing of handshake messages, v3

I polished this patch. It can be reviewed at https://codereview.chromium.org/17109007.

I decided to remove the NSS_SURVIVE_DOUBLE_BYPASS_FAILURE support so that
ss->ssl3.hs.messages is used for only one purpose. This allowed me to simplify
some logic.
Comment 106 Eldorel 2013-06-15 14:41:47 PDT
Wan-Teh,
    I just want to say thank you very much for addressing this bug. I've been tracking it for 2 years but don't have the knowledge to even begin to address it myself.

    I know that this is a pretty complicated fix, and I'm sure that most of us appreciate your work. 

Thank you.
Comment 107 Wan-Teh Chang 2013-06-17 14:15:49 PDT
Created attachment 763789 [details] [diff] [review]
Support the PKCS #11 bypass mode with TLS 1.2, clean up the hashing of handshake messages, v4

Brian: you are welcome to take a look at this patch. Adam already
reviewed it at https://codereview.chromium.org/17109007/.

Patch checked in: https://hg.mozilla.org/projects/nss/rev/c0076cf37035
Comment 108 Brian Smith (:briansmith, :bsmith, use NEEDINFO?) 2014-07-11 17:40:37 PDT
Comment on attachment 763789 [details] [diff] [review]
Support the PKCS #11 bypass mode with TLS 1.2, clean up the hashing of handshake messages, v4

I think agl's review of these was good enough.
Comment 109 Bramus 2014-10-16 01:06:10 PDT
Why is this bug marked as "RESOLVED FIXED"?

Try surfing to https://cpanel.ikdoeict.be:2087/. Even with Nightly 36.0a1 (2014-10-15), an error will show on screen: “An error occurred during a connection to cpanel.ikdoeict.be:2087. Cannot communicate securely with peer: no common encryption algorithm(s). (Error code: ssl_error_no_cypher_overlap)”

When checking that host:port using nmap, is shows that there are (some) TLSv1.2 ciphers supported: 

$ nmap --script +ssl-enum-ciphers -p2087 cpanel.ikdoeict.be

Starting Nmap 6.40 ( http://nmap.org ) at 2014-10-16 09:40 CEST
Nmap scan report for cpanel.ikdoeict.be (193.190.130.176)
Host is up (0.037s latency).
rDNS record for 193.190.130.176: www.instudy.be
PORT     STATE SERVICE
2087/tcp open  eli
| ssl-enum-ciphers: 
|   SSLv3: No supported ciphers found
|   TLSv1.0: No supported ciphers found
|   TLSv1.1: No supported ciphers found
|   TLSv1.2: 
|     ciphers: 
|       TLS_RSA_WITH_AES_128_CBC_SHA256 - strong
|       TLS_RSA_WITH_AES_128_GCM_SHA256 - strong
|       TLS_RSA_WITH_AES_256_CBC_SHA256 - strong
|       TLS_RSA_WITH_AES_256_GCM_SHA384 - strong
|     compressors: 
|       NULL
|_  least strength: strong

Nmap done: 1 IP address (1 host up) scanned in 2.33 seconds


Ergo, it does not look like "RESOLVED FIXED" to me.
Comment 110 Reed Loden [:reed] (use needinfo?) 2014-10-16 01:18:13 PDT
(In reply to Bramus from comment #109)
> |   TLSv1.2: 
> |     ciphers: 
> |       TLS_RSA_WITH_AES_128_CBC_SHA256 - strong
> |       TLS_RSA_WITH_AES_128_GCM_SHA256 - strong
> |       TLS_RSA_WITH_AES_256_CBC_SHA256 - strong
> |       TLS_RSA_WITH_AES_256_GCM_SHA384 - strong

See bug 1029179. NSS supports these ciphers (bug 880543), but Firefox does not include them in its offered cipher list, which is why the connection fails.

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