Re: [PATCH v3 net-next 0/4] kernel TLS

[Dave Watson]

Hi Richard, 

On 07/06/17 04:30 PM, Richard Weinberger wrote:
> Dave,
> 
> On Wed, Jun 14, 2017 at 8:36 PM, Dave Watson  wrote:
> >  Documentation/networking/tls.txt   | 135 +++
> >  MAINTAINERS|  10 +
> >  include/linux/socket.h |   1 +
> >  include/net/inet_connection_sock.h |   4 +
> >  include/net/tcp.h  |  27 ++
> >  include/net/tls.h  | 237 
> >  include/uapi/linux/tcp.h   |   1 +
> >  include/uapi/linux/tls.h   |  79 
> >  net/Kconfig|   1 +
> >  net/Makefile   |   1 +
> >  net/ipv4/Makefile  |   2 +-
> >  net/ipv4/sysctl_net_ipv4.c |  25 ++
> >  net/ipv4/tcp.c |  33 +-
> >  net/ipv4/tcp_ipv4.c|   2 +
> >  net/ipv4/tcp_rate.c|   1 +
> >  net/ipv4/tcp_ulp.c | 134 +++
> >  net/tls/Kconfig|  12 +
> >  net/tls/Makefile   |   7 +
> >  net/tls/tls_main.c | 487 +++
> >  net/tls/tls_sw.c   | 772 
> > +
> >  20 files changed, 1968 insertions(+), 3 deletions(-)
> >  create mode 100644 Documentation/networking/tls.txt
> >  create mode 100644 include/net/tls.h
> >  create mode 100644 include/uapi/linux/tls.h
> >  create mode 100644 net/ipv4/tcp_ulp.c
> >  create mode 100644 net/tls/Kconfig
> >  create mode 100644 net/tls/Makefile
> >  create mode 100644 net/tls/tls_main.c
> >  create mode 100644 net/tls/tls_sw.c
> 
> Sorry for the late question. Do I miss something or is this IPv4 only?

The hooks it currently overrides / uses from proto_ops (sendmsg, sendpage,
get/setsockopt, close) are the same for ipv4 & ipv6, so it should work
for both.  Our test suites have been passing in both, at least.

Re: [PATCH v3 net-next 0/4] kernel TLS

[Richard Weinberger]

Dave,

On Wed, Jun 14, 2017 at 8:36 PM, Dave Watson  wrote:
>  Documentation/networking/tls.txt   | 135 +++
>  MAINTAINERS|  10 +
>  include/linux/socket.h |   1 +
>  include/net/inet_connection_sock.h |   4 +
>  include/net/tcp.h  |  27 ++
>  include/net/tls.h  | 237 
>  include/uapi/linux/tcp.h   |   1 +
>  include/uapi/linux/tls.h   |  79 
>  net/Kconfig|   1 +
>  net/Makefile   |   1 +
>  net/ipv4/Makefile  |   2 +-
>  net/ipv4/sysctl_net_ipv4.c |  25 ++
>  net/ipv4/tcp.c |  33 +-
>  net/ipv4/tcp_ipv4.c|   2 +
>  net/ipv4/tcp_rate.c|   1 +
>  net/ipv4/tcp_ulp.c | 134 +++
>  net/tls/Kconfig|  12 +
>  net/tls/Makefile   |   7 +
>  net/tls/tls_main.c | 487 +++
>  net/tls/tls_sw.c   | 772 
> +
>  20 files changed, 1968 insertions(+), 3 deletions(-)
>  create mode 100644 Documentation/networking/tls.txt
>  create mode 100644 include/net/tls.h
>  create mode 100644 include/uapi/linux/tls.h
>  create mode 100644 net/ipv4/tcp_ulp.c
>  create mode 100644 net/tls/Kconfig
>  create mode 100644 net/tls/Makefile
>  create mode 100644 net/tls/tls_main.c
>  create mode 100644 net/tls/tls_sw.c

Sorry for the late question. Do I miss something or is this IPv4 only?

-- 
Thanks,
//richard

Re: [PATCH v3 net-next 0/4] kernel TLS

[David Miller]

From: Dave Watson 
Date: Wed, 14 Jun 2017 11:36:54 -0700

> This series adds support for kernel TLS encryption over TCP sockets.
> A standard TCP socket is converted to a TLS socket using a setsockopt.
> Only symmetric crypto is done in the kernel, as well as TLS record
> framing.  The handshake remains in userspace, and the negotiated
> cipher keys/iv are provided to the TCP socket.
> 
> We implemented support for this API in OpenSSL 1.1.0, the code is
> available at https://github.com/Mellanox/tls-openssl/tree/master
> 
> It should work with any TLS library with similar modifications,
> a test tool using gnutls is here: https://github.com/Mellanox/tls-af_ktls_tool
> 
> RFC patch to openssl:
> https://mta.openssl.org/pipermail/openssl-dev/2017-June/009384.html
 ...

Series applied, thanks for all of this hard work!

Re: [PATCH v3 net-next 0/4] kernel TLS

[Tom Herbert]

On Wed, Jun 14, 2017 at 3:17 PM, Dave Watson  wrote:
> On 06/14/17 01:54 PM, Tom Herbert wrote:
>> On Wed, Jun 14, 2017 at 11:36 AM, Dave Watson  wrote:
>> > This series adds support for kernel TLS encryption over TCP sockets.
>> > A standard TCP socket is converted to a TLS socket using a setsockopt.
>> > Only symmetric crypto is done in the kernel, as well as TLS record
>> > framing.  The handshake remains in userspace, and the negotiated
>> > cipher keys/iv are provided to the TCP socket.
>> >
>> I don't see support for TLS receive path in the kernel, only the send
>> path. Am I missing something?
>
> Correct, this is only TX.  Since it sounds likely some hardware might
> only be able to offload TX, we decided to configure TX and RX
> separately.  Using the OpenSSL patches, it should be transparent to
> users even if only one side is offloaded.
>
> The software RX patches exist but haven't been polished up yet.

Thanks for the clarification, looking forward to RX patches also!

Tom

Re: [PATCH v3 net-next 0/4] kernel TLS

[Dave Watson]

On 06/14/17 01:54 PM, Tom Herbert wrote:
> On Wed, Jun 14, 2017 at 11:36 AM, Dave Watson  wrote:
> > This series adds support for kernel TLS encryption over TCP sockets.
> > A standard TCP socket is converted to a TLS socket using a setsockopt.
> > Only symmetric crypto is done in the kernel, as well as TLS record
> > framing.  The handshake remains in userspace, and the negotiated
> > cipher keys/iv are provided to the TCP socket.
> >
> I don't see support for TLS receive path in the kernel, only the send
> path. Am I missing something?

Correct, this is only TX.  Since it sounds likely some hardware might
only be able to offload TX, we decided to configure TX and RX
separately.  Using the OpenSSL patches, it should be transparent to
users even if only one side is offloaded.

The software RX patches exist but haven't been polished up yet.

Re: [PATCH v3 net-next 0/4] kernel TLS

[Dave Watson]

Hi Hannes, 

On 06/14/17 10:15 PM, Hannes Frederic Sowa wrote:
> one question for this patch set:
> 
> What is the reason for not allowing key updates for the TX path? I was
> always loud pointing out the problems with TLSv1.2 renegotiation and
> TLSv1.3 key update alerts. This patch set uses encryption in a
> synchronous way directly in the socket layer and thus wouldn't suffer
> from problems regarding updates of the key. My hunch is that you leave
> this option open so you can later on introduce asynchronous crypto which
> might be used on hardware? It looks also be doable in case of MSG_MORE.
> Otherwise by allowing key updates to the data path I would not see any
> problems with key updates in TLS.

I don't currently have any reasons to not support renegotation, we
just don't currently use it, so I didn't add support for it.  I don't
work on the hardware, but yes it looks like it would have to keep the
old keys around until everything sent using them has been acked.

> Anyway, this patch seems easy and maybe with key updates added later on
> doesn't seem to have any problems pointed out by me so far.

Indeed, it would be easy to flush any unencrypted data, and then
change the keys.

Re: [PATCH v3 net-next 0/4] kernel TLS

[Tom Herbert]

On Wed, Jun 14, 2017 at 11:36 AM, Dave Watson  wrote:
> This series adds support for kernel TLS encryption over TCP sockets.
> A standard TCP socket is converted to a TLS socket using a setsockopt.
> Only symmetric crypto is done in the kernel, as well as TLS record
> framing.  The handshake remains in userspace, and the negotiated
> cipher keys/iv are provided to the TCP socket.
>
I don't see support for TLS receive path in the kernel, only the send
path. Am I missing something?

Thanks,
Tom

> We implemented support for this API in OpenSSL 1.1.0, the code is
> available at https://github.com/Mellanox/tls-openssl/tree/master
>
> It should work with any TLS library with similar modifications,
> a test tool using gnutls is here: https://github.com/Mellanox/tls-af_ktls_tool
>
> RFC patch to openssl:
> https://mta.openssl.org/pipermail/openssl-dev/2017-June/009384.html
>
> Changes from V2:
>
> * EXPORT_SYMBOL_GPL in patch 1
> * Ensure cleanup code always called before sk_stream_kill_queues to
>   avoid warnings
>
> Changes from V1:
>
> * EXPORT_SYMBOL GPL in patch 2
> * Add link to OpenSSL patch & gnutls example in documentation patch.
> * sk_write_pending check was rolled in to wait_for_memory path,
>   avoids special case and fixes lock inbalance issue.
> * Unify flag handling for sendmsg/sendfile
>
> Changes from RFC V2:
>
> * Generic ULP (upper layer protocol) framework instead of TLS specific
>   setsockopts
> * Dropped Mellanox hardware patches, will come as separate series.
>   Framework will work for both.
>
> RFC V2:
>
> http://www.mail-archive.com/netdev@vger.kernel.org/msg160317.html
>
> Changes from RFC V1:
>
> * Socket based on changing TCP proto_ops instead of crypto framework
> * Merged code with Mellanox's hardware tls offload
> * Zerocopy sendmsg support added - sendpage/sendfile is no longer
>   necessary for zerocopy optimization
>
> RFC V1:
>
> http://www.mail-archive.com/netdev@vger.kernel.org/msg88021.html
>
> * Socket based on crypto userspace API framework, required two
>   sockets in userspace, one encrypted, one unencrypted.
>
> Paper: https://netdevconf.org/1.2/papers/ktls.pdf
>
> Aviad Yehezkel (1):
>   tcp: export do_tcp_sendpages and tcp_rate_check_app_limited functions
>
> Boris Pismenny (2):
>   tcp: ULP infrastructure
>   tls: Documentation
>
> Ilya Lesokhin (1):
>   tls: kernel TLS support
>
>  Documentation/networking/tls.txt   | 135 +++
>  MAINTAINERS|  10 +
>  include/linux/socket.h |   1 +
>  include/net/inet_connection_sock.h |   4 +
>  include/net/tcp.h  |  27 ++
>  include/net/tls.h  | 237 
>  include/uapi/linux/tcp.h   |   1 +
>  include/uapi/linux/tls.h   |  79 
>  net/Kconfig|   1 +
>  net/Makefile   |   1 +
>  net/ipv4/Makefile  |   2 +-
>  net/ipv4/sysctl_net_ipv4.c |  25 ++
>  net/ipv4/tcp.c |  33 +-
>  net/ipv4/tcp_ipv4.c|   2 +
>  net/ipv4/tcp_rate.c|   1 +
>  net/ipv4/tcp_ulp.c | 134 +++
>  net/tls/Kconfig|  12 +
>  net/tls/Makefile   |   7 +
>  net/tls/tls_main.c | 487 +++
>  net/tls/tls_sw.c   | 772 
> +
>  20 files changed, 1968 insertions(+), 3 deletions(-)
>  create mode 100644 Documentation/networking/tls.txt
>  create mode 100644 include/net/tls.h
>  create mode 100644 include/uapi/linux/tls.h
>  create mode 100644 net/ipv4/tcp_ulp.c
>  create mode 100644 net/tls/Kconfig
>  create mode 100644 net/tls/Makefile
>  create mode 100644 net/tls/tls_main.c
>  create mode 100644 net/tls/tls_sw.c
>
> --
> 2.9.3
>

Re: [PATCH v3 net-next 0/4] kernel TLS

[Hannes Frederic Sowa]

Hello Dave,

On Wed, Jun 14, 2017, at 21:47, David Miller wrote:
> From: Dave Watson 
> Date: Wed, 14 Jun 2017 11:36:54 -0700
> 
> > This series adds support for kernel TLS encryption over TCP sockets.
> > A standard TCP socket is converted to a TLS socket using a setsockopt.
> > Only symmetric crypto is done in the kernel, as well as TLS record
> > framing.  The handshake remains in userspace, and the negotiated
> > cipher keys/iv are provided to the TCP socket.
> > 
> > We implemented support for this API in OpenSSL 1.1.0, the code is
> > available at https://github.com/Mellanox/tls-openssl/tree/master
> > 
> > It should work with any TLS library with similar modifications,
> > a test tool using gnutls is here: 
> > https://github.com/Mellanox/tls-af_ktls_tool
> > 
> > RFC patch to openssl:
> > https://mta.openssl.org/pipermail/openssl-dev/2017-June/009384.html
>  ...
> 
> I really want to apply this, so everyone give it a good review :-)

one question for this patch set:

What is the reason for not allowing key updates for the TX path? I was
always loud pointing out the problems with TLSv1.2 renegotiation and
TLSv1.3 key update alerts. This patch set uses encryption in a
synchronous way directly in the socket layer and thus wouldn't suffer
from problems regarding updates of the key. My hunch is that you leave
this option open so you can later on introduce asynchronous crypto which
might be used on hardware? It looks also be doable in case of MSG_MORE.
Otherwise by allowing key updates to the data path I would not see any
problems with key updates in TLS.

The reason why I am asking is that it is hard to predict how many bytes
will be send through a connection. TLSv1.3 recommends (SHOULD)
implementation writes to update the key after 362GB, afaik NIST even has
a lower margin. After that the symmetric crypto might become too weak.

Anyway, this patch seems easy and maybe with key updates added later on
doesn't seem to have any problems pointed out by me so far.

Thanks,
Hannes

Re: [PATCH v3 net-next 0/4] kernel TLS

[David Miller]

From: Dave Watson 
Date: Wed, 14 Jun 2017 11:36:54 -0700

> This series adds support for kernel TLS encryption over TCP sockets.
> A standard TCP socket is converted to a TLS socket using a setsockopt.
> Only symmetric crypto is done in the kernel, as well as TLS record
> framing.  The handshake remains in userspace, and the negotiated
> cipher keys/iv are provided to the TCP socket.
> 
> We implemented support for this API in OpenSSL 1.1.0, the code is
> available at https://github.com/Mellanox/tls-openssl/tree/master
> 
> It should work with any TLS library with similar modifications,
> a test tool using gnutls is here: https://github.com/Mellanox/tls-af_ktls_tool
> 
> RFC patch to openssl:
> https://mta.openssl.org/pipermail/openssl-dev/2017-June/009384.html
 ...

I really want to apply this, so everyone give it a good review :-)