From ed41584c92d1be3e3fbeff45fbdb8440c9878b58 Mon Sep 17 00:00:00 2001 From: DL6ER Date: Sun, 24 Mar 2024 12:42:49 +0100 Subject: [PATCH 1/2] Add extra logging around network issues (EDE: network error) Signed-off-by: DL6ER --- src/dnsmasq/forward.c | 24 +++++++++++++++++++++++- src/dnsmasq_interface.c | 27 +++++++++++++++++++++++++-- src/dnsmasq_interface.h | 2 ++ 3 files changed, 50 insertions(+), 3 deletions(-) diff --git a/src/dnsmasq/forward.c b/src/dnsmasq/forward.c index 2176c231..f9316a08 100644 --- a/src/dnsmasq/forward.c +++ b/src/dnsmasq/forward.c @@ -105,7 +105,12 @@ int send_from(int fd, int nowild, char *packet, size_t len, #ifdef HAVE_LINUX_NETWORK /* If interface is still in DAD, EINVAL results - ignore that. */ if (errno != EINVAL) - my_syslog(LOG_ERR, _("failed to send packet: %s"), strerror(errno)); + { + my_syslog(LOG_ERR, _("failed to send packet: %s"), strerror(errno)); + /********** Pi-hole modification **********/ + FTL_connection_error("failed to send UDP reply", to); + /******************************************/ + } #endif return 0; } @@ -567,6 +572,12 @@ static int forward_query(int udpfd, union mysockaddr *udpaddr, break; forward->forwardall++; } + /**** Pi-hole modification ****/ + else + { + FTL_connection_error("failed to send UDP request", &srv->addr); + } + /******************************/ } if (++start == last) @@ -2087,12 +2098,19 @@ static ssize_t tcp_talk(int first, int last, int start, unsigned char *packet, data_sent = 1; else if (errno == ETIMEDOUT || errno == EHOSTUNREACH) timedout = 1; + /**** Pi-hole modification ****/ + if (errno != 0) + FTL_connection_error("failed to send TCP(fast-open) packet", &serv->addr); + /******************************/ #endif /* If fastopen failed due to lack of reply, then there's no point in trying again in non-FASTOPEN mode. */ if (timedout || (!data_sent && connect(serv->tcpfd, &serv->addr.sa, sa_len(&serv->addr)) == -1)) { + /**** Pi-hole modification ****/ + FTL_connection_error("failed to send TCP(connect) packet", &serv->addr); + /******************************/ close(serv->tcpfd); serv->tcpfd = -1; continue; @@ -2107,6 +2125,10 @@ static ssize_t tcp_talk(int first, int last, int start, unsigned char *packet, !read_write(serv->tcpfd, &c2, 1, 1) || !read_write(serv->tcpfd, payload, (rsize = (c1 << 8) | c2), 1)) { + /**** Pi-hole modification ****/ + FTL_connection_error("failed to send TCP(read_write) packet", &serv->addr); + /******************************/ + close(serv->tcpfd); serv->tcpfd = -1; /* We get data then EOF, reopen connection to same server, diff --git a/src/dnsmasq_interface.c b/src/dnsmasq_interface.c index b8257208..686c3016 100644 --- a/src/dnsmasq_interface.c +++ b/src/dnsmasq_interface.c @@ -72,7 +72,7 @@ static void FTL_forwarded(const unsigned int flags, const char *name, const unio static void FTL_reply(const unsigned int flags, const char *name, const union all_addr *addr, const char* arg, const int id, const char* file, const int line); static void FTL_upstream_error(const union all_addr *addr, const unsigned int flags, const int id, const char* file, const int line); static void FTL_dnssec(const char *result, const union all_addr *addr, const int id, const char* file, const int line); -static void mysockaddr_extract_ip_port(union mysockaddr *server, char ip[ADDRSTRLEN+1], in_port_t *port); +static void mysockaddr_extract_ip_port(const union mysockaddr *server, char ip[ADDRSTRLEN+1], in_port_t *port); static void alladdr_extract_ip(union all_addr *addr, const sa_family_t family, char ip[ADDRSTRLEN+1]); static void check_pihole_PTR(char *domain); #define query_set_dnssec(query, dnssec) _query_set_dnssec(query, dnssec, __FILE__, __LINE__) @@ -1829,7 +1829,7 @@ static void alladdr_extract_ip(union all_addr *addr, const sa_family_t family, c inet_ntop(family, addr, ip, ADDRSTRLEN); } -static void mysockaddr_extract_ip_port(union mysockaddr *server, char ip[ADDRSTRLEN+1], in_port_t *port) +static void mysockaddr_extract_ip_port(const union mysockaddr *server, char ip[ADDRSTRLEN+1], in_port_t *port) { // Extract IP address inet_ntop(server->sa.sa_family, @@ -3507,4 +3507,27 @@ void get_dnsmasq_metrics_obj(cJSON *json) { for (unsigned int i = 0; i < __METRIC_MAX; i++) cJSON_AddNumberToObject(json, get_metric_name(i), daemon->metrics[i]); +} + +void FTL_connection_error(const char *reason, const union mysockaddr *addr) +{ + // Make a private copy of the error + const char *error = strerror(errno); + + if(config.debug.queries.v.b) + { + const int id = daemon->log_display_id; + + // Format the address into a string (if available) + in_port_t port = 0; + char ip[ADDRSTRLEN] = { 0 }; + if(addr != NULL) + mysockaddr_extract_ip_port(addr, ip, &port); + + // Log to FTL.log + log_debug(DEBUG_QUERIES, "Connection error: %s (%s) for %s#%u (ID %d)", reason, error, ip, port, id); + } + + // Log to pihole.log + my_syslog(LOG_ERR, "%s: %s", reason, error); } \ No newline at end of file diff --git a/src/dnsmasq_interface.h b/src/dnsmasq_interface.h index f58851dc..3335fc6a 100644 --- a/src/dnsmasq_interface.h +++ b/src/dnsmasq_interface.h @@ -48,6 +48,8 @@ void FTL_TCP_worker_terminating(bool finished); bool FTL_unlink_DHCP_lease(const char *ipaddr, const char **hint); +void FTL_connection_error(const char *reason, const union mysockaddr *addr); + // defined in src/dnsmasq/cache.c extern char *querystr(char *desc, unsigned short type); From 563b02ccc7a898694243e784ec9d8557e25cf6db Mon Sep 17 00:00:00 2001 From: DL6ER Date: Tue, 2 Apr 2024 13:43:43 +0200 Subject: [PATCH 2/2] Add new CONNECTION_ERROR message to the Pi-hole diagnosis system Signed-off-by: DL6ER --- src/database/message-table.c | 81 +++++++++++++++++++++++++++++++++++- src/database/message-table.h | 1 + src/dnsmasq_interface.c | 43 +++++++++++++------ src/enums.h | 1 + 4 files changed, 113 insertions(+), 13 deletions(-) diff --git a/src/database/message-table.c b/src/database/message-table.c index f6eaef8c..a8f00f7e 100644 --- a/src/database/message-table.c +++ b/src/database/message-table.c @@ -60,6 +60,8 @@ static const char *get_message_type_str(const enum message_type type) return "DISK_EXTENDED"; case CERTIFICATE_DOMAIN_MISMATCH_MESSAGE: return "CERTIFICATE_DOMAIN_MISMATCH"; + case CONNECTION_ERROR_MESSAGE: + return "CONNECTION_ERROR"; case MAX_MESSAGE: default: return "UNKNOWN"; @@ -92,6 +94,8 @@ static enum message_type get_message_type_from_string(const char *typestr) return DISK_MESSAGE_EXTENDED; else if (strcmp(typestr, "CERTIFICATE_DOMAIN_MISMATCH") == 0) return CERTIFICATE_DOMAIN_MISMATCH_MESSAGE; + else if (strcmp(typestr, "CONNECTION_ERROR") == 0) + return CONNECTION_ERROR_MESSAGE; else return MAX_MESSAGE; } @@ -183,6 +187,14 @@ static unsigned char message_blob_types[MAX_MESSAGE][5] = SQLITE_NULL, // not used SQLITE_NULL, // not used SQLITE_NULL // not used + }, + { + // CONNECTION_ERROR_MESSAGE: The message column contains the server address + SQLITE_TEXT, // reason + SQLITE_TEXT, // error message + SQLITE_NULL, // not used + SQLITE_NULL, // not used + SQLITE_NULL // not used } }; // Create message table in the database @@ -710,6 +722,40 @@ static void format_certificate_domain_mismatch(char *plain, const int sizeof_pla free(escaped_domain); } +static void format_connection_error(char *plain, const int sizeof_plain, char *html, const int sizeof_html, + const char *server, const char *reason, const char *error) +{ + if(snprintf(plain, sizeof_plain, "Connection error (%s): %s (%s)", server, reason, error) > sizeof_plain) + log_warn("format_connection_error(): Buffer too small to hold plain message, warning truncated"); + + // Return early if HTML text is not required + if(sizeof_html < 1 || html == NULL) + return; + + char *escaped_reason = escape_html(reason); + char *escaped_error = escape_html(error); + char *escaped_server = escape_html(server); + + // Return early if memory allocation failed + if(escaped_reason == NULL || escaped_error == NULL || escaped_server == NULL) + { + if(escaped_reason != NULL) + free(escaped_reason); + if(escaped_error != NULL) + free(escaped_error); + if(escaped_server != NULL) + free(escaped_server); + return; + } + + if(snprintf(html, sizeof_html, "Connection error (%s): %s (%s)", server, reason, error) > sizeof_html) + log_warn("format_connection_error(): Buffer too small to hold HTML message, warning truncated"); + + free(escaped_reason); + free(escaped_error); + free(escaped_server); +} + int count_messages(const bool filter_dnsmasq_warnings) { int count = 0; @@ -798,7 +844,7 @@ bool format_messages(cJSON *array) // Generate messages char plain[1024] = { 0 }, html[2048] = { 0 }; - const int mtype = get_message_type_from_string(mtypestr); + const enum message_type mtype = get_message_type_from_string(mtypestr); switch(mtype) { case REGEX_MESSAGE: @@ -944,6 +990,23 @@ bool format_messages(cJSON *array) break; } + + case CONNECTION_ERROR_MESSAGE: + { + const char *server = (const char*)sqlite3_column_text(stmt, 3); + const char *reason = (const char*)sqlite3_column_text(stmt, 4); + const char *error = (const char*)sqlite3_column_text(stmt, 5); + + format_connection_error(plain, sizeof(plain), html, sizeof(html), + server, reason, error); + + break; + } + + case MAX_MESSAGE: // Fall through + default: + log_warn("format_messages() - Unknown message type: %s", mtypestr); + break; } // Add the plain message @@ -1183,3 +1246,19 @@ void log_certificate_domain_mismatch(const char *certfile, const char *domain) if(rowid == -1) log_err("log_certificate_domain_mismatch(): Failed to add message to database"); } + +void log_connection_error(const char *server, const char *reason, const char *error) +{ + // Create message + char buf[2048]; + format_connection_error(buf, sizeof(buf), NULL, 0, server, reason, error); + + // Log to FTL.log + log_warn("%s", buf); + + // Log to database + const int rowid = add_message(CONNECTION_ERROR_MESSAGE, server, 2, reason, error); + + if(rowid == -1) + log_err("logg_connection_error(): Failed to add message to database"); +} diff --git a/src/database/message-table.h b/src/database/message-table.h index 14956bf5..d92bbe8d 100644 --- a/src/database/message-table.h +++ b/src/database/message-table.h @@ -29,5 +29,6 @@ void logg_warn_dnsmasq_message(char *message); void log_resource_shortage(const double load, const int nprocs, const int shmem, const int disk, const char *path, const char *msg); void logg_inaccessible_adlist(const int dbindex, const char *address); void log_certificate_domain_mismatch(const char *certfile, const char *domain); +void log_connection_error(const char *server, const char *reason, const char *error); #endif //MESSAGETABLE_H diff --git a/src/dnsmasq_interface.c b/src/dnsmasq_interface.c index 686c3016..1691056a 100644 --- a/src/dnsmasq_interface.c +++ b/src/dnsmasq_interface.c @@ -3514,20 +3514,39 @@ void FTL_connection_error(const char *reason, const union mysockaddr *addr) // Make a private copy of the error const char *error = strerror(errno); - if(config.debug.queries.v.b) - { - const int id = daemon->log_display_id; + // Format the address into a string (if available) + in_port_t port = 0; + char ip[ADDRSTRLEN + 1] = { 0 }; + if(addr != NULL) + mysockaddr_extract_ip_port(addr, ip, &port); - // Format the address into a string (if available) - in_port_t port = 0; - char ip[ADDRSTRLEN] = { 0 }; - if(addr != NULL) - mysockaddr_extract_ip_port(addr, ip, &port); - - // Log to FTL.log - log_debug(DEBUG_QUERIES, "Connection error: %s (%s) for %s#%u (ID %d)", reason, error, ip, port, id); - } + // Log to FTL.log + const int id = daemon->log_display_id; + log_debug(DEBUG_QUERIES, "Connection error (%s#%u, ID %d): %s (%s)", ip, port, id, reason, error); // Log to pihole.log my_syslog(LOG_ERR, "%s: %s", reason, error); + + // Add to Pi-hole diagnostics but do not add messages more often than + // once every five seconds to avoid hammering the database with errors + // on continuously failing connections + static time_t last = 0; + if(time(NULL) - last > 5) + { + last = time(NULL); + char *server = NULL; + if(ip[0] != '\0') + { + const size_t len = strlen(ip) + 6; + server = calloc(len, sizeof(char)); + if(server != NULL) + { + snprintf(server, len, "%s#%u", ip, port); + server[len - 1] = '\0'; + } + } + log_connection_error(server, reason, error); + if(server != NULL) + free(server); + } } \ No newline at end of file diff --git a/src/enums.h b/src/enums.h index dac8ff8d..ab09498a 100644 --- a/src/enums.h +++ b/src/enums.h @@ -273,6 +273,7 @@ enum message_type { INACCESSIBLE_ADLIST_MESSAGE, DISK_MESSAGE_EXTENDED, CERTIFICATE_DOMAIN_MISMATCH_MESSAGE, + CONNECTION_ERROR_MESSAGE, MAX_MESSAGE, } __attribute__ ((packed));