Mercurial > dovecot > core-2.2

changeset 13403:6a3f3a5ad9a5

Find changesets by keywords (author, files, the commit message), revision number or hash, or revset expression.
lib-ssl-iostream: Code cleanups, fixes, asserts and comments.
author Timo Sirainen <tss@iki.fi>
date Tue, 06 Sep 2011 13:40:50 +0300
parents 44bbbb238f2a
children c3dc563c9800
files src/lib-ssl-iostream/iostream-openssl.c src/lib-ssl-iostream/iostream-openssl.h src/lib-ssl-iostream/istream-openssl.c src/lib-ssl-iostream/ostream-openssl.c
diffstat 4 files changed, 128 insertions(+), 48 deletions(-) [+]
line wrap: on
line diff
--- a/src/lib-ssl-iostream/iostream-openssl.c	Mon Sep 05 14:23:11 2011 +0300
+++ b/src/lib-ssl-iostream/iostream-openssl.c	Tue Sep 06 13:40:50 2011 +0300
@@ -178,6 +178,11 @@
 		return -1;
 	}
 
+	/* BIO pairs use default buffer sizes (17 kB in OpenSSL 0.9.8e).
+	   Each of the BIOs have one "write buffer". BIO_write() copies data
+	   to them, while BIO_read() reads from the other BIO's write buffer
+	   into the given buffer. The bio_int is used by OpenSSL and bio_ext
+	   is used by this library. */
 	if (BIO_new_bio_pair(&bio_int, 0, &bio_ext, 0) != 1) {
 		i_error("BIO_new_bio_pair() failed: %s", ssl_iostream_error());
 		SSL_free(ssl);
@@ -192,6 +197,7 @@
 	ssl_io->plain_input = *input;
 	ssl_io->plain_output = *output;
 	ssl_io->source = i_strdup(source);
+	/* bio_int will be freed by SSL_free() */
 	SSL_set_bio(ssl_io->ssl, bio_int, bio_int);
         SSL_set_ex_data(ssl_io->ssl, dovecot_ssl_extdata_index, ssl_io);
 
@@ -244,11 +250,14 @@
 {
 	size_t bytes, max_bytes;
 	ssize_t sent;
-	unsigned char buffer[1024];
+	unsigned char buffer[IO_BLOCK_SIZE];
 	bool bytes_sent = FALSE;
 	int ret;
 
+	o_stream_cork(ssl_io->plain_output);
 	while ((bytes = BIO_ctrl_pending(ssl_io->bio_ext)) > 0) {
+		/* bytes contains how many SSL encrypted bytes we should be
+		   sending out */
 		max_bytes = o_stream_get_buffer_avail_size(ssl_io->plain_output);
 		if (bytes > max_bytes) {
 			if (max_bytes == 0) {
@@ -260,9 +269,14 @@
 		if (bytes > sizeof(buffer))
 			bytes = sizeof(buffer);
 
+		/* BIO_read() is guaranteed to return all the bytes that
+		   BIO_ctrl_pending() returned */
 		ret = BIO_read(ssl_io->bio_ext, buffer, bytes);
 		i_assert(ret == (int)bytes);
 
+		/* we limited number of read bytes to plain_output's
+		   available size. this send() is guaranteed to either
+		   fully succeed or completely fail due to some error. */
 		sent = o_stream_send(ssl_io->plain_output, buffer, bytes);
 		if (sent < 0) {
 			i_assert(ssl_io->plain_output->stream_errno != 0);
@@ -273,22 +287,27 @@
 		i_assert(sent == (ssize_t)bytes);
 		bytes_sent = TRUE;
 	}
+	o_stream_uncork(ssl_io->plain_output);
 	return bytes_sent;
 }
 
 static bool ssl_iostream_bio_input(struct ssl_iostream *ssl_io)
 {
 	const unsigned char *data;
-	size_t size;
+	size_t bytes, size;
 	bool bytes_read = FALSE;
 	int ret;
 
-	while (BIO_ctrl_get_read_request(ssl_io->bio_ext) > 0) {
+	while ((bytes = BIO_ctrl_get_write_guarantee(ssl_io->bio_ext)) > 0) {
+		/* bytes contains how many bytes we can write to bio_ext */
 		(void)i_stream_read_data(ssl_io->plain_input, &data, &size, 0);
 		if (size == 0) {
 			/* wait for more input */
 			break;
 		}
+		if (size > bytes)
+			size = bytes;
+
 		ret = BIO_write(ssl_io->bio_ext, data, size);
 		i_assert(ret == (ssize_t)size);
 
@@ -303,11 +322,29 @@
 	bool ret;
 
 	ret = ssl_iostream_bio_output(ssl_io);
-	if (ssl_iostream_bio_input(ssl_io))
+	if (ssl_iostream_bio_input(ssl_io)) {
+		if (ssl_io->ostream_flush_waiting_input) {
+			ssl_io->ostream_flush_waiting_input = FALSE;
+			o_stream_set_flush_pending(ssl_io->plain_output, TRUE);
+		}
+		ssl_io->want_read = FALSE;
 		ret = TRUE;
+	}
 	return ret;
 }
 
+int ssl_iostream_more(struct ssl_iostream *ssl_io)
+{
+	int ret;
+
+	if (!ssl_io->handshaked) {
+		if ((ret = ssl_iostream_handshake(ssl_io)) <= 0)
+			return ret;
+	}
+	(void)ssl_iostream_bio_sync(ssl_io);
+	return 1;
+}
+
 int ssl_iostream_handle_error(struct ssl_iostream *ssl_io, int ret,
 			      const char *func_name)
 {
@@ -317,12 +354,16 @@
 	err = SSL_get_error(ssl_io->ssl, ret);
 	switch (err) {
 	case SSL_ERROR_WANT_WRITE:
-		if (!ssl_iostream_bio_sync(ssl_io))
+		if (!ssl_iostream_bio_sync(ssl_io)) {
+			i_panic("SSL ostream buffer size not unlimited");
 			return 0;
+		}
 		return 1;
 	case SSL_ERROR_WANT_READ:
-		if (!ssl_iostream_bio_sync(ssl_io))
+		if (!ssl_iostream_bio_sync(ssl_io)) {
+			ssl_io->want_read = TRUE;
 			return 0;
+		}
 		return 1;
 	case SSL_ERROR_SYSCALL:
 		/* eat up the error queue */
@@ -384,6 +425,7 @@
 				return ret;
 		}
 	}
+	/* handshake finished */
 	(void)ssl_iostream_bio_sync(ssl_io);
 
 	i_free_and_null(ssl_io->last_error);
--- a/src/lib-ssl-iostream/iostream-openssl.h	Mon Sep 05 14:23:11 2011 +0300
+++ b/src/lib-ssl-iostream/iostream-openssl.h	Tue Sep 06 13:40:50 2011 +0300
@@ -45,6 +45,8 @@
 	unsigned int handshaked:1;
 	unsigned int cert_received:1;
 	unsigned int cert_broken:1;
+	unsigned int want_read:1;
+	unsigned int ostream_flush_waiting_input:1;
 };
 
 extern int dovecot_ssl_extdata_index;
@@ -57,7 +59,17 @@
 			  const char *key_source, EVP_PKEY **pkey_r);
 const char *ssl_iostream_get_use_certificate_error(const char *cert);
 
+/* Sync plain_input/plain_output streams with BIOs. Returns TRUE if at least
+   one byte was read/written. */
 bool ssl_iostream_bio_sync(struct ssl_iostream *ssl_io);
+/* Call when there's more data available in plain_input/plain_output.
+   Returns 1 if it's ok to continue with SSL_read/SSL_write, 0 if not
+   (still handshaking), -1 if error occurred. */
+int ssl_iostream_more(struct ssl_iostream *ssl_io);
+
+/* Returns 1 if the operation should be retried (we read/wrote more data),
+   0 if the operation should retried later once more data has been
+   read/written, -1 if a fatal error occurred (errno is set). */
 int ssl_iostream_handle_error(struct ssl_iostream *ssl_io, int ret,
 			      const char *func_name);
 
--- a/src/lib-ssl-iostream/istream-openssl.c	Mon Sep 05 14:23:11 2011 +0300
+++ b/src/lib-ssl-iostream/istream-openssl.c	Tue Sep 06 13:40:50 2011 +0300
@@ -27,12 +27,13 @@
 		stream->istream.eof = TRUE;
 		return -1;
 	}
-	if (!sstream->ssl_io->handshaked) {
-		if ((ret = ssl_iostream_handshake(sstream->ssl_io)) <= 0) {
-			if (ret < 0)
-				stream->istream.stream_errno = errno;
-			return ret;
+	ret = ssl_iostream_more(sstream->ssl_io);
+	if (ret <= 0) {
+		if (ret < 0) {
+			/* handshake failed */
+			stream->istream.stream_errno = errno;
 		}
+		return ret;
 	}
 
 	if (!i_stream_get_buffer_space(stream, 1, &size))
@@ -40,6 +41,7 @@
 
 	while ((ret = SSL_read(sstream->ssl_io->ssl,
 			       stream->w_buffer + stream->pos, size)) <= 0) {
+		/* failed to read anything */
 		ret = ssl_iostream_handle_error(sstream->ssl_io, ret,
 						"SSL_read");
 		if (ret <= 0) {
@@ -50,7 +52,7 @@
 			}
 			return ret;
 		}
-		(void)ssl_iostream_bio_sync(sstream->ssl_io);
+		/* we did some BIO I/O, try reading again */
 	}
 	stream->pos += ret;
 	return ret;
--- a/src/lib-ssl-iostream/ostream-openssl.c	Mon Sep 05 14:23:11 2011 +0300
+++ b/src/lib-ssl-iostream/ostream-openssl.c	Tue Sep 06 13:40:50 2011 +0300
@@ -66,22 +66,25 @@
 static int o_stream_ssl_flush_buffer(struct ssl_ostream *sstream)
 {
 	size_t pos = 0;
-	int ret;
+	int ret = 1;
 
 	while (pos < sstream->buffer->used) {
+		/* we're writing plaintext data to OpenSSL, which it encrypts
+		   and writes to bio_int's buffer. ssl_iostream_bio_sync()
+		   reads it from there and adds to plain_output stream. */
 		ret = SSL_write(sstream->ssl_io->ssl,
 				CONST_PTR_OFFSET(sstream->buffer->data, pos),
 				sstream->buffer->used - pos);
 		if (ret <= 0) {
 			ret = ssl_iostream_handle_error(sstream->ssl_io, ret,
 							"SSL_write");
-			if (ret <= 0) {
-				if (ret < 0) {
-					sstream->ostream.ostream.stream_errno =
-						errno;
-				}
-				buffer_delete(sstream->buffer, 0, pos);
-				return ret;
+			if (ret < 0) {
+				sstream->ostream.ostream.stream_errno = errno;
+				break;
+			}
+			if (ret == 0) {
+				/* bio_int's buffer is full */
+				break;
 			}
 		} else {
 			pos += ret;
@@ -89,7 +92,7 @@
 		}
 	}
 	buffer_delete(sstream->buffer, 0, pos);
-	return 1;
+	return ret <= 0 ? ret : 1;
 }
 
 static int o_stream_ssl_flush(struct ostream_private *stream)
@@ -97,34 +100,33 @@
 	struct ssl_ostream *sstream = (struct ssl_ostream *)stream;
 	int ret;
 
-	if (!sstream->ssl_io->handshaked) {
-		if ((ret = ssl_iostream_handshake(sstream->ssl_io)) <= 0) {
-			if (ret < 0)
-				stream->ostream.stream_errno = errno;
-			return ret;
-		}
+	if ((ret = ssl_iostream_more(sstream->ssl_io)) < 0) {
+		/* handshake failed */
+		stream->ostream.stream_errno = errno;
+	} else if (ret > 0 && sstream->buffer != NULL &&
+		   sstream->buffer->used > 0) {
+		/* we can try to send some of our buffered data */
+		ret = o_stream_ssl_flush_buffer(sstream);
 	}
 
-	if (sstream->buffer != NULL && sstream->buffer->used > 0) {
-		if ((ret = o_stream_ssl_flush_buffer(sstream)) <= 0)
-			return ret;
+	if (ret == 0 && sstream->ssl_io->want_read) {
+		/* we need to read more data until we can continue. */
+		sstream->ssl_io->ostream_flush_waiting_input = TRUE;
+		ret = 1;
 	}
-	return 1;
+	return ret;
 }
 
 static ssize_t
-o_stream_ssl_sendv(struct ostream_private *stream,
-		   const struct const_iovec *iov, unsigned int iov_count)
+o_stream_ssl_sendv_try(struct ssl_ostream *sstream,
+		       const struct const_iovec *iov, unsigned int iov_count,
+		       size_t *bytes_sent_r)
 {
-	struct ssl_ostream *sstream = (struct ssl_ostream *)stream;
 	unsigned int i;
-	size_t bytes_sent = 0;
 	size_t pos;
-	int ret = 0;
+	ssize_t ret;
 
-	if (o_stream_flush(&stream->ostream) <= 0)
-		return o_stream_ssl_buffer(sstream, iov, iov_count, 0);
-
+	*bytes_sent_r = 0;
 	for (i = 0, pos = 0; i < iov_count; ) {
 		ret = SSL_write(sstream->ssl_io->ssl,
 				CONST_PTR_OFFSET(iov[i].iov_base, pos),
@@ -132,13 +134,14 @@
 		if (ret <= 0) {
 			ret = ssl_iostream_handle_error(sstream->ssl_io, ret,
 							"SSL_write");
-			if (ret <= 0) {
-				if (ret < 0)
-					stream->ostream.stream_errno = errno;
+			if (ret < 0) {
+				sstream->ostream.ostream.stream_errno = errno;
 				break;
 			}
+			if (ret == 0)
+				break;
 		} else {
-			bytes_sent += ret;
+			*bytes_sent_r += ret;
 			if ((size_t)ret < iov[i].iov_len)
 				pos += ret;
 			else {
@@ -148,20 +151,41 @@
 			(void)ssl_iostream_bio_sync(sstream->ssl_io);
 		}
 	}
+	return ret <= 0 ? ret : 1;
+}
+
+static ssize_t
+o_stream_ssl_sendv(struct ostream_private *stream,
+		   const struct const_iovec *iov, unsigned int iov_count)
+{
+	struct ssl_ostream *sstream = (struct ssl_ostream *)stream;
+	size_t bytes_sent = 0;
+	int ret;
+
+	if (!sstream->ssl_io->handshaked)
+		ret = 0;
+	else {
+		ret = o_stream_ssl_sendv_try(sstream, iov, iov_count,
+					     &bytes_sent);
+	}
+
 	bytes_sent = o_stream_ssl_buffer(sstream, iov, iov_count, bytes_sent);
 	return bytes_sent != 0 ? (ssize_t)bytes_sent : ret;
 }
 
 static int plain_flush_callback(struct ssl_ostream *sstream)
 {
-	int ret;
+	int ret, ret2;
 
+	/* try to actually flush the pending data */
 	if ((ret = o_stream_flush(sstream->ssl_io->plain_output)) < 0)
-		return 1;
+		return -1;
 
-	if (ret > 0)
-		return o_stream_flush(&sstream->ostream.ostream);
-	return 1;
+	/* we may be able to copy more data, try it */
+	ret2 = o_stream_flush(&sstream->ostream.ostream);
+	if (ret2 < 0)
+		return -1;
+	return ret > 0 && ret2 > 0 ? 1 : 0;
 }
 
 struct ostream *o_stream_create_ssl(struct ssl_iostream *ssl_io)