bug#15648: 24.2.50; gnutls SSL connection to IMAP server causes emacs to crash completely

[Ted Zlatanov]

On Fri, 25 Oct 2013 17:09:13 +0300 Eli Zaretskii <eliz@gnu.org> wrote: 

EZ> However, to find the best place where to fix this in Emacs, could you
EZ> please help me understand in more detail what happens in this case?  I
EZ> imagine that gnutls-boot is called with the parameters that specify a
EZ> certificate file that GnuTLS cannot access.  But why isn't this caught
EZ> inside gnutls-boot, and how come we allow a NULL gnutls_state be
EZ> plugged into the process object?  This fragment from gnutls-boot:

EZ>   GNUTLS_LOG (1, max_log_level, "gnutls_init");
EZ>   ret = fn_gnutls_init (&state, GNUTLS_CLIENT);
EZ>   XPROCESS (proc)->gnutls_state = state;
EZ>   if (ret < GNUTLS_E_SUCCESS)
EZ>     return gnutls_make_error (ret);

EZ> ought to fail.  The bug report cites this error message:

EZ>   GnuTLS error: #<proces IMAP over SSL>, -64

EZ> Is that error the result of the above error checking?

(btw, no idea why the subject keeps getting duplicated but it's annoying)

Hard to tell if -64 means anything, but it shouldn't cause a crash.

EZ> If so, perhaps the problem is that we leave the process object
EZ> marked as a GnuTLS process, but with a NULL state?  Should we remove
EZ> the mark, or maybe delete the process object in gnutls-negotiate?

I would abort with a message like any other error.  Here's what we do:

          if (STRINGP (trustfile))
            {
              GNUTLS_LOG2 (1, max_log_level, "setting the trustfile: ",
                           SSDATA (trustfile));
              ret = fn_gnutls_certificate_set_x509_trust_file
                (x509_cred,
                 SSDATA (trustfile),
                 file_format);

              if (ret < GNUTLS_E_SUCCESS)
                return gnutls_make_error (ret);
            }
          else
            {
              emacs_gnutls_deinit (proc);
              error ("Invalid trustfile");
            }

In other words, we pass the file down and assume the return code from
`fn_gnutls_certificate_set_x509_trust_file' will be accurate.  In this
case I don't know what it returned but would assume GNUTLS_E_SUCCESS
since there was no error.

In general I trust the return codes and don't verify the state
explicitly.  I don't see how the `gnutls_state' could have been set to
NULL by a missing trustfile; the function call that sets the trustfile
only touches the `x509_cred' variable.  Could this NULL be happening
later?

In this specific case I traced the function calls all the way down the
GnuTLS library and see that it checks that file was opened correctly and
more.  It should return an error for that missing file.

http://git.savannah.gnu.org/gitweb/?p=gnutls.git;a=blob_plain;f=lib/gnutls_x509.c;hb=HEAD
gnutls.git/lib/gnutls_x509.c:gnutls_certificate_set_x509_trust_file()

...
  cas.data = (void*)read_binary_file (cafile, &size);
  if (cas.data == NULL)
    {
      gnutls_assert ();
      return GNUTLS_E_FILE_ERROR;
    }

and read_binary_file uses this code:

http://git.savannah.gnu.org/gitweb/?p=gnutls.git;a=blob_plain;f=gl/read-file.c;hb=HEAD

gnutls.git/gl/read-file.c:fread_file():

...
    if (fstat (fileno (stream), &st) >= 0 && S_ISREG (st.st_mode))
 
I can't test this specific issue so it's hard to know where the error is
happening.  Can anyone duplicate that problem?

I could add extra debugging and checking but I don't understand, if the
file name is invalid, how the fread_file() function is succeeding.
Maybe that's part of the problem.

Ted