[PATCH] chunk: Update protocol to support in-band SSL negotiation

Jeff Garzik <[email protected]>
Newsgroups org.kernel.vger.hail-devel
Message-ID <[email protected]> 
commit 0df0e18e7fdf2674e4a22f0ca8e57a157dad6ca9
Author: Jeff Garzik <[email protected]>
Date:   Fri Mar 5 10:04:00 2010 -0500

    Protocol: replace SSL/no-SSL split ports with in-band SSL negotiation
    
    Initiating SSL immediately upon connection, based on incoming TCP
    connection port, is a relic of poor HTTP design and should not have
    been carried over to chunk protocol.  Modern protocols (SMTP, IMAP,
    etc.) all support binding to a single port, and then negotiating
    SSL/TLS based on a command from the client.
    
    Change chunk protocol such that clients must issue a CHO_START_TLS
    command at the beginning of the TCP connection, if they wish
    encryption.
    
    Signed-off-by: Jeff Garzik <[email protected]>

diff --git a/doc/setup.txt b/doc/setup.txt
index a5e7c66..5ef7225 100644
--- a/doc/setup.txt
+++ b/doc/setup.txt
@@ -27,14 +27,6 @@ _cld._udp.phx2.ex.com has SRV record 10 50 8081 maika.phx2.ex.com.
 		<Port>8082</Port>
 	</Listen>
 
-   You can also set the interface and the encryption:
-
-	<Listen>
-		<Node>192.168.1.24</Node>
-		<Port>18082</Port>
-		<Encrypt>true</Encrypt>
-	</Listen>
-
    For clouds and their many cheap nodes with one Ethernet it usually is
    not a great idea to specify interfaces, since they often use IPv6 or
    acquire IP addresses from DHCP. So, just specify a port.
diff --git a/include/chunk_msg.h b/include/chunk_msg.h
index 154201d..a25fcb5 100644
--- a/include/chunk_msg.h
+++ b/include/chunk_msg.h
@@ -32,16 +32,22 @@ enum {
 };
 
 enum chunksrv_ops {
-	CHO_NOP			= 0,
-	CHO_GET			= 1,
-	CHO_GET_META		= 2,
-	CHO_PUT			= 3,
-	CHO_DEL			= 4,
-	CHO_LIST		= 5,
-	CHO_LOGIN		= 6,
-	CHO_TABLE_OPEN		= 7,
-	CHO_CHECK_START		= 8,
-	CHO_CHECK_STATUS	= 9,
+	CHO_NOP			= 0,	/* No-op (ping server) */
+	CHO_GET			= 1,	/* GET object */
+	CHO_GET_META		= 2,	/* GET object metadata */
+	CHO_PUT			= 3,	/* PUT object */
+	CHO_DEL			= 4,	/* Delete object */
+	CHO_LIST		= 5,	/* List objects */
+	CHO_LOGIN		= 6,	/* Login as user */
+	CHO_TABLE_OPEN		= 7,	/* Open table */
+	CHO_CHECK_START		= 8,	/* Begin self-check */
+	CHO_CHECK_STATUS	= 9,	/* Query self-check status */
+
+	/* START-TLS is special.  It MUST be the first request of a TCP
+	 * cxn, and no chunkd-specific response is returned.  The SSL
+	 * functions' success/failure is sufficient indication.
+	 */
+	CHO_START_TLS		= 10,	/* Encrypt all subsequent msgs */
 };
 
 enum chunk_errcode {
diff --git a/lib/chunkdc.c b/lib/chunkdc.c
index e9e0e0f..9c1e967 100644
--- a/lib/chunkdc.c
+++ b/lib/chunkdc.c
@@ -191,6 +191,27 @@ static bool stc_login(struct st_client *stc)
 	return true;
 }
 
+static bool stc_start_tls_cmd(struct st_client *stc)
+{
+	struct chunksrv_req *req = (struct chunksrv_req *) stc->req_buf;
+
+	if (stc->verbose)
+		fprintf(stderr, "libstc: START-TLS\n");
+
+	/* initialize request */
+	req_init(stc, req);
+	req->op = CHO_START_TLS;
+	strcpy(req->sig, "START-TLS");
+
+	/* write request */
+	if (!net_write(stc, req, req_len(req)))
+		return false;
+
+	/* no response - SSL negotiation begins immediately */
+
+	return true;
+}
+
 struct st_client *stc_new(const char *service_host, int port,
 			  const char *user, const char *secret_key,
 			  bool use_ssl)
@@ -253,6 +274,9 @@ struct st_client *stc_new(const char *service_host, int port,
 		goto err_out;
 
 	if (use_ssl) {
+		if (!stc_start_tls_cmd(stc))
+			goto err_out;
+
 		stc->ssl_ctx = SSL_CTX_new(TLSv1_client_method());
 		if (!stc->ssl_ctx)
 			goto err_out;
@@ -266,7 +290,8 @@ struct st_client *stc_new(const char *service_host, int port,
 		if (!SSL_set_fd(stc->ssl, stc->fd))
 			goto err_out_ssl;
 
-		if (SSL_connect(stc->ssl) <= 0)
+		rc = SSL_connect(stc->ssl);
+		if (rc <= 0)
 			goto err_out_ssl;
 	}
 
@@ -276,9 +301,15 @@ struct st_client *stc_new(const char *service_host, int port,
 	return stc;
 
 err_out_ssl:
-	SSL_free(stc->ssl);
+	if (stc->ssl) {
+		SSL_free(stc->ssl);
+		stc->ssl = NULL;
+	}
 err_out_ctx:
-	SSL_CTX_free(stc->ssl_ctx);
+	if (stc->ssl_ctx) {
+		SSL_CTX_free(stc->ssl_ctx);
+		stc->ssl_ctx = NULL;
+	}
 err_out:
 	close(stc->fd);
 	stc_free(stc);
diff --git a/server/chunkd.h b/server/chunkd.h
index 42bcb21..cd83f7b 100644
--- a/server/chunkd.h
+++ b/server/chunkd.h
@@ -102,6 +102,7 @@ struct client {
 	SSL			*ssl;
 	bool			read_want_write;
 	bool			write_want_read;
+	bool			first_req;
 
 	struct list_head	write_q;	/* list of async writes */
 	bool			writing;
@@ -150,7 +151,6 @@ struct listen_cfg {
 	char			*node;
 	char			*port;
 	char			*port_file;
-	bool			encrypt;
 };
 
 struct geo {
diff --git a/server/cldu.c b/server/cldu.c
index 9c27138..dbcf11d 100644
--- a/server/cldu.c
+++ b/server/cldu.c
@@ -540,11 +540,9 @@ static int cldu_make_ffile(char **ret, struct cld_session *sp)
 
 		rc = asprintf(&str,
 			" <Socket>\r\n"
-			"  <Type>%s</Type>\r\n"
 			"  <Host>%s</Host>\r\n"
 			"  <Port>%s</Port>\r\n"
 			" </Socket>\r\n",
-			cfg->encrypt ? "chunk-ssl" : "chunk",
 			host,
 			cfg->port);
 		if (rc == -1) {
diff --git a/server/config.c b/server/config.c
index 69b5e9b..0422239 100644
--- a/server/config.c
+++ b/server/config.c
@@ -398,16 +398,6 @@ static void cfg_elm_end (GMarkupParseContext *context,
 		}
 	}
 
-	else if (cc->in_listen && cc->text &&
-		 !strcmp(element_name, "Encrypt")) {
-		if (!strcasecmp(cc->text, "yes") ||
-		    !strcasecmp(cc->text, "true"))
-			cc->tmp_listen.encrypt = true;
-
-		free(cc->text);
-		cc->text = NULL;
-	}
-
 	else if (!strcmp(element_name, "Group") && cc->text) {
 		free(chunkd_srv.group);
 		chunkd_srv.group = cc->text;
diff --git a/server/server.c b/server/server.c
index 0d83311..4d9a09e 100644
--- a/server/server.c
+++ b/server/server.c
@@ -357,7 +357,7 @@ static void cli_free(struct client *cli)
 	free(cli);
 }
 
-static struct client *cli_alloc(bool use_ssl)
+static struct client *cli_alloc(void)
 {
 	struct client *cli;
 
@@ -366,18 +366,10 @@ static struct client *cli_alloc(bool use_ssl)
 	if (!cli)
 		return NULL;
 
-	if (use_ssl) {
-		cli->ssl = SSL_new(ssl_ctx);
-		if (!cli->ssl) {
-			applog(LOG_ERR, "SSL_new failed");
-			free(cli);
-			return NULL;
-		}
-	}
-
 	cli->state = evt_read_fixed;
 	INIT_LIST_HEAD(&cli->write_q);
 	cli->req_ptr = &cli->creq;
+	cli->first_req = true;
 
 	return cli;
 }
@@ -1038,6 +1030,7 @@ static const char *op2str(enum chunksrv_ops op)
 	case CHO_TABLE_OPEN:	return "CHO_TABLE_OPEN";
 	case CHO_CHECK_START:	return "CHO_CHECK_START";
 	case CHO_CHECK_STATUS:	return "CHO_CHECK_STATUS";
+	case CHO_START_TLS:	return "CHO_START_TLS";
 
 	default:
 		return "BUG/UNKNOWN!";
@@ -1082,7 +1075,8 @@ static bool cli_evt_exec_req(struct client *cli, unsigned int events)
 
 	cli->state = evt_recycle;
 
-	if (G_UNLIKELY((!logged_in) && (req->op != CHO_LOGIN))) {
+	if (G_UNLIKELY((!logged_in) && (req->op != CHO_LOGIN) &&
+		       (req->op != CHO_START_TLS))) {
 		cli->state = evt_dispose;
 		return true;
 	}
@@ -1142,11 +1136,37 @@ static bool cli_evt_exec_req(struct client *cli, unsigned int events)
 	case CHO_CHECK_STATUS:
 		rcb = chk_status(cli);
 		break;
+	case CHO_START_TLS:
+		if (!cli->first_req) {
+			cli->state = evt_dispose;
+			rcb = true;
+		} else {
+			cli->ssl = SSL_new(ssl_ctx);
+			if (!cli->ssl) {
+				applog(LOG_ERR, "SSL_new failed");
+				cli->state = evt_dispose;
+				rcb = true;
+				break;
+			}
+
+			if (!SSL_set_fd(cli->ssl, cli->fd)) {
+				applog(LOG_ERR, "SSL_set_fd failed");
+				cli->state = evt_dispose;
+				rcb = true;
+				break;
+			}
+
+			cli->state = evt_ssl_accept;
+			rcb = true;
+		}
+		break;
 	default:
 		rcb = cli_err(cli, che_InvalidURI, true);
 		break;
 	}
 
+	cli->first_req = false;
+
 out:
 	return rcb;
 
@@ -1220,7 +1240,7 @@ static bool cli_evt_ssl_accept(struct client *cli, unsigned int events)
 
 	rc = SSL_accept(cli->ssl);
 	if (rc > 0) {
-		cli->state = evt_read_fixed;
+		cli->state = evt_recycle;
 		return true;
 	}
 
@@ -1236,6 +1256,8 @@ static bool cli_evt_ssl_accept(struct client *cli, unsigned int events)
 		return false;
 	}
 
+	applog(LOG_ERR, "SSL_accept returned %d", rc);
+
 out:
 	cli->state = evt_dispose;
 	return true;
@@ -1294,10 +1316,10 @@ static bool tcp_srv_event(int fd, short events, void *userdata)
 	socklen_t addrlen = sizeof(struct sockaddr_in6);
 	struct client *cli;
 	char host[64];
-	int rc, on = 1;
+	int on = 1;
 	struct server_poll *sp;
 
-	cli = cli_alloc(sock->cfg->encrypt);
+	cli = cli_alloc();
 	if (!cli) {
 		applog(LOG_ERR, "out of memory");
 		return false;	/* end main loop; terminate server */
@@ -1321,32 +1343,6 @@ static bool tcp_srv_event(int fd, short events, void *userdata)
 		applog(LOG_WARNING, "TCP_NODELAY failed: %s",
 		       strerror(errno));
 
-	if (sock->cfg->encrypt) {
-		if (!SSL_set_fd(cli->ssl, cli->fd))
-			goto err_out_fd;
-
-		rc = SSL_accept(cli->ssl);
-		if (rc <= 0) {
-			rc = SSL_get_error(cli->ssl, rc);
-			if (rc == SSL_ERROR_WANT_READ)
-				cli->state = evt_ssl_accept;
-			else if (rc == SSL_ERROR_WANT_WRITE) {
-				cli->state = evt_ssl_accept;
-				cli->read_want_write = true;
-			}
-			else {
-				unsigned long e = ERR_get_error();
-				char estr[121] = "(none?)";
-
-				if (e)
-					ERR_error_string(e, estr);
-				applog(LOG_WARNING, "%s SSL error %s",
-				       cli->addr_host, estr);
-				goto err_out_fd;
-			}
-		}
-	}
-
 	sp = calloc(1, sizeof(*sp));
 	if (!sp)
 		goto err_out_fd;
@@ -1356,11 +1352,6 @@ static bool tcp_srv_event(int fd, short events, void *userdata)
 	sp->cb = tcp_cli_event;
 	sp->userdata = cli;
 
-	if (cli->read_want_write) {
-		cli->writing = true;
-		sp->events |= POLLOUT;
-	}
-
 	g_hash_table_insert(chunkd_srv.fd_info, GINT_TO_POINTER(cli->fd), sp);
 
 	/* pretty-print incoming cxn info */
diff --git a/test/auth.c b/test/auth.c
index 78b1f8b..e4bfaf0 100644
--- a/test/auth.c
+++ b/test/auth.c
@@ -43,7 +43,7 @@ static void test(bool do_encrypt)
 	size_t len = 0;
 	void *mem;
 
-	port = stc_readport(do_encrypt ? TEST_PORTFILE_SSL : TEST_PORTFILE);
+	port = stc_readport(TEST_PORTFILE);
 	OK(port > 0);
 
 	stc1 = stc_new(TEST_HOST, port, TEST_USER, TEST_USER_KEY, do_encrypt);
diff --git a/test/basic-object.c b/test/basic-object.c
index c4d803a..92ec8c8 100644
--- a/test/basic-object.c
+++ b/test/basic-object.c
@@ -41,7 +41,7 @@ static void test(bool do_encrypt)
 	size_t len = 0;
 	void *mem;
 
-	port = stc_readport(do_encrypt ? TEST_PORTFILE_SSL : TEST_PORTFILE);
+	port = stc_readport(TEST_PORTFILE);
 	OK(port > 0);
 
 	stc = stc_new(TEST_HOST, port, TEST_USER, TEST_USER_KEY, do_encrypt);
diff --git a/test/it-works.c b/test/it-works.c
index 597c58b..a3c01ee 100644
--- a/test/it-works.c
+++ b/test/it-works.c
@@ -35,7 +35,7 @@ static void test(bool ssl)
 	int port;
 	bool rcb;
 
-	port = stc_readport(ssl ? TEST_PORTFILE_SSL : TEST_PORTFILE);
+	port = stc_readport(TEST_PORTFILE);
 	OK(port > 0);
 
 	stc = stc_new(TEST_HOST, port, TEST_USER, TEST_USER_KEY, ssl);
diff --git a/test/large-object.c b/test/large-object.c
index 5188aad..d1a031e 100644
--- a/test/large-object.c
+++ b/test/large-object.c
@@ -108,7 +108,7 @@ static void test(bool do_encrypt)
 
 	memset(data, 0xdeadbeef, sizeof(data));
 
-	port = stc_readport(do_encrypt ? TEST_PORTFILE_SSL : TEST_PORTFILE);
+	port = stc_readport(TEST_PORTFILE);
 	OK(port > 0);
 
 	stc = stc_new(TEST_HOST, port, TEST_USER, TEST_USER_KEY, do_encrypt);
diff --git a/test/lotsa-objects.c b/test/lotsa-objects.c
index 048c1c3..d7dd95c 100644
--- a/test/lotsa-objects.c
+++ b/test/lotsa-objects.c
@@ -47,7 +47,7 @@ static void test(int n_objects, bool do_encrypt)
 	char *k;
 	struct timeval ta, tb;
 
-	port = stc_readport(do_encrypt ? TEST_PORTFILE_SSL : TEST_PORTFILE);
+	port = stc_readport(TEST_PORTFILE);
 	OK(port > 0);
 
 	stc = stc_new(TEST_HOST, port, TEST_USER, TEST_USER_KEY, do_encrypt);
diff --git a/test/nop.c b/test/nop.c
index 526b8c0..bd89a7b 100644
--- a/test/nop.c
+++ b/test/nop.c
@@ -42,7 +42,7 @@ static void test(int n_nops, bool do_encrypt)
 	int i;
 	struct timeval ta, tb;
 
-	port = stc_readport(do_encrypt ? TEST_PORTFILE_SSL : TEST_PORTFILE);
+	port = stc_readport(TEST_PORTFILE);
 	OK(port > 0);
 
 	stc = stc_new(TEST_HOST, port, TEST_USER, TEST_USER_KEY, do_encrypt);
diff --git a/test/server-test.cfg b/test/server-test.cfg
index d98fca4..29b69e2 100644
--- a/test/server-test.cfg
+++ b/test/server-test.cfg
@@ -10,12 +10,6 @@
 	<PortFile>chunkd.port</PortFile>
 </Listen>
 
-<Listen>
-	<Port>auto</Port>
-	<PortFile>chunkd-ssl.port</PortFile>
-	<Encrypt>true</Encrypt>
-</Listen>
-
 <PID>chunkd.pid</PID>
 
 <Path>data/chunk</Path>
diff --git a/test/test.h b/test/test.h
index 75aa250..cc4111f 100644
--- a/test/test.h
+++ b/test/test.h
@@ -36,7 +36,6 @@
 
 #define TEST_PORTFILE_CLD	"cld.port"
 #define TEST_PORTFILE		"chunkd.port"
-#define TEST_PORTFILE_SSL	"chunkd-ssl.port"
 
 #define TEST_CHUNKD_CFG		"server-test.cfg"
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.