Re: [v3,net-next,2/2] tls: Use correct sk->sk_prot for IPV6
Hi Guenter, On 2/23/2018 11:52 PM, Guenter Roeck wrote: 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 ? We'll re-spin a v4 by EOW. 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 != _prot) return -EINVAL; to tls_init() ? AFAIK there are users of TLS over IPv6 who wouldn't find this acceptable. Best, Boris.
Re: [v3,net-next,2/2] tls: Use correct sk->sk_prot for IPV6
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 != _prot)
return -EINVAL;
to tls_init() ?
Thanks,
Guenter
> Fixes: 3c4d7559159b ('tls: kernel TLS support')
> Signed-off-by: Boris Pismenny
> Signed-off-by: Ilya Lesokhin
> ---
> 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
>
> #include
> +#include
>
> 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 = _sw_prot;
> + prot = _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 == _prot)
> + ip_ver = TLSV6;
> + else if (sk->sk_prot != _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 = _base_prot;
> +
> + sk->sk_prot = _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], _prot);
> + build_protos(tls_prots[TLSV6], _prot);
>
> tcp_register_ulp(_tls_ulp_ops);
>
[PATCH v3 net-next 2/2] tls: Use correct sk->sk_prot for IPV6
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.
Fixes: 3c4d7559159b ('tls: kernel TLS support')
Signed-off-by: Boris Pismenny
Signed-off-by: Ilya Lesokhin
---
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
#include
+#include
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 = _sw_prot;
+ prot = _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 == _prot)
+ ip_ver = TLSV6;
+ else if (sk->sk_prot != _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 = _base_prot;
+
+ sk->sk_prot = _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], _prot);
+ build_protos(tls_prots[TLSV6], _prot);
tcp_register_ulp(_tls_ulp_ops);
--
1.8.3.1
