starred/hickory-dns

Fork 0

mirror of https://github.com/hickory-dns/hickory-dns.git synced 2026-04-25 03:05:51 +03:00

[GH-ISSUE #2195] [RFC] re-structure `named.toml` syntax to reject invalid configurations #918

New issue

Closed

opened 2026-03-16 00:53:24 +03:00 by kerem · 4 comments

kerem commented

2026-03-16 00:53:24 +03:00

Owner

Originally created by @japaric on GitHub (Apr 25, 2024).
Original GitHub issue: https://github.com/hickory-dns/hickory-dns/issues/2195

According to this comment

enable_dnssec - enable signing of the zone for DNSSEC

The enable_dnssec setting is used to enable (runtime) signing of DNS records.

Currently, this setting sits at the zones level, see below

[[zones]]
zone = "."
zone_type = "Hint"
enabled_dnssec = true
stores = { type = "recursor", roots = "/tmp/root.hints" }

However, in a recursive resolver (a recursor store) we don't want to (DNSSEC) sign records; instead we want to DNSSEC validate them. In RFC #2194, the idea is to put the setting inside the stores field (which maps to the RecursiveConfig type). Once you pick a dnssec_validation option, the enabled_dnssec setting one level above becomes irrelevant.

Only in the case of authoritative name servers -- the file and sqlite store types -- we want to be able to (DNSSEC) sign records. The forward resolver mode (the forward store type) should neither DNSSEC sign records nor DNSSEC validate the answers to the queries it forwards. To me that means, that enable_dnssec should be inside FileConfig and SqliteConfig, that is beneath zones.stores

Overall, it seems that ZoneConfig allows representing impossible configurations and storing settings unrelated to the operating mode of the zone (resolver vs authoritative name server). For example,

zones.allow_axfr only applies to (secondary?) name servers that serve a zone file
what happens if you set the zones.zone_type to Primary (authoritative name server) but the store to the recursor type (recursive resolver) -- those are different roles
zones.file is "a short-hand for StoreConfig::FileConfig"; what happens if you set both zones.file and zones.stores.type = "file"? which one takes precedence? what happens if you set zones.stores.type to something other than file.
zones.keys are only going to be used if enable_dnssec is true but they can be specified when enable_dnssec is false and you don't get a warning that they are not used (+)

I think it would make sense to:

(1) rework the ZoneConfig struct so that in memory one cannot represent impossible configurations nor store settings that won't be used. I think that would mean using an enum-struct to represent the ZoneType. Something like: (NOTE: very rough approximation as per my understanding of hickory's features)

struct ZoneConfig {
    zone: String,
    zone_type: ZoneType,
}

enum ZoneType {
    Authoritative(AuthoritativeNameServerConfig),
    Hint(RecursiveResolverConfig),
    Forward(ForwardResolverConfig),
}

struct AuthoritativeNameServerConfig {
    dnssec_sign: Option<DnssecSign>
    storage: NameServerStorage,
    // ..
}

struct DnssecSign {
    keys: Vec<Key>
    // ..
}

enum NameServerStorage {
    // zone is stored in a plain `.zone` text file
    File(FileConfig),
    // zone is stored is a sqlite database
    Sqlite(SqliteConfig),
}

struct RecursiveResolverConfig {
    // see hickory-dns RFC #2194
    dnssec_validation: DnssecValidation,
    // ..
}

(2) lint the named.toml file and emit warnings when settings that are not part of ZoneConfig are used (+)

(+) the toml crate is lax / permissive in this regard. it does not emit an error when the input includes a field that's not part of the struct is being deserialized into ( see example below ). performing the linting might require quite a bit of extra code -- unless serde has some "strict" mode that we could use here

use serde::Deserialize;

#[derive(Debug, Deserialize)]
struct Config {
    foo: bool,
}

fn main() {
    let input = "bar = true
foo = false";

    println!("{:?}", toml::from_str::<Config>(&input));
}

prints

Ok(Config { foo: false })

Originally created by @japaric on GitHub (Apr 25, 2024). Original GitHub issue: https://github.com/hickory-dns/hickory-dns/issues/2195 According to [this comment](https://github.com/hickory-dns/hickory-dns/blob/6334a01430088ead8642cafaee592ec7bf49831f/crates/server/src/config/mod.rs#L215) > `enable_dnssec` - enable signing of the zone for DNSSEC The `enable_dnssec` setting is used to enable (runtime) signing of DNS records. Currently, this setting sits at the `zones` level, see below ``` toml [[zones]] zone = "." zone_type = "Hint" enabled_dnssec = true stores = { type = "recursor", roots = "/tmp/root.hints" } ``` However, in a recursive resolver (a `recursor` store) we don't want to (DNSSEC) sign records; instead we want to DNSSEC *validate* them. In RFC #2194, the idea is to put the setting inside the `stores` field (which maps to the `RecursiveConfig` type). Once you pick a `dnssec_validation` option, the `enabled_dnssec` setting one level above becomes irrelevant. Only in the case of *authoritative* name servers -- the `file` and `sqlite` store types -- we want to be able to (DNSSEC) sign records. The forward resolver mode (the `forward` store type) should neither DNSSEC sign records nor DNSSEC validate the answers to the queries it forwards. To me that means, that `enable_dnssec` should be inside `FileConfig` and `SqliteConfig`, that is beneath `zones.stores` Overall, it seems that `ZoneConfig` allows representing impossible configurations and storing settings unrelated to the operating mode of the zone (resolver vs authoritative name server). For example, - `zones.allow_axfr` only applies to (secondary?) name servers that serve a zone file - what happens if you set the `zones.zone_type` to `Primary` (authoritative name server) but the `store` to the `recursor` type (recursive resolver) -- those are different roles - `zones.file` is "a short-hand for `StoreConfig::FileConfig`"; what happens if you set both `zones.file` and `zones.stores.type = "file"`? which one takes precedence? what happens if you set `zones.stores.type` to something other than `file`. - `zones.keys` are only going to be used if `enable_dnssec` is `true` but they can be specified when `enable_dnssec` is `false` and you don't get a warning that they are not used (+) I think it would make sense to: (1) rework the `ZoneConfig` struct so that *in memory* one cannot represent impossible configurations nor store settings that won't be used. I think that would mean using an enum-struct to represent the `ZoneType`. Something like: (NOTE: very rough approximation as per my understanding of hickory's features) ``` rust struct ZoneConfig { zone: String, zone_type: ZoneType, } enum ZoneType { Authoritative(AuthoritativeNameServerConfig), Hint(RecursiveResolverConfig), Forward(ForwardResolverConfig), } struct AuthoritativeNameServerConfig { dnssec_sign: Option<DnssecSign> storage: NameServerStorage, // .. } struct DnssecSign { keys: Vec<Key> // .. } enum NameServerStorage { // zone is stored in a plain `.zone` text file File(FileConfig), // zone is stored is a sqlite database Sqlite(SqliteConfig), } struct RecursiveResolverConfig { // see hickory-dns RFC #2194 dnssec_validation: DnssecValidation, // .. } ``` (2) lint the `named.toml` file and emit warnings when settings that are not part of `ZoneConfig` are used (+) (+) the `toml` crate is lax / permissive in this regard. it does not emit an error when the input includes a field that's not part of the `struct` is being deserialized into ( see example below ). performing the linting might require quite a bit of extra code -- unless serde has some "strict" mode that we could use here ``` rust use serde::Deserialize; #[derive(Debug, Deserialize)] struct Config { foo: bool, } fn main() { let input = "bar = true foo = false"; println!("{:?}", toml::from_str::<Config>(&input)); } ``` prints ``` console Ok(Config { foo: false }) ```

kerem

2026-03-16 00:53:24 +03:00

closed this issue
added the
cleanup
label

kerem commented

2026-03-16 00:53:40 +03:00

Author

Owner

@djc commented on GitHub (Apr 25, 2024):

Reworking this sounds sensible. I found #[serde(deny_unknown_fields)] which sounds like it'll do what we need?

@djc commented on GitHub (Apr 25, 2024): Reworking this sounds sensible. I found [`#[serde(deny_unknown_fields)]`](https://serde.rs/container-attrs.html#deny_unknown_fields) which sounds like it'll do what we need?

kerem commented

2026-03-16 00:53:45 +03:00

Author

Owner

@japaric commented on GitHub (Apr 25, 2024):

I found #[serde(deny_unknown_fields)] which sounds like it'll do what we need?

nice finding. that indeed does the trick.

changing ZoneConfig would be a breaking change. are breaking changes currently being merged into main?

@japaric commented on GitHub (Apr 25, 2024): > I found [#[serde(deny_unknown_fields)]](https://serde.rs/container-attrs.html#deny_unknown_fields) which sounds like it'll do what we need? nice finding. that indeed does the trick. changing `ZoneConfig` would be a breaking change. are breaking changes currently being merged into `main`?

kerem commented

2026-03-16 00:53:50 +03:00

Author

Owner

@djc commented on GitHub (Apr 25, 2024):

Yes, we're currently waiting on a new Quinn release which will let us bump Quinn and rustls and related dependencies and will plan to do a new (semver-incompatible) release soon after.

@djc commented on GitHub (Apr 25, 2024): Yes, we're currently waiting on a new Quinn release which will let us bump Quinn and rustls and related dependencies and will plan to do a new (semver-incompatible) release soon after.

kerem commented

2026-03-16 00:53:55 +03:00

Author

Owner

@marcus0x62 commented on GitHub (Nov 29, 2024):

@japaric the chained authority feature is going to make this a bit more complicated. The stores for a given zone can now be a TOML map or an array of tables, and in the array of tables case can use different authority implementations with different configuration options. See consulting blocklist for an example of this.

Also, there is a custom serde visitor to make parsing errors useful for a chained authority configuration due to how serde handles untagged enums.

@marcus0x62 commented on GitHub (Nov 29, 2024): @japaric the [chained authority](https://github.com/hickory-dns/hickory-dns/pull/2161) feature is going to make this a bit more complicated. The stores for a given zone can now be a TOML map or an array of tables, and in the array of tables case can use different authority implementations with different configuration options. See [consulting blocklist](https://github.com/hickory-dns/hickory-dns/blob/main/tests/test-data/test_configs/consulting_blocklist.toml) for an example of this. Also, there is a [custom serde visitor](https://github.com/hickory-dns/hickory-dns/pull/2495) to make parsing errors useful for a chained authority configuration due to how serde handles untagged enums.

kerem referenced this issue

2026-03-16 02:25:28 +03:00

[PR #918] [MERGED] enforce better code hygiene irt debug statements #1787