Re: [PATCH] Add $socket_timeout configuration option.

[egbert.]

On Wed, Jul 15, 2015 at 08:15:55PM -0700, Kevin J. McCarthy wrote:
> Kevin J. McCarthy wrote:
> > Kevin J. McCarthy wrote:
> > > I wasn't really happy having a loop counter option for GnuTLS.  It turns
> > > out the GNUTLS_E_AGAIN is translated directly from EAGAIN/EWOULDBLOCK.
> > > Since we're not using non-blocking io, this will only be triggered by
> > > our timeout.
> > 
> > Sorry, I was wrong.  It will return GNUTLS_E_AGAIN for situations other
> > than the timeout we set, so my patch is incorrect.  I'll have to think
> > about this some more.
> 
> After looking some more, I've re-convinced myself the patch approach is
> actually okay.
> 
> It looks like GnuTLS will return the GNUTLS_E_AGAIN for a partial
> read only if recv returns a EAGAIN or EINTR.  Otherwise it will keep
> looping until it gets the needed amount. (see _gnutls_stream_read() and
> _gnutls_io_read_buffered())
> 
> The EAGAIN should only occur for a timeout on no data, and the EINTR
> should only occur for a signal.  Since the timeout is ours and we aren't
> setting signals, there is no more reason to loop or handle these errors
> than there is with raw_socket_read or raw_socket_write.  Therefore I
> believe patch should be okay after all.
> 
> I am attaching a somewhat more aggressive patch that removes the looping
> from tls_socket_read, and returns tls_socket_write to its behavior of
> aborting on any error.
> 
> Feedback and testing is definitely appreciated!

Looks like nice work, Kevin. Cool that this will hopefully get crossed off 
the list. But personally I can’t give any feedback or test it until next 
week at the earliest. 

Grts!

Allen

>  WHERE short SendmailWait;
>  WHERE short SleepTime INITVAL (1);
> +WHERE short SocketTimeout;
>  WHERE short TimeInc;
>  WHERE short Timeout;
>  WHERE short Wrap;
>  WHERE short WrapHeaders;
>  WHERE short WriteInc;
>  
>  WHERE short ScoreThresholdDelete;
>  WHERE short ScoreThresholdRead;
> diff --git a/init.h b/init.h
> --- a/init.h
> +++ b/init.h
> @@ -2883,16 +2883,22 @@
>** smtp[s]://[user[:pass]@]host[:port]
>** .te
>** .pp
>** where ``[...]'' denotes an optional part.
>** Setting this variable overrides the value of the $$sendmail
>** variable.
>*/
>  #endif /* USE_SMTP */
> +  { "socket_timeout", DT_NUM, R_NONE, UL &SocketTimeout, 30 },
> +  /*
> +  ** .pp
> +  ** Set socket send and receive timeouts to this many seconds.  When
> +  ** set to zero, send and receive operations will not time out.
> +  */
>{ "sort",  DT_SORT, R_INDEX|R_RESORT, UL &Sort, SORT_DATE },
>/*
>** .pp
>** Specifies how to sort messages in the ``index'' menu.  Valid values
>** are:
>** .il
>** .dd date or date-sent
>** .dd date-received
> diff --git a/mutt_socket.c b/mutt_socket.c
> --- a/mutt_socket.c
> +++ b/mutt_socket.c
> @@ -339,16 +339,17 @@
>return 0;
>  }
>  
>  /* socket_connect: set up to connect to a socket fd. */
>  static int socket_connect (int fd, struct sockaddr* sa)
>  {
>int sa_size;
>int save_errno;
> +  struct timeval socket_timeout_tv = { 0, 0 };
>  
>if (sa->sa_family == AF_INET)
>  sa_size = sizeof (struct sockaddr_in);
>  #ifdef HAVE_GETADDRINFO
>else if (sa->sa_family == AF_INET6)
>  sa_size = sizeof (struct sockaddr_in6);
>  #endif
>else
> @@ -359,16 +360,35 @@
>
>if (ConnectTimeout > 0)
>alarm (ConnectTimeout);
>  
>mutt_allow_interrupt (1);
>  
>save_errno = 0;
>  
> +  if (SocketTimeout > 0)
> +  {
> +socket_timeout_tv.tv_sec = SocketTimeout;
> +if (setsockopt (fd, SOL_SOCKET, SO_RCVTIMEO, &socket_timeout_tv,
> +sizeof(struct timeval)))
> +{
> +  mutt_perror ("setsockopt");
> +  mutt_sleep(3);
> +  return -1;
> +}
> +if (setsockopt (fd, SOL_SOCKET, SO_SNDTIMEO, &socket_timeout_tv,
> +sizeof(struct timeval)))
> +{
> +  mutt_perror ("setsockopt");
> +  mutt_sleep(3);
> +  return -1;
> +}
> +  }
> +
>if (connect (fd, sa, sa_size) < 0)
>{
>save_errno = errno;
>dprint (2, (debugfile, "Connection failed. errno: %d...\n", errno));
>SigInt = 0;/* reset in case we caught SIGINTR while in connect() */
>}
>  
>if (ConnectTimeout > 0)
> diff --git a/mutt_ssl_gnutls.c b/mutt_ssl_gnutls.c
> --- a/mutt_ssl_gnutls.c
> +++ b/mutt_ssl_gnutls.c
> @@ -134,26 +134,23 @@
>  
>if (!data)
>{
>  mutt_error (_("Error: no TLS socket open"));
>  mutt_sleep (2);
>  return -1;
>}
>  
> -  do {
> -ret = gnutls_record_recv (data->state, buf, len);
> -if (ret < 0 && gnutls_error_is_fatal(ret) == 1)
> -{
> -  mutt_error ("tls_socket_read (%s)", gnutls_strerror (ret));
> -  mutt_sleep (4);
> -  return -1;
> -}
> +  ret = gnutls_record_recv (data->state, buf, len);
> +  if (ret < 0)
> +  {
> +mutt_error ("tls_socket_read (%s)", gnutls_strerror (ret));
> +mutt_sleep (4);
> +return -1;
>}
> -

[Mutt] #3762: Segmentation fault in pgp_keys when a subkey is of type "authentication"

[Mutt]

#3762: Segmentation fault in pgp_keys when a subkey is of type "authentication"
---+--
 Reporter:  boyska |  Owner:  mutt-dev
 Type:  defect | Status:  new
 Priority:  major  |  Milestone:  1.6
Component:  crypto |Version:  1.5.23
 Keywords:  pgp, segfault  |
---+--
 I have tried hard to debug the exact cause of the bug, but my
 understanding of it is still not full. For sure it only seems to happen
 when I have a subkey of type "authentication" (that is `'a'`).

 This bug seems to be new of version `1.5.23`: I was not experiencing it
 with `-rc1`

 Despite this, handling it is quite simple, so I am attaching a patch.

-- 
Ticket URL: 
Mutt 
The Mutt mail user agent

Re: [Mutt] #3762: Segmentation fault in pgp_keys when a subkey is of type "authentication"

[Mutt]

#3762: Segmentation fault in pgp_keys when a subkey is of type "authentication"
-+---
  Reporter:  boyska  |  Owner:  mutt-dev
  Type:  defect  | Status:  new
  Priority:  major   |  Milestone:  1.6
 Component:  crypto  |Version:  1.5.23
Resolution:  |   Keywords:  pgp, segfault
-+---

Comment (by kevin8t8):

 If it's not private and the key is available on a keyserver, would you
 mind publishing the key id so I can take a look?  I can't see how
 rfc822_parse_adrlist(NULL, "") would segfault.

 Also, have you unset $pgp_ignore_subkeys, or is it the default (set)?

-- 
Ticket URL: 
Mutt 
The Mutt mail user agent

Re: [Mutt] #3762: Segmentation fault in pgp_keys when a subkey is of type "authentication"

[Mutt]

#3762: Segmentation fault in pgp_keys when a subkey is of type "authentication"
-+---
  Reporter:  boyska  |  Owner:  mutt-dev
  Type:  defect  | Status:  new
  Priority:  major   |  Milestone:  1.6
 Component:  crypto  |Version:  1.5.23
Resolution:  |   Keywords:  pgp, segfault
-+---

Comment (by kevin8t8):

 Also, would you confirm the exact version of Mutt you're experiencing this
 with (the output of mutt -v).

 Thanks!

-- 
Ticket URL: 
Mutt 
The Mutt mail user agent

Re: [PATCH] Add $socket_timeout configuration option.

[Kevin J. McCarthy]

egbert. wrote:
> Looks like nice work, Kevin. Cool that this will hopefully get crossed off 
> the list.

Thanks Allen, but I have to give you credit for the original patch!

> But personally I can’t give any feedback or test it until next 
> week at the earliest. 

That's fine.  Please just let me know how it goes when you have a chance
to test it.

-- 
Kevin J. McCarthy
GPG Fingerprint: 8975 A9B3 3AA3 7910 385C  5308 ADEF 7684 8031 6BDA
http://www.8t8.us/configs/gpg-key-transition-statement.txt


signature.asc
Description: PGP signature