#3862: Error in POP3 authentication via SASL mechanism DIGEST-MD5
-----------------------+----------------------
Reporter: g1pimutt | Owner: mutt-dev
Type: defect | Status: new
Priority: major | Milestone:
Component: POP | Version:
Resolution: | Keywords:
-----------------------+----------------------
Comment (by g1pimutt):
I was confused too, because the loop has so many exit points that is not
easy to understand what it does. After spreading dprint()s all over, I
came to the conclusion that in the case of DIGEST-MD5 the correct flow is
(apart from the interact-ion substeps)
1st iteration: send, receive, call sasl_client_step, get SASL_CONTINUE
2nd iteration: send, receive, call sasl_client_step, get SASL_OK
3rd iteration: send, receive, exit the loop without calling
sasl_client_step
After the loop ends, the code checks both rc == SASL_OK and inbuf ==
"+OK", while failures in the loop (i.e. sasl_client_step returning < 0)
are handled by "goto bail": all is fine.
The test as it was, {{{(rc != SASL_CONTINUE && (olen == 0 || rc !=
SASL_OK))}}}, misinterpreted olen==0 (by which sasl_client_step means
"next sent line must be empty") as "we are done", causing premature exit
from the loop and skipping the 3rd iteration. My SECOND patch fixes
exactly this defect.
> I think the only thing that makes me nervous is that it (theoretically)
could exit prematurely now.
You probably meant "loop forever", since {{{A && (B || C)}}} is certainly
true whenever {{{A && B}}} is true, whatever C. I don't think it can
happen, unless sasl_client_step is really buggy and keeps returning
SASL_CONTINUE.
I believe you were right in rejecting my first patch: we shouldn't look at
the POP3 protocol in the middle of the SASL exchange, even if we know the
authentication phase is over and was successful. That's why I would not
add {{{|| !mutt_strncmp(...)}}} to the test.
--
Ticket URL: <https://dev.mutt.org/trac/ticket/3862#comment:10>
Mutt <http://www.mutt.org/>
The Mutt mail user agent
