Commit c97d302deccfbeba057e5543d64afaf9351c9752

Patrick Steinhardt 2018-11-28T13:45:41

Merge pull request #4879 from libgit2/ethomson/defer_cert_cred_cb Allow certificate and credential callbacks to decline to act

diff --git a/include/git2/errors.h b/include/git2/errors.h
index b0ce45f..2c0ac1c 100644
--- a/include/git2/errors.h
+++ b/include/git2/errors.h
@@ -52,7 +52,7 @@ typedef enum {
 	GIT_EDIRECTORY      = -23,      /**< The operation is not valid for a directory */
 	GIT_EMERGECONFLICT  = -24,	/**< A merge conflict exists and cannot continue */
 
-	GIT_PASSTHROUGH     = -30,	/**< Internal only */
+	GIT_PASSTHROUGH     = -30,	/**< A user-configured callback refused to act */
 	GIT_ITEROVER        = -31,	/**< Signals end of iteration with iterator */
 	GIT_RETRY           = -32,	/**< Internal only */
 	GIT_EMISMATCH       = -33,	/**< Hashsum mismatch in object */
diff --git a/include/git2/sys/transport.h b/include/git2/sys/transport.h
index a395de5..aac6f9f 100644
--- a/include/git2/sys/transport.h
+++ b/include/git2/sys/transport.h
@@ -226,7 +226,10 @@ GIT_EXTERN(int) git_transport_smart(
  * @param cert the certificate to pass to the caller
  * @param valid whether we believe the certificate is valid
  * @param hostname the hostname we connected to
- * @return the return value of the callback
+ * @return the return value of the callback: 0 for no error, GIT_PASSTHROUGH
+ *         to indicate that there is no callback registered (or the callback
+ *         refused to validate the certificate and callers should behave as
+ *         if no callback was set), or < 0 for an error
  */
 GIT_EXTERN(int) git_transport_smart_certificate_check(git_transport *transport, git_cert *cert, int valid, const char *hostname);
 
@@ -237,7 +240,10 @@ GIT_EXTERN(int) git_transport_smart_certificate_check(git_transport *transport, 
  * @param transport a smart transport
  * @param user the user we saw on the url (if any)
  * @param methods available methods for authentication
- * @return the return value of the callback
+ * @return the return value of the callback: 0 for no error, GIT_PASSTHROUGH
+ *         to indicate that there is no callback registered (or the callback
+ *         refused to provide credentials and callers should behave as if no
+ *         callback was set), or < 0 for an error
  */
 GIT_EXTERN(int) git_transport_smart_credentials(git_cred **out, git_transport *transport, const char *user, int methods);
 
diff --git a/include/git2/types.h b/include/git2/types.h
index e77e628..3c127e3 100644
--- a/include/git2/types.h
+++ b/include/git2/types.h
@@ -333,6 +333,9 @@ typedef struct {
  * this certificate is valid
  * @param host Hostname of the host libgit2 connected to
  * @param payload Payload provided by the caller
+ * @return 0 to proceed with the connection, < 0 to fail the connection
+ *         or > 0 to indicate that the callback refused to act and that
+ *         the existing validity determination should be honored
  */
 typedef int (*git_transport_certificate_check_cb)(git_cert *cert, int valid, const char *host, void *payload);
 
diff --git a/src/transports/http.c b/src/transports/http.c
index 5121996..7f9d350 100644
--- a/src/transports/http.c
+++ b/src/transports/http.c
@@ -371,6 +371,7 @@ static int on_headers_complete(http_parser *parser)
 								  allowed_auth_types,
 								  t->owner->cred_acquire_payload);
 
+				/* treat GIT_PASSTHROUGH as if callback isn't set */
 				if (error == GIT_PASSTHROUGH) {
 					no_callback = 1;
 				} else if (error < 0) {
@@ -639,6 +640,9 @@ static int http_connect(http_subtransport *t)
 		giterr_clear();
 		error = t->owner->certificate_check_cb(cert, is_valid, t->connection_data.host, t->owner->message_cb_payload);
 
+		if (error == GIT_PASSTHROUGH)
+			error = is_valid ? 0 : GIT_ECERTIFICATE;
+
 		if (error < 0) {
 			if (!giterr_last())
 				giterr_set(GITERR_NET, "user cancelled certificate check");
diff --git a/src/transports/smart.c b/src/transports/smart.c
index e972d30..9fcbdcf 100644
--- a/src/transports/smart.c
+++ b/src/transports/smart.c
@@ -481,6 +481,11 @@ int git_transport_smart_certificate_check(git_transport *transport, git_cert *ce
 {
 	transport_smart *t = (transport_smart *)transport;
 
+	assert(transport && cert && hostname);
+
+	if (!t->certificate_check_cb)
+		return GIT_PASSTHROUGH;
+
 	return t->certificate_check_cb(cert, valid, hostname, t->message_cb_payload);
 }
 
@@ -488,6 +493,11 @@ int git_transport_smart_credentials(git_cred **out, git_transport *transport, co
 {
 	transport_smart *t = (transport_smart *)transport;
 
+	assert(out && transport);
+
+	if (!t->cred_acquire_cb)
+		return GIT_PASSTHROUGH;
+
 	return t->cred_acquire_cb(out, t->url, user, methods, t->cred_acquire_payload);
 }
 
diff --git a/src/transports/ssh.c b/src/transports/ssh.c
index 7d9114c..9e01a4a 100644
--- a/src/transports/ssh.c
+++ b/src/transports/ssh.c
@@ -447,11 +447,11 @@ static int request_creds(git_cred **out, ssh_subtransport *t, const char *user, 
 		error = t->owner->cred_acquire_cb(&cred, t->owner->url, user, auth_methods,
 						  t->owner->cred_acquire_payload);
 
-		if (error == GIT_PASSTHROUGH)
+		if (error == GIT_PASSTHROUGH) {
 			no_callback = 1;
-		else if (error < 0)
+		} else if (error < 0) {
 			return error;
-		else if (!cred) {
+		} else if (!cred) {
 			giterr_set(GITERR_SSH, "callback failed to initialize SSH credentials");
 			return -1;
 		}
@@ -584,7 +584,8 @@ post_extract:
 		cert_ptr = &cert;
 
 		error = t->owner->certificate_check_cb((git_cert *) cert_ptr, 0, host, t->owner->message_cb_payload);
-		if (error < 0) {
+
+		if (error < 0 && error != GIT_PASSTHROUGH) {
 			if (!giterr_last())
 				giterr_set(GITERR_NET, "user cancelled hostkey check");
 
diff --git a/src/transports/winhttp.c b/src/transports/winhttp.c
index e925dbd..5e7bde7 100644
--- a/src/transports/winhttp.c
+++ b/src/transports/winhttp.c
@@ -228,7 +228,7 @@ static int fallback_cred_acquire_cb(
 		}
 
 		hCoInitResult = CoInitializeEx(NULL, COINIT_MULTITHREADED);
-			
+
 		if (SUCCEEDED(hCoInitResult) || hCoInitResult == RPC_E_CHANGED_MODE) {
 			IInternetSecurityManager* pISM;
 
@@ -295,6 +295,9 @@ static int certificate_check(winhttp_stream *s, int valid)
 	error = t->owner->certificate_check_cb((git_cert *) &cert, valid, t->connection_data.host, t->owner->message_cb_payload);
 	CertFreeCertificateContext(cert_ctx);
 
+	if (error == GIT_PASSTHROUGH)
+		error = valid ? 0 : GIT_ECERTIFICATE;
+
 	if (error < 0 && !giterr_last())
 		giterr_set(GITERR_NET, "user cancelled certificate check");