joohoi / acme-dns

Limited DNS server with RESTful HTTP API to handle ACME DNS challenges easily and securely.
MIT License
2.19k stars 234 forks source link

Bad things happen if `[general]` is missing #292

Open candlerb opened 2 years ago

candlerb commented 2 years ago

I found this out the hard way... I couldn't understand why acme-dns was trying to do weird stuff, like use the root domain, and then crashing:

# /usr/local/bin/acme-dns
INFO[0000] Using config file                             file=/etc/acme-dns/config.cfg
INFO[0000] Connected to database
DEBU[0000] Adding new record to domain                   domain=. recordtype=SOA
INFO[0000] Listening DNS                                 addr= proto=
FATA[0000] name does not qualify for automatic certificate management:

It turns out that due to a copy-paste error I'd missed [general] from the top of the config file. D'oh!

It would be very helpful if the config parser rejected unexpected or bad settings.

gbonnefille commented 2 years ago

It is generally hard to do what you request. Config files are read to find values that override default ones. To detect extra-numerous keys requires to have a schema of the configuration and validate any input against this schema. But a INI is « open » by nature.

candlerb commented 2 years ago

Seems to be a limit of toml.DecodeFile then.

OK: how about make general.domain a mandatory setting and give a clearer error if it's missing? (e.g. "Required setting 'domain' in section '[general]' is missing")

candlerb commented 2 years ago

Aha, it is possible: see "Example (StrictDecoding)" at https://godocs.io/github.com/BurntSushi/toml

Something like this should do the trick:

        md, err := toml.DecodeFile(fname, &conf)
        if err != nil {
                // Return with config file parsing errors from toml package
                return conf, err
        }
        undecoded := md.Undecoded()
        if len(undecoded) > 0 {
                return conf, fmt.Errorf("Unexpected keys: %v", undecoded)
        }