From e440f51395f10e307f720213bd75393e446024a3 Mon Sep 17 00:00:00 2001 From: Matt Caswell Date: Thu, 8 Mar 2018 17:44:12 +0000 Subject: [PATCH] Give more information in the SSL_stateless return code Allow users to distinguish between an error occurring and an HRR being issued. Fixes #5549 Reviewed-by: Rich Salz (Merged from https://github.com/openssl/openssl/pull/5562) --- crypto/err/openssl.txt | 1 + doc/man3/DTLSv1_listen.pod | 5 +++-- include/openssl/sslerr.h | 1 + ssl/ssl_err.c | 2 ++ ssl/ssl_lib.c | 5 ++++- ssl/statem/extensions_srvr.c | 9 +++++++-- test/sslapitest.c | 32 ++++++++++++++++++++++++++------ 7 files changed, 44 insertions(+), 11 deletions(-) diff --git a/crypto/err/openssl.txt b/crypto/err/openssl.txt index 3f1c735417..842a4209dc 100644 --- a/crypto/err/openssl.txt +++ b/crypto/err/openssl.txt @@ -2546,6 +2546,7 @@ SSL_R_NO_CIPHERS_SPECIFIED:183:no ciphers specified SSL_R_NO_CIPHER_MATCH:185:no cipher match SSL_R_NO_CLIENT_CERT_METHOD:331:no client cert method SSL_R_NO_COMPRESSION_SPECIFIED:187:no compression specified +SSL_R_NO_COOKIE_CALLBACK_SET:287:no cookie callback set SSL_R_NO_GOST_CERTIFICATE_SENT_BY_PEER:330:\ Peer haven't sent GOST certificate, required for selected ciphersuite SSL_R_NO_METHOD_SPECIFIED:188:no method specified diff --git a/doc/man3/DTLSv1_listen.pod b/doc/man3/DTLSv1_listen.pod index 50f16bc81b..70f6a25cde 100644 --- a/doc/man3/DTLSv1_listen.pod +++ b/doc/man3/DTLSv1_listen.pod @@ -88,8 +88,9 @@ start. =head1 RETURN VALUES For SSL_stateless() a return value of 1 indicates success and the B object -will be set up ready to continue the handshake. A return value of 0 indicates -failure. User code may retry the SSL_stateless() call. +will be set up ready to continue the handshake. A return value of 0 or -1 +indicates failure. If the value is 0 then a HelloRetryRequest was sent. A value +of -1 indicates any other error. User code may retry the SSL_stateless() call. For DTLSv1_listen() a return value of >= 1 indicates success. The B object will be set up ready to continue the handshake. the B value will also be diff --git a/include/openssl/sslerr.h b/include/openssl/sslerr.h index 1a02268846..32fe3662f2 100644 --- a/include/openssl/sslerr.h +++ b/include/openssl/sslerr.h @@ -587,6 +587,7 @@ int ERR_load_SSL_strings(void); # define SSL_R_NO_CIPHER_MATCH 185 # define SSL_R_NO_CLIENT_CERT_METHOD 331 # define SSL_R_NO_COMPRESSION_SPECIFIED 187 +# define SSL_R_NO_COOKIE_CALLBACK_SET 287 # define SSL_R_NO_GOST_CERTIFICATE_SENT_BY_PEER 330 # define SSL_R_NO_METHOD_SPECIFIED 188 # define SSL_R_NO_PEM_EXTENSIONS 389 diff --git a/ssl/ssl_err.c b/ssl/ssl_err.c index f0bde60994..34e8ec4076 100644 --- a/ssl/ssl_err.c +++ b/ssl/ssl_err.c @@ -952,6 +952,8 @@ static const ERR_STRING_DATA SSL_str_reasons[] = { "no client cert method"}, {ERR_PACK(ERR_LIB_SSL, 0, SSL_R_NO_COMPRESSION_SPECIFIED), "no compression specified"}, + {ERR_PACK(ERR_LIB_SSL, 0, SSL_R_NO_COOKIE_CALLBACK_SET), + "no cookie callback set"}, {ERR_PACK(ERR_LIB_SSL, 0, SSL_R_NO_GOST_CERTIFICATE_SENT_BY_PEER), "Peer haven't sent GOST certificate, required for selected ciphersuite"}, {ERR_PACK(ERR_LIB_SSL, 0, SSL_R_NO_METHOD_SPECIFIED), diff --git a/ssl/ssl_lib.c b/ssl/ssl_lib.c index accef0c0ce..f5219c22d1 100644 --- a/ssl/ssl_lib.c +++ b/ssl/ssl_lib.c @@ -5352,7 +5352,10 @@ int SSL_stateless(SSL *s) if (ret > 0 && s->ext.cookieok) return 1; - return 0; + if (s->hello_retry_request == SSL_HRR_PENDING && !ossl_statem_in_error(s)) + return 0; + + return -1; } void SSL_force_post_handshake_auth(SSL *ssl) diff --git a/ssl/statem/extensions_srvr.c b/ssl/statem/extensions_srvr.c index bcabb858be..b9692f46e4 100644 --- a/ssl/statem/extensions_srvr.c +++ b/ssl/statem/extensions_srvr.c @@ -1682,10 +1682,15 @@ EXT_RETURN tls_construct_stoc_cookie(SSL *s, WPACKET *pkt, unsigned int context, EVP_PKEY *pkey; int ret = EXT_RETURN_FAIL; - if (s->ctx->app_gen_cookie_cb == NULL - || (s->s3->flags & TLS1_FLAGS_STATELESS) == 0) + if ((s->s3->flags & TLS1_FLAGS_STATELESS) == 0) return EXT_RETURN_NOT_SENT; + if (s->ctx->app_gen_cookie_cb == NULL) { + SSLfatal(s, SSL_AD_INTERNAL_ERROR, SSL_F_TLS_CONSTRUCT_STOC_COOKIE, + SSL_R_NO_COOKIE_CALLBACK_SET); + return EXT_RETURN_FAIL; + } + if (!WPACKET_put_bytes_u16(pkt, TLSEXT_TYPE_cookie) || !WPACKET_start_sub_packet_u16(pkt) || !WPACKET_start_sub_packet_u16(pkt) diff --git a/test/sslapitest.c b/test/sslapitest.c index c183ca8108..ce903646c1 100644 --- a/test/sslapitest.c +++ b/test/sslapitest.c @@ -2734,20 +2734,40 @@ static int test_stateless(void) &cctx, cert, privkey))) goto end; + /* The arrival of CCS messages can confuse the test */ + SSL_CTX_clear_options(cctx, SSL_OP_ENABLE_MIDDLEBOX_COMPAT); + + if (!TEST_true(create_ssl_objects(sctx, cctx, &serverssl, &clientssl, + NULL, NULL)) + /* Send the first ClientHello */ + || !TEST_false(create_ssl_connection(serverssl, clientssl, + SSL_ERROR_WANT_READ)) + /* + * This should fail with a -1 return because we have no callbacks + * set up + */ + || !TEST_int_eq(SSL_stateless(serverssl), -1)) + goto end; + + /* Fatal error so abandon the connection from this client */ + SSL_free(clientssl); + clientssl = NULL; + /* Set up the cookie generation and verification callbacks */ SSL_CTX_set_cookie_generate_cb(sctx, generate_cookie_callback); SSL_CTX_set_cookie_verify_cb(sctx, verify_cookie_callback); - /* The arrival of CCS messages can confuse the test */ - SSL_CTX_clear_options(cctx, SSL_OP_ENABLE_MIDDLEBOX_COMPAT); - + /* + * Create a new connection from the client (we can reuse the server SSL + * object). + */ if (!TEST_true(create_ssl_objects(sctx, cctx, &serverssl, &clientssl, NULL, NULL)) /* Send the first ClientHello */ || !TEST_false(create_ssl_connection(serverssl, clientssl, SSL_ERROR_WANT_READ)) /* This should fail because there is no cookie */ - || !TEST_false(SSL_stateless(serverssl))) + || !TEST_int_eq(SSL_stateless(serverssl), 0)) goto end; /* Abandon the connection from this client */ @@ -2764,12 +2784,12 @@ static int test_stateless(void) || !TEST_false(create_ssl_connection(serverssl, clientssl, SSL_ERROR_WANT_READ)) /* This should fail because there is no cookie */ - || !TEST_false(SSL_stateless(serverssl)) + || !TEST_int_eq(SSL_stateless(serverssl), 0) /* Send the second ClientHello */ || !TEST_false(create_ssl_connection(serverssl, clientssl, SSL_ERROR_WANT_READ)) /* This should succeed because a cookie is now present */ - || !TEST_true(SSL_stateless(serverssl)) + || !TEST_int_eq(SSL_stateless(serverssl), 1) /* Complete the connection */ || !TEST_true(create_ssl_connection(serverssl, clientssl, SSL_ERROR_NONE))) -- 2.34.1