{"action":"created","issue":{"url":"https://api.github.com/repos/python-trio/trio/issues/819","repository_url":"https://api.github.com/repos/python-trio/trio","labels_url":"https://api.github.com/repos/python-trio/trio/issues/819/labels{/name}","comments_url":"https://api.github.com/repos/python-trio/trio/issues/819/comments","events_url":"https://api.github.com/repos/python-trio/trio/issues/819/events","html_url":"https://github.com/python-trio/trio/issues/819","id":393723939,"node_id":"MDU6SXNzdWUzOTM3MjM5Mzk=","number":819,"title":"How should we handle TLS 1.3 session tickets?","user":{"login":"njsmith","id":609896,"node_id":"MDQ6VXNlcjYwOTg5Ng==","avatar_url":"https://avatars0.githubusercontent.com/u/609896?v=4","gravatar_id":"","url":"https://api.github.com/users/njsmith","html_url":"https://github.com/njsmith","followers_url":"https://api.github.com/users/njsmith/followers","following_url":"https://api.github.com/users/njsmith/following{/other_user}","gists_url":"https://api.github.com/users/njsmith/gists{/gist_id}","starred_url":"https://api.github.com/users/njsmith/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/njsmith/subscriptions","organizations_url":"https://api.github.com/users/njsmith/orgs","repos_url":"https://api.github.com/users/njsmith/repos","events_url":"https://api.github.com/users/njsmith/events{/privacy}","received_events_url":"https://api.github.com/users/njsmith/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"assignees":[],"milestone":null,"comments":2,"created_at":"2018-12-23T05:01:37Z","updated_at":"2018-12-23T08:43:05Z","closed_at":null,"author_association":"MEMBER","body":"We have a test that tries running `SSLStream` over an unbuffered transport, in part to confirm we aren't making assumptions about buffering. It turns out that openssl 1.1.1/TLS 1.3 does make assumptions about buffering, and the test hangs. Now what?\r\n\r\nSpecifically: we [create an unbuffered transport, then call `do_handshake` on both the client and server sides](https://github.com/python-trio/trio/blob/5a03bee3fbf1a5364a3584ef8150b04143dcecdb/trio/tests/test_ssl.py#L762-L771). The `client.do_handshake()` completes successfully. The `server.do_handshake()` hangs.\r\n\r\nI believe what's happening is that when openssl 1.1.1 is doing TLS 1.3, the server automatically sends some session tickets after the handshake completes. Technically these aren't part of the handshake, so the `client.do_handshake()` call doesn't wait for them. But on the server side we don't know this; all we can see is that we called `SSL_do_handshake` and it spit out some data to send, so we assume that the handshake won't complete until we send that data.\r\n\r\nI guess there are two scenarios that the designers imagined: (1) the client is trying to read application-level data, so it's reading from the transport, and it gets the session tickets as a bonus. (2) the client isn't trying to read application-level data, so it's not reading from the transport, but the transport has enough intrinsic buffering that the session tickets can just sit there forever without causing any problems. But in our test neither is true, so it breaks.\r\n\r\nAs far as I know, this shouldn't have too much real-world impact, since real transports almost always have some buffering. (Empirically the session tickets appear to be 510 bytes, which is smaller than pretty much any real transport buffer.) And, there is a workaround available: modify `clogged_stream_maker` so the server does `await server.do_handshake(); await server.send_all(b\"x\")`, and the client does `await client.do_handshake(); await client.receive_some(1)`. Sending a bit of application level data from server→client like this should flush out the session tickets and get back to the state we want for the rest of the tests.\r\n\r\nNonetheless, it's not ideal that openssl is making subtle assumptions about buffering like this. Shoudl we do something?\r\n\r\nUnfortunately, TLS 1.3's decision to move session tickets out of the handshake doesn't give us a lot of options here. We could disable session tickets, but that seems sub-optimal (and currently the `ssl` module doesn't expose a knob for this anyway).\r\n\r\nWe could potentially delay sending the session tickets until the first time we send application-level data. That way `server.do_handshake()` wouldn't need to make any assumptions about buffering, and it's totally reasonable that `server.send_all(...)` has to wait for the peer to call `client.receive_some(...)`. But, there are two challenges:\r\n\r\n* I'm not sure how to implement this. Specifically, it requires figuring out that the data openssl has dumped into its send buffer is the session tickets, as opposed to part of the handshake. Is there any reliable way to do this?\r\n\r\n* There are some cases where session tickets are successfully delivered now, but wouldn't be if we implemented this. Specifically, if you have a connection where the server never sends any application-level data, *but* the client does read from network. This sounds a little weird, but it could happen: e.g. any twisted or asyncio client will act like this; or, if you're using trio, you could intentionally implement a client like this exactly because you want to get session tickets.\r\n\r\nSo.... I'm not having any brilliant ideas here. I think for right now we'll just implement the workaround to get the test suite working, and then wait to see if this becomes an issue or not.\r\n\r\nCC: @tiran – I don't think there's anything for you to do here in particular, but it seems like the kind of issue that you like to know about."},"comment":{"url":"https://api.github.com/repos/python-trio/trio/issues/comments/449622456","html_url":"https://github.com/python-trio/trio/issues/819#issuecomment-449622456","issue_url":"https://api.github.com/repos/python-trio/trio/issues/819","id":449622456,"node_id":"MDEyOklzc3VlQ29tbWVudDQ0OTYyMjQ1Ng==","user":{"login":"njsmith","id":609896,"node_id":"MDQ6VXNlcjYwOTg5Ng==","avatar_url":"https://avatars0.githubusercontent.com/u/609896?v=4","gravatar_id":"","url":"https://api.github.com/users/njsmith","html_url":"https://github.com/njsmith","followers_url":"https://api.github.com/users/njsmith/followers","following_url":"https://api.github.com/users/njsmith/following{/other_user}","gists_url":"https://api.github.com/users/njsmith/gists{/gist_id}","starred_url":"https://api.github.com/users/njsmith/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/njsmith/subscriptions","organizations_url":"https://api.github.com/users/njsmith/orgs","repos_url":"https://api.github.com/users/njsmith/repos","events_url":"https://api.github.com/users/njsmith/events{/privacy}","received_events_url":"https://api.github.com/users/njsmith/received_events","type":"User","site_admin":false},"created_at":"2018-12-23T08:43:05Z","updated_at":"2018-12-23T08:43:05Z","author_association":"MEMBER","body":"Boiling this down further: the major issue with the auto-sending session tickets is that they cannot work reliably in any usage where the server never sends data to the client. As a special case, that includes our toy testcases that just do things like establish a connection and then close it immediately, but it's much more general than that. Also, it doesn't matter whether you use standards-compliant mode or HTTPS-compliant mode.\r\n\r\nHere are two openssl issues where people ran into these problems before me:\r\n\r\n* https://github.com/openssl/openssl/issues/6342 (hi @tiran!) – not much useful here\r\n* https://github.com/openssl/openssl/issues/6904 – lots of discussion, leading to this PR: https://github.com/openssl/openssl/pull/6944\r\n\r\nAs noted in the second issue, we could potentially avoid the close-related problems if the client always did a full round-trip shutdown, because waiting for the server's close_notify would automatically consume the session tickets. Currently Trio never does a full round-trip shutdown: if we're in standards-compliant mode then we send a close_notify and then immediately close the connection, and if we're in HTTPS-compatible mode then we skip the close_notify entirely. I suppose we could detect when we're using TLS 1.3 and switch to attempting the full round-trip close_notify dance, and ignore errors as appropriate. This seems suboptimal though; upgrading to TLS 1.3 isn't supposed to *add* round-trips. And anyway, when we're a server we can't control what clients connecting to us do! And clients are specifically allowed by the standard to skip doing a full round-trip shutdown.\r\n\r\nThe PR attempts to implement a different solution: it ignores EPIPE/ECONNRESET if those occur while attempting to send session tickets. But this has two problems AFAICT. First, we use memory BIOs and then do the I/O ourselves, so any EPIPE/ECONNRESET occurs later, after the call to `SSL_do_handshake`. They [suggest](https://github.com/openssl/openssl/pull/6944#issuecomment-434619051) that in this case it's our job to figure out how to deal with it, which would be fair enough, except that we can't implement the same behavior, because we don't know whether the data we're attempting to send is the session tickets, because openssl doesn't tell us that.\r\n\r\nBut that doesn't really matter, because there's another problem that's much more serious. Even if you ignore EPIPE/ECONNRESET while sending the session tickets, that doesn't guarantee you can go back to calling `recv` afterwards, because of how TCP works!\r\n\r\n...Ugh, I guess I'm filing a bug on openssl."}}
{"actor":{"display_login":"njsmith"}}