Hi Dirkjan,
On Tue, Jan 22, 2019 at 11:07:07PM -0800, Dirkjan Bussink wrote:
> I have adjusted the patch to make it more robust and more match the style of
> how we use other options. How does this look to you?
Unfortunately it does introduce the problem I feared for BoringSSL :
+#if !defined(SSL_OP_NO_RENEGOTIATION) || SSL_OP_NO_RENEGOTIATION == 0
if (where & SSL_CB_HANDSHAKE_START) {
/* Disable renegotiation (CVE-2009-3555) */
if ((conn->flags & (CO_FL_CONNECTED | CO_FL_EARLY_SSL_HS |
CO_FL_EARLY_DATA)) == CO_FL_CONNECTED) {
@@ -1475,6 +1476,7 @@ void ssl_sock_infocbk(const SSL *ssl, int where, int ret)
conn->err_code = CO_ER_SSL_RENEG;
}
}
+#endif
As you can see it will enable this code when SSL_OP_NO_RENEGOTIATION=0,
which is what BoringSSL does and it needs this code to be disabled. Thus
I think it's better to simply do this :
+#ifndef SSL_OP_NO_RENEGOTIATION
+ /* Please note that BoringSSL defines this macro to zero so don't
+ * change this to #if and do not assign a default value to this macro!
+ */
Thanks,
Willy
