<div dir="ltr"><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Tue, Jan 22, 2019 at 1:48 PM Viktor Dukhovni <<a href="mailto:openssl-users@dukhovni.org">openssl-users@dukhovni.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><br>
<br>
> On Jan 22, 2019, at 2:06 PM, Adam Langley <<a href="mailto:agl@imperialviolet.org" target="_blank">agl@imperialviolet.org</a>> wrote:<br>
> <br>
> (This is another installment of our experiences with deploying the<br>
> RFC-final TLS 1.3—previous messages: [1][2]. We share these with the<br>
> community to hopefully avoid other people hitting the same issues.)<br>
> <br>
> [...]<br>
> <br>
> However, OpenSSL 1.1.1a signals SSL_CB_HANDSHAKE_START when TLS 1.3<br>
> post-handshake messages are received[5], including KeyUpdate. This<br>
> causes KeyUpdate messages to break with, at least, HAProxy, and with<br>
> NGINX prior to this commit[6]. (There may well be more, but that level<br>
> of breakage was enough to drown any other signal.)<br>
> <br>
> Lastly, OpenSSL 1.1.1a imposes a hard limit of 32 KeyUpdate messages<br>
> per connection[7]. Therefore clients that send periodic KeyUpdates<br>
> based on elapsed time or transmitted bytes will eventually hit that<br>
> limit, which is fatal to the connection.<br>
> <br>
> Therefore KeyUpdate messages are not currently viable on the web, at<br>
> least when client initiated.<br>
> <br>
> [1] <a href="https://mailarchive.ietf.org/arch/msg/tls/PLtOD4kROZFfNtPKzSoMyIUOzuE" rel="noreferrer" target="_blank">https://mailarchive.ietf.org/arch/msg/tls/PLtOD4kROZFfNtPKzSoMyIUOzuE</a><br>
> [2] <a href="https://mailarchive.ietf.org/arch/msg/tls/pixg5cBXHuwd3MtMIn_xIhWmGGQ" rel="noreferrer" target="_blank">https://mailarchive.ietf.org/arch/msg/tls/pixg5cBXHuwd3MtMIn_xIhWmGGQ</a><br>
> [3] <a href="https://bugs.openjdk.java.net/browse/JDK-8211806" rel="noreferrer" target="_blank">https://bugs.openjdk.java.net/browse/JDK-8211806</a><br>
> [4] <a href="https://bugs.openjdk.java.net/browse/JDK-8213202" rel="noreferrer" target="_blank">https://bugs.openjdk.java.net/browse/JDK-8213202</a><br>
> [5] <a href="https://github.com/openssl/openssl/issues/8069" rel="noreferrer" target="_blank">https://github.com/openssl/openssl/issues/8069</a><br>
> [6] <a href="https://trac.nginx.org/nginx/changeset/e3ba4026c02d2c1810fd6f2cecf499fc39dde5ee/nginx/src/event/ngx_event_openssl.c" rel="noreferrer" target="_blank">https://trac.nginx.org/nginx/changeset/e3ba4026c02d2c1810fd6f2cecf499fc39dde5ee/nginx/src/event/ngx_event_openssl.c</a><br>
> [7] <a href="https://github.com/openssl/openssl/issues/8068" rel="noreferrer" target="_blank">https://github.com/openssl/openssl/issues/8068</a><br>
> [8] <a href="https://twitter.com/__subodh/status/1085642001595265024" rel="noreferrer" target="_blank">https://twitter.com/__subodh/status/1085642001595265024</a><br>
<br>
I think we should remediate the reported issues in the 1.1.1b release.<br>
We should probably clear the keyUpdate count when sufficient application<br>
data has been received from the peer.  Where sufficient could be as little<br>
as 1 byte, or could be something more reasonable (say 1MB, allowing for<br>
up to 32 rekeys per MB, which is plenty).<br>
<br>
As for applications mishandling "SSL_CB_HANDSHAKE_START", not quite sure<br>
what to do there, but perhaps we could define a new even for keyUpdates<br>
that does not mislead applications into assuming a new "handshake".<br></blockquote><div><br></div><div>I think this is clearly the right option. This is a compatibility break, IMO.</div><div><br></div><div>Prior to SSL_OP_NO_RENEGOTIATION (new in the same release that added 1.3), this was the only way to disable renegotiations. I've seen a lot of codebases do this. I don't think one could even call it a mishandling. The natural interpretation of "SSL_CB_HANDSHAKE_START" is that a handshake has started. Thus, if you wish to block renegotiations, absent a more direct API, it's natural to count those and fail if you see more than one.</div><div><br></div><div>A KeyUpdate is not a handshake. It's some internal rejiggering of the record layer. In an alternate universe, the IETF might have specified KeyUpdate as distinct record type. (It probably should have, given that KeyUpdate in DTLS and QUIC necessarily look different.)</div><div><br></div><div>Skimming the code, it looks like OpenSSL only surfaces SSL_CB_HANDSHAKE_START because it internally sends post-handshake messages through the handshake path? This is an implementation detail shouldn't be visible in the public API. At the public API, one needs to think about what a given API <i>means</i>.</div><div><br></div><div>(Does that implementation strategy even work? Handshakes tie up read and write flows, while the unidirectional post-handshake messages shouldn't. If you're trying to send a KeyUpdate and the transport blocks, does that block SSL_read? If I send you half a NewSessionTicket and pause, does that block SSL_write?)</div><div><br></div><div>BoringSSL does not signal SSL_CB_HANDSHAKE_START on KeyUpdate, just something out the msg_callback. One could imagine dedicated SSL_CB_* hooks for KeyUpdate if anyone needed it, I suppose, but it's unclear to me if that's necessary.</div><div><br></div><div>David</div></div></div>