diff mbox series

crypto tls session: GNUTLS internal buffer is now cleared of stale data

Message ID PH0PR15MB448020F90D7E38336344FF92F3029@PH0PR15MB4480.namprd15.prod.outlook.com
State New
Headers show
Series crypto tls session: GNUTLS internal buffer is now cleared of stale data | expand

Commit Message

Marcks, Harrison June 29, 2021, 3:50 p.m. UTC
From 7a95cd3f827be55153e7e457caa89351c48f247d Mon Sep 17 00:00:00 2001
From: harrison marcks <harrison.marcks@ncr.com>
Date: Tue, 29 Jun 2021 16:50:00 +0100
Subject: [PATCH] crypto tls session: GNUTLS internal buffer is now cleared of
 stale data

QEMU Serial devices only request up their "ITL" level. As the GNUTLS is
not a socket proper, if the data on the line exceeds this level then QEMU
won't go back and look for it. The fix adds a watch on the tls channel
that uses an internal gnutls function to check pending records. Then
changes the condition in the tls-channel watch to read from the buffer
again (if there are indeed records still pending)

Signed-off-by: harrison marcks <harrison.marcks@ncr.com>
---
 crypto/tlssession.c         | 18 ++++++++
 crypto/trace-events         |  1 +
 include/crypto/tlssession.h | 28 ++++++++++++
 include/io/channel-tls.h    |  2 +
 io/channel-tls.c            | 89 ++++++++++++++++++++++++++++++++++++-
 io/trace-events             |  7 +++
 6 files changed, 144 insertions(+), 1 deletion(-)

Comments

Marcks, Harrison July 5, 2021, 9:13 a.m. UTC | #1
ping
https://lists.nongnu.org/archive/html/qemu-devel/2021-06/msg07713.html

-----Original Message-----
From: Marcks, Harrison <Harrison.Marcks@ncr.com> 
Sent: 29 June 2021 16:50
To: qemu-devel@nongnu.org
Cc: berrange@redhat.com; Marcks, Harrison <Harrison.Marcks@ncr.com>
Subject: [PATCH] crypto tls session: GNUTLS internal buffer is now cleared of stale data

From 7a95cd3f827be55153e7e457caa89351c48f247d Mon Sep 17 00:00:00 2001
From: harrison marcks <harrison.marcks@ncr.com>
Date: Tue, 29 Jun 2021 16:50:00 +0100
Subject: [PATCH] crypto tls session: GNUTLS internal buffer is now cleared of  stale data

QEMU Serial devices only request up their "ITL" level. As the GNUTLS is not a socket proper, if the data on the line exceeds this level then QEMU won't go back and look for it. The fix adds a watch on the tls channel that uses an internal gnutls function to check pending records. Then changes the condition in the tls-channel watch to read from the buffer again (if there are indeed records still pending)

Signed-off-by: harrison marcks <harrison.marcks@ncr.com>
---
 crypto/tlssession.c         | 18 ++++++++
 crypto/trace-events         |  1 +
 include/crypto/tlssession.h | 28 ++++++++++++
 include/io/channel-tls.h    |  2 +
 io/channel-tls.c            | 89 ++++++++++++++++++++++++++++++++++++-
 io/trace-events             |  7 +++
 6 files changed, 144 insertions(+), 1 deletion(-)

diff --git a/crypto/tlssession.c b/crypto/tlssession.c index 33203e8ca7..94bd464516 100644
--- a/crypto/tlssession.c
+++ b/crypto/tlssession.c
@@ -457,6 +457,24 @@ qcrypto_tls_session_write(QCryptoTLSSession *session,  }
 
 
+QCryptoTLSSessionRecordStatus
+qcrypto_tls_session_read_check_buffer(QCryptoTLSSession *session) {
+    QCryptoTLSSessionRecordStatus status = QCRYPTO_TLS_RECORD_EMPTY;
+
+    if (gnutls_record_check_pending(session->handle) > 0) {
+        status = QCRYPTO_TLS_RECORDS_PENDING;
+    }
+
+    trace_qcrypto_tls_session_read_check_buffer(
+                                        session
+                                        , status
+                                        , gnutls_record_check_pending(session->handle)
+                                        );
+    return status;
+}
+
+
 ssize_t
 qcrypto_tls_session_read(QCryptoTLSSession *session,
                          char *buf,
diff --git a/crypto/trace-events b/crypto/trace-events index 9e594d30e8..d47edf4050 100644
--- a/crypto/trace-events
+++ b/crypto/trace-events
@@ -21,3 +21,4 @@ qcrypto_tls_creds_x509_load_cert_list(void *creds, const char *file) "TLS creds  # tlssession.c  qcrypto_tls_session_new(void *session, void *creds, const char *hostname, const char *authzid, int endpoint) "TLS session new session=%p creds=%p hostname=%s authzid=%s endpoint=%d"
 qcrypto_tls_session_check_creds(void *session, const char *status) "TLS session check creds session=%p status=%s"
+qcrypto_tls_session_read_check_buffer(void* session, int status, long recordsN) "TLS session buffer check session=%p status=%d records pending=%ld"
diff --git a/include/crypto/tlssession.h b/include/crypto/tlssession.h index 15b9cef086..4108271d62 100644
--- a/include/crypto/tlssession.h
+++ b/include/crypto/tlssession.h
@@ -321,4 +321,32 @@ int qcrypto_tls_session_get_key_size(QCryptoTLSSession *sess,
  */
 char *qcrypto_tls_session_get_peer_name(QCryptoTLSSession *sess);
 
+/**
+ * QCryptoTLSSessionRecordStatus:
+ * enum definitions for telling outside functions whether
+ * there are "records"/bytes waiting to be read in GNUTLS.
+ *
+ * QCRYPTO_TLS_RECORD_NULL is an empty/init state
+ *
+ */
+typedef enum {
+    QCRYPTO_TLS_RECORD_NULL, /* empty state */
+    QCRYPTO_TLS_RECORD_EMPTY,
+    QCRYPTO_TLS_RECORDS_PENDING
+} QCryptoTLSSessionRecordStatus;
+
+/**
+ * qcrypto_tls_session_read_check_buffer:
+ * @session: the TLS session object
+ *
+ * checks the internal GNUTLS buffer for the remaining bytes and
+ * returns one of:
+ *      QCRYPTO_TLS_RECORD_EMPTY - No pending bytes
+ *      QCRYPTO_TLS_RECORDS_PENDING - pending bytes
+ *
+ * Returns: QCryptoTLSSessionRecordStatus  */ 
+QCryptoTLSSessionRecordStatus 
+qcrypto_tls_session_read_check_buffer(QCryptoTLSSession *session);
+
 #endif /* QCRYPTO_TLSSESSION_H */
diff --git a/include/io/channel-tls.h b/include/io/channel-tls.h index fdbdf12feb..401fe786a2 100644
--- a/include/io/channel-tls.h
+++ b/include/io/channel-tls.h
@@ -29,6 +29,7 @@
 #define QIO_CHANNEL_TLS(obj)                                     \
     OBJECT_CHECK(QIOChannelTLS, (obj), TYPE_QIO_CHANNEL_TLS)
 
+typedef struct QIOChannelTLSSource QIOChannelTLSSource;
 typedef struct QIOChannelTLS QIOChannelTLS;
 
 /**
@@ -49,6 +50,7 @@ struct QIOChannelTLS {
     QIOChannel *master;
     QCryptoTLSSession *session;
     QIOChannelShutdown shutdown;
+    QIOChannelTLSSource *source;
 };
 
 /**
diff --git a/io/channel-tls.c b/io/channel-tls.c index 7ec8ceff2f..77b80f2bf8 100644
--- a/io/channel-tls.c
+++ b/io/channel-tls.c
@@ -24,6 +24,12 @@
 #include "io/channel-tls.h"
 #include "trace.h"
 
+struct QIOChannelTLSSource {
+    GSource parent;
+    GIOCondition condition;
+    QIOChannelTLS *tioc;
+};
+
 
 static ssize_t qio_channel_tls_write_handler(const char *buf,
                                              size_t len, @@ -269,6 +275,10 @@ static ssize_t qio_channel_tls_readv(QIOChannel *ioc,
         ssize_t ret = qcrypto_tls_session_read(tioc->session,
                                                iov[i].iov_base,
                                                iov[i].iov_len);
+        trace_qio_channel_tls_readv_iov_len(tioc->session
+                                            , iov[i].iov_base
+                                            , iov[i].iov_len);
+
         if (ret < 0) {
             if (errno == EAGAIN) {
                 if (got) {
@@ -290,6 +300,15 @@ static ssize_t qio_channel_tls_readv(QIOChannel *ioc,
             break;
         }
     }
+
+    if (qcrypto_tls_session_read_check_buffer(tioc->session)
+        == QCRYPTO_TLS_RECORDS_PENDING) {
+        tioc->source->condition |= G_IO_IN;
+        trace_qio_channel_tls_readv_extra_records(tioc->session
+                                                , tioc->source->condition
+                                                , got);
+    }
+
     return got;
 }
 
@@ -385,12 +404,80 @@ static void qio_channel_tls_set_aio_fd_handler(QIOChannel *ioc,
     qio_channel_set_aio_fd_handler(tioc->master, ctx, io_read, io_write, opaque);  }
 
+static gboolean
+qio_channel_tls_source_check(GSource *source);
+
+static gboolean
+qio_channel_tls_source_prepare(GSource *source,
+                                gint *timeout) {
+    *timeout = -1;
+    trace_qio_channel_tls_source_prepare(source, *timeout);
+    return qio_channel_tls_source_check(source);
+}
+
+static gboolean
+qio_channel_tls_source_check(GSource *source) {
+    QIOChannelTLSSource *tsource = (QIOChannelTLSSource *)source;
+    trace_qio_channel_tls_source_check(tsource->condition);
+    return (G_IO_IN | G_IO_OUT) & tsource->condition; }
+
+static gboolean
+qio_channel_tls_source_dispatch(GSource *source,
+                                   GSourceFunc callback,
+                                   gpointer user_data) {
+    QIOChannelFunc func = (QIOChannelFunc)callback;
+    QIOChannelTLSSource *tsource = (QIOChannelTLSSource *)source;
+    trace_qio_channel_tls_source_dispatch(tsource->tioc
+                            , tsource->condition
+                            , func);
+
+    return (*func)(QIO_CHANNEL(tsource->tioc),
+                   ((G_IO_IN | G_IO_OUT) & tsource->condition),
+                   user_data);
+}
+
+static void
+qio_channel_tls_source_finalize(GSource *source) {
+    QIOChannelTLSSource *tsource = (QIOChannelTLSSource *)source;
+    trace_qio_channel_tls_source_finalize(tsource, tsource->tioc);
+
+    object_unref(OBJECT(tsource->tioc));
+}
+
+GSourceFuncs qio_channel_tls_source_funcs = {
+    qio_channel_tls_source_prepare,
+    qio_channel_tls_source_check,
+    qio_channel_tls_source_dispatch,
+    qio_channel_tls_source_finalize
+};
+
+
 static GSource *qio_channel_tls_create_watch(QIOChannel *ioc,
                                              GIOCondition condition)  {
     QIOChannelTLS *tioc = QIO_CHANNEL_TLS(ioc);
 
-    return qio_channel_create_watch(tioc->master, condition);
+    QIOChannelTLSSource *tsource;
+    GSource *source;
+
+    source = g_source_new(&qio_channel_tls_source_funcs,
+                          sizeof(QIOChannelTLSSource));
+    tsource = (QIOChannelTLSSource *)source;
+
+    tsource->tioc = tioc;
+    tioc->source = tsource;
+    object_ref(OBJECT(tioc));
+
+    tsource->condition = condition;
+    trace_qio_channel_tls_create_watch(tsource
+                                        , tioc
+                                        , condition);
+    return source;
 }
 
 QCryptoTLSSession *
diff --git a/io/trace-events b/io/trace-events index d7bc70b966..2f82c15e68 100644
--- a/io/trace-events
+++ b/io/trace-events
@@ -42,6 +42,13 @@ qio_channel_tls_handshake_fail(void *ioc) "TLS handshake fail ioc=%p"
 qio_channel_tls_handshake_complete(void *ioc) "TLS handshake complete ioc=%p"
 qio_channel_tls_credentials_allow(void *ioc) "TLS credentials allow ioc=%p"
 qio_channel_tls_credentials_deny(void *ioc) "TLS credentials deny ioc=%p"
+qio_channel_tls_source_prepare(void *source, int timeout) "TLS source=%p prepare timeout=%d"
+qio_channel_tls_source_check(int condition) "TLS source condition=%d"
+qio_channel_tls_source_dispatch(void* channel_p, int condition, void* func) "TLS dispatch source=%p condition=%d callback func=%p"
+qio_channel_tls_source_finalize(void* source_p, void* channel_p) "TLS source finalize source=%p ioc=%p"
+qio_channel_tls_create_watch(void* tsource, void* tioc, int condition) "TLS create watch source=%p channel=%p condition=%d"
+qio_channel_tls_readv_extra_records(void* session_p, int condition, long got) "TLS readv extra recs session=%p condition=%d got=%ld"
+qio_channel_tls_readv_iov_len(void* session_p, void* iov_base, long iov_len) "TLS readv iov len session=%p iov_base=%p iov_len=%ld"
 
 # channel-websock.c
 qio_channel_websock_new_server(void *ioc, void *master) "Websock new client ioc=%p master=%p"
--
2.17.1
Daniel P. Berrangé July 5, 2021, 9:32 a.m. UTC | #2
On Tue, Jun 29, 2021 at 03:50:04PM +0000, Marcks, Harrison wrote:
> From 7a95cd3f827be55153e7e457caa89351c48f247d Mon Sep 17 00:00:00 2001
> From: harrison marcks <harrison.marcks@ncr.com>
> Date: Tue, 29 Jun 2021 16:50:00 +0100
> Subject: [PATCH] crypto tls session: GNUTLS internal buffer is now cleared of
>  stale data
> 
> QEMU Serial devices only request up their "ITL" level. As the GNUTLS is
> not a socket proper, if the data on the line exceeds this level then QEMU
> won't go back and look for it. The fix adds a watch on the tls channel
> that uses an internal gnutls function to check pending records. Then
> changes the condition in the tls-channel watch to read from the buffer
> again (if there are indeed records still pending)
> 
> Signed-off-by: harrison marcks <harrison.marcks@ncr.com>
> ---
>  crypto/tlssession.c         | 18 ++++++++
>  crypto/trace-events         |  1 +
>  include/crypto/tlssession.h | 28 ++++++++++++
>  include/io/channel-tls.h    |  2 +
>  io/channel-tls.c            | 89 ++++++++++++++++++++++++++++++++++++-
>  io/trace-events             |  7 +++
>  6 files changed, 144 insertions(+), 1 deletion(-)

Unfortunately this patch doesn't apply to git master:

  https://patchew.org/QEMU/PH0PR15MB448020F90D7E38336344FF92F3029@PH0PR15MB4480.namprd15.prod.outlook.com/

> diff --git a/crypto/tlssession.c b/crypto/tlssession.c
> index 33203e8ca7..94bd464516 100644
> --- a/crypto/tlssession.c
> +++ b/crypto/tlssession.c
> @@ -457,6 +457,24 @@ qcrypto_tls_session_write(QCryptoTLSSession *session,
>  }
>  
>  
> +QCryptoTLSSessionRecordStatus
> +qcrypto_tls_session_read_check_buffer(QCryptoTLSSession *session)
> +{
> +    QCryptoTLSSessionRecordStatus status = QCRYPTO_TLS_RECORD_EMPTY;
> +
> +    if (gnutls_record_check_pending(session->handle) > 0) {
> +        status = QCRYPTO_TLS_RECORDS_PENDING;
> +    }
> +
> +    trace_qcrypto_tls_session_read_check_buffer(
> +                                        session
> +                                        , status

Putting the ',' on a new line is not a QEMU codestyle.

> +                                        , gnutls_record_check_pending(session->handle)
> +                                        );

The ); shouldn't be on a new line either

> +    return status;
> +}
> +
> +
>  ssize_t
>  qcrypto_tls_session_read(QCryptoTLSSession *session,
>                           char *buf,
> diff --git a/crypto/trace-events b/crypto/trace-events
> index 9e594d30e8..d47edf4050 100644
> --- a/crypto/trace-events
> +++ b/crypto/trace-events
> @@ -21,3 +21,4 @@ qcrypto_tls_creds_x509_load_cert_list(void *creds, const char *file) "TLS creds
>  # tlssession.c
>  qcrypto_tls_session_new(void *session, void *creds, const char *hostname, const char *authzid, int endpoint) "TLS session new session=%p creds=%p hostname=%s authzid=%s endpoint=%d"
>  qcrypto_tls_session_check_creds(void *session, const char *status) "TLS session check creds session=%p status=%s"
> +qcrypto_tls_session_read_check_buffer(void* session, int status, long recordsN) "TLS session buffer check session=%p status=%d records pending=%ld"

QEMU code style is 'void *' not 'void*' - as seen in the line above

> diff --git a/include/crypto/tlssession.h b/include/crypto/tlssession.h
> index 15b9cef086..4108271d62 100644
> --- a/include/crypto/tlssession.h
> +++ b/include/crypto/tlssession.h
> @@ -321,4 +321,32 @@ int qcrypto_tls_session_get_key_size(QCryptoTLSSession *sess,
>   */
>  char *qcrypto_tls_session_get_peer_name(QCryptoTLSSession *sess);
>  
> +/**
> + * QCryptoTLSSessionRecordStatus:
> + * enum definitions for telling outside functions whether
> + * there are "records"/bytes waiting to be read in GNUTLS.
> + *
> + * QCRYPTO_TLS_RECORD_NULL is an empty/init state
> + *
> + */
> +typedef enum {
> +    QCRYPTO_TLS_RECORD_NULL, /* empty state */
> +    QCRYPTO_TLS_RECORD_EMPTY,
> +    QCRYPTO_TLS_RECORDS_PENDING
> +} QCryptoTLSSessionRecordStatus;
> +
> +/**
> + * qcrypto_tls_session_read_check_buffer:
> + * @session: the TLS session object
> + *
> + * checks the internal GNUTLS buffer for the remaining bytes and
> + * returns one of:
> + *      QCRYPTO_TLS_RECORD_EMPTY - No pending bytes
> + *      QCRYPTO_TLS_RECORDS_PENDING - pending bytes
> + *
> + * Returns: QCryptoTLSSessionRecordStatus
> + */
> +QCryptoTLSSessionRecordStatus
> +qcrypto_tls_session_read_check_buffer(QCryptoTLSSession *session);
> +
>  #endif /* QCRYPTO_TLSSESSION_H */
> diff --git a/include/io/channel-tls.h b/include/io/channel-tls.h
> index fdbdf12feb..401fe786a2 100644
> --- a/include/io/channel-tls.h
> +++ b/include/io/channel-tls.h
> @@ -29,6 +29,7 @@
>  #define QIO_CHANNEL_TLS(obj)                                     \
>      OBJECT_CHECK(QIOChannelTLS, (obj), TYPE_QIO_CHANNEL_TLS)
>  
> +typedef struct QIOChannelTLSSource QIOChannelTLSSource;
>  typedef struct QIOChannelTLS QIOChannelTLS;
>  
>  /**
> @@ -49,6 +50,7 @@ struct QIOChannelTLS {
>      QIOChannel *master;
>      QCryptoTLSSession *session;
>      QIOChannelShutdown shutdown;
> +    QIOChannelTLSSource *source;
>  };
>  
>  /**
> diff --git a/io/channel-tls.c b/io/channel-tls.c
> index 7ec8ceff2f..77b80f2bf8 100644
> --- a/io/channel-tls.c
> +++ b/io/channel-tls.c
> @@ -24,6 +24,12 @@
>  #include "io/channel-tls.h"
>  #include "trace.h"
>  
> +struct QIOChannelTLSSource {
> +    GSource parent;
> +    GIOCondition condition;
> +    QIOChannelTLS *tioc;
> +};
> +
>  
>  static ssize_t qio_channel_tls_write_handler(const char *buf,
>                                               size_t len,
> @@ -269,6 +275,10 @@ static ssize_t qio_channel_tls_readv(QIOChannel *ioc,
>          ssize_t ret = qcrypto_tls_session_read(tioc->session,
>                                                 iov[i].iov_base,
>                                                 iov[i].iov_len);
> +        trace_qio_channel_tls_readv_iov_len(tioc->session
> +                                            , iov[i].iov_base
> +                                            , iov[i].iov_len);

Again with this invalid code style.

>          if (ret < 0) {
>              if (errno == EAGAIN) {
>                  if (got) {
> @@ -290,6 +300,15 @@ static ssize_t qio_channel_tls_readv(QIOChannel *ioc,
>              break;
>          }
>      }
> +
> +    if (qcrypto_tls_session_read_check_buffer(tioc->session)
> +        == QCRYPTO_TLS_RECORDS_PENDING) {

"==" on the previous line

> +        tioc->source->condition |= G_IO_IN;

This will reference a NULL pointer if no source has been created

> +        trace_qio_channel_tls_readv_extra_records(tioc->session
> +                                                , tioc->source->condition
> +                                                , got);

And again

> +    }
> +
>      return got;
>  }
>  
> @@ -385,12 +404,80 @@ static void qio_channel_tls_set_aio_fd_handler(QIOChannel *ioc,
>      qio_channel_set_aio_fd_handler(tioc->master, ctx, io_read, io_write, opaque);
>  }
>  
> +static gboolean
> +qio_channel_tls_source_check(GSource *source);
> +
> +static gboolean
> +qio_channel_tls_source_prepare(GSource *source,
> +                                gint *timeout)

Parameters are not lined up

> +{
> +    *timeout = -1;
> +    trace_qio_channel_tls_source_prepare(source, *timeout);
> +    return qio_channel_tls_source_check(source);
> +}
> +
> +static gboolean
> +qio_channel_tls_source_check(GSource *source)
> +{
> +    QIOChannelTLSSource *tsource = (QIOChannelTLSSource *)source;
> +    trace_qio_channel_tls_source_check(tsource->condition);
> +    return (G_IO_IN | G_IO_OUT) & tsource->condition;
> +}
> +
> +static gboolean
> +qio_channel_tls_source_dispatch(GSource *source,
> +                                   GSourceFunc callback,
> +                                   gpointer user_data)
> +{
> +    QIOChannelFunc func = (QIOChannelFunc)callback;
> +    QIOChannelTLSSource *tsource = (QIOChannelTLSSource *)source;
> +    trace_qio_channel_tls_source_dispatch(tsource->tioc
> +                            , tsource->condition
> +                            , func);
> +
> +    return (*func)(QIO_CHANNEL(tsource->tioc),
> +                   ((G_IO_IN | G_IO_OUT) & tsource->condition),
> +                   user_data);
> +}
> +
> +static void
> +qio_channel_tls_source_finalize(GSource *source)
> +{
> +    QIOChannelTLSSource *tsource = (QIOChannelTLSSource *)source;
> +    trace_qio_channel_tls_source_finalize(tsource, tsource->tioc);
> +
> +    object_unref(OBJECT(tsource->tioc));
> +}
> +
> +GSourceFuncs qio_channel_tls_source_funcs = {
> +    qio_channel_tls_source_prepare,
> +    qio_channel_tls_source_check,
> +    qio_channel_tls_source_dispatch,
> +    qio_channel_tls_source_finalize
> +};
> +
> +
>  static GSource *qio_channel_tls_create_watch(QIOChannel *ioc,
>                                               GIOCondition condition)
>  {
>      QIOChannelTLS *tioc = QIO_CHANNEL_TLS(ioc);
>  
> -    return qio_channel_create_watch(tioc->master, condition);
> +    QIOChannelTLSSource *tsource;
> +    GSource *source;
> +
> +    source = g_source_new(&qio_channel_tls_source_funcs,
> +                          sizeof(QIOChannelTLSSource));
> +    tsource = (QIOChannelTLSSource *)source;
> +
> +    tsource->tioc = tioc;
> +    tioc->source = tsource;

So this assumes there's only ever 1 source present. That's certainly
the normal case, but I don't like making that assumption.

IMHO the link should be one way from QIOChannelTLSSource -> QIOChannelTLS.
Just keep a 'bool' flag in the QIOChannelTLS struct to indicate whether
there's pending data cached. This would avoid the NULL pointer problem
mentioned earlier.

> +    object_ref(OBJECT(tioc));
> +
> +    tsource->condition = condition;
> +    trace_qio_channel_tls_create_watch(tsource
> +                                        , tioc
> +                                        , condition);
> +    return source;
>  }
>  
>  QCryptoTLSSession *
> diff --git a/io/trace-events b/io/trace-events
> index d7bc70b966..2f82c15e68 100644
> --- a/io/trace-events
> +++ b/io/trace-events
> @@ -42,6 +42,13 @@ qio_channel_tls_handshake_fail(void *ioc) "TLS handshake fail ioc=%p"
>  qio_channel_tls_handshake_complete(void *ioc) "TLS handshake complete ioc=%p"
>  qio_channel_tls_credentials_allow(void *ioc) "TLS credentials allow ioc=%p"
>  qio_channel_tls_credentials_deny(void *ioc) "TLS credentials deny ioc=%p"
> +qio_channel_tls_source_prepare(void *source, int timeout) "TLS source=%p prepare timeout=%d"
> +qio_channel_tls_source_check(int condition) "TLS source condition=%d"
> +qio_channel_tls_source_dispatch(void* channel_p, int condition, void* func) "TLS dispatch source=%p condition=%d callback func=%p"

'void *' not 'void*'

> +qio_channel_tls_source_finalize(void* source_p, void* channel_p) "TLS source finalize source=%p ioc=%p"
> +qio_channel_tls_create_watch(void* tsource, void* tioc, int condition) "TLS create watch source=%p channel=%p condition=%d"
> +qio_channel_tls_readv_extra_records(void* session_p, int condition, long got) "TLS readv extra recs session=%p condition=%d got=%ld"
> +qio_channel_tls_readv_iov_len(void* session_p, void* iov_base, long iov_len) "TLS readv iov len session=%p iov_base=%p iov_len=%ld"
>  
>  # channel-websock.c
>  qio_channel_websock_new_server(void *ioc, void *master) "Websock new client ioc=%p master=%p"
> -- 
> 2.17.1
> 

Regards,
Daniel
diff mbox series

Patch

diff --git a/crypto/tlssession.c b/crypto/tlssession.c
index 33203e8ca7..94bd464516 100644
--- a/crypto/tlssession.c
+++ b/crypto/tlssession.c
@@ -457,6 +457,24 @@  qcrypto_tls_session_write(QCryptoTLSSession *session,
 }
 
 
+QCryptoTLSSessionRecordStatus
+qcrypto_tls_session_read_check_buffer(QCryptoTLSSession *session)
+{
+    QCryptoTLSSessionRecordStatus status = QCRYPTO_TLS_RECORD_EMPTY;
+
+    if (gnutls_record_check_pending(session->handle) > 0) {
+        status = QCRYPTO_TLS_RECORDS_PENDING;
+    }
+
+    trace_qcrypto_tls_session_read_check_buffer(
+                                        session
+                                        , status
+                                        , gnutls_record_check_pending(session->handle)
+                                        );
+    return status;
+}
+
+
 ssize_t
 qcrypto_tls_session_read(QCryptoTLSSession *session,
                          char *buf,
diff --git a/crypto/trace-events b/crypto/trace-events
index 9e594d30e8..d47edf4050 100644
--- a/crypto/trace-events
+++ b/crypto/trace-events
@@ -21,3 +21,4 @@  qcrypto_tls_creds_x509_load_cert_list(void *creds, const char *file) "TLS creds
 # tlssession.c
 qcrypto_tls_session_new(void *session, void *creds, const char *hostname, const char *authzid, int endpoint) "TLS session new session=%p creds=%p hostname=%s authzid=%s endpoint=%d"
 qcrypto_tls_session_check_creds(void *session, const char *status) "TLS session check creds session=%p status=%s"
+qcrypto_tls_session_read_check_buffer(void* session, int status, long recordsN) "TLS session buffer check session=%p status=%d records pending=%ld"
diff --git a/include/crypto/tlssession.h b/include/crypto/tlssession.h
index 15b9cef086..4108271d62 100644
--- a/include/crypto/tlssession.h
+++ b/include/crypto/tlssession.h
@@ -321,4 +321,32 @@  int qcrypto_tls_session_get_key_size(QCryptoTLSSession *sess,
  */
 char *qcrypto_tls_session_get_peer_name(QCryptoTLSSession *sess);
 
+/**
+ * QCryptoTLSSessionRecordStatus:
+ * enum definitions for telling outside functions whether
+ * there are "records"/bytes waiting to be read in GNUTLS.
+ *
+ * QCRYPTO_TLS_RECORD_NULL is an empty/init state
+ *
+ */
+typedef enum {
+    QCRYPTO_TLS_RECORD_NULL, /* empty state */
+    QCRYPTO_TLS_RECORD_EMPTY,
+    QCRYPTO_TLS_RECORDS_PENDING
+} QCryptoTLSSessionRecordStatus;
+
+/**
+ * qcrypto_tls_session_read_check_buffer:
+ * @session: the TLS session object
+ *
+ * checks the internal GNUTLS buffer for the remaining bytes and
+ * returns one of:
+ *      QCRYPTO_TLS_RECORD_EMPTY - No pending bytes
+ *      QCRYPTO_TLS_RECORDS_PENDING - pending bytes
+ *
+ * Returns: QCryptoTLSSessionRecordStatus
+ */
+QCryptoTLSSessionRecordStatus
+qcrypto_tls_session_read_check_buffer(QCryptoTLSSession *session);
+
 #endif /* QCRYPTO_TLSSESSION_H */
diff --git a/include/io/channel-tls.h b/include/io/channel-tls.h
index fdbdf12feb..401fe786a2 100644
--- a/include/io/channel-tls.h
+++ b/include/io/channel-tls.h
@@ -29,6 +29,7 @@ 
 #define QIO_CHANNEL_TLS(obj)                                     \
     OBJECT_CHECK(QIOChannelTLS, (obj), TYPE_QIO_CHANNEL_TLS)
 
+typedef struct QIOChannelTLSSource QIOChannelTLSSource;
 typedef struct QIOChannelTLS QIOChannelTLS;
 
 /**
@@ -49,6 +50,7 @@  struct QIOChannelTLS {
     QIOChannel *master;
     QCryptoTLSSession *session;
     QIOChannelShutdown shutdown;
+    QIOChannelTLSSource *source;
 };
 
 /**
diff --git a/io/channel-tls.c b/io/channel-tls.c
index 7ec8ceff2f..77b80f2bf8 100644
--- a/io/channel-tls.c
+++ b/io/channel-tls.c
@@ -24,6 +24,12 @@ 
 #include "io/channel-tls.h"
 #include "trace.h"
 
+struct QIOChannelTLSSource {
+    GSource parent;
+    GIOCondition condition;
+    QIOChannelTLS *tioc;
+};
+
 
 static ssize_t qio_channel_tls_write_handler(const char *buf,
                                              size_t len,
@@ -269,6 +275,10 @@  static ssize_t qio_channel_tls_readv(QIOChannel *ioc,
         ssize_t ret = qcrypto_tls_session_read(tioc->session,
                                                iov[i].iov_base,
                                                iov[i].iov_len);
+        trace_qio_channel_tls_readv_iov_len(tioc->session
+                                            , iov[i].iov_base
+                                            , iov[i].iov_len);
+
         if (ret < 0) {
             if (errno == EAGAIN) {
                 if (got) {
@@ -290,6 +300,15 @@  static ssize_t qio_channel_tls_readv(QIOChannel *ioc,
             break;
         }
     }
+
+    if (qcrypto_tls_session_read_check_buffer(tioc->session)
+        == QCRYPTO_TLS_RECORDS_PENDING) {
+        tioc->source->condition |= G_IO_IN;
+        trace_qio_channel_tls_readv_extra_records(tioc->session
+                                                , tioc->source->condition
+                                                , got);
+    }
+
     return got;
 }
 
@@ -385,12 +404,80 @@  static void qio_channel_tls_set_aio_fd_handler(QIOChannel *ioc,
     qio_channel_set_aio_fd_handler(tioc->master, ctx, io_read, io_write, opaque);
 }
 
+static gboolean
+qio_channel_tls_source_check(GSource *source);
+
+static gboolean
+qio_channel_tls_source_prepare(GSource *source,
+                                gint *timeout)
+{
+    *timeout = -1;
+    trace_qio_channel_tls_source_prepare(source, *timeout);
+    return qio_channel_tls_source_check(source);
+}
+
+static gboolean
+qio_channel_tls_source_check(GSource *source)
+{
+    QIOChannelTLSSource *tsource = (QIOChannelTLSSource *)source;
+    trace_qio_channel_tls_source_check(tsource->condition);
+    return (G_IO_IN | G_IO_OUT) & tsource->condition;
+}
+
+static gboolean
+qio_channel_tls_source_dispatch(GSource *source,
+                                   GSourceFunc callback,
+                                   gpointer user_data)
+{
+    QIOChannelFunc func = (QIOChannelFunc)callback;
+    QIOChannelTLSSource *tsource = (QIOChannelTLSSource *)source;
+    trace_qio_channel_tls_source_dispatch(tsource->tioc
+                            , tsource->condition
+                            , func);
+
+    return (*func)(QIO_CHANNEL(tsource->tioc),
+                   ((G_IO_IN | G_IO_OUT) & tsource->condition),
+                   user_data);
+}
+
+static void
+qio_channel_tls_source_finalize(GSource *source)
+{
+    QIOChannelTLSSource *tsource = (QIOChannelTLSSource *)source;
+    trace_qio_channel_tls_source_finalize(tsource, tsource->tioc);
+
+    object_unref(OBJECT(tsource->tioc));
+}
+
+GSourceFuncs qio_channel_tls_source_funcs = {
+    qio_channel_tls_source_prepare,
+    qio_channel_tls_source_check,
+    qio_channel_tls_source_dispatch,
+    qio_channel_tls_source_finalize
+};
+
+
 static GSource *qio_channel_tls_create_watch(QIOChannel *ioc,
                                              GIOCondition condition)
 {
     QIOChannelTLS *tioc = QIO_CHANNEL_TLS(ioc);
 
-    return qio_channel_create_watch(tioc->master, condition);
+    QIOChannelTLSSource *tsource;
+    GSource *source;
+
+    source = g_source_new(&qio_channel_tls_source_funcs,
+                          sizeof(QIOChannelTLSSource));
+    tsource = (QIOChannelTLSSource *)source;
+
+    tsource->tioc = tioc;
+    tioc->source = tsource;
+    object_ref(OBJECT(tioc));
+
+    tsource->condition = condition;
+    trace_qio_channel_tls_create_watch(tsource
+                                        , tioc
+                                        , condition);
+    return source;
 }
 
 QCryptoTLSSession *
diff --git a/io/trace-events b/io/trace-events
index d7bc70b966..2f82c15e68 100644
--- a/io/trace-events
+++ b/io/trace-events
@@ -42,6 +42,13 @@  qio_channel_tls_handshake_fail(void *ioc) "TLS handshake fail ioc=%p"
 qio_channel_tls_handshake_complete(void *ioc) "TLS handshake complete ioc=%p"
 qio_channel_tls_credentials_allow(void *ioc) "TLS credentials allow ioc=%p"
 qio_channel_tls_credentials_deny(void *ioc) "TLS credentials deny ioc=%p"
+qio_channel_tls_source_prepare(void *source, int timeout) "TLS source=%p prepare timeout=%d"
+qio_channel_tls_source_check(int condition) "TLS source condition=%d"
+qio_channel_tls_source_dispatch(void* channel_p, int condition, void* func) "TLS dispatch source=%p condition=%d callback func=%p"
+qio_channel_tls_source_finalize(void* source_p, void* channel_p) "TLS source finalize source=%p ioc=%p"
+qio_channel_tls_create_watch(void* tsource, void* tioc, int condition) "TLS create watch source=%p channel=%p condition=%d"
+qio_channel_tls_readv_extra_records(void* session_p, int condition, long got) "TLS readv extra recs session=%p condition=%d got=%ld"
+qio_channel_tls_readv_iov_len(void* session_p, void* iov_base, long iov_len) "TLS readv iov len session=%p iov_base=%p iov_len=%ld"
 
 # channel-websock.c
 qio_channel_websock_new_server(void *ioc, void *master) "Websock new client ioc=%p master=%p"