After much discussion on IRC (and some wild hiking in svn_auth land),
I'm now happy to present my proposal of a solution to issue 1330. So
what does this patch do exactly, you ask. Remember this prompt?
~> svn ls https://cheap.bastard.com/svn/
Error validating server certificate: Unknown certificate issuer. Accept? (y/N):
Well, forget about it. With this patch you will get the following instead:
~> svn ls https://cheap.bastard.com/svn/
Error validating server certificate:
- Unknown certificate issuer
Fingerprint: 11:f4:44:17:2c:e6:8e:80:a0:b8:3d:bd:b6:c7:43:1d:90:f7:69:7b
Distinguished name: SE
(R)eject, accept (t)emporarily or accept (p)ermanently?
As you can see, you get a choice not only to reject or accept the
certificate temporarily for this session only as before, but you can
also choose to accept it permanently. If you do that you will never get
that prompt again for that particular certificate. Note that you do not
get to choose to accept the cert permanently if there are other problems
with the cert such as wrong hostname, not yet valid or expired. Here's
an example of such a prompt:
~> svn ls https://snakeoil.security.com/svn/
Error validating server certificate:
- Unknown certificate issuer
Fingerprint: bc:10:cd:db:1d:8c:db:07:a7:76:76:50:ce:e7:ef:89:5f:3a:60:12
Distinguished name: Snakeoil Security Inc
- Hostname mismatch (evil.hacker.net)
- Certificate expired or not yet valid
Valid from May 12 15:08:43 2002 GMT until May 12 15:08:43 2003 GMT
(R)eject or accept (t)emporarily?
Another change included in this patch is the removal of all the insecure
ssl-ignore-* options.
Diffstat, log message and patch (against trunk r7042) are included for
your pleasure. It passes make check without errors.
I look forward to your comments!
/Tobias
Diffstat:
clients/cmdline/cl.h | 1
clients/cmdline/prompt.c | 84 +++++++++++++++++++++++++------------
include/svn_auth.h | 23 +++-------
include/svn_config.h | 3 -
libsvn_client/auth.c | 102 ++++++++++++++++++++++++++--------------------
libsvn_ra_dav/session.c | 21 +++++++--
libsvn_subr/config_file.c | 16 ++-----
7 files changed, 146 insertions(+), 104 deletions(-)
Log message:
Fix issue #1330: accept server cert permanently
User visible changes
====================
If the server cert is signed by an unknown CA, but is otherwise fine,
the user can choose to reject the cert, to accept it temporarily or to
accept it permanently.
If the server certificate has other problems (incorrect hostname, not
yet valid or expired), the user can coose to reject the cert or to
accept it temporarily.
The ssl-ignore-* server configuration options are now gone because they
are insecure. Subversion mostly behaves in the same way as Mozilla and
other web browsers.
Implementation details
======================
See the implementation proposal in notes/server-cert-revamp.txt.
subversion/include/svn_config.h
Remove the ssl-ignore-* config options.
subversion/include/svn_auth.h
(svn_auth_cred_server_ssl_t): Replace failures_allow with
trust_permanantly.
(svn_auth_ssl_server_prompt_func_t): Add the text encoded cert to
the arguments of the server cert prompt function prototype.
subversion/libsvn_subr/config_file.c
(ensure_auth_dirs): Create the SVN_AUTH_CRED_SERVER_SSL auth dir.
(svn_config_ensure): Remove the ssl-ignore-* config options from the
template servers file.
subversion/libsvn_client/auth.c
(ssl_server_file_provider_baton_t): New type
(server_ssl_file_first_credentials): Rewritten to look for the cert
in the auth system.
(server_ssl_file_save_credentials): New function used to store a
cert in the auth system.
(svn_client_get_ssl_server_file_provider): Allocate a provider baton
and initialize the provider type in the baton.
(server_ssl_prompt_first_cred): Pass the realmstring (which is the
text encoded certificate) to the prompt function.
subversion/clients/cmdline/cl.h
(svn_cl__auth_ssl_server_prompt): Add the text_cert argument to the
prompt prototype.
subversion/clients/cmdline/prompt.c
(svn_cl__auth_ssl_server_prompt): Rewritten to decode the supplied
text encoded cert, present an informative prompt to the user, and
create the appropriate credentials.
subversion/libsvn_ra_dav/session.c
(server_ssl_callback): Export the cert to a text string and use that
as the realm string. Save credentials permanently if requested.
Index: subversion/include/svn_config.h
===================================================================
--- subversion/include/svn_config.h (revision 7042)
+++ subversion/include/svn_config.h (working copy)
@@ -63,9 +63,6 @@
#define SVN_CONFIG_OPTION_NEON_DEBUG_MASK "neon-debug-mask"
#define SVN_CONFIG_OPTION_SSL_AUTHORITY_FILES "ssl-authority-files"
#define SVN_CONFIG_OPTION_SSL_TRUST_DEFAULT_CA "ssl-trust-default-ca"
-#define SVN_CONFIG_OPTION_SSL_IGNORE_UNKNOWN_CA "ssl-ignore-unknown-ca"
-#define SVN_CONFIG_OPTION_SSL_IGNORE_INVALID_DATE "ssl-ignore-invalid-date"
-#define SVN_CONFIG_OPTION_SSL_IGNORE_HOST_MISMATCH "ssl-ignore-host-mismatch"
#define SVN_CONFIG_OPTION_SSL_CLIENT_CERT_FILE "ssl-client-cert-file"
#define SVN_CONFIG_OPTION_SSL_CLIENT_CERT_PASSWORD "ssl-client-cert-password"
Index: subversion/include/svn_auth.h
===================================================================
--- subversion/include/svn_auth.h (revision 7042)
+++ subversion/include/svn_auth.h (working copy)
@@ -193,25 +193,11 @@
* @a cert contains two inner structures representing fields from the
* server and issuer certificates, as well as the start and expiry
* times. @a failures_allow is set for each flag allowed.
- *
- * failures flags defined within neon:
- * * SVN_AUTH_SSL_NOTYETVALID : certificate is not yet valid
- * * SVN_AUTH_SSL_EXPIRED : certificate has expired
- * * SVN_AUTH_SSL_CNMISMATCH : name on certificate does not match server
- * * SVN_AUTH_SSL_UNKNOWNCA : cert is signed by an untrusted authority
*/
-#define SVN_AUTH_SSL_NOTYETVALID (1<<0)
-#define SVN_AUTH_SSL_EXPIRED (1<<1)
-#define SVN_AUTH_SSL_CNMISMATCH (1<<2)
-#define SVN_AUTH_SSL_UNKNOWNCA (1<<3)
-#define SVN_AUTH_SSL_FAILMASK (0x0f)
-
#define SVN_AUTH_CRED_SERVER_SSL "svn.ssl.server"
-
typedef struct
{
- int failures_allow;
-
+ svn_boolean_t trust_permanantly;
} svn_auth_cred_server_ssl_t;
@@ -275,10 +261,16 @@
*
* for more information.
*/
+#define SVN_AUTH_SSL_NOTYETVALID (1<<0)
+#define SVN_AUTH_SSL_EXPIRED (1<<1)
+#define SVN_AUTH_SSL_CNMISMATCH (1<<2)
+#define SVN_AUTH_SSL_UNKNOWNCA (1<<3)
+#define SVN_AUTH_SSL_FAILMASK (0x0f)
typedef svn_error_t *
(*svn_auth_ssl_server_prompt_func_t) (svn_auth_cred_server_ssl_t **cred,
void *baton,
int failures_in,
+ const char *ascii_cert,
apr_pool_t *pool);
@@ -372,6 +364,7 @@
/** A configuration directory that overrides the default
~/.subversion. */
#define SVN_AUTH_PARAM_CONFIG_DIR SVN_AUTH_PARAM_PREFIX "config-dir"
+
/** Get an initial set of credentials.
*
* Ask @a auth_baton to set @a *credentials to a set of credentials
Index: subversion/libsvn_subr/config_file.c
===================================================================
--- subversion/libsvn_subr/config_file.c (revision 7042)
+++ subversion/libsvn_subr/config_file.c (working copy)
@@ -496,6 +496,12 @@
if (kind == svn_node_none)
apr_err = apr_dir_make (auth_subdir, APR_OS_DEFAULT, pool);
+ auth_subdir = svn_path_join_many (pool, auth_dir,
+ SVN_AUTH_CRED_SERVER_SSL, NULL);
+ svn_io_check_path (auth_subdir, &kind, pool);
+ if (kind == svn_node_none)
+ apr_err = apr_dir_make (auth_subdir, APR_OS_DEFAULT, pool);
+
}
@@ -805,9 +811,6 @@
"### neon-debug-mask Debug mask for Neon HTTP library\n"
"### ssl-authority-files List of files, each of a trusted CAs\n"
"### ssl-trust-default-ca Trust the system 'default' CAs\n"
- "### ssl-ignore-unknown-ca Allow untrusted server certificates\n"
- "### ssl-ignore-invalid-date Allow expired/postdated certificates\n"
- "### ssl-ignore-host-mismatch Allow certificates for other servers\n"
"### ssl-client-cert-file PKCS#12 format client certificate file\n"
"### ssl-client-cert-password Client Key password, if needed.\n"
"###\n"
@@ -823,10 +826,6 @@
"### match is found, the server info is from the section with the\n"
"### corresponding name.\n"
"\n"
- "### Note that the ssl-ignore overrides significantly decrease the\n"
- "### security of the connection, and may allow a third party to\n"
- "### intercept or even modify the transmitted data.\n"
- "\n"
"# [groups]\n"
"# group1 = *.collab.net\n"
"# othergroup = repository.blarggitywhoomph.com\n"
@@ -840,9 +839,6 @@
"# http-proxy-password = doubleblah\n"
"# http-timeout = 60\n"
"# neon-debug-mask = 130\n"
- "# ssl-ignore-unknown-ca = true\n"
- "# ssl-ignore-host-mismatch = true\n"
- "# ssl-ignore-invalid-date = true\n"
"\n"
"### Information for the second group:\n"
"# [othergroup]\n"
Index: subversion/libsvn_client/auth.c
===================================================================
--- subversion/libsvn_client/auth.c (revision 7042)
+++ subversion/libsvn_client/auth.c (working copy)
@@ -436,70 +436,80 @@
/*** SSL file providers. ***/
+typedef struct {
+ /* the cred_kind being fetched (see svn_auth.h)*/
+ const char *cred_kind;
+ /* cache: realmstring which identifies the credentials file */
+ const char *realmstring;
+} ssl_server_file_provider_baton_t;
+
/* retieve ssl server CA failure overrides (if any) from servers
config */
static svn_error_t *
-server_ssl_file_first_credentials (void **credentials_p,
+server_ssl_file_first_credentials (void **credentials,
void **iter_baton,
void *provider_baton,
apr_hash_t *parameters,
const char *realmstring,
apr_pool_t *pool)
{
- const char *temp_setting;
- int failures_in = (int) apr_hash_get (parameters,
- SVN_AUTH_PARAM_SSL_SERVER_FAILURES_IN,
- APR_HASH_KEY_STRING);
- svn_config_t *cfg = apr_hash_get (parameters,
- SVN_AUTH_PARAM_CONFIG,
- APR_HASH_KEY_STRING);
- const char *server_group = apr_hash_get (parameters,
- SVN_AUTH_PARAM_SERVER_GROUP,
- APR_HASH_KEY_STRING);
- svn_auth_cred_server_ssl_t *cred;
- int failures_allow = 0;
+ ssl_server_file_provider_baton_t *pb = provider_baton;
+ apr_hash_t *creds_hash = NULL;
+ const char *config_dir;
+ svn_error_t *error;
- temp_setting = svn_config_get_server_setting (cfg, server_group,
- SVN_CONFIG_OPTION_SSL_IGNORE_UNKNOWN_CA,
- "false");
- if (strcasecmp (temp_setting, "true") == 0)
- {
- failures_allow |= SVN_AUTH_SSL_UNKNOWNCA;
- }
+ pb->realmstring = apr_pstrdup (pool, realmstring);
- temp_setting = svn_config_get_server_setting (cfg, server_group,
- SVN_CONFIG_OPTION_SSL_IGNORE_HOST_MISMATCH,
- "false");
- if (strcasecmp (temp_setting, "true") == 0)
- {
- failures_allow |= SVN_AUTH_SSL_CNMISMATCH;
- }
+ config_dir = apr_hash_get (parameters,
+ SVN_AUTH_PARAM_CONFIG_DIR,
+ APR_HASH_KEY_STRING);
- temp_setting = svn_config_get_server_setting (cfg, server_group,
- SVN_CONFIG_OPTION_SSL_IGNORE_INVALID_DATE,
- "false");
- if (strcasecmp (temp_setting, "true") == 0)
- {
- failures_allow |= SVN_AUTH_SSL_NOTYETVALID | SVN_AUTH_SSL_EXPIRED;
- }
+ error = svn_config_read_auth_data (&creds_hash, pb->cred_kind,
+ pb->realmstring, config_dir, pool);
- /* don't return creds unless we consider the certificate completely
- * acceptable */
- if ( (failures_in & ~failures_allow) == 0)
+ if (error || !creds_hash)
{
- cred = apr_palloc (pool, sizeof (*cred));
- *credentials_p = cred;
- cred->failures_allow = failures_allow;
+ svn_error_clear(error);
+ *credentials = NULL;
}
else
{
- *credentials_p = NULL;
+ svn_auth_cred_server_ssl_t *creds = apr_pcalloc (pool, sizeof(*creds));
+ creds->trust_permanantly = TRUE;
+ *credentials = creds;
}
+
*iter_baton = NULL;
return SVN_NO_ERROR;
}
+static svn_error_t *
+server_ssl_file_save_credentials (svn_boolean_t *saved,
+ void *credentials,
+ void *provider_baton,
+ apr_hash_t *parameters,
+ apr_pool_t *pool)
+{
+ ssl_server_file_provider_baton_t *pb = provider_baton;
+ const char *ascii_cert = pb->realmstring;
+ apr_hash_t *creds_hash = NULL;
+ const char *config_dir;
+
+ config_dir = apr_hash_get (parameters,
+ SVN_AUTH_PARAM_CONFIG_DIR,
+ APR_HASH_KEY_STRING);
+
+ creds_hash = apr_hash_make (pool);
+ SVN_ERR (svn_config_write_auth_data (creds_hash,
+ pb->cred_kind,
+ ascii_cert,
+ config_dir,
+ pool));
+ *saved = TRUE;
+ return SVN_NO_ERROR;
+}
+
/* retrieve and load the ssl client certificate file from servers
config */
static svn_error_t *
@@ -576,7 +586,7 @@
SVN_AUTH_CRED_SERVER_SSL,
&server_ssl_file_first_credentials,
NULL,
- NULL
+ &server_ssl_file_save_credentials,
};
static const svn_auth_provider_t client_ssl_cert_file_provider =
@@ -604,7 +614,12 @@
apr_pool_t *pool)
{
svn_auth_provider_object_t *po = apr_pcalloc (pool, sizeof(*po));
+ ssl_server_file_provider_baton_t *pb = apr_pcalloc (pool, sizeof(*pb));
+
+ pb->cred_kind = SVN_AUTH_CRED_SERVER_SSL;
+
po->vtable = &server_ssl_file_provider;
+ po->provider_baton = pb;
*provider = po;
}
@@ -705,8 +720,9 @@
SVN_AUTH_PARAM_SSL_SERVER_FAILURES_IN,
APR_HASH_KEY_STRING);
+ /* The realmstring is really the text encoded certificate */
SVN_ERR (pb->prompt_func ((svn_auth_cred_server_ssl_t **) credentials_p,
- pb->prompt_baton, failures_in, pool));
+ pb->prompt_baton, failures_in, realmstring, pool));
*iter_baton = NULL;
return SVN_NO_ERROR;
Index: subversion/clients/cmdline/cl.h
===================================================================
--- subversion/clients/cmdline/cl.h (revision 7042)
+++ subversion/clients/cmdline/cl.h (working copy)
@@ -278,6 +278,7 @@
svn_cl__auth_ssl_server_prompt (svn_auth_cred_server_ssl_t **cred_p,
void *baton,
int failures_in,
+ const char *ascii_cert,
apr_pool_t *pool);
Index: subversion/clients/cmdline/prompt.c
===================================================================
--- subversion/clients/cmdline/prompt.c (revision 7042)
+++ subversion/clients/cmdline/prompt.c (working copy)
@@ -24,6 +24,8 @@
#include <apr_lib.h>
+#include <ne_ssl.h>
+
#include "svn_wc.h"
#include "svn_client.h"
#include "svn_string.h"
@@ -185,51 +187,77 @@
svn_cl__auth_ssl_server_prompt (svn_auth_cred_server_ssl_t **cred_p,
void *baton,
int failures_in,
+ const char *ascii_cert,
apr_pool_t *pool)
{
- svn_boolean_t previous_output = FALSE;
- int failure;
+ ne_ssl_certificate *cert;
+ char fingerprint[NE_SSL_DIGESTLEN];
+ char valid_from[NE_SSL_VDATELEN], valid_until[NE_SSL_VDATELEN];
+ int allow_perm_accept;
const char *choice;
-
+ char *dname;
svn_stringbuf_t *buf = svn_stringbuf_create
- ("Error validating server certificate: ", pool);
+ ("Error validating server certificate:\n", pool);
- failure = failures_in & SVN_AUTH_SSL_UNKNOWNCA;
- if (failure)
+ cert = ne_ssl_cert_import(ascii_cert);
+
+ if (failures_in & SVN_AUTH_SSL_UNKNOWNCA)
{
- svn_stringbuf_appendcstr (buf, "Unknown certificate issuer");
- previous_output = TRUE;
+ svn_stringbuf_appendcstr (buf, " - Unknown certificate issuer\n");
+ svn_stringbuf_appendcstr (buf, " Fingerprint: ");
+ if (ne_ssl_cert_digest(cert, fingerprint) != 0)
+ {
+ strcpy(fingerprint, "<unknown>");
+ }
+ svn_stringbuf_appendcstr (buf, fingerprint);
+ svn_stringbuf_appendcstr (buf, "\n");
+ dname = ne_ssl_readable_dname(ne_ssl_cert_issuer(cert));
+ svn_stringbuf_appendcstr (buf, " Distinguished name: ");
+ svn_stringbuf_appendcstr (buf, dname);
+ svn_stringbuf_appendcstr (buf, "\n");
+ free(dname);
}
- failure = failures_in & SVN_AUTH_SSL_CNMISMATCH;
- if (failure)
+ if (failures_in & SVN_AUTH_SSL_CNMISMATCH)
{
- if (previous_output)
- {
- svn_stringbuf_appendcstr (buf, ", ");
- }
- svn_stringbuf_appendcstr (buf, "Hostname mismatch");
- previous_output = TRUE;
+ svn_stringbuf_appendcstr (buf, " - Hostname mismatch (");
+ svn_stringbuf_appendcstr (buf, ne_ssl_cert_identity(cert));
+ svn_stringbuf_appendcstr (buf, ")\n");
+
}
- failure = failures_in & (SVN_AUTH_SSL_EXPIRED | SVN_AUTH_SSL_NOTYETVALID);
- if (failure)
+ if (failures_in & (SVN_AUTH_SSL_EXPIRED | SVN_AUTH_SSL_NOTYETVALID))
{
- if (previous_output)
- {
- svn_stringbuf_appendcstr (buf, ", ");
- }
- svn_stringbuf_appendcstr (buf, "Certificate expired or not yet valid");
- previous_output = TRUE;
+ svn_stringbuf_appendcstr (buf, " - Certificate expired or not yet valid\n");
+ ne_ssl_cert_validity(cert, valid_from, valid_until);
+ svn_stringbuf_appendcstr (buf, " Valid from ");
+ svn_stringbuf_appendcstr (buf, valid_from);
+ svn_stringbuf_appendcstr (buf, " until ");
+ svn_stringbuf_appendcstr (buf, valid_until);
+ svn_stringbuf_appendcstr (buf, "\n");
}
+ ne_ssl_cert_free(cert);
- svn_stringbuf_appendcstr (buf, ". Accept? (y/N): ");
+ allow_perm_accept = failures_in == SVN_AUTH_SSL_UNKNOWNCA;
+ if (allow_perm_accept)
+ {
+ svn_stringbuf_appendcstr (buf, "(R)eject, accept (t)emporarily or accept (p)ermanently? ");
+ }
+ else
+ {
+ svn_stringbuf_appendcstr (buf, "(R)eject or accept (t)emporarily? ");
+ }
SVN_ERR (prompt (&choice, buf->data, FALSE, pool));
-
- if (choice && (choice[0] == 'y' || choice[0] == 'Y'))
+
+ if (choice && (choice[0] == 't' || choice[0] == 'T'))
{
*cred_p = apr_pcalloc (pool, sizeof (**cred_p));
- (*cred_p)->failures_allow = failures_in;
+ (*cred_p)->trust_permanantly = FALSE;
}
+ else if (allow_perm_accept && choice && (choice[0] == 'p' || choice[0] == 'P'))
+ {
+ *cred_p = apr_pcalloc (pool, sizeof (**cred_p));
+ (*cred_p)->trust_permanantly = TRUE;
+ }
else
{
*cred_p = NULL;
Index: subversion/libsvn_ra_dav/session.c
===================================================================
--- subversion/libsvn_ra_dav/session.c (revision 7042)
+++ subversion/libsvn_ra_dav/session.c (working copy)
@@ -117,12 +117,12 @@
const ne_ssl_certificate *cert)
{
svn_ra_session_t *ras = userdata;
+ svn_auth_cred_server_ssl_t *server_creds = NULL;
void *creds;
- svn_auth_cred_server_ssl_t *server_creds;
svn_auth_iterstate_t *state;
apr_pool_t *pool;
svn_error_t *error;
- int failures_allowed = 0;
+ char *ascii_cert = ne_ssl_cert_export(cert);
svn_auth_set_parameter(ras->callbacks->auth_baton,
SVN_AUTH_PARAM_SSL_SERVER_FAILURES_IN,
@@ -131,9 +131,11 @@
apr_pool_create(&pool, ras->pool);
error = svn_auth_first_credentials(&creds, &state,
SVN_AUTH_CRED_SERVER_SSL,
- "none", /* ### fix? */
+ ascii_cert,
ras->callbacks->auth_baton,
pool);
+ free(ascii_cert);
+
if (error || !creds)
{
svn_error_clear(error);
@@ -141,10 +143,19 @@
else
{
server_creds = creds;
- failures_allowed = (server_creds) ? server_creds->failures_allow : 0;
+ if (server_creds->trust_permanantly)
+ {
+ error = svn_auth_save_credentials(state, pool);
+ if (error)
+ {
+ /* It would be nice to show the error to the user
+ * somehow... */
+ svn_error_clear(error);
+ }
+ }
}
apr_pool_destroy(pool);
- return (failures & ~failures_allowed);
+ return server_creds ? 0 : failures;
}
static int
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Thu Sep 11 00:23:46 2003