From: Hitoshi Mitake <mitake@dcl.info.waseda.ac.jp>
Date: Thu, 11 Feb 2010 15:11:28 GMT
Subject: Re: [PATCH 3/4] git-imap-send: Implement CRAM-MD5 auth method
Message-ID: <4B741E20.3010602@dcl.info.waseda.ac.jp>
In-Reply-To: <40aa078e1002110659y9493052l3752c7e0afb6eb3c@mail.gmail.com>

(2010年02月11日 23:59), Erik Faye-Lund wrote:
> On Thu, Feb 11, 2010 at 3:55 PM, Hitoshi Mitake
> <mitake@dcl.info.waseda.ac.jp>  wrote:
>> (2010年02月09日 23:22), Erik Faye-Lund wrote:
>>>
>>> On Tue, Feb 9, 2010 at 1:09 PM, Hitoshi Mitake
>>> <mitake@dcl.info.waseda.ac.jp>    wrote:
>>>>
>>>> +static int auth_cram_md5(struct imap_store *ctx, struct imap_cmd *cmd,
>>>> const char *prompt)
>>>> +{
>>>> +       int ret;
>>>> +       char digest[DIGEST_HEX_LEN];
>>>> +       char buf[256], base64_out[256];
>>>> +
>>>> +       memset(buf, 0, 256);
>>>> +       base64_decode(buf, prompt, strlen(prompt));
>>>> +
>>>> +       memset(digest, 0, DIGEST_HEX_LEN);
>>>> +       md5_hex_hmac(digest, (const unsigned char *)buf, strlen(buf),
>>>> +                    (const unsigned char *)server.pass,
>>>> strlen(server.pass));
>>>> +
>>>> +       memset(buf, 0, 256);
>>>> +       strcpy(buf, server.user);
>>>> +       strcpy(buf + strlen(buf), " ");
>>>> +       strcpy(buf + strlen(buf), digest);
>>>> +       memset(base64_out, 0, 256);
>>>> +       base64_encode(base64_out, buf, strlen(buf));
>>>> +
>>>> +       ret = socket_write(&ctx->imap->buf.sock, base64_out,
>>>> strlen(base64_out));
>>>
>>> Since this is the only location in this function that accesses
>>> anything inside ctx, how about just passing the imap_socket itself to
>>> the function? That'd make it a bit simpler if, say, I was rewriting
>>> send-email in C and wanted to add CRAM-MD5 AUTH support (given that
>>> I'd done the work to use imap_socket first)...
>>>
>>
>> Do you mean that
>>   auth_cram_md5(struct imap_store *ctx, struct imap_cmd *cmd, const char
>> *prompt)
>> should be,
>>   auth_cram_md5(struct imap_socket *socket, struct imap_cmd *cmd, const char
>> *prompt)
>>   ?
>>
>> If this improves portability of cram-md5 auth, of course I agree.
>> But struct imap_socket is defined in imap-send.c yet.
>>
>
> Yes, it's what I meant. It's only a minor nit-pick, as some
> refactoring would have to be done anyway. But I think it'd be a good
> change to only pull in the state needed, but that's my personal
> opinion.

My latest patch doesn't change the point.
Because auth_cram_md5() is a call back, so changing type of it
needs some works...

>
>> If you want to separate imap-send.c and cram-md5 auth for git-send-email,
>> I'll cooperate :)
>>
>
> Not at this point, if ever. I'm fine with you not doing anything about
> my comment. I was merely thinking out loud... ;)
>

I'm looking forward to using your new git-send-email.
I'm also a heavy user of it :)