Joe Orton <jorton_at_redhat.com> writes:
> The current behaviour (svn prompting for a filename when an SSL client
> cert is requested) is pretty unusual - I don't know why it works like
> this. I can't think of a common use case where the behaviour is
> particularly useful, and certainly there are lots where it is actively
> unhelpful, e.g. as per the referenced bug.
>
> If you are using an SSL server which requires client cert auth, you will
> most likely have configured that beforehand. If you are using it
> regularly you certainly won't be typing in that filename every commit.
>
> If you are using such a server, and you *don't know* that it requires
> client cert auth, chances are you don't have one.
>
> If you're using some global-ish PKI with lots of servers which might
> require client cert auth, you will have configured that beforehand too.
>
> Rather than pushing yet-more config knobs down into ra_* I would suggest
> adding a config toggle which only adds the prompting provider if a
> config boolean is enabled (but is off by default). That would solve
> this bug and make the default behaviour correct to boot. Possible
> problems:
>
> 1) this is arguably a backwards compat break, but it's not like this is
> going to break scripts since it's only removing a case which always
> requires interactive input.
>
> 2) the default error for the "SSL server requested a client cert but
> none was provided" is probably an obscure SSL error message; this is the
> only real value of the current prompt. This could probably be improved.
>
> Thoughts?
I started a patch to do what Joe suggested. There are some unexpected
difficulties, however, due to the way our code is arranged. Search for
"### TODO" in the patch below for details.
An easy solution would be to just put the new ssl-client-cert-prompt
option into the 'config' file instead of into 'servers'. On the one
hand, 'servers' seems like the more logical place for it; on the other
hand, putting it into 'config' would work right now without massive code
reorganization... And really, who would ever want to control on a
server-by-server basis whether or not they get prompted for client cert
paths?
Feedback welcome. While awaiting others' thoughts, I'm going to move on
to issue #2489.
[[[
##################################################################
### ###
### NOT READY FOR COMMIT; SEARCH FOR "### TODO" IN THE PATCH ###
### FOR DISCUSSION OF KNOWN ISSUES. ###
### ###
##################################################################
Fix issue #2410: don't prompt for client cert path unless configured to.
* subversion/libsvn_subr/config_file.c
(svn_config_ensure): Document new ssl-client-cert-prompt option.
Include a comment about how it might not be in the right place.
* subversion/libsvn_subr/cmdline.c
(svn_cmdline_set_up_auth_baton): Don't register the client cert
prompter unless cfg says it's okay. Include a comment about why
this doesn't work the way we'd want it to.
* subversion/include/svn_config.h
(SVN_CONFIG_OPTION_SSL_CLIENT_CERT_PROMPT): Define new option.
]]]
Index: subversion/libsvn_subr/config_file.c
===================================================================
--- subversion/libsvn_subr/config_file.c (revision 32000)
+++ subversion/libsvn_subr/config_file.c (working copy)
@@ -747,6 +747,8 @@
"### ssl-client-cert-file PKCS#12 format client certificate file"
NL
"### ssl-client-cert-password Client Key password, if needed." NL
+ "### ssl-client-cert-prompt Prompt for client cert file path" NL
+ "### (see farther down for details)." NL
"### ssl-pkcs11-provider Name of PKCS#11 provider to use." NL
"### http-library Which library to use for http/https"
NL
@@ -787,6 +789,18 @@
"### saving of *new* credentials; it doesn't invalidate existing" NL
"### caches. (To do that, remove the cache files by hand.)" NL
"###" NL
+ /* ### TODO: Right now, ssl-client-cert-prompt can only appear
+ * ### in the [global] section of the 'servers' file, because
+ * ### we don't have the server name available when we call
+ * ### svn_cmdline_set_up_auth_baton() to register the authn
+ * ### providers. Therefore, should ssl-client-cert-prompt go
+ * ### in the 'config' file instead of in 'servers'? Or... ?
+ */
+ "### Set ssl-client-cert-prompt to 'yes' to cause the client to" NL
+ "### prompt for a path to a client cert file when the server" NL
+ "### requests a client cert but no client cert file is found in the" NL
+ "### expected place (see ssl-client-cert-file). Defaults to 'no'." NL
+ "###" NL
"### HTTP timeouts, if given, are specified in seconds. A timeout" NL
"### of 0, i.e. zero, causes a builtin default to be used." NL
"###" NL
@@ -858,6 +872,8 @@
"# No neon-debug-mask, so neon debugging is disabled." NL
"# ssl-authority-files = /path/to/CAcert.pem;/path/to/CAcert2.pem" NL
"#" NL
+ "# ssl-client-cert-prompt = no" NL
+ "#" NL
"# Password caching parameters:" NL
"# store-passwords = no" NL
"# store-plaintext-passwords = no" NL;
Index: subversion/libsvn_subr/cmdline.c
===================================================================
--- subversion/libsvn_subr/cmdline.c (revision 32000)
+++ subversion/libsvn_subr/cmdline.c (working copy)
@@ -568,19 +568,45 @@
2, /* retry limit */ pool);
APR_ARRAY_PUSH(providers, svn_auth_provider_object_t *) = provider;
- /* Three ssl prompt providers, for server-certs, client-certs,
- and client-cert-passphrases. */
+ /* SSL prompt providers for server-certs and client-cert-passphrases. */
svn_auth_get_ssl_server_trust_prompt_provider
(&provider, svn_cmdline_auth_ssl_server_trust_prompt, pb, pool);
APR_ARRAY_PUSH(providers, svn_auth_provider_object_t *) = provider;
- svn_auth_get_ssl_client_cert_prompt_provider
- (&provider, svn_cmdline_auth_ssl_client_cert_prompt, pb, 2, pool);
- APR_ARRAY_PUSH(providers, svn_auth_provider_object_t *) = provider;
-
svn_auth_get_ssl_client_cert_pw_prompt_provider
(&provider, svn_cmdline_auth_ssl_client_cert_pw_prompt, pb, 2, pool);
APR_ARRAY_PUSH(providers, svn_auth_provider_object_t *) = provider;
+
+ /* Register an SSL prompt provider for client-cert files, but
+ only if config says that prompting for them is okay. */
+ {
+ svn_boolean_t do_prompt;
+
+ /* ### TODO: Aha, but this doesn't work, because here cfg
+ ### represents only the 'config' file, not the 'servers'
+ ### file. We have to either move the option over to the
+ ### 'config' file, which seems like a less than ideal place
+ ### for it, or get access to 'servers' here. But see the
+ ### "### TODO" comment in svn_config_ensure() in
+ ### libsvn_subr/config_file.c, where we initialize the
+ ### servers file, for discussion of another problem: the
+ ### fact that we don't have the server name yet, so we can
+ ### only use the option if it appears in the [global]
+ ### section.
+ */
+ SVN_ERR(svn_config_get_server_setting_bool(
+ cfg, &do_prompt, NULL,
+ SVN_CONFIG_OPTION_SSL_CLIENT_CERT_PROMPT, FALSE));
+
+ if (do_prompt)
+ {
+ svn_auth_get_ssl_client_cert_prompt_provider(
+ &provider, svn_cmdline_auth_ssl_client_cert_prompt,
+ pb, 2, pool);
+ APR_ARRAY_PUSH(providers, svn_auth_provider_object_t *)
+ = provider;
+ }
+ }
}
else if (trust_server_cert)
{
Index: subversion/include/svn_config.h
===================================================================
--- subversion/include/svn_config.h (revision 32000)
+++ subversion/include/svn_config.h (working copy)
@@ -72,6 +72,7 @@
#define SVN_CONFIG_OPTION_SSL_TRUST_DEFAULT_CA "ssl-trust-default-ca"
#define SVN_CONFIG_OPTION_SSL_CLIENT_CERT_FILE "ssl-client-cert-file"
#define SVN_CONFIG_OPTION_SSL_CLIENT_CERT_PASSWORD "ssl-client-cert-password"
+#define SVN_CONFIG_OPTION_SSL_CLIENT_CERT_PROMPT "ssl-client-cert-prompt"
#define SVN_CONFIG_OPTION_SSL_PKCS11_PROVIDER "ssl-pkcs11-provider"
#define SVN_CONFIG_OPTION_HTTP_LIBRARY "http-library"
#define SVN_CONFIG_OPTION_STORE_PASSWORDS "store-passwords"
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe_at_subversion.tigris.org
For additional commands, e-mail: dev-help_at_subversion.tigris.org
Received on 2008-07-04 23:04:08 CEST