[maint] batman-adv: Avoid probe ELP information leak

Message ID 20180831130844.5434-1-sven@narfation.org (mailing list archive)
State Accepted, archived
Commit 6c876e572f592c31132a55b5fb8427e168e5fb3c
Delegated to: Simon Wunderlich
Headers
Series [maint] batman-adv: Avoid probe ELP information leak |

Commit Message

Sven Eckelmann Aug. 31, 2018, 1:08 p.m. UTC
  The probe ELPs for WiFi interfaces are expanded to contain at least
BATADV_ELP_MIN_PROBE_SIZE bytes. This is usually a lot more than the
number of bytes which the template ELP packet requires.

These extra padding bytes were not initialized and thus could contain data
which were previously stored at the same location. It is therefore required
to set it to some predefined or random values to avoid leaking private
information from the system transmitting these kind of packets.

Fixes: bedcadfaa92b ("batman-adv: ELP - send unicast ELP packets for throughput sampling")
Signed-off-by: Sven Eckelmann <sven@narfation.org>
---
 net/batman-adv/bat_v_elp.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
  

Comments

Sven Eckelmann Aug. 31, 2018, 1:20 p.m. UTC | #1
On Freitag, 31. August 2018 15:08:44 CEST Sven Eckelmann wrote:
> The probe ELPs for WiFi interfaces are expanded to contain at least
> BATADV_ELP_MIN_PROBE_SIZE bytes. This is usually a lot more than the
> number of bytes which the template ELP packet requires.
> 
> These extra padding bytes were not initialized and thus could contain data
> which were previously stored at the same location. It is therefore required
> to set it to some predefined or random values to avoid leaking private
> information from the system transmitting these kind of packets.

I was just able to extract (passive) real information from a system using 
that. Here is some example with non-compromising data:

    15:14:32.930238 02:ba:de:af:fe:01 (oui Unknown) > 02:ba:de:af:fe:02 (oui Unknown), ethertype Unknown (0x4305), length 214: 
        0x0000:  030f 02ba deaf fe01 0000 0000 0000 0000  ................
        0x0010:  6574 7479 206f 6e20 7474 7953 302e 0000  etty.on.ttyS0...
        0x0020:  0200 0400 1080 0000 4143 5449 4f4e 3d61  ........ACTION=a
        0x0030:  6464 0044 4556 5041 5448 3d2f 6465 7669  dd.DEVPATH=/devi
        0x0040:  6365 732f 7669 7274 7561 6c2f 7474 792f  ces/virtual/tty/
        0x0050:  7474 7973 3200 5355 4253 5953 5445 4d3d  ttys2.SUBSYSTEM=
        0x0060:  7474 7900 5359 4e54 485f 5555 4944 3d30  tty.SYNTH_UUID=0
        0x0070:  0044 4556 4e41 4d45 3d2f 6465 762f 7474  .DEVNAME=/dev/tt
        0x0080:  7973 3200 5345 514e 554d 3d31 3836 3500  ys2.SEQNUM=1865.
        0x0090:  5553 4543 5f49 4e49 5449 414c 495a 4544  USEC_INITIALIZED
        0x00a0:  3d32 3237 3830 3933 3900 4d41 4a4f 523d  =22780939.MAJOR=
        0x00b0:  3300 4d49 4e4f 523d 3530 0054 4147 533d  3.MINOR=50.TAGS=
        0x00c0:  3a73 7973 7465 6d64                      :systemd

Kind regards,
	Sven
  
Antonio Quartulli Sept. 2, 2018, 4:08 p.m. UTC | #2
Hi,

On 31/08/18 21:08, Sven Eckelmann wrote:
> The probe ELPs for WiFi interfaces are expanded to contain at least
> BATADV_ELP_MIN_PROBE_SIZE bytes. This is usually a lot more than the
> number of bytes which the template ELP packet requires.
> 
> These extra padding bytes were not initialized and thus could contain data
> which were previously stored at the same location. It is therefore required
> to set it to some predefined or random values to avoid leaking private
> information from the system transmitting these kind of packets.
> 
> Fixes: bedcadfaa92b ("batman-adv: ELP - send unicast ELP packets for throughput sampling")
> Signed-off-by: Sven Eckelmann <sven@narfation.org>

Acked-by: Antonio Quartulli <a@unstable.cc>

Great catch Sven!
It seems like the leakage can be fairly severe, therefore this patch
should definitely be shipped to stable later.

Cheers,
  
Sven Eckelmann Sept. 5, 2018, 4:06 p.m. UTC | #3
On Freitag, 31. August 2018 15:08:44 CEST Sven Eckelmann wrote:
> The probe ELPs for WiFi interfaces are expanded to contain at least
> BATADV_ELP_MIN_PROBE_SIZE bytes. This is usually a lot more than the
> number of bytes which the template ELP packet requires.
> 
> These extra padding bytes were not initialized and thus could contain data
> which were previously stored at the same location. It is therefore required
> to set it to some predefined or random values to avoid leaking private
> information from the system transmitting these kind of packets.

Added as 6c876e572f59 [1]

Kind regards,
	Sven

[1] https://git.open-mesh.org/batman-adv.git/commit/6c876e572f592c31132a55b5fb8427e168e5fb3c
  

Patch

diff --git a/net/batman-adv/bat_v_elp.c b/net/batman-adv/bat_v_elp.c
index 71c20c1d..e103c759 100644
--- a/net/batman-adv/bat_v_elp.c
+++ b/net/batman-adv/bat_v_elp.c
@@ -241,7 +241,7 @@  batadv_v_elp_wifi_neigh_probe(struct batadv_hardif_neigh_node *neigh)
 		 * the packet to be exactly of that size to make the link
 		 * throughput estimation effective.
 		 */
-		skb_put(skb, probe_len - hard_iface->bat_v.elp_skb->len);
+		skb_put_zero(skb, probe_len - hard_iface->bat_v.elp_skb->len);
 
 		batadv_dbg(BATADV_DBG_BATMAN, bat_priv,
 			   "Sending unicast (probe) ELP packet on interface %s to %pM\n",