fix

src/util/streams/securetransport.c

@@ -50,6 +50,89 @@ typedef struct {
 	git_cert_x509 cert_info;
 } securetransport_stream;
 
+static int securetransport_certificate(git_cert **out, git_stream *stream)
+{
+	securetransport_stream *st = (securetransport_stream *) stream;
+	SecTrustRef trust = NULL;
+	SecCertificateRef sec_cert;
+	OSStatus ret;
+
+	if ((ret = SSLCopyPeerTrust(st->ctx, &trust)) != noErr)
+		return securetransport_error(ret);
+
+	sec_cert = SecTrustGetCertificateAtIndex(trust, 0);
+	st->der_data = SecCertificateCopyData(sec_cert);
+	CFRelease(trust);
+
+	if (st->der_data == NULL) {
+		git_error_set(GIT_ERROR_SSL, "retrieved invalid certificate data");
+		return -1;
+	}
+
+	st->cert_info.parent.cert_type = GIT_CERT_X509;
+	st->cert_info.data = (void *) CFDataGetBytePtr(st->der_data);
+	st->cert_info.len = CFDataGetLength(st->der_data);
+
+	*out = (git_cert *)&st->cert_info;
+	return 0;
+}
+
+/*
+ * Contrary to typical network IO callbacks, Secure Transport write callback is
+ * expected to write *all* passed data, not just as much as it can, and any
+ * other case would be considered a failure.
+ *
+ * This behavior is actually not specified in the Apple documentation, but is
+ * required for things to work correctly (and incidentally, that's also how
+ * Apple implements it in its projects at opensource.apple.com).
+ *
+ * Libgit2 streams happen to already have this very behavior so this is just
+ * passthrough.
+ */
+static OSStatus write_cb(SSLConnectionRef conn, const void *data, size_t *len)
+{
+	securetransport_stream *st = (securetransport_stream *)conn;
+	git_stream *io = st->io;
+	OSStatus ret;
+
+	st->error = 0;
+
+	ret = git_stream__write_full(io, data, *len, 0);
+
+	if (ret < 0) {
+		st->error = ret;
+		return (ret == GIT_TIMEOUT) ?
+		       -9853 /* errSSLNetworkTimeout */:
+		       -36 /* ioErr */;
+	}
+
+	return noErr;
+}
+
+static ssize_t securetransport_write(
+	git_stream *stream,
+	const char *data,
+	size_t len,
+	int flags)
+{
+	securetransport_stream *st = (securetransport_stream *) stream;
+	size_t data_len, processed;
+	OSStatus ret;
+
+	GIT_UNUSED(flags);
+
+	data_len = min(len, SSIZE_MAX);
+	if ((ret = SSLWrite(st->ctx, data, data_len, &processed)) != noErr) {
+		if (st->error == GIT_TIMEOUT)
+			return GIT_TIMEOUT;
+
+		return securetransport_error(ret);
+	}
+
+	GIT_ASSERT(processed < SSIZE_MAX);
+	return (ssize_t)processed;
+}
+
 /*
  * Contrary to typical network IO callbacks, Secure Transport read callback is
  * expected to read *exactly* the requested number of bytes, not just as much
@@ -90,198 +173,10 @@ static OSStatus read_cb(SSLConnectionRef conn, void *data, size_t *len)
 	return error;
 }
 
-/*
- * Contrary to typical network IO callbacks, Secure Transport write callback is
- * expected to write *all* passed data, not just as much as it can, and any
- * other case would be considered a failure.
- *
- * This behavior is actually not specified in the Apple documentation, but is
- * required for things to work correctly (and incidentally, that's also how
- * Apple implements it in its projects at opensource.apple.com).
- *
- * Libgit2 streams happen to already have this very behavior so this is just
- * passthrough.
- */
-static OSStatus write_cb(SSLConnectionRef conn, const void *data, size_t *len)
-{
-	securetransport_stream *st = (securetransport_stream *)conn;
-	git_stream *io = st->io;
-	OSStatus ret;
-
-	st->error = 0;
-
-	ret = git_stream__write_full(io, data, *len, 0);
-
-	if (ret < 0) {
-		st->error = ret;
-		return (ret == GIT_TIMEOUT) ?
-		       -9853 /* errSSLNetworkTimeout */:
-		       -36 /* ioErr */;
-	}
-
-	return noErr;
-}
-
-static int securetransport_create_context(
-	securetransport_stream *st,
-	const char *host)
-{
-	SecTrustRef trust = NULL;
-	SecTrustResultType sec_res;
-	OSStatus ret;
-
-	st->ctx = SSLCreateContext(NULL, kSSLClientSide, kSSLStreamType);
-	if (!st->ctx) {
-		git_error_set(GIT_ERROR_NET, "failed to create SSL context");
-		return -1;
-	}
-
-	/* Configure the context */
-	if ((ret = SSLSetIOFuncs(st->ctx, read_cb, write_cb)) != noErr ||
-	    (ret = SSLSetConnection(st->ctx, st)) != noErr ||
-	    (ret = SSLSetSessionOption(st->ctx, kSSLSessionOptionBreakOnServerAuth, true)) != noErr ||
-	    (ret = SSLSetProtocolVersionMin(st->ctx, kTLSProtocol1)) != noErr ||
-	    (ret = SSLSetProtocolVersionMax(st->ctx, kTLSProtocol12)) != noErr ||
-	    (ret = SSLSetPeerDomainName(st->ctx, host, strlen(host))) != noErr) {
-		CFRelease(st->ctx);
-		return securetransport_error(ret);
-	}
-
-	/* Connect */
-	ret = SSLHandshake(st->ctx);
-
-	if (ret != errSSLServerAuthCompleted && st->error != 0)
-		return -1;
-	else if (ret != errSSLServerAuthCompleted) {
-		git_error_set(GIT_ERROR_SSL, "unexpected return value from ssl handshake %d", (int)ret);
-		return -1;
-	}
-
-	if ((ret = SSLCopyPeerTrust(st->ctx, &trust)) != noErr)
-		goto on_error;
-
-	if (!trust)
-		return GIT_ECERTIFICATE;
-
-	if ((ret = SecTrustEvaluate(trust, &sec_res)) != noErr)
-		goto on_error;
-
-	CFRelease(trust);
-
-	if (sec_res == kSecTrustResultInvalid ||
-	    sec_res == kSecTrustResultOtherError) {
-		git_error_set(GIT_ERROR_SSL, "internal security trust error");
-		return -1;
-	}
-
-	if (sec_res == kSecTrustResultDeny ||
-	    sec_res == kSecTrustResultRecoverableTrustFailure ||
-	    sec_res == kSecTrustResultFatalTrustFailure) {
-		git_error_set(GIT_ERROR_SSL, "untrusted connection error");
-		return GIT_ECERTIFICATE;
-	}
-
-	return 0;
-
-on_error:
-	if (trust)
-		CFRelease(trust);
-
-	return securetransport_error(ret);
-}
-
-static int securetransport_connect(
+static ssize_t securetransport_read(
 	git_stream *stream,
-	const char *host,
-	const char *port,
-	const git_stream_connect_options *opts)
-{
-	securetransport_stream *st = (securetransport_stream *)stream;
-	int error;
-
-	GIT_ASSERT_ARG(stream);
-	GIT_ASSERT_ARG(host);
-
-	/* TODO: asser tstate */
-
-	if ((error = git_stream_socket_new(&st->io)) < 0 ||
-	    (error = git_stream_connect(st->io, host, port, opts)) < 0 ||
-	    (error = securetransport_create_context(st, host)) < 0) {
-		git_stream_close(st->io);
-		git_stream_free(st->io);
-		return error;
-	}
-
-	/* TODO: make this state */
-	st->owned = 1;
-
-	return 0;
-}
-
-static int securetransport_wrap(
-	git_stream *stream,
-	git_stream *in,
-	const char *host)
-{
-	securetransport_stream *st = (securetransport_stream *)stream;
-
-	GIT_ASSERT_ARG(stream);
-	GIT_ASSERT_ARG(in);
-	GIT_ASSERT_ARG(host);
-
-	st->io = in;
-
-	return securetransport_create_context(st, host);
-}
-
-static int securetransport_certificate(git_cert **out, git_stream *stream)
-{
-	securetransport_stream *st = (securetransport_stream *) stream;
-	SecTrustRef trust = NULL;
-	SecCertificateRef sec_cert;
-	OSStatus ret;
-
-	if ((ret = SSLCopyPeerTrust(st->ctx, &trust)) != noErr)
-		return securetransport_error(ret);
-
-	sec_cert = SecTrustGetCertificateAtIndex(trust, 0);
-	st->der_data = SecCertificateCopyData(sec_cert);
-	CFRelease(trust);
-
-	if (st->der_data == NULL) {
-		git_error_set(GIT_ERROR_SSL, "retrieved invalid certificate data");
-		return -1;
-	}
-
-	st->cert_info.parent.cert_type = GIT_CERT_X509;
-	st->cert_info.data = (void *) CFDataGetBytePtr(st->der_data);
-	st->cert_info.len = CFDataGetLength(st->der_data);
-
-	*out = (git_cert *)&st->cert_info;
-	return 0;
-}
-
-static ssize_t securetransport_write(git_stream *stream, const char *data, size_t len, int flags)
-{
-	securetransport_stream *st = (securetransport_stream *) stream;
-	size_t data_len, processed;
-	OSStatus ret;
-
-	GIT_UNUSED(flags);
-
-	data_len = min(len, SSIZE_MAX);
-	if ((ret = SSLWrite(st->ctx, data, data_len, &processed)) != noErr) {
-		if (st->error == GIT_TIMEOUT)
-			return GIT_TIMEOUT;
-
-		return securetransport_error(ret);
-	}
-
-	GIT_ASSERT(processed < SSIZE_MAX);
-	return (ssize_t)processed;
-}
-
-static ssize_t securetransport_read(git_stream *stream, void *data, size_t len)
+	void *data,
+	size_t len)
 {
 	securetransport_stream *st = (securetransport_stream *)stream;
 	size_t processed;
@@ -297,12 +192,137 @@ static ssize_t securetransport_read(git_stream *stream, void *data, size_t len)
 	return processed;
 }
 
+static int securetransport_create_context(
+	securetransport_stream *st,
+	const char *host)
+{
+	SecTrustRef trust = NULL;
+	SecTrustResultType sec_res;
+	OSStatus ret;
+	int error = -1;
+
+	st->ctx = SSLCreateContext(NULL, kSSLClientSide, kSSLStreamType);
+
+	if (!st->ctx) {
+		git_error_set(GIT_ERROR_NET, "failed to create SSL context");
+		return -1;
+	}
+
+	/* Set up context */
+
+	if ((ret = SSLSetIOFuncs(st->ctx, read_cb, write_cb)) != noErr ||
+	    (ret = SSLSetConnection(st->ctx, st)) != noErr ||
+	    (ret = SSLSetSessionOption(st->ctx, kSSLSessionOptionBreakOnServerAuth, true)) != noErr ||
+	    (ret = SSLSetProtocolVersionMin(st->ctx, kTLSProtocol1)) != noErr ||
+	    (ret = SSLSetProtocolVersionMax(st->ctx, kTLSProtocol12)) != noErr ||
+	    (ret = SSLSetPeerDomainName(st->ctx, host, strlen(host))) != noErr) {
+		error = securetransport_error(ret);
+		goto on_error;
+	}
+
+	/* Connect */
+
+	ret = SSLHandshake(st->ctx);
+
+	if (ret != errSSLServerAuthCompleted && st->error != 0) {
+		error = -1;
+		goto on_error;
+	} else if (ret != errSSLServerAuthCompleted) {
+		git_error_set(GIT_ERROR_SSL, "unexpected return value from ssl handshake %d", (int)ret);
+		error = -1;
+		goto on_error;
+	}
+
+	if ((ret = SSLCopyPeerTrust(st->ctx, &trust)) != noErr) {
+		error = securetransport_error(ret);
+		goto on_error;
+	}
+
+	if (!trust)
+		return GIT_ECERTIFICATE;
+
+	if ((ret = SecTrustEvaluate(trust, &sec_res)) != noErr) {
+		error = securetransport_error(ret);
+		goto on_error;
+	}
+
+	CFRelease(trust);
+
+	if (sec_res == kSecTrustResultInvalid ||
+	    sec_res == kSecTrustResultOtherError) {
+		git_error_set(GIT_ERROR_SSL, "internal security trust error");
+		error = -1;
+		goto on_error;
+	}
+
+	if (sec_res == kSecTrustResultDeny ||
+	    sec_res == kSecTrustResultRecoverableTrustFailure ||
+	    sec_res == kSecTrustResultFatalTrustFailure) {
+		git_error_set(GIT_ERROR_SSL, "untrusted connection error");
+		return GIT_ECERTIFICATE;
+	}
+
+	return 0;
+
+on_error:
+	if (trust)
+		CFRelease(trust);
+
+	if (st->ctx) {
+		CFRelease(st->ctx);
+		st->ctx = NULL;
+	}
+
+	return error;
+}
+
+int securetransport_connect(
+	git_stream *stream,
+	const char *host,
+	const char *port,
+	const git_stream_connect_options *opts)
+{
+	securetransport_stream *st = (securetransport_stream *)stream;
+
+	GIT_ASSERT_ARG(stream);
+	GIT_ASSERT_ARG(host);
+	GIT_ASSERT_ARG(port);
+
+	if (git_stream_socket_new(&st->io) < 0)
+		return -1;
+
+	st->owned = 1;
+
+	if (git_stream_connect(st->io, host, port, opts) < 0)
+		return -1;
+
+	return securetransport_create_context(st, host);
+}
+
+int securetransport_wrap(
+	git_stream *stream,
+	git_stream *in,
+	const char *host)
+{
+	securetransport_stream *st = (securetransport_stream *)stream;
+
+	GIT_ASSERT_ARG(stream);
+	GIT_ASSERT_ARG(in);
+	GIT_ASSERT_ARG(host);
+
+	st->io = in;
+	st->owned = 0;
+
+	return securetransport_create_context(st, host);
+}
+
 static int securetransport_close(git_stream *stream)
 {
-	securetransport_stream *st = (securetransport_stream *) stream;
+	securetransport_stream *st = (securetransport_stream *)stream;
 	OSStatus ret;
 
 	ret = SSLClose(st->ctx);
+
 	if (ret != noErr && ret != errSSLClosedGraceful)
 		return securetransport_error(ret);
 
@@ -311,22 +331,27 @@ static int securetransport_close(git_stream *stream)
 
 static void securetransport_free(git_stream *stream)
 {
-	securetransport_stream *st = (securetransport_stream *) stream;
+	securetransport_stream *st = (securetransport_stream *)stream;
 
 	if (st->owned)
 		git_stream_free(st->io);
 
 	if (st->ctx)
 		CFRelease(st->ctx);
+
 	if (st->der_data)
 		CFRelease(st->der_data);
+
 	git__free(st);
 }
 
 int git_stream_securetransport_new(git_stream **out)
 {
-	securetransport_stream *st =
-		git__calloc(1, sizeof(securetransport_stream));
+	securetransport_stream *st;
+
+	GIT_ASSERT_ARG(out);
+
+	st = git__calloc(1, sizeof(securetransport_stream));
 	GIT_ERROR_CHECK_ALLOC(st);
 
 	st->parent.version = GIT_STREAM_VERSION;
@@ -339,7 +364,7 @@ int git_stream_securetransport_new(git_stream **out)
 	st->parent.close = securetransport_close;
 	st->parent.free = securetransport_free;
 
-	*out = (git_stream *) st;
+	*out = (git_stream *)st;
 	return 0;
 }