You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@subversion.apache.org by br...@apache.org on 2014/08/01 22:07:21 UTC
svn commit: r1615212 - in /subversion/branches/1.8.x: ./
subversion/include/private/svn_cert.h subversion/libsvn_ra_serf/util.c
subversion/libsvn_subr/dirent_uri.c
subversion/tests/libsvn_subr/dirent_uri-test.c
Author: breser
Date: Fri Aug 1 20:07:20 2014
New Revision: 1615212
URL: http://svn.apache.org/r1615212
Log:
Merge r1615211 from trunk:
* r1615211
Provide a new function to handle X.509 hostname matching.
Justification:
Follow RFC 6125.
Votes:
+1: breser, philip, stefan2
Added:
subversion/branches/1.8.x/subversion/include/private/svn_cert.h
- copied unchanged from r1615211, subversion/trunk/subversion/include/private/svn_cert.h
Modified:
subversion/branches/1.8.x/ (props changed)
subversion/branches/1.8.x/subversion/libsvn_ra_serf/util.c
subversion/branches/1.8.x/subversion/libsvn_subr/dirent_uri.c
subversion/branches/1.8.x/subversion/tests/libsvn_subr/dirent_uri-test.c
Propchange: subversion/branches/1.8.x/
------------------------------------------------------------------------------
Merged /subversion/trunk:r1615211
Modified: subversion/branches/1.8.x/subversion/libsvn_ra_serf/util.c
URL: http://svn.apache.org/viewvc/subversion/branches/1.8.x/subversion/libsvn_ra_serf/util.c?rev=1615212&r1=1615211&r2=1615212&view=diff
==============================================================================
--- subversion/branches/1.8.x/subversion/libsvn_ra_serf/util.c (original)
+++ subversion/branches/1.8.x/subversion/libsvn_ra_serf/util.c Fri Aug 1 20:07:20 2014
@@ -28,7 +28,6 @@
#define APR_WANT_STRFUNC
#include <apr.h>
#include <apr_want.h>
-#include <apr_fnmatch.h>
#include <serf.h>
#include <serf_bucket_types.h>
@@ -49,6 +48,7 @@
#include "private/svn_fspath.h"
#include "private/svn_subr_private.h"
#include "private/svn_auth_private.h"
+#include "private/svn_cert.h"
#include "ra_serf.h"
@@ -287,6 +287,8 @@ ssl_server_cert(void *baton, int failure
apr_array_header_t *san;
svn_boolean_t found_san_entry = FALSE;
svn_boolean_t found_matching_hostname = FALSE;
+ svn_string_t *actual_hostname =
+ svn_string_create(conn->session->session_url.hostname, scratch_pool);
serf_cert = serf_ssl_cert_certificate(cert, scratch_pool);
@@ -299,10 +301,9 @@ ssl_server_cert(void *baton, int failure
for (i = 0; i < san->nelts; i++)
{
const char *s = APR_ARRAY_IDX(san, i, const char*);
- if (APR_SUCCESS == apr_fnmatch(s,
- conn->session->session_url.hostname,
- APR_FNM_PERIOD |
- APR_FNM_CASE_BLIND))
+ svn_string_t *cert_hostname = svn_string_create(s, scratch_pool);
+
+ if (svn_cert__match_dns_identity(cert_hostname, actual_hostname))
{
found_matching_hostname = TRUE;
break;
@@ -323,11 +324,15 @@ ssl_server_cert(void *baton, int failure
if (subject)
hostname = svn_hash_gets(subject, "CN");
- if (hostname
- && apr_fnmatch(hostname, conn->session->session_url.hostname,
- APR_FNM_PERIOD | APR_FNM_CASE_BLIND) == APR_SUCCESS)
+ if (hostname)
{
- found_matching_hostname = TRUE;
+ svn_string_t *cert_hostname = svn_string_create(hostname,
+ scratch_pool);
+
+ if (svn_cert__match_dns_identity(cert_hostname, actual_hostname))
+ {
+ found_matching_hostname = TRUE;
+ }
}
}
Modified: subversion/branches/1.8.x/subversion/libsvn_subr/dirent_uri.c
URL: http://svn.apache.org/viewvc/subversion/branches/1.8.x/subversion/libsvn_subr/dirent_uri.c?rev=1615212&r1=1615211&r2=1615212&view=diff
==============================================================================
--- subversion/branches/1.8.x/subversion/libsvn_subr/dirent_uri.c (original)
+++ subversion/branches/1.8.x/subversion/libsvn_subr/dirent_uri.c Fri Aug 1 20:07:20 2014
@@ -38,6 +38,7 @@
#include "dirent_uri.h"
#include "private/svn_fspath.h"
+#include "private/svn_cert.h"
/* The canonical empty path. Can this be changed? Well, change the empty
test below and the path library will work, not so sure about the fs/wc
@@ -2597,3 +2598,81 @@ svn_urlpath__canonicalize(const char *ur
}
return uri;
}
+
+
+/* -------------- The cert API (see private/svn_cert.h) ------------- */
+
+svn_boolean_t
+svn_cert__match_dns_identity(svn_string_t *pattern, svn_string_t *hostname)
+{
+ apr_size_t pattern_pos = 0, hostname_pos = 0;
+
+ /* support leading wildcards that composed of the only character in the
+ * left-most label. */
+ if (pattern->len >= 2 &&
+ pattern->data[pattern_pos] == '*' &&
+ pattern->data[pattern_pos + 1] == '.')
+ {
+ while (hostname_pos < hostname->len &&
+ hostname->data[hostname_pos] != '.')
+ {
+ hostname_pos++;
+ }
+ /* Assume that the wildcard must match something. Rule 2 says
+ * that *.example.com should not match example.com. If the wildcard
+ * ends up not matching anything then it matches .example.com which
+ * seems to be essentially the same as just example.com */
+ if (hostname_pos == 0)
+ return FALSE;
+
+ pattern_pos++;
+ }
+
+ while (pattern_pos < pattern->len && hostname_pos < hostname->len)
+ {
+ char pattern_c = pattern->data[pattern_pos];
+ char hostname_c = hostname->data[hostname_pos];
+
+ /* fold case as described in RFC 4343.
+ * Note: We actually convert to lowercase, since our URI
+ * canonicalization code converts to lowercase and generally
+ * most certs are issued with lowercase DNS names, meaning
+ * this avoids the fold operation in most cases. The RFC
+ * suggests the opposite transformation, but doesn't require
+ * any specific implementation in any case. It is critical
+ * that this folding be locale independent so you can't use
+ * tolower(). */
+ pattern_c = canonicalize_to_lower(pattern_c);
+ hostname_c = canonicalize_to_lower(hostname_c);
+
+ if (pattern_c != hostname_c)
+ {
+ /* doesn't match */
+ return FALSE;
+ }
+ else
+ {
+ /* characters match so skip both */
+ pattern_pos++;
+ hostname_pos++;
+ }
+ }
+
+ /* ignore a trailing period on the hostname since this has no effect on the
+ * security of the matching. See the following for the long explanation as
+ * to why:
+ * https://bugzilla.mozilla.org/show_bug.cgi?id=134402#c28
+ */
+ if (pattern_pos == pattern->len &&
+ hostname_pos == hostname->len - 1 &&
+ hostname->data[hostname_pos] == '.')
+ hostname_pos++;
+
+ if (pattern_pos != pattern->len || hostname_pos != hostname->len)
+ {
+ /* end didn't match */
+ return FALSE;
+ }
+
+ return TRUE;
+}
Modified: subversion/branches/1.8.x/subversion/tests/libsvn_subr/dirent_uri-test.c
URL: http://svn.apache.org/viewvc/subversion/branches/1.8.x/subversion/tests/libsvn_subr/dirent_uri-test.c?rev=1615212&r1=1615211&r2=1615212&view=diff
==============================================================================
--- subversion/branches/1.8.x/subversion/tests/libsvn_subr/dirent_uri-test.c (original)
+++ subversion/branches/1.8.x/subversion/tests/libsvn_subr/dirent_uri-test.c Fri Aug 1 20:07:20 2014
@@ -37,6 +37,7 @@
#include "svn_pools.h"
#include "svn_dirent_uri.h"
#include "private/svn_fspath.h"
+#include "private/svn_cert.h"
#include "../svn_test.h"
@@ -2714,6 +2715,145 @@ test_fspath_get_longest_ancestor(apr_poo
return SVN_NO_ERROR;
}
+struct cert_match_dns_test {
+ const char *pattern;
+ const char *hostname;
+ svn_boolean_t expected;
+};
+
+static svn_error_t *
+run_cert_match_dns_tests(struct cert_match_dns_test *tests, apr_pool_t *pool)
+{
+ struct cert_match_dns_test *ct;
+ apr_pool_t *iterpool = svn_pool_create(pool);
+
+ for (ct = tests; ct->pattern; ct++)
+ {
+ svn_boolean_t result;
+ svn_string_t *pattern, *hostname;
+
+ svn_pool_clear(iterpool);
+
+ pattern = svn_string_create(ct->pattern, iterpool);
+ hostname = svn_string_create(ct->hostname, iterpool);
+
+ result = svn_cert__match_dns_identity(pattern, hostname);
+ if (result != ct->expected)
+ return svn_error_createf(SVN_ERR_TEST_FAILED, NULL,
+ "Expected %s but got %s for pattern '%s' on "
+ "hostname '%s'",
+ ct->expected ? "match" : "no match",
+ result ? "match" : "no match",
+ pattern->data, hostname->data);
+
+ }
+
+ svn_pool_destroy(iterpool);
+
+ return SVN_NO_ERROR;
+}
+
+static struct cert_match_dns_test cert_match_dns_tests[] = {
+ { "foo.example.com", "foo.example.com", TRUE }, /* exact match */
+ { "foo.example.com", "FOO.EXAMPLE.COM", TRUE }, /* case differences */
+ { "FOO.EXAMPLE.COM", "foo.example.com", TRUE },
+ { "*.example.com", "FoO.ExAmPlE.CoM", TRUE },
+ { "*.ExAmPlE.CoM", "foo.example.com", TRUE },
+ { "ABCDEFGHIJKLMNOPQRSTUVWXYZ", "abcdefghijklmnopqrstuvwxyz", TRUE },
+ { "abcdefghijklmnopqrstuvwxyz", "ABCDEFGHIJKLMNOPQRSTUVWXYZ", TRUE },
+ { "foo.example.com", "bar.example.com", FALSE }, /* difference at start */
+ { "foo.example.com", "foo.example.net", FALSE }, /* difference at end */
+ { "foo.example.com", "foo.example.commercial", FALSE }, /* hostname longer */
+ { "foo.example.commercial", "foo.example.com", FALSE }, /* pattern longer */
+ { "foo.example.comcom", "foo.example.com", FALSE }, /* repeated suffix */
+ { "foo.example.com", "foo.example.comcom", FALSE },
+ { "foo.example.com.com", "foo.example.com", FALSE },
+ { "foo.example.com", "foo.example.com.com", FALSE },
+ { "foofoo.example.com", "foo.example.com", FALSE }, /* repeated prefix */
+ { "foo.example.com", "foofoo.example.com", FALSE },
+ { "foo.foo.example.com", "foo.example.com", FALSE },
+ { "foo.example.com", "foo.foo.example.com", FALSE },
+ { "foo.*.example.com", "foo.bar.example.com", FALSE }, /* RFC 6125 s. 6.4.3
+ Rule 1 */
+ { "*.example.com", "foo.example.com", TRUE }, /* RFC 6125 s. 6.4.3 Rule 2 */
+ { "*.example.com", "bar.foo.example.com", FALSE }, /* Rule 2 */
+ { "*.example.com", "example.com", FALSE }, /* Rule 2 */
+ { "*.example.com", ".example.com", FALSE }, /* RFC doesn't say what to do
+ here and a leading period on
+ a hostname doesn't make sense
+ so we'll just reject this. */
+ { "*", "foo.example.com", FALSE }, /* wildcard must be left-most label,
+ implies that there must be more than
+ one label. */
+ { "*", "example.com", FALSE },
+ { "*", "com", FALSE },
+ { "*.example.com", "foo.example.net", FALSE }, /* difference in literal text
+ with a wildcard. */
+ { "*.com", "example.com", TRUE }, /* See Errata ID 3090 for RFC 6125,
+ probably shouldn't allow this but
+ we do for now. */
+ { "*.", "example.com", FALSE }, /* test some dubious 2 character wildcard
+ patterns */
+ { "*.", "example.", TRUE }, /* This one feels questionable */
+ { "*.", "example", FALSE },
+ { "*.", ".", FALSE },
+ { "a", "a", TRUE }, /* check that single letter exact matches work */
+ { "a", "b", FALSE }, /* and single letter not matches shouldn't */
+ { "*.*.com", "foo.example.com", FALSE }, /* unsupported wildcards */
+ { "*.*.com", "example.com", FALSE },
+ { "**.example.com", "foo.example.com", FALSE },
+ { "**.example.com", "example.com", FALSE },
+ { "f*.example.com", "foo.example.com", FALSE },
+ { "f*.example.com", "bar.example.com", FALSE },
+ { "*o.example.com", "foo.example.com", FALSE },
+ { "*o.example.com", "bar.example.com", FALSE },
+ { "f*o.example.com", "foo.example.com", FALSE },
+ { "f*o.example.com", "bar.example.com", FALSE },
+ { "foo.e*.com", "foo.example.com", FALSE },
+ { "foo.*e.com", "foo.example.com", FALSE },
+ { "foo.e*e.com", "foo.example.com", FALSE },
+ { "foo.example.com", "foo.example.com.", TRUE }, /* trailing dot */
+ { "*.example.com", "foo.example.com.", TRUE },
+ { "foo", "foo.", TRUE },
+ { "foo.example.com.", "foo.example.com", FALSE },
+ { "*.example.com.", "foo.example.com", FALSE },
+ { "foo.", "foo", FALSE },
+ { "foo.example.com", "foo.example.com..", FALSE },
+ { "*.example.com", "foo.example.com..", FALSE },
+ { "foo", "foo..", FALSE },
+ { "foo.example.com..", "foo.example.com", FALSE },
+ { "*.example.com..", "foo.example.com", FALSE },
+ { "foo..", "foo", FALSE },
+ { NULL }
+};
+
+static svn_error_t *
+test_cert_match_dns_identity(apr_pool_t *pool)
+{
+ return run_cert_match_dns_tests(cert_match_dns_tests, pool);
+}
+
+/* This test table implements results that should happen if we supported
+ * RFC 6125 s. 6.4.3 Rule 3. We don't so it's expected to fail for now. */
+static struct cert_match_dns_test rule3_tests[] = {
+ { "baz*.example.net", "baz1.example.net", TRUE },
+ { "*baz.example.net", "foobaz.example.net", TRUE },
+ { "b*z.example.net", "buuz.example.net", TRUE },
+ { "b*z.example.net", "bz.example.net", FALSE }, /* presume wildcard can't
+ match nothing */
+ { "baz*.example.net", "baz.example.net", FALSE },
+ { "*baz.example.net", "baz.example.net", FALSE },
+ { "b*z.example.net", "buuzuuz.example.net", TRUE }, /* presume wildcard
+ should be greedy */
+ { NULL }
+};
+
+static svn_error_t *
+test_rule3(apr_pool_t *pool)
+{
+ return run_cert_match_dns_tests(rule3_tests, pool);
+}
+
/* The test table. */
@@ -2812,5 +2952,9 @@ struct svn_test_descriptor_t test_funcs[
"test svn_fspath__dirname/basename/split"),
SVN_TEST_PASS2(test_fspath_get_longest_ancestor,
"test svn_fspath__get_longest_ancestor"),
+ SVN_TEST_PASS2(test_cert_match_dns_identity,
+ "test svn_cert__match_dns_identity"),
+ SVN_TEST_XFAIL2(test_rule3,
+ "test match with RFC 6125 s. 6.4.3 Rule 3"),
SVN_TEST_NULL
};