Testing the gnutls support

Testing the gnutls support

[Lars Magne Ingebrigtsen]

I've done some testing of the gnutls support, and it pretty consistently
seems to fail if I request a lot of data.

For instance, if I do a header retrieval in a Gmail IMAP group, I end up
with this:

* 19 FETCH (UID 45 RFC822.SIZE 215416 BODYSTRUCTURE ((("TEXT" [...] ("CREATION-DATE" "M
Process *nnimap*<1> connection broken by remote peer

That's after receiving 15496 characters.

So something isn't quite working as it should do...

This works, though:

(let ((process
       (open-gnutls-stream "http" (current-buffer)
			   "www.google.com" "https")))
  (process-send-string process "GET / HTTP/1.0\r\nServer: www.google.com\r\n\r\n"))

It seems related to the length of the text we receive.  Are there any
buffers in the gnutls library that needs flushing or anything?  

-- 
(domestic pets only, the antidote for overdose, milk.)
  larsi@gnus.org * Lars Magne Ingebrigtsen

Re: Testing the gnutls support

[Ted Zlatanov]

On Thu, 07 Oct 2010 23:42:40 +0200 Lars Magne Ingebrigtsen <larsi@gnus.org> wrote: 

LMI> I've done some testing of the gnutls support, and it pretty consistently
LMI> seems to fail if I request a lot of data.

LMI> For instance, if I do a header retrieval in a Gmail IMAP group, I end up
LMI> with this:

LMI> * 19 FETCH (UID 45 RFC822.SIZE 215416 BODYSTRUCTURE ((("TEXT" [...] ("CREATION-DATE" "M
LMI> Process *nnimap*<1> connection broken by remote peer

LMI> That's after receiving 15496 characters.

LMI> So something isn't quite working as it should do...

LMI> This works, though:

LMI> (let ((process
LMI>        (open-gnutls-stream "http" (current-buffer)
LMI> 			   "www.google.com" "https")))
LMI>   (process-send-string process "GET / HTTP/1.0\r\nServer: www.google.com\r\n\r\n"))

LMI> It seems related to the length of the text we receive.  Are there any
LMI> buffers in the gnutls library that needs flushing or anything?  

Not that I'm aware of.  All the examples just read the data and there
are no "flush" functions in the API.  You can see how they prep a socket
in the examples, nothing unusual is going on:

http://www.gnu.org/software/gnutls/manual/html_node/Helper-function-for-TCP-connections.html

...and then the clients just use the gnutls_{read,write}() functions.

Can you check what gnutls_record_get_max_size() returns for you?  That's
negotiated.  Also is there an error in that session (if you turn up
logging)?

I don't think it's related to gnutls_record_set_max_size() because the
max size is 4K and you're getting errors around 16K.  But being so close
to 16K is very suspicious and indicates a buffer issue.  Does the error
happen on send, receive, or both?  Is there a 16K threshold or does that
number vary a lot?

emacs_gnutls_write() does "fsync (STDOUT_FILENO)" after each write.
This is a leftover from Simon's original patch:

  printf("wrote %d bytes\n", bytes_written);
  fsync(STDOUT_FILENO);

and I forgot to take it out.  Can you remove it?  I can't push that
commit currently, sorry.  If that turns out to be the trigger...

If not we can try playing with
gnutls_transport_set_{push,pull}_function() I guess.  But I can't see
why the default {send,recv}() call is inadequate.

Ted

Re: Testing the gnutls support

[Lars Magne Ingebrigtsen]

Ted Zlatanov <tzz@lifelogs.com> writes:

> emacs_gnutls_write() does "fsync (STDOUT_FILENO)" after each write.
> This is a leftover from Simon's original patch:
>
>   printf("wrote %d bytes\n", bytes_written);
>   fsync(STDOUT_FILENO);
>
> and I forgot to take it out.  Can you remove it?  I can't push that
> commit currently, sorry.  If that turns out to be the trigger...

Ok, fixed.

I think the main problem was that emacs_gnutls_read() returned 0 instead
something negative on errors.  Fixing that allows me to read lots of
data via gnutls.

However, if I set the debugging level to 9, I get the following:

Debugger entered--Lisp error: (file-error "writing to process" "interrupted system call" #<process http<1>>)
  process-send-string(#<process http<1>> "GET /market.php HTTP/1.0.\n.\n")
  (let ((process (open-gnutls-stream "http" (current-buffer) "chepa.netfonds.no" "https"))) (process-send-string process "GET /market.php HTTP/1.0.\n.\n"))
  eval((let ((process (open-gnutls-stream "http" (current-buffer) "chepa.netfonds.no" "https"))) (process-send-string process "GET /market.php HTTP/1.0.\n.\n")))
  eval-last-sexp-1(nil)
  eval-last-sexp(nil)
  call-interactively(eval-last-sexp nil nil)

Hm.  Hm?  
  
-- 
(domestic pets only, the antidote for overdose, milk.)
  larsi@gnus.org * Lars Magne Ingebrigtsen

Re: Testing the gnutls support

[Eli Zaretskii]

> From: Lars Magne Ingebrigtsen <larsi@gnus.org>
> Date: Sat, 09 Oct 2010 15:18:16 +0200
> Cc: gnutls-devel@gnu.org
> 
> However, if I set the debugging level to 9, I get the following:
> 
> Debugger entered--Lisp error: (file-error "writing to process" "interrupted system call" #<process http<1>>)
>   process-send-string(#<process http<1>> "GET /market.php HTTP/1.0.\n.\n")

That's strange.  process-send-string calls emacs_write to write to the
process, but emacs_write already handles EINTR, by retrying the write
which failed.  So why does this signal raise a Lisp error?

Re: Testing the gnutls support

[Lars Magne Ingebrigtsen]

Eli Zaretskii <eliz@gnu.org> writes:

>> Debugger entered--Lisp error: (file-error "writing to process" "interrupted system call" #<process http<1>>)
>>   process-send-string(#<process http<1>> "GET /market.php HTTP/1.0.\n.\n")
>
> That's strange.  process-send-string calls emacs_write to write to the
> process, but emacs_write already handles EINTR, by retrying the write
> which failed.  So why does this signal raise a Lisp error?

This is in emacs_gnutls_write, not emacs_write.  But thanks for the
tip -- it was indeed faulty EINTR handling.  I've now peeked at the
gnutls documentation and changed it to use the proper EINTR handling it
needs, and it seems to work in my test cases now.

-- 
(domestic pets only, the antidote for overdose, milk.)
  larsi@gnus.org * Lars Magne Ingebrigtsen

Re: Testing the gnutls support

[Ted Zlatanov]

On Sat, 09 Oct 2010 15:56:04 +0200 Lars Magne Ingebrigtsen <larsi@gnus.org> wrote: 

LMI> Eli Zaretskii <eliz@gnu.org> writes:
>>> Debugger entered--Lisp error: (file-error "writing to process" "interrupted system call" #<process http<1>>)
>>> process-send-string(#<process http<1>> "GET /market.php HTTP/1.0.\n.\n")
>> 
>> That's strange.  process-send-string calls emacs_write to write to the
>> process, but emacs_write already handles EINTR, by retrying the write
>> which failed.  So why does this signal raise a Lisp error?

LMI> This is in emacs_gnutls_write, not emacs_write.  But thanks for the
LMI> tip -- it was indeed faulty EINTR handling.  I've now peeked at the
LMI> gnutls documentation and changed it to use the proper EINTR handling it
LMI> needs, and it seems to work in my test cases now.

Wonderful.  Thanks for fixing the gnutls-boot plist symbols, too.

I had a long conversation (er, mostly monologue) with the GnuTLS guys
about the right way to set up callbacks and verification.  So:

1) we should be verifying the host name matches the certificate--but
should this be done in gnutls.el or gnutls.c?

2) we should set up a verification callback (but this is not available
in mainstream Debian/Ubuntu yet, since it's new in 2.10).  In the
callback we should let the user accept an invalid certificate.  I'd like
to defer this until 2.10 is in the Debian mainstream.

3) We should give users a way to accept certificates.  Right now they
can pass a list of trust file names themselves but I think this should
be a more general facility.

Ted

Re: Testing the gnutls support

[Lars Magne Ingebrigtsen]

Ted Zlatanov <tzz@lifelogs.com> writes:

> 1) we should be verifying the host name matches the certificate--but
> should this be done in gnutls.el or gnutls.c?

The negotiation is done in emacs_gnutls_read now, sort of, isn't it?
I'm not quite sure how to pass that info back to Elisp land -- it'd have
to be a callback of some sort, wouldn't it?

> 2) we should set up a verification callback (but this is not available
> in mainstream Debian/Ubuntu yet, since it's new in 2.10).  In the
> callback we should let the user accept an invalid certificate.  I'd like
> to defer this until 2.10 is in the Debian mainstream.

Sounds OK to me.

> 3) We should give users a way to accept certificates.  Right now they
> can pass a list of trust file names themselves but I think this should
> be a more general facility.

Yes.

-- 
(domestic pets only, the antidote for overdose, milk.)
  larsi@gnus.org * Lars Magne Ingebrigtsen

Re: Testing the gnutls support

[Ted Zlatanov]

On Sun, 10 Oct 2010 17:26:49 +0200 Lars Magne Ingebrigtsen <larsi@gnus.org> wrote: 

LMI> Ted Zlatanov <tzz@lifelogs.com> writes:
>> 1) we should be verifying the host name matches the certificate--but
>> should this be done in gnutls.el or gnutls.c?

LMI> The negotiation is done in emacs_gnutls_read now, sort of, isn't it?
LMI> I'm not quite sure how to pass that info back to Elisp land -- it'd have
LMI> to be a callback of some sort, wouldn't it?

Yes.  From GnuTLS doc/examples/ex-rfc2818.c:

(in main)

  gnutls_certificate_set_verify_function (xcred, verify_certificate_callback);
  gnutls_certificate_set_verify_flags(xcred, GNUTLS_VERIFY_ALLOW_X509_V1_CA_CRT);

(and then)

/* This function will try to verify the peer's certificate, and
 * also check if the hostname matches, and the activation, expiration dates.
 */
static int verify_certificate_callback (gnutls_session_t session)
{
...
  hostname = gnutls_session_get_ptr(session);
  ret = gnutls_certificate_verify_peers2 (session, &status);
...
  if (!gnutls_x509_crt_check_hostname (cert, hostname))
    {
      printf ("The certificate's owner does not match hostname '%s'\n",
              hostname);
      return GNUTLS_E_CERTIFICATE_ERROR;
    }
...
}

There's a lot more in that function but the basic idea is to make sure
the presented certificate matches the host name.

>> 2) we should set up a verification callback (but this is not available
>> in mainstream Debian/Ubuntu yet, since it's new in 2.10).  In the
>> callback we should let the user accept an invalid certificate.  I'd like
>> to defer this until 2.10 is in the Debian mainstream.

LMI> Sounds OK to me.

I can make an autoconf check to look for
gnutls_certificate_set_verify_function() and do the above.  I don't have
the tuits and 2.10 is not widely installed, though, so I would rather
not work on this right now.

>> 3) We should give users a way to accept certificates.  Right now they
>> can pass a list of trust file names themselves but I think this should
>> be a more general facility.

LMI> Yes.

My only concern is that someone can deposit a certificate in your
directory and fool you.  I think this should be a file of certificates
instead, which users can encrypt with EPA, or maybe store with the
Secrets API.  But that leads me back to auth-source.el since it already
has this kind of logic in it.  WDYT about depending on auth-source.el
for GnuTLS interactions?  I'm divided because OTOH gnutls.el is a
low-level wrapper and a dependency on auth-source.el will bring in other
baggage.

Ted

Re: Testing the gnutls support

[Lars Magne Ingebrigtsen]

Ted Zlatanov <tzz@lifelogs.com> writes:

> I think this should be a file of certificates instead, which users can
> encrypt with EPA, or maybe store with the Secrets API.  But that leads
> me back to auth-source.el since it already has this kind of logic in
> it.  WDYT about depending on auth-source.el for GnuTLS interactions?
> I'm divided because OTOH gnutls.el is a low-level wrapper and a
> dependency on auth-source.el will bring in other baggage.

Perhaps auth-source should split the Secrets API stuff out into a
separate library, and then both gnutls.el and auth-source.el could both
use it?

-- 
(domestic pets only, the antidote for overdose, milk.)
  larsi@gnus.org * Lars Magne Ingebrigtsen

Re: Testing the gnutls support

[Ted Zlatanov]

On Mon, 11 Oct 2010 18:25:25 +0200 Lars Magne Ingebrigtsen <larsi@gnus.org> wrote: 

LMI> Ted Zlatanov <tzz@lifelogs.com> writes:
>> I think this should be a file of certificates instead, which users can
>> encrypt with EPA, or maybe store with the Secrets API.  But that leads
>> me back to auth-source.el since it already has this kind of logic in
>> it.  WDYT about depending on auth-source.el for GnuTLS interactions?
>> I'm divided because OTOH gnutls.el is a low-level wrapper and a
>> dependency on auth-source.el will bring in other baggage.

LMI> Perhaps auth-source should split the Secrets API stuff out into a
LMI> separate library, and then both gnutls.el and auth-source.el could both
LMI> use it?

I mean the more general logic for "here's a host name and a protocol and
some search criteria, find some authentication tokens."  That's really
90% of auth-source.el and it seems that I would reimplement most of it
if gnutls.el did its own handling of certificate storage.

I think I'll change the auth-source API, since I'll be rewriting at
least some of that library.  It may make sense to make the query
language more powerful, handle get/change/delete operations on any
search results, and make the authentication tokens more than just
strings (they'll probably embed all the information needed for a search,
acting like database table rows).  I'll add a layer for backward
compatibility, of course.

Ted

Re: Testing the gnutls support

[Lars Magne Ingebrigtsen]

I've now made url.el use gnutls.el if it has been loaded, too.  This is
still experimental code, but if you want to test it, say

(require 'gnutls)

in your .emacs.

-- 
(domestic pets only, the antidote for overdose, milk.)
  larsi@gnus.org * Lars Magne Ingebrigtsen

Re: Testing the gnutls support

[Ted Zlatanov]

On Sat, 09 Oct 2010 23:50:55 -0500 Ted Zlatanov <tzz@lifelogs.com> wrote: 

TZ> 1) we should be verifying the host name matches the certificate--but
TZ> should this be done in gnutls.el or gnutls.c?

This is done in gnutls.c (using a host name passed through
`gnutls-negotiate' from gnutls.el) now.  The verification can be turned
off.

TZ> 2) we should set up a verification callback (but this is not available
TZ> in mainstream Debian/Ubuntu yet, since it's new in 2.10).  In the
TZ> callback we should let the user accept an invalid certificate.  I'd like
TZ> to defer this until 2.10 is in the Debian mainstream.

TZ> 3) We should give users a way to accept certificates.  Right now they
TZ> can pass a list of trust file names themselves but I think this should
TZ> be a more general facility.

I am not sure this is useful as part of the GnuTLS connection mechanism
because pausing external network-level C functions to interact with the
user can cause some serious bugs.  It seemed like a good approach but
after working with the GnuTLS code I'm pretty sure it will cause
problems long-term.

It's probably cleaner to save every invalid certificate in a list and
give the user a UI to choose which certificates they wish to accept,
perhaps linking to the last validation failure and whatever else will
help the user identify which certificates he wants to accept (maybe a
hash ID of the certificate in the messages buffer).

Any opinions?

Ted

Re: Testing the gnutls support

[Ted Zlatanov]

On Mon, 28 Mar 2011 13:52:26 -0500 Ted Zlatanov <tzz@lifelogs.com> wrote: 

TZ> On Sat, 09 Oct 2010 23:50:55 -0500 Ted Zlatanov <tzz@lifelogs.com> wrote: 
TZ> 1) we should be verifying the host name matches the certificate--but
TZ> should this be done in gnutls.el or gnutls.c?

TZ> This is done in gnutls.c (using a host name passed through
TZ> `gnutls-negotiate' from gnutls.el) now.  The verification can be turned
TZ> off.

s/done/done in the GnuTLS patch posted here last week/, sorry :)

Ted

Re: Testing the gnutls support

[Lars Magne Ingebrigtsen]

Ted Zlatanov <tzz@lifelogs.com> writes:

> It's probably cleaner to save every invalid certificate in a list and
> give the user a UI to choose which certificates they wish to accept,
> perhaps linking to the last validation failure and whatever else will
> help the user identify which certificates he wants to accept (maybe a
> hash ID of the certificate in the messages buffer).

What's the use case here?

If I'm connecting to imap.gmail.com, I probably do want to be prompted
with a "invalid certificate" if the certificate is invalid.  And
possibly a "view certificate" before accepting it anyway.  Is anything
more complicated than that necessary?

-- 
(domestic pets only, the antidote for overdose, milk.)
  larsi@gnus.org * Lars Magne Ingebrigtsen

Re: Testing the gnutls support

[Ted Zlatanov]

On Tue, 29 Mar 2011 22:29:59 +0200 Lars Magne Ingebrigtsen <larsi@gnus.org> wrote: 

LMI> Ted Zlatanov <tzz@lifelogs.com> writes:
>> It's probably cleaner to save every invalid certificate in a list and
>> give the user a UI to choose which certificates they wish to accept,
>> perhaps linking to the last validation failure and whatever else will
>> help the user identify which certificates he wants to accept (maybe a
>> hash ID of the certificate in the messages buffer).

LMI> What's the use case here?

LMI> If I'm connecting to imap.gmail.com, I probably do want to be prompted
LMI> with a "invalid certificate" if the certificate is invalid.  And
LMI> possibly a "view certificate" before accepting it anyway.  Is anything
LMI> more complicated than that necessary?

Normally GnuTLS-using programs, through a callback, do the prompting and
viewing when the invalid certificate is presented.  I think, considering
Emacs as an environment, that doing minibuffer prompting during a C
callback from an external library can cause serious problems.  So I'd
rather save the invalid certificate in a list at the time it's
presented and fail the connection.

After the connection fails, the code that uses gnutls.el can look at
`gnutls-rejected-certificates' (which will have the certificate and
enough information about the connection to figure out what it's for).
And it can then save some of those certificates and `gnutls-negotiate'
will pick them up.

`gnutls-negotiate' can pick up certificates either implicitly by trying
~/.emacs.d/certs/SERVER[.PORT].pem or explicitly if they are passed in
externally.  The GnuTLS maintainers suggested the former approach.  I
think it's more manageable long-term as well.

So, from the proto-stream.el perspective, you would try the connection
and if it fails, look at `gnutls-rejected-certificates' for an entry
relevant to the connection you just failed to make.  You would then ask
the user "do you want to accept certificate?" and show the info; if they
accept you'd save to ~/.emacs.d/certs/SERVER[.PORT].pem.

To know if you need to save the port in the name you could ask
auth-source for all the entries for SERVER or you could ask the user.

Ted

Re: Testing the gnutls support

[Lars Magne Ingebrigtsen]

Ted Zlatanov <tzz@lifelogs.com> writes:

> After the connection fails, the code that uses gnutls.el can look at
> `gnutls-rejected-certificates' (which will have the certificate and
> enough information about the connection to figure out what it's for).
> And it can then save some of those certificates and `gnutls-negotiate'
> will pick them up.

Right; that makes sense, I think.

> So, from the proto-stream.el perspective, you would try the connection
> and if it fails, look at `gnutls-rejected-certificates' for an entry
> relevant to the connection you just failed to make.  You would then ask
> the user "do you want to accept certificate?" and show the info; if they
> accept you'd save to ~/.emacs.d/certs/SERVER[.PORT].pem.

Do we really need to save the certificate?  Wouldn't just noting that we
accepted the cert suffice?

-- 
(domestic pets only, the antidote for overdose, milk.)
  larsi@gnus.org * Lars Magne Ingebrigtsen

Re: Testing the gnutls support

[Ted Zlatanov]

On Fri, 01 Apr 2011 17:39:21 +0200 Lars Magne Ingebrigtsen <larsi@gnus.org> wrote: 

>> So, from the proto-stream.el perspective, you would try the connection
>> and if it fails, look at `gnutls-rejected-certificates' for an entry
>> relevant to the connection you just failed to make.  You would then ask
>> the user "do you want to accept certificate?" and show the info; if they
>> accept you'd save to ~/.emacs.d/certs/SERVER[.PORT].pem.

LMI> Do we really need to save the certificate?  Wouldn't just noting that we
LMI> accepted the cert suffice?

Yes, because another invalid certificate presented by the same host
shouldn't be automatically accepted.  Are you OK with the rest of the
flow?  I can add at least the defcustoms and UI necessary to the GnuTLS
patch, and maybe add the proto-stream.el glue.

Ted

Re: Testing the gnutls support

[Lars Magne Ingebrigtsen]

Ted Zlatanov <tzz@lifelogs.com> writes:

> Yes, because another invalid certificate presented by the same host
> shouldn't be automatically accepted.

Well, one could store a SHA1 of the certificate or something, but I
guess that doesn't make much of a difference...

> Are you OK with the rest of the flow?  I can add at least the
> defcustoms and UI necessary to the GnuTLS patch, and maybe add the
> proto-stream.el glue.

Yes, sounds to me.

-- 
(domestic pets only, the antidote for overdose, milk.)
  bloggery blog http://lars.ingebrigtsen.no/