daemon/tls: fixed improper use of callback, leaks

the TLS sessions now bypass the usuall event loop asynchronous iops
this is because the whole operation is synchronous right now, and
implementing asynchronous send operations would require TLS session to
restart write events on the event loop and making sure the "on complete"
callback is called eventually

diff --git a/daemon/io.c b/daemon/io.c
index 1a69dbeeb012143c85fc0c312522465217613fd5..5d1e9837d9ffd8930310ecad209af16545f182e7 100644 (file)
--- a/daemon/io.c
+++ b/daemon/io.c
@@ -208,7 +208,7 @@ static void tcp_recv(uv_stream_t *handle, ssize_t nread, const uv_buf_t *buf)
         * so the whole message reassembly and demuxing logic is inside worker */
        int ret = 0;
        if (s->has_tls) {
-               ret = worker_process_tls(worker, handle, (const uint8_t *)buf->base, nread);
+               ret = tls_process(worker, handle, (const uint8_t *)buf->base, nread);
        } else {
                ret = worker_process_tcp(worker, handle, (const uint8_t *)buf->base, nread);
        }

diff --git a/daemon/tls.c b/daemon/tls.c
index 56b449e8e0af8cf851aa8ec735e55ae8a1d624f5..b6fdac8368910d9ddad1b5b32cc07c3532e286ed 100644 (file)
--- a/daemon/tls.c
+++ b/daemon/tls.c
@@ -41,10 +41,6 @@ struct tls_ctx_t {
        ssize_t nread;
        ssize_t consumed;
        uint8_t recv_buf[4096];
-
-       /* for writing to the network */
-       uv_write_t *writer;
-       uv_write_cb     write_cb;
 };
 
 /** @internal Debugging facility. */
@@ -109,38 +105,6 @@ kres_gnutls_pull(gnutls_transport_ptr_t h, void *buf, size_t len)
        return transfer;
 }
 
-static         ssize_t
-kres_gnutls_push_vec(gnutls_transport_ptr_t h, const giovec_t * iov, int iovcnt)
-{
-       struct tls_ctx_t *t = (struct tls_ctx_t *)h;
-       int     ret;
-
-       DEBUG_MSG("vecpush %d (%p) handle: <%p> writer <%p>\n", iovcnt, iov, t->handle, t->writer);
-
-       /*
-        * because of the struct of giovec_t is identical to struct iovec;
-        * and uv_buf_t header (uv-unix.h) says it may be cast to struct
-        * iovec; so we should be able to just cast directly.
-        */
-       ret = uv_write(t->writer, t->handle, (uv_buf_t *) iov, iovcnt, t->write_cb);
-       if (ret >= 0) {
-               /* Pending ioreq on current task */
-               return (ssize_t) ret;
-       }
-       switch (ret) {
-       case UV_EAGAIN:
-               errno = EAGAIN;
-               break;
-       case UV_EINTR:
-               errno = EINTR;
-               break;
-       default:
-               kr_log_error("[tls] uv_write unknown error: %d\n", ret);
-               errno = EIO;    /* dkg just picked this at random */
-       }
-       return -1;
-}
-
 struct tls_ctx_t *
 tls_new(struct worker_ctx *worker)
 {
@@ -236,65 +200,50 @@ tls_free(struct tls_ctx_t *tls)
        free(tls);
 }
 
-int
-push_tls(struct qr_task *task, uv_handle_t * handle, knot_pkt_t * pkt,
-        uv_write_t * writer, qr_task_send_cb on_send)
+int tls_push(struct qr_task *task, uv_handle_t* handle, knot_pkt_t * pkt)
 {
-       ssize_t count;
-       if (!pkt) {
-               kr_log_error("[tls] cannot push null packet\n");
-               return on_send(task, handle, kr_error(EIO));
+       if (!pkt || !handle || !handle->data) {
+               return kr_error(EINVAL);
        }
-       uint16_t pkt_size = htons(pkt->size);
 
+       ssize_t count;
        struct session *session = handle->data;
-       if (!session) {
-               kr_log_error("[tls] no session on push\n");
-               return on_send(task, handle, kr_error(EIO));
-       }
+       uint16_t pkt_size = htons(pkt->size);
        struct tls_ctx_t *tls_p = session->tls_ctx;
        if (!tls_p) {
                kr_log_error("[tls] no tls context on push\n");
-               /* FIXME: might be necessary if we ever do outbound TLS */
-               return on_send(task, handle, kr_error(EIO));
+               return kr_error(ENOENT);
        }
-       tls_p->handle = (uv_stream_t *) handle;
-       tls_p->writer = writer;
        gnutls_record_cork(tls_p->session);
        count = gnutls_record_send(tls_p->session, &pkt_size, sizeof(pkt_size));
        if (count != sizeof(pkt_size)) {
                kr_log_error("[tls] gnutls_record_send pkt_size fail wanted: %u (%zd) %s\n",
                             pkt_size, count, gnutls_strerror_name(count));
-               return on_send(task, handle, kr_error(EIO));
+               return kr_error(EIO);
        }
        count = gnutls_record_send(tls_p->session, pkt->wire, pkt->size);
        if (count != pkt->size) {
                kr_log_error("[tls] gnutls_record_send wire fail wanted: %zu (%zd) %s\n",
                             pkt->size, count, gnutls_strerror_name(count));
-               return on_send(task, handle, kr_error(EIO));
+               return kr_error(EIO);
        }
        count = gnutls_record_uncork(tls_p->session, 0);
        if (count != sizeof(pkt_size) + pkt->size) {
                if (count == GNUTLS_E_AGAIN || count == GNUTLS_E_INTERRUPTED) {
                        kr_log_error("[tls] gnutls_record_send incomplete: %zu (%zd) %s\n",
                             pkt->size, count, gnutls_strerror_name(count));
-                       /*
-                        * FIXME: we need to know when this frees up; when it
-                        * does, we should do gnutls_record_send(tls.session,
-                        * NULL, 0);   how do i know?
-                        */
+                       return kr_error(EAGAIN);
                } else {
                        kr_log_error("[tls] gnutls_record_send wire fail wanted: %zu (%zd) %s\n",
                             pkt->size, count, gnutls_strerror_name(count));
                }
-               return on_send(task, handle, kr_error(EIO));
+               return kr_error(EIO);
        }
-       return count;
+       return 0;
 }
 
 
-int 
-worker_process_tls(struct worker_ctx *worker, uv_stream_t * handle, const uint8_t * buf, ssize_t nread)
+int tls_process(struct worker_ctx *worker, uv_stream_t * handle, const uint8_t * buf, ssize_t nread)
 {
        struct session *session = handle->data;
        struct tls_ctx_t *tls_p = session->tls_ctx;

diff --git a/daemon/tls.h b/daemon/tls.h
index 14646272b25f356c4ec47b7e53380caf605eceed..fdce96bd761d4142daffcec8722c67da4aa59487 100644 (file)
--- a/daemon/tls.h
+++ b/daemon/tls.h
@@ -24,7 +24,5 @@ struct tls_ctx_t;
 struct tls_ctx_t* tls_new(struct worker_ctx *worker);
 void tls_free(struct tls_ctx_t* tls);
 
-int push_tls(struct qr_task *task, uv_handle_t *handle, knot_pkt_t *pkt,
-            uv_write_t *writer, qr_task_send_cb on_send);
-
-int worker_process_tls(struct worker_ctx *worker, uv_stream_t *handle, const uint8_t *buf, ssize_t nread);
+int tls_push(struct qr_task *task, uv_handle_t* handle, knot_pkt_t * pkt);
+int tls_process(struct worker_ctx *worker, uv_stream_t *handle, const uint8_t *buf, ssize_t nread);

diff --git a/daemon/worker.c b/daemon/worker.c
index 3a5380f11c2b699c2800961d761161d0ba0beca7..1ab3d5d76ebdcda62eaa4b3ca0b8bdd9279b7da1 100644 (file)
--- a/daemon/worker.c
+++ b/daemon/worker.c
@@ -444,30 +444,32 @@ static int qr_task_send(struct qr_task *task, uv_handle_t *handle, struct sockad
        if (!handle) {
                return qr_task_on_send(task, handle, kr_error(EIO));
        }
-       struct req *send_req = req_borrow(task->worker);
-       if (!send_req) {
-               return qr_task_on_send(task, handle, kr_error(ENOMEM));
+
+       /* Synchronous push to TLS context, bypassing event loop. */
+       struct session *session = handle->data;
+       if (session->has_tls) {
+               int ret = tls_push(task, handle, pkt);
+               return qr_task_on_send(task, handle, ret);
        }
 
        /* Send using given protocol */
        int ret = 0;
+       struct req *send_req = req_borrow(task->worker);
+       if (!send_req) {
+               return qr_task_on_send(task, handle, kr_error(ENOMEM));
+       }
        if (handle->type == UV_UDP) {
                uv_buf_t buf = { (char *)pkt->wire, pkt->size };
                send_req->as.send.data = task;
                ret = uv_udp_send(&send_req->as.send, (uv_udp_t *)handle, &buf, 1, addr, &on_send);
        } else {
-               struct session *session = handle->data;
-               if (session->has_tls) {
-                       ret = push_tls(task, handle, pkt, &send_req->as.write, qr_task_on_send);
-               } else {
-                       uint16_t pkt_size = htons(pkt->size);
-                       uv_buf_t buf[2] = {
-                               { (char *)&pkt_size, sizeof(pkt_size) },
-                               { (char *)pkt->wire, pkt->size }
-                       };
-                       send_req->as.write.data = task;
-                       ret = uv_write(&send_req->as.write, (uv_stream_t *)handle, buf, 2, &on_write);
-               }
+               uint16_t pkt_size = htons(pkt->size);
+               uv_buf_t buf[2] = {
+                       { (char *)&pkt_size, sizeof(pkt_size) },
+                       { (char *)pkt->wire, pkt->size }
+               };
+               send_req->as.write.data = task;
+               ret = uv_write(&send_req->as.write, (uv_stream_t *)handle, buf, 2, &on_write);
        }
        if (ret == 0) {
                qr_task_ref(task); /* Pending ioreq on current task */