| Message ID | 20210415033925.1290401-4-dje@google.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Add support for ipv6 host forwarding | expand |
On Wed, Apr 14, 2021 at 8:40 PM Doug Evans <dje@google.com> wrote: > ... in preparation for adding ipv6 host forwarding support. > > Tested: > avocado run tests/acceptance/hostfwd.py > > Signed-off-by: Doug Evans <dje@google.com> > --- > > [...] > > diff --git a/tests/acceptance/hostfwd.py b/tests/acceptance/hostfwd.py > new file mode 100644 > index 0000000000..9b9db142c3 > --- /dev/null > +++ b/tests/acceptance/hostfwd.py > @@ -0,0 +1,91 @@ > [...] > + > + def test_qmp_hostfwd_ipv4_parsing_errors(self): > + """Verify handling of various kinds of parsing errors.""" > + self.vm.add_args('-nodefaults', > + '-netdev', 'user,id=vnet', > + '-device', 'virtio-net,netdev=vnet') > + self.vm.launch() > + self.assertEquals(self.hmc('hostfwd_remove abc::42'), > + "Invalid format: bad protocol name 'abc'\r\n") > + self.assertEquals(self.hmc('hostfwd_add abc::65022-:22'), > + "Invalid host forwarding rule 'abc::65022-:22'" > + \ > + " (bad protocol name 'abc')\r\n") > + self.assertEquals(self.hmc('hostfwd_add :foo'), > + "Invalid host forwarding rule ':foo'" + \ > + " (missing host-guest separator)\r\n") > + self.assertEquals(self.hmc('hostfwd_add :a.b.c.d:66-:66'), > + "Invalid host forwarding rule > ':a.b.c.d:66-:66'" + \ > + " (For host address: address resolution failed > for" \ > + " 'a.b.c.d:66': Name or service not known)\r\n") > + self.assertEquals(self.hmc('hostfwd_add ::66-a.b.c.d:66'), > + "Invalid host forwarding rule > '::66-a.b.c.d:66'" + \ > + " (For guest address: address resolution > failed" + \ > + " for 'a.b.c.d:66': Name or service not > known)\r\n") > + self.assertEquals(self.hmc('hostfwd_add ::-1-foo'), > + "Invalid host forwarding rule '::-1-foo'" + \ > + " (For host address: error parsing port in" + \ > + " address ':')\r\n") > + self.assertEquals(self.hmc('hostfwd_add ::66-foo'), > + "Invalid host forwarding rule '::66-foo' (For" > + \ > + " guest address: error parsing address > 'foo')\r\n") > + self.assertEquals(self.hmc('hostfwd_add ::66-:0'), > + "Invalid host forwarding rule '::66-:0'" + \ > + " (For guest address: invalid port '0')\r\n") > One improvement I think I'd like to make here is that I'm not sure how portable the text of the result of, e.g., gai_strerror() is, and relax the expected text of the error messages in the potentially host-specific part. But I'll wait until everything else is reviewed.
Hi On Thu, Apr 15, 2021 at 7:44 AM Doug Evans <dje@google.com> wrote: > ... in preparation for adding ipv6 host forwarding support. > > Tested: > avocado run tests/acceptance/hostfwd.py > > Signed-off-by: Doug Evans <dje@google.com> > --- > > Changes from v5: > > Use InetSocketAddress and getaddrinfo(). > Use new libslirp calls: slirp_remove_hostxfwd, slirp_add_hostxfwd. > > include/qemu/sockets.h | 2 + > net/slirp.c | 200 ++++++++++++++++++++++++------------ > tests/acceptance/hostfwd.py | 91 ++++++++++++++++ > util/qemu-sockets.c | 17 +-- > 4 files changed, 241 insertions(+), 69 deletions(-) > create mode 100644 tests/acceptance/hostfwd.py > > diff --git a/include/qemu/sockets.h b/include/qemu/sockets.h > index 94f4e8de83..6fd71775ce 100644 > --- a/include/qemu/sockets.h > +++ b/include/qemu/sockets.h > @@ -29,6 +29,8 @@ int socket_set_fast_reuse(int fd); > #define SHUT_RDWR 2 > #endif > > +int sockaddr_getport(const struct sockaddr *addr); > + > int inet_ai_family_from_address(InetSocketAddress *addr, > Error **errp); > const char *inet_parse_host_port(InetSocketAddress *addr, > diff --git a/net/slirp.c b/net/slirp.c > index a01a0fccd3..4be065c30b 100644 > --- a/net/slirp.c > +++ b/net/slirp.c > @@ -641,15 +641,108 @@ static SlirpState *slirp_lookup(Monitor *mon, const > char *id) > } > } > > +static const char *parse_protocol(const char *str, bool *is_udp, > + Error **errp) > +{ > + char buf[10]; > + const char *p = str; > + > + if (get_str_sep(buf, sizeof(buf), &p, ':') < 0) { > + error_setg(errp, "missing protocol name separator"); > + return NULL; > + } > + > + if (!strcmp(buf, "tcp") || buf[0] == '\0') { > + *is_udp = false; > + } else if (!strcmp(buf, "udp")) { > + *is_udp = true; > + } else { > + error_setg(errp, "bad protocol name '%s'", buf); > + return NULL; > + } > + > + return p; > +} > + > +static int parse_hostfwd_sockaddr(const char *str, int socktype, > + struct sockaddr_storage *saddr, > + Error **errp) > +{ > + struct addrinfo hints, *res = NULL, *e; > + InetSocketAddress *addr = g_new(InetSocketAddress, 1); > + int gai_rc; > + int rc = -1; > + > + const char *optstr = inet_parse_host_port(addr, str, errp); > + if (optstr == NULL) { > + goto fail_return; > + } > + > + memset(&hints, 0, sizeof(hints)); > + hints.ai_flags = AI_PASSIVE; /* ignored if host is not ""(->NULL) */ > + hints.ai_flags |= AI_NUMERICHOST | AI_NUMERICSERV; > + hints.ai_socktype = socktype; > + hints.ai_family = PF_INET; > + > + /* > + * Calling getaddrinfo for guest addresses is dubious, but addresses > are > + * restricted to numeric only. Convert "" to NULL for getaddrinfo's > + * benefit. > + */ > + gai_rc = getaddrinfo(*addr->host ? addr->host : NULL, > + *addr->port ? addr->port : NULL, &hints, &res); > + if (gai_rc != 0) { > + error_setg(errp, "address resolution failed for '%s': %s", > + str, gai_strerror(gai_rc)); > + goto fail_return; > + } > + if (res->ai_next != NULL) { > + /* > + * The caller only wants one address, and except for "any" for > both > + * ipv4 and ipv6 (which we've already precluded above), we > shouldn't > + * get more than one. To assist debugging print all we find. > + */ > + GString *s = g_string_new(NULL); > + for (e = res; e != NULL; e = e->ai_next) { > + char host[NI_MAXHOST]; > + char serv[NI_MAXSERV]; > + int ret = getnameinfo((struct sockaddr *)e->ai_addr, > e->ai_addrlen, > + host, sizeof(host), > + serv, sizeof(serv), > + NI_NUMERICHOST | NI_NUMERICSERV); > + if (ret == 0) { > + g_string_append_printf(s, "\n %s:%s", host, serv); > + } else { > + g_string_append_printf(s, "\n unknown, got: %s", > + gai_strerror(ret)); > + } > + } > + error_setg(errp, "multiple addresses resolved for '%s':%s", > + str, s->str); > + g_string_free(s, TRUE); > + goto fail_return; > + } > + > + memcpy(saddr, res->ai_addr, res->ai_addrlen); > + rc = 0; > + > + fail_return: > + qapi_free_InetSocketAddress(addr); > + if (res) { > + freeaddrinfo(res); > + } > + return rc; > +} > + > void hmp_hostfwd_remove(Monitor *mon, const QDict *qdict) > { > - struct in_addr host_addr = { .s_addr = INADDR_ANY }; > - int host_port; > - char buf[256]; > + struct sockaddr_storage host_addr; > const char *src_str, *p; > SlirpState *s; > - int is_udp = 0; > + bool is_udp; > + Error *error = NULL; > int err; > + int flags = 0; > const char *arg1 = qdict_get_str(qdict, "arg1"); > const char *arg2 = qdict_get_try_str(qdict, "arg2"); > > @@ -664,110 +757,91 @@ void hmp_hostfwd_remove(Monitor *mon, const QDict > *qdict) > return; > } > > + g_assert(src_str != NULL); > p = src_str; > - if (!p || get_str_sep(buf, sizeof(buf), &p, ':') < 0) { > - goto fail_syntax; > - } > > - if (!strcmp(buf, "tcp") || buf[0] == '\0') { > - is_udp = 0; > - } else if (!strcmp(buf, "udp")) { > - is_udp = 1; > - } else { > + p = parse_protocol(p, &is_udp, &error); > + if (p == NULL) { > goto fail_syntax; > } > - > - if (get_str_sep(buf, sizeof(buf), &p, ':') < 0) { > - goto fail_syntax; > - } > - if (buf[0] != '\0' && !inet_aton(buf, &host_addr)) { > - goto fail_syntax; > + if (is_udp) { > + flags |= SLIRP_HOSTFWD_UDP; > This fails to build with the system version of libslirp, as there has not been releases with this new symbol yet. I am not sure what's the status for upstream release, we should create an issue there and discuss it. Anyway, you'll probably need to make the new code conditional with SLIRP_CHECK_VERSION(), as we don't want qemu to depend on too recent releases. thanks } > > - if (qemu_strtoi(p, NULL, 10, &host_port)) { > + if (parse_hostfwd_sockaddr(p, is_udp ? SOCK_DGRAM : SOCK_STREAM, > + &host_addr, &error) < 0) { > goto fail_syntax; > } > > - err = slirp_remove_hostfwd(s->slirp, is_udp, host_addr, host_port); > + err = slirp_remove_hostxfwd(s->slirp, (struct sockaddr *) &host_addr, > + sizeof(host_addr), flags); > > monitor_printf(mon, "host forwarding rule for %s %s\n", src_str, > err ? "not found" : "removed"); > return; > > fail_syntax: > - monitor_printf(mon, "invalid format\n"); > + monitor_printf(mon, "Invalid format: %s\n", error_get_pretty(error)); > + error_free(error); > } > > static int slirp_hostfwd(SlirpState *s, const char *redir_str, Error > **errp) > { > - struct in_addr host_addr = { .s_addr = INADDR_ANY }; > - struct in_addr guest_addr = { .s_addr = 0 }; > - int host_port, guest_port; > + struct sockaddr_storage host_addr, guest_addr; > const char *p; > char buf[256]; > - int is_udp; > - char *end; > - const char *fail_reason = "Unknown reason"; > + bool is_udp; > + Error *error = NULL; > + int flags = 0; > + int port; > > + g_assert(redir_str != NULL); > p = redir_str; > - if (!p || get_str_sep(buf, sizeof(buf), &p, ':') < 0) { > - fail_reason = "No : separators"; > - goto fail_syntax; > - } > - if (!strcmp(buf, "tcp") || buf[0] == '\0') { > - is_udp = 0; > - } else if (!strcmp(buf, "udp")) { > - is_udp = 1; > - } else { > - fail_reason = "Bad protocol name"; > - goto fail_syntax; > - } > > - if (get_str_sep(buf, sizeof(buf), &p, ':') < 0) { > - fail_reason = "Missing : separator"; > + p = parse_protocol(p, &is_udp, &error); > + if (p == NULL) { > goto fail_syntax; > } > - if (buf[0] != '\0' && !inet_aton(buf, &host_addr)) { > - fail_reason = "Bad host address"; > - goto fail_syntax; > + if (is_udp) { > + flags |= SLIRP_HOSTFWD_UDP; > } > > if (get_str_sep(buf, sizeof(buf), &p, '-') < 0) { > - fail_reason = "Bad host port separator"; > - goto fail_syntax; > - } > - host_port = strtol(buf, &end, 0); > - if (*end != '\0' || host_port < 0 || host_port > 65535) { > - fail_reason = "Bad host port"; > + error_setg(&error, "missing host-guest separator"); > goto fail_syntax; > } > > - if (get_str_sep(buf, sizeof(buf), &p, ':') < 0) { > - fail_reason = "Missing guest address"; > + if (parse_hostfwd_sockaddr(buf, is_udp ? SOCK_DGRAM : SOCK_STREAM, > + &host_addr, &error) < 0) { > + error_prepend(&error, "For host address: "); > goto fail_syntax; > } > - if (buf[0] != '\0' && !inet_aton(buf, &guest_addr)) { > - fail_reason = "Bad guest address"; > + > + if (parse_hostfwd_sockaddr(p, is_udp ? SOCK_DGRAM : SOCK_STREAM, > + &guest_addr, &error) < 0) { > + error_prepend(&error, "For guest address: "); > goto fail_syntax; > } > - > - guest_port = strtol(p, &end, 0); > - if (*end != '\0' || guest_port < 1 || guest_port > 65535) { > - fail_reason = "Bad guest port"; > + port = sockaddr_getport((struct sockaddr *) &guest_addr); > + if (port == 0) { > + error_setg(&error, "For guest address: invalid port '0'"); > goto fail_syntax; > } > > - if (slirp_add_hostfwd(s->slirp, is_udp, host_addr, host_port, > guest_addr, > - guest_port) < 0) { > - error_setg(errp, "Could not set up host forwarding rule '%s'", > - redir_str); > + if (slirp_add_hostxfwd(s->slirp, > + (struct sockaddr *) &host_addr, > sizeof(host_addr), > + (struct sockaddr *) &guest_addr, > sizeof(guest_addr), > + flags) < 0) { > + error_setg(errp, "Could not set up host forwarding rule '%s': %s", > + redir_str, strerror(errno)); > return -1; > } > return 0; > > fail_syntax: > error_setg(errp, "Invalid host forwarding rule '%s' (%s)", redir_str, > - fail_reason); > + error_get_pretty(error)); > + error_free(error); > return -1; > } > > diff --git a/tests/acceptance/hostfwd.py b/tests/acceptance/hostfwd.py > new file mode 100644 > index 0000000000..9b9db142c3 > --- /dev/null > +++ b/tests/acceptance/hostfwd.py > @@ -0,0 +1,91 @@ > +# Hostfwd command tests > +# > +# Copyright 2021 Google LLC > +# > +# This program is free software; you can redistribute it and/or modify it > +# under the terms of the GNU General Public License as published by the > +# Free Software Foundation; either version 2 of the License, or > +# (at your option) any later version. > +# > +# This program is distributed in the hope that it will be useful, but > WITHOUT > +# ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or > +# FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License > +# for more details. > + > + > +from avocado_qemu import Test > + > + > +class Hostfwd(Test): > + """ > + :avocado: tags=hostfwd > + """ > + def hmc(self, cmd): > + return self.vm.command('human-monitor-command', command_line=cmd) > + > + def test_qmp_hostfwd_ipv4(self): > + self.vm.add_args('-nodefaults', > + '-netdev', 'user,id=vnet', > + '-device', 'virtio-net,netdev=vnet') > + self.vm.launch() > + self.assertEquals(self.hmc('hostfwd_add vnet tcp::65022-:22'), '') > + self.assertEquals(self.hmc('hostfwd_remove vnet tcp::65022'), > + 'host forwarding rule for tcp::65022 > removed\r\n') > + self.assertEquals(self.hmc('hostfwd_add tcp::65022-:22'), '') > + self.assertEquals(self.hmc('hostfwd_remove tcp::65022'), > + 'host forwarding rule for tcp::65022 > removed\r\n') > + self.assertEquals(self.hmc('hostfwd_add udp::65042-:42'), '') > + self.assertEquals(self.hmc('hostfwd_remove udp::65042'), > + 'host forwarding rule for udp::65042 > removed\r\n') > + > + def test_qmp_hostfwd_ipv4_functional_errors(self): > + """Verify handling of various kinds of errors given valid > addresses.""" > + self.vm.add_args('-nodefaults', > + '-netdev', 'user,id=vnet', > + '-device', 'virtio-net,netdev=vnet') > + self.vm.launch() > + self.assertEquals(self.hmc('hostfwd_remove ::65022'), > + 'host forwarding rule for ::65022 not > found\r\n') > + self.assertEquals(self.hmc('hostfwd_add udp::65042-:42'), '') > + self.assertEquals(self.hmc('hostfwd_add udp::65042-:42'), > + "Could not set up host forwarding rule" + \ > + " 'udp::65042-:42': Address already in use\r\n") > + self.assertEquals(self.hmc('hostfwd_remove ::65042'), > + 'host forwarding rule for ::65042 not > found\r\n') > + self.assertEquals(self.hmc('hostfwd_remove udp::65042'), > + 'host forwarding rule for udp::65042 > removed\r\n') > + self.assertEquals(self.hmc('hostfwd_remove udp::65042'), > + 'host forwarding rule for udp::65042 not > found\r\n') > + > + def test_qmp_hostfwd_ipv4_parsing_errors(self): > + """Verify handling of various kinds of parsing errors.""" > + self.vm.add_args('-nodefaults', > + '-netdev', 'user,id=vnet', > + '-device', 'virtio-net,netdev=vnet') > + self.vm.launch() > + self.assertEquals(self.hmc('hostfwd_remove abc::42'), > + "Invalid format: bad protocol name 'abc'\r\n") > + self.assertEquals(self.hmc('hostfwd_add abc::65022-:22'), > + "Invalid host forwarding rule 'abc::65022-:22'" > + \ > + " (bad protocol name 'abc')\r\n") > + self.assertEquals(self.hmc('hostfwd_add :foo'), > + "Invalid host forwarding rule ':foo'" + \ > + " (missing host-guest separator)\r\n") > + self.assertEquals(self.hmc('hostfwd_add :a.b.c.d:66-:66'), > + "Invalid host forwarding rule > ':a.b.c.d:66-:66'" + \ > + " (For host address: address resolution failed > for" \ > + " 'a.b.c.d:66': Name or service not known)\r\n") > + self.assertEquals(self.hmc('hostfwd_add ::66-a.b.c.d:66'), > + "Invalid host forwarding rule > '::66-a.b.c.d:66'" + \ > + " (For guest address: address resolution > failed" + \ > + " for 'a.b.c.d:66': Name or service not > known)\r\n") > + self.assertEquals(self.hmc('hostfwd_add ::-1-foo'), > + "Invalid host forwarding rule '::-1-foo'" + \ > + " (For host address: error parsing port in" + \ > + " address ':')\r\n") > + self.assertEquals(self.hmc('hostfwd_add ::66-foo'), > + "Invalid host forwarding rule '::66-foo' (For" > + \ > + " guest address: error parsing address > 'foo')\r\n") > + self.assertEquals(self.hmc('hostfwd_add ::66-:0'), > + "Invalid host forwarding rule '::66-:0'" + \ > + " (For guest address: invalid port '0')\r\n") > diff --git a/util/qemu-sockets.c b/util/qemu-sockets.c > index c0069f2565..983efeee2f 100644 > --- a/util/qemu-sockets.c > +++ b/util/qemu-sockets.c > @@ -46,23 +46,28 @@ > #endif > > > -static int inet_getport(struct addrinfo *e) > +int sockaddr_getport(const struct sockaddr *addr) > { > - struct sockaddr_in *i4; > - struct sockaddr_in6 *i6; > + const struct sockaddr_in *i4; > + const struct sockaddr_in6 *i6; > > - switch (e->ai_family) { > + switch (addr->sa_family) { > case PF_INET6: > - i6 = (void*)e->ai_addr; > + i6 = (void *)addr; > return ntohs(i6->sin6_port); > case PF_INET: > - i4 = (void*)e->ai_addr; > + i4 = (void *)addr; > return ntohs(i4->sin_port); > default: > return 0; > } > } > > +static int inet_getport(struct addrinfo *e) > +{ > + return sockaddr_getport(e->ai_addr); > +} > + > static void inet_setport(struct addrinfo *e, int port) > { > struct sockaddr_in *i4; > -- > 2.31.1.295.g9ea45b61b8-goog > > >
diff --git a/include/qemu/sockets.h b/include/qemu/sockets.h index 94f4e8de83..6fd71775ce 100644 --- a/include/qemu/sockets.h +++ b/include/qemu/sockets.h @@ -29,6 +29,8 @@ int socket_set_fast_reuse(int fd); #define SHUT_RDWR 2 #endif +int sockaddr_getport(const struct sockaddr *addr); + int inet_ai_family_from_address(InetSocketAddress *addr, Error **errp); const char *inet_parse_host_port(InetSocketAddress *addr, diff --git a/net/slirp.c b/net/slirp.c index a01a0fccd3..4be065c30b 100644 --- a/net/slirp.c +++ b/net/slirp.c @@ -641,15 +641,108 @@ static SlirpState *slirp_lookup(Monitor *mon, const char *id) } } +static const char *parse_protocol(const char *str, bool *is_udp, + Error **errp) +{ + char buf[10]; + const char *p = str; + + if (get_str_sep(buf, sizeof(buf), &p, ':') < 0) { + error_setg(errp, "missing protocol name separator"); + return NULL; + } + + if (!strcmp(buf, "tcp") || buf[0] == '\0') { + *is_udp = false; + } else if (!strcmp(buf, "udp")) { + *is_udp = true; + } else { + error_setg(errp, "bad protocol name '%s'", buf); + return NULL; + } + + return p; +} + +static int parse_hostfwd_sockaddr(const char *str, int socktype, + struct sockaddr_storage *saddr, + Error **errp) +{ + struct addrinfo hints, *res = NULL, *e; + InetSocketAddress *addr = g_new(InetSocketAddress, 1); + int gai_rc; + int rc = -1; + + const char *optstr = inet_parse_host_port(addr, str, errp); + if (optstr == NULL) { + goto fail_return; + } + + memset(&hints, 0, sizeof(hints)); + hints.ai_flags = AI_PASSIVE; /* ignored if host is not ""(->NULL) */ + hints.ai_flags |= AI_NUMERICHOST | AI_NUMERICSERV; + hints.ai_socktype = socktype; + hints.ai_family = PF_INET; + + /* + * Calling getaddrinfo for guest addresses is dubious, but addresses are + * restricted to numeric only. Convert "" to NULL for getaddrinfo's + * benefit. + */ + gai_rc = getaddrinfo(*addr->host ? addr->host : NULL, + *addr->port ? addr->port : NULL, &hints, &res); + if (gai_rc != 0) { + error_setg(errp, "address resolution failed for '%s': %s", + str, gai_strerror(gai_rc)); + goto fail_return; + } + if (res->ai_next != NULL) { + /* + * The caller only wants one address, and except for "any" for both + * ipv4 and ipv6 (which we've already precluded above), we shouldn't + * get more than one. To assist debugging print all we find. + */ + GString *s = g_string_new(NULL); + for (e = res; e != NULL; e = e->ai_next) { + char host[NI_MAXHOST]; + char serv[NI_MAXSERV]; + int ret = getnameinfo((struct sockaddr *)e->ai_addr, e->ai_addrlen, + host, sizeof(host), + serv, sizeof(serv), + NI_NUMERICHOST | NI_NUMERICSERV); + if (ret == 0) { + g_string_append_printf(s, "\n %s:%s", host, serv); + } else { + g_string_append_printf(s, "\n unknown, got: %s", + gai_strerror(ret)); + } + } + error_setg(errp, "multiple addresses resolved for '%s':%s", + str, s->str); + g_string_free(s, TRUE); + goto fail_return; + } + + memcpy(saddr, res->ai_addr, res->ai_addrlen); + rc = 0; + + fail_return: + qapi_free_InetSocketAddress(addr); + if (res) { + freeaddrinfo(res); + } + return rc; +} + void hmp_hostfwd_remove(Monitor *mon, const QDict *qdict) { - struct in_addr host_addr = { .s_addr = INADDR_ANY }; - int host_port; - char buf[256]; + struct sockaddr_storage host_addr; const char *src_str, *p; SlirpState *s; - int is_udp = 0; + bool is_udp; + Error *error = NULL; int err; + int flags = 0; const char *arg1 = qdict_get_str(qdict, "arg1"); const char *arg2 = qdict_get_try_str(qdict, "arg2"); @@ -664,110 +757,91 @@ void hmp_hostfwd_remove(Monitor *mon, const QDict *qdict) return; } + g_assert(src_str != NULL); p = src_str; - if (!p || get_str_sep(buf, sizeof(buf), &p, ':') < 0) { - goto fail_syntax; - } - if (!strcmp(buf, "tcp") || buf[0] == '\0') { - is_udp = 0; - } else if (!strcmp(buf, "udp")) { - is_udp = 1; - } else { + p = parse_protocol(p, &is_udp, &error); + if (p == NULL) { goto fail_syntax; } - - if (get_str_sep(buf, sizeof(buf), &p, ':') < 0) { - goto fail_syntax; - } - if (buf[0] != '\0' && !inet_aton(buf, &host_addr)) { - goto fail_syntax; + if (is_udp) { + flags |= SLIRP_HOSTFWD_UDP; } - if (qemu_strtoi(p, NULL, 10, &host_port)) { + if (parse_hostfwd_sockaddr(p, is_udp ? SOCK_DGRAM : SOCK_STREAM, + &host_addr, &error) < 0) { goto fail_syntax; } - err = slirp_remove_hostfwd(s->slirp, is_udp, host_addr, host_port); + err = slirp_remove_hostxfwd(s->slirp, (struct sockaddr *) &host_addr, + sizeof(host_addr), flags); monitor_printf(mon, "host forwarding rule for %s %s\n", src_str, err ? "not found" : "removed"); return; fail_syntax: - monitor_printf(mon, "invalid format\n"); + monitor_printf(mon, "Invalid format: %s\n", error_get_pretty(error)); + error_free(error); } static int slirp_hostfwd(SlirpState *s, const char *redir_str, Error **errp) { - struct in_addr host_addr = { .s_addr = INADDR_ANY }; - struct in_addr guest_addr = { .s_addr = 0 }; - int host_port, guest_port; + struct sockaddr_storage host_addr, guest_addr; const char *p; char buf[256]; - int is_udp; - char *end; - const char *fail_reason = "Unknown reason"; + bool is_udp; + Error *error = NULL; + int flags = 0; + int port; + g_assert(redir_str != NULL); p = redir_str; - if (!p || get_str_sep(buf, sizeof(buf), &p, ':') < 0) { - fail_reason = "No : separators"; - goto fail_syntax; - } - if (!strcmp(buf, "tcp") || buf[0] == '\0') { - is_udp = 0; - } else if (!strcmp(buf, "udp")) { - is_udp = 1; - } else { - fail_reason = "Bad protocol name"; - goto fail_syntax; - } - if (get_str_sep(buf, sizeof(buf), &p, ':') < 0) { - fail_reason = "Missing : separator"; + p = parse_protocol(p, &is_udp, &error); + if (p == NULL) { goto fail_syntax; } - if (buf[0] != '\0' && !inet_aton(buf, &host_addr)) { - fail_reason = "Bad host address"; - goto fail_syntax; + if (is_udp) { + flags |= SLIRP_HOSTFWD_UDP; } if (get_str_sep(buf, sizeof(buf), &p, '-') < 0) { - fail_reason = "Bad host port separator"; - goto fail_syntax; - } - host_port = strtol(buf, &end, 0); - if (*end != '\0' || host_port < 0 || host_port > 65535) { - fail_reason = "Bad host port"; + error_setg(&error, "missing host-guest separator"); goto fail_syntax; } - if (get_str_sep(buf, sizeof(buf), &p, ':') < 0) { - fail_reason = "Missing guest address"; + if (parse_hostfwd_sockaddr(buf, is_udp ? SOCK_DGRAM : SOCK_STREAM, + &host_addr, &error) < 0) { + error_prepend(&error, "For host address: "); goto fail_syntax; } - if (buf[0] != '\0' && !inet_aton(buf, &guest_addr)) { - fail_reason = "Bad guest address"; + + if (parse_hostfwd_sockaddr(p, is_udp ? SOCK_DGRAM : SOCK_STREAM, + &guest_addr, &error) < 0) { + error_prepend(&error, "For guest address: "); goto fail_syntax; } - - guest_port = strtol(p, &end, 0); - if (*end != '\0' || guest_port < 1 || guest_port > 65535) { - fail_reason = "Bad guest port"; + port = sockaddr_getport((struct sockaddr *) &guest_addr); + if (port == 0) { + error_setg(&error, "For guest address: invalid port '0'"); goto fail_syntax; } - if (slirp_add_hostfwd(s->slirp, is_udp, host_addr, host_port, guest_addr, - guest_port) < 0) { - error_setg(errp, "Could not set up host forwarding rule '%s'", - redir_str); + if (slirp_add_hostxfwd(s->slirp, + (struct sockaddr *) &host_addr, sizeof(host_addr), + (struct sockaddr *) &guest_addr, sizeof(guest_addr), + flags) < 0) { + error_setg(errp, "Could not set up host forwarding rule '%s': %s", + redir_str, strerror(errno)); return -1; } return 0; fail_syntax: error_setg(errp, "Invalid host forwarding rule '%s' (%s)", redir_str, - fail_reason); + error_get_pretty(error)); + error_free(error); return -1; } diff --git a/tests/acceptance/hostfwd.py b/tests/acceptance/hostfwd.py new file mode 100644 index 0000000000..9b9db142c3 --- /dev/null +++ b/tests/acceptance/hostfwd.py @@ -0,0 +1,91 @@ +# Hostfwd command tests +# +# Copyright 2021 Google LLC +# +# This program is free software; you can redistribute it and/or modify it +# under the terms of the GNU General Public License as published by the +# Free Software Foundation; either version 2 of the License, or +# (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, but WITHOUT +# ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or +# FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License +# for more details. + + +from avocado_qemu import Test + + +class Hostfwd(Test): + """ + :avocado: tags=hostfwd + """ + def hmc(self, cmd): + return self.vm.command('human-monitor-command', command_line=cmd) + + def test_qmp_hostfwd_ipv4(self): + self.vm.add_args('-nodefaults', + '-netdev', 'user,id=vnet', + '-device', 'virtio-net,netdev=vnet') + self.vm.launch() + self.assertEquals(self.hmc('hostfwd_add vnet tcp::65022-:22'), '') + self.assertEquals(self.hmc('hostfwd_remove vnet tcp::65022'), + 'host forwarding rule for tcp::65022 removed\r\n') + self.assertEquals(self.hmc('hostfwd_add tcp::65022-:22'), '') + self.assertEquals(self.hmc('hostfwd_remove tcp::65022'), + 'host forwarding rule for tcp::65022 removed\r\n') + self.assertEquals(self.hmc('hostfwd_add udp::65042-:42'), '') + self.assertEquals(self.hmc('hostfwd_remove udp::65042'), + 'host forwarding rule for udp::65042 removed\r\n') + + def test_qmp_hostfwd_ipv4_functional_errors(self): + """Verify handling of various kinds of errors given valid addresses.""" + self.vm.add_args('-nodefaults', + '-netdev', 'user,id=vnet', + '-device', 'virtio-net,netdev=vnet') + self.vm.launch() + self.assertEquals(self.hmc('hostfwd_remove ::65022'), + 'host forwarding rule for ::65022 not found\r\n') + self.assertEquals(self.hmc('hostfwd_add udp::65042-:42'), '') + self.assertEquals(self.hmc('hostfwd_add udp::65042-:42'), + "Could not set up host forwarding rule" + \ + " 'udp::65042-:42': Address already in use\r\n") + self.assertEquals(self.hmc('hostfwd_remove ::65042'), + 'host forwarding rule for ::65042 not found\r\n') + self.assertEquals(self.hmc('hostfwd_remove udp::65042'), + 'host forwarding rule for udp::65042 removed\r\n') + self.assertEquals(self.hmc('hostfwd_remove udp::65042'), + 'host forwarding rule for udp::65042 not found\r\n') + + def test_qmp_hostfwd_ipv4_parsing_errors(self): + """Verify handling of various kinds of parsing errors.""" + self.vm.add_args('-nodefaults', + '-netdev', 'user,id=vnet', + '-device', 'virtio-net,netdev=vnet') + self.vm.launch() + self.assertEquals(self.hmc('hostfwd_remove abc::42'), + "Invalid format: bad protocol name 'abc'\r\n") + self.assertEquals(self.hmc('hostfwd_add abc::65022-:22'), + "Invalid host forwarding rule 'abc::65022-:22'" + \ + " (bad protocol name 'abc')\r\n") + self.assertEquals(self.hmc('hostfwd_add :foo'), + "Invalid host forwarding rule ':foo'" + \ + " (missing host-guest separator)\r\n") + self.assertEquals(self.hmc('hostfwd_add :a.b.c.d:66-:66'), + "Invalid host forwarding rule ':a.b.c.d:66-:66'" + \ + " (For host address: address resolution failed for" \ + " 'a.b.c.d:66': Name or service not known)\r\n") + self.assertEquals(self.hmc('hostfwd_add ::66-a.b.c.d:66'), + "Invalid host forwarding rule '::66-a.b.c.d:66'" + \ + " (For guest address: address resolution failed" + \ + " for 'a.b.c.d:66': Name or service not known)\r\n") + self.assertEquals(self.hmc('hostfwd_add ::-1-foo'), + "Invalid host forwarding rule '::-1-foo'" + \ + " (For host address: error parsing port in" + \ + " address ':')\r\n") + self.assertEquals(self.hmc('hostfwd_add ::66-foo'), + "Invalid host forwarding rule '::66-foo' (For" + \ + " guest address: error parsing address 'foo')\r\n") + self.assertEquals(self.hmc('hostfwd_add ::66-:0'), + "Invalid host forwarding rule '::66-:0'" + \ + " (For guest address: invalid port '0')\r\n") diff --git a/util/qemu-sockets.c b/util/qemu-sockets.c index c0069f2565..983efeee2f 100644 --- a/util/qemu-sockets.c +++ b/util/qemu-sockets.c @@ -46,23 +46,28 @@ #endif -static int inet_getport(struct addrinfo *e) +int sockaddr_getport(const struct sockaddr *addr) { - struct sockaddr_in *i4; - struct sockaddr_in6 *i6; + const struct sockaddr_in *i4; + const struct sockaddr_in6 *i6; - switch (e->ai_family) { + switch (addr->sa_family) { case PF_INET6: - i6 = (void*)e->ai_addr; + i6 = (void *)addr; return ntohs(i6->sin6_port); case PF_INET: - i4 = (void*)e->ai_addr; + i4 = (void *)addr; return ntohs(i4->sin_port); default: return 0; } } +static int inet_getport(struct addrinfo *e) +{ + return sockaddr_getport(e->ai_addr); +} + static void inet_setport(struct addrinfo *e, int port) { struct sockaddr_in *i4;
... in preparation for adding ipv6 host forwarding support. Tested: avocado run tests/acceptance/hostfwd.py Signed-off-by: Doug Evans <dje@google.com> --- Changes from v5: Use InetSocketAddress and getaddrinfo(). Use new libslirp calls: slirp_remove_hostxfwd, slirp_add_hostxfwd. include/qemu/sockets.h | 2 + net/slirp.c | 200 ++++++++++++++++++++++++------------ tests/acceptance/hostfwd.py | 91 ++++++++++++++++ util/qemu-sockets.c | 17 +-- 4 files changed, 241 insertions(+), 69 deletions(-) create mode 100644 tests/acceptance/hostfwd.py