Skip to content
This repository is currently being migrated. It's locked while the migration is in progress.

LUA-656 added support for ssl (e.g., HTTPS) #2

Merged
merged 1 commit into from
May 9, 2016
Merged

LUA-656 added support for ssl (e.g., HTTPS) #2

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
109 changes: 105 additions & 4 deletions ngx_http_upstream_check_module.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,7 @@ struct ngx_http_upstream_check_peer_s {
ngx_event_t check_ev;
ngx_event_t check_timeout_ev;
ngx_peer_connection_t pc;
ngx_ssl_t ssl;

void *check_data;
ngx_event_handler_pt send_handler;
Expand Down Expand Up @@ -241,6 +242,7 @@ struct ngx_http_upstream_check_srv_conf_s {
ngx_array_t *fastcgi_params;

ngx_uint_t default_down;
ngx_uint_t ssl;
Copy link

Choose a reason for hiding this comment

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

This looks like a boolean flag, so perhaps we can use ngx_uint_t ssl:1; here?

Copy link
Author

Choose a reason for hiding this comment

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

Yes, it's a boolean. Unfortunately, Nginx doesn't define (or use the standard) boolean type. I think using bit fields would make sense for a collection of flags, not for a single one.

Copy link

Choose a reason for hiding this comment

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

True, but if there was another boolean flag to be added - the we wouldn't have to modify this one :)
Also, default_down looks like a boolean flag too.
But anyway, this is a very minor thing and can be skipped :)

Copy link
Author

Choose a reason for hiding this comment

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

Indeed for having both default_down and ssl, but I would skip this change to avoid changing unnecessary things. IMHO, it makes it hard to sync with upstream.

};


Expand Down Expand Up @@ -341,6 +343,15 @@ static ngx_int_t ngx_http_upstream_check_peek_one_byte(ngx_connection_t *c);

static void ngx_http_upstream_check_begin_handler(ngx_event_t *event);
static void ngx_http_upstream_check_connect_handler(ngx_event_t *event);
static void ngx_http_upstream_check_connect_done(
ngx_http_upstream_check_peer_t *peer,
ngx_http_upstream_check_srv_conf_t *ucscf, ngx_connection_t *c,
ngx_int_t ok);

static void ngx_http_upstream_check_ssl_init(
ngx_http_upstream_check_peer_t *peer,
ngx_http_upstream_check_srv_conf_t *ucscf, ngx_connection_t *c);
static void ngx_http_upstream_check_ssl_handshake(ngx_connection_t *c);

static void ngx_http_upstream_check_peek_handler(ngx_event_t *event);

Expand Down Expand Up @@ -1118,7 +1129,8 @@ ngx_http_upstream_check_connect_handler(ngx_event_t *event)
if (peer->pc.connection != NULL) {
c = peer->pc.connection;
if ((rc = ngx_http_upstream_check_peek_one_byte(c)) == NGX_OK) {
goto upstream_check_connect_done;
ngx_http_upstream_check_connect_done(peer, ucscf, c, 1);
return;
} else {
ngx_close_connection(c);
peer->pc.connection = NULL;
Expand Down Expand Up @@ -1153,7 +1165,19 @@ ngx_http_upstream_check_connect_handler(ngx_event_t *event)
c->write->log = c->log;
c->pool = peer->pool;

upstream_check_connect_done:
if (ucscf->ssl) {
ngx_http_upstream_check_ssl_init(peer, ucscf, c);
return;
}

ngx_http_upstream_check_connect_done(peer, ucscf, c, rc == NGX_OK);
Copy link

Choose a reason for hiding this comment

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

+100 karma points for this (removal of goto) :)

Copy link
Author

Choose a reason for hiding this comment

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

I've added another one, so no karma points for me ;-)

Copy link

Choose a reason for hiding this comment

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

o.0 yes, i see that now :) why was that goto necessary? :)

Copy link
Author

@lneto lneto May 4, 2016 •
edited
Loading

Choose a reason for hiding this comment

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

It's never necessary, but, IMHO, it's better to handle errors in that way. (See http://blog.regehr.org/archives/894)

}

static void
ngx_http_upstream_check_connect_done(ngx_http_upstream_check_peer_t *peer,
ngx_http_upstream_check_srv_conf_t *ucscf, ngx_connection_t *c,
ngx_int_t ok)
{
peer->state = NGX_HTTP_CHECK_CONNECT_DONE;

c->write->handler = peer->send_handler;
Expand All @@ -1162,11 +1186,67 @@ ngx_http_upstream_check_connect_handler(ngx_event_t *event)
ngx_add_timer(&peer->check_timeout_ev, ucscf->check_timeout);

/* The kqueue's loop interface needs it. */
if (rc == NGX_OK) {
if (ok) {
c->write->handler(c->write);
}
}

static void
ngx_http_upstream_check_ssl_init(ngx_http_upstream_check_peer_t *peer,
ngx_http_upstream_check_srv_conf_t *ucscf, ngx_connection_t *c)
{
ngx_int_t rc;
ngx_uint_t protocols, flags;

ngx_memzero(&peer->ssl, sizeof(ngx_ssl_t));

protocols = NGX_SSL_TLSv1|NGX_SSL_SSLv2|NGX_SSL_SSLv3;
flags = NGX_SSL_BUFFER|NGX_SSL_CLIENT;
if (ngx_ssl_create(&peer->ssl, protocols, NULL) != NGX_OK ||
ngx_ssl_create_connection(&peer->ssl, c, flags) != NGX_OK) {
goto failed;
}

rc = ngx_ssl_handshake(c);
if (rc == NGX_ERROR) {
goto failed;
}

if (rc == NGX_AGAIN) {
ngx_add_timer(c->read, ucscf->check_timeout);
c->ssl->handler = ngx_http_upstream_check_ssl_handshake;
return;
}

ngx_http_upstream_check_ssl_handshake(c);
return;

failed:
ngx_http_upstream_check_status_update(peer, 0);
ngx_http_upstream_check_clean_event(peer);
}

static void
ngx_http_upstream_check_ssl_handshake(ngx_connection_t *c)
{
ngx_http_upstream_check_peer_t *peer;
ngx_http_upstream_check_srv_conf_t *ucscf;

peer = (ngx_http_upstream_check_peer_t *) c->data;
ucscf = peer->conf;

if (c->read->timer_set)
ngx_del_timer(c->read);

if (!c->ssl->handshaked) {
ngx_http_upstream_check_status_update(peer, 0);
ngx_http_upstream_check_clean_event(peer);
return;
}

ngx_http_upstream_check_connect_done(peer, ucscf, c, 1);
}

static ngx_int_t
ngx_http_upstream_check_peek_one_byte(ngx_connection_t *c)
{
Expand Down Expand Up @@ -3079,7 +3159,7 @@ static char *
ngx_http_upstream_check(ngx_conf_t *cf, ngx_command_t *cmd, void *conf)
{
ngx_str_t *value, s;
ngx_uint_t i, port, rise, fall, default_down;
ngx_uint_t i, port, rise, fall, default_down, ssl;
ngx_msec_t interval, timeout;
ngx_http_upstream_check_srv_conf_t *ucscf;

Expand All @@ -3090,6 +3170,7 @@ ngx_http_upstream_check(ngx_conf_t *cf, ngx_command_t *cmd, void *conf)
interval = 30000;
timeout = 1000;
default_down = 1;
ssl = 0;

value = cf->args->elts;

Expand Down Expand Up @@ -3193,6 +3274,25 @@ ngx_http_upstream_check(ngx_conf_t *cf, ngx_command_t *cmd, void *conf)
continue;
}

if (ngx_strncmp(value[i].data, "ssl=", 4) == 0) {
s.len = value[i].len - 4;
s.data = value[i].data + 4;

if (ngx_strcasecmp(s.data, (u_char *) "true") == 0) {
ssl = 1;
} else if (ngx_strcasecmp(s.data, (u_char *) "false") == 0) {
ssl = 0;
} else {
ngx_conf_log_error(NGX_LOG_EMERG, cf, 0,
"invalid value \"%s\", "
"it must be \"true\" or \"false\"",
value[i].data);
return NGX_CONF_ERROR;
}

continue;
}

goto invalid_check_parameter;
}

Expand All @@ -3202,6 +3302,7 @@ ngx_http_upstream_check(ngx_conf_t *cf, ngx_command_t *cmd, void *conf)
ucscf->fall_count = fall;
ucscf->rise_count = rise;
ucscf->default_down = default_down;
ucscf->ssl = ssl;

if (ucscf->check_type_conf == NGX_CONF_UNSET_PTR) {
ngx_str_set(&s, "tcp");
Expand Down