tcp: ULP infrastructure

Add the infrustructure for attaching Upper Layer Protocols (ULPs) over TCP sockets. Based on a similar infrastructure in tcp_cong. The idea is that any ULP can add its own logic by changing the TCP proto_ops structure to its own methods.

Example usage:

setsockopt(sock, SOL_TCP, TCP_ULP, "tls", sizeof("tls"));

modules will call:
tcp_register_ulp(&tcp_tls_ulp_ops);

to register/unregister their ulp, with an init function and name.

A list of registered ulps will be returned by tcp_get_available_ulp, which is hooked up to /proc. Example:

$ cat /proc/sys/net/ipv4/tcp_available_ulp
tls

There is currently no functionality to remove or chain ULPs, but it should be possible to add these in the future if needed.

Signed-off-by Boris Pismenny <[email protected]>

Signed-off-by Dave Watson <[email protected]>

---
 include/net/inet_connection_sock.h |   4 ++
 include/net/tcp.h                  |  25 +++++++
 include/uapi/linux/tcp.h           |   1 +
 net/ipv4/Makefile                  |   2 +-
 net/ipv4/sysctl_net_ipv4.c         |  25 +++++++
 net/ipv4/tcp.c                     |  28 ++++++++
 net/ipv4/tcp_ipv4.c                |   2 +
 net/ipv4/tcp_ulp.c                 | 134 +++++++++++++++++++++++++++++++++++++
 8 files changed, 220 insertions(+), 1 deletion(-)
 create mode 100644 net/ipv4/tcp_ulp.c

Hello,

Quoted 25 lines · Dave Watson

Add the infrustructure for attaching Upper Layer Protocols (ULPs) over TCP sockets. Based on a similar infrastructure in tcp_cong. The idea is that any ULP can add its own logic by changing the TCP proto_ops structure to its own methods.

Example usage:

setsockopt(sock, SOL_TCP, TCP_ULP, "tls", sizeof("tls"));

modules will call:
tcp_register_ulp(&tcp_tls_ulp_ops);

to register/unregister their ulp, with an init function and name.

A list of registered ulps will be returned by tcp_get_available_ulp, which is hooked up to /proc. Example:

$ cat /proc/sys/net/ipv4/tcp_available_ulp
tls

There is currently no functionality to remove or chain ULPs, but it should be possible to add these in the future if needed.

Signed-off-by Boris Pismenny <[email protected]>

Signed-off-by Dave Watson <[email protected]>

Quoted 11 lines

---
 include/net/inet_connection_sock.h |   4 ++
 include/net/tcp.h                  |  25 +++++++
 include/uapi/linux/tcp.h           |   1 +
 net/ipv4/Makefile                  |   2 +-
 net/ipv4/sysctl_net_ipv4.c         |  25 +++++++
 net/ipv4/tcp.c                     |  28 ++++++++
 net/ipv4/tcp_ipv4.c                |   2 +
 net/ipv4/tcp_ulp.c                 | 134 +++++++++++++++++++++++++++++++++++++
 8 files changed, 220 insertions(+), 1 deletion(-)
 create mode 100644 net/ipv4/tcp_ulp.c

I know I'm pretty late to the game (and maybe this has already been discussed but I couldn't find anything in the archives), but I am wondering what the take is on potential races of the setsockopt() vs other system-calls.

For example one might race the setsockopt() with a sendmsg() and the sendmsg might end up blocking on the lock_sock in tcp_sendmsg, waiting for tcp_set_ulp() to finish changing sk_prot. When the setsockopt() finishes, we are then inside tcp_sendmsg() coming directly from sendmsg(), while we should have been in the ULP's sendmsg.

It seems like TLS-ULP is resilient to this (or at least, won't cause a panic), but there might be more exotic users of ULP in the future, that change other callbacks and then things might go wrong.

Thoughts?

Thanks,
Christoph

Add the infrustructure for attaching Upper Layer Protocols (ULPs) over TCP sockets. Based on a similar infrastructure in tcp_cong. The idea is that any ULP can add its own logic by changing the TCP proto_ops structure to its own methods.

Example usage:

setsockopt(sock, SOL_TCP, TCP_ULP, "tls", sizeof("tls"));

modules will call:
tcp_register_ulp(&tcp_tls_ulp_ops);

to register/unregister their ulp, with an init function and name.

A list of registered ulps will be returned by tcp_get_available_ulp, which is hooked up to /proc. Example:

$ cat /proc/sys/net/ipv4/tcp_available_ulp
tls

There is currently no functionality to remove or chain ULPs, but it should be possible to add these in the future if needed.

Signed-off-by Boris Pismenny <[email protected]>

Signed-off-by Dave Watson <[email protected]>

Quoted 17 lines · Dave Watson

Add the infrustructure for attaching Upper Layer Protocols (ULPs) over TCP sockets. Based on a similar infrastructure in tcp_cong. The idea is that any ULP can add its own logic by changing the TCP proto_ops structure to its own methods.

Example usage:

setsockopt(sock, SOL_TCP, TCP_ULP, "tls", sizeof("tls"));

modules will call:
tcp_register_ulp(&tcp_tls_ulp_ops);

to register/unregister their ulp, with an init function and name.

A list of registered ulps will be returned by tcp_get_available_ulp, which is hooked up to /proc. Example:

Quoted 8 lines

$ cat /proc/sys/net/ipv4/tcp_available_ulp
tls

There is currently no functionality to remove or chain ULPs, but it should be possible to add these in the future if needed.

Signed-off-by Boris Pismenny <[email protected]>

Signed-off-by Dave Watson <[email protected]>

Hey Dave,

I'm seeing the following while fuzzing, which was bisected to this commit:

==================================================================

BUG: KASAN: null-ptr-deref in copy_to_user include/linux/uaccess.h:168 [inline]
BUG: KASAN: null-ptr-deref in do_tcp_getsockopt.isra.33+0x24f/0x1e30 net/ipv4/tcp.c:3057
Read of size 4 at addr 0000000000000020 by task syz-executor1/15452

Quoted 27 lines · Alexander (Sasha Levin)

Quoted 17 lines · Dave Watson

Add the infrustructure for attaching Upper Layer Protocols (ULPs) over TCP sockets. Based on a similar infrastructure in tcp_cong. The idea is that any ULP can add its own logic by changing the TCP proto_ops structure to its own methods.

Example usage:

setsockopt(sock, SOL_TCP, TCP_ULP, "tls", sizeof("tls"));

modules will call:
tcp_register_ulp(&tcp_tls_ulp_ops);

to register/unregister their ulp, with an init function and name.

A list of registered ulps will be returned by tcp_get_available_ulp, which is hooked up to /proc. Example:

Quoted 8 lines

$ cat /proc/sys/net/ipv4/tcp_available_ulp
tls

There is currently no functionality to remove or chain ULPs, but it should be possible to add these in the future if needed.

Signed-off-by Boris Pismenny <[email protected]>

Signed-off-by Dave Watson <[email protected]>

Quoted 8 lines

Hey Dave,

I'm seeing the following while fuzzing, which was bisected to this commit:

==================================================================

BUG: KASAN: null-ptr-deref in copy_to_user include/linux/uaccess.h:168 [inline]
BUG: KASAN: null-ptr-deref in do_tcp_getsockopt.isra.33+0x24f/0x1e30 net/ipv4/tcp.c:3057
Read of size 4 at addr 0000000000000020 by task syz-executor1/15452

At a glance, this looks like it was fixed already by

https://www.mail-archive.com/[email protected]/msg175226.html

Can you recheck with that patch, or verify that you already have it? Thanks.

Add the infrustructure for attaching Upper Layer Protocols (ULPs) over TCP sockets. Based on a similar infrastructure in tcp_cong. The idea is that any ULP can add its own logic by changing the TCP proto_ops structure to its own methods.

Example usage:

setsockopt(sock, SOL_TCP, TCP_ULP, "tls", sizeof("tls"));

Quoted 9 lines · Dave Watson

Add the infrustructure for attaching Upper Layer Protocols (ULPs) over TCP sockets. Based on a similar infrastructure in tcp_cong. The idea is that any ULP can add its own logic by changing the TCP proto_ops structure to its own methods.

Example usage:

setsockopt(sock, SOL_TCP, TCP_ULP, "tls", sizeof("tls"));

One question: is there a good reason why the ULP infrastructure should just be for TCP sockets. For example, I'd really like to be able something like:

setsockopt(sock, SOL_SOCKET, SO_ULP, &ulp_param, sizeof(ulp_param));

Where ulp_param is a structure containing the ULP name as well as some ULP specific parameters that are passed to init_ulp. ulp_init could determine whether the socket family is appropriate for the ULP being requested.

Quoted 14 lines · Tom Herbert

Quoted 9 lines · Dave Watson

Add the infrustructure for attaching Upper Layer Protocols (ULPs) over TCP sockets. Based on a similar infrastructure in tcp_cong. The idea is that any ULP can add its own logic by changing the TCP proto_ops structure to its own methods.

Example usage:

setsockopt(sock, SOL_TCP, TCP_ULP, "tls", sizeof("tls"));

One question: is there a good reason why the ULP infrastructure should just be for TCP sockets. For example, I'd really like to be able something like:

Quoted 6 lines

setsockopt(sock, SOL_SOCKET, SO_ULP, &ulp_param, sizeof(ulp_param));

Where ulp_param is a structure containing the ULP name as well as some ULP specific parameters that are passed to init_ulp. ulp_init could determine whether the socket family is appropriate for the ULP being requested.

Using SOL_SOCKET instead seems reasonable to me. I can see how ulp_params could have some use, perhaps at a slight loss in clarity. TLS needs its own setsockopts anyway though, for renegotiate for example.

include/net/tls.h new file +10 -0

+
+static inline struct tls_context *tls_get_ctx(const struct sock *sk)
+{
+	struct inet_connection_sock *icsk = inet_csk(sk);
+
+	return icsk->icsk_ulp_data;
+}
+
+static inline struct tls_sw_context *tls_sw_ctx(
+		const struct tls_context *tls_ctx)

net/tls/Kconfig new file +2 -0

@@ -0,0 +1,12 @@
+#
+# TLS configuration

Quoted 11 lines · On Wed, 14 Jun 2017 11:37:39 -0700 Dave Watson <[email protected]> wrote:

net/tls/Kconfig new file +8 -0

@@ -0,0 +1,12 @@
+#
+# TLS configuration
+#
+config TLS
+	tristate "Transport Layer Security support"
+	depends on NET
+	default m
+	---help---

net/tls/Kconfig line 12

Quoted 4 lines

net/tls/Kconfig new file +4 -0

+	Enable kernel support for TLS protocol. This allows symmetric
+	encryption handling of the TLS protocol to be done in-kernel.
+
+	If unsure, say M.

I understand that this will be useful to lots of people and most distributions will enable it. But the defacto policy in kernel configuration has been that new features in kernel default to being disabled.

net/tls/tls_sw.c new file +28 -0

+static int tls_do_encryption(struct tls_context *tls_ctx,
+			     struct tls_sw_context *ctx, size_t data_len,
+			     gfp_t flags)
+{
+	unsigned int req_size = sizeof(struct aead_request) +
+		crypto_aead_reqsize(ctx->aead_send);
+	struct aead_request *aead_req;
+	int rc;
+
+	aead_req = kmalloc(req_size, flags);
+	if (!aead_req)
+		return -ENOMEM;
+
+	ctx->sg_encrypted_data[0].offset += tls_ctx->prepend_size;
+	ctx->sg_encrypted_data[0].length -= tls_ctx->prepend_size;
+
+	aead_request_set_tfm(aead_req, ctx->aead_send);
+	aead_request_set_ad(aead_req, TLS_AAD_SPACE_SIZE);
+	aead_request_set_crypt(aead_req, ctx->sg_aead_in, ctx->sg_aead_out,
+			       data_len, tls_ctx->iv);
+	rc = crypto_aead_encrypt(aead_req);
+
+	ctx->sg_encrypted_data[0].offset -= tls_ctx->prepend_size;
+	ctx->sg_encrypted_data[0].length += tls_ctx->prepend_size;
+
+	kfree(aead_req);
+	return rc;
+}

net/tls/tls_sw.c new file +2 -0

+int tls_set_sw_offload(struct sock *sk, struct tls_context *ctx)
+{

Sorry for replying to old mail...

Quoted 2 lines

net/tls/tls_sw.c new file +2 -0

+int tls_set_sw_offload(struct sock *sk, struct tls_context *ctx)
+{

...

net/tls/tls_sw.c line 747

Quoted 10 lines

net/tls/tls_sw.c new file +10 -0

+
+	if (!sw_ctx->aead_send) {
+		sw_ctx->aead_send = crypto_alloc_aead("gcm(aes)", 0, 0);
+		if (IS_ERR(sw_ctx->aead_send)) {
+			rc = PTR_ERR(sw_ctx->aead_send);
+			sw_ctx->aead_send = NULL;
+			goto free_rec_seq;
+		}
+	}
+

When I look on how you allocate the aead transformation, it seems that you should either register an asynchronous callback with aead_request_set_callback(), or request for a synchronous algorithm.

Otherwise you will crash on an asynchronous crypto return, no?

Also, it seems that you have your scatterlists on a per crypto transformation base istead of per crypto request. Is this intentional?

Quoted 21 lines · Steffen Klassert

Sorry for replying to old mail...

Quoted 2 lines

net/tls/tls_sw.c new file +2 -0

+int tls_set_sw_offload(struct sock *sk, struct tls_context *ctx)
+{

...

net/tls/tls_sw.c line 747

Quoted 10 lines

net/tls/tls_sw.c new file +10 -0

+
+	if (!sw_ctx->aead_send) {
+		sw_ctx->aead_send = crypto_alloc_aead("gcm(aes)", 0, 0);
+		if (IS_ERR(sw_ctx->aead_send)) {
+			rc = PTR_ERR(sw_ctx->aead_send);
+			sw_ctx->aead_send = NULL;
+			goto free_rec_seq;
+		}
+	}
+

When I look on how you allocate the aead transformation, it seems that you should either register an asynchronous callback with aead_request_set_callback(), or request for a synchronous algorithm.

Quoted 1 line

Otherwise you will crash on an asynchronous crypto return, no?

The intention is for it to be synchronous, and gather directly from userspace buffers. It looks like calling crypto_alloc_aead("gcm(aes)", 0, CRYPTO_ALG_ASYNC) is the correct way to request synchronous algorithms only?

Quoted 21 lines · Steffen Klassert

Sorry for replying to old mail...

Quoted 2 lines

net/tls/tls_sw.c new file +2 -0

+int tls_set_sw_offload(struct sock *sk, struct tls_context *ctx)
+{

...

net/tls/tls_sw.c line 747

Quoted 10 lines

net/tls/tls_sw.c new file +10 -0

+
+	if (!sw_ctx->aead_send) {
+		sw_ctx->aead_send = crypto_alloc_aead("gcm(aes)", 0, 0);
+		if (IS_ERR(sw_ctx->aead_send)) {
+			rc = PTR_ERR(sw_ctx->aead_send);
+			sw_ctx->aead_send = NULL;
+			goto free_rec_seq;
+		}
+	}
+

When I look on how you allocate the aead transformation, it seems that you should either register an asynchronous callback with aead_request_set_callback(), or request for a synchronous algorithm.

Quoted 1 line

Otherwise you will crash on an asynchronous crypto return, no?

The intention is for it to be synchronous, and gather directly from userspace buffers. It looks like calling crypto_alloc_aead("gcm(aes)", 0, CRYPTO_ALG_ASYNC) is the correct way to request synchronous algorithms only?

Quoted 3 lines

Also, it seems that you have your scatterlists on a per crypto transformation base istead of per crypto request. Is this intentional?

We hold the socket lock and only one crypto op can happen at a time, so we reuse the scatterlists.

Quoted 24 lines · Dave Watson

Quoted 22 lines · Steffen Klassert

Sorry for replying to old mail...

Quoted 2 lines

net/tls/tls_sw.c new file +2 -0

+int tls_set_sw_offload(struct sock *sk, struct tls_context *ctx)
+{

...

net/tls/tls_sw.c line 747

Quoted 10 lines

net/tls/tls_sw.c new file +10 -0

+
+	if (!sw_ctx->aead_send) {
+		sw_ctx->aead_send = crypto_alloc_aead("gcm(aes)", 0, 0);
+		if (IS_ERR(sw_ctx->aead_send)) {
+			rc = PTR_ERR(sw_ctx->aead_send);
+			sw_ctx->aead_send = NULL;
+			goto free_rec_seq;
+		}
+	}
+

When I look on how you allocate the aead transformation, it seems that you should either register an asynchronous callback with aead_request_set_callback(), or request for a synchronous algorithm.

Otherwise you will crash on an asynchronous crypto return, no?

Quoted 4 lines

The intention is for it to be synchronous, and gather directly from userspace buffers. It looks like calling crypto_alloc_aead("gcm(aes)", 0, CRYPTO_ALG_ASYNC) is the correct way to request synchronous algorithms only?

Yes, but then you loose the aes-ni based algorithms because they are asynchronous. If you want to have good crypto performance, it is better to implement the asynchronous callbacks.