vmd's doing something close to shotgun parsing of the "local prefix" and
"local inet6 prefix" settings in vm.conf(5). The parser intermixes ipv4
and ipv6 parsing even when we know which one is valid in the parsing
context. This makes me sad.
Even worse, we're not validating the inputs at time of parsing and
deferring to vm creation time. This makes me even sadder.
The diff below:
- splits parsing ipv4 and ipv6 into distinct functions
- puts the validation into those functions (e.g prefix length, prefix
has properly zero'd octets)
- does *not* muck (yet) with the existing validation sprinkled in
priv.c or config.c
I thought about making pulling apart the prefix from prefix length
parsing, but getaddrinfo(3) appears to not like parsing addresses like
"192.168.0.0" vs "192.168.0.0/16". (I'm not a network person...maybe I'm
being dumb here.)
kn: this addresses some of your feedback on my previous diff from a few
weeks ago.
ok?
diff refs/heads/master refs/heads/vmd-parse
commit - 5d90c77abd2d7447f16f88ac9ea9e0485eac9f73
commit + 228fe48802ec6250e3487aa791daceba4626b03f
blob - b538d40be1a1e600c1021d95e3fadd310079fa7a
blob + f5a2507ff5742ea3a62b0112e14d17aa8cbdf99d
--- usr.sbin/vmd/config.c
+++ usr.sbin/vmd/config.c
@@ -49,7 +49,7 @@ config_init_localprefix(struct vmd_config *cfg)
{
struct sockaddr_in6 *sin6;
- if (host(VMD_DHCP_PREFIX, &cfg->cfg_localprefix) == -1)
+ if (parse_prefix4(VMD_DHCP_PREFIX, &cfg->cfg_localprefix, NULL) == -1)
return (-1);
/* IPv6 is disabled by default */
@@ -58,7 +58,7 @@ config_init_localprefix(struct vmd_config *cfg)
/* Generate random IPv6 prefix only once */
if (cfg->cfg_flags & VMD_CFG_AUTOINET6)
return (0);
- if (host(VMD_ULA_PREFIX, &cfg->cfg_localprefix6) == -1)
+ if (parse_prefix6(VMD_ULA_PREFIX, &cfg->cfg_localprefix6, NULL) == -1)
return (-1);
/* Randomize the 56 bits "Global ID" and "Subnet ID" */
sin6 = ss2sin6(&cfg->cfg_localprefix6.ss);
blob - 09468e3fe2c9f4f9193710c65667132f79a90df3
blob + 3d030c201db3e8167831846cb1c8f6e3facc40fc
--- usr.sbin/vmd/parse.y
+++ usr.sbin/vmd/parse.y
@@ -190,31 +190,30 @@ main : LOCAL INET6 {
}
| LOCAL INET6 PREFIX STRING {
struct address h;
+ const char *err;
- if (host($4, &h) == -1 ||
- h.ss.ss_family != AF_INET6 ||
- h.prefixlen > 64 || h.prefixlen < 0) {
- yyerror("invalid local inet6 prefix: %s", $4);
- free($4);
+ if (parse_prefix6($4, &h, &err)) {
+ yyerror("invalid local inet6 prefix: %s", err);
YYERROR;
+ } else {
+ env->vmd_cfg.cfg_flags |= VMD_CFG_INET6;
+ env->vmd_cfg.cfg_flags &= ~VMD_CFG_AUTOINET6;
+ memcpy(&env->vmd_cfg.cfg_localprefix6, &h,
+ sizeof(h));
}
-
- env->vmd_cfg.cfg_flags |= VMD_CFG_INET6;
- env->vmd_cfg.cfg_flags &= ~VMD_CFG_AUTOINET6;
- memcpy(&env->vmd_cfg.cfg_localprefix6, &h, sizeof(h));
+ free($4);
}
| LOCAL PREFIX STRING {
struct address h;
+ const char *err;
- if (host($3, &h) == -1 ||
- h.ss.ss_family != AF_INET ||
- h.prefixlen > 32 || h.prefixlen < 0) {
- yyerror("invalid local prefix: %s", $3);
- free($3);
+ if (parse_prefix4($3, &h, &err)) {
+ yyerror("invalid local prefix: %s", err);
YYERROR;
- }
-
- memcpy(&env->vmd_cfg.cfg_localprefix, &h, sizeof(h));
+ } else
+ memcpy(&env->vmd_cfg.cfg_localprefix, &h,
+ sizeof(h));
+ free($3);
}
| SOCKET OWNER owner_id {
env->vmd_ps.ps_csock.cs_uid = $3.uid;
@@ -1404,42 +1403,119 @@ int
return (0);
}
+/*
+ * Parse an ipv4 address and prefix for local interfaces and validate
+ * constraints for vmd networking.
+ */
int
-host(const char *str, struct address *h)
+parse_prefix4(const char *str, struct address *h, const char **errstr)
{
- struct addrinfo hints, *res;
- int prefixlen;
- char *s, *p;
- const char *errstr;
+ struct addrinfo hints, *res = NULL;
+ in_addr_t addr;
+ int prefixlen, ret;
+ char *s = NULL, *p = NULL;
- if ((s = strdup(str)) == NULL) {
- log_warn("%s", __func__);
- goto fail;
+ if ((s = strdup(str)) == NULL)
+ fatal("%s: strdup", __func__);
+ p = strrchr(s, '/');
+ if (p == NULL) {
+ if (errstr)
+ *errstr = "missing netmask";
+ return (1);
}
+ *p++ = '\0';
- if ((p = strrchr(s, '/')) != NULL) {
- *p++ = '\0';
- prefixlen = strtonum(p, 0, 128, &errstr);
- if (errstr) {
- log_warnx("prefixlen is %s: %s", errstr, p);
- goto fail;
+ /* Parse our prefix. For ipv4, we accept 16 as maximum. */
+ prefixlen = strtonum(p, 1, 16, errstr);
+ if (prefixlen == 0) {
+ free(s);
+ return (1);
+ }
+
+ /* Attempt to construct an address from the user input. */
+ memset(&hints, 0, sizeof(hints));
+ hints.ai_family = AF_INET;
+ hints.ai_flags = AI_NUMERICHOST;
+ if ((ret = getaddrinfo(s, NULL, &hints, &res)) != 0) {
+ if (errstr != NULL)
+ *errstr = gai_strerror(ret);
+ free(s);
+ return (1);
+ }
+ free(s);
+
+ memset(h, 0, sizeof(*h));
+ memcpy(&h->ss, res->ai_addr, res->ai_addrlen);
+ h->prefixlen = prefixlen;
+ freeaddrinfo(res);
+
+ /* Our prefix address should end with zeros. */
+ addr = ss2sin(&h->ss)->sin_addr.s_addr;
+ if (addr & 0xFFFF0000) {
+ if (errstr != NULL)
+ *errstr = "last two octets must be zero";
+ return (1);
+ }
+
+ return (0);
+}
+
+/*
+ * Parse an ipv6 address and prefix for local interfaces and validate
+ * constraints for vmd networking.
+ */
+int
+parse_prefix6(const char *str, struct address *h, const char **errstr)
+{
+ struct addrinfo hints, *res = NULL;
+ struct in6_addr *addr6 = NULL;
+ int prefixlen, ret;
+ char *s = NULL, *p = NULL;
+ size_t i;
+
+ if ((s = strdup(str)) == NULL)
+ fatal("%s: strdup", __func__);
+ p = strrchr(s, '/');
+ if (p == NULL) {
+ if (errstr)
+ *errstr = "missing netmask";
+ return (1);
+ }
+ *p++ = '\0';
+
+ /* Parse our prefix. For ipv6, we accept 64 as a maximum. */
+ prefixlen = strtonum(p, 1, 64, errstr);
+ if (prefixlen == 0) {
+ free(s);
+ return (1);
+ }
+
+ /* Attempt to construct an address from the user input. */
+ memset(&hints, 0, sizeof(hints));
+ hints.ai_family = AF_INET6;
+ hints.ai_flags = AI_NUMERICHOST;
+ if ((ret = getaddrinfo(s, NULL, &hints, &res)) != 0) {
+ if (errstr != NULL)
+ *errstr = gai_strerror(ret);
+ free(s);
+ return (1);
+ }
+ free(s);
+
+ memset(h, 0, sizeof(*h));
+ memcpy(&h->ss, res->ai_addr, res->ai_addrlen);
+ h->prefixlen = prefixlen;
+ freeaddrinfo(res);
+
+ /* Last 8 octets are reserved for vmd. */
+ addr6 = &ss2sin6(&h->ss)->sin6_addr;
+ for (i = 8; i < 16; i++) {
+ if (addr6->s6_addr[i] != 0) {
+ if (errstr != NULL)
+ *errstr = "last eight octets must be zero";
+ return (1);
}
- } else
- prefixlen = 128;
-
- memset(&hints, 0, sizeof(hints));
- hints.ai_family = AF_UNSPEC;
- hints.ai_flags = AI_NUMERICHOST;
- if (getaddrinfo(s, NULL, &hints, &res) == 0) {
- memset(h, 0, sizeof(*h));
- memcpy(&h->ss, res->ai_addr, res->ai_addrlen);
- h->prefixlen = prefixlen;
- freeaddrinfo(res);
- free(s);
- return (0);
}
- fail:
- free(s);
- return (-1);
+ return (0);
}
blob - 9c25b0c92ade2e1a1d3a1f67548becfc3d4eca7b
blob + 91a8e1117f4859026db8adaa01d951ce1d9b4c11
--- usr.sbin/vmd/vmd.h
+++ usr.sbin/vmd/vmd.h
@@ -518,7 +518,8 @@ int host(const char *, struct address *);
/* parse.y */
int parse_config(const char *);
int cmdline_symset(char *);
-int host(const char *, struct address *);
+int parse_prefix4(const char *, struct address *, const char **);
+int parse_prefix6(const char *, struct address *, const char **);
/* virtio.c */
int virtio_get_base(int, char *, size_t, int, const char *);
