Re: [PATCH RFC 4/5] tls: RX path for ktls

[Dave Watson]

On 03/08/18 09:48 PM, Boris Pismenny wrote:
> Hi Dave,
> 
> On 03/08/18 18:50, Dave Watson wrote:
> > Add rx path for tls software implementation.
> > 
> > recvmsg, splice_read, and poll implemented.
> > 
> > An additional sockopt TLS_RX is added, with the same interface as
> > TLS_TX.  Either TLX_RX or TLX_TX may be provided separately, or
> > together (with two different setsockopt calls with appropriate keys).
> > 
> > Control messages are passed via CMSG in a similar way to transmit.
> > If no cmsg buffer is passed, then only application data records
> > will be passed to userspace, and EIO is returned for other types of
> > alerts.
> > 
> > EBADMSG is passed for decryption errors, and E2BIG is passed for framing
> > errors.  Both are unrecoverable.
> 
> I think E2BIG is for too long argument list. EMSGSIZE might be more
> appropriate.

Sounds good.

> Also, we must check that the record is not too short (cipher specific).
> For TLS1.2 with AES-GCM the minimum length is 8 (IV) + 16 (TAG).
> The correct error for this case is EBADMSG, like a decryption failure.
> 
> Also, how about bad TLS version (e.g. not TLS1.2)?
> A separate error type is required for bad version, because it triggers a
> unique alert in libraries such as OpenSSL.
> I thought of using EINVAL for bad version. What do you think?

Ah, I did not realize there was a separate alert for that, sounds good.

> 
> I wonder if we should provide a more flexible method of obtaining errors for
> the future.
> Maybe use a special CMSG for errors?
> This CMSG will be triggered only after the socket enters the error state.

I'm not opposed to this in principle, but without a concrete use am
hesitant to add it.  I don't know of any other error codes that could
be returned besides the ones discussed above.

> > 
> > +
> > +int tls_sw_recvmsg(struct sock *sk,
> > +  struct msghdr *msg,
> > +  size_t len,
> > +  int nonblock,
> > +  int flags,
> > +  int *addr_len)
> > +{
> > +   struct tls_context *tls_ctx = tls_get_ctx(sk);
> > +   struct tls_sw_context *ctx = tls_sw_ctx(tls_ctx);
> > +   unsigned char control;
> > +   struct strp_msg *rxm;
> > +   struct sk_buff *skb;
> > +   ssize_t copied = 0;
> > +   bool cmsg = false;
> > +   int err = 0;
> > +   long timeo;
> Maybe try to read from the error queue here?

Sure.

Re: [PATCH RFC 4/5] tls: RX path for ktls

[Boris Pismenny]

Hi Dave,

On 03/08/18 18:50, Dave Watson wrote:

Add rx path for tls software implementation.

recvmsg, splice_read, and poll implemented.

An additional sockopt TLS_RX is added, with the same interface as
TLS_TX.  Either TLX_RX or TLX_TX may be provided separately, or
together (with two different setsockopt calls with appropriate keys).

Control messages are passed via CMSG in a similar way to transmit.
If no cmsg buffer is passed, then only application data records
will be passed to userspace, and EIO is returned for other types of
alerts.

EBADMSG is passed for decryption errors, and E2BIG is passed for framing
errors.  Both are unrecoverable.


I think E2BIG is for too long argument list. EMSGSIZE might be more 
appropriate.

Also, we must check that the record is not too short (cipher specific).
For TLS1.2 with AES-GCM the minimum length is 8 (IV) + 16 (TAG).
The correct error for this case is EBADMSG, like a decryption failure.

Also, how about bad TLS version (e.g. not TLS1.2)?
A separate error type is required for bad version, because it triggers a 
unique alert in libraries such as OpenSSL.

I thought of using EINVAL for bad version. What do you think?

I wonder if we should provide a more flexible method of obtaining errors 
for the future.

Maybe use a special CMSG for errors?
This CMSG will be triggered only after the socket enters the error state.



+
+int tls_sw_recvmsg(struct sock *sk,
+  struct msghdr *msg,
+  size_t len,
+  int nonblock,
+  int flags,
+  int *addr_len)
+{
+   struct tls_context *tls_ctx = tls_get_ctx(sk);
+   struct tls_sw_context *ctx = tls_sw_ctx(tls_ctx);
+   unsigned char control;
+   struct strp_msg *rxm;
+   struct sk_buff *skb;
+   ssize_t copied = 0;
+   bool cmsg = false;
+   int err = 0;
+   long timeo;

Maybe try to read from the error queue here?

+
+   flags |= nonblock;
+
+   lock_sock(sk);
+
+   timeo = sock_rcvtimeo(sk, flags & MSG_DONTWAIT);
+   do {
+   bool zc = false;
+   int chunk = 0;
+
+   skb = tls_wait_data(sk, flags, timeo, );
+   if (!skb)
+   goto recv_end;
+
+   rxm = strp_msg(skb);
+   if (!cmsg) {
+   int cerr;
+
+   cerr = put_cmsg(msg, SOL_TLS, TLS_GET_RECORD_TYPE,
+   sizeof(ctx->control), >control);
+   cmsg = true;
+   control = ctx->control;
+   if (ctx->control != TLS_RECORD_TYPE_DATA) {
+   if (cerr || msg->msg_flags & MSG_CTRUNC) {
+   err = -EIO;
+   goto recv_end;
+   }
+   }
+   } else if (control != ctx->control) {
+   goto recv_end;
+   }
+
+   if (!ctx->decrypted) {
+   int page_count;
+   int to_copy;
+
+   page_count = iov_iter_npages(>msg_iter,
+MAX_SKB_FRAGS);
+   to_copy = rxm->full_len - tls_ctx->rx.overhead_size;
+   if (to_copy <= len && page_count < MAX_SKB_FRAGS &&
+   likely(!(flags & MSG_PEEK)))  {
+   struct scatterlist sgin[MAX_SKB_FRAGS + 1];
+   char unused[21];
+   int pages = 0;
+
+   zc = true;
+   sg_init_table(sgin, MAX_SKB_FRAGS + 1);
+   sg_set_buf([0], unused, 13);
+
+   err = zerocopy_from_iter(sk, >msg_iter,
+to_copy, ,
+, [1],
+MAX_SKB_FRAGS, false);
+   if (err < 0)
+   goto fallback_to_reg_recv;
+
+   err = decrypt_skb(sk, skb, sgin);
+   for (; pages > 0; pages--)
+   put_page(sg_page([pages]));
+   if (err < 0) {
+   tls_err_abort(sk, EBADMSG);
+   goto recv_end;
+   }
+   } else {
+fallback_to_reg_recv:
+   err = decrypt_skb(sk, skb, NULL);
+   if (err < 0) {
+   tls_err_abort(sk, EBADMSG);
+   goto recv_end;
+   }
+   }
+   

[PATCH RFC 4/5] tls: RX path for ktls

[Dave Watson]

Add rx path for tls software implementation.

recvmsg, splice_read, and poll implemented.

An additional sockopt TLS_RX is added, with the same interface as
TLS_TX.  Either TLX_RX or TLX_TX may be provided separately, or
together (with two different setsockopt calls with appropriate keys).

Control messages are passed via CMSG in a similar way to transmit.
If no cmsg buffer is passed, then only application data records
will be passed to userspace, and EIO is returned for other types of
alerts.

EBADMSG is passed for decryption errors, and E2BIG is passed for framing
errors.  Both are unrecoverable.

strparser is used to parse TLS framing.   Decryption is done directly
in to userspace buffers if they are large enough to support it, otherwise
sk_cow_data is called (similar to ipsec), and buffers are decrypted in
place and copied.  splice_read always decrypts in place, since no
buffers are provided to decrypt in to.

sk_poll is overridden, and only returns POLLIN if a full TLS message is
received.  Otherwise we wait for strparser to finish reading a full frame.
Actual decryption is only done during recvmsg or splice_read calls.

Signed-off-by: Dave Watson 
---
 include/net/tls.h|  27 ++-
 include/uapi/linux/tls.h |   2 +
 net/tls/Kconfig  |   1 +
 net/tls/tls_main.c   |  62 -
 net/tls/tls_sw.c | 574 ++-
 5 files changed, 596 insertions(+), 70 deletions(-)

diff --git a/include/net/tls.h b/include/net/tls.h
index 6b44875..7202026 100644
--- a/include/net/tls.h
+++ b/include/net/tls.h
@@ -40,6 +40,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include 
 
@@ -58,8 +59,18 @@
 
 struct tls_sw_context {
struct crypto_aead *aead_send;
+   struct crypto_aead *aead_recv;
struct crypto_wait async_wait;
 
+   /* Receive context */
+   struct strparser strp;
+   void (*saved_data_ready)(struct sock *sk);
+   unsigned int (*sk_poll)(struct file *file, struct socket *sock,
+   struct poll_table_struct *wait);
+   struct sk_buff *recv_pkt;
+   u8 control;
+   bool decrypted;
+
/* Sending context */
char aad_space[TLS_AAD_SPACE_SIZE];
 
@@ -96,12 +107,17 @@ struct tls_context {
struct tls_crypto_info crypto_send;
struct tls12_crypto_info_aes_gcm_128 crypto_send_aes_gcm_128;
};
+   union {
+   struct tls_crypto_info crypto_recv;
+   struct tls12_crypto_info_aes_gcm_128 crypto_recv_aes_gcm_128;
+   };
 
void *priv_ctx;
 
u8 tx_conf:2;
 
struct cipher_context tx;
+   struct cipher_context rx;
 
struct scatterlist *partially_sent_record;
u16 partially_sent_offset;
@@ -128,12 +144,19 @@ int tls_sk_attach(struct sock *sk, int optname, char 
__user *optval,
  unsigned int optlen);
 
 
-int tls_set_sw_offload(struct sock *sk, struct tls_context *ctx);
+int tls_set_sw_offload(struct sock *sk, struct tls_context *ctx, int tx);
 int tls_sw_sendmsg(struct sock *sk, struct msghdr *msg, size_t size);
 int tls_sw_sendpage(struct sock *sk, struct page *page,
int offset, size_t size, int flags);
 void tls_sw_close(struct sock *sk, long timeout);
-void tls_sw_free_tx_resources(struct sock *sk);
+void tls_sw_free_resources(struct sock *sk);
+int tls_sw_recvmsg(struct sock *sk, struct msghdr *msg, size_t len,
+  int nonblock, int flags, int *addr_len);
+unsigned int tls_sw_poll(struct file *file, struct socket *sock,
+struct poll_table_struct *wait);
+ssize_t tls_sw_splice_read(struct socket *sock, loff_t *ppos,
+  struct pipe_inode_info *pipe,
+  size_t len, unsigned int flags);
 
 void tls_sk_destruct(struct sock *sk, struct tls_context *ctx);
 void tls_icsk_clean_acked(struct sock *sk);
diff --git a/include/uapi/linux/tls.h b/include/uapi/linux/tls.h
index 293b2cd..c6633e9 100644
--- a/include/uapi/linux/tls.h
+++ b/include/uapi/linux/tls.h
@@ -38,6 +38,7 @@
 
 /* TLS socket options */
 #define TLS_TX 1   /* Set transmit parameters */
+#define TLS_RX 2   /* Set receive parameters */
 
 /* Supported versions */
 #define TLS_VERSION_MINOR(ver) ((ver) & 0xFF)
@@ -59,6 +60,7 @@
 #define TLS_CIPHER_AES_GCM_128_REC_SEQ_SIZE8
 
 #define TLS_SET_RECORD_TYPE1
+#define TLS_GET_RECORD_TYPE2
 
 struct tls_crypto_info {
__u16 version;
diff --git a/net/tls/Kconfig b/net/tls/Kconfig
index eb58303..89b8745a 100644
--- a/net/tls/Kconfig
+++ b/net/tls/Kconfig
@@ -7,6 +7,7 @@ config TLS
select CRYPTO
select CRYPTO_AES
select CRYPTO_GCM
+   select STREAM_PARSER
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 c671560..9a6d533