480514 - Implement TLS 1.2 (RFC 5246)

Status: RESOLVED FIXED

(NSS :: Libraries, enhancement, P1)

Description

[Jean-Yves Perrier [:teoli]]

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]

The Internet Explorer 8 in Windows 7 and Windows Server 2008 R2 also support TLS 1.2.

Comment 2

[Jean-Marc Desperrier]

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]

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]

Attachment: TLS 1.2 PRF implementation patch for freebl

TLS 1.2 PRF implementation.

Comment 5

[Kuat Eshengazin]

Attachment: 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?)]

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]

(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]

Attachment: TLS12_PRF_v2.patch

TLS 1.2 PRF implementation. FREEBL_VERSION increased.

Comment 9

[Wan-Teh Chang]

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]

Attachment: TLS_1_2_PRF_freebl_v3.patch

According Wan-Teh's note TLS12_PRF renamed to TLS_1_2_PRF

Comment 11

[Kuat Eshengazin]

Attachment: TLS_1_2_PRF_blapitest_v2.patch

Comment 12

[Wan-Teh Chang]

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]

Attachment: TLS_P_hash_freebl_v4.patch

sftk_P_hash renamed and exported as TLS_P_hash

Comment 14

[Kuat Eshengazin]

Attachment: TLS_P_hash_blapitest_v1.patch

Comment 15

[Kuat Eshengazin]

Attachment: work-in-progress-aug-09.patch

Work is in progress(client handshake works against gnutls-serv). Please do not review.

Comment 16

[Robert Relyea]

Comment on attachment 462792 [details] [diff] [review]
TLS_P_hash_freebl_v4.patch

This addresses WTC's comments.

Comment 17

[Robert Relyea]

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]

Attachment: Fix comment nit about P_hash

Kuat, I think this comment is more accurate.  What do you think?

Comment 19

[Kuat Eshengazin]

(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]

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]

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?)]

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]

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?)]

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 25

[Jo Hermans]

in the news : http://www.theregister.co.uk/2011/09/19/beast_exploits_paypal_ssl/

Comment 26

[theosophe74]

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]

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]

"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]

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]

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]

<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?)]

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]

TLS 1.2 needs to be implemented in Mozilla ASAP

Comment 34

[jay garcia]

Important article from the ISC:

http://www.dshield.org/diary.html?storyid=11629

Comment 35

[Peter Stendahl-Juvonen]

(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)]

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]

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

[Emily Bowman]

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]

(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]

(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

[Emily Bowman]

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]

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]

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]

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]

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]

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?)]

(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]

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]

+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]

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]

Sorry, the link was not correct, this https://class.coursera.org/crypto-004/lecture/38 is the correct one.

Comment 52

[Martin Schöttler]

(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]

(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 54

[vulcain]

Should add [parity-opera]
http://my.opera.com/core/blog/2009/02/25/new-in-opera-presto-2-2-tls-1-2-support

Comment 55

[Adam Langley]

Attachment: (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]

> 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]

(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?)]

(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]

(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]

-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]

(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]

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]

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]

Attachment: 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]

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]

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]

(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]

Attachment: Inter-diff of the changes due to #66

Comment 69

[Adam Langley]

Attachment: TLS 1.2 support

Comment 70

[David Carlos Manuelda]

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]

Attachment: [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]

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]

Attachment: 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]

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]

Attachment: 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]

(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]

Attachment: 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]

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]

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]

(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]

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]

Attachment: 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]

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]

Great work everyone!

Comment 85

[Kurt Roeckx]

Can someone comment on the current status of this?

Comment 86

[STUNGMEBAS]

(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]

(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]

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]

Attachment: 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]

Attachment: 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]

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]

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]

(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]

(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]

(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]

>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]

Attachment: 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]

(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]

Attachment: 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]

Attachment: 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]

Attachment: 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]

Attachment: 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]

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]

Attachment: 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]

Attachment: 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]

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]

Attachment: 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?)]

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]

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]]

(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.