Welcome! Log In Create A New Profile

Advanced

Re: [PATCH] OpenSSL-1.1.0锛歋upport Asynchronous Operations of SSL with openSSL-1.1.0

Maxim Dounin
September 08, 2021 12:22PM
Hello!

On Tue, Sep 07, 2021 at 04:48:08PM +0800, Junli Liu wrote:

> # HG changeset patch
> # User Junli Liu<jlliudh@isoftstone.com>
> # Date 1631003347 -28800
> # Tue Sep 07 16:29:07 2021 +0800
> # Node ID 301a837387ed63bb2e455942ef2ef79bc9aaa972
> # Parent 2245324a507abc54cf0274fd1b1e81bfac7c1c73
> OpenSSL-1.1.0:Support Asynchronous Operations of SSL with openSSL-1.1.0
>
> Security is critical to the foundation of networking and Transport Layer Security (TLS) is the backbone protocol for Internet security today. But normally, the introduction of TLS usually leads to network performance degradation, because encryption and decryption need to consume more compute resources.OpenSSL-1.1.0 has involved features of asynchronous operations to improve the performance of network and often combined with hardware feature.
>
> This changeset make Nginx can work well with OpenSSL asynchronous operations when process http request.

This patch seems to be borrowed from Intel's async work
(https://github.com/intel/asynch_mode_nginx). Do you have legal
rights to submit this code?

Also, our previous testing of the code by Intel suggests that the
code is buggy and unstable, and only beneficial when using Intel
QAT with RSA certificates, but not with ECDSA. Do you have any
testing results which indicate that the patch is beneficial?

See below for some obvious comments about the code (not a full
review).

>
> diff -r 2245324a507a -r 301a837387ed auto/lib/openssl/conf
> --- a/auto/lib/openssl/conf Thu Sep 02 12:25:37 2021 +0300
> +++ b/auto/lib/openssl/conf Tue Sep 07 16:29:07 2021 +0800
> @@ -139,4 +139,33 @@
> exit 1
> fi
>
> + OPENSSL_ASYNC=
> + if [ "$NGX_SSL_ASYNC" != NO ]; then
> +
> + OPENSSL_ASYNC=NO
> +
> + ngx_feature="OpenSSL library"
> + ngx_feature_name=
> + ngx_feature_run=no
> + ngx_feature_incs="#include <openssl/ssl.h>"
> + ngx_feature_path=
> + ngx_feature_libs="-lssl -lcrypto"
> + ngx_feature_test="#ifndef SSL_MODE_ASYNC
> + error: not define async
> + #endif
> + "
> + . auto/feature
> + if [ $ngx_found = yes ]; then
> + have=NGX_SSL_ASYNC . auto/have
> + OPENSSL_ASYNC=YES
> + fi
> + fi
> +
> + if [ -n "$OPENSSL_ASYNC" -a "$OPENSSL_ASYNC" != YES ]; then
> +cat << END
> +$1: error: For using asynchronous mode, The OpenSSL must be version 1.1.0 or greater.
> +
> +END
> + exit 1
> + fi
> fi

Environment variables is not something nginx configure is using to
enable or disable features.

Note well that these checks are not going to work when building
OpenSSL with nginx as per "./configure --with-openssl=...". Given
that it tests the SSL_MODE_ASYNC macro, a better idea
would be to use compile-time checks in the code instead.

> diff -r 2245324a507a -r 301a837387ed src/core/nginx.c
> --- a/src/core/nginx.c Thu Sep 02 12:25:37 2021 +0300
> +++ b/src/core/nginx.c Tue Sep 07 16:29:07 2021 +0800
> @@ -182,6 +182,12 @@
> static ngx_uint_t ngx_show_help;
> static ngx_uint_t ngx_show_version;
> static ngx_uint_t ngx_show_configure;
> +#if (NGX_SSL && NGX_SSL_ASYNC)
> +/* indicate that nginx start without ngx_ssl_init()
> + * which will involve OpenSSL configuration file to
> + * start OpenSSL engine */
> +static ngx_uint_t ngx_no_ssl_init;
> +#endif
> static u_char *ngx_prefix;
> static u_char *ngx_error_log;
> static u_char *ngx_conf_file;
> @@ -238,7 +244,13 @@
>
> /* STUB */
> #if (NGX_OPENSSL)
> - ngx_ssl_init(log);
> +#if (NGX_SSL && NGX_SSL_ASYNC)
> + if (!ngx_no_ssl_init) {
> +#endif
> + ngx_ssl_init(log);
> +#if (NGX_SSL && NGX_SSL_ASYNC)
> + }
> +#endif
> #endif
>
> /*
> @@ -248,6 +260,9 @@
>
> ngx_memzero(&init_cycle, sizeof(ngx_cycle_t));
> init_cycle.log = log;
> +#if (NGX_SSL && NGX_SSL_ASYNC)
> + init_cycle.no_ssl_init = ngx_no_ssl_init;
> +#endif
> ngx_cycle = &init_cycle;
>
> init_cycle.pool = ngx_create_pool(1024, log);
> @@ -782,11 +797,17 @@
>
> case 't':
> ngx_test_config = 1;
> +#if (NGX_SSL && NGX_SSL_ASYNC)
> + ngx_no_ssl_init = 1;
> +#endif
> break;

Note that this approach looks incorrect: testing nginx
configuration may require properly initialized OpenSSL library,
including various engines loaded, for example, to load default
settings and/or private keys from hardware tokens. While skipping
OpenSSL initialization might work for a proof-of-concept code such
as Intel's async work, this is not the approach usable for
generic-purpose server such as nginx.

[...]

--
Maxim Dounin
http://mdounin.ru/
_______________________________________________
nginx-devel mailing list
nginx-devel@nginx.org
http://mailman.nginx.org/mailman/listinfo/nginx-devel
Subject Author Views Posted

[PATCH] OpenSSL-1.1.0锛歋upport Asynchronous Operations of SSL with openSSL-1.1.0

Junli Liu 81 September 07, 2021 04:50AM

Re: [PATCH] OpenSSL-1.1.0锛歋upport Asynchronous Operations of SSL with openSSL-1.1.0

Maxim Dounin 25 September 08, 2021 12:22PM



Sorry, you do not have permission to post/reply in this forum.

Online Users

Guests: 60
Record Number of Users: 6 on February 13, 2018
Record Number of Guests: 421 on December 02, 2018
Powered by nginx      Powered by FreeBSD      PHP Powered      Powered by MariaDB      ipv6 ready