[PATCH] allow to use engine keyform for server private key

[Maxim Dounin]

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 at nginx.org
> http://mailman.nginx.org/mailman/listinfo/nginx-devel

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