On Tue, Apr 12, 2011 at 7:19 PM, Daisuke Nojiri <dnoj...@google.com> wrote: > This patch adds: -drop-udp, -allow-udp ADDR:PORT, -drop-log FILE > e.g.) $ qemu -net user -drop-log qemu.drop -drop-udp -allow-udp > 10.0.2.3:53 > -drop-udp enables usermode firewall for out-going UDP packats from a guest. > All UDP packets except ones allowed by -allow-udp will be dropped. Dropped > packets are logged in the file specified by FILE. PORT can be a single > number > (e.g. 53) or a range (e.g. [80-81]). If ADDR is ommitted, all addresses > match > the rule. > Signed-off-by: Daisuke Nojiri <dnoj...@google.com>
I missed somehow 1/3, so I'll comment to this one. > --- a/qemu-options.hx > +++ b/qemu-options.hx > @@ -1119,6 +1119,24 @@ DEF("netdev", HAS_ARG, QEMU_OPTION_netdev, > "vde|" > #endif > "socket],id=str[,option][,option][,...]\n", QEMU_ARCH_ALL) > + > +DEF("drop-udp", 0, QEMU_OPTION_drop_udp, With the TCP firewall in mind, I'd use a more general syntax, something like "deny=proto:udp". More complete rules would need denying a block inside allowed block, so address part should be used here (or "all" for deny all). > +"-drop-udp\tDrop UDP packets by usermode firewall\n", > +QEMU_ARCH_ALL) > + > +DEF("allow-udp", HAS_ARG, QEMU_OPTION_allow_udp, Likewise, "allow=proto:udp:addr:port". Then, if "udp" is left out, the rule should apply to all protocols. The address part should allow for range specifiers, like "10.0.0.0/24". Colon is not so great choice when thinking also of IPv6 addresses. > + "-allow-udp addr:port\n" > + " Add an allowed rule for -drop-udp. If destination > matches\n" > + " the rule, the packet won't be dropped. 'port' can be a > single\n" > + " number (e.g. 53) or a range (e.g. [80-81]. If 'addr' > is omitted, > + " all addresses match.\n", > + QEMU_ARCH_ALL) > + > +DEF("drop-log", HAS_ARG, QEMU_OPTION_drop_log, > + "-drop-log file\n" > + " Set usermode firewall log filename to 'file'.\n", > + QEMU_ARCH_ALL) > + > STEXI > @item -net nic[,vlan=@var{n}][,macaddr=@var{mac}][,model=@var{type}] > [,name=@var{name}][,addr=@var{addr}][,vectors=@var{v}] > @findex -net > diff --git a/slirp/libslirp.h b/slirp/libslirp.h > index 67c70e3..1ce5d68 100644 > --- a/slirp/libslirp.h > +++ b/slirp/libslirp.h > @@ -44,6 +44,15 @@ void slirp_socket_recv(Slirp *slirp, struct in_addr > guest_addr, > size_t slirp_socket_can_recv(Slirp *slirp, struct in_addr guest_addr, > int guest_port); > > +/* Usermode firewall functions */ > +void slirp_enable_drop_udp(void); > +void slirp_set_drop_log_fd(FILE *fd); > +void slirp_add_allow(const char *optarg, u_int8_t proto); > +int slirp_drop_log(const char *format, ...); > +int slirp_should_drop(unsigned long dst_addr, > + unsigned short dst_port, > + u_int8_t proto); > + > #else /* !CONFIG_SLIRP */ > > static inline void slirp_select_fill(int *pnfds, fd_set *readfds, > diff --git a/slirp/slirp.c b/slirp/slirp.c > index 1593be1..d321316 100644 > --- a/slirp/slirp.c > +++ b/slirp/slirp.c > @@ -1111,3 +1111,192 @@ static int slirp_state_load(QEMUFile *f, void > *opaque, int version_id) > > return 0; > } > + > +/* > + * Allow rule for the usermode firewall > + */ > +struct ufw_allowed { > + struct ufw_allowed *next; Please use QLIST macros. > + unsigned long dst_addr; This is not IPv6 safe, though Slirp does not implement IPv6. I'd still use the proper address type. > + /* Port range. For a single port, dst_lport = dst_hport. */ > + unsigned short dst_lport; /* in host byte order */ > + unsigned short dst_hport; /* in host byte order */ > +}; > + > +/* > + * Global variables for the usermode firewall > + */ > +static int drop_udp = 0; > +static FILE *drop_log_fd = NULL; > +static struct ufw_allowed *fw_allowed_udp = NULL; Don't use global variables. It's possible to have several interfaces, each on a different user mode stack, so each interface may have different rules. There's struct Slirp defined in slirp.h, please put these there. > + > +void slirp_enable_drop_udp(void) > +{ > + drop_udp = 1; > +} > + > +void slirp_set_drop_log_fd(FILE *fd) > +{ > + drop_log_fd = fd; > +} > + > +int slirp_should_drop(unsigned long dst_addr, > + unsigned short dst_port, > + u_int8_t proto) { > + struct ufw_allowed *fwa = NULL; > + unsigned short dport; /* host byte order */ > + > + switch (proto) { > + case IPPROTO_UDP: > + if (drop_udp == 0) { > + return 0; > + } else { > + fwa = fw_allowed_udp; > + } > + break; > + case IPPROTO_TCP: > + default: > + return 1; /* unrecognized protocol. default drop. */ > + } > + > + /* Find matching allow rule. 0 works as a wildcard for address. */ > + for (; fwa; fwa = fwa->next) { > + dport = ntohs(dst_port); > + if ((fwa->dst_lport <= dport) && (dport <= fwa->dst_hport)) { > + /* allow any destination if 0 */ > + if (fwa->dst_addr == 0 || fwa->dst_addr == dst_addr) { > + return 0; > + } > + } > + } > + return 1; > +} > + > +/* > + * Register an allow rule for user mode firewall > + */ > +void slirp_add_allow_internal(unsigned long dst_addr, static? I'm not sure which functions are really needed outside of this file. > + unsigned short dst_lport, > + unsigned short dst_hport, > + u_int8_t proto) { > + struct ufw_allowed *fwa; > + > + fwa = (struct ufw_allowed *)malloc(sizeof(struct ufw_allowed)); The cast is useless in C. Please use qemu_malloc, then the following check is not needed. > + if (!fwa) { > + DEBUG_MISC((dfd, "Unabled to create a new firewall rule " > + "due to malloc failure\n")); > + exit(-1); > + } > + > + fwa->dst_addr = dst_addr; > + fwa->dst_lport = dst_lport; > + fwa->dst_hport = dst_hport; > + > + switch (proto) { > + case IPPROTO_UDP: > + fwa->next = fw_allowed_udp; > + fw_allowed_udp = fwa; > + break; > + case IPPROTO_TCP: > + default: > + free(fwa); This should then become qemu_free(). > + return; /* unrecognized protocol */ > + } > +} > + > +/* > + * Parse a null-terminated string specifying a network port or port range > (e.g. > + * "[1024-65535]"). In case of a single port, lport and hport are the same. > + * Returns 0 on success and -1 on error. > + */ > +int parse_port_range(const char *str, > + unsigned short *lport, > + unsigned short *hport) { > + unsigned int low = 0, high = 0; > + char *p, *arg = strdup(str); qemu_strdup(), but I'd avoid that by using strtol(). > + > + p = rindex(arg, ']'); > + if ((*arg == '[') & (p != NULL)) { > + p = arg + 1; /* skip '[' */ > + low = atoi(strsep(&p, "-")); > + high = atoi(p); > + } else { > + low = atoi(arg); > + high = low; > + } > + > + free(arg); > + > + if ((low > 0) && (high > 0) && (low <= high) && (high < 65536)) { > + *lport = low; > + *hport = high; > + return 0; > + } > + > + return -1; > +} > + > +/* > + * Add allow rules for the usermode firewall. > + */ > +void slirp_add_allow(const char *optarg, u_int8_t proto) > +{ > + /* > + * we expect the following format: > + * dst_addr:dst_port OR dst_addr:[dst_lport-dst_hport] > + */ > + char *argument = strdup(optarg), *p = argument; > + char *dst_addr_str, *dst_port_str; > + struct in_addr dst_addr; > + unsigned short dst_lport, dst_hport; > + > + dst_addr_str = strsep(&p, ":"); > + dst_port_str = p; > + > + if (dst_addr_str == NULL || dst_port_str == NULL) { > + fprintf(stderr, > + "Invalid argument %s for -allow. We expect " > + "dst_addr:dst_port or dst_addr:[dst_lport-dst_hport]\n", > + optarg); > + exit(1); > + } > + > + /* handling ":port" notation (when IP address is omitted entirely). */ > + if (*dst_addr_str == '\0') { > + dst_addr.s_addr = 0; > + } > + /* inet_aton returns 0 on failure. */ > + else if (inet_aton(dst_addr_str, &dst_addr) == 0) { else belongs to the line with }. > + fprintf(stderr, "Invalid destination IP address: %s\n", > dst_addr_str); > + exit(1); > + } > + > + if (parse_port_range(dst_port_str, &dst_lport, &dst_hport) == -1) { > + fprintf(stderr, "Invalid destination port or port range\n"); > + exit(1); > + } > + > + slirp_add_allow_internal(dst_addr.s_addr, dst_lport, dst_hport, proto); > + > + free(argument); > +} > + > +/* > + * Write to drop-log > + */ > +int slirp_drop_log(const char *format, ...) > +{ > + va_list args; > + > + if (!drop_log_fd) { > + return 0; > + } > + > + va_start(args, format); > + vfprintf(drop_log_fd, format, args); > + va_end(args); > + > + fflush(drop_log_fd); > + > + return 1; > +} > diff --git a/slirp/slirp.h b/slirp/slirp.h > index 954289a..29ee425 100644 > --- a/slirp/slirp.h > +++ b/slirp/slirp.h > @@ -299,6 +299,15 @@ int tcp_emu(struct socket *, struct mbuf *); > int tcp_ctl(struct socket *); > struct tcpcb *tcp_drop(struct tcpcb *tp, int err); > > +/* slirp.c */ > +void slirp_add_allow_internal(unsigned long dst_addr, > + unsigned short dst_lport, > + unsigned short dst_hport, > + u_int8_t proto); > +int parse_port_range(const char *str, > + unsigned short *lport, > + unsigned short *hport); > + > #ifdef USE_PPP > #define MIN_MRU MINMRU > #define MAX_MRU MAXMRU > diff --git a/slirp/udp.c b/slirp/udp.c > index 02b3793..db7e4ca 100644 > --- a/slirp/udp.c > +++ b/slirp/udp.c > @@ -67,6 +67,8 @@ udp_input(register struct mbuf *m, int iphlen) > DEBUG_ARG("m = %lx", (long)m); > DEBUG_ARG("iphlen = %d", iphlen); > > + time_t timestamp = time(NULL); > + > /* > * Strip IP options, if any; should skip this, > * make available to user, and use on returned packets, > @@ -98,6 +100,28 @@ udp_input(register struct mbuf *m, int iphlen) > ip->ip_len = len; > } > > + /* > + * User mode firewall > + */ > + if (slirp_should_drop(ip->ip_dst.s_addr, uh->uh_dport, > IPPROTO_UDP)) { > + slirp_drop_log( > + "Dropped UDP: src:0x%08x:0x%04hx dst:0x%08x:0x%04hx %ld\n", > + ntohl(ip->ip_src.s_addr), > + ntohs(uh->uh_sport), > + ntohl(ip->ip_dst.s_addr), > + ntohs(uh->uh_dport), > + timestamp); Maybe tracepoints could be used instead of logging, though a separate log may be useful too. > + goto bad; > + } else { > + slirp_drop_log( > + "Allowed UDP: src:0x%08x:0x%04hx dst:0x%08x:0x%04hx %ld\n", > + ntohl(ip->ip_src.s_addr), > + ntohs(uh->uh_sport), > + ntohl(ip->ip_dst.s_addr), > + ntohs(uh->uh_dport), > + timestamp); > + } > + > /* > * Save a copy of the IP header in case we want restore it > * for sending an ICMP error message in response. > diff --git a/vl.c b/vl.c > index 68c3b53..b0583e3 100644 > --- a/vl.c > +++ b/vl.c > @@ -2287,6 +2287,24 @@ int main(int argc, char **argv, char **envp) > exit(1); > break; > #endif > + case QEMU_OPTION_drop_udp: > + slirp_enable_drop_udp(); > + break; > + case QEMU_OPTION_allow_udp: > + slirp_add_allow(optarg, IPPROTO_UDP); > + break; > + case QEMU_OPTION_drop_log: > + { > + FILE *drop_log_fd; > + drop_log_fd = fopen(optarg, "w"); > + > + if (!drop_log_fd) { > + fprintf(stderr, "Cannot open drop log: %s\n", > optarg); > + exit(1); > + } > + slirp_set_drop_log_fd(drop_log_fd); > + } > + break; > case QEMU_OPTION_bt: > add_device_config(DEV_BT, optarg); > break; >