Hi,
On 12/11/17 00:07, Arne Schwabe wrote:
> This can be used to redirect all IPv6 traffic to the tun interface,
> effectively black holing the IPv6 traffic. Without ICMPv6 error messages this
> will result in timeouts when the server does not send error codes.
> block-ipv6 allows client side only blocking on all platforms that OpenVPN
> supports IPv6. On Android it is only way to do sensible IPv6 blocking on
> Android < 5.0 and broken devices (Samsung).
> ---
> doc/openvpn.8 | 17 +++++
> src/openvpn/forward.c | 170
> ++++++++++++++++++++++++++++++++++++++++++++++++--
> src/openvpn/forward.h | 3 +-
> src/openvpn/misc.c | 4 +-
> src/openvpn/options.c | 17 +++++
> src/openvpn/options.h | 1 +
> src/openvpn/proto.h | 19 ++++++
> 7 files changed, 224 insertions(+), 7 deletions(-)
>
> diff --git a/doc/openvpn.8 b/doc/openvpn.8
> index a4189ac2..26b5f7de 100644
> --- a/doc/openvpn.8
> +++ b/doc/openvpn.8
> @@ -1232,6 +1232,23 @@ Like \-\-redirect\-gateway, but omit actually changing
> the default
> gateway. Useful when pushing private subnets.
> .\"*********************************************************
> .TP
> +.B \-\-block\-ipv6
> +Instead of sending IPv6 packets over the VPN tunnel, all IPv6 packets are
> +answered with a ICMPv6 no route host message. This options is intended for
> +cases when IPv6 should be blocked and other options are not available.
> +
> +Following config block would send all IPv6 traffic to OpenVPN and answer all
> +requests with no route to host, effectively blocking IPv6.
> +
> +.B \-\-ifconfig-ipv6
> +fd15:53b6:dead::2/64 fd15:53b6:dead::1
> +.br
> +.B \-\-redirect\-gateway
> +ipv6
> +.br
> +.B \-\-block\-ipv6
> +.\"*********************************************************
> +.TP
> .B \-\-tun\-mtu n
> Take the TUN device MTU to be
> .B n
> diff --git a/src/openvpn/forward.c b/src/openvpn/forward.c
> index 1b7455bb..21cce90b 100644
> --- a/src/openvpn/forward.c
> +++ b/src/openvpn/forward.c
> @@ -1198,7 +1198,7 @@ process_incoming_tun(struct context *c)
> * The --passtos and --mssfix options require
> * us to examine the IP header (IPv4 or IPv6).
> */
> - process_ip_header(c, PIPV4_PASSTOS|PIP_MSSFIX|PIPV4_CLIENT_NAT,
> &c->c2.buf);
> + process_ip_header(c,
> PIPV4_PASSTOS|PIP_MSSFIX|PIPV4_CLIENT_NAT|PIPV6_IMCP_NOHOST, &c->c2.buf);
>
> #ifdef PACKET_TRUNCATION_CHECK
> /* if (c->c2.buf.len > 1) --c->c2.buf.len; */
> @@ -1209,6 +1209,9 @@ process_incoming_tun(struct context *c)
> &c->c2.n_trunc_pre_encrypt);
> #endif
>
> + }
> + if (c->c2.buf.len > 0)
> + {
> encrypt_sign(c, true);
> }
I might not have a full understanding of this chunk: but could you
explain how this is related to this patch?
> else
> @@ -1219,6 +1222,155 @@ process_incoming_tun(struct context *c)
> gc_free(&gc);
> }
>
> +/**
> + * Calculates an IPv6 checksum with a pseudo header as required by TCP, UDP
> and ICMPv6
> + * @param payload the TCP, ICMPv6 or UDP packet
> + * @param len_payload length of payload
> + * @param src_addr
> + * @param dest_addr
> + * @return calculated checksum in host order
> + */
> +static uint16_t
> +ipv6_checksum(const uint8_t *payload,
> + const int len_payload,
> + const int next_header,
> + const uint8_t *src_addr,
> + const uint8_t *dest_addr)
> +{
> + uint32_t sum = 0;
> +
> + /* make 16 bit words out of every two adjacent 8 bit words and */
> + /* calculate the sum of all 16 bit words */
> + for (int i = 0; i < len_payload; i += 2)
> + {
> + sum += (uint16_t) (((payload[i] << 8) & 0xFF00) + ((i + 1 <
> len_payload) ? (payload[i + 1] & 0xFF) : 0));
> +
> + }
> +
> + /* add the pseudo header which contains the IP source and destination
> addresses */
> + for (int i = 0; i < 16; i += 2)
> + {
> + sum += (uint16_t) ((src_addr[i] << 8) & 0xFF00) + (src_addr[i+1] &
> 0xFF);
> +
> + }
> + for (int i = 0; i < 16; i += 2)
> + {
> + sum += (uint16_t) ((dest_addr[i] << 8) & 0xFF00) + (dest_addr[i+1] &
> 0xFF);
> + }
> +
> + /* the length of the payload */
> + sum += (uint16_t) len_payload;
> +
> + /* The next header */
> + sum += (uint16_t) next_header;
> +
> + /* keep only the last 16 bits of the 32 bit calculated sum and add the
> carries */
> + while (sum >> 16)
> + {
> + sum = (sum & 0xFFFF) + (sum >> 16);
> + }
> +
> + /* Take the one's complement of sum */
> + return ((uint16_t) ~sum);
> +}
The function above is basically a copy of udp_checksum() with very
little differences.
I'd NAK this patch because it would be better to convert these
differences in function arguments and then re-use the same code for both
UDP and ICMP checksum calculation.
This way we avoid code duplication.
Differences are:
- address length
- protocol number being added to `sum`
While there, please fix some codestyle issues that still exist in this
function.
> +/**
> + * Forges a IPv6 ICMP with a no route to host error code from the IPv6
> packet in buf and sends it back via
> + * the tun device
> + */
> +void
> +ipv6_send_icmp_unreachable(struct context * c, struct buffer *buf)
> +{
> +#define ICMPV6LEN 1280
> + struct openvpn_icmp6hdr icmp6out;
> + CLEAR(icmp6out);
> +
> + /*
> + * Get a buffer to the ip packet, is_ipv6 automatically forwards
> + * the buffer to the ip packet
> + */
> + struct buffer inputipbuf = *buf;
Please add an empty line here (like you've done everywhere else). I
believe that having an empty line between variables declarations and
actual code makes the whole thing easier to parse for poor brains like mine.
> + is_ipv6(TUNNEL_TYPE(c->c1.tuntap), &inputipbuf);
> +
> + if (BLEN(&inputipbuf) < (int) sizeof(struct openvpn_ipv6hdr))
> + {
> + return;
> + }
> +
> +
> + const struct openvpn_ipv6hdr* pip6 = (struct openvpn_ipv6hdr *)
> BPTR(&inputipbuf);
there should be no space in the middle of a cast.
> +
> + struct openvpn_ipv6hdr pip6out;
> +
> + /* Copy version, traffic class, flow label from input packet */
> + pip6out = *pip6;
> +
> + pip6out.version_prio = pip6->version_prio;
> +
> + pip6out.daddr = pip6->saddr;
> + inet_pton( AF_INET6, c->options.ifconfig_ipv6_remote, &pip6out.saddr );
> +
no space between '(' and the first function argument.
> + pip6out.nexthdr = OPENVPN_IPPROTO_ICMPV6;
> +
> +
one empty line is enough.
> + /*
> + * The ICMPv6 unreachable code worked best in my tests with Windows,
> Linux and Android (arne)
> + * Windows did not like the adminitively prohibited return code (no fast
> fail)
> + */
> + icmp6out.icmp6_type = OPENVPN_ICMP6_DESTINATION_UNREACHABLE;
> + icmp6out.icmp6_code = OPENVPN_ICMP6_DU_NOROUTE;
> +
> + int icmpheader_len = sizeof(struct openvpn_ipv6hdr) + sizeof(struct
> openvpn_icmp6hdr);
> + int ethheader_len = sizeof(struct openvpn_ethhdr);
> + int totalheader_len = icmpheader_len;
new line here too ? I'd leave this to your judgement from now on.
> + if (TUNNEL_TYPE(c->c1.tuntap) == DEV_TYPE_TAP)
> + totalheader_len += ethheader_len;
> +
> + /*
> + * Calculate size for payload, defined in the standard that the
> resulting frame
> + * should <= 1280 and have as much as possible of the original packet
> + */
> + int payload_len = min_int(min_int(ICMPV6LEN, TUN_MTU_SIZE(&c->c2.frame)
> - icmpheader_len), buf_len(&inputipbuf));
> +
> + pip6out.payload_len = htons(sizeof(struct openvpn_icmp6hdr) +
> payload_len);
> +
> + /* Construct the packet as outgoing packet back to the client */
> + c->c2.to_tun = c->c2.buffers->aux_buf;
> + ASSERT(buf_init(&c->c2.to_tun, totalheader_len));
> +
> + /* Fill the end of the buffer with original packet */
> + ASSERT(buf_safe(&c->c2.to_tun, payload_len));
> + ASSERT(buf_copy_n(&c->c2.to_tun, &inputipbuf, payload_len));
> +
> + /* ICMP Header, copy into buffer to allow checksum calculation */
> + ASSERT(buf_write_prepend(&c->c2.to_tun, &icmp6out, sizeof(struct
> openvpn_icmp6hdr)));
> +
> + /* Calculate checksum over the packet and write to header */
> + ((struct openvpn_icmp6hdr*) BPTR(&c->c2.to_tun))->icmp6_cksum = htons
> (ipv6_checksum(BPTR(&c->c2.to_tun), BLEN(&c->c2.to_tun),
> OPENVPN_IPPROTO_ICMPV6,
> + (const uint8_t*) &pip6out.saddr,
> (uint8_t*) &pip6out.daddr));
> +
> + /* IPv6 Header */
> + ASSERT(buf_write_prepend(&c->c2.to_tun, &pip6out, sizeof(struct
> openvpn_ipv6hdr)));
> +
I see lots of asserts here and in other functions below.
Is it possible that a bridged/routed client could send a malformed IPv6
packet to convert this ASSERTs into a DoS for the VPN client?
Actually, any chance that this function is executed on the server? or is
this impossible?
> + /* Working IPv6 block for TAP will also need the client to respond to
> IPv6 nd with
> + * its own (fake) adress
> + */
> + if (TUNNEL_TYPE(c->c1.tuntap) == DEV_TYPE_TAP)
> + {
> + if (BLEN(buf) < (int) sizeof (struct openvpn_ethhdr))
> + return;
> +
> + const struct openvpn_ethhdr* orig_ethhdr = (struct openvpn_ethhdr *)
> BPTR(buf);
> +
> + /* Copy frametype and reverse source/destination for the response */
> + struct openvpn_ethhdr ethhdr;
> + memcpy(ethhdr.source, orig_ethhdr->dest, OPENVPN_ETH_ALEN);
> + memcpy(ethhdr.dest, orig_ethhdr->source, OPENVPN_ETH_ALEN);
> + ethhdr.proto = htons(OPENVPN_ETH_P_IPV6);
> + ASSERT(buf_write_prepend(&c->c2.to_tun, ðhdr, sizeof(struct
> openvpn_ethhdr)));
> + }
> +}
> +
> +
> void
> process_ip_header(struct context *c, unsigned int flags, struct buffer *buf)
> {
> @@ -1240,7 +1392,10 @@ process_ip_header(struct context *c, unsigned int
> flags, struct buffer *buf)
> {
> flags &= ~PIPV4_EXTRACT_DHCP_ROUTER;
> }
> -
> + if (!c->options.block_ipv6)
> + {
> + flags &= ~PIPV6_IMCP_NOHOST;
> + }
> if (buf->len > 0)
> {
> /*
> @@ -1275,7 +1430,7 @@ process_ip_header(struct context *c, unsigned int
> flags, struct buffer *buf)
> /* possibly do NAT on packet */
> if ((flags & PIPV4_CLIENT_NAT) && c->options.client_nat)
> {
> - const int direction = (flags & PIPV4_OUTGOING) ?
> CN_INCOMING : CN_OUTGOING;
> + const int direction = (flags & PIP_OUTGOING) ?
> CN_INCOMING : CN_OUTGOING;
> client_nat_transform(c->options.client_nat, &ipbuf,
> direction);
> }
> /* possibly extract a DHCP router message */
> @@ -1295,6 +1450,13 @@ process_ip_header(struct context *c, unsigned int
> flags, struct buffer *buf)
> {
> mss_fixup_ipv6(&ipbuf,
> MTU_TO_MSS(TUN_MTU_SIZE_DYNAMIC(&c->c2.frame)));
> }
> + if (!(flags & PIP_OUTGOING) && (flags & PIPV6_IMCP_NOHOST))
> + {
> + ipv6_send_icmp_unreachable(c, buf);
> + // Drop the IPv6 packet
please don't use c++ style comments
> + buf->len=0;
> + }
> +
> }
> }
> }
> @@ -1478,7 +1640,7 @@ process_outgoing_tun(struct context *c)
> * The --mssfix option requires
> * us to examine the IP header (IPv4 or IPv6).
> */
> - process_ip_header(c,
> PIP_MSSFIX|PIPV4_EXTRACT_DHCP_ROUTER|PIPV4_CLIENT_NAT|PIPV4_OUTGOING,
> &c->c2.to_tun);
> + process_ip_header(c,
> PIP_MSSFIX|PIPV4_EXTRACT_DHCP_ROUTER|PIPV4_CLIENT_NAT|PIP_OUTGOING,
> &c->c2.to_tun);
>
> if (c->c2.to_tun.len <= MAX_RW_SIZE_TUN(&c->c2.frame))
> {
> diff --git a/src/openvpn/forward.h b/src/openvpn/forward.h
> index 9fde5a30..62199f9e 100644
> --- a/src/openvpn/forward.h
> +++ b/src/openvpn/forward.h
> @@ -251,9 +251,10 @@ bool send_control_channel_string(struct context *c,
> const char *str, int msgleve
>
> #define PIPV4_PASSTOS (1<<0)
> #define PIP_MSSFIX (1<<1) /* v4 and v6 */
> -#define PIPV4_OUTGOING (1<<2)
> +#define PIP_OUTGOING (1<<2)
> #define PIPV4_EXTRACT_DHCP_ROUTER (1<<3)
> #define PIPV4_CLIENT_NAT (1<<4)
> +#define PIPV6_IMCP_NOHOST (1<<5)
>
> void process_ip_header(struct context *c, unsigned int flags, struct buffer
> *buf);
>
> diff --git a/src/openvpn/misc.c b/src/openvpn/misc.c
> index 001fe1c4..18f0cb5b 100644
> --- a/src/openvpn/misc.c
> +++ b/src/openvpn/misc.c
> @@ -150,7 +150,7 @@ openvpn_execve_check(const struct argv *a, const struct
> env_set *es, const unsig
> const int stat = openvpn_execve(a, es, flags);
> int ret = false;
>
> - if (platform_system_ok(stat))
> + if (platform_system_ok(stat)|| true)
space before the '||'
> {
> ret = true;
> }
> @@ -204,7 +204,7 @@ openvpn_execve(const struct argv *a, const struct env_set
> *es, const unsigned in
> char *const *envp = (char *const *)make_env_array(es, true, &gc);
> pid_t pid;
>
> - pid = fork();
> + pid = vfork();
Can you explain how this is related to this patch?
> if (pid == (pid_t)0) /* child side */
> {
> execve(cmd, argv, envp);
> diff --git a/src/openvpn/options.c b/src/openvpn/options.c
> index 641a26e2..daa9ff05 100644
> --- a/src/openvpn/options.c
> +++ b/src/openvpn/options.c
> @@ -226,6 +226,8 @@ static const char usage_message[] =
> " Add 'bypass-dns' flag to similarly bypass tunnel for
> DNS.\n"
> "--redirect-private [flags]: Like --redirect-gateway, but omit actually
> changing\n"
> " the default gateway. Useful when pushing private
> subnets.\n"
> + "--block-ipv6 : (client only) Instead sending IPv6 to the server
> generate\n"
> + " ICMPv6 host unreachable messages.\n"
> "--client-nat snat|dnat network netmask alias : on client add 1-to-1 NAT
> rule.\n"
> #ifdef ENABLE_PUSH_PEER_INFO
> "--push-peer-info : (client only) push client info to server.\n"
> @@ -2084,6 +2086,15 @@ options_postprocess_verify_ce(const struct options
> *options, const struct connec
> msg(M_USAGE, "--lladdr can only be used in --dev tap mode");
> }
>
> + if (options->block_ipv6 && !options->ifconfig_ipv6_remote)
> + {
> + msg(M_USAGE, "--block-ipv6 needs a valid --ifconfig-ipv6
> configuration");
> + }
> +
Instead of requiring the user to configure an ipv6 on its interface, you
could just fake a source address for the ICMP packets. This would make
it much easier for the user as he does not to come up with his own
"fake" ipv6 configuration.
> + if (options->block_ipv6 && dev != DEV_TYPE_TUN)
> + {
> + msg(M_USAGE, "--block-ipv6 can be only used in --dev tun mode");
> + }
> /*
> * Sanity check on MTU parameters
> */
> @@ -2242,6 +2253,7 @@ options_postprocess_verify_ce(const struct options
> *options, const struct connec
> msg(M_USAGE, "TCP server mode allows at most one --remote address");
> }
>
> +
free gift?
> #if P2MP_SERVER
>
> /*
> @@ -6360,6 +6372,11 @@ add_option(struct options *options,
> #endif
> options->routes->flags |= RG_ENABLE;
> }
> + else if (streq(p[0], "block-ipv6") && !p[1])
> + {
> + VERIFY_PERMISSION(OPT_P_ROUTE);
> + options->block_ipv6 = true;
> + }
> else if (streq(p[0], "remote-random-hostname") && !p[1])
> {
> VERIFY_PERMISSION(OPT_P_GENERAL);
> diff --git a/src/openvpn/options.h b/src/openvpn/options.h
> index 496c1143..38e9e431 100644
> --- a/src/openvpn/options.h
> +++ b/src/openvpn/options.h
> @@ -340,6 +340,7 @@ struct options
> bool route_delay_defined;
> struct route_option_list *routes;
> struct route_ipv6_option_list *routes_ipv6; /* IPv6 */
> + bool block_ipv6;
> bool route_nopull;
> bool route_gateway_via_dhcp;
> bool allow_pull_fqdn; /* as a client, allow server to push a FQDN for
> certain parameters */
> diff --git a/src/openvpn/proto.h b/src/openvpn/proto.h
> index 57f25c90..7d300af9 100644
> --- a/src/openvpn/proto.h
> +++ b/src/openvpn/proto.h
> @@ -98,6 +98,7 @@ struct openvpn_iphdr {
> #define OPENVPN_IPPROTO_IGMP 2 /* IGMP protocol */
> #define OPENVPN_IPPROTO_TCP 6 /* TCP protocol */
> #define OPENVPN_IPPROTO_UDP 17 /* UDP protocol */
> +#define OPENVPN_IPPROTO_ICMPV6 58 /* ICMPV6 protocol */
the defines above have all the values aligned. could you re-align them all?
> uint8_t protocol;
>
> uint16_t check;
> @@ -120,6 +121,24 @@ struct openvpn_ipv6hdr {
> struct in6_addr daddr;
> };
>
> +/*
> + * ICMPv6 header
> + */
> +struct openvpn_icmp6hdr {
> +# define OPENVPN_ICMP6_DESTINATION_UNREACHABLE 1
> +# define OPENVPN_ND_ROUTER_SOLICIT 133
> +# define OPENVPN_ND_ROUTER_ADVERT 134
> +# define OPENVPN_ND_NEIGHBOR_SOLICIT 135
> +# define OPENVPN_ND_NEIGHBOR_ADVERT 136
> +# define OPENVPN_ND_INVERSE_SOLICIT 141
> +# define OPENVPN_ND_INVERSE_ADVERT 142
> + uint8_t icmp6_type;
> +# define OPENVPN_ICMP6_DU_NOROUTE 0
> +# define OPENVPN_ICMP6_DU_COMMUNICATION_PROHIBTED 1
> + uint8_t icmp6_code;
> + uint16_t icmp6_cksum;
> + uint8_t icmp6_dataun[4];
> +};
>
> /*
> * UDP header
>
Cheers,
--
Antonio Quartulli
signature.asc
Description: OpenPGP digital signature
------------------------------------------------------------------------------ Check out the vibrant tech community on one of the world's most engaging tech sites, Slashdot.org! http://sdm.link/slashdot
_______________________________________________ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
