[PATCH 2/4] git-imap-send: Add support for SSL.

[PATCH 2/4] git-imap-send: Add support for SSL.

[Robert Shearman]

Allow SSL to be used when a imaps:// URL is used for the host name.

Also, automatically use TLS when not using imaps:// by using the IMAP STARTTLS command, if the server supports it.

Tested with Courier and Gimap IMAP servers.
---
 Documentation/git-imap-send.txt |    5 +-
 Makefile                        |    4 +-
 imap-send.c                     |  166 +++++++++++++++++++++++++++++++++++----
 3 files changed, 157 insertions(+), 18 deletions(-)

diff --git a/Documentation/git-imap-send.txt b/Documentation/git-imap-send.txt
index b3d8da3..e4a5873 100644
--- a/Documentation/git-imap-send.txt
+++ b/Documentation/git-imap-send.txt
@@ -37,10 +37,11 @@ configuration file (shown with examples):
     Tunnel = "ssh -q user@server.com /usr/bin/imapd ./Maildir 2> /dev/null"
 
 [imap]
-    Host = imap.server.com
+    Host = imaps://imap.example.com
     User = bob
     Pass = pwd
-    Port = 143
+    Port = 993
+    sslverify = false
 ..........................
 
 
diff --git a/Makefile b/Makefile
index bddd1a7..d9265f7 100644
--- a/Makefile
+++ b/Makefile
@@ -1193,7 +1193,9 @@ endif
 git-%$X: %.o $(GITLIBS)
 	$(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) $(filter %.o,$^) $(LIBS)
 
-git-imap-send$X: imap-send.o $(LIB_FILE)
+git-imap-send$X: imap-send.o $(GITLIBS)
+	$(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) $(filter %.o,$^) \
+		$(LIBS) $(OPENSSL_LINK) $(OPENSSL_LIBSSL)
 
 http.o http-walker.o http-push.o transport.o: http.h
 
diff --git a/imap-send.c b/imap-send.c
index 89a1532..d138726 100644
--- a/imap-send.c
+++ b/imap-send.c
@@ -23,6 +23,12 @@
  */
 
 #include "cache.h"
+#ifdef NO_OPENSSL
+typedef void *SSL;
+#else
+# include <openssl/ssl.h>
+# include <openssl/err.h>
+#endif
 
 typedef struct store_conf {
 	char *name;
@@ -129,6 +135,8 @@ typedef struct imap_server_conf {
 	int port;
 	char *user;
 	char *pass;
+	int use_ssl;
+	int ssl_verify;
 } imap_server_conf_t;
 
 typedef struct imap_store_conf {
@@ -148,6 +156,7 @@ typedef struct _list {
 
 typedef struct {
 	int fd;
+	SSL *ssl;
 } Socket_t;
 
 typedef struct {
@@ -201,6 +210,7 @@ enum CAPABILITY {
 	UIDPLUS,
 	LITERALPLUS,
 	NAMESPACE,
+	STARTTLS,
 };
 
 static const char *cap_list[] = {
@@ -208,6 +218,7 @@ static const char *cap_list[] = {
 	"UIDPLUS",
 	"LITERAL+",
 	"NAMESPACE",
+	"STARTTLS",
 };
 
 #define RESP_OK    0
@@ -225,19 +236,101 @@ static const char *Flags[] = {
 	"Deleted",
 };
 
+#ifndef NO_OPENSSL
+static void ssl_socket_perror(const char *func)
+{
+	fprintf(stderr, "%s: %s\n", func, ERR_error_string(ERR_get_error(), 0));
+}
+#endif
+
 static void
 socket_perror( const char *func, Socket_t *sock, int ret )
 {
-	if (ret < 0)
-		perror( func );
+#ifndef NO_OPENSSL
+	if (sock->ssl) {
+		int sslerr = SSL_get_error(sock->ssl, ret);
+		switch (sslerr) {
+		case SSL_ERROR_NONE:
+			break;
+		case SSL_ERROR_SYSCALL:
+			perror("SSL_connect");
+			break;
+		default:
+			ssl_socket_perror("SSL_connect");
+			break;
+		}
+	} else
+#endif
+	{
+		if (ret < 0)
+			perror(func);
+		else
+			fprintf(stderr, "%s: unexpected EOF\n", func);
+	}
+}
+
+static int ssl_socket_connect(Socket_t *sock, int use_tls_only, int verify)
+{
+#ifdef NO_OPENSSL
+	fprintf(stderr, "SSL requested but SSL support not compiled in\n");
+	return -1;
+#else
+	SSL_METHOD *meth;
+	SSL_CTX *ctx;
+	int ret;
+
+	SSL_library_init();
+	SSL_load_error_strings();
+
+	if (use_tls_only)
+		meth = TLSv1_method();
 	else
-		fprintf( stderr, "%s: unexpected EOF\n", func );
+		meth = SSLv23_method();
+
+	if (!meth) {
+		ssl_socket_perror("SSLv23_method");
+		return -1;
+	}
+
+	ctx = SSL_CTX_new(meth);
+
+	if (verify)
+		SSL_CTX_set_verify(ctx, SSL_VERIFY_PEER, NULL);
+
+	if (!SSL_CTX_set_default_verify_paths(ctx)) {
+		ssl_socket_perror("SSL_CTX_set_default_verify_paths");
+		return -1;
+	}
+	sock->ssl = SSL_new(ctx);
+	if (!sock->ssl) {
+		ssl_socket_perror("SSL_new");
+		return -1;
+	}
+	if (!SSL_set_fd(sock->ssl, sock->fd)) {
+		ssl_socket_perror("SSL_set_fd");
+		return -1;
+	}
+
+	ret = SSL_connect(sock->ssl);
+	if (ret <= 0) {
+		socket_perror("SSL_connect", sock, ret);
+		return -1;
+	}
+
+	return 0;
+#endif
 }
 
 static int
 socket_read( Socket_t *sock, char *buf, int len )
 {
-	ssize_t n = xread( sock->fd, buf, len );
+	ssize_t n;
+#ifndef NO_OPENSSL
+	if (sock->ssl)
+		n = SSL_read(sock->ssl, buf, len);
+	else
+#endif
+		n = xread( sock->fd, buf, len );
 	if (n <= 0) {
 		socket_perror( "read", sock, n );
 		close( sock->fd );
@@ -249,7 +342,13 @@ socket_read( Socket_t *sock, char *buf, int len )
 static int
 socket_write( Socket_t *sock, const char *buf, int len )
 {
-	int n = write_in_full( sock->fd, buf, len );
+	int n;
+#ifndef NO_OPENSSL
+	if (sock->ssl)
+		n = SSL_write(sock->ssl, buf, len);
+	else
+#endif
+		n = write_in_full( sock->fd, buf, len );
 	if (n != len) {
 		socket_perror( "write", sock, n );
 		close( sock->fd );
@@ -258,6 +357,17 @@ socket_write( Socket_t *sock, const char *buf, int len )
 	return n;
 }
 
+static void socket_shutdown(Socket_t *sock)
+{
+#ifndef NO_OPENSSL
+	if (sock->ssl) {
+		SSL_shutdown(sock->ssl);
+		SSL_free(sock->ssl);
+	}
+#endif
+	close(sock->fd);
+}
+
 /* simple line buffering */
 static int
 buffer_gets( buffer_t * b, char **s )
@@ -875,7 +985,7 @@ imap_close_server( imap_store_t *ictx )
 
 	if (imap->buf.sock.fd != -1) {
 		imap_exec( ictx, NULL, "LOGOUT" );
-		close( imap->buf.sock.fd );
+		socket_shutdown( &imap->buf.sock );
 	}
 	free_list( imap->ns_personal );
 	free_list( imap->ns_other );
@@ -906,6 +1016,7 @@ imap_open_store( imap_server_conf_t *srvc )
 
 	ctx->imap = imap = xcalloc( sizeof(*imap), 1 );
 	imap->buf.sock.fd = -1;
+	imap->buf.sock.ssl = NULL;
 	imap->in_progress_append = &imap->in_progress;
 
 	/* open connection to IMAP server */
@@ -958,10 +1069,15 @@ imap_open_store( imap_server_conf_t *srvc )
 			perror( "connect" );
 			goto bail;
 		}
-		imap_info( "ok\n" );
-
+		
 		imap->buf.sock.fd = s;
 
+		if (srvc->use_ssl &&
+		    ssl_socket_connect(&imap->buf.sock, 0, srvc->ssl_verify)) {
+			close(s);
+			goto bail;
+		}
+		imap_info( "ok\n" );
 	}
 
 	/* read the greeting string */
@@ -986,7 +1102,18 @@ imap_open_store( imap_server_conf_t *srvc )
 		goto bail;
 
 	if (!preauth) {
-
+#ifndef NO_OPENSSL
+		if (!srvc->use_ssl && CAP(STARTTLS)) {
+			if (imap_exec(ctx, 0, "STARTTLS") != RESP_OK)
+				goto bail;
+			if (ssl_socket_connect(&imap->buf.sock, 1,
+					       srvc->ssl_verify))
+				goto bail;
+			/* capabilities may have changed, so get the new capabilities */
+			if (imap_exec(ctx, 0, "CAPABILITY") != RESP_OK)
+				goto bail;
+		}
+#endif
 		imap_info ("Logging in...\n");
 		if (!srvc->user) {
 			fprintf( stderr, "Skipping server %s, no user\n", srvc->host );
@@ -1014,7 +1141,9 @@ imap_open_store( imap_server_conf_t *srvc )
 			fprintf( stderr, "Skipping account %s@%s, server forbids LOGIN\n", srvc->user, srvc->host );
 			goto bail;
 		}
-		imap_warn( "*** IMAP Warning *** Password is being sent in the clear\n" );
+		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;
@@ -1242,6 +1371,8 @@ static imap_server_conf_t server =
 	0,	/* port */
 	NULL,	/* user */
 	NULL,	/* pass */
+	0,   	/* use_ssl */
+	1,   	/* ssl_verify */
 };
 
 static char *imap_folder;
@@ -1262,11 +1393,11 @@ git_imap_config(const char *key, const char *val, void *cb)
 	if (!strcmp( "folder", key )) {
 		imap_folder = xstrdup( val );
 	} else if (!strcmp( "host", key )) {
-		{
-			if (!prefixcmp(val, "imap:"))
-				val += 5;
-			if (!server.port)
-				server.port = 143;
+		if (!prefixcmp(val, "imap:"))
+			val += 5;
+		else if (!prefixcmp(val, "imaps:")) {
+			val += 6;
+			server.use_ssl = 1;
 		}
 		if (!prefixcmp(val, "//"))
 			val += 2;
@@ -1280,6 +1411,8 @@ 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( "ssl_verify", key ))
+		server.ssl_verify = git_config_bool( key, val );
 	return 0;
 }
 
@@ -1299,6 +1432,9 @@ main(int argc, char **argv)
 	setup_git_directory_gently( NULL );
 	git_config(git_imap_config, NULL);
 
+	if (!server.port)
+		server.port = server.use_ssl ? 993 : 143;
+
 	if (!imap_folder) {
 		fprintf( stderr, "no imap store specified\n" );
 		return 1;
-- 
1.5.6.GIT

Re: [PATCH 2/4] git-imap-send: Add support for SSL.

[Junio C Hamano]

Robert Shearman <robertshearman@gmail.com> writes:

> Allow SSL to be used when a imaps:// URL is used for the host name.
>
> Also, automatically use TLS when not using imaps:// by using the IMAP STARTTLS command, if the server supports it.
>
> Tested with Courier and Gimap IMAP servers.

Sign-off?

> diff --git a/Documentation/git-imap-send.txt b/Documentation/git-imap-send.txt
> index b3d8da3..e4a5873 100644
> --- a/Documentation/git-imap-send.txt
> +++ b/Documentation/git-imap-send.txt
> @@ -37,10 +37,11 @@ configuration file (shown with examples):
>      Tunnel = "ssh -q user@server.com /usr/bin/imapd ./Maildir 2> /dev/null"
>  
>  [imap]
> -    Host = imap.server.com
> +    Host = imaps://imap.example.com
>      User = bob
>      Pass = pwd
> -    Port = 143
> +    Port = 993
> +    sslverify = false
>  ..........................

Don't we also want to keep a vanilla configuration in the example, or is
imaps the norm and unencrypted imap is exception these days?

Don't we need to support custom certificates, keys and CAs, just like our
code that supports https does, by honoring GIT_SSL_* environment variables
and configuration file entries?  The patch itself looks fairly clean, and
I'd like to queue this for wider testing, initially even without GIT_SSL_*
support.  But I'd like to see any patch with substantial amount of changes
properly signed off.

Thanks.

Re: [PATCH 2/4] git-imap-send: Add support for SSL.

[Abhijit Menon-Sen]

At 2008-07-08 16:20:19 -0700, gitster@pobox.com wrote:
>
> > -    Port = 143
> > +    Port = 993
> > +    sslverify = false
> >  ..........................
> 
> Don't we also want to keep a vanilla configuration in the example, or
> is imaps the norm and unencrypted imap is exception these days?

The norm is IMAP+STARTTLS on port 143, not IMAPS on port 993. The latter
is also widely deployed for compatibility with older clients, but it is
non-standard and its use isn't exactly encouraged.

-- ams

Re: [PATCH 2/4] git-imap-send: Add support for SSL.

[Rob Shearman]

2008/7/9 Junio C Hamano <gitster@pobox.com>:
> Robert Shearman <robertshearman@gmail.com> writes:
>> diff --git a/Documentation/git-imap-send.txt b/Documentation/git-imap-send.txt
>> index b3d8da3..e4a5873 100644
>> --- a/Documentation/git-imap-send.txt
>> +++ b/Documentation/git-imap-send.txt
>> @@ -37,10 +37,11 @@ configuration file (shown with examples):
>>      Tunnel = "ssh -q user@server.com /usr/bin/imapd ./Maildir 2> /dev/null"
>>
>>  [imap]
>> -    Host = imap.server.com
>> +    Host = imaps://imap.example.com
>>      User = bob
>>      Pass = pwd
>> -    Port = 143
>> +    Port = 993
>> +    sslverify = false
>>  ..........................
>
> Don't we also want to keep a vanilla configuration in the example, or is
> imaps the norm and unencrypted imap is exception these days?

Good point. I'll fix the documentation to use imap:// instead of
imaps:// and not change the port number. However, I'm not sure the
examples should be telling the user what they should do, but rather
what they can do.

> Don't we need to support custom certificates, keys and CAs, just like our
> code that supports https does, by honoring GIT_SSL_* environment variables
> and configuration file entries?

Yes, eventually we will want that support in imap-send too. It should
be fairly trivial to do, although testing will be more difficult.

>  The patch itself looks fairly clean, and
> I'd like to queue this for wider testing, initially even without GIT_SSL_*
> support.  But I'd like to see any patch with substantial amount of changes
> properly signed off.

Great. I'll resend the series later with changes from the comments
I've received and properly signed-off.

-- 
Rob Shearman

Re: [PATCH 2/4] git-imap-send: Add support for SSL.

[Josh Triplett]

On Tue, 2008-07-08 at 23:18 +0100, Robert Shearman wrote:
> --- a/Documentation/git-imap-send.txt
> +++ b/Documentation/git-imap-send.txt
> @@ -37,10 +37,11 @@ configuration file (shown with examples):
>      Tunnel = "ssh -q user@server.com /usr/bin/imapd ./Maildir 2> /dev/null"
>  
>  [imap]
> -    Host = imap.server.com
> +    Host = imaps://imap.example.com
>      User = bob
>      Pass = pwd
> -    Port = 143
> +    Port = 993
> +    sslverify = false
[...]
> @@ -1280,6 +1411,8 @@ 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( "ssl_verify", key ))
> +		server.ssl_verify = git_config_bool( key, val );

The example and the code disagree on the name of the
sslverify/ssl_verify option.  Also, ssl_verify needs explanation.

- Josh Triplett

Re: [PATCH 2/4] git-imap-send: Add support for SSL.

[Junio C Hamano]

"Rob Shearman" <robertshearman@gmail.com> writes:

> 2008/7/9 Junio C Hamano <gitster@pobox.com>:
> ...
>> Don't we also want to keep a vanilla configuration in the example, or is
>> imaps the norm and unencrypted imap is exception these days?
>
> Good point. I'll fix the documentation to use imap:// instead of
> imaps:// and not change the port number. However, I'm not sure the
> examples should be telling the user what they should do, but rather
> what they can do.

My comment was purely about losing the basic example by replacing it with
something more advanced.  Nobody prevents you from having _more_ examples.
"You can do this, you can do that, also here is an example of how to use
SSL".

Re: [PATCH 2/4] git-imap-send: Add support for SSL.

[Rob Shearman]

2008/7/9 Josh Triplett <josht@linux.vnet.ibm.com>:
> On Tue, 2008-07-08 at 23:18 +0100, Robert Shearman wrote:
>> --- a/Documentation/git-imap-send.txt
>> +++ b/Documentation/git-imap-send.txt
>> @@ -37,10 +37,11 @@ configuration file (shown with examples):
>>      Tunnel = "ssh -q user@server.com /usr/bin/imapd ./Maildir 2> /dev/null"
>>
>>  [imap]
>> -    Host = imap.server.com
>> +    Host = imaps://imap.example.com
>>      User = bob
>>      Pass = pwd
>> -    Port = 143
>> +    Port = 993
>> +    sslverify = false
> [...]
>> @@ -1280,6 +1411,8 @@ 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( "ssl_verify", key ))
>> +             server.ssl_verify = git_config_bool( key, val );
>
> The example and the code disagree on the name of the
> sslverify/ssl_verify option.

I wouldn't exactly call it "disagree". The config variable is limited
by not allowing underscores, whereas the C language does allow them.

> Also, ssl_verify needs explanation.

See patch 4/4.

-- 
Rob Shearman

[PATCH 2/4] git-imap-send: Add support for SSL.

[Robert Shearman]

From: Robert Shearman <rob@codeweavers.com>

Allow SSL to be used when a imaps:// URL is used for the host name.

Also, automatically use TLS when not using imaps:// by using the IMAP STARTTLS command, if the server supports it.

Tested with Courier and Gimap IMAP servers.

Signed-off-by: Robert Shearman <robertshearman@gmail.com>
---
 Documentation/git-imap-send.txt |    3 +-
 Makefile                        |    4 +-
 imap-send.c                     |  166 +++++++++++++++++++++++++++++++++++----
 3 files changed, 156 insertions(+), 17 deletions(-)

diff --git a/Documentation/git-imap-send.txt b/Documentation/git-imap-send.txt
index b3d8da3..136c82b 100644
--- a/Documentation/git-imap-send.txt
+++ b/Documentation/git-imap-send.txt
@@ -37,10 +37,11 @@ configuration file (shown with examples):
     Tunnel = "ssh -q user@server.com /usr/bin/imapd ./Maildir 2> /dev/null"
 
 [imap]
-    Host = imap.server.com
+    Host = imap://imap.example.com
     User = bob
     Pass = pwd
     Port = 143
+    sslverify = false
 ..........................
 
 
diff --git a/Makefile b/Makefile
index 4796565..55ec6ee 100644
--- a/Makefile
+++ b/Makefile
@@ -1193,7 +1193,9 @@ endif
 git-%$X: %.o $(GITLIBS)
 	$(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) $(filter %.o,$^) $(LIBS)
 
-git-imap-send$X: imap-send.o $(LIB_FILE)
+git-imap-send$X: imap-send.o $(GITLIBS)
+	$(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) $(filter %.o,$^) \
+		$(LIBS) $(OPENSSL_LINK) $(OPENSSL_LIBSSL)
 
 http.o http-walker.o http-push.o transport.o: http.h
 
diff --git a/imap-send.c b/imap-send.c
index 24d76a7..26d1dba 100644
--- a/imap-send.c
+++ b/imap-send.c
@@ -23,6 +23,12 @@
  */
 
 #include "cache.h"
+#ifdef NO_OPENSSL
+typedef void *SSL;
+#else
+# include <openssl/ssl.h>
+# include <openssl/err.h>
+#endif
 
 typedef struct store_conf {
 	char *name;
@@ -129,6 +135,8 @@ typedef struct imap_server_conf {
 	int port;
 	char *user;
 	char *pass;
+	int use_ssl;
+	int ssl_verify;
 } imap_server_conf_t;
 
 typedef struct imap_store_conf {
@@ -148,6 +156,7 @@ typedef struct _list {
 
 typedef struct {
 	int fd;
+	SSL *ssl;
 } Socket_t;
 
 typedef struct {
@@ -201,6 +210,7 @@ enum CAPABILITY {
 	UIDPLUS,
 	LITERALPLUS,
 	NAMESPACE,
+	STARTTLS,
 };
 
 static const char *cap_list[] = {
@@ -208,6 +218,7 @@ static const char *cap_list[] = {
 	"UIDPLUS",
 	"LITERAL+",
 	"NAMESPACE",
+	"STARTTLS",
 };
 
 #define RESP_OK    0
@@ -225,19 +236,101 @@ static const char *Flags[] = {
 	"Deleted",
 };
 
+#ifndef NO_OPENSSL
+static void ssl_socket_perror(const char *func)
+{
+	fprintf(stderr, "%s: %s\n", func, ERR_error_string(ERR_get_error(), 0));
+}
+#endif
+
 static void
 socket_perror( const char *func, Socket_t *sock, int ret )
 {
-	if (ret < 0)
-		perror( func );
+#ifndef NO_OPENSSL
+	if (sock->ssl) {
+		int sslerr = SSL_get_error(sock->ssl, ret);
+		switch (sslerr) {
+		case SSL_ERROR_NONE:
+			break;
+		case SSL_ERROR_SYSCALL:
+			perror("SSL_connect");
+			break;
+		default:
+			ssl_socket_perror("SSL_connect");
+			break;
+		}
+	} else
+#endif
+	{
+		if (ret < 0)
+			perror(func);
+		else
+			fprintf(stderr, "%s: unexpected EOF\n", func);
+	}
+}
+
+static int ssl_socket_connect(Socket_t *sock, int use_tls_only, int verify)
+{
+#ifdef NO_OPENSSL
+	fprintf(stderr, "SSL requested but SSL support not compiled in\n");
+	return -1;
+#else
+	SSL_METHOD *meth;
+	SSL_CTX *ctx;
+	int ret;
+
+	SSL_library_init();
+	SSL_load_error_strings();
+
+	if (use_tls_only)
+		meth = TLSv1_method();
 	else
-		fprintf( stderr, "%s: unexpected EOF\n", func );
+		meth = SSLv23_method();
+
+	if (!meth) {
+		ssl_socket_perror("SSLv23_method");
+		return -1;
+	}
+
+	ctx = SSL_CTX_new(meth);
+
+	if (verify)
+		SSL_CTX_set_verify(ctx, SSL_VERIFY_PEER, NULL);
+
+	if (!SSL_CTX_set_default_verify_paths(ctx)) {
+		ssl_socket_perror("SSL_CTX_set_default_verify_paths");
+		return -1;
+	}
+	sock->ssl = SSL_new(ctx);
+	if (!sock->ssl) {
+		ssl_socket_perror("SSL_new");
+		return -1;
+	}
+	if (!SSL_set_fd(sock->ssl, sock->fd)) {
+		ssl_socket_perror("SSL_set_fd");
+		return -1;
+	}
+
+	ret = SSL_connect(sock->ssl);
+	if (ret <= 0) {
+		socket_perror("SSL_connect", sock, ret);
+		return -1;
+	}
+
+	return 0;
+#endif
 }
 
 static int
 socket_read( Socket_t *sock, char *buf, int len )
 {
-	ssize_t n = xread( sock->fd, buf, len );
+	ssize_t n;
+#ifndef NO_OPENSSL
+	if (sock->ssl)
+		n = SSL_read(sock->ssl, buf, len);
+	else
+#endif
+		n = xread( sock->fd, buf, len );
 	if (n <= 0) {
 		socket_perror( "read", sock, n );
 		close( sock->fd );
@@ -249,7 +342,13 @@ socket_read( Socket_t *sock, char *buf, int len )
 static int
 socket_write( Socket_t *sock, const char *buf, int len )
 {
-	int n = write_in_full( sock->fd, buf, len );
+	int n;
+#ifndef NO_OPENSSL
+	if (sock->ssl)
+		n = SSL_write(sock->ssl, buf, len);
+	else
+#endif
+		n = write_in_full( sock->fd, buf, len );
 	if (n != len) {
 		socket_perror( "write", sock, n );
 		close( sock->fd );
@@ -258,6 +357,17 @@ socket_write( Socket_t *sock, const char *buf, int len )
 	return n;
 }
 
+static void socket_shutdown(Socket_t *sock)
+{
+#ifndef NO_OPENSSL
+	if (sock->ssl) {
+		SSL_shutdown(sock->ssl);
+		SSL_free(sock->ssl);
+	}
+#endif
+	close(sock->fd);
+}
+
 /* simple line buffering */
 static int
 buffer_gets( buffer_t * b, char **s )
@@ -875,7 +985,7 @@ imap_close_server( imap_store_t *ictx )
 
 	if (imap->buf.sock.fd != -1) {
 		imap_exec( ictx, NULL, "LOGOUT" );
-		close( imap->buf.sock.fd );
+		socket_shutdown( &imap->buf.sock );
 	}
 	free_list( imap->ns_personal );
 	free_list( imap->ns_other );
@@ -906,6 +1016,7 @@ imap_open_store( imap_server_conf_t *srvc )
 
 	ctx->imap = imap = xcalloc( sizeof(*imap), 1 );
 	imap->buf.sock.fd = -1;
+	imap->buf.sock.ssl = NULL;
 	imap->in_progress_append = &imap->in_progress;
 
 	/* open connection to IMAP server */
@@ -958,10 +1069,15 @@ imap_open_store( imap_server_conf_t *srvc )
 			perror( "connect" );
 			goto bail;
 		}
-		imap_info( "ok\n" );
-
+		
 		imap->buf.sock.fd = s;
 
+		if (srvc->use_ssl &&
+		    ssl_socket_connect(&imap->buf.sock, 0, srvc->ssl_verify)) {
+			close(s);
+			goto bail;
+		}
+		imap_info( "ok\n" );
 	}
 
 	/* read the greeting string */
@@ -986,7 +1102,18 @@ imap_open_store( imap_server_conf_t *srvc )
 		goto bail;
 
 	if (!preauth) {
-
+#ifndef NO_OPENSSL
+		if (!srvc->use_ssl && CAP(STARTTLS)) {
+			if (imap_exec(ctx, 0, "STARTTLS") != RESP_OK)
+				goto bail;
+			if (ssl_socket_connect(&imap->buf.sock, 1,
+					       srvc->ssl_verify))
+				goto bail;
+			/* capabilities may have changed, so get the new capabilities */
+			if (imap_exec(ctx, 0, "CAPABILITY") != RESP_OK)
+				goto bail;
+		}
+#endif
 		imap_info ("Logging in...\n");
 		if (!srvc->user) {
 			fprintf( stderr, "Skipping server %s, no user\n", srvc->host );
@@ -1014,7 +1141,9 @@ imap_open_store( imap_server_conf_t *srvc )
 			fprintf( stderr, "Skipping account %s@%s, server forbids LOGIN\n", srvc->user, srvc->host );
 			goto bail;
 		}
-		imap_warn( "*** IMAP Warning *** Password is being sent in the clear\n" );
+		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;
@@ -1242,6 +1371,8 @@ static imap_server_conf_t server =
 	0,	/* port */
 	NULL,	/* user */
 	NULL,	/* pass */
+	0,   	/* use_ssl */
+	1,   	/* ssl_verify */
 };
 
 static char *imap_folder;
@@ -1262,11 +1393,11 @@ git_imap_config(const char *key, const char *val, void *cb)
 	if (!strcmp( "folder", key )) {
 		imap_folder = xstrdup( val );
 	} else if (!strcmp( "host", key )) {
-		{
-			if (!prefixcmp(val, "imap:"))
-				val += 5;
-			if (!server.port)
-				server.port = 143;
+		if (!prefixcmp(val, "imap:"))
+			val += 5;
+		else if (!prefixcmp(val, "imaps:")) {
+			val += 6;
+			server.use_ssl = 1;
 		}
 		if (!prefixcmp(val, "//"))
 			val += 2;
@@ -1280,6 +1411,8 @@ 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( "ssl_verify", key ))
+		server.ssl_verify = git_config_bool( key, val );
 	return 0;
 }
 
@@ -1300,6 +1433,9 @@ main(int argc, char **argv)
 	setup_git_directory_gently(&nongit_ok);
 	git_config(git_imap_config, NULL);
 
+	if (!server.port)
+		server.port = server.use_ssl ? 993 : 143;
+
 	if (!imap_folder) {
 		fprintf( stderr, "no imap store specified\n" );
 		return 1;
-- 
1.5.6.2.224.g26efb.dirty

Re: [PATCH 2/4] git-imap-send: Add support for SSL.

[Junio C Hamano]

Robert Shearman <robertshearman@gmail.com> writes:

> diff --git a/imap-send.c b/imap-send.c
> index 24d76a7..26d1dba 100644
> --- a/imap-send.c
> +++ b/imap-send.c
> @@ -23,6 +23,12 @@
>   */
>  
>  #include "cache.h"
> +#ifdef NO_OPENSSL
> +typedef void *SSL;
> +#else
> +# include <openssl/ssl.h>
> +# include <openssl/err.h>
> +#endif

This unfortunately is causing compilation issues.  <openssl/ssl.h> wants
to include <ctype.h> and gets upset by seeing our isalpha() and friends
that are defined indirectly in "cache.h" expanded.

In <ctype.h> (on FC9), isCHARACTERISTC() are defined like this:

	#define __exctype(name) extern int name (int) __THROW
	...
        __exctype (isalnum);
        __exctype (isalpha);

but we have been using our own locale agonistic and signed-chars safe
macros defined in git-compat-util.h.  Including <ctype.h> breaks at the
syntax level, but more importantly if the system <ctype.h> redefines
isalpha() and friends as macro, then it would break _our_ code that expect
these macros are the sane_ctype[] based ones we have.

A hack like the one attached below would make it "work" but it is too
ugly.  Probably we need to bite the bullet and rename ours not to
collide, so that external library headers can safely include <ctype.h>.

Sigh...

diff --git a/git-compat-util.h b/git-compat-util.h
index 8c7e114..0af6406 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -302,6 +302,7 @@ static inline int has_extension(const char *filename, const char *ext)
 }
 
 /* Sane ctype - no locale, and works with signed chars */
+#define _CTYPE_H
 #undef isspace
 #undef isdigit
 #undef isalpha

Re: [PATCH 2/4] git-imap-send: Add support for SSL.

[Linus Torvalds]

On Wed, 9 Jul 2008, Junio C Hamano wrote:
> 
> A hack like the one attached below would make it "work" but it is too
> ugly.  Probably we need to bite the bullet and rename ours not to
> collide, so that external library headers can safely include <ctype.h>.

We should be able to safely include <ctype.h> as-is.

It should just happen _before_ including git-compat-util.h. That's why 
git-compat-util.h does all those #undef's - exactly because ctype.h does 
get included on various systems by various header files when 
git-compat-util.h includes all those other files.

So the problem with Robert's patch is that it happens after "cache.h". The 
system headers should be included at the top of git-compat-util.h, or 
before it.

		Linus

Re: [PATCH 2/4] git-imap-send: Add support for SSL.

[Junio C Hamano]

Linus Torvalds <torvalds@linux-foundation.org> writes:

> We should be able to safely include <ctype.h> as-is.
>
> It should just happen _before_ including git-compat-util.h. That's why 
> git-compat-util.h does all those #undef's - exactly because ctype.h does 
> get included on various systems by various header files when 
> git-compat-util.h includes all those other files.

Eh, that's true,... although I am not convinced we should keep doing this.

With and without NO_OPENSSL, on top of Robert's patch, these seem to pass
compile test.

--

 git-compat-util.h |    5 +++++
 imap-send.c       |    4 ----
 2 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/git-compat-util.h b/git-compat-util.h
index 545df59..65c4671 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -99,6 +99,11 @@
 #include <iconv.h>
 #endif
 
+#ifndef NO_OPENSSL
+#include <openssl/ssl.h>
+#include <openssl/err.h>
+#endif
+
 /* On most systems <limits.h> would have given us this, but
  * not on some systems (e.g. GNU/Hurd).
  */
diff --git a/imap-send.c b/imap-send.c
index 9dc5d08..8026334 100644
--- a/imap-send.c
+++ b/imap-send.c
@@ -25,9 +25,6 @@
 #include "cache.h"
 #ifdef NO_OPENSSL
 typedef void *SSL;
-#else
-# include <openssl/ssl.h>
-# include <openssl/err.h>
 #endif
 
 typedef struct store_conf {

Re: [PATCH 2/4] git-imap-send: Add support for SSL.

[Mike Hommey]

On Wed, Jul 09, 2008 at 10:29:00PM +0100, Robert Shearman wrote:
> From: Robert Shearman <rob@codeweavers.com>
> 
> Allow SSL to be used when a imaps:// URL is used for the host name.
> 
> Also, automatically use TLS when not using imaps:// by using the IMAP STARTTLS command, if the server supports it.
> 
> Tested with Courier and Gimap IMAP servers.

Please do an alternative implementation using GnuTLS. Why ? Because curl
can be built against openssl or gnutls, and it would be sad if git
ended up depending on both indirectly.

Mike

Re: [PATCH 2/4] git-imap-send: Add support for SSL.

[Jeff King]

On Wed, Jul 09, 2008 at 10:24:27PM +0100, Rob Shearman wrote:

> >>  [imap]
> >> +    sslverify = false
> > [...]
> >> +     else if (!strcmp( "ssl_verify", key ))
> >
> > The example and the code disagree on the name of the
> > sslverify/ssl_verify option.
> 
> I wouldn't exactly call it "disagree". The config variable is limited
> by not allowing underscores, whereas the C language does allow them.

I think his point is that the example says "sslverify" but the code is
looking for the config variable "ssl_verify". So that config won't work.

However as you mention, underscore isn't allowed, so your strcmp line is
totally bogus anyway, but you silently fix it in your 3/4 "style" fix.

-Peff