Skip to content

Commit

Permalink
Merge pull request #2062 from pi-hole/new/edns_reasoning
Browse files Browse the repository at this point in the history
Improve EDNS handling
  • Loading branch information
DL6ER authored Sep 14, 2024
2 parents 975e7cf + 3e090f5 commit 031c466
Show file tree
Hide file tree
Showing 23 changed files with 667 additions and 208 deletions.
15 changes: 15 additions & 0 deletions src/api/config.c
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,8 @@ cJSON *addJSONConfValue(const enum conf_type conf_type, union conf_value *val)
return cJSON_CreateStringReference(get_web_theme_str(val->web_theme));
case CONF_ENUM_TEMP_UNIT:
return cJSON_CreateStringReference(get_temp_unit_str(val->temp_unit));
case CONF_ENUM_BLOCKING_EDNS_MODE:
return cJSON_CreateStringReference(get_edns_mode_str(val->edns_mode));
case CONF_STRUCT_IN_ADDR:
{
// Special case 0.0.0.0 -> return empty string
Expand Down Expand Up @@ -391,6 +393,19 @@ static const char *getJSONvalue(struct conf_item *conf_item, cJSON *elem, struct
log_debug(DEBUG_CONFIG, "%s = %d", conf_item->k, conf_item->v.temp_unit);
break;
}
case CONF_ENUM_BLOCKING_EDNS_MODE:
{
// Check type
if(!cJSON_IsString(elem))
return "not of type string";
const int edns_mode = get_edns_mode_val(elem->valuestring);
if(edns_mode == -1)
return "invalid option";
// Set item
conf_item->v.edns_mode = edns_mode;
log_debug(DEBUG_CONFIG, "%s = %d", conf_item->k, conf_item->v.edns_mode);
break;
}
case CONF_ENUM_PRIVACY_LEVEL:
{
// Check type
Expand Down
3 changes: 3 additions & 0 deletions src/api/docs/content/specs/config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -252,6 +252,8 @@ components:
type: boolean
mode:
type: string
edns:
type: string
specialDomains:
type: object
properties:
Expand Down Expand Up @@ -671,6 +673,7 @@ components:
blocking:
active: true
mode: 'NULL'
edns: 'NONE'
specialDomains:
mozillaCanary: true
iCloudPrivateRelay: true
Expand Down
4 changes: 4 additions & 0 deletions src/api/docs/content/specs/stats.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -486,6 +486,10 @@ components:
type: integer
description: Type CACHE_STALE queries
example: 0
EXTERNAL_BLOCKED_EDE15:
type: integer
description: Type EXTERNAL_BLOCKED_EDE15 queries
example: 0
replies:
type: object
description: Number of individual replies
Expand Down
1 change: 1 addition & 0 deletions src/api/queries.c
Original file line number Diff line number Diff line change
Expand Up @@ -1015,6 +1015,7 @@ int api_queries(struct ftl_conn *api)
case QUERY_EXTERNAL_BLOCKED_IP:
case QUERY_EXTERNAL_BLOCKED_NULL:
case QUERY_EXTERNAL_BLOCKED_NXRA:
case QUERY_EXTERNAL_BLOCKED_EDE15:
case QUERY_RETRIED:
case QUERY_RETRIED_DNSSEC:
case QUERY_IN_PROGRESS:
Expand Down
15 changes: 15 additions & 0 deletions src/config/cli.c
Original file line number Diff line number Diff line change
Expand Up @@ -306,6 +306,21 @@ static bool readStringValue(struct conf_item *conf_item, const char *value, stru
}
break;
}
case CONF_ENUM_BLOCKING_EDNS_MODE:
{
const int edns_mode = get_edns_mode_val(value);
if(edns_mode != -1)
conf_item->v.edns_mode = edns_mode;
else
{
char *allowed = NULL;
CONFIG_ITEM_ARRAY(conf_item->a, allowed);
log_err("Config setting %s is invalid, allowed options are: %s", conf_item->k, allowed);
free(allowed);
return false;
}
break;
}
case CONF_STRUCT_IN_ADDR:
{
struct in_addr addr4 = { 0 };
Expand Down
19 changes: 19 additions & 0 deletions src/config/config.c
Original file line number Diff line number Diff line change
Expand Up @@ -278,6 +278,7 @@ void duplicate_config(struct config *dst, struct config *src)
case CONF_ENUM_LISTENING_MODE:
case CONF_ENUM_WEB_THEME:
case CONF_ENUM_TEMP_UNIT:
case CONF_ENUM_BLOCKING_EDNS_MODE:
case CONF_STRUCT_IN_ADDR:
case CONF_STRUCT_IN6_ADDR:
case CONF_ALL_DEBUG_BOOL:
Expand Down Expand Up @@ -314,6 +315,7 @@ bool compare_config_item(const enum conf_type t, const union conf_value *val1, c
case CONF_ENUM_LISTENING_MODE:
case CONF_ENUM_WEB_THEME:
case CONF_ENUM_TEMP_UNIT:
case CONF_ENUM_BLOCKING_EDNS_MODE:
case CONF_STRUCT_IN_ADDR:
case CONF_STRUCT_IN6_ADDR:
case CONF_ALL_DEBUG_BOOL:
Expand Down Expand Up @@ -370,6 +372,7 @@ void free_config(struct config *conf)
case CONF_ENUM_LISTENING_MODE:
case CONF_ENUM_WEB_THEME:
case CONF_ENUM_TEMP_UNIT:
case CONF_ENUM_BLOCKING_EDNS_MODE:
case CONF_STRUCT_IN_ADDR:
case CONF_STRUCT_IN6_ADDR:
case CONF_ALL_DEBUG_BOOL:
Expand Down Expand Up @@ -620,6 +623,21 @@ static void initConfig(struct config *conf)
conf->dns.blocking.mode.d.blocking_mode = MODE_NULL;
conf->dns.blocking.mode.c = validate_stub; // Only type-based checking

conf->dns.blocking.edns.k = "dns.blocking.edns";
conf->dns.blocking.edns.h = "Should FTL enrich blocked replies with EDNS0 information?";
{
struct enum_options blocking_edns[] =
{
{ get_edns_mode_str(EDNS_MODE_NONE), "In NONE mode, no additional EDNS information is added to blocked queries" },
{ get_edns_mode_str(EDNS_MODE_CODE), "In CODE mode, blocked queries will be enriched with EDNS info-code BLOCKED (15)" },
{ get_edns_mode_str(EDNS_MODE_TEXT), "In TEXT mode, blocked queries will be enriched with EDNS info-code BLOCKED (15) and a text message describing the reason for the block" }
};
CONFIG_ADD_ENUM_OPTIONS(conf->dns.blocking.edns.a, blocking_edns);
}
conf->dns.blocking.edns.t = CONF_ENUM_BLOCKING_EDNS_MODE;
conf->dns.blocking.edns.d.edns_mode = EDNS_MODE_TEXT;
conf->dns.blocking.edns.c = validate_stub; // Only type-based checking

conf->dns.revServers.k = "dns.revServers";
conf->dns.revServers.h = "Reverse server (former also called \"conditional forwarding\") feature\n Array of reverse servers each one in one of the following forms: \"<enabled>,<ip-address>[/<prefix-len>],<server>[#<port>],<domain>\"\n\n Individual components:\n\n <enabled>: either \"true\" or \"false\"\n\n <ip-address>[/<prefix-len>]: Address range for the reverse server feature in CIDR notation. If the prefix length is omitted, either 32 (IPv4) or 128 (IPv6) are substituted (exact address match). This is almost certainly not what you want here.\n Example: \"192.168.0.0/24\" for the range 192.168.0.1 - 192.168.0.255\n\n <server>[#<port>]: Target server to be used for the reverse server feature\n Example: \"192.168.0.1#53\"\n\n <domain>: Domain used for the reverse server feature (e.g., \"fritz.box\")\n Example: \"fritz.box\"";
conf->dns.revServers.a = cJSON_CreateStringReference("array of reverse servers each one in one of the following forms: \"<enabled>,<ip-address>[/<prefix-len>],<server>[#<port>],<domain>\", e.g., \"true,192.168.0.0/24,192.168.0.1,fritz.box\"");
Expand Down Expand Up @@ -1772,6 +1790,7 @@ const char * __attribute__ ((const)) get_conf_type_str(const enum conf_type type
case CONF_ENUM_LISTENING_MODE:
case CONF_ENUM_WEB_THEME:
case CONF_ENUM_TEMP_UNIT:
case CONF_ENUM_BLOCKING_EDNS_MODE:
return "enum (string)";
case CONF_ENUM_PRIVACY_LEVEL:
return "enum (unsigned integer)";
Expand Down
3 changes: 3 additions & 0 deletions src/config/config.h
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@ union conf_value {
enum listening_mode listeningMode; // enum listening_mode value
enum web_theme web_theme; // enum web_theme value
enum temp_unit temp_unit; // enum temp_unit value
enum edns_mode edns_mode; // enum edns_mode value
struct in_addr in_addr; // struct in_addr value
struct in6_addr in6_addr; // struct in6_addr value
cJSON *json; // cJSON * value
Expand All @@ -83,6 +84,7 @@ enum conf_type {
CONF_ENUM_PRIVACY_LEVEL,
CONF_ENUM_LISTENING_MODE,
CONF_ENUM_WEB_THEME,
CONF_ENUM_BLOCKING_EDNS_MODE,
CONF_ENUM_TEMP_UNIT,
CONF_STRUCT_IN_ADDR,
CONF_STRUCT_IN6_ADDR,
Expand Down Expand Up @@ -155,6 +157,7 @@ struct config {
struct {
struct conf_item active;
struct conf_item mode;
struct conf_item edns;
} blocking;
struct {
struct conf_item mozillaCanary;
Expand Down
19 changes: 19 additions & 0 deletions src/config/env.c
Original file line number Diff line number Diff line change
Expand Up @@ -499,6 +499,25 @@ bool __attribute__((nonnull(1,2,3))) readEnvValue(struct conf_item *conf_item, s
}
break;
}
case CONF_ENUM_BLOCKING_EDNS_MODE:
{
const int edns_mode = get_edns_mode_val(envvar);
if(edns_mode != -1)
{
conf_item->v.edns_mode = edns_mode;
item->valid = true;
}
else
{

item->error = "not an allowed option";
item->allowed = conf_item->h;
log_warn("ENV %s is %s, allowed options are: %s",
conf_item->e, item->error, item->allowed);
item->valid = false;
}
break;
}
case CONF_ENUM_PRIVACY_LEVEL:
{
int val = 0;
Expand Down
19 changes: 19 additions & 0 deletions src/config/toml_helper.c
Original file line number Diff line number Diff line change
Expand Up @@ -362,6 +362,9 @@ void writeTOMLvalue(FILE * fp, const int indent, const enum conf_type t, union c
case CONF_ENUM_TEMP_UNIT:
printTOMLstring(fp, get_temp_unit_str(v->temp_unit), toml);
break;
case CONF_ENUM_BLOCKING_EDNS_MODE:
printTOMLstring(fp, get_edns_mode_str(v->edns_mode), toml);
break;
case CONF_STRUCT_IN_ADDR:
{
// Special case: 0.0.0.0 -> return empty string
Expand Down Expand Up @@ -653,6 +656,22 @@ void readTOMLvalue(struct conf_item *conf_item, const char* key, toml_table_t *t
log_debug(DEBUG_CONFIG, "%s DOES NOT EXIST or is not a valid string", conf_item->k);
break;
}
case CONF_ENUM_BLOCKING_EDNS_MODE:
{
toml_datum_t val = toml_string_in(toml, key);
if(val.ok)
{
const int edns_mode = get_edns_mode_val(val.u.s);
free(val.u.s);
if(edns_mode != -1)
conf_item->v.edns_mode = edns_mode;
else
log_warn("Config setting %s is invalid, allowed options are: %s", conf_item->k, conf_item->h);
}
else
log_debug(DEBUG_CONFIG, "%s DOES NOT EXIST or is not a valid string", conf_item->k);
break;
}
case CONF_ENUM_PRIVACY_LEVEL:
{
const toml_datum_t val = toml_int_in(toml, key);
Expand Down
7 changes: 4 additions & 3 deletions src/database/query-table.c
Original file line number Diff line number Diff line change
Expand Up @@ -1256,9 +1256,10 @@ void DB_read_queries(void)
case QUERY_GRAVITY: // Blocked by gravity
case QUERY_REGEX: // Blocked by regex denylist
case QUERY_DENYLIST: // Blocked by exact denylist
case QUERY_EXTERNAL_BLOCKED_IP: // Blocked by external provider
case QUERY_EXTERNAL_BLOCKED_NULL: // Blocked by external provider
case QUERY_EXTERNAL_BLOCKED_NXRA: // Blocked by external provider
case QUERY_EXTERNAL_BLOCKED_IP: // Blocked upstream
case QUERY_EXTERNAL_BLOCKED_NULL: // Blocked upstream
case QUERY_EXTERNAL_BLOCKED_NXRA: // Blocked upstream
case QUERY_EXTERNAL_BLOCKED_EDE15: // Blocked upstream
case QUERY_GRAVITY_CNAME: // Blocked by gravity (inside CNAME path)
case QUERY_REGEX_CNAME: // Blocked by regex denylist (inside CNAME path)
case QUERY_DENYLIST_CNAME: // Blocked by exact denylist (inside CNAME path)
Expand Down
93 changes: 91 additions & 2 deletions src/datastructure.c
Original file line number Diff line number Diff line change
Expand Up @@ -435,7 +435,8 @@ int _findCacheID(const int domainID, const int clientID, const enum query_type q

// Initialize cache entry
dns_cache->magic = MAGICBYTE;
dns_cache->blocking_status = UNKNOWN_BLOCKED;
dns_cache->blocking_status = QUERY_UNKNOWN;
dns_cache->expires = 0;
dns_cache->domainID = domainID;
dns_cache->clientID = clientID;
dns_cache->query_type = query_type;
Expand Down Expand Up @@ -570,7 +571,9 @@ void FTL_reset_per_client_domain_data(void)
continue;

// Reset blocking status
dns_cache->blocking_status = UNKNOWN_BLOCKED;
dns_cache->blocking_status = QUERY_UNKNOWN;
// Reset expiry
dns_cache->expires = 0;
// Reset domainlist ID
dns_cache->list_id = -1;
}
Expand Down Expand Up @@ -700,6 +703,8 @@ const char * __attribute__ ((const)) get_query_status_str(const enum query_statu
return "SPECIAL_DOMAIN";
case QUERY_CACHE_STALE:
return "CACHE_STALE";
case QUERY_EXTERNAL_BLOCKED_EDE15:
return "EXTERNAL_BLOCKED_EDE15";
case QUERY_STATUS_MAX:
default:
return "INVALID";
Expand Down Expand Up @@ -872,6 +877,7 @@ bool __attribute__ ((const)) is_blocked(const enum query_status status)
case QUERY_EXTERNAL_BLOCKED_IP:
case QUERY_EXTERNAL_BLOCKED_NULL:
case QUERY_EXTERNAL_BLOCKED_NXRA:
case QUERY_EXTERNAL_BLOCKED_EDE15:
case QUERY_GRAVITY_CNAME:
case QUERY_REGEX_CNAME:
case QUERY_DENYLIST_CNAME:
Expand Down Expand Up @@ -969,6 +975,7 @@ bool __attribute__ ((const)) is_cached(const enum query_status status)
case QUERY_EXTERNAL_BLOCKED_IP:
case QUERY_EXTERNAL_BLOCKED_NULL:
case QUERY_EXTERNAL_BLOCKED_NXRA:
case QUERY_EXTERNAL_BLOCKED_EDE15:
case QUERY_GRAVITY_CNAME:
case QUERY_REGEX_CNAME:
case QUERY_DENYLIST_CNAME:
Expand Down Expand Up @@ -1019,6 +1026,8 @@ static const char* __attribute__ ((const)) query_status_str(const enum query_sta
return "SPECIAL_DOMAIN";
case QUERY_CACHE_STALE:
return "CACHE_STALE";
case QUERY_EXTERNAL_BLOCKED_EDE15:
return "EXTERNAL_BLOCKED_EDE15";
case QUERY_STATUS_MAX:
return NULL;
}
Expand Down Expand Up @@ -1063,6 +1072,59 @@ void _query_set_status(queriesData *query, const enum query_status new_status, c
return;
}

// Memorize this in the DNS cache if blocked due to the response
// We do not cache intermittent statuses as they are subject to change
if(!init &&
new_status != QUERY_UNKNOWN &&
new_status != QUERY_DBBUSY &&
new_status != QUERY_IN_PROGRESS &&
new_status != QUERY_RETRIED &&
new_status != QUERY_RETRIED_DNSSEC)
{
const int cacheID = findCacheID(query->domainID, query->clientID, query->type, true);
DNSCacheData *dns_cache = getDNSCache(cacheID, true);
if(dns_cache != NULL && dns_cache->blocking_status != new_status)
{
// Memorize blocking status DNS cache for the domain/client combination
dns_cache->blocking_status = new_status;

// Set expiration time for this cache entry (if applicable)
// We set this only if not already set to avoid extending the TTL of an
// existing entry
if(config.dns.cache.upstreamBlockedTTL.v.ui > 0 &&
dns_cache->expires == 0 &&
(new_status == QUERY_EXTERNAL_BLOCKED_NXRA ||
new_status == QUERY_EXTERNAL_BLOCKED_NULL ||
new_status == QUERY_EXTERNAL_BLOCKED_IP ||
new_status == QUERY_EXTERNAL_BLOCKED_EDE15))
{
// Set expiration time for this cache entry
dns_cache->expires = time(NULL) + config.dns.cache.upstreamBlockedTTL.v.ui;
}

if(config.debug.queries.v.b)
{
// Debug logging
const char *qtype = get_query_type_str(dns_cache->query_type, NULL, NULL);
const char *domain = getDomainString(query);
const char *clientstr = getClientIPString(query);
const char *statusstr = get_query_status_str(new_status);

if(dns_cache->expires > 0)
{
log_debug(DEBUG_QUERIES, "DNS cache: %s/%s/%s -> %s, expires in %lis",
qtype, clientstr, domain, statusstr,
(long)(dns_cache->expires - time(NULL)));
}
else
{
log_debug(DEBUG_QUERIES, "DNS cache: %s/%s/%s -> %s, no expiry",
qtype, clientstr, domain, statusstr);
}
}
}
}

// else: update global counters, ...
if(!init)
{
Expand Down Expand Up @@ -1217,3 +1279,30 @@ int __attribute__ ((pure)) get_temp_unit_val(const char *temp_unit)
// Invalid value
return -1;
}

const char * __attribute__ ((const)) get_edns_mode_str(const enum edns_mode edns_mode)
{
switch(edns_mode)
{
case EDNS_MODE_NONE:
return "NONE";
case EDNS_MODE_CODE:
return "CODE";
case EDNS_MODE_TEXT:
return "TEXT";
}
return NULL;
}

int __attribute__ ((pure)) get_edns_mode_val(const char *edns_mode)
{
if(strcasecmp(edns_mode, "NONE") == 0)
return EDNS_MODE_NONE;
else if(strcasecmp(edns_mode, "CODE") == 0)
return EDNS_MODE_CODE;
else if(strcasecmp(edns_mode, "TEXT") == 0)
return EDNS_MODE_TEXT;

// Invalid value
return -1;
}
Loading

0 comments on commit 031c466

Please sign in to comment.