[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH] Slirp reverse UDP firewall
From: |
Blue Swirl |
Subject: |
Re: [Qemu-devel] [PATCH] Slirp reverse UDP firewall |
Date: |
Wed, 13 Apr 2011 23:52:35 +0300 |
On Tue, Apr 12, 2011 at 7:19 PM, Daisuke Nojiri <address@hidden> 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 <address@hidden>
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
> address@hidden -net nic[,address@hidden,address@hidden,address@hidden
> [,address@hidden,address@hidden,address@hidden
> address@hidden -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;
>
Re: [Qemu-devel] [PATCH] Slirp reverse UDP firewall,
Blue Swirl <=
Re: [Qemu-devel] [PATCH] Slirp reverse UDP firewall, Stefan Berger, 2011/04/14