Skip to content

Uncheked result of method SSL_set_tlsext_status_type() #2395

Open
@Anchels

Description

@Anchels

Greetings! I've been investigating lua-nginx-module with Svace static analyzer and it found a curious method to look at.

ngx_http_lua_ffi_socket_tcp_sslhandshake(ngx_http_request_t *r,
ngx_http_lua_socket_tcp_upstream_t *u, ngx_ssl_session_t *sess,
int enable_session_reuse, ngx_str_t *server_name, int verify,
int ocsp_status_req, STACK_OF(X509) *chain, EVP_PKEY *pkey,
const char **errmsg)
{
ngx_int_t rc, i;
ngx_connection_t *c;
ngx_http_lua_ctx_t *ctx;
ngx_http_lua_co_ctx_t *coctx;
const char *busy_msg;
ngx_ssl_conn_t *ssl_conn;
X509 *x509;
ngx_log_debug0(NGX_LOG_DEBUG_HTTP, r->connection->log, 0,
"lua tcp socket ssl handshake");
if (u == NULL
|| u->peer.connection == NULL
|| u->read_closed
|| u->write_closed)
{
*errmsg = "closed";
return NGX_ERROR;
}
if (u->request != r) {
*errmsg = "bad request";
return NGX_ERROR;
}
busy_msg = ngx_http_lua_socket_tcp_check_busy(r, u, SOCKET_OP_CONNECT
| SOCKET_OP_READ
| SOCKET_OP_WRITE);
if (busy_msg != NULL) {
*errmsg = busy_msg;
return NGX_ERROR;
}
if (u->raw_downstream || u->body_downstream) {
*errmsg = "not supported for downstream sockets";
return NGX_ERROR;
}
c = u->peer.connection;
u->ssl_session_reuse = 1;
if (c->ssl && c->ssl->handshaked) {
if (sess != NULL) {
return NGX_DONE;
}
u->ssl_session_reuse = enable_session_reuse;
(void) ngx_http_lua_ssl_handshake_retval_handler(r, u, NULL);
return NGX_OK;
}
if (ngx_ssl_create_connection(u->conf->ssl, c,
NGX_SSL_BUFFER|NGX_SSL_CLIENT)
!= NGX_OK)
{
*errmsg = "failed to create ssl connection";
return NGX_ERROR;
}
ssl_conn = c->ssl->connection;
ctx = ngx_http_get_module_ctx(r, ngx_http_lua_module);
if (ctx == NULL) {
return NGX_HTTP_LUA_FFI_NO_REQ_CTX;
}
coctx = ctx->cur_co_ctx;
c->sendfile = 0;
if (sess != NULL) {
if (ngx_ssl_set_session(c, sess) != NGX_OK) {
*errmsg = "ssl set session failed";
return NGX_ERROR;
}
ngx_log_debug1(NGX_LOG_DEBUG_HTTP, c->log, 0,
"lua ssl set session: %p", sess);
} else {
u->ssl_session_reuse = enable_session_reuse;
}
if (chain != NULL) {
ngx_http_lua_assert(pkey != NULL); /* ensured by resty.core */
if (sk_X509_num(chain) < 1) {
ERR_clear_error();
*errmsg = "invalid client certificate chain";
return NGX_ERROR;
}
x509 = sk_X509_value(chain, 0);
if (x509 == NULL) {
ERR_clear_error();
*errmsg = "ssl fetch client certificate from chain failed";
return NGX_ERROR;
}
if (SSL_use_certificate(ssl_conn, x509) == 0) {
ERR_clear_error();
*errmsg = "ssl set client certificate failed";
return NGX_ERROR;
}
/* read rest of the chain */
for (i = 1; i < (ngx_int_t) sk_X509_num(chain); i++) {
x509 = sk_X509_value(chain, i);
if (x509 == NULL) {
ERR_clear_error();
*errmsg = "ssl fetch client intermediate certificate from "
"chain failed";
return NGX_ERROR;
}
if (SSL_add1_chain_cert(ssl_conn, x509) == 0) {
ERR_clear_error();
*errmsg = "ssl set client intermediate certificate failed";
return NGX_ERROR;
}
}
if (SSL_use_PrivateKey(ssl_conn, pkey) == 0) {
ERR_clear_error();
*errmsg = "ssl set client private key failed";
return NGX_ERROR;
}
}
if (server_name != NULL && server_name->data != NULL) {
ngx_log_debug1(NGX_LOG_DEBUG_HTTP, r->connection->log, 0,
"lua ssl server name: \"%V\"", server_name);
#ifdef SSL_CTRL_SET_TLSEXT_HOSTNAME
if (SSL_set_tlsext_host_name(c->ssl->connection,
(char *) server_name->data)
== 0)
{
*errmsg = "SSL_set_tlsext_host_name failed";
return NGX_ERROR;
}
#else
*errmsg = "no TLS extension support";
return NGX_ERROR;
#endif
}
u->ssl_verify = verify;
if (ocsp_status_req) {
#ifdef NGX_HTTP_LUA_USE_OCSP
SSL_set_tlsext_status_type(c->ssl->connection,
TLSEXT_STATUSTYPE_ocsp);
#else
*errmsg = "no OCSP support";
return NGX_ERROR;
#endif
}
if (server_name == NULL || server_name->len == 0) {
u->ssl_name.len = 0;
} else {
if (u->ssl_name.data) {
/* buffer already allocated */
if (u->ssl_name.len >= server_name->len) {
/* reuse it */
ngx_memcpy(u->ssl_name.data, server_name->data,
server_name->len);
u->ssl_name.len = server_name->len;
} else {
ngx_free(u->ssl_name.data);
goto new_ssl_name;
}
} else {
new_ssl_name:
u->ssl_name.data = ngx_alloc(server_name->len, ngx_cycle->log);
if (u->ssl_name.data == NULL) {
u->ssl_name.len = 0;
*errmsg = "no memory";
return NGX_ERROR;
}
ngx_memcpy(u->ssl_name.data, server_name->data, server_name->len);
u->ssl_name.len = server_name->len;
}
}
u->write_co_ctx = coctx;
#if 0
#ifdef NGX_HTTP_LUA_USE_OCSP
SSL_set_tlsext_status_type(c->ssl->connection, TLSEXT_STATUSTYPE_ocsp);
#endif
#endif
rc = ngx_ssl_handshake(c);
ngx_log_debug1(NGX_LOG_DEBUG_HTTP, r->connection->log, 0,
"ngx_ssl_handshake returned: %d", rc);
if (rc == NGX_AGAIN) {
if (c->write->timer_set) {
ngx_del_timer(c->write);
}
ngx_add_timer(c->read, u->connect_timeout);
u->conn_waiting = 1;
u->write_prepare_retvals = ngx_http_lua_ssl_handshake_retval_handler;
ngx_http_lua_cleanup_pending_operation(coctx);
coctx->cleanup = ngx_http_lua_coctx_cleanup;
coctx->data = u;
c->ssl->handler = ngx_http_lua_ssl_handshake_handler;
if (ctx->entered_content_phase) {
r->write_event_handler = ngx_http_lua_content_wev_handler;
} else {
r->write_event_handler = ngx_http_core_run_phases;
}
return NGX_AGAIN;
}
ngx_http_lua_ssl_handshake_handler(c);
if (rc == NGX_ERROR) {
*errmsg = u->error_ret;
return NGX_ERROR;
}
return NGX_OK;
}

Here the return value of method incovation SSL_set_tlsext_status_type() (which calls SSL_ctrl() under the hood) is not checked at the following cases:

SSL_set_tlsext_status_type(c->ssl->connection,
TLSEXT_STATUSTYPE_ocsp);

and

SSL_set_tlsext_status_type(c->ssl->connection, TLSEXT_STATUSTYPE_ocsp);

but usually it is checked for the function SSL_ctrl()


The Question:

After a long research and official OpenSSL docs read I'm still not sure if it's correct not to check the returning value in the cases above.

What do you think about this?


Found by Linux Verification Center (linuxtesting.org) with SVACE.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions