Hello!

On Wed, Jul 30, 2014 at 07:29:10PM +0400, Dmitrii Pichulin wrote:

> # HG changeset patch
> # User Dmitrii Pichulin
> # Date 1406733892 -14400
> # Wed Jul 30 19:24:52 2014 +0400
> # Node ID a4c89ae85f45153760637058a75f4338b3974219
> # Parent 4d092aa2f4637ce50284d2accd99a8e91aae2b4c
> allow to use engine keyform for server private key
>
> diff -r 4d092aa2f463 -r a4c89ae85f45 src/event/ngx_event_openssl.c
> --- a/src/event/ngx_event_openssl.c Mon Jul 28 12:27:57 2014 -0700
> +++ b/src/event/ngx_event_openssl.c Wed Jul 30 19:24:52 2014 +0400
> @@ -17,6 +17,11 @@
> ngx_uint_t engine; /* unsigned engine:1; */
> } ngx_openssl_conf_t;
>
> +typedef struct {
> + const void *password;
> + const char *prompt_info;
> +} PW_CB_DATA;
> +
>
> static int ngx_ssl_password_callback(char *buf, int size, int rwflag,
> void *userdata);
> @@ -265,11 +270,16 @@
> ngx_ssl_certificate(ngx_conf_t *cf, ngx_ssl_t *ssl, ngx_str_t *cert,
> ngx_str_t *key, ngx_array_t *passwords)
> {
> + char *p, *last;
> BIO *bio;
> X509 *x509;
> + ENGINE *engine;
> + EVP_PKEY *private_key;
> + PW_CB_DATA pwd_data;
> u_long n;
> ngx_str_t *pwd;
> ngx_uint_t tries;
> + u_char pwd_buf[NGX_SSL_PASSWORD_BUFFER_SIZE];
>
> if (ngx_conf_full_name(cf->cycle, cert, 1) != NGX_OK) {
> return NGX_ERROR;
> @@ -352,6 +362,75 @@
>
> BIO_free(bio);
>
> + if (ngx_strncmp(key->data, "engine:", sizeof("engine:") - 1) == 0) {
> +
> + p = (char *) key->data + sizeof("engine:") - 1;
> + last = ngx_strchr(p, ':');
> +
> + if (last == NULL) {
> + ngx_conf_log_error(NGX_LOG_EMERG, cf, 0, "invalid syntax: %V", key);
> + return NGX_ERROR;
> + }
> +
> + p[last - p] = '\0';
> +
> + engine = ENGINE_by_id(p);

After Piotr's patch (http://hg.nginx.org/nginx/rev/4d092aa2f463)
we are able to work with OpenSSL compiled with OPENSSL_NO_ENGINE.
Breaking this wouldn't be a good idea.

> +
> + if (engine == NULL) {
> + ngx_ssl_error(NGX_LOG_EMERG, ssl->log, 0,
> + "ENGINE_by_id(\"%s\") failed", p);
> + return NGX_ERROR;
> + }
> +
> + p[last - p] = ':';
> +
> + if (passwords) {
> + pwd = passwords->elts;
> +
> + ngx_cpystrn(pwd_buf, pwd->data, pwd->len + 1);
> +
> + pwd_data.password = pwd_buf;
> + } else {
> + pwd_data.password = NULL;
> + }
> + pwd_data.prompt_info = NULL;
> +
> + last++;
> +
> + private_key = ENGINE_load_private_key(engine, last, 0,
> + (void *) &pwd_data);

I don't see how it's expected to work. You only pass private data
for UI callbacks, but not callbacks itself.

Anyway, proper implementation of passing key passwords into an engine
seems to be rather big, and as per my reading of the code under
crypto/engine won't work with most of the engines anyway. It
might be better idea to don't try to do this for now.

> +
> + ngx_memzero(pwd_buf, NGX_SSL_PASSWORD_BUFFER_SIZE);
> +
> + if (private_key == NULL) {
> + ngx_ssl_error(NGX_LOG_EMERG, ssl->log, 0,
> + "ENGINE_load_private_key(\"%s\") failed", last);
> +
> + if (ENGINE_free(engine) == 0) {
> + ngx_ssl_error(NGX_LOG_EMERG, ssl->log, 0,
> + "ENGINE_free() failed");
> + }
> + return NGX_ERROR;
> + }

The above referenced commit shows that we don't check
ENGINE_free() return codes, so probably we shouldn't try this here as
well.

> +
> + if (ENGINE_free(engine) == 0) {
> + ngx_ssl_error(NGX_LOG_EMERG, ssl->log, 0, "ENGINE_free() failed");
> + EVP_PKEY_free(private_key);
> + return NGX_ERROR;
> + }
> +
> + if (SSL_CTX_use_PrivateKey(ssl->ctx, private_key) == 0) {
> + ngx_ssl_error(NGX_LOG_EMERG, ssl->log, 0,
> + "SSL_CTX_use_PrivateKey(\"%s\") failed", last);
> + EVP_PKEY_free(private_key);
> + return NGX_ERROR;
> + }
> +
> + EVP_PKEY_free(private_key);
> +
> + return NGX_OK;
> + }
> +
> if (ngx_conf_full_name(cf->cycle, key, 1) != NGX_OK) {
> return NGX_ERROR;
> }
>
> _______________________________________________
> nginx-devel mailing list
> nginx-devel@nginx.org
> http://mailman.nginx.org/mailman/listinfo/nginx-devel

--
Maxim Dounin
http://nginx.org/

_______________________________________________
nginx-devel mailing list
nginx-devel@nginx.org
http://mailman.nginx.org/mailman/listinfo/nginx-devel