From: Erik Faye-Lund <kusmabite@googlemail.com>
Date: Thu, 11 Feb 2010 14:59:25 GMT
Subject: Re: [PATCH 3/4] git-imap-send: Implement CRAM-MD5 auth method
Message-ID: <40aa078e1002110659y9493052l3752c7e0afb6eb3c@mail.gmail.com>
In-Reply-To: <4B741A44.7060003@dcl.info.waseda.ac.jp>

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.

> 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... ;)

-- 
Erik "kusma" Faye-Lund