git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
* [PATCH v3 0/2]  http: store credential when PKI auth is used
@ 2021-03-12  2:40 John Szakmeister
  2021-03-12  2:40 ` [PATCH v3 1/2] " John Szakmeister
                   ` (2 more replies)
  0 siblings, 3 replies; 4+ messages in thread
From: John Szakmeister @ 2021-03-12  2:40 UTC (permalink / raw)
  To: git, Jeff King; +Cc: Junio, brian, John Szakmeister

Essentially the same, but rejecting the correcting credential this time.

John Szakmeister (2):
  http: store credential when PKI auth is used
  http: drop the check for an empty proxy password before approving

 http.c | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

-- 
2.30.1


^ permalink raw reply	[flat|nested] 4+ messages in thread

* [PATCH v3 1/2] http: store credential when PKI auth is used
  2021-03-12  2:40 [PATCH v3 0/2] http: store credential when PKI auth is used John Szakmeister
@ 2021-03-12  2:40 ` John Szakmeister
  2021-03-12  2:40 ` [PATCH v3 2/2] http: drop the check for an empty proxy password before approving John Szakmeister
  2021-03-12 21:03 ` [PATCH v3 0/2] http: store credential when PKI auth is used brian m. carlson
  2 siblings, 0 replies; 4+ messages in thread
From: John Szakmeister @ 2021-03-12  2:40 UTC (permalink / raw)
  To: git, Jeff King; +Cc: Junio, brian, John Szakmeister

We already looked for the PKI credentials in the credential store, but
failed to approve it on success.  Meaning, the PKI certificate password
was never stored and git would request it on every connection to the
remote.  Let's complete the chain by storing the certificate password on
success.

Likewise, we also need to reject the credential when there is a failure.
Curl appears to report client-related certificate issues are reported
with the CURLE_SSL_CERTPROBLEM error.  This includes not only a bad
password, but potentially other client certificate related problems.
Since we cannot get more information from curl, we'll go ahead and
reject the credential upon receiving that error, just to be safe and
avoid caching or saving a bad password.

Signed-off-by: John Szakmeister <john@szakmeister.net>
---
 http.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/http.c b/http.c
index f8ea28bb2e..60d01c6e83 100644
--- a/http.c
+++ b/http.c
@@ -1637,7 +1637,17 @@ static int handle_curl_result(struct slot_results *results)
 		credential_approve(&http_auth);
 		if (proxy_auth.password)
 			credential_approve(&proxy_auth);
+		credential_approve(&cert_auth);
 		return HTTP_OK;
+	} else if (results->curl_result == CURLE_SSL_CERTPROBLEM) {
+		/*
+		 * We can't tell from here whether it's a bad path, bad
+		 * certificate, bad password, or something else wrong
+		 * with the certificate.  So we reject the credential to
+		 * avoid caching or saving a bad password.
+		 */
+		credential_reject(&cert_auth);
+		return HTTP_NOAUTH;
 	} else if (missing_target(results))
 		return HTTP_MISSING_TARGET;
 	else if (results->http_code == 401) {
-- 
2.30.1


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* [PATCH v3 2/2] http: drop the check for an empty proxy password before approving
  2021-03-12  2:40 [PATCH v3 0/2] http: store credential when PKI auth is used John Szakmeister
  2021-03-12  2:40 ` [PATCH v3 1/2] " John Szakmeister
@ 2021-03-12  2:40 ` John Szakmeister
  2021-03-12 21:03 ` [PATCH v3 0/2] http: store credential when PKI auth is used brian m. carlson
  2 siblings, 0 replies; 4+ messages in thread
From: John Szakmeister @ 2021-03-12  2:40 UTC (permalink / raw)
  To: git, Jeff King; +Cc: Junio, brian, John Szakmeister

credential_approve() already checks for a non-empty password before
saving, so there's no need to do the extra check here.

Signed-off-by: John Szakmeister <john@szakmeister.net>
---
 http.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/http.c b/http.c
index 60d01c6e83..3aeabf0947 100644
--- a/http.c
+++ b/http.c
@@ -1635,8 +1635,7 @@ static int handle_curl_result(struct slot_results *results)
 
 	if (results->curl_result == CURLE_OK) {
 		credential_approve(&http_auth);
-		if (proxy_auth.password)
-			credential_approve(&proxy_auth);
+		credential_approve(&proxy_auth);
 		credential_approve(&cert_auth);
 		return HTTP_OK;
 	} else if (results->curl_result == CURLE_SSL_CERTPROBLEM) {
-- 
2.30.1


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH v3 0/2]  http: store credential when PKI auth is used
  2021-03-12  2:40 [PATCH v3 0/2] http: store credential when PKI auth is used John Szakmeister
  2021-03-12  2:40 ` [PATCH v3 1/2] " John Szakmeister
  2021-03-12  2:40 ` [PATCH v3 2/2] http: drop the check for an empty proxy password before approving John Szakmeister
@ 2021-03-12 21:03 ` brian m. carlson
  2 siblings, 0 replies; 4+ messages in thread
From: brian m. carlson @ 2021-03-12 21:03 UTC (permalink / raw)
  To: John Szakmeister; +Cc: git, Jeff King, Junio

[-- Attachment #1: Type: text/plain, Size: 467 bytes --]

On 2021-03-12 at 02:40:25, John Szakmeister wrote:
> Essentially the same, but rejecting the correcting credential this time.
> 
> John Szakmeister (2):
>   http: store credential when PKI auth is used
>   http: drop the check for an empty proxy password before approving
> 
>  http.c | 13 +++++++++++--
>  1 file changed, 11 insertions(+), 2 deletions(-)

I looked and this seems sensible.
-- 
brian m. carlson (he/him or they/them)
Houston, Texas, US

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 263 bytes --]

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2021-03-12 21:05 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-03-12  2:40 [PATCH v3 0/2] http: store credential when PKI auth is used John Szakmeister
2021-03-12  2:40 ` [PATCH v3 1/2] " John Szakmeister
2021-03-12  2:40 ` [PATCH v3 2/2] http: drop the check for an empty proxy password before approving John Szakmeister
2021-03-12 21:03 ` [PATCH v3 0/2] http: store credential when PKI auth is used brian m. carlson

Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).