honorCipherOrder should be supported for SNI TLS/SSL connections

[Rush]

Test is here:
https://gist.github.com/RushPL/9376770
Below test for primary domain is correct:

> openssl s_client -servername test1.com -connect localhost:4000 2> /dev/null|grep Cipher
New, TLSv1/SSLv3, Cipher is ECDHE-RSA-AES256-GCM-SHA384
    Cipher    : ECDHE-RSA-AES256-GCM-SHA384

Below test for secondary (SNI resolved) domain is not correct as the cipher should be the same as in the above test:

> openssl s_client -servername test2.com -connect localhost:4000 2> /dev/null|grep Cipher
New, TLSv1/SSLv3, Cipher is RC4-SHA
    Cipher    : RC4-SHA

Forcing server's cipher order is necessary to implement forwarding secrecy https://community.qualys.com/blogs/securitylabs/2013/06/25/ssl-labs-deploying-forward-secrecy

This forbids NSA or other third parties to listen on connections even if the private keys are compromised. Special key exchange happens on negotiation.

I have tried to workaround lack of honorCipherOrder option by manually passing the constant to the Credentials constructor but it fails for some reason. Please advise.

[indutny]

Indeed, this is a bug. Confirming.

[indutny]

Not exactly what I was initially think about, but anyway a fix: #7255

[Rush]

Does it really fix it? It does not for me. In my gist I tried to do secureOptions: require('constants').SSL_OP_CIPHER_SERVER_PREFERENCE but it did not affect the behaviour even before your fix.

[indutny]

You forgot to add ecdhCurve: tls.DEFAULT_ECDH_CURVE to it, without it credentials won't be used for EC* ciphers.

[indutny]

Anyway, I think that my PR has some value. I'm open for discussion of setting ecdhCurve by default in node v0.11-v0.12.

[Rush]

I do not have any opinion on setting ecdhCurve. My only point is that behavior should be consistent between options passed to tls.createServer and options passed to crypto.createCredentials. If ecdhCurve must be explicit it should be explicit in both cases.

[indutny]

I agree with you, but it could be used for different purposes and it is a public method, I'm afraid we can't change it's behavior in v0.10 . I'll open a PR for v0.11 shortly.

[indutny]

Proposed changes are in #7265