Hello!

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

> Hello!
>
> On Thu, Apr 22, 2010 at 06:12:57PM -0700, Alan Batie wrote:
>
> >
> > 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.
>
> 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.

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 Dounin
# HG changeset patch
# User Maxim Dounin <mdounin@mdounin.ru>
# Date 1272242475 -14400
# Node ID 54a94faecee92b6638a35b3dafe401efc461c647
# Parent 19b134bf21c0e34a17590df5d83813cbe62735be
Mail: only wait for CRLF from backend.

Backend reply shorter that one we expect (at least 3 chars + CRLF) currently
results in timeout. Wait only for CRLF and reject too short replies instead.

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

b->last += n;

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

@@ -743,11 +743,12 @@ ngx_mail_proxy_read_response(ngx_mail_se
}

p = b->pos;
+ n = b->last - b->pos - 2;

switch (s->protocol) {

case NGX_MAIL_POP3_PROTOCOL:
- if (p[0] == '+' && p[1] == 'O' && p[2] == 'K') {
+ if (n > 2 && p[0] == '+' && p[1] == 'O' && p[2] == 'K') {
return NGX_OK;
}
break;
@@ -756,7 +757,9 @@ ngx_mail_proxy_read_response(ngx_mail_se
switch (state) {

case ngx_imap_start:
- if (p[0] == '*' && p[1] == ' ' && p[2] == 'O' && p[3] == 'K') {
+ if (n > 3 && p[0] == '*' && p[1] == ' ' && p[2] == 'O'
+ && p[3] == 'K')
+ {
return NGX_OK;
}
break;
@@ -781,6 +784,10 @@ ngx_mail_proxy_read_response(ngx_mail_se
break;

default: /* NGX_MAIL_SMTP_PROTOCOL */
+ if (n < 3) {
+ break;
+ }
+
switch (state) {

case ngx_smtp_start:
_______________________________________________
nginx mailing list
nginx@nginx.org
http://nginx.org/mailman/listinfo/nginx