[PATCH v4] imapd.c: imapoptions: implement idle timeout

[ellie timoney]

This version of the patch is now on master.  Thanks very much for
putting it together :)

I've opened https://github.com/cyrusimap/cyrus-imapd/issues/42 for
backporting it to 2.5 -- expecting to do it soon, but making sure it
doesn't get forgotten if I get distracted!

Cheers,

ellie

On Thu, Sep 22, 2016, at 02:26 AM, Philipp Gesang wrote:
> Use the value of the configuration variable “timeout” as an upper
> limit in minutes for idle connections. To allow further
> customization, add a new configuration option “imapidletimeout”
> which, if greater than zero, will be used instead. The value
> defaults to zero (not set).
> 
> RFC 2177 recommends that a client re-issue the IDLE command at
> least every 29 minutes if it wishes to continue, otherwise the
> server is free to treat the client as disconnected.
> 
> The rationale is that sometimes connections aren’t properly
> reset. Currently, a connection is not collected if it was in IDLE
> state at that point. If this happens repeatedly, imapd keeps
> accumulating dead connections which can cause DOS. This patch
> solves the problem by forcing imapd to stop idling after
> exceeding the configured timeout.
> ---
>  imap/imapd.c    | 73
>  ++++++++++++++++++++++++++++++++++++++++++++++++++++++---
>  lib/imapoptions |  5 ++++
>  2 files changed, 75 insertions(+), 3 deletions(-)
> 
> diff --git a/imap/imapd.c b/imap/imapd.c
> index e2bd237..745110c 100644
> --- a/imap/imapd.c
> +++ b/imap/imapd.c
> @@ -58,6 +58,9 @@
>  #include <sys/wait.h>
>  #include <netinet/in.h>
>  #include <arpa/inet.h>
> +#include <time.h>
> +#include <stdbool.h>
> +#include <errno.h>
>  
>  #include <sasl/sasl.h>
>  
> @@ -3055,6 +3058,25 @@ static void cmd_id(char *tag)
>      imapd_id.did_id = 1;
>  }
>  
> +static bool deadline_exceeded(const struct timespec *d)
> +{
> +    struct timespec now;
> +
> +    if (d->tv_sec <= 0) {
> +        /* No deadline configured */
> +        return false;
> +    }
> +
> +    errno = 0;
> +    if (clock_gettime(CLOCK_MONOTONIC, &now) == -1) {
> +        syslog(LOG_ERR, "clock_gettime (%d %m): error reading clock",
> errno);
> +        return false;
> +    }
> +
> +    return now.tv_sec > d->tv_sec ||
> +           (now.tv_sec == d->tv_sec && now.tv_nsec > d->tv_nsec);
> +}
> +
>  /*
>   * Perform an IDLE command
>   */
> @@ -3064,6 +3086,26 @@ static void cmd_idle(char *tag)
>      int flags;
>      static struct buf arg;
>      static int idle_period = -1;
> +    static time_t idle_timeout = -1;
> +    struct timespec deadline = { 0, 0 };
> +
> +    if (idle_timeout == -1) {
> +        idle_timeout = config_getint(IMAPOPT_IMAPIDLETIMEOUT);
> +        if (idle_timeout <= 0) {
> +            idle_timeout = config_getint(IMAPOPT_TIMEOUT);
> +        }
> +        idle_timeout *= 60; /* unit is minutes */
> +    }
> +
> +    if (idle_timeout > 0) {
> +        errno = 0;
> +        if (clock_gettime(CLOCK_MONOTONIC, &deadline) == -1) {
> +            syslog(LOG_ERR, "clock_gettime (%d %m): error reading
> clock",
> +                   errno);
> +        } else {
> +            deadline.tv_sec += idle_timeout;
> +        }
> +    }
>  
>      if (!backend_current) {  /* Local mailbox */
>  
> @@ -3080,6 +3122,13 @@ static void cmd_idle(char *tag)
>  
>          index_release(imapd_index);
>          while ((flags = idle_wait(imapd_in->fd))) {
> +            if (deadline_exceeded(&deadline)) {
> +                syslog(LOG_DEBUG, "timeout for user '%s' while idling",
> +                       imapd_userid);
> +                shut_down(0);
> +                break;
> +            }
> +
>              if (flags & IDLE_INPUT) {
>                  /* Get continuation data */
>                  c = getword(imapd_in, &arg);
> @@ -3112,7 +3161,8 @@ static void cmd_idle(char *tag)
>          idle_stop(index_mboxname(imapd_index));
>      }
>      else {  /* Remote mailbox */
> -        int done = 0, shutdown = 0;
> +        int done = 0;
> +        enum { shutdown_skip, shutdown_bye, shutdown_silent } shutdown =
> shutdown_skip;
>          char buf[2048];
>  
>          /* get polling period */
> @@ -3144,6 +3194,14 @@ static void cmd_idle(char *tag)
>  
>          /* Pipe updates to client while waiting for end of command */
>          while (!done) {
> +            if (deadline_exceeded(&deadline)) {
> +                syslog(LOG_DEBUG,
> +                       "timeout for user '%s' while idling on remote
> mailbox",
> +                       imapd_userid);
> +                shutdown = shutdown_silent;
> +                goto done;
> +            }
> +
>              /* Flush any buffered output */
>              prot_flush(imapd_out);
>  
> @@ -3152,7 +3210,8 @@ static void cmd_idle(char *tag)
>                  (shutdown_file(buf, sizeof(buf)) ||
>                   (imapd_userid &&
>                    userdeny(imapd_userid, config_ident, buf,
>                    sizeof(buf))))) {
> -                shutdown = done = 1;
> +                done = 1;
> +                shutdown = shutdown_bye;
>                  goto done;
>              }
>  
> @@ -3176,12 +3235,20 @@ static void cmd_idle(char *tag)
>              pipe_until_tag(backend_current, tag, 0);
>          }
>  
> -        if (shutdown) {
> +        switch (shutdown) {
> +        case shutdown_bye:
> +            ;
>              char *p;
>  
>              for (p = buf; *p == '['; p++); /* can't have [ be first char
>              */
>              prot_printf(imapd_out, "* BYE [ALERT] %s\r\n", p);
> +            /* fallthrough */
> +        case shutdown_silent:
>              shut_down(0);
> +            break;
> +        case shutdown_skip:
> +        default:
> +            break;
>          }
>      }
>  
> diff --git a/lib/imapoptions b/lib/imapoptions
> index 2b26241..b4e238c 100644
> --- a/lib/imapoptions
> +++ b/lib/imapoptions
> @@ -2115,6 +2115,11 @@ product version in the capabilities */
>     allow a bit of leeway for clients that try to NOOP every 30
>     minutes. */
>  
> +{ "imapidletimeout", 0, INT }
> +/* Timeout for idling clients (RFC 2177) in minutes. If set to
> +   zero (the default) or less, the value of "timeout" will be
> +   used instead. */
> +
>  { "tls_ca_file", NULL, STRING, "2.5.0", "tls_client_ca_file" }
>  /* Deprecated in favor of \fItls_client_ca_file\fR. */
>