nmh-workers
[Top] [All Lists]

Re: [Nmh-workers] Big patch: Add XOAUTH2 support for SMTP and POP

2014-12-05 23:56:13
David Levine <levinedl(_at_)acm(_dot_)org> writes:

test-mhlogin failed for me because each line of my actual output
ends with a Ctrl-M.  On Linux, so I don't know why.

Did you use git am?  I just checked and found that plain old
patch(1) preserves the CRs in the tests, but git am loses them.
That's a shame.  Probably standard version control line ending
massaging involved here.  Oh well; guess I'll have to fix this in
fakehttp.c after all.

Any idea with test-inc and test-msgcheck would fail with:

    'inc: POP server does not support SASL'
    'msgchk: POP server does not support SASL'

I did build with sasl, of course, and confirmed with mhparam.

You don't need to build with SASL.  I wonder if make check
rebuilds everything?  Maybe you need to make clean first?
Not sure if the dependencies are right.  I'll see if I can
reproduce this.

warning: squelched 5 whitespace errors
warning: 10 lines add whitespace errors.

They seem to fall into 3 categories:

- in jsmn

  I'd rather leave this pristine.

- trailing space in an error message

  The error message really has that trailing space on FreeBSD.
  I forgot to highlight this issue earlier, but the 'refresh
  finds no http server' in test-send tests an error message from
  curl that appears on Linux but not FreeBSD.  I'm going to dig
  into curl to see if I can figure out what's going on, but it
  means I'm not entirely sure the test suite is correct on
  all platforms.

- trailing space in send.c that I moved to another line

  removed

You chose not to use Cyrus SASL for XOAUTH2.  I wouldn't
hesitate to use it:  nmh already can be configured Cyrus SASL
and some of us do use it.  It is configured in by the Fedora
package.

The SASL interaction here is super trivial.  It's the tiniest
part of the patch.  I wouldn't even know where to begin to do
what I've done with Cyrus instead.  And I'd be astonished if that
version wasn't bigger and more complicated.  It's so tiny I'll
include it right here:

    if (xoauth_client_res != NULL) {
        char *server_mechs;
        if ((server_mechs = EHLOset("AUTH")) == NULL
            || stringdex("XOAUTH2", server_mechs) == -1) {
            sm_end(NOTOK);
            return sm_ierror("SMTP server does not support SASL XOAUTH2");
        }
        if (sm_auth_xoauth2(xoauth_client_res) != RP_OK) {
            sm_end(NOTOK);
            return NOTOK;
        }
    }

[...]

static int
sm_auth_xoauth2(const char *client_res)
{
    int status = smtalk(SM_AUTH, "AUTH XOAUTH2 %s", client_res);
    if (status == 235) {
        /* It worked! */
        return RP_OK;
    }

    /*
     * Status is 334 and sm_reply.text contains base64-encoded JSON.  As far as
     * epg can tell, no matter the error, the JSON is always the same:
     * {"status":"400","schemes":"Bearer","scope":"https://mail.google.com/"}
     * I tried these errors:
     * - garbage token
     * - expired token
     * - wrong scope
     * - wrong username
     */
    /* Then we're supposed to send an empty response ("\r\n"). */
    smtalk(SM_AUTH, "");
    /*
     * And now we always get this, again, no matter the error:
     * 535-5.7.8 Username and Password not accepted. Learn more at
     * 535 5.7.8 http://support.google.com/mail/bin/answer.py?answer=14257
     */
    return RP_BHST;
}

To do:

I'll take care of these and come back with another patch after
the first round of feedback.

Thanks!

_______________________________________________
Nmh-workers mailing list
Nmh-workers(_at_)nongnu(_dot_)org
https://lists.nongnu.org/mailman/listinfo/nmh-workers

<Prev in Thread] Current Thread [Next in Thread>