Re: [Openvpn-devel] [PATCH 4/9] vlan: Prepend and remove VLAN identifiers on outgoing and incoming frames
[ I just noticed that I accidentally sent this only to David and not to the list. It was written and sent on Thu, 01 Apr 2010 15:46:21 +0200 ] David Sommerseth wrote: > But what kind traffic does hit the OpenVPN clients? Does the OpenVPN > server send only traffic to the corresponding VLAN the OpenVPN client is > "assigned" to? Or does it receive packages for all the VLAN's and does > the "filtering" on the client side? The client is unmodified and everything is handled on the server-side. VLAN handling is transparent for the clients. They only see packets belonging to their VLAN and packets sent by them are implicitly assigned to the VLAN they belong to. > From a security and not the least from a performance perspective, the > OpenVPN clients should only receive traffic which hits it's own VLAN > (ie. the server does the "filtering" before sending data to the client). I agree. It also has the nice side-effect for us, that clients don't need to be upgraded. :) > I'm not sure if I saw this in code or not ... but if it is in place and > somebody could point me to the patch which does it, I would be happy. I attempt to achieve this in patches 5 and 6. Patch 5 takes care of unicast traffic and patch 6 takes care of broadcast traffic. As far I understand it, packets are generally routed based on struct mroute_addr. As soon as a client sends a packet to the openvpn server, the packet's sending address is learned and stored as an mroute_addr. If a bit-wise comparison of a destination mroute_addr and learned client's mroute_addr match, the packet is routed to that client. In tap mode, the addresses of interest are the MAC addresses. In patch 5 I extend what is stored in the mroute_addr: Instead of only storing the 6 bytes of the MAC address, I also append the 2 bytes of the VID. So now a destination MAC address of one VLAN will never match the destination MAC address of another VLAN, because the mroute_addr fields never match. For broadcasting, no mroute_addr comparisons are done, so the change from patch 5 doesn't help. For a broadcast, the packet is simply forwarded to each of the connected clients (unless PF prevents it). Patch 6 adds a check to the broadcast loop to check whether the sender's vid and the intended broadcast recipient have a matching vid. So we now selectively broadcast based on matching vid. As I'm not fully familiar with OpenVPN's code base, so I can't be absolutely sure that patch 5 and 6 catch all cases, but it's what I've found to be relevant during my tests and code reviews. I agree with Peter, that this part is critical for security and should be thoroughly reviewed. Cheers Fabian signature.asc Description: OpenPGP digital signature
Re: [Openvpn-devel] [PATCH 4/9] vlan: Prepend and remove VLAN identifiers on outgoing and incoming frames
Hi, On Thu, Apr 01, 2010 at 01:49:02PM +0200, David Sommerseth wrote: > >From a security and not the least from a performance perspective, the > OpenVPN clients should only receive traffic which hits it's own VLAN > (ie. the server does the "filtering" before sending data to the client). > I'm not sure if I saw this in code or not ... but if it is in place and > somebody could point me to the patch which does it, I would be happy. The patch description (part 0) says so: - snip --- Packets coming in from the TAP device are assumed to be tagged with IEEE 802.1Q headers. OpenVPN removes the tag, remembers the VID and routes the packet to the destination client(s) on the other side of the secure tunnel which have a matching VID. - snip --- I have not yet reviewed the source to check whether this really takes place. Something else I need to check: the "standard" mroute code hashes based on ethernet address (in tap mode). What happens if the same MAC address shows up for two different VLAN IDs? (Not very likely for virtual ethernet devices, though, but this can happen in real-world scenarios). gert -- USENET is *not* the non-clickable part of WWW! //www.muc.de/~gert/ Gert Doering - Munich, Germany g...@greenie.muc.de fax: +49-89-35655025g...@net.informatik.tu-muenchen.de
Re: [Openvpn-devel] [PATCH 4/9] vlan: Prepend and remove VLAN identifiers on outgoing and incoming frames
On 01/04/10 13:28, Fabian Knittel wrote: > Peter Stuge schrieb: >> Jan Just Keijser wrote: >>> FYI: 802.1Q defines VLAN 1 as the 'native' LAN: all packets on VLAN 1 >>> are *by definition* not encapsulated (according to my CCNA guide ;-)) > [...] >>> Perhaps we need to make sure that VID 1 means untagged ... >> >> Any VID can be untagged. While 1 is the default it can change and >> OpenVPN shouldn't really care. >> >> One alternative approach to using tag 0 would be to introduce a >> vlan-pvid (or vlan-default-tag) option to set the PVID. > > So packets coming in on the tap device that aren't tagged would be > assumed to have a vid == PVID. And packets going out on the tap device > with a vid == PVID would go out untagged. (A vid of 0 would continue to > be rejected as configuration option.) > Not specifying --vlan-pvid would mean that only tagged packets are > accepted (and sent). > > I'm still unsure what to do with incoming frames from clients who's vid > matches the pvid and where the frames contain a full 802.1Q header with > a non-zero vid. I'll probably just drop those packets. Maybe we should > drop such packets regardless of the PVID value while in --vlan-tagging > mode. (Tags in tags are apparently specified by 802.1ad and we don't > support that anyway.) > Just paying attention to the discussion from the side line. But just a simple question from a VLAN newbie. This discussion here made me think about something, something which probably is not directly connected to vid == PVID. But what kind traffic does hit the OpenVPN clients? Does the OpenVPN server send only traffic to the corresponding VLAN the OpenVPN client is "assigned" to? Or does it receive packages for all the VLAN's and does the "filtering" on the client side? >From a security and not the least from a performance perspective, the OpenVPN clients should only receive traffic which hits it's own VLAN (ie. the server does the "filtering" before sending data to the client). I'm not sure if I saw this in code or not ... but if it is in place and somebody could point me to the patch which does it, I would be happy. kind regards, David Sommerseth
Re: [Openvpn-devel] [PATCH 4/9] vlan: Prepend and remove VLAN identifiers on outgoing and incoming frames
Peter Stuge schrieb: > Jan Just Keijser wrote: >> FYI: 802.1Q defines VLAN 1 as the 'native' LAN: all packets on VLAN 1 >> are *by definition* not encapsulated (according to my CCNA guide ;-)) [...] >> Perhaps we need to make sure that VID 1 means untagged ... > > Any VID can be untagged. While 1 is the default it can change and > OpenVPN shouldn't really care. > > One alternative approach to using tag 0 would be to introduce a > vlan-pvid (or vlan-default-tag) option to set the PVID. So packets coming in on the tap device that aren't tagged would be assumed to have a vid == PVID. And packets going out on the tap device with a vid == PVID would go out untagged. (A vid of 0 would continue to be rejected as configuration option.) Not specifying --vlan-pvid would mean that only tagged packets are accepted (and sent). I'm still unsure what to do with incoming frames from clients who's vid matches the pvid and where the frames contain a full 802.1Q header with a non-zero vid. I'll probably just drop those packets. Maybe we should drop such packets regardless of the PVID value while in --vlan-tagging mode. (Tags in tags are apparently specified by 802.1ad and we don't support that anyway.) > But explicitly allowing tag 0 can also be useful. What would be the use-case? Packets with tag 0 are priority packets and we currently completely ignore / drop the priority values. We could of course add an option to specify whether the priority part of 802.1Q packets should be preserved. Instead of removing the 802.1Q packets when untagging, we would instead change the vid to 0 and leave the priority fields untouched. We'd probably want an option separate from the --vlan-tag option to specify that though ... And would this be a global setting or per-client? Per-client could make the broadcasting code a tad bit more difficult, as the packets would need to be modified for each client. (Stripping or not stripping the 802.1Q headers...) Cheers Fabian signature.asc Description: OpenPGP digital signature
Re: [Openvpn-devel] [PATCH 4/9] vlan: Prepend and remove VLAN identifiers on outgoing and incoming frames
Peter Stuge wrote: Jan Just Keijser wrote: FYI: 802.1Q defines VLAN 1 as the 'native' LAN: all packets on VLAN 1 are *by definition* not encapsulated (according to my CCNA guide ;-)) 802.1Q != CCNA.. Look at the spec, Table 9-2 on page 86. (100 in PDF) VID Use 0 "no VLAN identifier is present in the frame" 1 "The default PVID value used for classifying frames on ingress .. The PVID value of a Port can be changed by management." FFF "Reserved for implementation use." be careful here (from table 9.2; I actually have an older copy of the 802.1Q spec): The null VLAN ID. Indicates that the tag header contains only priority information; no VLAN identifier is present in the frame. This VID value shall not be configured as a PVID or a member of a VID Set, or configured in any Filtering Database entry, or used in any Management operation. so it *does* have priority info in it and it is *not* the same as no VLAN identifier ... cheers, JJK
Re: [Openvpn-devel] [PATCH 4/9] vlan: Prepend and remove VLAN identifiers on outgoing and incoming frames
Jan Just Keijser wrote: > FYI: 802.1Q defines VLAN 1 as the 'native' LAN: all packets on VLAN 1 > are *by definition* not encapsulated (according to my CCNA guide ;-)) 802.1Q != CCNA.. Look at the spec, Table 9-2 on page 86. (100 in PDF) VID Use 0 "no VLAN identifier is present in the frame" 1 "The default PVID value used for classifying frames on ingress .. The PVID value of a Port can be changed by management." FFF "Reserved for implementation use." > Perhaps we need to make sure that VID 1 means untagged ... Any VID can be untagged. While 1 is the default it can change and OpenVPN shouldn't really care. One alternative approach to using tag 0 would be to introduce a vlan-pvid (or vlan-default-tag) option to set the PVID. But explicitly allowing tag 0 can also be useful. //Peter
Re: [Openvpn-devel] [PATCH 4/9] vlan: Prepend and remove VLAN identifiers on outgoing and incoming frames
Fabian Knittel wrote:
Peter Stuge schrieb:
Fabian Knittel wrote:
+ if (ntohs (vlanhdr.tpid) != OPENVPN_ETH_P_8021Q)
+{
+ /* Drop untagged frames */
+ goto err;
+}
It would be nice to be able to use VID 0 to mean untagged packets.
Hm, nice idea. I'll implement it in my next round of patches.
FYI: 802.1Q defines VLAN 1 as the 'native' LAN: all packets on VLAN 1
are *by definition* not encapsulated (according to my CCNA guide ;-))
VID 0 means "priority frames"
Perhaps we need to make sure that VID 1 means untagged ...
For more details, see http://www.javvin.com/protocolVLAN.html
cheers,
JJK
Re: [Openvpn-devel] [PATCH 4/9] vlan: Prepend and remove VLAN identifiers on outgoing and incoming frames
Fabian Knittel schrieb: > Peter Stuge schrieb: >> It would be nice to be able to use VID 0 to mean untagged packets. > > Hm, nice idea. I'll implement it in my next round of patches. I've just noticed a detail that might warrant discussion. To make sure we're talking about the same thing, this is how I intended --vlan-tag 0 to be handled: Packets coming in from the tap device: If the packet has no 802.1Q header or the 802.1Q header's VID field is 0, the vid is assumed to be 0 and the packet will be forwarded to all clients who's --vlan-tag is set to 0. Packets outgoing via the tap device: Packets originating from clients who's --vlan-tag is set to 0 are passed on to the tap device without change, i.e. no 802.1Q header is attached or removed. A potential 802.1Q header already present within the packet is left untouched. This would allow the Priority Code Point (PCP) field to be used. Now, to the slight difficulty: What happens with outgoing frames from clients who's --vlan-tag is set to 0, where the frame contains a 802.1Q header with VID field != 0? Pushed onto a network which uses vlan tags, this would allow a client to inject packets into a different network. (For --vlan-tag != 0 this is no problem, because we unconditionally wrap the packets in 802.1Q headers with a new VID.) Possible solutions: a) Inspect outgoing packets for --vlan-tag == 0 clients. Drop packets that have a VID != 0. b) Always wrap outgoing packets with 802.1Q headers and set the VID to 0. (This would make the code-patch uniform for all --vlan-tag values.) I think I slightly prefer solution a), as it allows the PCP field to be used and doesn't add needless overhead to outgoing packets. Cheers Fabian signature.asc Description: OpenPGP digital signature
Re: [Openvpn-devel] [PATCH 4/9] vlan: Prepend and remove VLAN identifiers on outgoing and incoming frames
Peter Stuge schrieb:
> Fabian Knittel wrote:
>> + if (ntohs (vlanhdr.tpid) != OPENVPN_ETH_P_8021Q)
>> +{
>> + /* Drop untagged frames */
>> + goto err;
>> +}
>
> It would be nice to be able to use VID 0 to mean untagged packets.
Hm, nice idea. I'll implement it in my next round of patches.
Cheers
Fabian
Re: [Openvpn-devel] [PATCH 4/9] vlan: Prepend and remove VLAN identifiers on outgoing and incoming frames
Fabian Knittel wrote:
> + if (ntohs (vlanhdr.tpid) != OPENVPN_ETH_P_8021Q)
> +{
> + /* Drop untagged frames */
> + goto err;
> +}
It would be nice to be able to use VID 0 to mean untagged packets.
//Peter
