[PATCH 1/2] http.c: prompt for SSL client certificate password

If an SSL client certificate is enabled (via http.sslcert or
GIT_SSL_CERT), prompt for the certificate password rather than
defaulting to OpenSSL's password prompt.  This causes the prompt to only
appear once each run.  Previously, OpenSSL prompted the user *many*
times, causing git to be unusable over HTTPS with client-side
certificates.

Note that the password is stored in memory in the clear while the
program is running.  This may be a security problem if git crashes and
core dumps.

The user is always prompted, even if the certificate is not encrypted.
This should be fine; unencrypted certificates are rare and a security
risk anyway.

Signed-off-by: Mark Lodato <redacted>
---

See http://osdir.com/ml/git/2009-02/msg03402.html for a discussion of
this topic and an example showing how horrible the current password
prompts are.

The next patch adds an option to disable this feature.  I split it into
two commits in case the configuration option is not wanted.

I did not create any tests because the existing http.sslcert option has
no tests to begin with.

I would really like to use git over HTTPS with client certs, but the
current situation is just unusable.  So, I'm hoping this gets included
in git.git at some point.  I would be happy to hear any comments people
have about this patch series.  Thanks!


 http.c |   40 +++++++++++++++++++++++++++++++++++++++-
 1 files changed, 39 insertions(+), 1 deletions(-)

diff --git a/http.c b/http.c
index 2e3d649..1fc3444 100644
--- a/http.c
+++ b/http.c

@@ -26,6 +26,8 @@ static long curl_low_speed_time = -1;
 static int curl_ftp_no_epsv;
 static const char *curl_http_proxy;
 static char *user_name, *user_pass;
+static char *ssl_cert_password;
+static int ssl_cert_password_required;
 
 static struct curl_slist *pragma_header;
 

@@ -167,6 +169,22 @@ static void init_curl_http_auth(CURL *result)
 	}
 }
 
+static int has_cert_password(void)
+{
+	if (ssl_cert_password != NULL)
+		return 1;
+	if (ssl_cert == NULL || ssl_cert_password_required != 1)
+		return 0;
+	/* Only prompt the user once. */
+	ssl_cert_password_required = -1;
+	ssl_cert_password = getpass("Certificate Password: ");
+	if (ssl_cert_password != NULL) {
+		ssl_cert_password = xstrdup(ssl_cert_password);
+		return 1;
+	} else
+		return 0;
+}
+
 static CURL *get_curl_handle(void)
 {
 	CURL *result = curl_easy_init();

@@ -189,6 +207,16 @@ static CURL *get_curl_handle(void)
 
 	if (ssl_cert != NULL)
 		curl_easy_setopt(result, CURLOPT_SSLCERT, ssl_cert);
+	if (has_cert_password())
+		curl_easy_setopt(result,
+#if LIBCURL_VERSION_NUM >= 0x071700
+				 CURLOPT_KEYPASSWD,
+#elif LIBCURL_VERSION_NUM >= 0x070903
+				 CURLOPT_SSLKEYPASSWD,
+#else
+				 CURLOPT_SSLCERTPASSWD,
+#endif
+				 ssl_cert_password);
 #if LIBCURL_VERSION_NUM >= 0x070902
 	if (ssl_key != NULL)
 		curl_easy_setopt(result, CURLOPT_SSLKEY, ssl_key);

@@ -329,8 +357,11 @@ void http_init(struct remote *remote)
 	if (getenv("GIT_CURL_FTP_NO_EPSV"))
 		curl_ftp_no_epsv = 1;
 
-	if (remote && remote->url && remote->url[0])
+	if (remote && remote->url && remote->url[0]) {
 		http_auth_init(remote->url[0]);
+		if (!prefixcmp(remote->url[0], "https://"))
+			ssl_cert_password_required = 1;
+	}
 
 #ifndef NO_CURL_EASY_DUPHANDLE
 	curl_default = get_curl_handle();

@@ -370,6 +401,13 @@ void http_cleanup(void)
 		free((void *)curl_http_proxy);
 		curl_http_proxy = NULL;
 	}
+
+	if (ssl_cert_password != NULL) {
+		memset(ssl_cert_password, 0, strlen(ssl_cert_password));
+		free(ssl_cert_password);
+		ssl_cert_password = NULL;
+	}
+	ssl_cert_password_required = 0;
 }
 
 struct active_request_slot *get_active_slot(void)

-- 
1.6.3.1

Add a configuration option, http.sslCertNoPass, and associated
environment variable, GIT_SSL_CERT_NO_PASS, to allow disabling of the
SSL client certificate password prompt from within git.  If this option
is set to true, or if the environment variable exists, git falls back to
OpenSSL's prompts (as in earlier versions of git).

This option is useful in (at least) two cases:
1. The certificate is not encrypted and the user does not want to be
   prompted needlessly.
2. The user does not wish to leave the password in the clear in git's
   (and libcurl's) memory, in case the program crashes and core dumps.

The environment variable may only be used to disable, not to re-enable,
git's password prompt.  This behavior mimics GIT_NO_VERIFY; the mere
existence of the variable is all that is checked.

Signed-off-by: Mark Lodato <redacted>
---
 Documentation/config.txt |    9 +++++++++
 http.c                   |    9 ++++++++-
 2 files changed, 17 insertions(+), 1 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 2c03162..65c3ac5 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt

@@ -1038,6 +1038,15 @@ http.sslKey::
 	over HTTPS. Can be overridden by the 'GIT_SSL_KEY' environment
 	variable.
 
+http.sslCertNoPass::
+	Disable git's password prompt for the SSL certificate.  OpenSSL
+	will still prompt the user, possibly many times, if the
+	certificate or private key is encrypted.  Useful if the
+	certificate is not encrypted (to disable the password prompt) or
+	if you do not wish to store the certificate password in git's
+	memory.  Can be overridden by the 'GIT_SSL_CERT_NO_PASS'
+	environment variable.
+
 http.sslCAInfo::
 	File containing the certificates to verify the peer with when
 	fetching or pushing over HTTPS. Can be overridden by the

diff --git a/http.c b/http.c
index 1fc3444..6ae59b6 100644
--- a/http.c
+++ b/http.c

@@ -131,6 +131,11 @@ static int http_options(const char *var, const char *value, void *cb)
 #endif
 	if (!strcmp("http.sslcainfo", var))
 		return git_config_string(&ssl_cainfo, var, value);
+	if (!strcmp("http.sslcertnopass", var)) {
+		if (git_config_bool(var, value))
+			ssl_cert_password_required = -1;
+		return 0;
+	}
 #ifdef USE_CURL_MULTI
 	if (!strcmp("http.maxrequests", var)) {
 		max_requests = git_config_int(var, value);

@@ -359,7 +364,9 @@ void http_init(struct remote *remote)
 
 	if (remote && remote->url && remote->url[0]) {
 		http_auth_init(remote->url[0]);
-		if (!prefixcmp(remote->url[0], "https://"))
+		if (ssl_cert_password_required == 0 &&
+		    !getenv("GIT_SSL_CERT_NO_PASS") &&
+		    !prefixcmp(remote->url[0], "https://"))
 			ssl_cert_password_required = 1;
 	}
 

-- 
1.6.3.1

Any thoughts on this?  I would love to see this in git 1.6.4, and I
don't think it affects people who do not use certificates.

~ Mark

On Wed, May 27, 2009 at 11:16 PM, Mark Lodato[off-list ref] wrote:

How it works if git is run from IDEs (no tty will be available)?
Is there a way to redefine the way the password is got?
What about scripting scenarios where passwordless certificates are
likely to be used?

Regards,
Constantine

On Fri, Jun 5, 2009 at 6:44 AM, Mark Lodato [off-list ref] wrote:

Any thoughts on this?  I would love to see this in git 1.6.4, and I
don't think it affects people who do not use certificates.

~ Mark

On Wed, May 27, 2009 at 11:16 PM, Mark Lodato[off-list ref] wrote:

quoted

If an SSL client certificate is enabled (via http.sslcert or
GIT_SSL_CERT), prompt for the certificate password rather than
defaulting to OpenSSL's password prompt.  This causes the prompt to only
appear once each run.  Previously, OpenSSL prompted the user *many*
times, causing git to be unusable over HTTPS with client-side
certificates.

Note that the password is stored in memory in the clear while the
program is running.  This may be a security problem if git crashes and
core dumps.

The user is always prompted, even if the certificate is not encrypted.
This should be fine; unencrypted certificates are rare and a security
risk anyway.

Signed-off-by: Mark Lodato <redacted>
---

See http://osdir.com/ml/git/2009-02/msg03402.html for a discussion of
this topic and an example showing how horrible the current password
prompts are.

The next patch adds an option to disable this feature.  I split it into
two commits in case the configuration option is not wanted.

I did not create any tests because the existing http.sslcert option has
no tests to begin with.

I would really like to use git over HTTPS with client certs, but the
current situation is just unusable.  So, I'm hoping this gets included
in git.git at some point.  I would be happy to hear any comments people
have about this patch series.  Thanks!


 http.c |   40 +++++++++++++++++++++++++++++++++++++++-
 1 files changed, 39 insertions(+), 1 deletions(-)

diff --git a/http.c b/http.c
index 2e3d649..1fc3444 100644
--- a/http.c
+++ b/http.c

@@ -26,6 +26,8 @@ static long curl_low_speed_time = -1;

 static int curl_ftp_no_epsv;
 static const char *curl_http_proxy;
 static char *user_name, *user_pass;
+static char *ssl_cert_password;
+static int ssl_cert_password_required;

 static struct curl_slist *pragma_header;

@@ -167,6 +169,22 @@ static void init_curl_http_auth(CURL *result)

       }
 }

+static int has_cert_password(void)
+{
+       if (ssl_cert_password != NULL)
+               return 1;
+       if (ssl_cert == NULL || ssl_cert_password_required != 1)
+               return 0;
+       /* Only prompt the user once. */
+       ssl_cert_password_required = -1;
+       ssl_cert_password = getpass("Certificate Password: ");
+       if (ssl_cert_password != NULL) {
+               ssl_cert_password = xstrdup(ssl_cert_password);
+               return 1;
+       } else
+               return 0;
+}
+
 static CURL *get_curl_handle(void)
 {
       CURL *result = curl_easy_init();

@@ -189,6 +207,16 @@ static CURL *get_curl_handle(void)

       if (ssl_cert != NULL)
               curl_easy_setopt(result, CURLOPT_SSLCERT, ssl_cert);
+       if (has_cert_password())
+               curl_easy_setopt(result,
+#if LIBCURL_VERSION_NUM >= 0x071700
+                                CURLOPT_KEYPASSWD,
+#elif LIBCURL_VERSION_NUM >= 0x070903
+                                CURLOPT_SSLKEYPASSWD,
+#else
+                                CURLOPT_SSLCERTPASSWD,
+#endif
+                                ssl_cert_password);
 #if LIBCURL_VERSION_NUM >= 0x070902
       if (ssl_key != NULL)
               curl_easy_setopt(result, CURLOPT_SSLKEY, ssl_key);

@@ -329,8 +357,11 @@ void http_init(struct remote *remote)

       if (getenv("GIT_CURL_FTP_NO_EPSV"))
               curl_ftp_no_epsv = 1;

-       if (remote && remote->url && remote->url[0])
+       if (remote && remote->url && remote->url[0]) {
               http_auth_init(remote->url[0]);
+               if (!prefixcmp(remote->url[0], "https://"))
+                       ssl_cert_password_required = 1;
+       }

 #ifndef NO_CURL_EASY_DUPHANDLE
       curl_default = get_curl_handle();

@@ -370,6 +401,13 @@ void http_cleanup(void)

               free((void *)curl_http_proxy);
               curl_http_proxy = NULL;
       }
+
+       if (ssl_cert_password != NULL) {
+               memset(ssl_cert_password, 0, strlen(ssl_cert_password));
+               free(ssl_cert_password);
+               ssl_cert_password = NULL;
+       }
+       ssl_cert_password_required = 0;
 }

 struct active_request_slot *get_active_slot(void)
--
1.6.3.1

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

On Fri, Jun 5, 2009 at 4:20 AM, Constantine
Plotnikov[off-list ref] wrote:

How it works if git is run from IDEs (no tty will be available)?

Then this will be no worse than the current situation, which also uses
standard input to prompt for the password.  Note that a TTY is also
required if an HTTP password is requested.

Is there a way to redefine the way the password is got?

No.  This may be nice, but it would be much more complicated to implement.

What about scripting scenarios where passwordless certificates are
likely to be used?

If you wish to use a client certificate without a password, then you
need the second patch in this series, which adds an option to disable
the password prompt.


Thanks for your input,
Mark

Any other thoughts, one way or the other?  Adding proper SSL/PKI
support would really help git adoption in the corporate world.  I am
willing to make any changes necessary to get this into git.git.

~ Mark

On Wed, May 27, 2009 at 11:16 PM, Mark Lodato[off-list ref] wrote:

Quoting Mark Lodato [off-list ref]:

Any other thoughts, one way or the other?  Adding proper SSL/PKI
support would really help git adoption in the corporate world.  I am
willing to make any changes necessary to get this into git.git.

Somebody mentioned that your patch forces people to type password even when the certificate isn't encrypted. How was this issue addressed?

It would be ideal if you can inspect the certificate and decide if you need to ask for decrypting password before using it (and otherwise you don't ask). If you can't do that, probably you can introduce a config var that says "this certificate is encrypted", and bypass your new code if that config var isn't set.

That way, people who are used to the old behavior don't have to change anything in their set-up.

If people didn't have to type password at all, and after your patch if they are forced to do something else to keep the old set-up working, that isn't nice.

-- 
Nanako Shiraishi
http://ivory.ap.teacup.com/nanako3/

On Fri, 12 Jun 2009, Nanako Shiraishi wrote:

It would be ideal if you can inspect the certificate and decide if you need 
to ask for decrypting password before using it (and otherwise you don't 
ask). If you can't do that, probably you can introduce a config var that 
says "this certificate is encrypted", and bypass your new code if that 
config var isn't set.

Is this really a common setup? Using an unencrypted private key sounds like a 
really bad security situation to me. The certificate is never encrupted, the 
passphrase is for the key.

And for the libcurl not supporting this, I figure it _could_ be done by simply 
letting libcurl prope the remote and see if it can access it without a 
passphrase as that would then imply that isn't necessary.

I'm not familiar enough with the code and architecture to deem how suitable 
such an action would be.

-- 

  / daniel.haxx.se

On Fri, Jun 12, 2009 at 11:56 AM, Daniel Stenberg[off-list ref] wrote:

On Fri, 12 Jun 2009, Nanako Shiraishi wrote:

quoted

It would be ideal if you can inspect the certificate and decide if you
need to ask for decrypting password before using it (and otherwise you don't
ask). If you can't do that, probably you can introduce a config var that
says "this certificate is encrypted", and bypass your new code if that
config var isn't set.

Is this really a common setup? Using an unencrypted private key sounds like
a really bad security situation to me. The certificate is never encrupted,
the passphrase is for the key.

For SSH using unencrypted private key is very common for scripting and
cron jobs. For HTTPS situation looks like being worse since there is
no analog of ssh-agent that covers at least some of scripting
scenarios. Do we want to disable scripting for HTTPS?

Constantine

Constantine Plotnikov [off-list ref] writes:

On Fri, Jun 12, 2009 at 11:56 AM, Daniel Stenberg[off-list ref] wrote:

quoted

On Fri, 12 Jun 2009, Nanako Shiraishi wrote:

quoted

It would be ideal if you can inspect the certificate and decide if you
need to ask for decrypting password before using it (and otherwise you don't
ask). If you can't do that, probably you can introduce a config var that
says "this certificate is encrypted", and bypass your new code if that
config var isn't set.

Is this really a common setup? Using an unencrypted private key sounds like
a really bad security situation to me. The certificate is never encrupted,
the passphrase is for the key.

For SSH using unencrypted private key is very common for scripting and
cron jobs. For HTTPS situation looks like being worse since there is
no analog of ssh-agent that covers at least some of scripting
scenarios. Do we want to disable scripting for HTTPS?

Actually you can use _encrypted_ private keys together with ssh-agent
and for example keychain helper for scripting.  You have to provide
password to all listed private keys only once at login.  I wonder if
something like this would be possible for HTTP certificates...

-- 
Jakub Narebski
Poland
ShadeHawk on #git

Jakub Narebski wrote:

quoted

For SSH using unencrypted private key is very common for scripting and
cron jobs. For HTTPS situation looks like being worse since there is
no analog of ssh-agent that covers at least some of scripting
scenarios. Do we want to disable scripting for HTTPS?

Actually you can use _encrypted_ private keys together with ssh-agent
and for example keychain helper for scripting.  You have to provide
password to all listed private keys only once at login.  I wonder if
something like this would be possible for HTTP certificates...

I wonder if it might be possible using a PKCS#11 interface?

e.g. there are various "software" PKCS#11 implementations
(<http://trac.opendnssec.org/wiki/SoftHSM> springs to mind).

If you store your keys in the PKCS#11 store, and unlock them prior to
calling git, then the OpenSSL library might be able to access them
without a passphrase. Locking the PKCS#11 store would then secure the keys.

A little cumbersome, but possibly workable.

Rogan

On Fri, Jun 12, 2009 at 12:50 PM, Jakub Narebski[off-list ref] wrote:

Constantine Plotnikov [off-list ref] writes:

quoted

On Fri, Jun 12, 2009 at 11:56 AM, Daniel Stenberg[off-list ref] wrote:

quoted

On Fri, 12 Jun 2009, Nanako Shiraishi wrote:

quoted

It would be ideal if you can inspect the certificate and decide if you
need to ask for decrypting password before using it (and otherwise you don't
ask). If you can't do that, probably you can introduce a config var that
says "this certificate is encrypted", and bypass your new code if that
config var isn't set.

Is this really a common setup? Using an unencrypted private key sounds like
a really bad security situation to me. The certificate is never encrupted,
the passphrase is for the key.

For SSH using unencrypted private key is very common for scripting and
cron jobs. For HTTPS situation looks like being worse since there is
no analog of ssh-agent that covers at least some of scripting
scenarios. Do we want to disable scripting for HTTPS?

Actually you can use _encrypted_ private keys together with ssh-agent
and for example keychain helper for scripting.  You have to provide
password to all listed private keys only once at login.  I wonder if
something like this would be possible for HTTP certificates...

I would love something like this - it would be useful for SVN as well.

On Fri, Jun 12, 2009 at 3:56 AM, Daniel Stenberg[off-list ref] wrote:

On Fri, 12 Jun 2009, Nanako Shiraishi wrote:

quoted

It would be ideal if you can inspect the certificate and decide if you
need to ask for decrypting password before using it (and otherwise you don't
ask). If you can't do that, probably you can introduce a config var that
says "this certificate is encrypted", and bypass your new code if that
config var isn't set.

Is this really a common setup? Using an unencrypted private key sounds like
a really bad security situation to me. The certificate is never encrupted,
the passphrase is for the key.

And for the libcurl not supporting this, I figure it _could_ be done by
simply letting libcurl prope the remote and see if it can access it without
a passphrase as that would then imply that isn't necessary.

I'm not familiar enough with the code and architecture to deem how suitable
such an action would be.

I don't think it is possible to check to see if it is encrypted from
within git (without calling OpenSSL directly.)  To implement this in
libcurl, a possible solution is to always set
SSL_CTX_set_default_passwd_cb(), and have the callback function prompt
the user on the first call if CURLOPT_KEYPASSWD is not set.  If there
is interest, I could try this out and, if it works, submit a patch for
libcurl.

The upside of doing the prompting in git is that it works with old
libcurl versions... but I'm not sure this is a big deal.  Having it in
libcurl is probably better.


On Thu, Jun 11, 2009 at 7:42 PM, Nanako Shiraishi[off-list ref] wrote:

Somebody mentioned that your patch forces people to type password
even when the certificate isn't encrypted. How was this issue addressed?

<snip...> If you can't do that, probably you can introduce a config var that says
"this certificate is encrypted", and bypass your new code if that config var isn't set.

Patch 2/2 gives the user a way to disable this new password prompt.  I
imagine it is a more common for the certificate to be encrypted than
not, so I believe the default should be to prompt.


Mark