Bug 1286141 - Add a control for all named groups

Bug 1286141 - Add a control for all named groups

[ttaubert, 2016-07-12 10:20:49]

LGTM with comments addressed

https://codereview.appspot.com/305750043/diff/1/external_tests/ssl_gtest/tls_...
File external_tests/ssl_gtest/tls_agent.cc (right):

https://codereview.appspot.com/305750043/diff/1/external_tests/ssl_gtest/tls_...
external_tests/ssl_gtest/tls_agent.cc:274: SECStatus rv =
SSL_NamedGroupPrefSet(ssl_fd_, group, en ? PR_TRUE : PR_FALSE);
Maybe:

SECStatus rv = SSL_NamedGroupPrefSet(ssl_fd_, group, (PRBool)en);

https://codereview.appspot.com/305750043/diff/1/external_tests/ssl_gtest/tls_...
external_tests/ssl_gtest/tls_agent.cc:370: void
TlsAgent::CheckKEAType(SSLKEAType type, size_t kea_size) const {
Could we overload CheckKEAType() instead of returning early?

https://codereview.appspot.com/305750043/diff/1/external_tests/ssl_gtest/tls_...
external_tests/ssl_gtest/tls_agent.cc:397: void
TlsAgent::CheckAuthType(SSLAuthType type, size_t auth_size) const {
Seems unused?

https://codereview.appspot.com/305750043/diff/1/lib/ssl/ssl.h
File lib/ssl/ssl.h (right):

https://codereview.appspot.com/305750043/diff/1/lib/ssl/ssl.h#newcode393
lib/ssl/ssl.h:393: ** signatures that are used.
Maybe: EC groups are used to negotiate both ECDHE key exchange and ECDSA
signatures for authentication.

[mt, 2016-07-12 23:15:20]

Updated.

https://codereview.appspot.com/305750043/diff/1/external_tests/ssl_gtest/tls_...
File external_tests/ssl_gtest/tls_agent.cc (right):

https://codereview.appspot.com/305750043/diff/1/external_tests/ssl_gtest/tls_...
external_tests/ssl_gtest/tls_agent.cc:274: SECStatus rv =
SSL_NamedGroupPrefSet(ssl_fd_, group, en ? PR_TRUE : PR_FALSE);
On 2016/07/12 10:20:49, ttaubert wrote:
> Maybe:
> 
> SECStatus rv = SSL_NamedGroupPrefSet(ssl_fd_, group, (PRBool)en);

Just following the established pattern (see the function below).  And C casts
are right out.

https://codereview.appspot.com/305750043/diff/1/external_tests/ssl_gtest/tls_...
external_tests/ssl_gtest/tls_agent.cc:370: void
TlsAgent::CheckKEAType(SSLKEAType type, size_t kea_size) const {
On 2016/07/12 10:20:49, ttaubert wrote:
> Could we overload CheckKEAType() instead of returning early?

Yes, that works much better.

https://codereview.appspot.com/305750043/diff/1/external_tests/ssl_gtest/tls_...
external_tests/ssl_gtest/tls_agent.cc:397: void
TlsAgent::CheckAuthType(SSLAuthType type, size_t auth_size) const {
On 2016/07/12 10:20:49, ttaubert wrote:
> Seems unused?

Done.

https://codereview.appspot.com/305750043/diff/1/lib/ssl/ssl.h
File lib/ssl/ssl.h (right):

https://codereview.appspot.com/305750043/diff/1/lib/ssl/ssl.h#newcode393
lib/ssl/ssl.h:393: ** signatures that are used.
On 2016/07/12 10:20:49, ttaubert wrote:
> Maybe: EC groups are used to negotiate both ECDHE key exchange and ECDSA
> signatures for authentication.

Done.

[mt, 2016-07-26 12:52:34]

rebased