On Mon, Nov 09, 2009 at 10:01:27PM +0100, Nils Frohberg wrote:
> Hi,
>
> smtpd currently ignores the server capabilities advertised on a
> singleline or the last line of a multiline response:
>
> [...]
> <<< 220 mail.tako.de ESMTP
> >>> EHLO something.tako.de
> <<< 250-mail.tako.de
> <<< 250-PIPELINING
> <<< 250-8BITMIME
> <<< 250-SIZE 0
> <<< 250 AUTH LOGIN PLAIN CRAM-MD5
> mta: leaving smtp phase
> mta: new status for [email protected]: AUTH not available
> [...]
>
> Note that although AUTH is advertised, smtpd misses it.
>
> I have included a patch that fixes the problem and that also only
> parses the server reply (for AUTH and STARTTLS) during the EHLO
> phase.
nice diff, committed, thanks!
>
> Nils
>
>
> Index: client.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/smtpd/client.c,v
> retrieving revision 1.9
> diff -u -r1.9 client.c
> --- client.c 25 Oct 2009 20:43:29 -0000 1.9
> +++ client.c 9 Nov 2009 20:00:02 -0000
> @@ -859,7 +859,12 @@
>
> sp->reply[0] = '\0';
>
> - /* get a reply, dealing with multiline responses */
> + /* get a reply, dealing with multiline responses
> + * multiline responses are provided by 3 digits followed by '-', and
> + * are terminated the same as single line responses: either 3 digits,
> + * or 3 digits followed by ' '.
> + * all responses can have additional data starting at the 4th char.
> + */
> for (;;) {
> errno = 0;
> if ((ln = buf_getln(&sp->r)) == NULL) {
> @@ -875,17 +880,28 @@
> if (sp->verbose)
> fprintf(sp->verbose, "<<< %s\n", ln);
>
> - if (strlen(ln) == 3 || ln[3] == ' ')
> + /* if we have 3 chars, there's no multiline or data string */
> + if (strlen(ln) == 3)
> break;
> - else if (ln[3] != '-') {
> - cause = "150 garbled multiline reply";
> + else if (strlen(ln) < 4 || (ln[3] != ' ' && ln[3] != '-')) {
> + /* we didn't get the entire smtp reply code,
> + * or we got a wrong single-/multiline character.
> + */
> + cause = "150 garbled smtp reply";
> goto done;
> }
>
> - if (strcmp(ln + 4, "STARTTLS") == 0)
> - sp->exts[CLIENT_EXT_STARTTLS].have = 1;
> - if (strncmp(ln + 4, "AUTH", 4) == 0)
> - sp->exts[CLIENT_EXT_AUTH].have = 1;
> + /* save some useful server capabilities during ehlo reply */
> + if (sp->state == CLIENT_EHLO) {
> + if (strcmp(ln + 4, "STARTTLS") == 0)
> + sp->exts[CLIENT_EXT_STARTTLS].have = 1;
> + else if (strncmp(ln + 4, "AUTH", 4) == 0)
> + sp->exts[CLIENT_EXT_AUTH].have = 1;
> + }
> +
> + /* this is either single line or the last multiline reply */
> + if (ln[3] == ' ')
> + break;
> }
>
> /* validate reply code */