• Jakub Kicinski's avatar
    net/tls: prevent skb_orphan() from leaking TLS plain text with offload · 41477662
    Jakub Kicinski authored
    sk_validate_xmit_skb() and drivers depend on the sk member of
    struct sk_buff to identify segments requiring encryption.
    Any operation which removes or does not preserve the original TLS
    socket such as skb_orphan() or skb_clone() will cause clear text
    leaks.
    
    Make the TCP socket underlying an offloaded TLS connection
    mark all skbs as decrypted, if TLS TX is in offload mode.
    Then in sk_validate_xmit_skb() catch skbs which have no socket
    (or a socket with no validation) and decrypted flag set.
    
    Note that CONFIG_SOCK_VALIDATE_XMIT, CONFIG_TLS_DEVICE and
    sk->sk_validate_xmit_skb are slightly interchangeable right now,
    they all imply TLS offload. The new checks are guarded by
    CONFIG_TLS_DEVICE because that's the option guarding the
    sk_buff->decrypted member.
    
    Second, smaller issue with orphaning is that it breaks
    the guarantee that packets will be delivered to device
    queues in-order. All TLS offload drivers depend on that
    scheduling property. This means skb_orphan_partial()'s
    trick of preserving partial socket references will cause
    issues in the drivers. We need a full orphan, and as a
    result netem delay/throttling will cause all TLS offload
    skbs to be dropped.
    
    Reusing the sk_buff->decrypted flag also protects from
    leaking clear text when incoming, decrypted skb is redirected
    (e.g. by TC).
    
    See commit 0608c69c ("bpf: sk_msg, sock{map|hash} redirect
    through ULP") for justification why the internal flag is safe.
    The only location which could leak the flag in is tcp_bpf_sendmsg(),
    which is taken care of by clearing the previously unused bit.
    
    v2:
     - remove superfluous decrypted mark copy (Willem);
     - remove the stale doc entry (Boris);
     - rely entirely on EOR marking to prevent coalescing (Boris);
     - use an internal sendpages flag instead of marking the socket
       (Boris).
    v3 (Willem):
     - reorganize the can_skb_orphan_partial() condition;
     - fix the flag leak-in through tcp_bpf_sendmsg.
    Signed-off-by: default avatarJakub Kicinski <jakub.kicinski@netronome.com>
    Acked-by: default avatarWillem de Bruijn <willemb@google.com>
    Reviewed-by: default avatarBoris Pismenny <borisp@mellanox.com>
    Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
    41477662
tcp_bpf.c 15.6 KB