dovecot-2.1: lib-ssl-iostream: Code cleanups, fixes, asserts and...

dovecot at dovecot.org dovecot at dovecot.org
Tue Sep 6 13:41:16 EEST 2011


details:   http://hg.dovecot.org/dovecot-2.1/rev/6a3f3a5ad9a5
changeset: 13403:6a3f3a5ad9a5
user:      Timo Sirainen <tss at iki.fi>
date:      Tue Sep 06 13:40:50 2011 +0300
description:
lib-ssl-iostream: Code cleanups, fixes, asserts and comments.

diffstat:

 src/lib-ssl-iostream/iostream-openssl.c |  54 ++++++++++++++++--
 src/lib-ssl-iostream/iostream-openssl.h |  12 ++++
 src/lib-ssl-iostream/istream-openssl.c  |  14 ++--
 src/lib-ssl-iostream/ostream-openssl.c  |  96 ++++++++++++++++++++------------
 4 files changed, 128 insertions(+), 48 deletions(-)

diffs (truncated from 375 to 300 lines):

diff -r 44bbbb238f2a -r 6a3f3a5ad9a5 src/lib-ssl-iostream/iostream-openssl.c
--- 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);
diff -r 44bbbb238f2a -r 6a3f3a5ad9a5 src/lib-ssl-iostream/iostream-openssl.h
--- 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);
 
diff -r 44bbbb238f2a -r 6a3f3a5ad9a5 src/lib-ssl-iostream/istream-openssl.c
--- 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;
diff -r 44bbbb238f2a -r 6a3f3a5ad9a5 src/lib-ssl-iostream/ostream-openssl.c
--- 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;


More information about the dovecot-cvs mailing list