From aa123cacde897f85e0d3a9ee9cb4944c42c5c44b Mon Sep 17 00:00:00 2001 From: DL6ER Date: Mon, 18 Dec 2023 22:00:18 +0100 Subject: [PATCH 1/7] Limit number of clients returned by the /api/history/clients endpoint to 20. This can be overwritten at compile-time and at run-time using the query parameter ?N=... Signed-off-by: DL6ER --- src/api/api.h | 1 + src/api/docs/content/specs/history.yaml | 18 +++++++ src/api/history.c | 68 ++++++++++++++++++++----- src/api/stats.c | 6 +-- src/datastructure.c | 2 +- 5 files changed, 77 insertions(+), 18 deletions(-) diff --git a/src/api/api.h b/src/api/api.h index efde80945..97907b2ca 100644 --- a/src/api/api.h +++ b/src/api/api.h @@ -24,6 +24,7 @@ int api_handler(struct mg_connection *conn, void *ignored); // Statistic methods +int __attribute__((pure)) cmpdesc(const void *a, const void *b); int api_stats_summary(struct ftl_conn *api); int api_stats_query_types(struct ftl_conn *api); int api_stats_upstreams(struct ftl_conn *api); diff --git a/src/api/docs/content/specs/history.yaml b/src/api/docs/content/specs/history.yaml index 88ec6fdd6..eac87aeb2 100644 --- a/src/api/docs/content/specs/history.yaml +++ b/src/api/docs/content/specs/history.yaml @@ -63,6 +63,8 @@ components: operationId: "get_client_metrics" description: | Request data needed to generate the \"Client activity over last 24 hours\" graph + parameters: + - $ref: 'history.yaml#/components/parameters/clients/N' responses: '200': description: OK @@ -180,10 +182,26 @@ components: ip: type: string description: Client IP address + total: + type: integer + description: Total number of queries example: - name: localhost ip: "127.0.0.1" + total: 13428 - name: ip6-localnet ip: "::1" + total: 2100 - name: null ip: "192.168.1.1" + total: 254 + parameters: + clients: + N: + in: query + description: Maximum number of clients to return + name: N + schema: + type: integer + required: false + example: 20 diff --git a/src/api/history.c b/src/api/history.c index 8a33d7cdb..e0a2de217 100644 --- a/src/api/history.c +++ b/src/api/history.c @@ -51,6 +51,7 @@ int api_history(struct ftl_conn *api) JSON_SEND_OBJECT(json); } +#define DEFAULT_MAX_CLIENTS 20 int api_history_clients(struct ftl_conn *api) { // Exit before processing any data if requested via config setting @@ -66,13 +67,32 @@ int api_history_clients(struct ftl_conn *api) JSON_SEND_OBJECT_UNLOCK(json); } + // Get number of clients to return´ + unsigned int Nc = min(counters->clients, DEFAULT_MAX_CLIENTS); + if(api->request->query_string != NULL) + { + // Does the user request a non-default number of clients + get_uint_var(api->request->query_string, "N", &Nc); + } + + // Lock shared memory lock_shm(); // Get clients which the user doesn't want to see // if skipclient[i] == true then this client should be hidden from // returned data. We initialize it with false bool *skipclient = calloc(counters->clients, sizeof(bool)); + int *temparray = calloc(2*counters->clients, sizeof(int)); + if(skipclient == NULL || temparray == NULL) + { + unlock_shm(); + return send_json_error(api, 500, "internal_error", + "Failed to allocate memory for client history", NULL); + } + // Check if the user wants to exclude any clients, this code path is + // only taken if the user has configured the web interface to exclude + // clients (it will most often be skipped) unsigned int excludeClients = cJSON_GetArraySize(config.webserver.api.excludeClients.v.json); if(excludeClients > 0) { @@ -93,17 +113,37 @@ int api_history_clients(struct ftl_conn *api) } } - // Also skip clients included in others (in alias-clients) + // Skip clients included in others (in alias-clients) for(int clientID = 0; clientID < counters->clients; clientID++) { // Get client pointer const clientsData* client = getClient(clientID, true); if(client == NULL) continue; + + // Check if this client should be skipped if(!client->flags.aliasclient && client->aliasclient_id > -1) skipclient[clientID] = true; } + // Get MAX_CLIENTS clients with the highest number of queries + for(int clientID = 0; clientID < counters->clients; clientID++) + { + // Get client pointer + const clientsData* client = getClient(clientID, true); + + // Skip invalid clients and also those managed by alias clients + if(client == NULL || skipclient[clientID]) + continue; + + // Store clientID and number of queries in temporary array + temparray[2*clientID + 0] = clientID; + temparray[2*clientID + 1] = client->count; + } + + // Sort temporary array + qsort(temparray, counters->clients, sizeof(int[2]), cmpdesc); + // Main return loop cJSON *history = JSON_NEW_ARRAY(); for(unsigned int slot = 0; slot < OVERTIME_SLOTS; slot++) @@ -113,21 +153,17 @@ int api_history_clients(struct ftl_conn *api) // Loop over clients to generate output to be sent to the client cJSON *data = JSON_NEW_ARRAY(); - for(int clientID = 0; clientID < counters->clients; clientID++) + for(unsigned int id = 0; id < Nc; id++) { - if(skipclient[clientID]) - continue; - // Get client pointer + const int clientID = temparray[2*id + 0]; const clientsData* client = getClient(clientID, true); - // Skip invalid clients and also those managed by alias clients - if(client == NULL || client->aliasclient_id >= 0) + // Skip invalid (recycled) clients + if(client == NULL) continue; - const int thisclient = client->overTime[slot]; - - JSON_ADD_NUMBER_TO_ARRAY(data, thisclient); + JSON_ADD_NUMBER_TO_ARRAY(data, client->overTime[slot]); } JSON_ADD_ITEM_TO_OBJECT(item, "data", data); JSON_ADD_ITEM_TO_ARRAY(history, item); @@ -137,22 +173,25 @@ int api_history_clients(struct ftl_conn *api) // Loop over clients to generate output to be sent to the client cJSON *clients = JSON_NEW_ARRAY(); - for(int clientID = 0; clientID < counters->clients; clientID++) + for(unsigned int id = 0; id < Nc; id++) { - if(skipclient[clientID]) - continue; - // Get client pointer + const int clientID = temparray[2*id + 0]; const clientsData* client = getClient(clientID, true); + + // Skip invalid (recycled) clients if(client == NULL) continue; + // Get client name and IP address const char *client_ip = getstr(client->ippos); const char *client_name = client->namepos != 0 ? getstr(client->namepos) : NULL; + // Create JSON object for this client cJSON *item = JSON_NEW_OBJECT(); JSON_REF_STR_IN_OBJECT(item, "name", client_name); JSON_REF_STR_IN_OBJECT(item, "ip", client_ip); + JSON_ADD_NUMBER_TO_OBJECT(item, "total", client->count); JSON_ADD_ITEM_TO_ARRAY(clients, item); } @@ -164,6 +203,7 @@ int api_history_clients(struct ftl_conn *api) // Free memory free(skipclient); + free(temparray); JSON_ADD_ITEM_TO_OBJECT(json, "clients", clients); JSON_SEND_OBJECT(json); diff --git a/src/api/stats.c b/src/api/stats.c index 545b80d70..20c1651cd 100644 --- a/src/api/stats.c +++ b/src/api/stats.c @@ -42,7 +42,7 @@ static int __attribute__((pure)) cmpasc(const void *a, const void *b) } */ // qsort subroutine, sort DESC -static int __attribute__((pure)) cmpdesc(const void *a, const void *b) +int __attribute__((pure)) cmpdesc(const void *a, const void *b) { const int *elem1 = (int*)a; const int *elem2 = (int*)b; @@ -139,7 +139,7 @@ int api_stats_top_domains(struct ftl_conn *api) { int count = 10; const int domains = counters->domains; - int *temparray = calloc(2*domains, sizeof(int*)); + int *temparray = calloc(2*domains, sizeof(int)); if(temparray == NULL) { log_err("Memory allocation failed in %s()", __FUNCTION__); @@ -282,7 +282,7 @@ int api_stats_top_clients(struct ftl_conn *api) { int count = 10; const int clients = counters->clients; - int *temparray = calloc(2*clients, sizeof(int*)); + int *temparray = calloc(2*clients, sizeof(int)); if(temparray == NULL) { log_err("Memory allocation failed in api_stats_top_clients()"); diff --git a/src/datastructure.c b/src/datastructure.c index 8cf2e5aef..8ceed7b46 100644 --- a/src/datastructure.c +++ b/src/datastructure.c @@ -314,7 +314,7 @@ int _findClientID(const char *clientIP, const bool count, const bool aliasclient // Set all MAC address bytes to zero client->hwlen = -1; memset(client->hwaddr, 0, sizeof(client->hwaddr)); - // This may be a alias-client, the ID is set elsewhere + // This may be an alias-client, the ID is set elsewhere client->flags.aliasclient = aliasclient; client->aliasclient_id = -1; From fa6a8b38e968ab9922ddebb3f59de7df4d246ac5 Mon Sep 17 00:00:00 2001 From: DL6ER Date: Mon, 18 Dec 2023 22:47:18 +0100 Subject: [PATCH 2/7] Limit the number of clients to return to the number of clients to avoid possible overflows for very large N and add N=0 as special option to return all clients (even if the user does not know how many there are) Signed-off-by: DL6ER --- src/api/docs/content/specs/history.yaml | 2 +- src/api/history.c | 6 ++++++ 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/src/api/docs/content/specs/history.yaml b/src/api/docs/content/specs/history.yaml index eac87aeb2..4d4766658 100644 --- a/src/api/docs/content/specs/history.yaml +++ b/src/api/docs/content/specs/history.yaml @@ -199,7 +199,7 @@ components: clients: N: in: query - description: Maximum number of clients to return + description: Maximum number of clients to return, setting this to 0 will return all clients name: N schema: type: integer diff --git a/src/api/history.c b/src/api/history.c index e0a2de217..51ff712ac 100644 --- a/src/api/history.c +++ b/src/api/history.c @@ -73,6 +73,12 @@ int api_history_clients(struct ftl_conn *api) { // Does the user request a non-default number of clients get_uint_var(api->request->query_string, "N", &Nc); + + // Limit the number of clients to return to the number of + // clients to avoid possible overflows for very large N + // Also allow N=0 to return all clients + if((int)Nc > counters->clients || Nc == 0) + Nc = counters->clients; } // Lock shared memory From b32d5adbdbd545a5b092dd828c0183e65dd413f6 Mon Sep 17 00:00:00 2001 From: DL6ER Date: Wed, 20 Dec 2023 16:57:44 +0100 Subject: [PATCH 3/7] Add special client "other clients" summing all client activity that has not been included due to more clients being active than being returned (due to limited N) Signed-off-by: DL6ER --- src/api/docs/content/specs/history.yaml | 18 ++++++++++++- src/api/history.c | 34 ++++++++++++++++++++++--- 2 files changed, 47 insertions(+), 5 deletions(-) diff --git a/src/api/docs/content/specs/history.yaml b/src/api/docs/content/specs/history.yaml index 4d4766658..099b8caa8 100644 --- a/src/api/docs/content/specs/history.yaml +++ b/src/api/docs/content/specs/history.yaml @@ -62,7 +62,13 @@ components: - Metrics operationId: "get_client_metrics" description: | - Request data needed to generate the \"Client activity over last 24 hours\" graph + Request data needed to generate the \"Client activity over last 24 hours\" graph. + This endpoint returns the top N clients, sorted by total number of queries within 24 hours. If N is set to 0, all clients will be returned. + The client name is only available if the client's IP address can be resolved to a hostname. + + The last client returned is a special client that contains the total number of queries that were not sent by any of the other shown clients , i.e. queries that were sent by clients that are not in the top N. This client is always present, even if it has 0 queries and can be identified by the special name "other clients" (mind the space in the hostname) and the IP address "0.0.0.0". + + Note that, due to privacy settings, the returned data may also be empty. parameters: - $ref: 'history.yaml#/components/parameters/clients/N' responses: @@ -164,11 +170,15 @@ components: - 12 - 65 - 67 + - 9 + - 5 - timestamp: 1511820500.583821 data: - 1 - 35 - 63 + - 20 + - 9 clients: type: array description: Data array @@ -195,6 +205,12 @@ components: - name: null ip: "192.168.1.1" total: 254 + - name: "pi.hole" + ip: "::" + total: 29 + - name: "other clients" + ip: "0.0.0.0" + total: 14 parameters: clients: N: diff --git a/src/api/history.c b/src/api/history.c index 51ff712ac..dfc1014a4 100644 --- a/src/api/history.c +++ b/src/api/history.c @@ -138,8 +138,8 @@ int api_history_clients(struct ftl_conn *api) // Get client pointer const clientsData* client = getClient(clientID, true); - // Skip invalid clients and also those managed by alias clients - if(client == NULL || skipclient[clientID]) + // Skip invalid clients + if(client == NULL) continue; // Store clientID and number of queries in temporary array @@ -152,6 +152,7 @@ int api_history_clients(struct ftl_conn *api) // Main return loop cJSON *history = JSON_NEW_ARRAY(); + int others_total = 0; for(unsigned int slot = 0; slot < OVERTIME_SLOTS; slot++) { cJSON *item = JSON_NEW_OBJECT(); @@ -159,7 +160,8 @@ int api_history_clients(struct ftl_conn *api) // Loop over clients to generate output to be sent to the client cJSON *data = JSON_NEW_ARRAY(); - for(unsigned int id = 0; id < Nc; id++) + int others = 0; + for(int id = 0; id < counters->clients; id++) { // Get client pointer const int clientID = temparray[2*id + 0]; @@ -169,8 +171,20 @@ int api_history_clients(struct ftl_conn *api) if(client == NULL) continue; + // Skip clients which should be hidden and add them to the "others" counter. + // Also skip clients when we reached the maximum number of clients to return + if(skipclient[clientID] || id >= (int)Nc) + { + others += client->overTime[slot]; + continue; + } + JSON_ADD_NUMBER_TO_ARRAY(data, client->overTime[slot]); } + // Add others as last element in the array + others_total += others; + JSON_ADD_NUMBER_TO_ARRAY(data, others); + JSON_ADD_ITEM_TO_OBJECT(item, "data", data); JSON_ADD_ITEM_TO_ARRAY(history, item); } @@ -179,7 +193,7 @@ int api_history_clients(struct ftl_conn *api) // Loop over clients to generate output to be sent to the client cJSON *clients = JSON_NEW_ARRAY(); - for(unsigned int id = 0; id < Nc; id++) + for(int id = 0; id < counters->clients; id++) { // Get client pointer const int clientID = temparray[2*id + 0]; @@ -189,6 +203,11 @@ int api_history_clients(struct ftl_conn *api) if(client == NULL) continue; + // Skip clients which should be hidden. Also skip clients when + // we reached the maximum number of clients to return + if(skipclient[clientID] || id >= (int)Nc) + continue; + // Get client name and IP address const char *client_ip = getstr(client->ippos); const char *client_name = client->namepos != 0 ? getstr(client->namepos) : NULL; @@ -201,6 +220,13 @@ int api_history_clients(struct ftl_conn *api) JSON_ADD_ITEM_TO_ARRAY(clients, item); } + // Add "others" client + cJSON *item = JSON_NEW_OBJECT(); + JSON_REF_STR_IN_OBJECT(item, "name", "other clients"); + JSON_REF_STR_IN_OBJECT(item, "ip", "0.0.0.0"); + JSON_ADD_NUMBER_TO_OBJECT(item, "total", others_total); + JSON_ADD_ITEM_TO_ARRAY(clients, item); + // Unlock already here to avoid keeping the lock during JSON generation // This is safe because we don't access any shared memory after this // point and all strings in the JSON are references to idempotent shared From fb1160df7156657039f4e320a6f358a76ca0fa63 Mon Sep 17 00:00:00 2001 From: DL6ER Date: Wed, 20 Dec 2023 16:59:12 +0100 Subject: [PATCH 4/7] Reduce default N from 20 to 10 as experiments suggest that even 10 is at the border of being still readable Signed-off-by: DL6ER --- src/api/history.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/api/history.c b/src/api/history.c index dfc1014a4..21b2b21e1 100644 --- a/src/api/history.c +++ b/src/api/history.c @@ -51,7 +51,7 @@ int api_history(struct ftl_conn *api) JSON_SEND_OBJECT(json); } -#define DEFAULT_MAX_CLIENTS 20 +#define DEFAULT_MAX_CLIENTS 10 int api_history_clients(struct ftl_conn *api) { // Exit before processing any data if requested via config setting From 03466460c35efbea43b861fe4ff9b1256575feb4 Mon Sep 17 00:00:00 2001 From: DL6ER Date: Sat, 13 Jan 2024 10:39:22 +0100 Subject: [PATCH 5/7] Add webserver.api.maxClients setting to set default number of clients to be returned for the client activity graph. This setting can be overwritten at run-time Signed-off-by: DL6ER --- src/api/history.c | 3 +-- src/config/config.c | 5 +++++ src/config/config.h | 1 + test/pihole.toml | 5 +++++ 4 files changed, 12 insertions(+), 2 deletions(-) diff --git a/src/api/history.c b/src/api/history.c index 21b2b21e1..2a2df6d41 100644 --- a/src/api/history.c +++ b/src/api/history.c @@ -51,7 +51,6 @@ int api_history(struct ftl_conn *api) JSON_SEND_OBJECT(json); } -#define DEFAULT_MAX_CLIENTS 10 int api_history_clients(struct ftl_conn *api) { // Exit before processing any data if requested via config setting @@ -68,7 +67,7 @@ int api_history_clients(struct ftl_conn *api) } // Get number of clients to return´ - unsigned int Nc = min(counters->clients, DEFAULT_MAX_CLIENTS); + unsigned int Nc = min(counters->clients, config.webserver.api.maxClients.v.u16); if(api->request->query_string != NULL) { // Does the user request a non-default number of clients diff --git a/src/config/config.c b/src/config/config.c index 3a1d38634..dfaf26ff7 100644 --- a/src/config/config.c +++ b/src/config/config.c @@ -972,6 +972,11 @@ void initConfig(struct config *conf) conf->webserver.api.maxHistory.t = CONF_UINT; conf->webserver.api.maxHistory.d.ui = MAXLOGAGE*3600; + conf->webserver.api.maxClients.k = "webserver.api.maxClients"; + conf->webserver.api.maxClients.h = "Up to how many clients should be returned in the activity graph endpoint (/api/history/clients)?\n This setting can be overwritten at run-time using the parameter N"; + conf->webserver.api.maxClients.t = CONF_UINT16; + conf->webserver.api.maxClients.d.u16 = 10; + conf->webserver.api.allow_destructive.k = "webserver.api.allow_destructive"; conf->webserver.api.allow_destructive.h = "Allow destructive API calls (e.g. deleting all queries, powering off the system, ...)"; conf->webserver.api.allow_destructive.t = CONF_BOOL; diff --git a/src/config/config.h b/src/config/config.h index f6f4baf08..13bce41a2 100644 --- a/src/config/config.h +++ b/src/config/config.h @@ -245,6 +245,7 @@ struct config { struct conf_item excludeClients; struct conf_item excludeDomains; struct conf_item maxHistory; + struct conf_item maxClients; struct conf_item allow_destructive; struct { struct conf_item limit; diff --git a/test/pihole.toml b/test/pihole.toml index dbce042af..79f99bdff 100644 --- a/test/pihole.toml +++ b/test/pihole.toml @@ -693,6 +693,11 @@ # 86400) maxHistory = 86400 + # Up to how many clients should be returned in the activity graph endpoint + # (/api/history/clients)? + # This setting can be overwritten at run-time using the parameter N + maxClients = 10 + # Allow destructive API calls (e.g. deleting all queries, powering off the system, ...) allow_destructive = true From 4e5521f40098dd710c62edc60fb2baf85f00d48c Mon Sep 17 00:00:00 2001 From: DL6ER Date: Sat, 13 Jan 2024 10:50:18 +0100 Subject: [PATCH 6/7] Add new config option to API Signed-off-by: DL6ER --- src/api/docs/content/specs/config.yaml | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/api/docs/content/specs/config.yaml b/src/api/docs/content/specs/config.yaml index 91ea040b8..50871941a 100644 --- a/src/api/docs/content/specs/config.yaml +++ b/src/api/docs/content/specs/config.yaml @@ -426,6 +426,8 @@ components: type: string maxHistory: type: integer + maxClients: + type: integer allow_destructive: type: boolean temp: @@ -700,6 +702,7 @@ components: excludeClients: [ '1.2.3.4', 'localhost', 'fe80::345' ] excludeDomains: [ 'google.de', 'pi-hole.net' ] maxHistory: 86400 + maxClients: 10 allow_destructive: true temp: limit: 60.0 From 3c58e3089b7409bbee08bcdb867db062f16c70ae Mon Sep 17 00:00:00 2001 From: DL6ER Date: Tue, 16 Jan 2024 21:51:55 +0100 Subject: [PATCH 7/7] Change order of objects in documentation (small nit pick) Signed-off-by: DL6ER --- src/api/docs/content/specs/history.yaml | 58 ++++++++++++------------- 1 file changed, 29 insertions(+), 29 deletions(-) diff --git a/src/api/docs/content/specs/history.yaml b/src/api/docs/content/specs/history.yaml index 099b8caa8..e6ea85d52 100644 --- a/src/api/docs/content/specs/history.yaml +++ b/src/api/docs/content/specs/history.yaml @@ -150,35 +150,6 @@ components: client_history: type: object properties: - history: - type: array - description: Data array - items: - type: object - properties: - timestamp: - type: number - description: Timestamp - data: - type: array - description: Data corresponding to the individual clients - items: - type: integer - example: - - timestamp: 1511819900.539157 - data: - - 12 - - 65 - - 67 - - 9 - - 5 - - timestamp: 1511820500.583821 - data: - - 1 - - 35 - - 63 - - 20 - - 9 clients: type: array description: Data array @@ -211,6 +182,35 @@ components: - name: "other clients" ip: "0.0.0.0" total: 14 + history: + type: array + description: Data array + items: + type: object + properties: + timestamp: + type: number + description: Timestamp + data: + type: array + description: Data corresponding to the individual clients + items: + type: integer + example: + - timestamp: 1511819900.539157 + data: + - 12 + - 65 + - 67 + - 9 + - 5 + - timestamp: 1511820500.583821 + data: + - 1 + - 35 + - 63 + - 20 + - 9 parameters: clients: N: