j3d1/tinc
1
0
Fork 0
Code Issues 1 Pull requests Releases Wiki Activity

Make sure the packet source MAC address is always set.

Browse source 
When tinc is used in router mode with a TAP device, Ethernet (MAC)
headers are not present in packets flowing over the VPN; it is the
node's responsibility to fill out this header before handing the
packet over to the TAP interface (which expects such headers).

Currently, tinc fills out the destination MAC address of the packet
(otherwise the host would not recognize the packets, and nothing would
work), but it does not fill out the source MAC address. In practice this
doesn't seem to cause any real issues (the host doesn't care about the
source address), but it does look weird when looking at the packets with
a sniffer, and it also result in the following valgrind warning:

    ==13651== Syscall param write(buf) points to uninitialised byte(s)
    ==13651==    at 0x5C4B620: __write_nocancel (syscall-template.S:81)
    ==13651==    by 0x1445AA: write_packet (device.c:183)
    ==13651==    by 0x118C7C: send_packet (net_packet.c:1259)
    ==13651==    by 0x12B70A: route_ipv4 (route.c:443)
    ==13651==    by 0x12D5F8: route (route.c:971)
    ==13651==    by 0x1152BC: receive_packet (net_packet.c:250)
    ==13651==    by 0x117E1B: receive_sptps_record (net_packet.c:904)
    ==13651==    by 0x1309A8: sptps_receive_data_datagram (sptps.c:488)
    ==13651==    by 0x130A90: sptps_receive_data (sptps.c:508)
    ==13651==    by 0x115569: receive_udppacket (net_packet.c:286)
    ==13651==    by 0x119856: handle_incoming_vpn_data (net_packet.c:1499)
    ==13651==    by 0x10F3DA: event_loop (event.c:287)
    ==13651==  Address 0xffeffea3a is on thread 1's stack
    ==13651==  in frame #6, created by receive_sptps_record (net_packet.c:821)
    ==13651==

This commit fixes the issue by filling out the source MAC address. It is
generated by negating the last byte of the device MAC address, which is
consistent with what route_arp() does.

In addition, this commit stops route_arp() from filling out the Ethernet
header of the packet - this is the responsibility of send_packet(), not
route().
This commit is contained in:
Etienne Dechamps 2015-11-07 11:04:13 +00:00 committed by Guus Sliepen
parent 684bd659ae
commit bdd84660c7
2 changed files with 7 additions and 5 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

6
src/net_packet.c
View file

@ -1252,8 +1252,12 @@ void send_packet(node_t *n, vpn_packet_t *packet) {
// If it's for myself, write it to the tun/tap device.
if(n == myself) {
if(overwrite_mac)
if(overwrite_mac) {
memcpy(DATA(packet), mymac.x, ETH_ALEN);
// Use an arbitrary fake source address.
memcpy(DATA(packet) + ETH_ALEN, DATA(packet), ETH_ALEN);
DATA(packet)[ETH_ALEN * 2 - 1] ^= 0xFF;
}
n->out_packets++;
n->out_bytes += packet->len;
devops.write(packet);

6
src/route.c
View file

@ -784,15 +784,13 @@ static void route_arp(node_t *source, vpn_packet_t *packet) {
if(subnet->owner == myself)
return; /* silently ignore */
memcpy(DATA(packet), DATA(packet) + ETH_ALEN, ETH_ALEN); /* copy destination address */
DATA(packet)[ETH_ALEN * 2 - 1] ^= 0xFF; /* mangle source address so it looks like it's not from us */
memcpy(&addr, arp.arp_tpa, sizeof addr); /* save protocol addr */
memcpy(arp.arp_tpa, arp.arp_spa, sizeof addr); /* swap destination and source protocol address */
memcpy(arp.arp_spa, &addr, sizeof addr); /* ... */
memcpy(arp.arp_tha, arp.arp_sha, ETH_ALEN); /* set target hard/proto addr */
memcpy(arp.arp_sha, DATA(packet) + ETH_ALEN, ETH_ALEN); /* add fake source hard addr */
memcpy(arp.arp_sha, DATA(packet) + ETH_ALEN, ETH_ALEN); /* set source hard/proto addr */
arp.arp_sha[ETH_ALEN - 1] ^= 0xFF; /* for consistency with route_packet() */
arp.arp_op = htons(ARPOP_REPLY);
/* Copy structs on stack back to packet */