Messages in this thread | | | Date | Mon, 18 May 2020 11:51:48 -0400 | From | Ben Boeckel <> | Subject | Re: [PATCH] dns: Apply a default TTL to records obtained from getaddrinfo() |
| |
On Mon, May 18, 2020 at 15:22:45 +0100, David Howells wrote: > Address records obtained from getaddrinfo() don't come with any TTL > information, even if they're obtained from the DNS, with the result that > key.dns_resolver upcall program doesn't set an expiry time on dns_resolver > records unless they include a component obtained directly from the DNS, > such as an SRV or AFSDB record. > > Fix this to apply a default TTL of 10mins in the event that we haven't got > one. This can be configured in /etc/keyutils/key.dns_resolver.conf by > adding the line: > > default_ttl: <number-of-seconds> > > to the file.
Is there precedent for this config file format? It looks like possible YAML, but this patch doesn't mention that anywhere. Is there a good reason to not be using an existing format (ini, toml, json, shell-alike, anything)? I have no preference for anything other than a format that already exists out there. Maybe one that supports comments too so that these settings can have context associated with them in real deployments (which effectively rules out json)?
> + while (fgets(buf, sizeof(buf) - 1, f)) { > + line++; > + if (buf[0] == '#') > + continue;
So it does support comments...
> + p = strchr(buf, '\n'); > + if (!p) > + error("%s:%u: line missing newline or too long", config_file, line); > + while (p > buf && isspace(p[-1])) > + p--; > + *p = 0; > + > + if (strncmp(buf, "default_ttl:", 12) == 0) { > + p = buf + 12; > + while (isspace(*p)) > + p++;
...but not if it starts with whitespace. So if one does indent the `default_ttl` setting for whatever reason, the comment is stuck at the first column.
> + if (sscanf(p, "%u%n", &u, &n) != 1) > + error("%s:%u: default_ttl: Bad value", > + config_file, line); > + if (p[n]) > + error("%s:%u: default_ttl: Extra data supplied", > + config_file, line);
But no trailing whitespace is allowed?
> + if (u < 1 || u > INT_MAX) > + error("%s:%u: default_ttl: Out of range", > + config_file, line);
The valid range should be mentioned in the docs (basically that 0 is not allowed and has no special meaning (it could mean leaving off the TTL as previously done)).
> + key_expiry = u; > + } else { > + error("%s:%u: Unknown option", config_file, line);
Forwards compatibility is hard with such behavior. Is there any reason this can't be a warning?
> @@ -24,6 +26,21 @@ It can be called in debugging mode to test its functionality by passing a > \fB\-D\fR flag on the command line. For this to work, the key description and > the callout information must be supplied. Verbosity can be increased by > supplying one or more \fB\-v\fR flags. > +.P > +A configuration file can be supplied to adjust various parameters. The file > +is looked for at: > +.IP > +/etc/keyutils/key.dns_resolver.conf > +.P > +unless otherwise specified with the \fB\-c\fR flag. > +.P > +Configuration options include: > +.TP > +.B default_ttl: <number> > +The number of seconds to set as the expiration on a cached record. This will > +be overridden if the program manages to retrieve TTL information along with > +the addresses (if, for example, it accesses the DNS directly). The default is > +600 seconds.
There's no mention of the leading whitespace support or comments here. Does the file deserve its own manpage?
--Ben
| |