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