Re: [PATCH v11 3/9] imap-send: add PLAIN authentication method to OpenSSL

> On 2 Jun 2025, at 5:57 AM, Junio C Hamano <gitster@pobox.com> wrote:
> 
> Aditya Garg <gargaditya08@live.com> writes:
> 
>> #else
>> 
>> +static char *plain_base64(const char *user UNUSED,
>> +          const char *access_token UNUSED)
>> +{
>> +    die("You are trying to use PLAIN authenticate method "
>> +        "with OpenSSL library, but its support has not been compiled in.");
>> +}
> 
> This comment may apply also to the earlier OAuth related two stub
> functions, but this is the "#else" side of "#ifndef NO_OPENSSL";
> double negation always makes all our spin, but in short, this is
> "You are not building with OpenSSL".  We cannot quite look at the
> post context of this hunk for sanity check, but inside the cram()
> stub function ...
> 
>> static char *cram(const char *challenge_64 UNUSED,
>>          const char *user UNUSED,
>>          const char *pass UNUSED)
> 
>    die("If you want to use CRAM-MD5 authenticate method, "
>        "you have to build git-imap-send with OpenSSL library.");
> 
> ... is the message it dies with.  So, shouldn't the error from the
> new stub function also say "If you want to use PLAIN, you have to
> build with OpenSSL"?

>> +static int auth_plain(struct imap_store *ctx, const char *prompt UNUSED)
>> +{
>> +    int ret;
>> +    char *b64;
>> +
>> +    b64 = plain_base64(ctx->cfg->user, ctx->cfg->pass);
>> +    if (!b64)
>> +        return error("PLAIN: base64 encoding failed");
>> +
>> +    /* Send the base64-encoded response */
>> +    ret = socket_write(&ctx->imap->buf.sock, b64, strlen(b64));
>> +    if (ret != (int)strlen(b64)) {
>> +        free(b64);
>> +        return error("IMAP error: sending PLAIN response failed");
>> +    }
>> +
>> +    free(b64);
>> +    return 0;
>> +}
> 
> And the same comment about not gracefully failing when our side lack
> support, even though we gracefully fail when the other side lacks
> support, given to an earlier step also applies here.
> 
>> @@ -1209,7 +1273,22 @@ static struct imap_store *imap_open_store(struct imap_server_conf *srvc, const c
>>        if (srvc->auth_method) {
>>            struct imap_cmd_cb cb;
>> 
>> -            if (!strcmp(srvc->auth_method, "CRAM-MD5")) {
>> +            if (!strcmp(srvc->auth_method, "PLAIN")) {
>> +                if (!CAP(AUTH_PLAIN)) {
>> +                    fprintf(stderr, "You specified "
>> +                        "PLAIN as authentication method, "
>> +                        "but %s doesn't support it.\n", srvc->host);
>> +                    goto bail;
>> +                }
>> +                /* PLAIN */
>> +
>> +                memset(&cb, 0, sizeof(cb));
>> +                cb.cont = auth_plain;
>> +                if (imap_exec(ctx, &cb, "AUTHENTICATE PLAIN") != RESP_OK) {
>> +                    fprintf(stderr, "IMAP error: AUTHENTICATE PLAIN failed\n");
>> +                    goto bail;
>> +                }
>> +            } else if (!strcmp(srvc->auth_method, "CRAM-MD5")) {
>>                if (!CAP(AUTH_CRAM_MD5)) {
>>                    fprintf(stderr, "You specified "
>>                        "CRAM-MD5 as authentication method, "