Commit 5c23b7b1 authored by Boris Glimcher's avatar Boris Glimcher Committed by Tomasz Zawadzki
Browse files

sock/posix: remove explicit SSL_connect and SSL_accept 



According to openssl documentation,
If necessary, a read/write function will negotiate a TLS/SSL session.
This will allow us to address #3077
And remove blocking code when establishing SSL conections.

SSL_set_app_data() sets a pointer for ssl object for additional
application data. Up till now impl_opts structures were being allocated
in posix_sock_create() and then passed to ssl_sock_connect_loop()
for a handshake.
Now, when SSL_connect() is removed, the handshake happens after
execution returns from posix_sock_create(), so the memory is
already discarded. Setting the pointer in the right place now
avoids accessing freed memory.

Fixes #3077

Change-Id: I8389b34b5e61573dafb946419f922603d8d79b1d
Signed-off-by: default avatarBoris Glimcher <Boris.Glimcher@emc.com>
Signed-off-by: default avatarKrzysztof Karas <krzysztof.karas@intel.com>
Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/19241


Reviewed-by: default avatarTomasz Zawadzki <tomasz.zawadzki@intel.com>
Reviewed-by: default avatarKonrad Sztyber <konrad.sztyber@intel.com>
Community-CI: Mellanox Build Bot
Tested-by: default avatarSPDK CI Jenkins <sys_sgci@intel.com>
parent 71ac6d7e

module/sock/posix/posix.c

+10 −44
Original line number Diff line number Diff line
@@ -807,11 +807,9 @@ err: 
}



static SSL *

ssl_sock_connect_loop(SSL_CTX *ctx, int fd, struct spdk_sock_impl_opts *impl_opts)

ssl_sock_setup_connect(SSL_CTX *ctx, int fd)

{

	int rc;

	SSL *ssl;

	int ssl_get_error;



	ssl = SSL_new(ctx);

	if (!ssl) {
@@ -819,26 +817,10 @@ ssl_sock_connect_loop(SSL_CTX *ctx, int fd, struct spdk_sock_impl_opts *impl_opt 
		return NULL;

	}

	SSL_set_fd(ssl, fd);

	SSL_set_app_data(ssl, impl_opts);

	SSL_set_connect_state(ssl);

	SSL_set_psk_use_session_callback(ssl, posix_sock_psk_use_session_client_cb);

	SPDK_DEBUGLOG(sock_posix, "SSL object creation finished: %p\n", ssl);

	SPDK_DEBUGLOG(sock_posix, "%s = SSL_state_string_long(%p)\n", SSL_state_string_long(ssl), ssl);

	while ((rc = SSL_connect(ssl)) != 1) {

		SPDK_DEBUGLOG(sock_posix, "%s = SSL_state_string_long(%p)\n", SSL_state_string_long(ssl), ssl);

		ssl_get_error = SSL_get_error(ssl, rc);

		SPDK_DEBUGLOG(sock_posix, "SSL_connect failed %d = SSL_connect(%p), %d = SSL_get_error(%p, %d)\n",

			      rc, ssl, ssl_get_error, ssl, rc);

		switch (ssl_get_error) {

		case SSL_ERROR_WANT_READ:

		case SSL_ERROR_WANT_WRITE:

			continue;

		default:

			break;

		}

		SPDK_ERRLOG("SSL_connect() failed, errno = %d\n", errno);

		SSL_free(ssl);

		return NULL;

	}

	SPDK_DEBUGLOG(sock_posix, "%s = SSL_state_string_long(%p)\n", SSL_state_string_long(ssl), ssl);

	SPDK_DEBUGLOG(sock_posix, "Negotiated Cipher suite:%s\n",

		      SSL_CIPHER_get_name(SSL_get_current_cipher(ssl)));
@@ -846,11 +828,9 @@ ssl_sock_connect_loop(SSL_CTX *ctx, int fd, struct spdk_sock_impl_opts *impl_opt 
}



static SSL *

ssl_sock_accept_loop(SSL_CTX *ctx, int fd, struct spdk_sock_impl_opts *impl_opts)

ssl_sock_setup_accept(SSL_CTX *ctx, int fd)

{

	int rc;

	SSL *ssl;

	int ssl_get_error;



	ssl = SSL_new(ctx);

	if (!ssl) {
@@ -858,26 +838,10 @@ ssl_sock_accept_loop(SSL_CTX *ctx, int fd, struct spdk_sock_impl_opts *impl_opts 
		return NULL;

	}

	SSL_set_fd(ssl, fd);

	SSL_set_app_data(ssl, impl_opts);

	SSL_set_accept_state(ssl);

	SSL_set_psk_find_session_callback(ssl, posix_sock_psk_find_session_server_cb);

	SPDK_DEBUGLOG(sock_posix, "SSL object creation finished: %p\n", ssl);

	SPDK_DEBUGLOG(sock_posix, "%s = SSL_state_string_long(%p)\n", SSL_state_string_long(ssl), ssl);

	while ((rc = SSL_accept(ssl)) != 1) {

		SPDK_DEBUGLOG(sock_posix, "%s = SSL_state_string_long(%p)\n", SSL_state_string_long(ssl), ssl);

		ssl_get_error = SSL_get_error(ssl, rc);

		SPDK_DEBUGLOG(sock_posix, "SSL_accept failed %d = SSL_accept(%p), %d = SSL_get_error(%p, %d)\n", rc,

			      ssl, ssl_get_error, ssl, rc);

		switch (ssl_get_error) {

		case SSL_ERROR_WANT_READ:

		case SSL_ERROR_WANT_WRITE:

			continue;

		default:

			break;

		}

		SPDK_ERRLOG("SSL_accept() failed, errno = %d\n", errno);

		SSL_free(ssl);

		return NULL;

	}

	SPDK_DEBUGLOG(sock_posix, "%s = SSL_state_string_long(%p)\n", SSL_state_string_long(ssl), ssl);

	SPDK_DEBUGLOG(sock_posix, "Negotiated Cipher suite:%s\n",

		      SSL_CIPHER_get_name(SSL_get_current_cipher(ssl)));
@@ -1080,9 +1044,9 @@ retry: 
					fd = -1;

					break;

				}

				ssl = ssl_sock_connect_loop(ctx, fd, &impl_opts);

				ssl = ssl_sock_setup_connect(ctx, fd);

				if (!ssl) {

					SPDK_ERRLOG("ssl_sock_connect_loop() failed, errno = %d\n", errno);

					SPDK_ERRLOG("ssl_sock_setup_connect() failed, errno = %d\n", errno);

					close(fd);

					fd = -1;

					SSL_CTX_free(ctx);
@@ -1126,6 +1090,7 @@ retry: 


	if (ssl) {

		sock->ssl = ssl;

		SSL_set_app_data(ssl, &sock->base.impl_opts);

	}



	return &sock->base;
@@ -1194,9 +1159,9 @@ _posix_sock_accept(struct spdk_sock *_sock, bool enable_ssl) 
			close(fd);

			return NULL;

		}

		ssl = ssl_sock_accept_loop(ctx, fd, &sock->base.impl_opts);

		ssl = ssl_sock_setup_accept(ctx, fd);

		if (!ssl) {

			SPDK_ERRLOG("ssl_sock_accept_loop() failed, errno = %d\n", errno);

			SPDK_ERRLOG("ssl_sock_setup_accept() failed, errno = %d\n", errno);

			close(fd);

			SSL_CTX_free(ctx);

			return NULL;
@@ -1218,6 +1183,7 @@ _posix_sock_accept(struct spdk_sock *_sock, bool enable_ssl) 


	if (ssl) {

		new_sock->ssl = ssl;

		SSL_set_app_data(ssl, &new_sock->base.impl_opts);

	}



	return &new_sock->base;