git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Hitoshi Mitake <mitake@dcl.info.waseda.ac.jp>
Cc: git@vger.kernel.org, Erik Faye-Lund <kusmabite@googlemail.com>,
	Jakub Narebski <jnareb@gmail.com>,
	Linus Torvalds <torvalds@linux-foundation.org>,
	Jeff King <peff@peff.net>
Subject: Re: [PATCH v4 1/2] git-imap-send: Add CRAM-MD5 authenticate method support
Date: Mon, 15 Feb 2010 22:34:07 -0800	[thread overview]
Message-ID: <7v8watg04g.fsf@alter.siamese.dyndns.org> (raw)
In-Reply-To: <7vsk95sht6.fsf@alter.siamese.dyndns.org> (Junio C. Hamano's message of "Fri\, 12 Feb 2010 23\:42\:13 -0800")

Junio C Hamano <gitster@pobox.com> writes:

> If you know the length of the decoded thing, then you would just know
> how much to hash.  Doesn't the EVP_DecodeBlock() give you that number?
> Why do you need a NUL termination to begin with?
>
> Because you pretend as if you do not have the actual length, you run
> strlen() instead.  I am not that familiar with the API to EVP_* functions,
> but I'd be surprised if it were designed in such a stupid way to force you
> to write into a pre-zeroed buffer.
>
> By the way, if you use strlen() on a pre-cleared and overallocated buffer,
> doesn't your ENCODED_SIZE(n) have to be one byte longer than what you are
> computing?
>
> Looking for EVP_DecodeBlock in http://www.google.com/codesearch seems to
> find usage examples of varying quality.  Your favorite isync-0.5 stores
> the result in "len", but it entirely ignores it and does the same silly
> calloc() and strlen().  Usage example in OpenSSL's own x509spki stores the
> return value in spki_len and uses that as the length of the stuff to call
> another function, which looks much more reasonable.  From this observation
> and a bit of reading of the manual, my understanding is that the function
> gives you the number of bytes written in the buffer.
>
> So does EncodeBlock(), I would think.

So here is a replacement version to see if I remember all the points
raised in the thread.

 * Use of HMAC_Init() is kept, so people with ancient OpenSSL can use it;

 * Lose "overallocate with xcalloc() and use strlen() to see how long the
   result is" pattern; EVP_DecodeBlock() and EVP_EncodeBlock() should be
   returning the length anyway, so use that directly;

 * Comment style fixes;

 * Lose "fprintf() then exit()"; die() instead; and

 * Call HMAC_CTX_cleanup() when done;

I didn't check the validity of the last one at all.  Please see if there
is any funnies.

-- >8 --
Subject: imap-send: support CRAM-MD5 authentication

CRAM-MD5 authentication ought to be independent from SSL, but NO_OPENSSL
build will not support this because the base64 and md5 code are used from
the OpenSSL library in this implementation.

Signed-off-by: Hitoshi Mitake <mitake@dcl.info.waseda.ac.jp>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

---
 Documentation/git-imap-send.txt |    4 +
 imap-send.c                     |  146 +++++++++++++++++++++++++++++++++++----
 2 files changed, 135 insertions(+), 15 deletions(-)

diff --git a/Documentation/git-imap-send.txt b/Documentation/git-imap-send.txt
index 57db955..6cafbe2 100644
--- a/Documentation/git-imap-send.txt
+++ b/Documentation/git-imap-send.txt
@@ -71,6 +71,10 @@ imap.preformattedHTML::
 	option causes Thunderbird to send the patch as a plain/text,
 	format=fixed email.  Default is `false`.
 
+imap.authMethod::
+	Specify authenticate method for authentication with IMAP server.
+	Current supported method is 'CRAM-MD5' only.
+
 Examples
 ~~~~~~~~
 
diff --git a/imap-send.c b/imap-send.c
index ba72fa4..bf1cd73 100644
--- a/imap-send.c
+++ b/imap-send.c
@@ -27,6 +27,9 @@
 #include "run-command.h"
 #ifdef NO_OPENSSL
 typedef void *SSL;
+#else
+#include <openssl/evp.h>
+#include <openssl/hmac.h>
 #endif
 
 struct store_conf {
@@ -140,6 +143,20 @@ struct imap_server_conf {
 	int use_ssl;
 	int ssl_verify;
 	int use_html;
+	char *auth_method;
+};
+
+static struct imap_server_conf server = {
+	NULL,	/* name */
+	NULL,	/* tunnel */
+	NULL,	/* host */
+	0,	/* port */
+	NULL,	/* user */
+	NULL,	/* pass */
+	0,   	/* use_ssl */
+	1,   	/* ssl_verify */
+	0,   	/* use_html */
+	NULL,	/* auth_method */
 };
 
 struct imap_store_conf {
@@ -214,6 +231,7 @@ enum CAPABILITY {
 	LITERALPLUS,
 	NAMESPACE,
 	STARTTLS,
+	AUTH_CRAM_MD5,
 };
 
 static const char *cap_list[] = {
@@ -222,6 +240,7 @@ static const char *cap_list[] = {
 	"LITERAL+",
 	"NAMESPACE",
 	"STARTTLS",
+	"AUTH=CRAM-MD5",
 };
 
 #define RESP_OK    0
@@ -949,6 +968,87 @@ static void imap_close_store(struct store *ctx)
 	free(ctx);
 }
 
+#ifndef NO_OPENSSL
+
+/*
+ * hexchar() and cram() functions are based on the code from the isync
+ * project (http://isync.sf.net/).
+ */
+static char hexchar(unsigned int b)
+{
+	return b < 10 ? '0' + b : 'a' + (b - 10);
+}
+
+#define ENCODED_SIZE(n) (4*((n+2)/3))
+static char *cram(const char *challenge_64, const char *user, const char *pass)
+{
+	int i, resp_len, encoded_len, decoded_len;
+	HMAC_CTX hmac;
+	unsigned char hash[16];
+	char hex[33];
+	char *response, *response_64, *challenge;
+
+	/*
+	 * length of challenge_64 (i.e. base-64 encoded string) is a good
+	 * enough upper bound for challenge (decoded result).
+	 */
+	encoded_len = strlen(challenge_64);
+	challenge = xmalloc(encoded_len);
+	decoded_len = EVP_DecodeBlock((unsigned char *)challenge,
+				      (unsigned char *)challenge_64, encoded_len);
+	if (decoded_len < 0)
+		die("invalid challenge %s", challenge_64);
+	HMAC_Init(&hmac, (unsigned char *)pass, strlen(pass), EVP_md5());
+	HMAC_Update(&hmac, (unsigned char *)challenge, decoded_len);
+	HMAC_Final(&hmac, hash, NULL);
+	HMAC_CTX_cleanup(&hmac);
+
+	hex[32] = 0;
+	for (i = 0; i < 16; i++) {
+		hex[2 * i] = hexchar((hash[i] >> 4) & 0xf);
+		hex[2 * i + 1] = hexchar(hash[i] & 0xf);
+	}
+
+	/* response: "<user> <digest in hex>" */
+	resp_len = strlen(user) + 1 + strlen(hex) + 1;
+	response = xmalloc(resp_len);
+	sprintf(response, "%s %s", user, hex);
+
+	response_64 = xmalloc(ENCODED_SIZE(resp_len));
+	encoded_len = EVP_EncodeBlock((unsigned char *)response_64,
+				      (unsigned char *)response, resp_len);
+	if (encoded_len < 0)
+		die("EVP_EncodeBlock error");
+	response_64[encoded_len] = '\0';
+	return (char *)response_64;
+}
+
+#else
+
+static char *cram(const char *challenge_64, const char *user, const char *pass)
+{
+	die("If you want to use CRAM-MD5 authenticate method, "
+	    "you have to build git-imap-send with OpenSSL library.");
+}
+
+#endif
+
+static int auth_cram_md5(struct imap_store *ctx, struct imap_cmd *cmd, const char *prompt)
+{
+	int ret;
+	char *response;
+
+	response = cram(prompt, server.user, server.pass);
+
+	ret = socket_write(&ctx->imap->buf.sock, response, strlen(response));
+	if (ret != strlen(response))
+		return error("IMAP error: sending response failed\n");
+
+	free(response);
+
+	return 0;
+}
+
 static struct store *imap_open_store(struct imap_server_conf *srvc)
 {
 	struct imap_store *ctx;
@@ -1130,9 +1230,34 @@ static struct store *imap_open_store(struct imap_server_conf *srvc)
 		if (!imap->buf.sock.ssl)
 			imap_warn("*** IMAP Warning *** Password is being "
 				  "sent in the clear\n");
-		if (imap_exec(ctx, NULL, "LOGIN \"%s\" \"%s\"", srvc->user, srvc->pass) != RESP_OK) {
-			fprintf(stderr, "IMAP error: LOGIN failed\n");
-			goto bail;
+
+		if (srvc->auth_method) {
+			struct imap_cmd_cb cb;
+
+			if (!strcmp(srvc->auth_method, "CRAM-MD5")) {
+				if (!CAP(AUTH_CRAM_MD5)) {
+					fprintf(stderr, "You specified"
+						"CRAM-MD5 as authentication method, "
+						"but %s doesn't support it.\n", srvc->host);
+					goto bail;
+				}
+				/* CRAM-MD5 */
+
+				memset(&cb, 0, sizeof(cb));
+				cb.cont = auth_cram_md5;
+				if (imap_exec(ctx, &cb, "AUTHENTICATE CRAM-MD5") != RESP_OK) {
+					fprintf(stderr, "IMAP error: AUTHENTICATE CRAM-MD5 failed\n");
+					goto bail;
+				}
+			} else {
+				fprintf(stderr, "Unknown authentication method:%s\n", srvc->host);
+				goto bail;
+			}
+		} else {
+			if (imap_exec(ctx, NULL, "LOGIN \"%s\" \"%s\"", srvc->user, srvc->pass) != RESP_OK) {
+				fprintf(stderr, "IMAP error: LOGIN failed\n");
+				goto bail;
+			}
 		}
 	} /* !preauth */
 
@@ -1310,18 +1435,6 @@ static int split_msg(struct msg_data *all_msgs, struct msg_data *msg, int *ofs)
 	return 1;
 }
 
-static struct imap_server_conf server = {
-	NULL,	/* name */
-	NULL,	/* tunnel */
-	NULL,	/* host */
-	0,	/* port */
-	NULL,	/* user */
-	NULL,	/* pass */
-	0,   	/* use_ssl */
-	1,   	/* ssl_verify */
-	0,   	/* use_html */
-};
-
 static char *imap_folder;
 
 static int git_imap_config(const char *key, const char *val, void *cb)
@@ -1361,6 +1474,9 @@ static int git_imap_config(const char *key, const char *val, void *cb)
 		server.port = git_config_int(key, val);
 	else if (!strcmp("tunnel", key))
 		server.tunnel = xstrdup(val);
+	else if (!strcmp("authmethod", key))
+		server.auth_method = xstrdup(val);
+
 	return 0;
 }
 

  reply	other threads:[~2010-02-16  6:34 UTC|newest]

Thread overview: 49+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-02-06 19:26 imap.preformattedHTML and imap.sslverify Junio C Hamano
2010-02-08 22:31 ` Jeremy White
2010-02-08 23:05   ` Junio C Hamano
2010-02-09 12:09     ` [PATCH 0/4] Some improvements for git-imap-send Hitoshi Mitake
2010-02-09 15:06       ` Jeff King
2010-02-09 15:13         ` Erik Faye-Lund
2010-02-09 16:57           ` Jeff King
2010-02-09 18:37             ` Erik Faye-Lund
2010-02-09 18:54               ` Jeff King
2010-02-11 14:38       ` [PATCH v2 1/2] git-imap-send: Add CRAM-MD5 authenticate method support Hitoshi Mitake
2010-02-11 14:55         ` Erik Faye-Lund
2010-02-11 14:59           ` Hitoshi Mitake
2010-02-11 15:06           ` [PATCH v3 " Hitoshi Mitake
2010-02-11 20:30             ` Junio C Hamano
2010-02-12 11:23               ` Hitoshi Mitake
2010-02-11 14:38       ` [PATCH v2 2/2] git-imap-send: Convert LF to CRLF before storing patch to draft box Hitoshi Mitake
2010-02-11 20:48         ` Junio C Hamano
2010-02-12 11:24           ` Hitoshi Mitake
2010-02-11 14:45       ` [PATCH v2 1/2] git-imap-send: Add CRAM-MD5 authenticate method support Hitoshi Mitake
2010-02-11 14:45       ` [PATCH v2 2/2] git-imap-send: Convert LF to CRLF before storing patch to draft box Hitoshi Mitake
2010-02-12 11:36       ` [PATCH v4 1/2] git-imap-send: Add CRAM-MD5 authenticate method support Hitoshi Mitake
2010-02-12 12:41         ` Erik Faye-Lund
2010-02-13  4:21           ` Hitoshi Mitake
2010-02-12 21:44         ` Junio C Hamano
2010-02-13  6:49           ` Hitoshi Mitake
2010-02-13  6:56             ` [PATCH v5 " Hitoshi Mitake
2010-02-13  7:42             ` [PATCH v4 " Junio C Hamano
2010-02-16  6:34               ` Junio C Hamano [this message]
2010-02-17  9:17                 ` Hitoshi Mitake
2010-02-17  9:18                   ` [PATCH v6 " Hitoshi Mitake
2010-02-17 18:31                   ` [PATCH v4 " Junio C Hamano
2010-02-18 15:45                     ` Hitoshi Mitake
2010-02-17  8:51               ` Hitoshi Mitake
2010-02-12 11:36       ` [PATCH v4 2/2] git-imap-send: Convert LF to CRLF before storing patch to draft box Hitoshi Mitake
2010-02-09 12:09     ` [PATCH 1/4] Add base64 encoder and decoder Hitoshi Mitake
2010-02-09 14:45       ` Erik Faye-Lund
2010-02-11 14:37         ` Hitoshi Mitake
2010-02-09 12:09     ` [PATCH 2/4] Add stuffs for MD5 hash algorithm Hitoshi Mitake
2010-02-09 12:09     ` [PATCH 3/4] git-imap-send: Implement CRAM-MD5 auth method Hitoshi Mitake
2010-02-09 14:22       ` Erik Faye-Lund
2010-02-11 14:55         ` Hitoshi Mitake
2010-02-11 14:59           ` Erik Faye-Lund
2010-02-11 15:11             ` Hitoshi Mitake
2010-02-09 12:09     ` [PATCH 4/4] git-imap-send: Add method to convert from LF to CRLF Hitoshi Mitake
2010-02-09 16:15       ` Jakub Narebski
2010-02-11 14:37         ` Hitoshi Mitake
2010-02-09 17:24       ` Linus Torvalds
2010-02-09 18:20         ` Junio C Hamano
2010-02-11 14:38           ` Hitoshi Mitake

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=7v8watg04g.fsf@alter.siamese.dyndns.org \
    --to=gitster@pobox.com \
    --cc=git@vger.kernel.org \
    --cc=jnareb@gmail.com \
    --cc=kusmabite@googlemail.com \
    --cc=mitake@dcl.info.waseda.ac.jp \
    --cc=peff@peff.net \
    --cc=torvalds@linux-foundation.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).