Hi Ilya,
On Mon, Sep 04, 2017 at 01:14:01PM +0300, Ilya Lesokhin wrote:
> The tls ulp overrides sk->prot with a new tls specific proto structs.
> The tls specific structs were previously based on the ipv4 specific
> tcp_prot sturct.
> As a result, attaching the tls ulp to an ipv6 tcp socket replaced
> some ipv6 callback with the ipv4 equivalents.
>
> This patch adds ipv6 tls proto structs and uses them when
> attached to ipv6 sockets.
>
Do you plan to update this patch with the missing TCPv6 support ?
As far as I can see, the part that was accepted upstream does not fix
the TCPv6 protocol issue which triggers CVE-2018-5703.
If adding IPv6 support is not possible/acceptable, would it make sense
to limit TLS support to TCPv4, ie add something like
if (sk->sk_prot != &tcp_prot)
return -EINVAL;
to tls_init() ?
Thanks,
Guenter
> Fixes: 3c4d7559159b ('tls: kernel TLS support')
> Signed-off-by: Boris Pismenny <bor...@mellanox.com>
> Signed-off-by: Ilya Lesokhin <il...@mellanox.com>
> ---
> net/tls/Kconfig | 1 +
> net/tls/tls_main.c | 50 ++++++++++++++++++++++++++++++++++++++------------
> 2 files changed, 39 insertions(+), 12 deletions(-)
>
> diff --git a/net/tls/Kconfig b/net/tls/Kconfig
> index eb58303..7e9cf8b 100644
> --- a/net/tls/Kconfig
> +++ b/net/tls/Kconfig
> @@ -7,6 +7,7 @@ config TLS
> select CRYPTO
> select CRYPTO_AES
> select CRYPTO_GCM
> + select IPV6
> default n
> ---help---
> Enable kernel support for TLS protocol. This allows symmetric
> diff --git a/net/tls/tls_main.c b/net/tls/tls_main.c
> index 60aff60..33c499e 100644
> --- a/net/tls/tls_main.c
> +++ b/net/tls/tls_main.c
> @@ -40,13 +40,25 @@
> #include <linux/sched/signal.h>
>
> #include <net/tls.h>
> +#include <net/transp_v6.h>
>
> MODULE_AUTHOR("Mellanox Technologies");
> MODULE_DESCRIPTION("Transport Layer Security Support");
> MODULE_LICENSE("Dual BSD/GPL");
>
> -static struct proto tls_base_prot;
> -static struct proto tls_sw_prot;
> +enum {
> + TLSV4,
> + TLSV6,
> + TLS_NUM_PROTS,
> +};
> +
> +enum {
> + TLS_BASE_TX,
> + TLS_SW_TX,
> + TLS_NUM_CONFIG,
> +};
> +
> +static struct proto tls_prots[TLS_NUM_PROTS][TLS_NUM_CONFIG];
>
> int wait_on_pending_writer(struct sock *sk, long *timeo)
> {
> @@ -342,6 +354,7 @@ static int do_tls_setsockopt_tx(struct sock *sk, char
> __user *optval,
> struct tls_context *ctx = tls_get_ctx(sk);
> struct proto *prot = NULL;
> int rc = 0;
> + int ip_ver = sk->sk_family == AF_INET6 ? TLSV6 : TLSV4;
>
> if (!optval || (optlen < sizeof(*crypto_info))) {
> rc = -EINVAL;
> @@ -396,7 +409,7 @@ static int do_tls_setsockopt_tx(struct sock *sk, char
> __user *optval,
>
> /* currently SW is default, we will have ethtool in future */
> rc = tls_set_sw_offload(sk, ctx);
> - prot = &tls_sw_prot;
> + prot = &tls_prots[ip_ver][TLS_SW_TX];
> if (rc)
> goto err_crypto_info;
>
> @@ -443,6 +456,12 @@ static int tls_init(struct sock *sk)
> struct inet_connection_sock *icsk = inet_csk(sk);
> struct tls_context *ctx;
> int rc = 0;
> + int ip_ver = TLSV4;
> +
> + if (sk->sk_prot == &tcpv6_prot)
> + ip_ver = TLSV6;
> + else if (sk->sk_prot != &tcp_prot)
> + return -EINVAL;
>
> /* allocate tls context */
> ctx = kzalloc(sizeof(*ctx), GFP_KERNEL);
> @@ -453,7 +472,8 @@ static int tls_init(struct sock *sk)
> icsk->icsk_ulp_data = ctx;
> ctx->setsockopt = sk->sk_prot->setsockopt;
> ctx->getsockopt = sk->sk_prot->getsockopt;
> - sk->sk_prot = &tls_base_prot;
> +
> + sk->sk_prot = &tls_prots[ip_ver][TLS_BASE_TX];
> out:
> return rc;
> }
> @@ -464,16 +484,22 @@ static int tls_init(struct sock *sk)
> .init = tls_init,
> };
>
> +static void build_protos(struct proto *prot, struct proto *base)
> +{
> + prot[TLS_BASE_TX] = *base;
> + prot[TLS_BASE_TX].setsockopt = tls_setsockopt;
> + prot[TLS_BASE_TX].getsockopt = tls_getsockopt;
> +
> + prot[TLS_SW_TX] = prot[TLS_BASE_TX];
> + prot[TLS_SW_TX].close = tls_sk_proto_close;
> + prot[TLS_SW_TX].sendmsg = tls_sw_sendmsg;
> + prot[TLS_SW_TX].sendpage = tls_sw_sendpage;
> +}
> +
> static int __init tls_register(void)
> {
> - tls_base_prot = tcp_prot;
> - tls_base_prot.setsockopt = tls_setsockopt;
> - tls_base_prot.getsockopt = tls_getsockopt;
> -
> - tls_sw_prot = tls_base_prot;
> - tls_sw_prot.sendmsg = tls_sw_sendmsg;
> - tls_sw_prot.sendpage = tls_sw_sendpage;
> - tls_sw_prot.close = tls_sk_proto_close;
> + build_protos(tls_prots[TLSV4], &tcp_prot);
> + build_protos(tls_prots[TLSV6], &tcpv6_prot);
>
> tcp_register_ulp(&tcp_tls_ulp_ops);
>
