Re: The netsec thread

[Paul Eggert <eggert@cs.ucla.edu>]

[-- Attachment #1: Type: text/plain, Size: 821 bytes --]

Lars Ingebrigtsen wrote:
> I've tried the resulting code, and everything seems to work OK to me,
> but please let me know whether you see any new TLS-related problems
> while browsing or something.

I had a problem building it on Fedora 30 which uses GnuTLS 3.6.8, because 
starting in GnuTLS 3.6 the functions gnutls_compression_get and 
gnutls_compression_get_name are deprecated and cause compile-time errors when 
one builds with --enable-gcc-warnings. If you override the compile-time errors 
the compatibility stubs in libgnutls return null values, and some 
network-stream-tests fail.

I temporarily worked around the build problem by installing the attached patch 
which omits calls to these functions in GnuTLS 3.6 and later, but this doesn't 
fix the runtime issues. Is that something you could take a look at?

[-- Attachment #2: 0001-Get-the-Gnutls-code-compiling-on-Fedora-30.patch --]
[-- Type: text/x-patch, Size: 9119 bytes --]

From 49a8c8506a8477fd27ba924f14aa196e0d0813f9 Mon Sep 17 00:00:00 2001
From: Paul Eggert <eggert@cs.ucla.edu>
Date: Fri, 23 Aug 2019 01:11:12 -0700
Subject: [PATCH] Get the Gnutls code compiling on Fedora 30

The recent changes caused the build to fail on Fedora 30 when built
with --enable-gcc-warnings, among other things with diagnostics that
gnutls_compression_get and gnutls_compression_get_name are deprecated
(this started with GnuTLS 3.6).  Fix this by refusing to call these
obsolescent and now-dummy functions in GnuTLS 3.6 and later.  However,
this is just a temporary workaround to get the build working; a real
fix is needed, as network-stream-tests fail.
* src/gnutls.c (HAVE_GNUTLS_COMPRESSION_GET): New macro.
(gnutls_compression_get, gnutls_compression_get_name):
Define only if HAVE_GNUTLS_COMPRESSION_GET.
(init_gnutls_functions): Load the two functions only if
HAVE_GNUTLS_COMPRESSION_GET.
(emacs_gnutls_certificate_export_pem): Use alloca instead of xmalloc.
(Fgnutls_peer_status): Just return "NULL" if the functions
are deprecated.
(Fgnutls_format_certificate): Fix pointer signedness glitches.
* src/process.c: Fix spacing.
---
 src/gnutls.c  | 60 +++++++++++++++++++++++++++++++--------------------
 src/process.c | 26 ++++++++++------------
 2 files changed, 48 insertions(+), 38 deletions(-)

diff --git a/src/gnutls.c b/src/gnutls.c
index db452e01aa..51536b1463 100644
--- a/src/gnutls.c
+++ b/src/gnutls.c
@@ -48,6 +48,10 @@ along with GNU Emacs.  If not, see <https://www.gnu.org/licenses/>.  */
 # define HAVE_GNUTLS_ETM_STATUS
 #endif
 
+#if GNUTLS_VERSION_NUMBER < 0x030600
+# define HAVE_GNUTLS_COMPRESSION_GET
+#endif
+
 /* gnutls_mac_get_nonce_size was added in GnuTLS 3.2.0, but was
    exported only since 3.3.0. */
 #if GNUTLS_VERSION_NUMBER >= 0x030300
@@ -217,10 +221,12 @@ DEF_DLL_FN (const char *, gnutls_cipher_get_name,
 	    (gnutls_cipher_algorithm_t));
 DEF_DLL_FN (gnutls_mac_algorithm_t, gnutls_mac_get, (gnutls_session_t));
 DEF_DLL_FN (const char *, gnutls_mac_get_name, (gnutls_mac_algorithm_t));
+#ifdef HAVE_GNUTLS_COMPRESSION_GET
 DEF_DLL_FN (gnutls_compression_method_t, gnutls_compression_get,
             (gnutls_session_t));
 DEF_DLL_FN (const char *, gnutls_compression_get_name,
             (gnutls_compression_method_t));
+#endif
 DEF_DLL_FN (unsigned, gnutls_safe_renegotiation_status, (gnutls_session_t));
 
 #  ifdef HAVE_GNUTLS3
@@ -368,8 +374,10 @@ init_gnutls_functions (void)
   LOAD_DLL_FN (library, gnutls_cipher_get_name);
   LOAD_DLL_FN (library, gnutls_mac_get);
   LOAD_DLL_FN (library, gnutls_mac_get_name);
+#  ifdef HAVE_GNUTLS_COMPRESSION_GET
   LOAD_DLL_FN (library, gnutls_compression_get);
   LOAD_DLL_FN (library, gnutls_compression_get_name);
+#  endif
   LOAD_DLL_FN (library, gnutls_safe_renegotiation_status);
 #  ifdef HAVE_GNUTLS3
   LOAD_DLL_FN (library, gnutls_rnd);
@@ -462,8 +470,10 @@ init_gnutls_functions (void)
 #  define gnutls_kx_get_name fn_gnutls_kx_get_name
 #  define gnutls_mac_get fn_gnutls_mac_get
 #  define gnutls_mac_get_name fn_gnutls_mac_get_name
-#  define gnutls_compression_get fn_gnutls_compression_get
-#  define gnutls_compression_get_name fn_gnutls_compression_get_name
+#  ifdef HAVE_GNUTLS_COMPRESSION_GET
+#   define gnutls_compression_get fn_gnutls_compression_get
+#   define gnutls_compression_get_name fn_gnutls_compression_get_name
+#  endif
 #  define gnutls_safe_renegotiation_status fn_gnutls_safe_renegotiation_status
 #  define gnutls_pk_algorithm_get_name fn_gnutls_pk_algorithm_get_name
 #  define gnutls_pk_bits_to_sec_param fn_gnutls_pk_bits_to_sec_param
@@ -1082,17 +1092,18 @@ emacs_gnutls_certificate_export_pem (gnutls_x509_crt_t cert)
 
   if (err == GNUTLS_E_SHORT_MEMORY_BUFFER)
     {
-      unsigned char *buf = xmalloc(size * sizeof (unsigned char));
+      USE_SAFE_ALLOCA;
+      char *buf = SAFE_ALLOCA (size);
       err = gnutls_x509_crt_export (cert, GNUTLS_X509_FMT_PEM, buf, &size);
       check_memory_full (err);
 
       if (err < GNUTLS_E_SUCCESS)
-        {
-          xfree (buf);
-          error ("GnuTLS certificate export error: %s", emacs_gnutls_strerror (err));
-        }
+	error ("GnuTLS certificate export error: %s",
+	       emacs_gnutls_strerror (err));
 
-      return build_string(buf);
+      Lisp_Object result = build_string (buf);
+      SAFE_FREE ();
+      return result;
     }
   else if (err < GNUTLS_E_SUCCESS)
     error ("GnuTLS certificate export error: %s", emacs_gnutls_strerror (err));
@@ -1481,20 +1492,21 @@ returned as the :certificate entry.  */)
 				  (gnutls_mac_get (state)))));
 
   /* Compression name. */
-  result = nconc2
-    (result, list2 (intern (":compression"),
-		    build_string (gnutls_compression_get_name
-				  (gnutls_compression_get (state)))));
+#ifdef HAVE_GNUTLS_COMPRESSION_GET
+  Lisp_Object compression = build_string (gnutls_compression_get_name
+					  (gnutls_compression_get (state)));
+#else
+  Lisp_Object compression = build_string ("NULL");
+#endif
+  result = nconc2 (result, list2 (intern (":compression"), compression));
 
   /* Encrypt-then-MAC. */
-  result = nconc2
-    (result, list2 (intern (":encrypt-then-mac"),
+  Lisp_Object etm_status = Qnil;
 #ifdef HAVE_GNUTLS_ETM_STATUS
-                    gnutls_session_etm_status (state) ? Qt : Qnil
-#else
-                    Qnil
+  if (gnutls_session_etm_status (state))
+    etm_status = Qt;
 #endif
-                    ));
+  result = nconc2 (result, list2 (intern (":encrypt-then-mac"), etm_status));
 
   /* Renegotiation Indication */
   result = nconc2
@@ -1561,7 +1573,8 @@ boot_error (struct Lisp_Process *p, const char *m, ...)
   va_end (ap);
 }
 
-DEFUN ("gnutls-format-certificate", Fgnutls_format_certificate, Sgnutls_format_certificate, 1, 1, 0,
+DEFUN ("gnutls-format-certificate", Fgnutls_format_certificate,
+       Sgnutls_format_certificate, 1, 1, 0,
        doc: /* Format a X.509 certificate to a string.
 
 Given a PEM-encoded X.509 certificate CERT, returns a human-readable
@@ -1578,14 +1591,14 @@ string representation.  */)
   if (err < GNUTLS_E_SUCCESS)
     error ("gnutls-format-certificate error: %s", emacs_gnutls_strerror (err));
 
-  unsigned char *crt_buf = SDATA (cert);
-  gnutls_datum_t crt_data = { crt_buf, strlen (crt_buf) };
+  gnutls_datum_t crt_data = { SDATA (cert), strlen (SSDATA (cert)) };
   err = gnutls_x509_crt_import (crt, &crt_data, GNUTLS_X509_FMT_PEM);
   check_memory_full (err);
   if (err < GNUTLS_E_SUCCESS)
     {
       gnutls_x509_crt_deinit (crt);
-      error ("gnutls-format-certificate error: %s", emacs_gnutls_strerror (err));
+      error ("gnutls-format-certificate error: %s",
+	     emacs_gnutls_strerror (err));
     }
 
   gnutls_datum_t out;
@@ -1594,7 +1607,8 @@ string representation.  */)
   if (err < GNUTLS_E_SUCCESS)
     {
       gnutls_x509_crt_deinit (crt);
-      error ("gnutls-format-certificate error: %s", emacs_gnutls_strerror (err));
+      error ("gnutls-format-certificate error: %s",
+	     emacs_gnutls_strerror (err));
     }
 
   char *out_buf = xmalloc ((out.size + 1) * sizeof (char));
diff --git a/src/process.c b/src/process.c
index 7097b7ace1..c3cc78afa2 100644
--- a/src/process.c
+++ b/src/process.c
@@ -4120,10 +4120,8 @@ usage: (make-network-process &rest ARGS)  */)
       hints.ai_socktype = socktype;
 
       msg = network_lookup_address_info_1 (host, portstring, &hints, &res);
-      if (!EQ(msg, Qt))
-        {
-          error ("%s", SSDATA (msg));
-        }
+      if (!EQ (msg, Qt))
+	error ("%s", SSDATA (msg));
 
       for (lres = res; lres; lres = lres->ai_next)
 	addrinfos = Fcons (conv_addrinfo_to_lisp (lres), addrinfos);
@@ -4593,10 +4591,12 @@ network_lookup_address_info_1 (Lisp_Object host, const char *service,
         str = SSDATA (code_convert_string_norecord
                       (build_string (str), Vlocale_coding_system, 0));
       AUTO_STRING (format, "%s/%s %s");
-      msg = CALLN (Fformat, format, host, build_string (service), build_string (str));
+      msg = CALLN (Fformat, format, host, build_string (service),
+		   build_string (str));
 #else
       AUTO_STRING (format, "%s/%s getaddrinfo error %d");
-      msg = CALLN (Fformat, format, host, build_string (service), make_number (ret));
+      msg = CALLN (Fformat, format, host, build_string (service),
+		   make_number (ret));
 #endif
     }
    return msg;
@@ -4634,18 +4634,14 @@ nil if none were found.  Each address is a vector of integers.  */)
   hints.ai_socktype = SOCK_DGRAM;
 
   msg = network_lookup_address_info_1 (name, NULL, &hints, &res);
-  if (!EQ(msg, Qt))
-    {
-      message ("%s", SSDATA(msg));
-    }
+  if (!EQ (msg, Qt))
+    message ("%s", SSDATA(msg));
   else
     {
       for (lres = res; lres; lres = lres->ai_next)
-        {
-          addresses = Fcons (conv_sockaddr_to_lisp
-                             (lres->ai_addr, lres->ai_addrlen),
-                             addresses);
-        }
+	addresses = Fcons (conv_sockaddr_to_lisp (lres->ai_addr,
+						  lres->ai_addrlen),
+			   addresses);
       addresses = Fnreverse (addresses);
 
       freeaddrinfo (res);
-- 
2.17.1