Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feature: support ssl.create_ctx and tcp:setsslctx #997

Open
wants to merge 29 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 18 commits
Commits
Show all changes
29 commits
Select commit Hold shift + click to select a range
e859096
doc: log level constatns add nginx phase
detailyang Nov 2, 2016
91a6d7d
Merge remote-tracking branch 'openresty/master'
detailyang Jan 13, 2017
635a569
Merge remote-tracking branch 'openresty/master'
detailyang Feb 2, 2017
1c10853
Merge remote-tracking branch 'openresty/master'
detailyang Feb 4, 2017
2ec655d
feature: ssl.create_ctx and tcp:setsslctx
detailyang Feb 19, 2017
df4387b
tests: remove unused openssl version judge
detailyang Feb 21, 2017
7086ff1
refactor: use protocols as the arg to create_ctx
detailyang Feb 26, 2017
ca8c3cb
refactor: remove unsed C macro
detailyang Feb 27, 2017
6ce2ea5
tests: remove duplicated tests
detailyang Feb 27, 2017
1b2eaac
refactor: remove superfluous variable
detailyang Feb 28, 2017
697f0eb
style: combine arguments to one line
detailyang Feb 28, 2017
0409076
refactor: expose tcp object metatable to REGISTRY
detailyang Feb 28, 2017
17c3141
refactor: caller should allocate error message buf
detailyang Mar 8, 2017
8b7e0f5
style: do not exceed 80 columns in source code
detailyang Mar 8, 2017
a30bbf3
tests: use lua-resty-core to test FFI
detailyang Mar 8, 2017
73c5aa8
refactor: copy literal string to caller err buffer
detailyang Mar 11, 2017
0b04b54
travis: use personal lua-resty-core to pass test
detailyang Mar 11, 2017
902ec8d
refactor: use ngx_min to decide the size of msg
detailyang Mar 11, 2017
c94df6d
style: align function argments
detailyang May 5, 2017
6e38b47
style: align function arguemnts (ditto)
detailyang May 5, 2017
8e06dc5
tests: remove unused module
detailyang May 7, 2017
c105935
style: align for aesthetic considerations
detailyang May 7, 2017
69f841f
refactor: replace ngx_copy to ngx_memcpy
detailyang May 7, 2017
3bcc9f7
Merge remote-tracking branch 'openresty/master' into lua-ffi-api-sslctx
detailyang May 7, 2017
f52e7a2
style: variable name tweaks
detailyang May 17, 2017
b57b9db
Merge remote-tracking branch 'openresty/master' into lua-ffi-api-sslctx
detailyang May 21, 2017
f13fd50
Merge remote-tracking branch 'openresty/master' into lua-ffi-api-sslctx
detailyang May 25, 2017
fb52c37
style: variable declaration in #if block
detailyang May 26, 2017
1f1b090
feature: support ciphers, CRL, ca, cert_store
detailyang Jun 1, 2017
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .travis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ install:
- git clone https://github.com/openresty/rds-json-nginx-module.git ../rds-json-nginx-module
- git clone https://github.com/openresty/srcache-nginx-module.git ../srcache-nginx-module
- git clone https://github.com/openresty/redis2-nginx-module.git ../redis2-nginx-module
- git clone https://github.com/openresty/lua-resty-core.git ../lua-resty-core
- git clone -b lua-ffi-api-sslctx https://github.com/detailyang/lua-resty-core.git ../lua-resty-core
- git clone -b v2.1-agentzh https://github.com/openresty/luajit2.git

before_script:
Expand Down
57 changes: 56 additions & 1 deletion src/ngx_http_lua_socket_tcp.c
Original file line number Diff line number Diff line change
Expand Up @@ -183,6 +183,12 @@ enum {
}


#if (NGX_HTTP_SSL)

#define ngx_http_lua_ngx_socket_tcp_mt_key "__ngx_socket_tcp_mt"

#endif

static char ngx_http_lua_req_socket_metatable_key;
static char ngx_http_lua_raw_req_socket_metatable_key;
static char ngx_http_lua_tcp_socket_metatable_key;
Expand Down Expand Up @@ -316,6 +322,20 @@ ngx_http_lua_inject_socket_tcp_api(ngx_log_t *log, lua_State *L)

lua_pushvalue(L, -1);
lua_setfield(L, -2, "__index");

#if (NGX_HTTP_SSL)

#ifndef NGX_LUA_NO_FFI_API

/* expose tcp object metatable to REGISTRY for FFI */
lua_pushliteral(L, ngx_http_lua_ngx_socket_tcp_mt_key);
lua_pushvalue(L, -2);
lua_rawset(L, LUA_REGISTRYINDEX);

#endif /* NGX_LUA_NO_FFI_API */

#endif /* NGX_HTTP_SSL */

lua_rawset(L, LUA_REGISTRYINDEX);
/* }}} */

Expand Down Expand Up @@ -587,6 +607,12 @@ ngx_http_lua_socket_tcp_connect(lua_State *L)

u->conf = llcf;

#if (NGX_HTTP_SSL)

u->ssl = llcf->ssl;

#endif

pc = &u->peer;

pc->log = r->connection->log;
Expand Down Expand Up @@ -1200,6 +1226,35 @@ ngx_http_lua_socket_conn_error_retval_handler(ngx_http_request_t *r,

#if (NGX_HTTP_SSL)


#ifndef NGX_LUA_NO_FFI_API

int
ngx_http_lua_ffi_socket_tcp_setsslctx(ngx_http_request_t *r,
ngx_http_lua_socket_tcp_upstream_t *u, void *cdata_ctx, char **err)
{
SSL_CTX *ssl_ctx = cdata_ctx;

ngx_ssl_t *ssl;

ssl = ngx_pcalloc(r->pool, sizeof(ngx_ssl_t));
if (ssl == NULL) {
*err = "no memory";
return NGX_ERROR;
}

ssl->ctx = ssl_ctx;

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should increment ref count of SSL_CTX*?

ssl->log = u->ssl->log;
ssl->buffer_size = u->ssl->buffer_size;

u->ssl = ssl;

return NGX_OK;
}

#endif /* NGX_LUA_NO_FFI_API */


static int
ngx_http_lua_socket_tcp_sslhandshake(lua_State *L)
{
Expand Down Expand Up @@ -1286,7 +1341,7 @@ ngx_http_lua_socket_tcp_sslhandshake(lua_State *L)
return 1;
}

if (ngx_ssl_create_connection(u->conf->ssl, c,
if (ngx_ssl_create_connection(u->ssl, c,
NGX_SSL_BUFFER|NGX_SSL_CLIENT)
!= NGX_OK)
{
Expand Down
1 change: 1 addition & 0 deletions src/ngx_http_lua_socket_tcp.h
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,7 @@ struct ngx_http_lua_socket_tcp_upstream_s {

#if (NGX_HTTP_SSL)
ngx_str_t ssl_name;
ngx_ssl_t *ssl;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Style: alignment issue. The ssl identifier should be aligned, excluding the * prefix.

#endif

unsigned ft_type:16;
Expand Down
160 changes: 160 additions & 0 deletions src/ngx_http_lua_ssl.c
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,10 @@
#if (NGX_HTTP_SSL)


static size_t ngx_http_lua_ssl_get_error(u_long e, u_char *ssl_err_buf,
size_t ssl_err_buf_len, u_char *default_errmsg, size_t default_errmsg_len);


int ngx_http_lua_ssl_ctx_index = -1;


Expand All @@ -34,4 +38,160 @@ ngx_http_lua_ssl_init(ngx_log_t *log)
}


static size_t
ngx_http_lua_ssl_get_error(u_long e, u_char *ssl_err_buf,
size_t ssl_err_buf_len, u_char *default_errmsg,
size_t default_errmsg_len)
{
size_t len;

if (e == 0) {
len = ngx_min(ssl_err_buf_len, default_errmsg_len);
ssl_err_buf = ngx_copy(ssl_err_buf, default_errmsg, len);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The return value is useless. Better use ngx_memcpy here instead of ngx_copy.

return len;
}

ERR_error_string_n(e, (char *) ssl_err_buf, ssl_err_buf_len);

return ngx_strlen(ssl_err_buf);
}


#ifndef NGX_LUA_NO_FFI_API


void *
ngx_http_lua_ffi_ssl_ctx_init(ngx_uint_t protocols, char **err)
{
ngx_ssl_t ssl;

ssl.log = ngx_cycle->log;
if (ngx_ssl_create(&ssl, protocols, NULL) != NGX_OK) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think if we should inherit existing configurations from ngx_http_lua_loc_conf_t's ssl field? Please see ngx_http_lua_set_ssl for more details. And here in the create_ctx API, we can simply allow the user to override the default settings. This would be better.

Copy link
Contributor Author

@detailyang detailyang Jun 1, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well. inheriting existing configurations from ngx_http_lua_loc_conf_ts ssl field will required this API need ngx_http_request_t. we should allow it run any phase. Especially, it can be allowed to run in init_by_lua_* to easily cache it. May we expose these FFI API (like ciphers, crl, ca and so on) to Lua Land . It will be more flexible and effective to feed arguments to SSL_CTX object.

Copy link
Member

@agentzh agentzh Jun 1, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@detailyang We'll add cosocket support to init_by_lua* soon. By then it will also have a (fake) request object, just like the timer handler. So I don't think this is a showstopper. Any other things I'm overlooking here?

*err = "failed to create ssl ctx";
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, I think it would be better to return OpenSSL's own error messages if there's any. Otherwise fixed error messages made up in the caller does not give much helpful info.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, this is not a priority though.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here we reuse the ngx_ssl_create API. However we cannot get more OpenSSL's error info from this API. Surely, we can rewrite this API to get more detail if we want :)

return NULL;
}

ngx_log_debug2(NGX_LOG_DEBUG_HTTP, ssl.log, 0,
"lua ssl ctx init: %p:%d", ssl.ctx, ssl.ctx->references);

return ssl.ctx;
}


int
ngx_http_lua_ffi_ssl_ctx_set_cert(void *cdata_ctx, void *cdata_cert,
u_char *err_buf, size_t *err_buf_len)
{
char *err;

#ifdef LIBRESSL_VERSION_NUMBER

err = "LibreSSL not supported";
goto failed;

#else

# if OPENSSL_VERSION_NUMBER < 0x1000205fL

err = "at least OpenSSL 1.0.2e required but found "
OPENSSL_VERSION_TEXT;
goto failed;

# else

X509 *x509 = NULL;
SSL_CTX *ssl_ctx = cdata_ctx;
STACK_OF(X509) *cert = cdata_cert;

#ifdef OPENSSL_IS_BORINGSSL
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here it should use 2 levels of indentation after #.

size_t i;
#else
int i;
#endif

if (sk_X509_num(cert) < 1) {
err = "sk_X509_num() failed";
goto failed;
}

x509 = sk_X509_value(cert, 0);
if (x509 == NULL) {
err = "sk_X509_value() failed";
goto failed;
}

if (SSL_CTX_use_certificate(ssl_ctx, x509) == 0) {
err = "SSL_CTX_use_certificate() failed";
goto failed;
}

/* read rest of the chain */

for (i = 1; i < sk_X509_num(cert); i++) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why call sk_X059_num for more than once?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we can simply save the result of the first sk_X509_num above to a local C variable.


x509 = sk_X509_value(cert, i);
if (x509 == NULL) {
err = "sk_X509_value() failed";
goto failed;
}

if (SSL_CTX_add1_chain_cert(ssl_ctx, x509) == 0) {
err = "SSL_add1_chain_cert() failed";
goto failed;
}
}

return NGX_OK;
# endif /* OPENSSL_VERSION_NUMBER < 0x1000205fL */
#endif

failed:

*err_buf_len = ngx_http_lua_ssl_get_error(ERR_get_error(), err_buf,
*err_buf_len, (u_char *) err,
Copy link
Member

@rainingmaster rainingmaster May 5, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For uniform format, should we align in front of the brackets? also in follow ngx_http_lua_ssl_get_error

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch

ngx_strlen(err));
return NGX_ERROR;
}


int
ngx_http_lua_ffi_ssl_ctx_set_priv_key(void *cdata_ctx, void *cdata_key,
u_char *err_buf, size_t *err_buf_len)
{
SSL_CTX *ssl_ctx = cdata_ctx;
EVP_PKEY *key = cdata_key;

char *err;

if (SSL_CTX_use_PrivateKey(ssl_ctx, key) == 0) {
err = "SSL_CTX_use_PrivateKey() failed";
goto failed;
}

return NGX_OK;

failed:

*err_buf_len = ngx_http_lua_ssl_get_error(ERR_get_error(), err_buf,
*err_buf_len, (u_char *)err,
ngx_strlen(err));
return NGX_ERROR;
}


void
ngx_http_lua_ffi_ssl_ctx_free(void *cdata)
{
SSL_CTX *ssl_ctx = cdata;

ngx_log_debug2(NGX_LOG_DEBUG_HTTP, ngx_cycle->log,
0, "lua ssl ctx free: %p:%d", ssl_ctx, ssl_ctx->references);

SSL_CTX_free(ssl_ctx);
}


#endif /* NGX_LUA_NO_FFI_API */


#endif /* NGX_HTTP_SSL */
40 changes: 40 additions & 0 deletions t/151-ssl-ctx.t
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
# vim:set ft= ts=4 sw=4 et fdm=marker:

use Test::Nginx::Socket::Lua;
use Digest::MD5 qw(md5_hex);

repeat_each(3);

plan tests => repeat_each() * (blocks());

$ENV{TEST_NGINX_HTML_DIR} ||= html_dir();

log_level 'debug';

no_long_string();

run_tests();

__DATA__

=== TEST 1: ssl ctx init and free
--- log_level: debug
--- http_config
lua_package_path "../lua-resty-core/lib/?.lua;;";
--- config
location /t {
content_by_lua_block {
local ssl = require "ngx.ssl"
local ssl_ctx, err = ssl.create_ctx({})
ssl_ctx = nil
collectgarbage("collect")
}
}
--- request
GET /t
--- ignore_response
--- grep_error_log eval: qr/lua ssl ctx (?:init|free): [0-9A-F]+:\d+/
--- grep_error_log_out eval
qr/^lua ssl ctx init: ([0-9A-F]+):1
lua ssl ctx free: ([0-9A-F]+):1
$/