Imap deadlock bug in 0.7.65

At line 729 of ngx_mail_proxy_module.c, there is this check for how much
data was received from an imap server response:

if (b->last - b->pos < 5) {
    return NGX_AGAIN;
}

Our zimbra server, oddly enough, running nginx itself, returns “+ \r\n”
in response to the initial phase of a login. As this is only 4
characters, nginx goes back for more, only there isn’t any more coming,
resulting in a timeout. Changing 5 to 4 fixes the problem, though
probably a “MIN_IMAP_RESPONSE” define would probably be better.

Hello!

On Thu, Apr 22, 2010 at 06:12:57PM -0700, Alan Batie wrote:

characters, nginx goes back for more, only there isn’t any more coming,
resulting in a timeout. Changing 5 to 4 fixes the problem, though
probably a “MIN_IMAP_RESPONSE” define would probably be better.

As far as I understand RFC 3501 the only situation where "+ " CRLF
form is permitted is server challenage in AUTHENTICATE command.
And nginx doesn’t use AUTHENTICATE command while talking to backends,
it uses LOGIN command instead.

Could you please provide something like tcpdump -xXs0 of such
a connection and some more details about your backend?

I’m ok with relaxing the above check, just curious.

Maxim D.

Hello!

On Fri, Apr 23, 2010 at 06:46:02AM +0400, Maxim D. wrote:

}

it uses LOGIN command instead.

Could you please provide something like tcpdump -xXs0 of such
a connection and some more details about your backend?

I’m ok with relaxing the above check, just curious.

Just for the record: Alan pointed off-list that he uses Zimbra 6
as a backend. Looking int source code indeed reveals that Zimbra
uses nginx 0.5.30 with local patches which causes it to return
"+ " CRLF as literal continuation response.

I believe the following patch would be appropriate:

— a/src/mail/ngx_mail_proxy_module.c
+++ b/src/mail/ngx_mail_proxy_module.c
@@ -726,7 +726,12 @@ ngx_mail_proxy_read_response(ngx_mail_se

 b->last += n;
  • if (b->last - b->pos < 5) {
  • /*
  • * check up to 4 chars unconditionally
    
  • * minimal expected reply is "+ " CRLF
    
  • */
    
  • if (b->last - b->pos < 4) {
    return NGX_AGAIN;
    }

Alternatively, the attached one may be used. It bumps limit to 2
(just CRLF) and adds appropriate length checking to individual reply
checks.

Maxim D.