diff options
| -rw-r--r-- | UPGRADE.txt | 7 | ||||
| -rw-r--r-- | channels/chan_sip.c | 4 | ||||
| -rw-r--r-- | configs/http.conf.sample | 6 | ||||
| -rw-r--r-- | include/asterisk/tcptls.h | 47 | ||||
| -rw-r--r-- | include/asterisk/utils.h | 1 | ||||
| -rw-r--r-- | main/http.c | 34 | ||||
| -rw-r--r-- | main/manager.c | 73 | ||||
| -rw-r--r-- | main/tcptls.c | 536 | ||||
| -rw-r--r-- | main/utils.c | 19 | ||||
| -rw-r--r-- | res/res_http_websocket.c | 7 |
10 files changed, 605 insertions, 129 deletions
diff --git a/UPGRADE.txt b/UPGRADE.txt index b431693d0..9d3f46222 100644 --- a/UPGRADE.txt +++ b/UPGRADE.txt @@ -107,6 +107,9 @@ AMI: * MixMonitorMute - call or system * StopMixMonitor - call or system + - Removed the undocumented manager.conf block-sockets option. It interferes with + TCP/TLS inactivity timeouts. + CDRs: - The "endbeforehexten" setting now defaults to "yes", instead of "no". When set to "no", yhis setting will cause a new CDR to be generated when a @@ -148,6 +151,10 @@ Configuration Files: - The unistim.conf 'dateformat' has changed meaning of options values to conform values used inside Unistim protocol +HTTP: + - Added http.conf session_inactivity timer option to close HTTP connections + that aren't doing anything. + ODBC: - The compatibility setting, allow_empty_string_in_nontext, has been removed. Empty column values will be stored as empty strings during realtime updates. diff --git a/channels/chan_sip.c b/channels/chan_sip.c index 3d9d80b20..f62b242ef 100644 --- a/channels/chan_sip.c +++ b/channels/chan_sip.c @@ -3183,11 +3183,15 @@ static void *_sip_tcp_helper_thread(struct ast_tcptls_session_instance *tcptls_s goto cleanup; } + ast_tcptls_stream_set_timeout_sequence(tcptls_session->stream_cookie, ast_tvnow(), + tcptls_session->client ? -1 : (authtimeout * 1000)); + for (;;) { struct ast_str *str_save; if (!tcptls_session->client && req.authenticated && !authenticated) { authenticated = 1; + ast_tcptls_stream_set_timeout_disable(tcptls_session->stream_cookie); ast_atomic_fetchadd_int(&unauth_sessions, -1); } diff --git a/configs/http.conf.sample b/configs/http.conf.sample index 5b9c9a76f..98c672b2a 100644 --- a/configs/http.conf.sample +++ b/configs/http.conf.sample @@ -39,6 +39,12 @@ bindaddr=127.0.0.1 ; ;sessionlimit=100 ; +; session_inactivity specifies the number of milliseconds to wait for +; more data over the HTTP connection before closing it. +; +; Default: 30000 +;session_inactivity=30000 +; ; Whether Asterisk should serve static content from http-static ; Default is no. ; diff --git a/include/asterisk/tcptls.h b/include/asterisk/tcptls.h index 6364158de..17b532cda 100644 --- a/include/asterisk/tcptls.h +++ b/include/asterisk/tcptls.h @@ -144,6 +144,51 @@ struct ast_tcptls_session_args { const char *name; }; +struct ast_tcptls_stream; + +/*! + * \brief Disable the TCP/TLS stream timeout timer. + * + * \param stream TCP/TLS stream control data. + * + * \return Nothing + */ +void ast_tcptls_stream_set_timeout_disable(struct ast_tcptls_stream *stream); + +/*! + * \brief Set the TCP/TLS stream inactivity timeout timer. + * + * \param stream TCP/TLS stream control data. + * \param timeout Number of milliseconds to wait for data transfer with the peer. + * + * \details This is basically how much time we are willing to spend + * in an I/O call before we declare the peer unresponsive. + * + * \note Setting timeout to -1 disables the timeout. + * \note Setting this timeout replaces the I/O sequence timeout timer. + * + * \return Nothing + */ +void ast_tcptls_stream_set_timeout_inactivity(struct ast_tcptls_stream *stream, int timeout); + +/*! + * \brief Set the TCP/TLS stream I/O sequence timeout timer. + * + * \param stream TCP/TLS stream control data. + * \param start Time the I/O sequence timer starts. + * \param timeout Number of milliseconds from the start time before timeout. + * + * \details This is how much time are we willing to allow the peer + * to complete an operation that can take several I/O calls. The + * main use is as an authentication timer with us. + * + * \note Setting timeout to -1 disables the timeout. + * \note Setting this timeout replaces the inactivity timeout timer. + * + * \return Nothing + */ +void ast_tcptls_stream_set_timeout_sequence(struct ast_tcptls_stream *stream, struct timeval start, int timeout); + /*! \brief * describes a server instance */ @@ -161,6 +206,8 @@ struct ast_tcptls_session_instance { * extra data. */ struct ast_str *overflow_buf; + /*! ao2 FILE stream cookie object associated with f. */ + struct ast_tcptls_stream *stream_cookie; }; #if defined(HAVE_FUNOPEN) diff --git a/include/asterisk/utils.h b/include/asterisk/utils.h index e441ba055..47a438eb3 100644 --- a/include/asterisk/utils.h +++ b/include/asterisk/utils.h @@ -369,6 +369,7 @@ static force_inline void ast_slinear_saturated_divide(short *input, short *value int ast_utils_init(void); int ast_wait_for_input(int fd, int ms); +int ast_wait_for_output(int fd, int ms); /*! * \brief Try to write string, but wait no more than ms milliseconds diff --git a/main/http.c b/main/http.c index 0c9395d9c..19b0199c8 100644 --- a/main/http.c +++ b/main/http.c @@ -71,8 +71,10 @@ ASTERISK_FILE_VERSION(__FILE__, "$Revision$") #define DEFAULT_PORT 8088 #define DEFAULT_TLS_PORT 8089 #define DEFAULT_SESSION_LIMIT 100 +#define DEFAULT_SESSION_INACTIVITY 30000 /* (ms) Idle time waiting for data. */ static int session_limit = DEFAULT_SESSION_LIMIT; +static int session_inactivity = DEFAULT_SESSION_INACTIVITY; static int session_count = 0; static struct ast_tls_config http_tls_cfg; @@ -1297,6 +1299,7 @@ static void *httpd_helper_thread(void *data) enum ast_http_method http_method = AST_HTTP_UNKNOWN; const char *transfer_encoding; int remaining_headers; + int flags; struct protoent *p; if (ast_atomic_fetchadd_int(&session_count, +1) >= session_limit) { @@ -1318,7 +1321,14 @@ static void *httpd_helper_thread(void *data) ast_log(LOG_WARNING, "Some HTTP requests may be slow to respond.\n"); } - if (!fgets(buf, sizeof(buf), ser->f)) { + /* make sure socket is non-blocking */ + flags = fcntl(ser->fd, F_GETFL); + flags |= O_NONBLOCK; + fcntl(ser->fd, F_SETFL, flags); + + ast_tcptls_stream_set_timeout_inactivity(ser->stream_cookie, session_inactivity); + + if (!fgets(buf, sizeof(buf), ser->f) || feof(ser->f)) { goto done; } @@ -1358,12 +1368,19 @@ static void *httpd_helper_thread(void *data) /* process "Request Headers" lines */ remaining_headers = MAX_HTTP_REQUEST_HEADERS; - while (fgets(header_line, sizeof(header_line), ser->f)) { - char *name, *value; + for (;;) { + char *name; + char *value; + + if (!fgets(header_line, sizeof(header_line), ser->f) || feof(ser->f)) { + ast_http_error(ser, 400, "Bad Request", "Timeout"); + goto done; + } /* Trim trailing characters */ ast_trim_blanks(header_line); if (ast_strlen_zero(header_line)) { + /* A blank line ends the request header section. */ break; } @@ -1431,7 +1448,7 @@ done: ast_variables_destroy(headers); if (ser->f) { - fclose(ser->f); + ast_tcptls_close_session_file(ser); } ao2_ref(ser, -1); ser = NULL; @@ -1541,6 +1558,9 @@ static int __ast_http_load(int reload) ast_sockaddr_setnull(&https_desc.local_address); + session_limit = DEFAULT_SESSION_LIMIT; + session_inactivity = DEFAULT_SESSION_INACTIVITY; + if (cfg) { v = ast_variable_browse(cfg, "general"); for (; v; v = v->next) { @@ -1586,6 +1606,12 @@ static int __ast_http_load(int reload) ast_log(LOG_WARNING, "Invalid %s '%s' at line %d of http.conf\n", v->name, v->value, v->lineno); } + } else if (!strcasecmp(v->name, "session_inactivity")) { + if (ast_parse_arg(v->value, PARSE_INT32 |PARSE_DEFAULT | PARSE_IN_RANGE, + &session_inactivity, DEFAULT_SESSION_INACTIVITY, 1, INT_MAX)) { + ast_log(LOG_WARNING, "Invalid %s '%s' at line %d of http.conf\n", + v->name, v->value, v->lineno); + } } else { ast_log(LOG_WARNING, "Ignoring unknown option '%s' in http.conf\n", v->name); } diff --git a/main/manager.c b/main/manager.c index 09d49f002..b4c70a0fc 100644 --- a/main/manager.c +++ b/main/manager.c @@ -1123,7 +1123,6 @@ static char *manager_channelvars; #define DEFAULT_REALM "asterisk" static char global_realm[MAXHOSTNAMELEN]; /*!< Default realm */ -static int block_sockets; static int unauth_sessions = 0; static struct stasis_subscription *acl_change_sub; @@ -1770,15 +1769,7 @@ static void session_destructor(void *obj) } if (session->f != NULL) { - /* - * Issuing shutdown() is necessary here to avoid a race - * condition where the last data written may not appear - * in the the TCP stream. See ASTERISK-23548 - */ fflush(session->f); - if (session->fd != -1) { - shutdown(session->fd, SHUT_RDWR); - } fclose(session->f); } if (eqe) { @@ -5888,12 +5879,9 @@ static void *session_do(void *data) ast_log(LOG_WARNING, "Failed to set manager tcp connection to TCP_NODELAY, getprotobyname(\"tcp\") failed\nSome manager actions may be slow to respond.\n"); } + /* make sure socket is non-blocking */ flags = fcntl(ser->fd, F_GETFL); - if (!block_sockets) { /* make sure socket is non-blocking */ - flags |= O_NONBLOCK; - } else { - flags &= ~O_NONBLOCK; - } + flags |= O_NONBLOCK; fcntl(ser->fd, F_SETFL, flags); ao2_lock(session); @@ -5919,11 +5907,17 @@ static void *session_do(void *data) } ao2_unlock(session); + ast_tcptls_stream_set_timeout_sequence(ser->stream_cookie, + ast_tvnow(), authtimeout * 1000); + astman_append(&s, "Asterisk Call Manager/%s\r\n", AMI_VERSION); /* welcome prompt */ for (;;) { if ((res = do_message(&s)) < 0 || s.write_error) { break; } + if (session->authenticated) { + ast_tcptls_stream_set_timeout_disable(ser->stream_cookie); + } } /* session is over, explain why and terminate */ if (session->authenticated) { @@ -6732,6 +6726,30 @@ static void xml_translate(struct ast_str **out, char *in, struct ast_variable *g } } +static void close_mansession_file(struct mansession *s) +{ + if (s->f) { + if (fclose(s->f)) { + ast_log(LOG_ERROR, "fclose() failed: %s\n", strerror(errno)); + } + s->f = NULL; + s->fd = -1; + } else if (s->fd != -1) { + /* + * Issuing shutdown() is necessary here to avoid a race + * condition where the last data written may not appear + * in the TCP stream. See ASTERISK-23548 + */ + shutdown(s->fd, SHUT_RDWR); + if (close(s->fd)) { + ast_log(LOG_ERROR, "close() failed: %s\n", strerror(errno)); + } + s->fd = -1; + } else { + ast_log(LOG_ERROR, "Attempted to close file/file descriptor on mansession without a valid file or file descriptor.\n"); + } +} + static void process_output(struct mansession *s, struct ast_str **out, struct ast_variable *params, enum output_format format) { char *buf; @@ -6759,29 +6777,7 @@ static void process_output(struct mansession *s, struct ast_str **out, struct as xml_translate(out, "", params, format); } - if (s->f) { - /* - * Issuing shutdown() is necessary here to avoid a race - * condition where the last data written may not appear - * in the the TCP stream. See ASTERISK-23548 - */ - if (s->fd != -1) { - shutdown(s->fd, SHUT_RDWR); - } - if (fclose(s->f)) { - ast_log(LOG_ERROR, "fclose() failed: %s\n", strerror(errno)); - } - s->f = NULL; - s->fd = -1; - } else if (s->fd != -1) { - shutdown(s->fd, SHUT_RDWR); - if (close(s->fd)) { - ast_log(LOG_ERROR, "close() failed: %s\n", strerror(errno)); - } - s->fd = -1; - } else { - ast_log(LOG_ERROR, "process output attempted to close file/file descriptor on mansession without a valid file or file descriptor.\n"); - } + close_mansession_file(s); } static int generic_http_callback(struct ast_tcptls_session_instance *ser, @@ -7572,7 +7568,6 @@ static char *handle_manager_show_settings(struct ast_cli_entry *e, int cmd, stru ast_cli(a->fd, FORMAT, "Timestamp events:", AST_CLI_YESNO(timestampevents)); ast_cli(a->fd, FORMAT, "Channel vars:", S_OR(manager_channelvars, "")); ast_cli(a->fd, FORMAT, "Debug:", AST_CLI_YESNO(manager_debug)); - ast_cli(a->fd, FORMAT, "Block sockets:", AST_CLI_YESNO(block_sockets)); #undef FORMAT #undef FORMAT2 @@ -8175,8 +8170,6 @@ static int __init_manager(int reload, int by_external_config) if (!strcasecmp(var->name, "enabled")) { manager_enabled = ast_true(val); - } else if (!strcasecmp(var->name, "block-sockets")) { - block_sockets = ast_true(val); } else if (!strcasecmp(var->name, "webenabled")) { webmanager_enabled = ast_true(val); } else if (!strcasecmp(var->name, "port")) { diff --git a/main/tcptls.c b/main/tcptls.c index 3a8e412b5..076f94bae 100644 --- a/main/tcptls.c +++ b/main/tcptls.c @@ -50,102 +50,483 @@ ASTERISK_FILE_VERSION(__FILE__, "$Revision$") #include "asterisk/astobj2.h" #include "asterisk/pbx.h" -/*! \brief - * replacement read/write functions for SSL support. - * We use wrappers rather than SSL_read/SSL_write directly so - * we can put in some debugging. - */ +/*! ao2 object used for the FILE stream fopencookie()/funopen() cookie. */ +struct ast_tcptls_stream { + /*! SSL state if not NULL */ + SSL *ssl; + /*! + * \brief Start time from when an I/O sequence must complete + * by struct ast_tcptls_stream.timeout. + * + * \note If struct ast_tcptls_stream.start.tv_sec is zero then + * start time is the current I/O request. + */ + struct timeval start; + /*! + * \brief The socket returned by accept(). + * + * \note Set to -1 if the stream is closed. + */ + int fd; + /*! + * \brief Timeout in ms relative to struct ast_tcptls_stream.start + * to wait for an event on struct ast_tcptls_stream.fd. + * + * \note Set to -1 to disable timeout. + * \note The socket needs to be set to non-blocking for the timeout + * feature to work correctly. + */ + int timeout; +}; -#ifdef DO_SSL -static HOOK_T ssl_read(void *cookie, char *buf, LEN_T len) +void ast_tcptls_stream_set_timeout_disable(struct ast_tcptls_stream *stream) { - int i = SSL_read(cookie, buf, len-1); -#if 0 - if (i >= 0) { - buf[i] = '\0'; - } - ast_verb(0, "ssl read size %d returns %d <%s>\n", (int)len, i, buf); -#endif - return i; + ast_assert(stream != NULL); + + stream->timeout = -1; } -static HOOK_T ssl_write(void *cookie, const char *buf, LEN_T len) +void ast_tcptls_stream_set_timeout_inactivity(struct ast_tcptls_stream *stream, int timeout) { -#if 0 - char *s = ast_alloca(len+1); + ast_assert(stream != NULL); - strncpy(s, buf, len); - s[len] = '\0'; - ast_verb(0, "ssl write size %d <%s>\n", (int)len, s); -#endif - return SSL_write(cookie, buf, len); + stream->start.tv_sec = 0; + stream->timeout = timeout; } -static int ssl_close(void *cookie) +void ast_tcptls_stream_set_timeout_sequence(struct ast_tcptls_stream *stream, struct timeval start, int timeout) { - int cookie_fd = SSL_get_fd(cookie); - int ret; + ast_assert(stream != NULL); - if (cookie_fd > -1) { - /* - * According to the TLS standard, it is acceptable for an application to only send its shutdown - * alert and then close the underlying connection without waiting for the peer's response (this - * way resources can be saved, as the process can already terminate or serve another connection). - */ - if ((ret = SSL_shutdown(cookie)) < 0) { - ast_log(LOG_ERROR, "SSL_shutdown() failed: %d\n", SSL_get_error(cookie, ret)); + stream->start = start; + stream->timeout = timeout; +} + +/*! + * \internal + * \brief fopencookie()/funopen() stream read function. + * + * \param cookie Stream control data. + * \param buf Where to put read data. + * \param size Size of the buffer. + * + * \retval number of bytes put into buf. + * \retval 0 on end of file. + * \retval -1 on error. + */ +static HOOK_T tcptls_stream_read(void *cookie, char *buf, LEN_T size) +{ + struct ast_tcptls_stream *stream = cookie; + struct timeval start; + int ms; + int res; + + if (!size) { + /* You asked for no data you got no data. */ + return 0; + } + + if (!stream || stream->fd == -1) { + errno = EBADF; + return -1; + } + + if (stream->start.tv_sec) { + start = stream->start; + } else { + start = ast_tvnow(); + } + +#if defined(DO_SSL) + if (stream->ssl) { + for (;;) { + res = SSL_read(stream->ssl, buf, size); + if (0 < res) { + /* We read some payload data. */ + return res; + } + switch (SSL_get_error(stream->ssl, res)) { + case SSL_ERROR_ZERO_RETURN: + /* Report EOF for a shutdown */ + ast_debug(1, "TLS clean shutdown alert reading data\n"); + return 0; + case SSL_ERROR_WANT_READ: + while ((ms = ast_remaining_ms(start, stream->timeout))) { + res = ast_wait_for_input(stream->fd, ms); + if (0 < res) { + /* Socket is ready to be read. */ + break; + } + if (res < 0) { + if (errno == EINTR || errno == EAGAIN) { + /* Try again. */ + continue; + } + ast_debug(1, "TLS socket error waiting for read data: %s\n", + strerror(errno)); + return -1; + } + } + break; + case SSL_ERROR_WANT_WRITE: + while ((ms = ast_remaining_ms(start, stream->timeout))) { + res = ast_wait_for_output(stream->fd, ms); + if (0 < res) { + /* Socket is ready to be written. */ + break; + } + if (res < 0) { + if (errno == EINTR || errno == EAGAIN) { + /* Try again. */ + continue; + } + ast_debug(1, "TLS socket error waiting for write space: %s\n", + strerror(errno)); + return -1; + } + } + break; + default: + /* Report EOF for an undecoded SSL or transport error. */ + ast_debug(1, "TLS transport or SSL error reading data\n"); + return 0; + } + if (!ms) { + /* Report EOF for a timeout */ + ast_debug(1, "TLS timeout reading data\n"); + return 0; + } + } + } +#endif /* defined(DO_SSL) */ + + for (;;) { + res = read(stream->fd, buf, size); + if (0 <= res) { + return res; } + if (errno != EINTR && errno != EAGAIN) { + /* Not a retryable error. */ + ast_debug(1, "TCP socket error reading data: %s\n", + strerror(errno)); + return -1; + } + ms = ast_remaining_ms(start, stream->timeout); + if (!ms) { + /* Report EOF for a timeout */ + ast_debug(1, "TCP timeout reading data\n"); + return 0; + } + ast_wait_for_input(stream->fd, ms); + } +} + +/*! + * \internal + * \brief fopencookie()/funopen() stream write function. + * + * \param cookie Stream control data. + * \param buf Where to get data to write. + * \param size Size of the buffer. + * + * \retval number of bytes written from buf. + * \retval -1 on error. + */ +static HOOK_T tcptls_stream_write(void *cookie, const char *buf, LEN_T size) +{ + struct ast_tcptls_stream *stream = cookie; + struct timeval start; + int ms; + int res; + int written; + int remaining; + + if (!size) { + /* You asked to write no data you wrote no data. */ + return 0; + } + + if (!stream || stream->fd == -1) { + errno = EBADF; + return -1; + } + + if (stream->start.tv_sec) { + start = stream->start; + } else { + start = ast_tvnow(); + } - if (!((SSL*)cookie)->server) { - /* For client threads, ensure that the error stack is cleared */ - ERR_remove_state(0); +#if defined(DO_SSL) + if (stream->ssl) { + written = 0; + remaining = size; + for (;;) { + res = SSL_write(stream->ssl, buf + written, remaining); + if (res == remaining) { + /* Everything was written. */ + return size; + } + if (0 < res) { + /* Successfully wrote part of the buffer. Try to write the rest. */ + written += res; + remaining -= res; + continue; + } + switch (SSL_get_error(stream->ssl, res)) { + case SSL_ERROR_ZERO_RETURN: + ast_debug(1, "TLS clean shutdown alert writing data\n"); + if (written) { + /* Report partial write. */ + return written; + } + errno = EBADF; + return -1; + case SSL_ERROR_WANT_READ: + ms = ast_remaining_ms(start, stream->timeout); + if (!ms) { + /* Report partial write. */ + ast_debug(1, "TLS timeout writing data (want read)\n"); + return written; + } + ast_wait_for_input(stream->fd, ms); + break; + case SSL_ERROR_WANT_WRITE: + ms = ast_remaining_ms(start, stream->timeout); + if (!ms) { + /* Report partial write. */ + ast_debug(1, "TLS timeout writing data (want write)\n"); + return written; + } + ast_wait_for_output(stream->fd, ms); + break; + default: + /* Undecoded SSL or transport error. */ + ast_debug(1, "TLS transport or SSL error writing data\n"); + if (written) { + /* Report partial write. */ + return written; + } + errno = EBADF; + return -1; + } } + } +#endif /* defined(DO_SSL) */ - SSL_free(cookie); - /* adding shutdown(2) here has no added benefit */ - if (close(cookie_fd)) { + written = 0; + remaining = size; + for (;;) { + res = write(stream->fd, buf + written, remaining); + if (res == remaining) { + /* Yay everything was written. */ + return size; + } + if (0 < res) { + /* Successfully wrote part of the buffer. Try to write the rest. */ + written += res; + remaining -= res; + continue; + } + if (errno != EINTR && errno != EAGAIN) { + /* Not a retryable error. */ + ast_debug(1, "TCP socket error writing: %s\n", strerror(errno)); + if (written) { + return written; + } + return -1; + } + ms = ast_remaining_ms(start, stream->timeout); + if (!ms) { + /* Report partial write. */ + ast_debug(1, "TCP timeout writing data\n"); + return written; + } + ast_wait_for_output(stream->fd, ms); + } +} + +/*! + * \internal + * \brief fopencookie()/funopen() stream close function. + * + * \param cookie Stream control data. + * + * \retval 0 on success. + * \retval -1 on error. + */ +static int tcptls_stream_close(void *cookie) +{ + struct ast_tcptls_stream *stream = cookie; + + if (!stream) { + errno = EBADF; + return -1; + } + + if (stream->fd != -1) { +#if defined(DO_SSL) + if (stream->ssl) { + int res; + + /* + * According to the TLS standard, it is acceptable for an + * application to only send its shutdown alert and then + * close the underlying connection without waiting for + * the peer's response (this way resources can be saved, + * as the process can already terminate or serve another + * connection). + */ + res = SSL_shutdown(stream->ssl); + if (res < 0) { + ast_log(LOG_ERROR, "SSL_shutdown() failed: %d\n", + SSL_get_error(stream->ssl, res)); + } + + if (!stream->ssl->server) { + /* For client threads, ensure that the error stack is cleared */ + ERR_remove_state(0); + } + + SSL_free(stream->ssl); + stream->ssl = NULL; + } +#endif /* defined(DO_SSL) */ + + /* + * Issuing shutdown() is necessary here to avoid a race + * condition where the last data written may not appear + * in the TCP stream. See ASTERISK-23548 + */ + shutdown(stream->fd, SHUT_RDWR); + if (close(stream->fd)) { ast_log(LOG_ERROR, "close() failed: %s\n", strerror(errno)); } + stream->fd = -1; } + ao2_t_ref(stream, -1, "Closed tcptls stream cookie"); + return 0; } -#endif /* DO_SSL */ + +/*! + * \internal + * \brief fopencookie()/funopen() stream destructor function. + * + * \param cookie Stream control data. + * + * \return Nothing + */ +static void tcptls_stream_dtor(void *cookie) +{ + struct ast_tcptls_stream *stream = cookie; + + ast_assert(stream->fd == -1); +} + +/*! + * \internal + * \brief fopencookie()/funopen() stream allocation function. + * + * \retval stream_cookie on success. + * \retval NULL on error. + */ +static struct ast_tcptls_stream *tcptls_stream_alloc(void) +{ + struct ast_tcptls_stream *stream; + + stream = ao2_alloc_options(sizeof(*stream), tcptls_stream_dtor, + AO2_ALLOC_OPT_LOCK_NOLOCK); + if (stream) { + stream->fd = -1; + stream->timeout = -1; + } + return stream; +} + +/*! + * \internal + * \brief Open a custom FILE stream for tcptls. + * + * \param stream Stream cookie control data. + * \param ssl SSL state if not NULL. + * \param fd Socket file descriptor. + * \param timeout ms to wait for an event on fd. -1 if timeout disabled. + * + * \retval fp on success. + * \retval NULL on error. + */ +static FILE *tcptls_stream_fopen(struct ast_tcptls_stream *stream, SSL *ssl, int fd, int timeout) +{ + FILE *fp; + +#if defined(HAVE_FOPENCOOKIE) /* the glibc/linux interface */ + static const cookie_io_functions_t cookie_funcs = { + tcptls_stream_read, + tcptls_stream_write, + NULL, + tcptls_stream_close + }; +#endif /* defined(HAVE_FOPENCOOKIE) */ + + if (fd == -1) { + /* Socket not open. */ + return NULL; + } + + stream->ssl = ssl; + stream->fd = fd; + stream->timeout = timeout; + ao2_t_ref(stream, +1, "Opening tcptls stream cookie"); + +#if defined(HAVE_FUNOPEN) /* the BSD interface */ + fp = funopen(stream, tcptls_stream_read, tcptls_stream_write, NULL, + tcptls_stream_close); +#elif defined(HAVE_FOPENCOOKIE) /* the glibc/linux interface */ + fp = fopencookie(stream, "w+", cookie_funcs); +#else + /* could add other methods here */ + ast_debug(2, "No stream FILE methods attempted!\n"); + fp = NULL; +#endif + + if (!fp) { + stream->fd = -1; + ao2_t_ref(stream, -1, "Failed to open tcptls stream cookie"); + } + return fp; +} HOOK_T ast_tcptls_server_read(struct ast_tcptls_session_instance *tcptls_session, void *buf, size_t count) { - if (tcptls_session->fd == -1) { - ast_log(LOG_ERROR, "server_read called with an fd of -1\n"); + if (!tcptls_session->stream_cookie || tcptls_session->stream_cookie->fd == -1) { + ast_log(LOG_ERROR, "TCP/TLS read called on invalid stream.\n"); errno = EIO; return -1; } -#ifdef DO_SSL - if (tcptls_session->ssl) { - return ssl_read(tcptls_session->ssl, buf, count); - } -#endif - return read(tcptls_session->fd, buf, count); + return tcptls_stream_read(tcptls_session->stream_cookie, buf, count); } HOOK_T ast_tcptls_server_write(struct ast_tcptls_session_instance *tcptls_session, const void *buf, size_t count) { - if (tcptls_session->fd == -1) { - ast_log(LOG_ERROR, "server_write called with an fd of -1\n"); + if (!tcptls_session->stream_cookie || tcptls_session->stream_cookie->fd == -1) { + ast_log(LOG_ERROR, "TCP/TLS write called on invalid stream.\n"); errno = EIO; return -1; } -#ifdef DO_SSL - if (tcptls_session->ssl) { - return ssl_write(tcptls_session->ssl, buf, count); - } -#endif - return write(tcptls_session->fd, buf, count); + return tcptls_stream_write(tcptls_session->stream_cookie, buf, count); } static void session_instance_destructor(void *obj) { struct ast_tcptls_session_instance *i = obj; + + if (i->stream_cookie) { + ao2_t_ref(i->stream_cookie, -1, "Destroying tcptls session instance"); + i->stream_cookie = NULL; + } ast_free(i->overflow_buf); } @@ -177,12 +558,21 @@ static void *handle_tcptls_connection(void *data) return NULL; } + tcptls_session->stream_cookie = tcptls_stream_alloc(); + if (!tcptls_session->stream_cookie) { + ast_tcptls_close_session_file(tcptls_session); + ao2_ref(tcptls_session, -1); + return NULL; + } + /* * open a FILE * as appropriate. */ if (!tcptls_session->parent->tls_cfg) { - if ((tcptls_session->f = fdopen(tcptls_session->fd, "w+"))) { - if(setvbuf(tcptls_session->f, NULL, _IONBF, 0)) { + tcptls_session->f = tcptls_stream_fopen(tcptls_session->stream_cookie, NULL, + tcptls_session->fd, -1); + if (tcptls_session->f) { + if (setvbuf(tcptls_session->f, NULL, _IONBF, 0)) { ast_tcptls_close_session_file(tcptls_session); } } @@ -192,19 +582,8 @@ static void *handle_tcptls_connection(void *data) SSL_set_fd(tcptls_session->ssl, tcptls_session->fd); if ((ret = ssl_setup(tcptls_session->ssl)) <= 0) { ast_log(LOG_ERROR, "Problem setting up ssl connection: %s\n", ERR_error_string(ERR_get_error(), err)); - } else { -#if defined(HAVE_FUNOPEN) /* the BSD interface */ - tcptls_session->f = funopen(tcptls_session->ssl, ssl_read, ssl_write, NULL, ssl_close); - -#elif defined(HAVE_FOPENCOOKIE) /* the glibc/linux interface */ - static const cookie_io_functions_t cookie_funcs = { - ssl_read, ssl_write, NULL, ssl_close - }; - tcptls_session->f = fopencookie(tcptls_session->ssl, "w+", cookie_funcs); -#else - /* could add other methods here */ - ast_debug(2, "no tcptls_session->f methods attempted!\n"); -#endif + } else if ((tcptls_session->f = tcptls_stream_fopen(tcptls_session->stream_cookie, + tcptls_session->ssl, tcptls_session->fd, -1))) { if ((tcptls_session->client && !ast_test_flag(&tcptls_session->parent->tls_cfg->flags, AST_SSL_DONT_VERIFY_SERVER)) || (!tcptls_session->client && ast_test_flag(&tcptls_session->parent->tls_cfg->flags, AST_SSL_VERIFY_CLIENT))) { X509 *peer; @@ -625,21 +1004,18 @@ error: void ast_tcptls_close_session_file(struct ast_tcptls_session_instance *tcptls_session) { if (tcptls_session->f) { - /* - * Issuing shutdown() is necessary here to avoid a race - * condition where the last data written may not appear - * in the TCP stream. See ASTERISK-23548 - */ fflush(tcptls_session->f); - if (tcptls_session->fd != -1) { - shutdown(tcptls_session->fd, SHUT_RDWR); - } if (fclose(tcptls_session->f)) { ast_log(LOG_ERROR, "fclose() failed: %s\n", strerror(errno)); } tcptls_session->f = NULL; tcptls_session->fd = -1; } else if (tcptls_session->fd != -1) { + /* + * Issuing shutdown() is necessary here to avoid a race + * condition where the last data written may not appear + * in the TCP stream. See ASTERISK-23548 + */ shutdown(tcptls_session->fd, SHUT_RDWR); if (close(tcptls_session->fd)) { ast_log(LOG_ERROR, "close() failed: %s\n", strerror(errno)); diff --git a/main/utils.c b/main/utils.c index 2826a41d5..bb8559d8c 100644 --- a/main/utils.c +++ b/main/utils.c @@ -1260,13 +1260,24 @@ int ast_pthread_create_detached_stack(pthread_t *thread, pthread_attr_t *attr, v int ast_wait_for_input(int fd, int ms) { struct pollfd pfd[1]; + + memset(pfd, 0, sizeof(pfd)); + pfd[0].fd = fd; + pfd[0].events = POLLIN | POLLPRI; + return ast_poll(pfd, 1, ms); +} + +int ast_wait_for_output(int fd, int ms) +{ + struct pollfd pfd[1]; + memset(pfd, 0, sizeof(pfd)); pfd[0].fd = fd; - pfd[0].events = POLLIN|POLLPRI; + pfd[0].events = POLLOUT; return ast_poll(pfd, 1, ms); } -static int ast_wait_for_output(int fd, int timeoutms) +static int wait_for_output(int fd, int timeoutms) { struct pollfd pfd = { .fd = fd, @@ -1326,7 +1337,7 @@ int ast_carefulwrite(int fd, char *s, int len, int timeoutms) int elapsed = 0; while (len) { - if (ast_wait_for_output(fd, timeoutms - elapsed)) { + if (wait_for_output(fd, timeoutms - elapsed)) { return -1; } @@ -1367,7 +1378,7 @@ int ast_careful_fwrite(FILE *f, int fd, const char *src, size_t len, int timeout int elapsed = 0; while (len) { - if (ast_wait_for_output(fd, timeoutms - elapsed)) { + if (wait_for_output(fd, timeoutms - elapsed)) { /* poll returned a fatal error, so bail out immediately. */ return -1; } diff --git a/res/res_http_websocket.c b/res/res_http_websocket.c index 07cb6b7be..744521507 100644 --- a/res/res_http_websocket.c +++ b/res/res_http_websocket.c @@ -757,8 +757,13 @@ int AST_OPTIONAL_API_NAME(ast_websocket_uri_cb)(struct ast_tcptls_session_instan protocol_handler->callback(session, get_vars, headers); ao2_ref(protocol_handler, -1); - /* By dropping the FILE* from the session it won't get closed when the HTTP server cleans up */ + /* + * By dropping the FILE* and fd from the session the connection + * won't get closed when the HTTP server cleans up because we + * passed the connection to the protocol handler. + */ ser->f = NULL; + ser->fd = -1; return 0; } |