ssh: request credentials again on authentication failure

Instead of completely giving up on the first failure, ask for
credentials as long as we fail to authenticate.

script/cibuild.sh

@@ -34,5 +34,5 @@ export GITTEST_REMOTE_SSH_PUBKEY="$HOME/.ssh/id_rsa.pub"
 export GITTEST_REMOTE_SSH_PASSPHRASE=""
 
 if [ -e ./libgit2_clar ]; then
-    ./libgit2_clar -sonline::push -sonline::clone::cred_callback_failure
+    ./libgit2_clar -sonline::push -sonline::clone::cred_callback
 fi

src/transports/ssh.c

@@ -34,7 +34,6 @@ typedef struct {
 	git_smart_subtransport parent;
 	transport_smart *owner;
 	ssh_stream *current_stream;
-	git_cred *cred;
 } ssh_subtransport;
 
 static int list_auth_methods(int *out, const char *host, const char *port);
@@ -341,6 +340,9 @@ static int _git_ssh_authenticate_session(
 		}
 	} while (LIBSSH2_ERROR_EAGAIN == rc || LIBSSH2_ERROR_TIMEOUT == rc);
 
+        if (rc == LIBSSH2_ERROR_PASSWORD_EXPIRED || rc == LIBSSH2_ERROR_AUTHENTICATION_FAILED)
+                return GIT_EAUTH;
+
 	if (rc != LIBSSH2_ERROR_NONE) {
 		ssh_error(session, "Failed to authenticate SSH session");
 		return -1;
@@ -349,6 +351,37 @@ static int _git_ssh_authenticate_session(
 	return 0;
 }
 
+static int request_creds(git_cred **out, ssh_subtransport *t, const char *user, int auth_methods)
+{
+	int error, no_callback = 0;
+	git_cred *cred = NULL;
+
+	if (!t->owner->cred_acquire_cb) {
+		no_callback = 1;
+	} else {
+		error = t->owner->cred_acquire_cb(&cred, t->owner->url, user, auth_methods,
+						  t->owner->cred_acquire_payload);
+
+		if (error == GIT_PASSTHROUGH)
+			no_callback = 1;
+		else if (error < 0)
+			return error;
+		else if (!cred) {
+			giterr_set(GITERR_SSH, "Callback failed to initialize SSH credentials");
+			return -1;
+		}
+	}
+
+	if (no_callback) {
+		giterr_set(GITERR_SSH, "authentication required but no callback set");
+		return -1;
+	}
+
+	*out = cred;
+
+	return 0;
+}
+
 static int _git_ssh_session_create(
 	LIBSSH2_SESSION** session,
 	gitno_socket socket)
@@ -389,8 +422,9 @@ static int _git_ssh_setup_conn(
 {
 	char *host=NULL, *port=NULL, *path=NULL, *user=NULL, *pass=NULL;
 	const char *default_port="22";
-	int no_callback = 0, auth_methods, error = 0;
+	int auth_methods, error = 0;
 	ssh_stream *s;
+	git_cred *cred = NULL;
 	LIBSSH2_SESSION* session=NULL;
 	LIBSSH2_CHANNEL* channel=NULL;
 
@@ -420,37 +454,31 @@ static int _git_ssh_setup_conn(
 		goto on_error;
 
 	if (user && pass) {
-		if ((error = git_cred_userpass_plaintext_new(&t->cred, user, pass)) < 0)
+		if ((error = git_cred_userpass_plaintext_new(&cred, user, pass)) < 0)
 			goto on_error;
-	} else if (!t->owner->cred_acquire_cb) {
-		no_callback = 1;
-	} else {
-		error = t->owner->cred_acquire_cb(&t->cred, t->owner->url, user, auth_methods,
-						  t->owner->cred_acquire_payload);
-
-		if (error == GIT_PASSTHROUGH)
-			no_callback = 1;
-		else if (error < 0)
-			goto on_error;
-		else if (!t->cred) {
-			giterr_set(GITERR_SSH, "Callback failed to initialize SSH credentials");
-			error = -1;
-			goto on_error;
-		}
 	}
 
-	if (no_callback) {
-		giterr_set(GITERR_SSH, "authentication required but no callback set");
-		error = -1;
-		goto on_error;
-	}
-
-	assert(t->cred);
-
 	if ((error = _git_ssh_session_create(&session, s->socket)) < 0)
 		goto on_error;
 
-	if ((error = _git_ssh_authenticate_session(session, t->cred)) < 0)
+	error = GIT_EAUTH;
+	/* if we already have something to try */
+	if (cred)
+		error = _git_ssh_authenticate_session(session, cred);
+
+	while (error == GIT_EAUTH) {
+		if (cred) {
+			cred->free(cred);
+			cred = NULL;
+		}
+
+		if ((error = request_creds(&cred, t, user, auth_methods)) < 0)
+			goto on_error;
+
+		error = _git_ssh_authenticate_session(session, cred);
+	}
+
+	if (error < 0)
 		goto on_error;
 
 	channel = libssh2_channel_open_session(session);
@@ -466,6 +494,9 @@ static int _git_ssh_setup_conn(
 	s->channel = channel;
 
 	t->current_stream = s;
+	if (cred)
+		cred->free(cred);
+
 	git__free(host);
 	git__free(port);
 	git__free(path);
@@ -482,6 +513,9 @@ on_error:
 	if (*stream)
 		ssh_stream_free(*stream);
 
+	if (cred)
+		cred->free(cred);
+
 	git__free(host);
 	git__free(port);
 	git__free(user);

tests/online/clone.c

@@ -246,6 +246,37 @@ void test_online_clone__cred_callback_failure_return_code_is_tunnelled(void)
 	cl_git_fail_with(-172, git_clone(&g_repo, remote_url, "./foo", &g_options));
 }
 
+static int cred_count_calls_cb(git_cred **cred, const char *url, const char *user,
+			       unsigned int allowed_types, void *data)
+{
+	size_t *counter = (size_t *) data;
+
+	GIT_UNUSED(url); GIT_UNUSED(user); GIT_UNUSED(allowed_types);
+
+	(*counter)++;
+
+	if (*counter == 3)
+		return GIT_EUSER;
+
+	return git_cred_userpass_plaintext_new(cred, "foo", "bar");
+}
+
+void test_online_clone__cred_callback_called_again_on_auth_failure(void)
+{
+	const char *remote_url = cl_getenv("GITTEST_REMOTE_URL");
+	const char *remote_user = cl_getenv("GITTEST_REMOTE_USER");
+	size_t counter = 0;
+
+	if (!remote_url || !remote_user)
+		clar__skip();
+
+	g_options.remote_callbacks.credentials = cred_count_calls_cb;
+	g_options.remote_callbacks.payload = &counter;
+
+	cl_git_fail_with(GIT_EUSER, git_clone(&g_repo, remote_url, "./foo", &g_options));
+	cl_assert_equal_i(3, counter);
+}
+
 void test_online_clone__credentials(void)
 {
 	/* Remote URL environment variable must be set.  User and password are optional.  */