There was a minor problem with the old patch. The code in the function
server_ssl_file_first_credentials did not check if the cert had expired
or if the hostname was incorrect. An updated patch and log message are
attached.
/Tobias
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. Only accept the cert if the only problem is that
the CA is unknown.
(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 7044)
+++ 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 7044)
+++ 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 7044)
+++ 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 7044)
+++ subversion/libsvn_client/auth.c (working copy)
@@ -436,70 +436,87 @@
/*** 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;
+ ssl_server_file_provider_baton_t *pb = provider_baton;
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;
+ apr_hash_t *creds_hash = NULL;
+ const char *config_dir;
+ svn_error_t *error = SVN_NO_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)
+ /* Only accept the cert if the only problem is that the CA is unknown. */
+ if (failures_in == SVN_AUTH_SSL_UNKNOWNCA)
{
- 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 +593,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 +621,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 +727,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 7044)
+++ 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 7044)
+++ 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 7044)
+++ 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 14:06:46 2003