[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Bug#1049985: buster-pu: package riemann-c-client/1.10.4-2



Package: release.debian.org
Severity: normal
Tags: buster
User: release.debian.org@packages.debian.org
Usertags: pu
X-Debbugs-Cc: romain@blogreen.org

[ Reason ]
Due to improper return value checks, when communicating with a remote
server over TLS riemann-c-client sometimes send the same data fragment
multiple times, resulting in the server receiving a malformed payload.

This happen with all versions of TLS, but TLS 1.3 trigger this bad
behaviour more often.  With more and more services using TLS 1.3, this
problem is more and more prevalent.

[ Impact ]
When the client send a large payload over TLS faster than the network
can send it, the improper return value checks cause portions of that
data to be send multiple times to the server.  When the transfer
eventually finish, the server detect that the payload is invalid and
drop the connection.  The client will then reconnect and retry the
transfer that might fail again and again.

Beside error messages in the server logs, these data corrupt data
transfer cause an unexpectedly hight bandwidth usage.

[ Tests ]
Manually tested on our infrastructure (mostly Debian oldstable).

[ Risks ]
Low

[ Checklist ]
  [x] *all* changes are documented in the d/changelog
  [x] I reviewed all changes and I approve them
  [x] attach debdiff against the package in (old)stable
  [x] the issue is verified as fixed in unstable

[ Changes ]
Adjust the checks of the return values of gnutls_record_send() /
gnutls_record_recv() and keep track of the quantity of data already
sent / received when the function return before completion so that it
can continue from the point it stopped the previous time.
diff -Nru riemann-c-client-1.10.4/debian/changelog riemann-c-client-1.10.4/debian/changelog
--- riemann-c-client-1.10.4/debian/changelog	2019-01-03 07:09:25.000000000 -1000
+++ riemann-c-client-1.10.4/debian/changelog	2023-08-13 12:08:48.000000000 -1000
@@ -1,3 +1,13 @@
+riemann-c-client (1.10.4-3) unstable; urgency=medium
+
+  * QA upload.
+  * Drop Vcs-* fields and gbp.conf.
+
+  [ Romain Tartière ]
+  * Fix GnuTLS send/recv.
+
+ -- Bastian Germann <bage@debian.org>  Mon, 14 Aug 2023 00:08:48 +0200
+
 riemann-c-client (1.10.4-2) unstable; urgency=medium
 
   * Orphaning the package.
diff -Nru riemann-c-client-1.10.4/debian/control riemann-c-client-1.10.4/debian/control
--- riemann-c-client-1.10.4/debian/control	2019-01-03 07:09:15.000000000 -1000
+++ riemann-c-client-1.10.4/debian/control	2023-08-13 12:08:48.000000000 -1000
@@ -10,8 +10,6 @@
 Standards-Version: 4.2.1
 Section: libs
 Homepage: https://git.madhouse-project.org/algernon/riemann-c-client
-Vcs-Git:  https://git.madhouse-project.org/algernon/riemann-c-client.git -b debian/master
-Vcs-Browser: https://git.madhouse-project.org/algernon/riemann-c-client/src/branch/debian/master
 
 Package: libriemann-client0
 Architecture: any
diff -Nru riemann-c-client-1.10.4/debian/gbp.conf riemann-c-client-1.10.4/debian/gbp.conf
--- riemann-c-client-1.10.4/debian/gbp.conf	2019-01-03 07:08:30.000000000 -1000
+++ riemann-c-client-1.10.4/debian/gbp.conf	1969-12-31 14:00:00.000000000 -1000
@@ -1,7 +0,0 @@
-[DEFAULT]
-debian-branch = debian/master
-debian-tag = debian/riemann-c-client-%(version)s
-upstream-tag = riemann-c-client-%(version)s
-compression = xz
-
-postbuild = lintian -I $GBP_CHANGES_FILE && echo "Lintian OK"
diff -Nru riemann-c-client-1.10.4/debian/patches/fix-gnutls-send-recv-when-return-eagain riemann-c-client-1.10.4/debian/patches/fix-gnutls-send-recv-when-return-eagain
--- riemann-c-client-1.10.4/debian/patches/fix-gnutls-send-recv-when-return-eagain	1969-12-31 14:00:00.000000000 -1000
+++ riemann-c-client-1.10.4/debian/patches/fix-gnutls-send-recv-when-return-eagain	2023-08-13 12:08:48.000000000 -1000
@@ -0,0 +1,46 @@
+Origin: upstream, 9e382db87bd1703423760bbe104a66e7cdfcf5a6
+Description: Fix GnuTLS send/recv when returning GNUTLS_E_AGAIN
+ Some values returned from gnutls_record_send() / gnutls_record_recv() indicate
+ that the operation could not be done. In such cases, the error should not
+ propagate to the caller but be operation should be retried.
+ .
+ Upstream fixed this issue in 9e382db87bd1703423760bbe104a66e7cdfcf5a6 with a
+ lot more changes, so this patch only fix the wrong behavior.
+Author: Romain Tartière <romain@blogreen.org>
+Forwarded: not-needed
+---
+--- riemann-c-client-1.10.4.orig/lib/riemann/client/tls-gnutls.c
++++ riemann-c-client-1.10.4/lib/riemann/client/tls-gnutls.c
+@@ -202,7 +202,9 @@ _riemann_client_send_message_tls (rieman
+   if (!buffer)
+     return -errno;
+ 
+-  sent = gnutls_record_send (client->tls.session, buffer, len);
++  do {
++    sent = gnutls_record_send (client->tls.session, buffer, len);
++  } while (sent == GNUTLS_E_AGAIN || sent == GNUTLS_E_INTERRUPTED);
+   if (sent < 0 || (size_t)sent != len)
+     {
+       free (buffer);
+@@ -220,7 +222,9 @@ _riemann_client_recv_message_tls (rieman
+   ssize_t received;
+   riemann_message_t *message;
+ 
+-  received = gnutls_record_recv (client->tls.session, &header, sizeof (header));
++  do {
++    received = gnutls_record_recv (client->tls.session, &header, sizeof (header));
++  } while (received == GNUTLS_E_AGAIN || received == GNUTLS_E_INTERRUPTED);
+   if (received != sizeof (header))
+     {
+       errno = EPROTO;
+@@ -230,7 +234,9 @@ _riemann_client_recv_message_tls (rieman
+ 
+   buffer = (uint8_t *) malloc (len);
+ 
+-  received = gnutls_record_recv (client->tls.session, buffer, len);
++  do {
++    received = gnutls_record_recv (client->tls.session, buffer, len);
++  } while (received == GNUTLS_E_AGAIN || received == GNUTLS_E_INTERRUPTED);
+   if (received != len)
+     {
+       free (buffer);
diff -Nru riemann-c-client-1.10.4/debian/patches/fix-gnutls-send-recv-when-return-less-than-expected riemann-c-client-1.10.4/debian/patches/fix-gnutls-send-recv-when-return-less-than-expected
--- riemann-c-client-1.10.4/debian/patches/fix-gnutls-send-recv-when-return-less-than-expected	1969-12-31 14:00:00.000000000 -1000
+++ riemann-c-client-1.10.4/debian/patches/fix-gnutls-send-recv-when-return-less-than-expected	2023-08-13 12:08:48.000000000 -1000
@@ -0,0 +1,91 @@
+Description: Fix GnuTLS send/recv when returning a lower value than expected
+ gnutls_record_send() / gnutls_record_recv() may be interrupted after some data
+ transmission but before the message was completely read/written.  When this
+ happen, the value returned by the function is positive but lower that the size
+ of the read/write.  In this case, we should not return an error, but rather
+ loop to recv/send the missing data.
+Author: Romain Tartière <romain@blogreen.org>
+Forwarded: https://git.madhouse-project.org/algernon/riemann-c-client/pulls/14
+---
+--- riemann-c-client-1.10.4.orig/lib/riemann/client/tls-gnutls.c
++++ riemann-c-client-1.10.4/lib/riemann/client/tls-gnutls.c
+@@ -202,13 +202,18 @@ _riemann_client_send_message_tls (rieman
+   if (!buffer)
+     return -errno;
+ 
+-  do {
+-    sent = gnutls_record_send (client->tls.session, buffer, len);
+-  } while (sent == GNUTLS_E_AGAIN || sent == GNUTLS_E_INTERRUPTED);
+-  if (sent < 0 || (size_t)sent != len)
++  size_t left = len;
++  while (left > 0)
+     {
+-      free (buffer);
+-      return -EPROTO;
++      do {
++        sent = gnutls_record_send (client->tls.session, buffer + len - left, left);
++      } while (sent == GNUTLS_E_AGAIN || sent == GNUTLS_E_INTERRUPTED);
++      if (sent < 0)
++        {
++          free (buffer);
++          return -EPROTO;
++        }
++      left -= sent;
+     }
+   free (buffer);
+   return 0;
+@@ -220,28 +225,41 @@ _riemann_client_recv_message_tls (rieman
+   uint32_t header, len;
+   uint8_t *buffer;
+   ssize_t received;
++  size_t left;
+   riemann_message_t *message;
+ 
+-  do {
+-    received = gnutls_record_recv (client->tls.session, &header, sizeof (header));
+-  } while (received == GNUTLS_E_AGAIN || received == GNUTLS_E_INTERRUPTED);
+-  if (received != sizeof (header))
++  len = sizeof (header);
++  left = len;
++  while (left > 0)
+     {
+-      errno = EPROTO;
+-      return NULL;
++      do {
++        received = gnutls_record_recv (client->tls.session, &header + len - left, left);
++      } while (received == GNUTLS_E_AGAIN || received == GNUTLS_E_INTERRUPTED);
++      if (received <= 0)
++        {
++          errno = EPROTO;
++          return NULL;
++        }
++      left -= received;
+     }
++
+   len = ntohl (header);
+ 
+   buffer = (uint8_t *) malloc (len);
+ 
+-  do {
+-    received = gnutls_record_recv (client->tls.session, buffer, len);
+-  } while (received == GNUTLS_E_AGAIN || received == GNUTLS_E_INTERRUPTED);
+-  if (received != len)
++  left = len;
++  while (left > 0)
+     {
+-      free (buffer);
+-      errno = EPROTO;
+-      return NULL;
++      do {
++	received = gnutls_record_recv (client->tls.session, buffer + len - left, left);
++      } while (received == GNUTLS_E_AGAIN || received == GNUTLS_E_INTERRUPTED);
++      if (received <= 0)
++        {
++          free (buffer);
++          errno = EPROTO;
++          return NULL;
++        }
++        left -= received;
+     }
+ 
+   message = riemann_message_from_buffer (buffer, len);
diff -Nru riemann-c-client-1.10.4/debian/patches/series riemann-c-client-1.10.4/debian/patches/series
--- riemann-c-client-1.10.4/debian/patches/series	1969-12-31 14:00:00.000000000 -1000
+++ riemann-c-client-1.10.4/debian/patches/series	2023-07-24 18:05:47.000000000 -1000
@@ -0,0 +1,2 @@
+fix-gnutls-send-recv-when-return-eagain
+fix-gnutls-send-recv-when-return-less-than-expected

Reply to: