conn: Properly close MetricsPort socket on EOF

Handle the EOF situation for a metrics connection. Furthermore, if we failed
to fetch the data from the inbuf properly, mark the socket as closed because
the caller, connection_process_inbuf(), assumes that we did so on error.

Fixes #40257

Signed-off-by: David Goulet <dgoulet@torproject.org>
This commit is contained in:
David Goulet 2021-01-27 09:34:34 -05:00 committed by Nick Mathewson
parent f420eacf18
commit 01c4abc2d4
5 changed files with 51 additions and 8 deletions

3
changes/ticket40257 Normal file
View File

@ -0,0 +1,3 @@
o Minor bugfixes (metrics port):
- Fix a bug warning when the socket was unexpectedly closed. Fixes bug
40257; bugfix on 0.4.5.1-alpha

View File

@ -5242,6 +5242,8 @@ connection_reached_eof(connection_t *conn)
return connection_dir_reached_eof(TO_DIR_CONN(conn));
case CONN_TYPE_CONTROL:
return connection_control_reached_eof(TO_CONTROL_CONN(conn));
case CONN_TYPE_METRICS:
return metrics_connection_reached_eof(conn);
default:
log_err(LD_BUG,"got unexpected conn type %d.", conn->type);
tor_fragile_assert();

View File

@ -20,6 +20,7 @@
#include "lib/net/address.h"
#include "core/mainloop/connection.h"
#include "core/or/connection_or.h"
#include "core/or/connection_st.h"
#include "core/or/policies.h"
#include "core/or/port_cfg_st.h"
@ -93,7 +94,8 @@ metrics_get_output(const metrics_format_t fmt)
/** Process what is in the inbuf of this connection of type metrics.
*
* Return 0 on success else -1 on error which will close the connection. */
* Return 0 on success else -1 on error for which the connection is marked for
* close. */
int
metrics_connection_process_inbuf(connection_t *conn)
{
@ -111,13 +113,14 @@ metrics_connection_process_inbuf(connection_t *conn)
goto err;
}
const int http_status = fetch_from_buf_http(conn->inbuf, &headers, 1024,
NULL, NULL, 1024, 0);
const int http_status =
connection_fetch_from_buf_http(conn, &headers, 1024, NULL, NULL, 1024, 0);
if (http_status < 0) {
errmsg = "HTTP/1.0 400 Bad Request\r\n\r\n";
goto err;
} else if (http_status == 0) {
/* no HTTP request yet. */
ret = 0;
goto done;
}
@ -155,6 +158,7 @@ metrics_connection_process_inbuf(connection_t *conn)
log_info(LD_EDGE, "HTTP metrics error: saying %s", escaped(errmsg));
connection_buf_add(errmsg, strlen(errmsg), conn);
}
connection_mark_and_flush(conn);
done:
tor_free(headers);
@ -243,6 +247,17 @@ metrics_parse_ports(or_options_t *options, smartlist_t *ports,
return ret;
}
/** Called when conn has gotten its socket closed. */
int
metrics_connection_reached_eof(connection_t *conn)
{
tor_assert(conn);
log_info(LD_EDGE, "Metrics connection reached EOF. Closing.");
connection_mark_for_close(conn);
return 0;
}
/** Initialize the subsystem. */
void
metrics_init(void)

View File

@ -27,6 +27,7 @@ buf_t *metrics_get_output(const metrics_format_t fmt);
/* Connection. */
int metrics_connection_process_inbuf(struct connection_t *conn);
int metrics_connection_reached_eof(struct connection_t *conn);
/* Configuration. */
int metrics_parse_ports(or_options_t *options, smartlist_t *ports,

View File

@ -8,6 +8,7 @@
#define CONFIG_PRIVATE
#define CONNECTION_PRIVATE
#define MAINLOOP_PRIVATE
#define METRICS_STORE_ENTRY_PRIVATE
#include "test/test.h"
@ -17,6 +18,7 @@
#include "app/config/config.h"
#include "core/mainloop/connection.h"
#include "core/mainloop/mainloop.h"
#include "core/or/connection_st.h"
#include "core/or/policies.h"
#include "core/or/port_cfg_st.h"
@ -96,43 +98,62 @@ static char _c_buf[256];
#define WRITE(conn, msg) \
buf_add(conn->inbuf, (msg), (strlen(msg)));
/* Free the previous conn object if any and allocate a new connection. In
* order to be allowed, set its address to 1.2.3.4 as per the policy. */
#define NEW_ALLOWED_CONN() \
do { \
close_closeable_connections(); \
conn = connection_new(CONN_TYPE_METRICS, AF_INET); \
tor_addr_from_ipv4h(&conn->addr, 0x01020304); \
} while (0)
static void
test_connection(void *arg)
{
int ret;
connection_t *conn = connection_new(CONN_TYPE_METRICS, AF_INET);
connection_t *conn = NULL;
or_options_t *options = get_options_mutable();
(void) arg;
/* Notice that in this test, we will allocate a new connection at every test
* case. This is because the metrics_connection_process_inbuf() marks for
* close the connection in case of an error and thus we can't call again an
* inbuf process function on a marked for close connection. */
tor_init_connection_lists();
/* Setup policy. */
set_metrics_port(options);
/* Set 1.2.3.5 IP, we should get rejected. */
NEW_ALLOWED_CONN();
tor_addr_from_ipv4h(&conn->addr, 0x01020305);
ret = metrics_connection_process_inbuf(conn);
tt_int_op(ret, OP_EQ, -1);
/* Set 1.2.3.4 so from now on we are allowed to process the inbuf. */
tor_addr_from_ipv4h(&conn->addr, 0x01020304);
/* No HTTP request yet. */
NEW_ALLOWED_CONN();
ret = metrics_connection_process_inbuf(conn);
tt_int_op(ret, OP_EQ, -1);
tt_int_op(ret, OP_EQ, 0);
connection_free_minimal(conn);
/* Bad request. */
NEW_ALLOWED_CONN();
WRITE(conn, "HTTP 4.7\r\n\r\n");
ret = metrics_connection_process_inbuf(conn);
tt_int_op(ret, OP_EQ, -1);
CONTAINS(conn, "HTTP/1.0 400 Bad Request\r\n\r\n");
/* Path not found. */
NEW_ALLOWED_CONN();
WRITE(conn, "GET /badpath HTTP/1.0\r\n\r\n");
ret = metrics_connection_process_inbuf(conn);
tt_int_op(ret, OP_EQ, -1);
CONTAINS(conn, "HTTP/1.0 404 Not Found\r\n\r\n");
/* Method not allowed. */
NEW_ALLOWED_CONN();
WRITE(conn, "POST /something HTTP/1.0\r\n\r\n");
ret = metrics_connection_process_inbuf(conn);
tt_int_op(ret, OP_EQ, -1);
@ -140,6 +161,7 @@ test_connection(void *arg)
/* Ask for metrics. The content should be above 0. We don't test the
* validity of the returned content but it is certainly not an error. */
NEW_ALLOWED_CONN();
WRITE(conn, "GET /metrics HTTP/1.0\r\n\r\n");
ret = metrics_connection_process_inbuf(conn);
tt_int_op(ret, OP_EQ, 0);