Merge branch 'pks/stream-truncated-writes'

ethomson · ethomson · commit 6853a250071d · 2019-01-31T14:46:21.000Z
diff --git a/src/stream.h b/src/stream.h
@@ -55,6 +55,21 @@ GIT_INLINE(ssize_t) git_stream_write(git_stream *st, const char *data, size_t le
 	return st->write(st, data, len, flags);
 }
 
+GIT_INLINE(int) git_stream__write_full(git_stream *st, const char *data, size_t len, int flags)
+{
+	size_t total_written = 0;
+
+	while (total_written < len) {
+		ssize_t written = git_stream_write(st, data + total_written, len - total_written, flags);
+		if (written <= 0)
+			return -1;
+
+		total_written += written;
+	}
+
+	return 0;
+}
+
 GIT_INLINE(int) git_stream_close(git_stream *st)
 {
 	return st->close(st);
diff --git a/src/streams/mbedtls.c b/src/streams/mbedtls.c
@@ -42,9 +42,9 @@
 #define GIT_SSL_DEFAULT_CIPHERS "TLS-ECDHE-ECDSA-WITH-AES-128-GCM-SHA256:TLS-ECDHE-RSA-WITH-AES-128-GCM-SHA256:TLS-ECDHE-ECDSA-WITH-AES-256-GCM-SHA384:TLS-ECDHE-RSA-WITH-AES-256-GCM-SHA384:TLS-DHE-RSA-WITH-AES-128-GCM-SHA256:TLS-DHE-DSS-WITH-AES-128-GCM-SHA256:TLS-DHE-RSA-WITH-AES-256-GCM-SHA384:TLS-DHE-DSS-WITH-AES-256-GCM-SHA384:TLS-ECDHE-ECDSA-WITH-AES-128-CBC-SHA256:TLS-ECDHE-RSA-WITH-AES-128-CBC-SHA256:TLS-ECDHE-ECDSA-WITH-AES-128-CBC-SHA:TLS-ECDHE-RSA-WITH-AES-128-CBC-SHA:TLS-ECDHE-ECDSA-WITH-AES-256-CBC-SHA384:TLS-ECDHE-RSA-WITH-AES-256-CBC-SHA384:TLS-ECDHE-ECDSA-WITH-AES-256-CBC-SHA:TLS-ECDHE-RSA-WITH-AES-256-CBC-SHA:TLS-DHE-RSA-WITH-AES-128-CBC-SHA256:TLS-DHE-RSA-WITH-AES-256-CBC-SHA256:TLS-DHE-RSA-WITH-AES-128-CBC-SHA:TLS-DHE-RSA-WITH-AES-256-CBC-SHA:TLS-DHE-DSS-WITH-AES-128-CBC-SHA256:TLS-DHE-DSS-WITH-AES-256-CBC-SHA256:TLS-DHE-DSS-WITH-AES-128-CBC-SHA:TLS-DHE-DSS-WITH-AES-256-CBC-SHA:TLS-RSA-WITH-AES-128-GCM-SHA256:TLS-RSA-WITH-AES-256-GCM-SHA384:TLS-RSA-WITH-AES-128-CBC-SHA256:TLS-RSA-WITH-AES-256-CBC-SHA256:TLS-RSA-WITH-AES-128-CBC-SHA:TLS-RSA-WITH-AES-256-CBC-SHA"
 #define GIT_SSL_DEFAULT_CIPHERS_COUNT 30
 
-mbedtls_ssl_config *git__ssl_conf;
+static mbedtls_ssl_config *git__ssl_conf;
 static int ciphers_list[GIT_SSL_DEFAULT_CIPHERS_COUNT];
-mbedtls_entropy_context *mbedtls_entropy;
+static mbedtls_entropy_context *mbedtls_entropy;
 
 /**
  * This function aims to clean-up the SSL context which
@@ -166,18 +166,16 @@ int git_mbedtls_stream_global_init(void)
 	return -1;
 }
 
-mbedtls_ssl_config *git__ssl_conf;
-
 static int bio_read(void *b, unsigned char *buf, size_t len)
 {
 	git_stream *io = (git_stream *) b;
-	return (int) git_stream_read(io, buf, len);
+	return (int) git_stream_read(io, buf, min(len, INT_MAX));
 }
 
 static int bio_write(void *b, const unsigned char *buf, size_t len)
 {
 	git_stream *io = (git_stream *) b;
-	return (int) git_stream_write(io, (const char *)buf, len, 0);
+	return (int) git_stream_write(io, (const char *)buf, min(len, INT_MAX), 0);
 }
 
 static int ssl_set_error(mbedtls_ssl_context *ssl, int error)
@@ -246,7 +244,7 @@ typedef struct {
 } mbedtls_stream;
 
 
-int mbedtls_connect(git_stream *stream)
+static int mbedtls_connect(git_stream *stream)
 {
 	int ret;
 	mbedtls_stream *st = (mbedtls_stream *) stream;
@@ -266,7 +264,7 @@ int mbedtls_connect(git_stream *stream)
 	return verify_server_cert(st->ssl);
 }
 
-int mbedtls_certificate(git_cert **out, git_stream *stream)
+static int mbedtls_certificate(git_cert **out, git_stream *stream)
 {
 	unsigned char *encoded_cert;
 	mbedtls_stream *st = (mbedtls_stream *) stream;
@@ -303,25 +301,27 @@ static int mbedtls_set_proxy(git_stream *stream, const git_proxy_options *proxy_
 	return git_stream_set_proxy(st->io, proxy_options);
 }
 
-ssize_t mbedtls_stream_write(git_stream *stream, const char *data, size_t data_len, int flags)
+static ssize_t mbedtls_stream_write(git_stream *stream, const char *data, size_t len, int flags)
 {
-	ssize_t written = 0, len = min(data_len, SSIZE_MAX);
 	mbedtls_stream *st = (mbedtls_stream *) stream;
+	int written;
 
 	GIT_UNUSED(flags);
 
-	do {
-		int error = mbedtls_ssl_write(st->ssl, (const unsigned char *)data + written, len - written);
-		if (error <= 0) {
-			return ssl_set_error(st->ssl, error);
-		}
-		written += error;
-	} while (written < len);
+	/*
+	 * `mbedtls_ssl_write` can only represent INT_MAX bytes
+	 * written via its return value. We thus need to clamp
+	 * the maximum number of bytes written.
+	 */
+	len = min(len, INT_MAX);
+
+	if ((written = mbedtls_ssl_write(st->ssl, (const unsigned char *)data, len)) <= 0)
+		return ssl_set_error(st->ssl, written);
 
 	return written;
 }
 
-ssize_t mbedtls_stream_read(git_stream *stream, void *data, size_t len)
+static ssize_t mbedtls_stream_read(git_stream *stream, void *data, size_t len)
 {
 	mbedtls_stream *st = (mbedtls_stream *) stream;
 	int ret;
@@ -332,7 +332,7 @@ ssize_t mbedtls_stream_read(git_stream *stream, void *data, size_t len)
 	return ret;
 }
 
-int mbedtls_stream_close(git_stream *stream)
+static int mbedtls_stream_close(git_stream *stream)
 {
 	mbedtls_stream *st = (mbedtls_stream *) stream;
 	int ret = 0;
@@ -345,7 +345,7 @@ int mbedtls_stream_close(git_stream *stream)
 	return st->owned ? git_stream_close(st->io) : 0;
 }
 
-void mbedtls_stream_free(git_stream *stream)
+static void mbedtls_stream_free(git_stream *stream)
 {
 	mbedtls_stream *st = (mbedtls_stream *) stream;
 
diff --git a/src/streams/openssl.c b/src/streams/openssl.c
@@ -572,9 +572,7 @@ typedef struct {
 	git_cert_x509 cert_info;
 } openssl_stream;
 
-int openssl_close(git_stream *stream);
-
-int openssl_connect(git_stream *stream)
+static int openssl_connect(git_stream *stream)
 {
 	int ret;
 	BIO *bio;
@@ -602,7 +600,7 @@ int openssl_connect(git_stream *stream)
 	return verify_server_cert(st->ssl, st->host);
 }
 
-int openssl_certificate(git_cert **out, git_stream *stream)
+static int openssl_certificate(git_cert **out, git_stream *stream)
 {
 	openssl_stream *st = (openssl_stream *) stream;
 	int len;
@@ -644,21 +642,20 @@ static int openssl_set_proxy(git_stream *stream, const git_proxy_options *proxy_
 	return git_stream_set_proxy(st->io, proxy_opts);
 }
 
-ssize_t openssl_write(git_stream *stream, const char *data, size_t data_len, int flags)
+static ssize_t openssl_write(git_stream *stream, const char *data, size_t data_len, int flags)
 {
 	openssl_stream *st = (openssl_stream *) stream;
 	int ret, len = min(data_len, INT_MAX);
 
 	GIT_UNUSED(flags);
 
-	if ((ret = SSL_write(st->ssl, data, len)) <= 0) {
+	if ((ret = SSL_write(st->ssl, data, len)) <= 0)
 		return ssl_set_error(st->ssl, ret);
-	}
 
 	return ret;
 }
 
-ssize_t openssl_read(git_stream *stream, void *data, size_t len)
+static ssize_t openssl_read(git_stream *stream, void *data, size_t len)
 {
 	openssl_stream *st = (openssl_stream *) stream;
 	int ret;
@@ -669,7 +666,7 @@ ssize_t openssl_read(git_stream *stream, void *data, size_t len)
 	return ret;
 }
 
-int openssl_close(git_stream *stream)
+static int openssl_close(git_stream *stream)
 {
 	openssl_stream *st = (openssl_stream *) stream;
 	int ret;
@@ -682,7 +679,7 @@ int openssl_close(git_stream *stream)
 	return st->owned ? git_stream_close(st->io) : 0;
 }
 
-void openssl_free(git_stream *stream)
+static void openssl_free(git_stream *stream)
 {
 	openssl_stream *st = (openssl_stream *) stream;
 
diff --git a/src/streams/socket.c b/src/streams/socket.c
@@ -69,7 +69,7 @@ static int close_socket(GIT_SOCKET s)
 
 }
 
-int socket_connect(git_stream *stream)
+static int socket_connect(git_stream *stream)
 {
 	struct addrinfo *info = NULL, *p;
 	struct addrinfo hints;
@@ -130,26 +130,22 @@ int socket_connect(git_stream *stream)
 	return 0;
 }
 
-ssize_t socket_write(git_stream *stream, const char *data, size_t data_len, int flags)
+static ssize_t socket_write(git_stream *stream, const char *data, size_t len, int flags)
 {
-	ssize_t ret, off = 0, len = min(data_len, SSIZE_MAX);
 	git_socket_stream *st = (git_socket_stream *) stream;
+	ssize_t written;
 
-	while (off < len) {
-		errno = 0;
-		ret = p_send(st->s, data + off, len - off, flags);
-		if (ret < 0) {
-			net_set_error("Error sending data");
-			return -1;
-		}
+	errno = 0;
 
-		off += ret;
+	if ((written = p_send(st->s, data, len, flags)) < 0) {
+		net_set_error("Error sending data");
+		return -1;
 	}
 
-	return off;
+	return written;
 }
 
-ssize_t socket_read(git_stream *stream, void *data, size_t len)
+static ssize_t socket_read(git_stream *stream, void *data, size_t len)
 {
 	ssize_t ret;
 	git_socket_stream *st = (git_socket_stream *) stream;
@@ -160,7 +156,7 @@ ssize_t socket_read(git_stream *stream, void *data, size_t len)
 	return ret;
 }
 
-int socket_close(git_stream *stream)
+static int socket_close(git_stream *stream)
 {
 	git_socket_stream *st = (git_socket_stream *) stream;
 	int error;
@@ -171,7 +167,7 @@ int socket_close(git_stream *stream)
 	return error;
 }
 
-void socket_free(git_stream *stream)
+static void socket_free(git_stream *stream)
 {
 	git_socket_stream *st = (git_socket_stream *) stream;
 
diff --git a/src/streams/stransport.c b/src/streams/stransport.c
@@ -149,9 +149,8 @@ static OSStatus write_cb(SSLConnectionRef conn, const void *data, size_t *len)
 {
 	git_stream *io = (git_stream *) conn;
 
-	if (git_stream_write(io, data, *len, 0) < 0) {
+	if (git_stream__write_full(io, data, *len, 0) < 0)
 		return -36; /* "ioErr" from MacErrors.h which is not available on iOS */
-	}
 
 	return noErr;
 }
diff --git a/src/transports/git.c b/src/transports/git.c
@@ -76,18 +76,15 @@ static int gen_proto(git_buf *request, const char *cmd, const char *url)
 static int send_command(git_proto_stream *s)
 {
 	git_buf request = GIT_BUF_INIT;
-	size_t write_size;
 	int error;
 
-	error = gen_proto(&request, s->cmd, s->url);
-	if (error < 0)
+	if ((error = gen_proto(&request, s->cmd, s->url)) < 0)
 		goto cleanup;
 
-	write_size = min(request.size, INT_MAX);
-	error = (int)git_stream_write(s->io, request.ptr, write_size, 0);
+	if ((error = git_stream__write_full(s->io, request.ptr, request.size, 0)) < 0)
+		goto cleanup;
 
-	if (error >= 0)
-		s->sent_command = 1;
+	s->sent_command = 1;
 
 cleanup:
 	git_buf_dispose(&request);
@@ -122,16 +119,15 @@ static int git_proto_stream_read(
 static int git_proto_stream_write(
 	git_smart_subtransport_stream *stream,
 	const char *buffer,
-	size_t buffer_len)
+	size_t len)
 {
 	git_proto_stream *s = (git_proto_stream *)stream;
-	size_t len = min(buffer_len, INT_MAX);
 	int error;
 
 	if (!s->sent_command && (error = send_command(s)) < 0)
 		return error;
 
-	return (int)git_stream_write(s->io, buffer, len, 0);
+	return git_stream__write_full(s->io, buffer, len, 0);
 }
 
 static void git_proto_stream_free(git_smart_subtransport_stream *stream)
diff --git a/src/transports/http.c b/src/transports/http.c
@@ -643,19 +643,19 @@ static int write_chunk(git_stream *io, const char *buffer, size_t len)
 	if (git_buf_oom(&buf))
 		return -1;
 
-	if (git_stream_write(io, buf.ptr, buf.size, 0) < 0) {
+	if (git_stream__write_full(io, buf.ptr, buf.size, 0) < 0) {
 		git_buf_dispose(&buf);
 		return -1;
 	}
 
 	git_buf_dispose(&buf);
 
 	/* Chunk body */
-	if (len > 0 && git_stream_write(io, buffer, len, 0) < 0)
+	if (len > 0 && git_stream__write_full(io, buffer, len, 0) < 0)
 		return -1;
 
 	/* Chunk footer */
-	if (git_stream_write(io, "\r\n", 2, 0) < 0)
+	if (git_stream__write_full(io, "\r\n", 2, 0) < 0)
 		return -1;
 
 	return 0;
@@ -853,8 +853,8 @@ static int proxy_connect(
 	if ((error = gen_connect_req(&request, t)) < 0)
 		goto done;
 
-	if ((error = git_stream_write(proxy_stream,
-	    request.ptr, request.size, 0)) < 0)
+	if ((error = git_stream__write_full(proxy_stream, request.ptr,
+					    request.size, 0)) < 0)
 		goto done;
 
 	git_buf_dispose(&request);
@@ -1034,8 +1034,8 @@ static int http_stream_read(
 		if (gen_request(&request, s, 0) < 0)
 			return -1;
 
-		if (git_stream_write(t->server.stream,
-		    request.ptr, request.size, 0) < 0) {
+		if (git_stream__write_full(t->server.stream, request.ptr,
+					   request.size, 0) < 0) {
 			git_buf_dispose(&request);
 			return -1;
 		}
@@ -1058,7 +1058,8 @@ static int http_stream_read(
 			s->chunk_buffer_len = 0;
 
 			/* Write the final chunk. */
-			if (git_stream_write(t->server.stream, "0\r\n\r\n", 5, 0) < 0)
+			if (git_stream__write_full(t->server.stream,
+						   "0\r\n\r\n", 5, 0) < 0)
 				return -1;
 		}
 
@@ -1157,8 +1158,8 @@ static int http_stream_write_chunked(
 		if (gen_request(&request, s, 0) < 0)
 			return -1;
 
-		if (git_stream_write(t->server.stream,
-		    request.ptr, request.size, 0) < 0) {
+		if (git_stream__write_full(t->server.stream, request.ptr,
+					   request.size, 0) < 0) {
 			git_buf_dispose(&request);
 			return -1;
 		}
@@ -1233,11 +1234,10 @@ static int http_stream_write_single(
 	if (gen_request(&request, s, len) < 0)
 		return -1;
 
-	if (git_stream_write(t->server.stream,
-	    request.ptr, request.size, 0) < 0)
+	if (git_stream__write_full(t->server.stream, request.ptr, request.size, 0) < 0)
 		goto on_error;
 
-	if (len && git_stream_write(t->server.stream, buffer, len, 0) < 0)
+	if (len && git_stream__write_full(t->server.stream, buffer, len, 0) < 0)
 		goto on_error;
 
 	git_buf_dispose(&request);

Original file line number	Diff line number	Diff line change
`@@ -572,9 +572,7 @@ typedef struct {`
`572`	`572`	`git_cert_x509 cert_info;`
`573`	`573`	`} openssl_stream;`
`574`	`574`
`575`		`-int openssl_close(git_stream *stream);`
`576`		`-`
`577`		`-int openssl_connect(git_stream *stream)`
	`575`	`+static int openssl_connect(git_stream *stream)`
`578`	`576`	`{`
`579`	`577`	`int ret;`
`580`	`578`	`BIO *bio;`
`@@ -602,7 +600,7 @@ int openssl_connect(git_stream *stream)`
`602`	`600`	`return verify_server_cert(st->ssl, st->host);`
`603`	`601`	`}`
`604`	`602`
`605`		`-int openssl_certificate(git_cert *out, git_stream stream)`
	`603`	`+static int openssl_certificate(git_cert *out, git_stream stream)`
`606`	`604`	`{`
`607`	`605`	`openssl_stream st = (openssl_stream ) stream;`
`608`	`606`	`int len;`
`@@ -644,21 +642,20 @@ static int openssl_set_proxy(git_stream stream, const git_proxy_options proxy_`
`644`	`642`	`return git_stream_set_proxy(st->io, proxy_opts);`
`645`	`643`	`}`
`646`	`644`
`647`		`-ssize_t openssl_write(git_stream stream, const char data, size_t data_len, int flags)`
	`645`	`+static ssize_t openssl_write(git_stream stream, const char data, size_t data_len, int flags)`
`648`	`646`	`{`
`649`	`647`	`openssl_stream st = (openssl_stream ) stream;`
`650`	`648`	`int ret, len = min(data_len, INT_MAX);`
`651`	`649`
`652`	`650`	`GIT_UNUSED(flags);`
`653`	`651`
`654`		`- if ((ret = SSL_write(st->ssl, data, len)) <= 0) {`
	`652`	`+ if ((ret = SSL_write(st->ssl, data, len)) <= 0)`
`655`	`653`	`return ssl_set_error(st->ssl, ret);`
`656`		`- }`
`657`	`654`
`658`	`655`	`return ret;`
`659`	`656`	`}`
`660`	`657`
`661`		`-ssize_t openssl_read(git_stream stream, void data, size_t len)`
	`658`	`+static ssize_t openssl_read(git_stream stream, void data, size_t len)`
`662`	`659`	`{`
`663`	`660`	`openssl_stream st = (openssl_stream ) stream;`
`664`	`661`	`int ret;`
`@@ -669,7 +666,7 @@ ssize_t openssl_read(git_stream stream, void data, size_t len)`
`669`	`666`	`return ret;`
`670`	`667`	`}`
`671`	`668`
`672`		`-int openssl_close(git_stream *stream)`
	`669`	`+static int openssl_close(git_stream *stream)`
`673`	`670`	`{`
`674`	`671`	`openssl_stream st = (openssl_stream ) stream;`
`675`	`672`	`int ret;`
`@@ -682,7 +679,7 @@ int openssl_close(git_stream *stream)`
`682`	`679`	`return st->owned ? git_stream_close(st->io) : 0;`
`683`	`680`	`}`
`684`	`681`
`685`		`-void openssl_free(git_stream *stream)`
	`682`	`+static void openssl_free(git_stream *stream)`
`686`	`683`	`{`
`687`	`684`	`openssl_stream st = (openssl_stream ) stream;`
`688`	`685`
Original file line number	Diff line number	Diff line change
`@@ -69,7 +69,7 @@ static int close_socket(GIT_SOCKET s)`
`69`	`69`
`70`	`70`	`}`
`71`	`71`
`72`		`-int socket_connect(git_stream *stream)`
	`72`	`+static int socket_connect(git_stream *stream)`
`73`	`73`	`{`
`74`	`74`	`struct addrinfo info = NULL, p;`
`75`	`75`	`struct addrinfo hints;`
`@@ -130,26 +130,22 @@ int socket_connect(git_stream *stream)`
`130`	`130`	`return 0;`
`131`	`131`	`}`
`132`	`132`
`133`		`-ssize_t socket_write(git_stream stream, const char data, size_t data_len, int flags)`
	`133`	`+static ssize_t socket_write(git_stream stream, const char data, size_t len, int flags)`
`134`	`134`	`{`
`135`		`- ssize_t ret, off = 0, len = min(data_len, SSIZE_MAX);`
`136`	`135`	`git_socket_stream st = (git_socket_stream ) stream;`
	`136`	`+ ssize_t written;`
`137`	`137`
`138`		`- while (off < len) {`
`139`		`- errno = 0;`
`140`		`- ret = p_send(st->s, data + off, len - off, flags);`
`141`		`- if (ret < 0) {`
`142`		`- net_set_error("Error sending data");`
`143`		`- return -1;`
`144`		`- }`
	`138`	`+ errno = 0;`
`145`	`139`
`146`		`- off += ret;`
	`140`	`+ if ((written = p_send(st->s, data, len, flags)) < 0) {`
	`141`	`+ net_set_error("Error sending data");`
	`142`	`+ return -1;`
`147`	`143`	`}`
`148`	`144`
`149`		`- return off;`
	`145`	`+ return written;`
`150`	`146`	`}`
`151`	`147`
`152`		`-ssize_t socket_read(git_stream stream, void data, size_t len)`
	`148`	`+static ssize_t socket_read(git_stream stream, void data, size_t len)`
`153`	`149`	`{`
`154`	`150`	`ssize_t ret;`
`155`	`151`	`git_socket_stream st = (git_socket_stream ) stream;`
`@@ -160,7 +156,7 @@ ssize_t socket_read(git_stream stream, void data, size_t len)`
`160`	`156`	`return ret;`
`161`	`157`	`}`
`162`	`158`
`163`		`-int socket_close(git_stream *stream)`
	`159`	`+static int socket_close(git_stream *stream)`
`164`	`160`	`{`
`165`	`161`	`git_socket_stream st = (git_socket_stream ) stream;`
`166`	`162`	`int error;`
`@@ -171,7 +167,7 @@ int socket_close(git_stream *stream)`
`171`	`167`	`return error;`
`172`	`168`	`}`
`173`	`169`
`174`		`-void socket_free(git_stream *stream)`
	`170`	`+static void socket_free(git_stream *stream)`
`175`	`171`	`{`
`176`	`172`	`git_socket_stream st = (git_socket_stream ) stream;`
`177`	`173`
Original file line number	Diff line number	Diff line change
`@@ -149,9 +149,8 @@ static OSStatus write_cb(SSLConnectionRef conn, const void data, size_t len)`
`149`	`149`	`{`
`150`	`150`	`git_stream io = (git_stream ) conn;`
`151`	`151`
`152`		`- if (git_stream_write(io, data, *len, 0) < 0) {`
	`152`	`+ if (git_stream__write_full(io, data, *len, 0) < 0)`
`153`	`153`	`return -36; /* "ioErr" from MacErrors.h which is not available on iOS */`
`154`		`- }`
`155`	`154`
`156`	`155`	`return noErr;`
`157`	`156`	`}`