{"thread":{"id":"19546","subject":"[PATCH 1/2] http.c: prompt for SSL client certificate password","startedAt":"2009-05-28T03:16:02Z","lastAt":"2009-06-13T11:22:49Z","messageCount":25,"participants":["Mark Lodato","Constantine Plotnikov","Nanako Shiraishi","Junio C Hamano","Daniel Stenberg","Jakub Narebski","Rogan Dawes"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"114873","messageId":"1243480563-5954-1-git-send-email-lodatom@gmail.com","threadId":"19546","inReplyTo":null,"subject":"[PATCH 1/2] http.c: prompt for SSL client certificate password","fromName":"Mark Lodato","fromEmail":"lodatom@gmail.com","sentAt":"2009-05-28T03:16:02Z","receivedAt":"2009-05-28T03:16:02Z","isPatch":true,"sender":{"key":"lodatom@gmail.com","avatar":"https://avatars.githubusercontent.com/u/58860?v=4"},"body":"If an SSL client certificate is enabled (via http.sslcert or\nGIT_SSL_CERT), prompt for the certificate password rather than\ndefaulting to OpenSSL's password prompt.  This causes the prompt to only\nappear once each run.  Previously, OpenSSL prompted the user *many*\ntimes, causing git to be unusable over HTTPS with client-side\ncertificates.\n\nNote that the password is stored in memory in the clear while the\nprogram is running.  This may be a security problem if git crashes and\ncore dumps.\n\nThe user is always prompted, even if the certificate is not encrypted.\nThis should be fine; unencrypted certificates are rare and a security\nrisk anyway.\n\nSigned-off-by: Mark Lodato <lodatom@gmail.com>\n---\n\nSee http://osdir.com/ml/git/2009-02/msg03402.html for a discussion of\nthis topic and an example showing how horrible the current password\nprompts are.\n\nThe next patch adds an option to disable this feature.  I split it into\ntwo commits in case the configuration option is not wanted.\n\nI did not create any tests because the existing http.sslcert option has\nno tests to begin with.\n\nI would really like to use git over HTTPS with client certs, but the\ncurrent situation is just unusable.  So, I'm hoping this gets included\nin git.git at some point.  I would be happy to hear any comments people\nhave about this patch series.  Thanks!\n\n\n http.c |   40 +++++++++++++++++++++++++++++++++++++++-\n 1 files changed, 39 insertions(+), 1 deletions(-)\n\ndiff --git a/http.c b/http.c\nindex 2e3d649..1fc3444 100644\n--- a/http.c\n+++ b/http.c\n@@ -26,6 +26,8 @@ static long curl_low_speed_time = -1;\n static int curl_ftp_no_epsv;\n static const char *curl_http_proxy;\n static char *user_name, *user_pass;\n+static char *ssl_cert_password;\n+static int ssl_cert_password_required;\n \n static struct curl_slist *pragma_header;\n \n@@ -167,6 +169,22 @@ static void init_curl_http_auth(CURL *result)\n \t}\n }\n \n+static int has_cert_password(void)\n+{\n+\tif (ssl_cert_password != NULL)\n+\t\treturn 1;\n+\tif (ssl_cert == NULL || ssl_cert_password_required != 1)\n+\t\treturn 0;\n+\t/* Only prompt the user once. */\n+\tssl_cert_password_required = -1;\n+\tssl_cert_password = getpass(\"Certificate Password: \");\n+\tif (ssl_cert_password != NULL) {\n+\t\tssl_cert_password = xstrdup(ssl_cert_password);\n+\t\treturn 1;\n+\t} else\n+\t\treturn 0;\n+}\n+\n static CURL *get_curl_handle(void)\n {\n \tCURL *result = curl_easy_init();\n@@ -189,6 +207,16 @@ static CURL *get_curl_handle(void)\n \n \tif (ssl_cert != NULL)\n \t\tcurl_easy_setopt(result, CURLOPT_SSLCERT, ssl_cert);\n+\tif (has_cert_password())\n+\t\tcurl_easy_setopt(result,\n+#if LIBCURL_VERSION_NUM >= 0x071700\n+\t\t\t\t CURLOPT_KEYPASSWD,\n+#elif LIBCURL_VERSION_NUM >= 0x070903\n+\t\t\t\t CURLOPT_SSLKEYPASSWD,\n+#else\n+\t\t\t\t CURLOPT_SSLCERTPASSWD,\n+#endif\n+\t\t\t\t ssl_cert_password);\n #if LIBCURL_VERSION_NUM >= 0x070902\n \tif (ssl_key != NULL)\n \t\tcurl_easy_setopt(result, CURLOPT_SSLKEY, ssl_key);\n@@ -329,8 +357,11 @@ void http_init(struct remote *remote)\n \tif (getenv(\"GIT_CURL_FTP_NO_EPSV\"))\n \t\tcurl_ftp_no_epsv = 1;\n \n-\tif (remote && remote->url && remote->url[0])\n+\tif (remote && remote->url && remote->url[0]) {\n \t\thttp_auth_init(remote->url[0]);\n+\t\tif (!prefixcmp(remote->url[0], \"https://\"))\n+\t\t\tssl_cert_password_required = 1;\n+\t}\n \n #ifndef NO_CURL_EASY_DUPHANDLE\n \tcurl_default = get_curl_handle();\n@@ -370,6 +401,13 @@ void http_cleanup(void)\n \t\tfree((void *)curl_http_proxy);\n \t\tcurl_http_proxy = NULL;\n \t}\n+\n+\tif (ssl_cert_password != NULL) {\n+\t\tmemset(ssl_cert_password, 0, strlen(ssl_cert_password));\n+\t\tfree(ssl_cert_password);\n+\t\tssl_cert_password = NULL;\n+\t}\n+\tssl_cert_password_required = 0;\n }\n \n struct active_request_slot *get_active_slot(void)\n-- \n1.6.3.1\n"},{"id":"114874","messageId":"1243480563-5954-2-git-send-email-lodatom@gmail.com","threadId":"19546","inReplyTo":"1243480563-5954-1-git-send-email-lodatom@gmail.com","subject":"[PATCH 2/2] http.c: add http.sslCertNoPass option","fromName":"Mark Lodato","fromEmail":"lodatom@gmail.com","sentAt":"2009-05-28T03:16:03Z","receivedAt":"2009-05-28T03:16:03Z","isPatch":true,"sender":{"key":"lodatom@gmail.com","avatar":"https://avatars.githubusercontent.com/u/58860?v=4"},"body":"Add a configuration option, http.sslCertNoPass, and associated\nenvironment variable, GIT_SSL_CERT_NO_PASS, to allow disabling of the\nSSL client certificate password prompt from within git.  If this option\nis set to true, or if the environment variable exists, git falls back to\nOpenSSL's prompts (as in earlier versions of git).\n\nThis option is useful in (at least) two cases:\n1. The certificate is not encrypted and the user does not want to be\n   prompted needlessly.\n2. The user does not wish to leave the password in the clear in git's\n   (and libcurl's) memory, in case the program crashes and core dumps.\n\nThe environment variable may only be used to disable, not to re-enable,\ngit's password prompt.  This behavior mimics GIT_NO_VERIFY; the mere\nexistence of the variable is all that is checked.\n\nSigned-off-by: Mark Lodato <lodatom@gmail.com>\n---\n Documentation/config.txt |    9 +++++++++\n http.c                   |    9 ++++++++-\n 2 files changed, 17 insertions(+), 1 deletions(-)\n\ndiff --git a/Documentation/config.txt b/Documentation/config.txt\nindex 2c03162..65c3ac5 100644\n--- a/Documentation/config.txt\n+++ b/Documentation/config.txt\n@@ -1038,6 +1038,15 @@ http.sslKey::\n \tover HTTPS. Can be overridden by the 'GIT_SSL_KEY' environment\n \tvariable.\n \n+http.sslCertNoPass::\n+\tDisable git's password prompt for the SSL certificate.  OpenSSL\n+\twill still prompt the user, possibly many times, if the\n+\tcertificate or private key is encrypted.  Useful if the\n+\tcertificate is not encrypted (to disable the password prompt) or\n+\tif you do not wish to store the certificate password in git's\n+\tmemory.  Can be overridden by the 'GIT_SSL_CERT_NO_PASS'\n+\tenvironment variable.\n+\n http.sslCAInfo::\n \tFile containing the certificates to verify the peer with when\n \tfetching or pushing over HTTPS. Can be overridden by the\ndiff --git a/http.c b/http.c\nindex 1fc3444..6ae59b6 100644\n--- a/http.c\n+++ b/http.c\n@@ -131,6 +131,11 @@ static int http_options(const char *var, const char *value, void *cb)\n #endif\n \tif (!strcmp(\"http.sslcainfo\", var))\n \t\treturn git_config_string(&ssl_cainfo, var, value);\n+\tif (!strcmp(\"http.sslcertnopass\", var)) {\n+\t\tif (git_config_bool(var, value))\n+\t\t\tssl_cert_password_required = -1;\n+\t\treturn 0;\n+\t}\n #ifdef USE_CURL_MULTI\n \tif (!strcmp(\"http.maxrequests\", var)) {\n \t\tmax_requests = git_config_int(var, value);\n@@ -359,7 +364,9 @@ void http_init(struct remote *remote)\n \n \tif (remote && remote->url && remote->url[0]) {\n \t\thttp_auth_init(remote->url[0]);\n-\t\tif (!prefixcmp(remote->url[0], \"https://\"))\n+\t\tif (ssl_cert_password_required == 0 &&\n+\t\t    !getenv(\"GIT_SSL_CERT_NO_PASS\") &&\n+\t\t    !prefixcmp(remote->url[0], \"https://\"))\n \t\t\tssl_cert_password_required = 1;\n \t}\n \n-- \n1.6.3.1\n"},{"id":"115466","messageId":"ca433830906041944s1a2b12en36eb88b23cb93a7c@mail.gmail.com","threadId":"19546","inReplyTo":"1243480563-5954-1-git-send-email-lodatom@gmail.com","subject":"Re: [PATCH 1/2] http.c: prompt for SSL client certificate password","fromName":"Mark Lodato","fromEmail":"lodatom@gmail.com","sentAt":"2009-06-05T02:44:58Z","receivedAt":"2009-06-05T02:44:58Z","isPatch":true,"sender":{"key":"lodatom@gmail.com","avatar":"https://avatars.githubusercontent.com/u/58860?v=4"},"body":"Any thoughts on this?  I would love to see this in git 1.6.4, and I\ndon't think it affects people who do not use certificates.\n\n~ Mark\n\nOn Wed, May 27, 2009 at 11:16 PM, Mark Lodato<lodatom@gmail.com> wrote:\n> If an SSL client certificate is enabled (via http.sslcert or\n> GIT_SSL_CERT), prompt for the certificate password rather than\n> defaulting to OpenSSL's password prompt.  This causes the prompt to only\n> appear once each run.  Previously, OpenSSL prompted the user *many*\n> times, causing git to be unusable over HTTPS with client-side\n> certificates.\n>\n> Note that the password is stored in memory in the clear while the\n> program is running.  This may be a security problem if git crashes and\n> core dumps.\n>\n> The user is always prompted, even if the certificate is not encrypted.\n> This should be fine; unencrypted certificates are rare and a security\n> risk anyway.\n>\n> Signed-off-by: Mark Lodato <lodatom@gmail.com>\n> ---\n>\n> See http://osdir.com/ml/git/2009-02/msg03402.html for a discussion of\n> this topic and an example showing how horrible the current password\n> prompts are.\n>\n> The next patch adds an option to disable this feature.  I split it into\n> two commits in case the configuration option is not wanted.\n>\n> I did not create any tests because the existing http.sslcert option has\n> no tests to begin with.\n>\n> I would really like to use git over HTTPS with client certs, but the\n> current situation is just unusable.  So, I'm hoping this gets included\n> in git.git at some point.  I would be happy to hear any comments people\n> have about this patch series.  Thanks!\n>\n>\n>  http.c |   40 +++++++++++++++++++++++++++++++++++++++-\n>  1 files changed, 39 insertions(+), 1 deletions(-)\n>\n> diff --git a/http.c b/http.c\n> index 2e3d649..1fc3444 100644\n> --- a/http.c\n> +++ b/http.c\n> @@ -26,6 +26,8 @@ static long curl_low_speed_time = -1;\n>  static int curl_ftp_no_epsv;\n>  static const char *curl_http_proxy;\n>  static char *user_name, *user_pass;\n> +static char *ssl_cert_password;\n> +static int ssl_cert_password_required;\n>\n>  static struct curl_slist *pragma_header;\n>\n> @@ -167,6 +169,22 @@ static void init_curl_http_auth(CURL *result)\n>        }\n>  }\n>\n> +static int has_cert_password(void)\n> +{\n> +       if (ssl_cert_password != NULL)\n> +               return 1;\n> +       if (ssl_cert == NULL || ssl_cert_password_required != 1)\n> +               return 0;\n> +       /* Only prompt the user once. */\n> +       ssl_cert_password_required = -1;\n> +       ssl_cert_password = getpass(\"Certificate Password: \");\n> +       if (ssl_cert_password != NULL) {\n> +               ssl_cert_password = xstrdup(ssl_cert_password);\n> +               return 1;\n> +       } else\n> +               return 0;\n> +}\n> +\n>  static CURL *get_curl_handle(void)\n>  {\n>        CURL *result = curl_easy_init();\n> @@ -189,6 +207,16 @@ static CURL *get_curl_handle(void)\n>\n>        if (ssl_cert != NULL)\n>                curl_easy_setopt(result, CURLOPT_SSLCERT, ssl_cert);\n> +       if (has_cert_password())\n> +               curl_easy_setopt(result,\n> +#if LIBCURL_VERSION_NUM >= 0x071700\n> +                                CURLOPT_KEYPASSWD,\n> +#elif LIBCURL_VERSION_NUM >= 0x070903\n> +                                CURLOPT_SSLKEYPASSWD,\n> +#else\n> +                                CURLOPT_SSLCERTPASSWD,\n> +#endif\n> +                                ssl_cert_password);\n>  #if LIBCURL_VERSION_NUM >= 0x070902\n>        if (ssl_key != NULL)\n>                curl_easy_setopt(result, CURLOPT_SSLKEY, ssl_key);\n> @@ -329,8 +357,11 @@ void http_init(struct remote *remote)\n>        if (getenv(\"GIT_CURL_FTP_NO_EPSV\"))\n>                curl_ftp_no_epsv = 1;\n>\n> -       if (remote && remote->url && remote->url[0])\n> +       if (remote && remote->url && remote->url[0]) {\n>                http_auth_init(remote->url[0]);\n> +               if (!prefixcmp(remote->url[0], \"https://\"))\n> +                       ssl_cert_password_required = 1;\n> +       }\n>\n>  #ifndef NO_CURL_EASY_DUPHANDLE\n>        curl_default = get_curl_handle();\n> @@ -370,6 +401,13 @@ void http_cleanup(void)\n>                free((void *)curl_http_proxy);\n>                curl_http_proxy = NULL;\n>        }\n> +\n> +       if (ssl_cert_password != NULL) {\n> +               memset(ssl_cert_password, 0, strlen(ssl_cert_password));\n> +               free(ssl_cert_password);\n> +               ssl_cert_password = NULL;\n> +       }\n> +       ssl_cert_password_required = 0;\n>  }\n>\n>  struct active_request_slot *get_active_slot(void)\n> --\n> 1.6.3.1\n>\n>\n"},{"id":"115493","messageId":"85647ef50906050120p6dd65b61g9e82b5c14b098246@mail.gmail.com","threadId":"19546","inReplyTo":"ca433830906041944s1a2b12en36eb88b23cb93a7c@mail.gmail.com","subject":"Re: [PATCH 1/2] http.c: prompt for SSL client certificate password","fromName":"Constantine Plotnikov","fromEmail":"constantine.plotnikov@gmail.com","sentAt":"2009-06-05T08:20:07Z","receivedAt":"2009-06-05T08:20:07Z","isPatch":true,"sender":{"key":"constantine.plotnikov@gmail.com","avatar":null},"body":"How it works if git is run from IDEs (no tty will be available)?\nIs there a way to redefine the way the password is got?\nWhat about scripting scenarios where passwordless certificates are\nlikely to be used?\n\nRegards,\nConstantine\n\nOn Fri, Jun 5, 2009 at 6:44 AM, Mark Lodato <lodatom@gmail.com> wrote:\n> Any thoughts on this?  I would love to see this in git 1.6.4, and I\n> don't think it affects people who do not use certificates.\n>\n> ~ Mark\n>\n> On Wed, May 27, 2009 at 11:16 PM, Mark Lodato<lodatom@gmail.com> wrote:\n>> If an SSL client certificate is enabled (via http.sslcert or\n>> GIT_SSL_CERT), prompt for the certificate password rather than\n>> defaulting to OpenSSL's password prompt.  This causes the prompt to only\n>> appear once each run.  Previously, OpenSSL prompted the user *many*\n>> times, causing git to be unusable over HTTPS with client-side\n>> certificates.\n>>\n>> Note that the password is stored in memory in the clear while the\n>> program is running.  This may be a security problem if git crashes and\n>> core dumps.\n>>\n>> The user is always prompted, even if the certificate is not encrypted.\n>> This should be fine; unencrypted certificates are rare and a security\n>> risk anyway.\n>>\n>> Signed-off-by: Mark Lodato <lodatom@gmail.com>\n>> ---\n>>\n>> See http://osdir.com/ml/git/2009-02/msg03402.html for a discussion of\n>> this topic and an example showing how horrible the current password\n>> prompts are.\n>>\n>> The next patch adds an option to disable this feature.  I split it into\n>> two commits in case the configuration option is not wanted.\n>>\n>> I did not create any tests because the existing http.sslcert option has\n>> no tests to begin with.\n>>\n>> I would really like to use git over HTTPS with client certs, but the\n>> current situation is just unusable.  So, I'm hoping this gets included\n>> in git.git at some point.  I would be happy to hear any comments people\n>> have about this patch series.  Thanks!\n>>\n>>\n>>  http.c |   40 +++++++++++++++++++++++++++++++++++++++-\n>>  1 files changed, 39 insertions(+), 1 deletions(-)\n>>\n>> diff --git a/http.c b/http.c\n>> index 2e3d649..1fc3444 100644\n>> --- a/http.c\n>> +++ b/http.c\n>> @@ -26,6 +26,8 @@ static long curl_low_speed_time = -1;\n>>  static int curl_ftp_no_epsv;\n>>  static const char *curl_http_proxy;\n>>  static char *user_name, *user_pass;\n>> +static char *ssl_cert_password;\n>> +static int ssl_cert_password_required;\n>>\n>>  static struct curl_slist *pragma_header;\n>>\n>> @@ -167,6 +169,22 @@ static void init_curl_http_auth(CURL *result)\n>>        }\n>>  }\n>>\n>> +static int has_cert_password(void)\n>> +{\n>> +       if (ssl_cert_password != NULL)\n>> +               return 1;\n>> +       if (ssl_cert == NULL || ssl_cert_password_required != 1)\n>> +               return 0;\n>> +       /* Only prompt the user once. */\n>> +       ssl_cert_password_required = -1;\n>> +       ssl_cert_password = getpass(\"Certificate Password: \");\n>> +       if (ssl_cert_password != NULL) {\n>> +               ssl_cert_password = xstrdup(ssl_cert_password);\n>> +               return 1;\n>> +       } else\n>> +               return 0;\n>> +}\n>> +\n>>  static CURL *get_curl_handle(void)\n>>  {\n>>        CURL *result = curl_easy_init();\n>> @@ -189,6 +207,16 @@ static CURL *get_curl_handle(void)\n>>\n>>        if (ssl_cert != NULL)\n>>                curl_easy_setopt(result, CURLOPT_SSLCERT, ssl_cert);\n>> +       if (has_cert_password())\n>> +               curl_easy_setopt(result,\n>> +#if LIBCURL_VERSION_NUM >= 0x071700\n>> +                                CURLOPT_KEYPASSWD,\n>> +#elif LIBCURL_VERSION_NUM >= 0x070903\n>> +                                CURLOPT_SSLKEYPASSWD,\n>> +#else\n>> +                                CURLOPT_SSLCERTPASSWD,\n>> +#endif\n>> +                                ssl_cert_password);\n>>  #if LIBCURL_VERSION_NUM >= 0x070902\n>>        if (ssl_key != NULL)\n>>                curl_easy_setopt(result, CURLOPT_SSLKEY, ssl_key);\n>> @@ -329,8 +357,11 @@ void http_init(struct remote *remote)\n>>        if (getenv(\"GIT_CURL_FTP_NO_EPSV\"))\n>>                curl_ftp_no_epsv = 1;\n>>\n>> -       if (remote && remote->url && remote->url[0])\n>> +       if (remote && remote->url && remote->url[0]) {\n>>                http_auth_init(remote->url[0]);\n>> +               if (!prefixcmp(remote->url[0], \"https://\"))\n>> +                       ssl_cert_password_required = 1;\n>> +       }\n>>\n>>  #ifndef NO_CURL_EASY_DUPHANDLE\n>>        curl_default = get_curl_handle();\n>> @@ -370,6 +401,13 @@ void http_cleanup(void)\n>>                free((void *)curl_http_proxy);\n>>                curl_http_proxy = NULL;\n>>        }\n>> +\n>> +       if (ssl_cert_password != NULL) {\n>> +               memset(ssl_cert_password, 0, strlen(ssl_cert_password));\n>> +               free(ssl_cert_password);\n>> +               ssl_cert_password = NULL;\n>> +       }\n>> +       ssl_cert_password_required = 0;\n>>  }\n>>\n>>  struct active_request_slot *get_active_slot(void)\n>> --\n>> 1.6.3.1\n>>\n>>\n> --\n> To unsubscribe from this list: send the line \"unsubscribe git\" in\n> the body of a message to majordomo@vger.kernel.org\n> More majordomo info at  http://vger.kernel.org/majordomo-info.html\n>\n"},{"id":"115705","messageId":"ca433830906070710k61705903ydf985d198e9ea318@mail.gmail.com","threadId":"19546","inReplyTo":"85647ef50906050120p6dd65b61g9e82b5c14b098246@mail.gmail.com","subject":"Re: [PATCH 1/2] http.c: prompt for SSL client certificate password","fromName":"Mark Lodato","fromEmail":"lodatom@gmail.com","sentAt":"2009-06-07T14:10:04Z","receivedAt":"2009-06-07T14:10:04Z","isPatch":true,"sender":{"key":"lodatom@gmail.com","avatar":"https://avatars.githubusercontent.com/u/58860?v=4"},"body":"On Fri, Jun 5, 2009 at 4:20 AM, Constantine\nPlotnikov<constantine.plotnikov@gmail.com> wrote:\n> How it works if git is run from IDEs (no tty will be available)?\n\nThen this will be no worse than the current situation, which also uses\nstandard input to prompt for the password.  Note that a TTY is also\nrequired if an HTTP password is requested.\n\n> Is there a way to redefine the way the password is got?\n\nNo.  This may be nice, but it would be much more complicated to implement.\n\n> What about scripting scenarios where passwordless certificates are\n> likely to be used?\n\nIf you wish to use a client certificate without a password, then you\nneed the second patch in this series, which adds an option to disable\nthe password prompt.\n\n\nThanks for your input,\nMark\n"},{"id":"116104","messageId":"ca433830906111600n2d45b5bdg3fb6e7c0a537ec78@mail.gmail.com","threadId":"19546","inReplyTo":"1243480563-5954-1-git-send-email-lodatom@gmail.com","subject":"Re: [PATCH 1/2] http.c: prompt for SSL client certificate password","fromName":"Mark Lodato","fromEmail":"lodatom@gmail.com","sentAt":"2009-06-11T23:00:43Z","receivedAt":"2009-06-11T23:00:43Z","isPatch":true,"sender":{"key":"lodatom@gmail.com","avatar":"https://avatars.githubusercontent.com/u/58860?v=4"},"body":"Any other thoughts, one way or the other?  Adding proper SSL/PKI\nsupport would really help git adoption in the corporate world.  I am\nwilling to make any changes necessary to get this into git.git.\n\n~ Mark\n\nOn Wed, May 27, 2009 at 11:16 PM, Mark Lodato<lodatom@gmail.com> wrote:\n> If an SSL client certificate is enabled (via http.sslcert or\n> GIT_SSL_CERT), prompt for the certificate password rather than\n> defaulting to OpenSSL's password prompt.  This causes the prompt to only\n> appear once each run.  Previously, OpenSSL prompted the user *many*\n> times, causing git to be unusable over HTTPS with client-side\n> certificates.\n>\n> Note that the password is stored in memory in the clear while the\n> program is running.  This may be a security problem if git crashes and\n> core dumps.\n>\n> The user is always prompted, even if the certificate is not encrypted.\n> This should be fine; unencrypted certificates are rare and a security\n> risk anyway.\n>\n> Signed-off-by: Mark Lodato <lodatom@gmail.com>\n> ---\n>\n> See http://osdir.com/ml/git/2009-02/msg03402.html for a discussion of\n> this topic and an example showing how horrible the current password\n> prompts are.\n>\n> The next patch adds an option to disable this feature.  I split it into\n> two commits in case the configuration option is not wanted.\n>\n> I did not create any tests because the existing http.sslcert option has\n> no tests to begin with.\n>\n> I would really like to use git over HTTPS with client certs, but the\n> current situation is just unusable.  So, I'm hoping this gets included\n> in git.git at some point.  I would be happy to hear any comments people\n> have about this patch series.  Thanks!\n>\n>\n>  http.c |   40 +++++++++++++++++++++++++++++++++++++++-\n>  1 files changed, 39 insertions(+), 1 deletions(-)\n>\n> diff --git a/http.c b/http.c\n> index 2e3d649..1fc3444 100644\n> --- a/http.c\n> +++ b/http.c\n> @@ -26,6 +26,8 @@ static long curl_low_speed_time = -1;\n>  static int curl_ftp_no_epsv;\n>  static const char *curl_http_proxy;\n>  static char *user_name, *user_pass;\n> +static char *ssl_cert_password;\n> +static int ssl_cert_password_required;\n>\n>  static struct curl_slist *pragma_header;\n>\n> @@ -167,6 +169,22 @@ static void init_curl_http_auth(CURL *result)\n>        }\n>  }\n>\n> +static int has_cert_password(void)\n> +{\n> +       if (ssl_cert_password != NULL)\n> +               return 1;\n> +       if (ssl_cert == NULL || ssl_cert_password_required != 1)\n> +               return 0;\n> +       /* Only prompt the user once. */\n> +       ssl_cert_password_required = -1;\n> +       ssl_cert_password = getpass(\"Certificate Password: \");\n> +       if (ssl_cert_password != NULL) {\n> +               ssl_cert_password = xstrdup(ssl_cert_password);\n> +               return 1;\n> +       } else\n> +               return 0;\n> +}\n> +\n>  static CURL *get_curl_handle(void)\n>  {\n>        CURL *result = curl_easy_init();\n> @@ -189,6 +207,16 @@ static CURL *get_curl_handle(void)\n>\n>        if (ssl_cert != NULL)\n>                curl_easy_setopt(result, CURLOPT_SSLCERT, ssl_cert);\n> +       if (has_cert_password())\n> +               curl_easy_setopt(result,\n> +#if LIBCURL_VERSION_NUM >= 0x071700\n> +                                CURLOPT_KEYPASSWD,\n> +#elif LIBCURL_VERSION_NUM >= 0x070903\n> +                                CURLOPT_SSLKEYPASSWD,\n> +#else\n> +                                CURLOPT_SSLCERTPASSWD,\n> +#endif\n> +                                ssl_cert_password);\n>  #if LIBCURL_VERSION_NUM >= 0x070902\n>        if (ssl_key != NULL)\n>                curl_easy_setopt(result, CURLOPT_SSLKEY, ssl_key);\n> @@ -329,8 +357,11 @@ void http_init(struct remote *remote)\n>        if (getenv(\"GIT_CURL_FTP_NO_EPSV\"))\n>                curl_ftp_no_epsv = 1;\n>\n> -       if (remote && remote->url && remote->url[0])\n> +       if (remote && remote->url && remote->url[0]) {\n>                http_auth_init(remote->url[0]);\n> +               if (!prefixcmp(remote->url[0], \"https://\"))\n> +                       ssl_cert_password_required = 1;\n> +       }\n>\n>  #ifndef NO_CURL_EASY_DUPHANDLE\n>        curl_default = get_curl_handle();\n> @@ -370,6 +401,13 @@ void http_cleanup(void)\n>                free((void *)curl_http_proxy);\n>                curl_http_proxy = NULL;\n>        }\n> +\n> +       if (ssl_cert_password != NULL) {\n> +               memset(ssl_cert_password, 0, strlen(ssl_cert_password));\n> +               free(ssl_cert_password);\n> +               ssl_cert_password = NULL;\n> +       }\n> +       ssl_cert_password_required = 0;\n>  }\n>\n>  struct active_request_slot *get_active_slot(void)\n> --\n> 1.6.3.1\n>\n>\n"},{"id":"116108","messageId":"20090612084209.6117@nanako3.lavabit.com","threadId":"19546","inReplyTo":"ca433830906111600n2d45b5bdg3fb6e7c0a537ec78@mail.gmail.com","subject":"Re: [PATCH 1/2] http.c: prompt for SSL client certificate password","fromName":"Nanako Shiraishi","fromEmail":"nanako3@lavabit.com","sentAt":"2009-06-11T23:42:09Z","receivedAt":"2009-06-11T23:42:09Z","isPatch":true,"sender":{"key":"nanako3@lavabit.com","avatar":"https://gravatar.com/avatar/3777b9e201c5883a62b1a6fdf7c53f2d712d1d80989146063ea861e33aad72a8?d=mp&s=160"},"body":"Quoting Mark Lodato <lodatom@gmail.com>:\n\n> Any other thoughts, one way or the other?  Adding proper SSL/PKI\n> support would really help git adoption in the corporate world.  I am\n> willing to make any changes necessary to get this into git.git.\n\nSomebody mentioned that your patch forces people to type password even when the certificate isn't encrypted. How was this issue addressed?\n\nIt 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.\n\nThat way, people who are used to the old behavior don't have to change anything in their set-up.\n\nIf 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.\n\n-- \nNanako Shiraishi\nhttp://ivory.ap.teacup.com/nanako3/\n"},{"id":"116110","messageId":"7vocsue354.fsf@alter.siamese.dyndns.org","threadId":"19546","inReplyTo":"ca433830906111600n2d45b5bdg3fb6e7c0a537ec78@mail.gmail.com","subject":"Re: [PATCH 1/2] http.c: prompt for SSL client certificate password","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-06-11T23:56:07Z","receivedAt":"2009-06-11T23:56:07Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Mark Lodato <lodatom@gmail.com> writes:\n\n>> The user is always prompted, even if the certificate is not encrypted.\n>> This should be fine; unencrypted certificates are rare and a security\n>> risk anyway.\n\nHmm, \"rare\" is in the eyes of beholder.  For automated settings, I would\nimagine that it is a necessary feature that we need to keep working.  Of\ncourse the local box that keeps an unencrypted certificate used this way\nmust be well protected to make it _not_ a security risk, but that is not\nan issue you are addressing with your patch anyway, so it is not nice to\ndismiss possible usability issues like this.\n\n>> I did not create any tests because the existing http.sslcert option has\n>> no tests to begin with.\n\nAgain, not nice.  Not having tests in this particular patch may be Ok, as\nlong as you or other people fix that deficiency with follow-up patches,\nbut please don't be proud that you are following a bad example.\n"},{"id":"116111","messageId":"7viqj2e2zv.fsf@alter.siamese.dyndns.org","threadId":"19546","inReplyTo":"20090612084209.6117@nanako3.lavabit.com","subject":"Re: [PATCH 1/2] http.c: prompt for SSL client certificate password","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-06-11T23:59:16Z","receivedAt":"2009-06-11T23:59:16Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Nanako Shiraishi <nanako3@lavabit.com> writes:\n\n> It would be ideal if you can inspect the certificate and decide if you\n> need to ask for decrypting password before using it (and otherwise you\n> don't ask). If you can't do that, probably you can introduce a config\n> var that says \"this certificate is encrypted\", and bypass your new code\n> if that config var isn't set.\n\nTrue, and true.\n"},{"id":"116123","messageId":"7vprdaarka.fsf@alter.siamese.dyndns.org","threadId":"19546","inReplyTo":"1243480563-5954-1-git-send-email-lodatom@gmail.com","subject":"Re: [PATCH 1/2] http.c: prompt for SSL client certificate password","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-06-12T06:34:29Z","receivedAt":"2009-06-12T06:34:29Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Mark Lodato <lodatom@gmail.com> writes:\n\n> @@ -189,6 +207,16 @@ static CURL *get_curl_handle(void)\n>  \n>  \tif (ssl_cert != NULL)\n>  \t\tcurl_easy_setopt(result, CURLOPT_SSLCERT, ssl_cert);\n> +\tif (has_cert_password())\n> +\t\tcurl_easy_setopt(result,\n> +#if LIBCURL_VERSION_NUM >= 0x071700\n> +\t\t\t\t CURLOPT_KEYPASSWD,\n> +#elif LIBCURL_VERSION_NUM >= 0x070903\n> +\t\t\t\t CURLOPT_SSLKEYPASSWD,\n> +#else\n> +\t\t\t\t CURLOPT_SSLCERTPASSWD,\n> +#endif\n> +\t\t\t\t ssl_cert_password);\n\nThis is purely style and readability, but if you do something like this\nmuch earlier in the file:\n\n    #if !defined(CURLOPT_KEYPASSWD)\n    # if defined(CURLOPT_SSLKEYPASSWD)\n    #  define CURLOPT_KEYTPASSWD CURLOPT_SSLKEYPASSWD\n    # elif defined(CURLOPT_SSLCERTPASSWD\n    #  define CURLOPT_KEYTPASSWD CURLOPT_SSLCERTPASSWD\n    # endif\n    #endif\n\nyou can write your main codepath using the latest cURL API without ifdef.\nThe callsite can simply say:\n\n\tif (must_set_cert_password())\n        \tcurl_easy_setopt(result, CURLOPT_KEYPASSWD, ssl_cert_password);\n\nwhich I think would be much easier to follow.\n\nThis assumes that KEYPASSWD is the latest API, and in older versions only\nnames are different, which your code implies.  I have a vague recollection\nthat SSLCERTPASSWD actually deprecated KEYPASSWD (i.e. your #if...#endif\nchain is wrong), but I didn't actually check the cURL documentation [*1*]\nto see if that is the case.\n\n[Reference]\n\n*1* http://cool.haxx.se/cvs.cgi/curl/docs/libcurl/symbols-in-versions?rev=HEAD\n"},{"id":"116128","messageId":"alpine.DEB.2.00.0906120943560.5566@yvahk2.pbagnpgbe.fr","threadId":"19546","inReplyTo":"20090612084209.6117@nanako3.lavabit.com","subject":"Re: [PATCH 1/2] http.c: prompt for SSL client certificate password","fromName":"Daniel Stenberg","fromEmail":"daniel@haxx.se","sentAt":"2009-06-12T07:56:47Z","receivedAt":"2009-06-12T07:56:47Z","isPatch":true,"sender":{"key":"daniel@haxx.se","avatar":"https://gravatar.com/avatar/69fdca87edd17cee21ca2e79fc2ff671d644603c3dc27167430f3cd3dbab7ba8?d=mp&s=160"},"body":"On Fri, 12 Jun 2009, Nanako Shiraishi wrote:\n\n> It would be ideal if you can inspect the certificate and decide if you need \n> to ask for decrypting password before using it (and otherwise you don't \n> ask). If you can't do that, probably you can introduce a config var that \n> says \"this certificate is encrypted\", and bypass your new code if that \n> config var isn't set.\n\nIs this really a common setup? Using an unencrypted private key sounds like a \nreally bad security situation to me. The certificate is never encrupted, the \npassphrase is for the key.\n\nAnd for the libcurl not supporting this, I figure it _could_ be done by simply \nletting libcurl prope the remote and see if it can access it without a \npassphrase as that would then imply that isn't necessary.\n\nI'm not familiar enough with the code and architecture to deem how suitable \nsuch an action would be.\n\n-- \n\n  / daniel.haxx.se\n"},{"id":"116129","messageId":"alpine.DEB.2.00.0906120957400.5566@yvahk2.pbagnpgbe.fr","threadId":"19546","inReplyTo":"7vprdaarka.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 1/2] http.c: prompt for SSL client certificate password","fromName":"Daniel Stenberg","fromEmail":"daniel@haxx.se","sentAt":"2009-06-12T07:59:16Z","receivedAt":"2009-06-12T07:59:16Z","isPatch":true,"sender":{"key":"daniel@haxx.se","avatar":"https://gravatar.com/avatar/69fdca87edd17cee21ca2e79fc2ff671d644603c3dc27167430f3cd3dbab7ba8?d=mp&s=160"},"body":"On Thu, 11 Jun 2009, Junio C Hamano wrote:\n\n>    #if !defined(CURLOPT_KEYPASSWD)\n>    # if defined(CURLOPT_SSLKEYPASSWD)\n>    #  define CURLOPT_KEYTPASSWD CURLOPT_SSLKEYPASSWD\n>    # elif defined(CURLOPT_SSLCERTPASSWD\n>    #  define CURLOPT_KEYTPASSWD CURLOPT_SSLCERTPASSWD\n>    # endif\n>    #endif\n\nJust note that these CURLOPT_* symbols provided by libcurl are enums, not \ndefines, so unfortunately you can't do it this exact #ifdef way.\n\n-- \n\n  / daniel.haxx.se\n"},{"id":"116163","messageId":"85647ef50906120838s37c186a9mec301e880b1a8a4e@mail.gmail.com","threadId":"19546","inReplyTo":"alpine.DEB.2.00.0906120943560.5566@yvahk2.pbagnpgbe.fr","subject":"Re: [PATCH 1/2] http.c: prompt for SSL client certificate password","fromName":"Constantine Plotnikov","fromEmail":"constantine.plotnikov@gmail.com","sentAt":"2009-06-12T15:38:29Z","receivedAt":"2009-06-12T15:38:29Z","isPatch":true,"sender":{"key":"constantine.plotnikov@gmail.com","avatar":null},"body":"On Fri, Jun 12, 2009 at 11:56 AM, Daniel Stenberg<daniel@haxx.se> wrote:\n> On Fri, 12 Jun 2009, Nanako Shiraishi wrote:\n>\n>> It would be ideal if you can inspect the certificate and decide if you\n>> need to ask for decrypting password before using it (and otherwise you don't\n>> ask). If you can't do that, probably you can introduce a config var that\n>> says \"this certificate is encrypted\", and bypass your new code if that\n>> config var isn't set.\n>\n> Is this really a common setup? Using an unencrypted private key sounds like\n> a really bad security situation to me. The certificate is never encrupted,\n> the passphrase is for the key.\n>\nFor SSH using unencrypted private key is very common for scripting and\ncron jobs. For HTTPS situation looks like being worse since there is\nno analog of ssh-agent that covers at least some of scripting\nscenarios. Do we want to disable scripting for HTTPS?\n\nConstantine\n"},{"id":"116167","messageId":"m3vdn12y6y.fsf@localhost.localdomain","threadId":"19546","inReplyTo":"85647ef50906120838s37c186a9mec301e880b1a8a4e@mail.gmail.com","subject":"Re: [PATCH 1/2] http.c: prompt for SSL client certificate password","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2009-06-12T16:50:48Z","receivedAt":"2009-06-12T16:50:48Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"Constantine Plotnikov <constantine.plotnikov@gmail.com> writes:\n> On Fri, Jun 12, 2009 at 11:56 AM, Daniel Stenberg<daniel@haxx.se> wrote:\n>> On Fri, 12 Jun 2009, Nanako Shiraishi wrote:\n>>\n>>> It would be ideal if you can inspect the certificate and decide if you\n>>> need to ask for decrypting password before using it (and otherwise you don't\n>>> ask). If you can't do that, probably you can introduce a config var that\n>>> says \"this certificate is encrypted\", and bypass your new code if that\n>>> config var isn't set.\n>>\n>> Is this really a common setup? Using an unencrypted private key sounds like\n>> a really bad security situation to me. The certificate is never encrupted,\n>> the passphrase is for the key.\n>>\n> For SSH using unencrypted private key is very common for scripting and\n> cron jobs. For HTTPS situation looks like being worse since there is\n> no analog of ssh-agent that covers at least some of scripting\n> scenarios. Do we want to disable scripting for HTTPS?\n\nActually you can use _encrypted_ private keys together with ssh-agent\nand for example keychain helper for scripting.  You have to provide\npassword to all listed private keys only once at login.  I wonder if\nsomething like this would be possible for HTTP certificates...\n\n-- \nJakub Narebski\nPoland\nShadeHawk on #git\n"},{"id":"116185","messageId":"4A32CD83.1090801@dawes.za.net","threadId":"19546","inReplyTo":"m3vdn12y6y.fsf@localhost.localdomain","subject":"Re: [PATCH 1/2] http.c: prompt for SSL client certificate password","fromName":"Rogan Dawes","fromEmail":"lists@dawes.za.net","sentAt":"2009-06-12T21:49:55Z","receivedAt":"2009-06-12T21:49:55Z","isPatch":true,"sender":{"key":"lists@dawes.za.net","avatar":null},"body":"Jakub Narebski wrote:\n>> For SSH using unencrypted private key is very common for scripting and\n>> cron jobs. For HTTPS situation looks like being worse since there is\n>> no analog of ssh-agent that covers at least some of scripting\n>> scenarios. Do we want to disable scripting for HTTPS?\n> \n> Actually you can use _encrypted_ private keys together with ssh-agent\n> and for example keychain helper for scripting.  You have to provide\n> password to all listed private keys only once at login.  I wonder if\n> something like this would be possible for HTTP certificates...\n\nI wonder if it might be possible using a PKCS#11 interface?\n\ne.g. there are various \"software\" PKCS#11 implementations\n(<http://trac.opendnssec.org/wiki/SoftHSM> springs to mind).\n\nIf you store your keys in the PKCS#11 store, and unlock them prior to\ncalling git, then the OpenSSL library might be able to access them\nwithout a passphrase. Locking the PKCS#11 store would then secure the keys.\n\nA little cumbersome, but possibly workable.\n\nRogan\n"},{"id":"116191","messageId":"ca433830906121531m6955da77we455136c6e9d0785@mail.gmail.com","threadId":"19546","inReplyTo":"7vocsue354.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 1/2] http.c: prompt for SSL client certificate password","fromName":"Mark Lodato","fromEmail":"lodatom@gmail.com","sentAt":"2009-06-12T22:31:11Z","receivedAt":"2009-06-12T22:31:11Z","isPatch":true,"sender":{"key":"lodatom@gmail.com","avatar":"https://avatars.githubusercontent.com/u/58860?v=4"},"body":"Thanks for reviewing the patch.\n\nOn Thu, Jun 11, 2009 at 7:56 PM, Junio C Hamano<gitster@pobox.com> wrote:\n> Mark Lodato <lodatom@gmail.com> writes:\n>\n>>> The user is always prompted, even if the certificate is not encrypted.\n>>> This should be fine; unencrypted certificates are rare and a security\n>>> risk anyway.\n>\n> Hmm, \"rare\" is in the eyes of beholder.  For automated settings, I would\n> imagine that it is a necessary feature that we need to keep working.  Of\n> course the local box that keeps an unencrypted certificate used this way\n> must be well protected to make it _not_ a security risk, but that is not\n> an issue you are addressing with your patch anyway, so it is not nice to\n> dismiss possible usability issues like this.\n\nSorry about that wording - it probably is a more common case than I\nimagine.  But patch 2/2 addresses this issue with an option to disable\nthe password prompt.  This does require one-time work for existing\nusers who use an unencrypted certificate, but overall I think the\npatch series is a big win since encrypted certificates are not usable\nat all currently.\n\n>>> I did not create any tests because the existing http.sslcert option has\n>>> no tests to begin with.\n>\n> Again, not nice.  Not having tests in this particular patch may be Ok, as\n> long as you or other people fix that deficiency with follow-up patches,\n> but please don't be proud that you are following a bad example.\n\n\nAgain, sorry about the wording.  I meant the above as an explanation\nof why I did not include a test - I was not sure how to write one.  I\nwould be happy to write such a test if someone could give me some\nguidance.\n\n\nThanks again!\nMark\n"},{"id":"116198","messageId":"ca433830906121611g5d079908ycc714adcc30c9aa@mail.gmail.com","threadId":"19546","inReplyTo":"m3vdn12y6y.fsf@localhost.localdomain","subject":"Re: [PATCH 1/2] http.c: prompt for SSL client certificate password","fromName":"Mark Lodato","fromEmail":"lodatom@gmail.com","sentAt":"2009-06-12T23:11:36Z","receivedAt":"2009-06-12T23:11:36Z","isPatch":true,"sender":{"key":"lodatom@gmail.com","avatar":"https://avatars.githubusercontent.com/u/58860?v=4"},"body":"On Fri, Jun 12, 2009 at 12:50 PM, Jakub Narebski<jnareb@gmail.com> wrote:\n> Constantine Plotnikov <constantine.plotnikov@gmail.com> writes:\n>> On Fri, Jun 12, 2009 at 11:56 AM, Daniel Stenberg<daniel@haxx.se> wrote:\n>>> On Fri, 12 Jun 2009, Nanako Shiraishi wrote:\n>>>\n>>>> It would be ideal if you can inspect the certificate and decide if you\n>>>> need to ask for decrypting password before using it (and otherwise you don't\n>>>> ask). If you can't do that, probably you can introduce a config var that\n>>>> says \"this certificate is encrypted\", and bypass your new code if that\n>>>> config var isn't set.\n>>>\n>>> Is this really a common setup? Using an unencrypted private key sounds like\n>>> a really bad security situation to me. The certificate is never encrupted,\n>>> the passphrase is for the key.\n>>>\n>> For SSH using unencrypted private key is very common for scripting and\n>> cron jobs. For HTTPS situation looks like being worse since there is\n>> no analog of ssh-agent that covers at least some of scripting\n>> scenarios. Do we want to disable scripting for HTTPS?\n>\n> Actually you can use _encrypted_ private keys together with ssh-agent\n> and for example keychain helper for scripting.  You have to provide\n> password to all listed private keys only once at login.  I wonder if\n> something like this would be possible for HTTP certificates...\n\nI would love something like this - it would be useful for SVN as well.\n"},{"id":"116199","messageId":"ca433830906121613y68e5bdax5778867c41b00339@mail.gmail.com","threadId":"19546","inReplyTo":"7vprdaarka.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 1/2] http.c: prompt for SSL client certificate password","fromName":"Mark Lodato","fromEmail":"lodatom@gmail.com","sentAt":"2009-06-12T23:13:32Z","receivedAt":"2009-06-12T23:13:32Z","isPatch":true,"sender":{"key":"lodatom@gmail.com","avatar":"https://avatars.githubusercontent.com/u/58860?v=4"},"body":"On Fri, Jun 12, 2009 at 2:34 AM, Junio C Hamano<gitster@pobox.com> wrote:\n> Mark Lodato <lodatom@gmail.com> writes:\n>\n>> @@ -189,6 +207,16 @@ static CURL *get_curl_handle(void)\n>>\n>>       if (ssl_cert != NULL)\n>>               curl_easy_setopt(result, CURLOPT_SSLCERT, ssl_cert);\n>> +     if (has_cert_password())\n>> +             curl_easy_setopt(result,\n>> +#if LIBCURL_VERSION_NUM >= 0x071700\n>> +                              CURLOPT_KEYPASSWD,\n>> +#elif LIBCURL_VERSION_NUM >= 0x070903\n>> +                              CURLOPT_SSLKEYPASSWD,\n>> +#else\n>> +                              CURLOPT_SSLCERTPASSWD,\n>> +#endif\n>> +                              ssl_cert_password);\n>\n> This is purely style and readability, but if you do something like this\n> much earlier in the file:\n>\n>    #if !defined(CURLOPT_KEYPASSWD)\n>    # if defined(CURLOPT_SSLKEYPASSWD)\n>    #  define CURLOPT_KEYTPASSWD CURLOPT_SSLKEYPASSWD\n>    # elif defined(CURLOPT_SSLCERTPASSWD\n>    #  define CURLOPT_KEYTPASSWD CURLOPT_SSLCERTPASSWD\n>    # endif\n>    #endif\n>\n> you can write your main codepath using the latest cURL API without ifdef.\n> The callsite can simply say:\n>\n>        if (must_set_cert_password())\n>                curl_easy_setopt(result, CURLOPT_KEYPASSWD, ssl_cert_password);\n>\n> which I think would be much easier to follow.\n\nI realized this after I submitted the patch.  Locally I have modified\nmy version to do something similar to the above, but checking libcurl\nversions rather than checking the existence of the macros (which don't\nexist, as Daniel pointed out.)  If this patch series is accepted, I\nwill make a cleaner version that includes this change.\n\nMark\n"},{"id":"116202","messageId":"ca433830906121626q52c15f6cjdb91ffee1f2d8652@mail.gmail.com","threadId":"19546","inReplyTo":"alpine.DEB.2.00.0906120943560.5566@yvahk2.pbagnpgbe.fr","subject":"Re: [PATCH 1/2] http.c: prompt for SSL client certificate password","fromName":"Mark Lodato","fromEmail":"lodatom@gmail.com","sentAt":"2009-06-12T23:26:45Z","receivedAt":"2009-06-12T23:26:45Z","isPatch":true,"sender":{"key":"lodatom@gmail.com","avatar":"https://avatars.githubusercontent.com/u/58860?v=4"},"body":"On Fri, Jun 12, 2009 at 3:56 AM, Daniel Stenberg<daniel@haxx.se> wrote:\n> On Fri, 12 Jun 2009, Nanako Shiraishi wrote:\n>\n>> It would be ideal if you can inspect the certificate and decide if you\n>> need to ask for decrypting password before using it (and otherwise you don't\n>> ask). If you can't do that, probably you can introduce a config var that\n>> says \"this certificate is encrypted\", and bypass your new code if that\n>> config var isn't set.\n>\n> Is this really a common setup? Using an unencrypted private key sounds like\n> a really bad security situation to me. The certificate is never encrupted,\n> the passphrase is for the key.\n>\n> And for the libcurl not supporting this, I figure it _could_ be done by\n> simply letting libcurl prope the remote and see if it can access it without\n> a passphrase as that would then imply that isn't necessary.\n>\n> I'm not familiar enough with the code and architecture to deem how suitable\n> such an action would be.\n\nI don't think it is possible to check to see if it is encrypted from\nwithin git (without calling OpenSSL directly.)  To implement this in\nlibcurl, a possible solution is to always set\nSSL_CTX_set_default_passwd_cb(), and have the callback function prompt\nthe user on the first call if CURLOPT_KEYPASSWD is not set.  If there\nis interest, I could try this out and, if it works, submit a patch for\nlibcurl.\n\nThe upside of doing the prompting in git is that it works with old\nlibcurl versions... but I'm not sure this is a big deal.  Having it in\nlibcurl is probably better.\n\n\nOn Thu, Jun 11, 2009 at 7:42 PM, Nanako Shiraishi<nanako3@lavabit.com> wrote:\n> Somebody mentioned that your patch forces people to type password\n> even when the certificate isn't encrypted. How was this issue addressed?\n>\n> <snip...> If you can't do that, probably you can introduce a config var that says\n> \"this certificate is encrypted\", and bypass your new code if that config var isn't set.\n\nPatch 2/2 gives the user a way to disable this new password prompt.  I\nimagine it is a more common for the certificate to be encrypted than\nnot, so I believe the default should be to prompt.\n\n\nMark\n"},{"id":"116204","messageId":"7vocst3s8n.fsf@alter.siamese.dyndns.org","threadId":"19546","inReplyTo":"ca433830906121613y68e5bdax5778867c41b00339@mail.gmail.com","subject":"Re: [PATCH 1/2] http.c: prompt for SSL client certificate password","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-06-13T00:14:00Z","receivedAt":"2009-06-13T00:14:00Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Mark Lodato <lodatom@gmail.com> writes:\n\n> If this patch series is accepted, I\n> will make a cleaner version that includes this change.\n\nSorry, but I do not understand this part of your message.\n"},{"id":"116205","messageId":"7vk53h3rey.fsf@alter.siamese.dyndns.org","threadId":"19546","inReplyTo":"ca433830906121626q52c15f6cjdb91ffee1f2d8652@mail.gmail.com","subject":"Re: [PATCH 1/2] http.c: prompt for SSL client certificate password","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-06-13T00:31:49Z","receivedAt":"2009-06-13T00:31:49Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Mark Lodato <lodatom@gmail.com> writes:\n\n>> And for the libcurl not supporting this, I figure it _could_ be done by\n>> simply letting libcurl prope the remote and see if it can access it without\n>> a passphrase as that would then imply that isn't necessary.\n>>\n>> I'm not familiar enough with the code and architecture to deem how suitable\n>> such an action would be.\n>\n> I don't think it is possible to check to see if it is encrypted from\n> within git (without calling OpenSSL directly).\n\nI think what Daniel is suggesting is to attempt making a test connection\n(that does not have to have anything to do with the real object transfer)\nwithout passphrase to see if it fails.  If it doesn't, you know you do not\nneed a passphrase to unlock the key/cert.\n\nWhile I still think that kind of automated detection would be necessary in\nthe longer term (in other words, we do not necessarily have to have it in\nthe initial implementation that appears in our official release), until that\nmaterializes, I think it is more prudent to follow the approach below.\n\n>> <snip...> If you can't do that, probably you can introduce a config var that says\n>> \"this certificate is encrypted\", and bypass your new code if that config var isn't set.\n\nI think I've said this already in another message, but \"I break your\nworking setup with my patch, but you can add this configuration to unbreak\nit\" should not be done lightly, certainly without a good reason.  And the\nreason here as far as I can see is that the code chooses not to bother\nwith the autodetection of encryptedness of the cert/key.  So...\n"},{"id":"116206","messageId":"ca433830906121733w7c88dfd4w1025b7b936e48e95@mail.gmail.com","threadId":"19546","inReplyTo":"7vocst3s8n.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 1/2] http.c: prompt for SSL client certificate password","fromName":"Mark Lodato","fromEmail":"lodatom@gmail.com","sentAt":"2009-06-13T00:33:23Z","receivedAt":"2009-06-13T00:33:23Z","isPatch":true,"sender":{"key":"lodatom@gmail.com","avatar":"https://avatars.githubusercontent.com/u/58860?v=4"},"body":"On Fri, Jun 12, 2009 at 8:14 PM, Junio C Hamano<gitster@pobox.com> wrote:\n> Mark Lodato <lodatom@gmail.com> writes:\n>\n>> If this patch series is accepted, I\n>> will make a cleaner version that includes this change.\n>\n> Sorry, but I do not understand this part of your message.\n>\n\nSorry about that.  I meant that I have cleaned up the code as you\nsuggested (see diff below), and that if you decide to include the\npatch series into git.git (I see now you included it in pu), I can\neither submit an additional patch to perform the cleanup, or submit a\nnew \"v2\" patch series incorporating these changes.  Is one preferred\nover the other?\n\nAlso, I wasn't sure where to put the #defines; I chose to put them in\nhttp.h, but should they go in http.c?\n\nThanks for the feedback!\nMark\n\n\ndiff --git c/http.c i/http.c\nindex 6ae59b6..7659ef4 100644\n--- c/http.c\n+++ i/http.c\n@@ -213,16 +213,8 @@ static CURL *get_curl_handle(void)\n        if (ssl_cert != NULL)\n                curl_easy_setopt(result, CURLOPT_SSLCERT, ssl_cert);\n        if (has_cert_password())\n-               curl_easy_setopt(result,\n-#if LIBCURL_VERSION_NUM >= 0x071700\n-                                CURLOPT_KEYPASSWD,\n-#elif LIBCURL_VERSION_NUM >= 0x070903\n-                                CURLOPT_SSLKEYPASSWD,\n-#else\n-                                CURLOPT_SSLCERTPASSWD,\n-#endif\n-                                ssl_cert_password);\n-#if LIBCURL_VERSION_NUM >= 0x070902\n+               curl_easy_setopt(result, CURLOPT_KEYPASSWD, ssl_cert_password);\n+#ifndef NO_CURLOPT_SSLKEY\n        if (ssl_key != NULL)\n                curl_easy_setopt(result, CURLOPT_SSLKEY, ssl_key);\n #endif\ndiff --git c/http.h i/http.h\nindex 26abebe..b49c280 100644\n--- c/http.h\n+++ i/http.h\n@@ -29,6 +29,12 @@\n #define curl_global_init(a) do { /* nothing */ } while(0)\n #endif\n\n+#if LIBCURL_VERSION_NUM < 0x070903\n+#define CURLOPT_KEYPASSWD CURLOPT_SSLCERTPASSWD\n+#elif LIBCURL_VERSION_NUM < 0x071700\n+#define CURLOPT_KEYPASSWD CURLOPT_SSLKEYPASSWD\n+#endif\n+\n #if (LIBCURL_VERSION_NUM < 0x070c04) || (LIBCURL_VERSION_NUM == 0x071000)\n #define NO_CURL_EASY_DUPHANDLE\n #endif\n"},{"id":"116207","messageId":"ca433830906121749t2cb008b2wf72a95d275277cd9@mail.gmail.com","threadId":"19546","inReplyTo":"7vk53h3rey.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 1/2] http.c: prompt for SSL client certificate password","fromName":"Mark Lodato","fromEmail":"lodatom@gmail.com","sentAt":"2009-06-13T00:49:20Z","receivedAt":"2009-06-13T00:49:20Z","isPatch":true,"sender":{"key":"lodatom@gmail.com","avatar":"https://avatars.githubusercontent.com/u/58860?v=4"},"body":"On Fri, Jun 12, 2009 at 8:31 PM, Junio C Hamano<gitster@pobox.com> wrote:\n> Mark Lodato <lodatom@gmail.com> writes:\n>\n>>> And for the libcurl not supporting this, I figure it _could_ be done by\n>>> simply letting libcurl prope the remote and see if it can access it without\n>>> a passphrase as that would then imply that isn't necessary.\n>>>\n>>> I'm not familiar enough with the code and architecture to deem how suitable\n>>> such an action would be.\n>>\n>> I don't think it is possible to check to see if it is encrypted from\n>> within git (without calling OpenSSL directly).\n>\n> I think what Daniel is suggesting is to attempt making a test connection\n> (that does not have to have anything to do with the real object transfer)\n> without passphrase to see if it fails.  If it doesn't, you know you do not\n> need a passphrase to unlock the key/cert.\n\nHmm, I did not do this initially since I thought it was not possible\nwithout calling OpenSSL directly.  If you do not set\nCURLOPT_KEYPASSWD, OpenSSL will prompt the user without telling the\nprogram.  But now that you and Daniel mention it, I think I now\nbelieve it is possible to autodetect by setting CURLOPT_KEYPASSWD to\n\"\" during the trial connection.  But is it OK to perform a trial\nconnection that serves no other purpose?  If so, I will work on\ncreating a new patch that does this.\n\n> While I still think that kind of automated detection would be necessary in\n> the longer term (in other words, we do not necessarily have to have it in\n> the initial implementation that appears in our official release), until that\n> materializes, I think it is more prudent to follow the approach below.\n\nUnderstood.  If the above works, I see no need to go with my original\npatch series.\n\n\n>>> <snip...> If you can't do that, probably you can introduce a config var that says\n>>> \"this certificate is encrypted\", and bypass your new code if that config var isn't set.\n>\n> I think I've said this already in another message, but \"I break your\n> working setup with my patch, but you can add this configuration to unbreak\n> it\" should not be done lightly, certainly without a good reason.  And the\n> reason here as far as I can see is that the code chooses not to bother\n> with the autodetection of encryptedness of the cert/key.  So...\n\nAgain, it wasn't that I didn't bother; it was that I thought this was\nnot possible.  If the autodetection doesn't pan out, I understand your\nreasoning and will change the default to be the old behavior.\n\n\nThanks again,\nMark\n"},{"id":"116209","messageId":"7vab4d2ayo.fsf@alter.siamese.dyndns.org","threadId":"19546","inReplyTo":"ca433830906121733w7c88dfd4w1025b7b936e48e95@mail.gmail.com","subject":"Re: [PATCH 1/2] http.c: prompt for SSL client certificate password","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-06-13T01:12:31Z","receivedAt":"2009-06-13T01:12:31Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Mark Lodato <lodatom@gmail.com> writes:\n\n> On Fri, Jun 12, 2009 at 8:14 PM, Junio C Hamano<gitster@pobox.com> wrote:\n>> Mark Lodato <lodatom@gmail.com> writes:\n>>\n>>> If this patch series is accepted, I\n>>> will make a cleaner version that includes this change.\n>>\n>> Sorry, but I do not understand this part of your message.\n>\n> Sorry about that.  I meant that I have cleaned up the code as you\n> suggested (see diff below), and that if you decide to include the\n> patch series into git.git (I see now you included it in pu), I can\n> either submit an additional patch to perform the cleanup, or submit a\n> new \"v2\" patch series incorporating these changes.  Is one preferred\n> over the other?\n\nAh, I see.\n\nHere is how we do things around here.\n\nReviewers are usually faster to comment and offer improvement suggestions\nthan I pick up patches and apply them to my tree (in any branches).  While\na patch is under active discussion with suggestions that make the code\nobviously better with simple changes, the submitter is expected to send\nnew \"v$n\" (n>=1) patches incorporating suggested improvements.  It often\nis simpler and cleaner if such \"replacement\" patches are sent for anything\nthat hasn't landed on 'next' (or 'master/maint' for that matter), and I\nmake sure not to merge something that still has iffiness to 'next' (iow,\nkeeping it on 'pu') to help this process.\n\nAfter the initial dust settles and reviewers agree that the patch is in a\ngood testable state, it lands in 'next', and if there are further\nimprovements and bugfixes, they are expected to be sent as incremental\npatches.  That way, we do not have to record obvious shortcomings that\ntend to appear in the initial submission in our history, while keeping the\nrecord of incremental updates on top of what has been judged as \"basically\nsound\" (aka \"advances to 'next'\").\n\nSo in this case, v2 is very much preferred.  There is no point recording\n\"Mark originally sent a code with #ifdef sprinkled heavily and then later\nrealized that the code becomes easier to read if #ifdef part is separated\nout to only define the constants used in the code\" as part of our official\nhistory.\n\nBy the way, I forgot to say this even though I noticed you are new:\nwelcome to git development community.\n"},{"id":"116228","messageId":"alpine.DEB.2.00.0906131318480.10804@yvahk2.pbagnpgbe.fr","threadId":"19546","inReplyTo":"7vk53h3rey.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 1/2] http.c: prompt for SSL client certificate password","fromName":"Daniel Stenberg","fromEmail":"daniel@haxx.se","sentAt":"2009-06-13T11:22:49Z","receivedAt":"2009-06-13T11:22:49Z","isPatch":true,"sender":{"key":"daniel@haxx.se","avatar":"https://gravatar.com/avatar/69fdca87edd17cee21ca2e79fc2ff671d644603c3dc27167430f3cd3dbab7ba8?d=mp&s=160"},"body":"On Fri, 12 Jun 2009, Junio C Hamano wrote:\n\n> I think what Daniel is suggesting is to attempt making a test connection \n> (that does not have to have anything to do with the real object transfer) \n> without passphrase to see if it fails.  If it doesn't, you know you do not \n> need a passphrase to unlock the key/cert.\n\nExactly.\n\nAlso note that (in response to Mark's other comments) you really should not go \n\"beneath\" libcurl, and use the SSL library directly without careful \nconsiderations since libcurl can be built to use one out of many SSL libs so \nit's far from sure that you're actually using OpenSSL. Or GnutTLS. Or NSS. \nOr...\n\nIf libcurl itself is not enough to solve the issue, I would probably claim \nthat it would be wise to consider making libcurl support it so that the API \nremains SSL-lib agnostic to the app (git).\n\n-- \n\n  / daniel.haxx.se\n"}]}