From 02e36ed3525a2f0fda1b21e948ec5f522cf9379c Mon Sep 17 00:00:00 2001 From: Matt Caswell Date: Mon, 21 Aug 2023 15:11:17 +0100 Subject: Update demos/tutorial to distinguish between stream and connection errors We can use SSL_get_stream_read_state() to distinguish these cases. Reviewed-by: Tomas Mraz Reviewed-by: Hugo Landau (Merged from https://github.com/openssl/openssl/pull/21765) --- demos/guide/quic-client-block.c | 32 ++++++++++++-- demos/guide/quic-multi-stream.c | 61 +++++++++++++++++++++++---- doc/man7/ossl-guide-quic-client-block.pod | 69 ++++++++++++++++++++++++++++++- doc/man7/ossl-guide-quic-multi-stream.pod | 47 ++++++++++++++------- 4 files changed, 181 insertions(+), 28 deletions(-) diff --git a/demos/guide/quic-client-block.c b/demos/guide/quic-client-block.c index be797707f1..54e52d5c28 100644 --- a/demos/guide/quic-client-block.c +++ b/demos/guide/quic-client-block.c @@ -251,13 +251,37 @@ int main(void) * QUIC terms this means that the peer has sent FIN on the stream to * indicate that no further data will be sent. */ - if (SSL_get_error(ssl, 0) != SSL_ERROR_ZERO_RETURN) { + switch (SSL_get_error(ssl, 0)) { + case SSL_ERROR_ZERO_RETURN: + /* Normal completion of the stream */ + break; + + case SSL_ERROR_SSL: /* - * Some error occurred other than a graceful close down by the - * peer. + * Some stream fatal error occurred. This could be because of a stream + * reset - or some failure occurred on the underlying connection. */ + switch (SSL_get_stream_read_state(ssl)) { + case SSL_STREAM_STATE_RESET_REMOTE: + printf("Stream reset occurred\n"); + /* The stream has been reset but the connection is still healthy. */ + break; + + case SSL_STREAM_STATE_CONN_CLOSED: + printf("Connection closed\n"); + /* Connection is already closed. Skip SSL_shutdown() */ + goto end; + + default: + printf("Unknown stream failure\n"); + break; + } + break; + + default: + /* Some other unexpected error occurred */ printf ("Failed reading remaining data\n"); - goto end; + break; } /* diff --git a/demos/guide/quic-multi-stream.c b/demos/guide/quic-multi-stream.c index 7a40d61ad4..86dc6e3502 100644 --- a/demos/guide/quic-multi-stream.c +++ b/demos/guide/quic-multi-stream.c @@ -288,13 +288,37 @@ int main(void) * QUIC terms this means that the peer has sent FIN on the stream to * indicate that no further data will be sent. */ - if (SSL_get_error(stream1, 0) != SSL_ERROR_ZERO_RETURN) { + switch (SSL_get_error(stream1, 0)) { + case SSL_ERROR_ZERO_RETURN: + /* Normal completion of the stream */ + break; + + case SSL_ERROR_SSL: /* - * Some error occurred other than a graceful close down by the - * peer. + * Some stream fatal error occurred. This could be because of a stream + * reset - or some failure occurred on the underlying connection. */ - printf ("Failed reading remaining data from stream 1\n"); - goto end; + switch (SSL_get_stream_read_state(stream1)) { + case SSL_STREAM_STATE_RESET_REMOTE: + printf("Stream reset occurred\n"); + /* The stream has been reset but the connection is still healthy. */ + break; + + case SSL_STREAM_STATE_CONN_CLOSED: + printf("Connection closed\n"); + /* Connection is already closed. Skip SSL_shutdown() */ + goto end; + + default: + printf("Unknown stream failure\n"); + break; + } + break; + + default: + /* Some other unexpected error occurred */ + printf ("Failed reading remaining data\n"); + break; } /* @@ -325,9 +349,30 @@ int main(void) printf("\n"); /* Check for errors on the stream */ - if (SSL_get_error(stream3, 0) != SSL_ERROR_ZERO_RETURN) { - printf ("Failed reading remaining data from stream 3\n"); - goto end; + switch (SSL_get_error(stream3, 0)) { + case SSL_ERROR_ZERO_RETURN: + /* Normal completion of the stream */ + break; + + case SSL_ERROR_SSL: + switch (SSL_get_stream_read_state(stream3)) { + case SSL_STREAM_STATE_RESET_REMOTE: + printf("Stream reset occurred\n"); + break; + + case SSL_STREAM_STATE_CONN_CLOSED: + printf("Connection closed\n"); + goto end; + + default: + printf("Unknown stream failure\n"); + break; + } + break; + + default: + printf ("Failed reading remaining data\n"); + break; } /* diff --git a/doc/man7/ossl-guide-quic-client-block.pod b/doc/man7/ossl-guide-quic-client-block.pod index 6ae193567f..595135c696 100644 --- a/doc/man7/ossl-guide-quic-client-block.pod +++ b/doc/man7/ossl-guide-quic-client-block.pod @@ -241,8 +241,73 @@ client, so we won't repeat it here. We can also perform data transfer using a default QUIC stream that is automatically associated with the B object for us. We can transmit data -using L, and receive data using L in exactly -the same way as for TLS. Again, we won't repeat it here. +using L, and receive data using L in the same +way as for TLS. The main difference is that we have to account for failures +slightly differently. With QUIC the stream can be reset by the peer (which is +fatal for that stream), but the underlying connection itself may still be +healthy. + + /* + * Get up to sizeof(buf) bytes of the response. We keep reading until the + * server closes the connection. + */ + while (SSL_read_ex(ssl, buf, sizeof(buf), &readbytes)) { + /* + * OpenSSL does not guarantee that the returned data is a string or + * that it is NUL terminated so we use fwrite() to write the exact + * number of bytes that we read. The data could be non-printable or + * have NUL characters in the middle of it. For this simple example + * we're going to print it to stdout anyway. + */ + fwrite(buf, 1, readbytes, stdout); + } + /* In case the response didn't finish with a newline we add one now */ + printf("\n"); + + /* + * Check whether we finished the while loop above normally or as the + * result of an error. The 0 argument to SSL_get_error() is the return + * code we received from the SSL_read_ex() call. It must be 0 in order + * to get here. Normal completion is indicated by SSL_ERROR_ZERO_RETURN. In + * QUIC terms this means that the peer has sent FIN on the stream to + * indicate that no further data will be sent. + */ + switch (SSL_get_error(ssl, 0)) { + case SSL_ERROR_ZERO_RETURN: + /* Normal completion of the stream */ + break; + + case SSL_ERROR_SSL: + /* + * Some stream fatal error occurred. This could be because of a stream + * reset - or some failure occurred on the underlying connection. + */ + switch (SSL_get_stream_read_state(ssl)) { + case SSL_STREAM_STATE_RESET_REMOTE: + printf("Stream reset occurred\n"); + /* The stream has been reset but the connection is still healthy. */ + break; + + case SSL_STREAM_STATE_CONN_CLOSED: + printf("Connection closed\n"); + /* Connection is already closed. Skip SSL_shutdown() */ + goto end; + + default: + printf("Unknown stream failure\n"); + break; + } + break; + + default: + /* Some other unexpected error occurred */ + printf ("Failed reading remaining data\n"); + break; + } + +In the above code example you can see that B indicates a stream +fatal error. We can use L to determine whether the +stream has been reset, or if some other fatal error has occurred. =head2 Shutting down the connection diff --git a/doc/man7/ossl-guide-quic-multi-stream.pod b/doc/man7/ossl-guide-quic-multi-stream.pod index 9956fff094..4291c30fa7 100644 --- a/doc/man7/ossl-guide-quic-multi-stream.pod +++ b/doc/man7/ossl-guide-quic-multi-stream.pod @@ -131,14 +131,6 @@ send more data on a stream after L has been called. It is also possible to abandon a stream abnormally by calling L. -=begin comment - -TODO(QUIC): What does SSL_get_error() return in the event that the -peer has reset a stream? If SSL_ERROR_SSL how does an application distinguish -between a stream reset and a connection level fatal error? - -=end comment - Once a stream object is no longer needed it should be freed via a call to L. An application should not call L on it since this is only meaningful for connection level B objects. Freeing the stream @@ -277,7 +269,10 @@ function to find out more details. Since this is a blocking application this will either return B or B indicating a fundamental problem, or it will return B indicating that the stream is concluded and there will be no more data available to read from -it. +it. Care must be taken to distinguish between an error at the stream level (i.e. +a stream reset) and an error at the connection level (i.e. a connection closed). +The L function can be used to distinguish between +these different cases. /* * Check whether we finished the while loop above normally or as the @@ -287,13 +282,37 @@ it. * QUIC terms this means that the peer has sent FIN on the stream to * indicate that no further data will be sent. */ - if (SSL_get_error(stream1, 0) != SSL_ERROR_ZERO_RETURN) { + switch (SSL_get_error(ssl, 0)) { + case SSL_ERROR_ZERO_RETURN: + /* Normal completion of the stream */ + break; + + case SSL_ERROR_SSL: /* - * Some error occurred other than a graceful close down by the - * peer. + * Some stream fatal error occurred. This could be because of a stream + * reset - or some failure occurred on the underlying connection. */ - printf ("Failed reading remaining data from stream 1\n"); - goto end; + switch (SSL_get_stream_read_state(ssl)) { + case SSL_STREAM_STATE_RESET_REMOTE: + printf("Stream reset occurred\n"); + /* The stream has been reset but the connection is still healthy. */ + break; + + case SSL_STREAM_STATE_CONN_CLOSED: + printf("Connection closed\n"); + /* Connection is already closed. Skip SSL_shutdown() */ + goto end; + + default: + printf("Unknown stream failure\n"); + break; + } + break; + + default: + /* Some other unexpected error occurred */ + printf ("Failed reading remaining data\n"); + break; } =head2 Accepting an incoming stream -- cgit v1.2.3