On Mon, Jun 04, 2018 at 03:08:24PM +0200, David van Moolenbroek wrote:
> Dear OVS team, all,
>
> While testing an ethernet switch from a certain unnamed vendor, we found an
> issue with the switch's RSTP implementation, which turned out to be a
> modified version of the Open vSwitch 2.5.0 RSTP code. Despite the older OVS
> version and the customizations, it appears that the source of the issue is
> an OVS bug which is still present today. As such, I am reporting the issue
> for your consideration.
>
> The issue revolves around the fact that the "rstp_bpdu" structure, as
> defined in lib/rstp-common.h, ends with a seven-byte padding field: "uint8_t
> padding[7];". Among other things, the rstp_bpdu structure is used to
> generate RSTP BPDUs for sending, specifically in the functions tx_config(),
> tx_rstp(), and tx_tcn() in lib/rstp-state-machines.c. Presumably, the
> padding field exists to ensure that the resulting ethernet frames meet the
> minimum size of 64 bytes.
>
> However, the functions tx_config() and tx_rstp() do not initialize the
> structure's padding field in any way. The result is that packets sent by
> those functions will contain seven padding bytes with contents of whatever
> happened to be on the stack already. These junk bytes are visible in packet
> dumps of RSTP BPDUs sent by the ethernet switch we were testing. I contend
> that this behavior is both unintentional and undesirable. With a bit of a
> stretch, one might even argue that there is a potential security angle here:
> hypothetically, such an "information leak" might be used to defeat ASLR and
> thus aid in exploiting bugs elsewhere.
>
> In addition, it looks like the same problem applies to the one-byte
> "version1_length" field in tx_config(), even though we have not been able to
> test this in practice.
>
> Needless to say, a few extra memset() calls would suffice to resolve the
> problem. Alternatively, it may be possible to send just the part of the
> rstp_bpdu structure that is applicable for the BPDU type. I am mentioning
> this possibility because another device in our test setup turned out to have
> a problem with the MAC header length of the RSTP BPDU frames as sent by the
> ethernet switch, expecting exactly 39 bytes while OVS's frames are 46 bytes.
> To be clear: that is a bug in the other device, as IEEE 802.1D-2004 (page
> 65) effectively states that the receiving side should ignore trailing bytes
> for future compatibility. However, in the light of interoperability and
> Postel's law and all, it would seem "neater" to omit sending excess bytes in
> the BPDU itself, and leave any ethernet frame padding to lower-level
> functions. However, I do not know if that can easily be done in OVS, let
> alone whether you share my view in this regard, so please feel free to
> disregard this alternative entirely!
Thanks for the report. I think that all of these problems are easy to
fix. I sent a patch:
https://patchwork.ozlabs.org/patch/925221/
If it is convenient for you to test, I'd be pleased to hear whether this
fixes the problems you observed with the noncompliant implementation you
mentioned.
Thanks,
Ben.
_______________________________________________
discuss mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-discuss
