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/18 12:27:15 UTC
svn commit: r1618575 - in /subversion/branches/svn-auth-x509/subversion:
libsvn_subr/x509parse.c tests/libsvn_subr/x509-test.c
Author: breser
Date: Mon Aug 18 10:27:15 2014
New Revision: 1618575
URL: http://svn.apache.org/r1618575
Log:
On svn-auth-x509 branch, don't use memcmp() without checking that the
buffer is long enough for the comparision.
This fixes the certificate that Ivan found in his auth store that caused an
error with the parser. The parser was assuming (very wrongly) that the OID
for the signature algorithm was always 9 bytes. This commit goes beyond just
fixing that cert and generally fixes all the usages of memcmp() to be safe.
* subversion/libsvn_subr/x509parse.c
(equal, oids_equal, starts_with): Functions to wrap safely calling memcmp().
Makes things easier to read.
(x509_get_ext): Use equal() rather than memcmp().
(x509parse_dn_gets): Use starts_with() rather than memcmp() and format per
our style.
(x509parse_get_hostnames): Use starts_with() rather than memcmp().
(svn_x509_parse_cert): Use oids_equal() rather than memcmp().
* subversion/tests/libsvn_subr/x509-test.c
(x509_test): Add a test for a certificate with a short signature algorithm
OID.
Modified:
subversion/branches/svn-auth-x509/subversion/libsvn_subr/x509parse.c
subversion/branches/svn-auth-x509/subversion/tests/libsvn_subr/x509-test.c
Modified: subversion/branches/svn-auth-x509/subversion/libsvn_subr/x509parse.c
URL: http://svn.apache.org/viewvc/subversion/branches/svn-auth-x509/subversion/libsvn_subr/x509parse.c?rev=1618575&r1=1618574&r2=1618575&view=diff
==============================================================================
--- subversion/branches/svn-auth-x509/subversion/libsvn_subr/x509parse.c (original)
+++ subversion/branches/svn-auth-x509/subversion/libsvn_subr/x509parse.c Mon Aug 18 10:27:15 2014
@@ -139,6 +139,34 @@ asn1_get_int(const unsigned char **p, co
return SVN_NO_ERROR;
}
+static svn_boolean_t
+equal(const char *left, apr_size_t left_len,
+ const char *right, apr_size_t right_len)
+{
+ if (left_len != right_len)
+ return FALSE;
+
+ return memcmp(left, right, right_len) == 0;
+}
+
+static svn_boolean_t
+oids_equal(x509_buf *left, x509_buf *right)
+{
+ return equal((const char *)left->p, left->len,
+ (const char *)right->p, right->len);
+}
+
+static svn_boolean_t
+starts_with(const x509_buf *buf,
+ const char *starts_with,
+ apr_size_t sw_len)
+{
+ if (buf->len < sw_len)
+ return FALSE;
+
+ return memcmp((const char *)buf->p, starts_with, sw_len) == 0;
+}
+
/*
* Version ::= INTEGER { v1(0), v2(1), v3(2) }
*/
@@ -581,8 +609,8 @@ x509_get_ext(apr_array_header_t *dnsname
NULL);
/* skip all extensions except SubjectAltName */
- if (len != sizeof(OID_SUBJECT_ALT_NAME) - 1 ||
- memcmp(*p, OID_SUBJECT_ALT_NAME, sizeof(OID_SUBJECT_ALT_NAME) - 1) != 0)
+ if (!equal((const char *)*p, len,
+ OID_SUBJECT_ALT_NAME, sizeof(OID_SUBJECT_ALT_NAME) - 1))
{
*p += ext_len - (*p - ext_start);
continue;
@@ -875,49 +903,53 @@ x509parse_dn_gets(svn_stringbuf_t *buf,
if (name != dn)
svn_stringbuf_appendcstr(buf, ", ");
- if (memcmp(name->oid.p, OID_X520, 2) == 0) {
- switch (name->oid.p[2]) {
- case X520_COMMON_NAME:
- svn_stringbuf_appendcstr(buf, "CN=");
- break;
-
- case X520_COUNTRY:
- svn_stringbuf_appendcstr(buf, "C=");
- break;
-
- case X520_LOCALITY:
- svn_stringbuf_appendcstr(buf, "L=");
- break;
-
- case X520_STATE:
- svn_stringbuf_appendcstr(buf, "ST=");
- break;
-
- case X520_ORGANIZATION:
- svn_stringbuf_appendcstr(buf, "O=");
- break;
-
- case X520_ORG_UNIT:
- svn_stringbuf_appendcstr(buf, "OU=");
- break;
-
- default:
- temp = apr_psprintf(scratch_pool, "0x%02X=", name->oid.p[2]);
- svn_stringbuf_appendcstr(buf, temp);
- break;
+ if (starts_with(&name->oid, OID_X520, sizeof(OID_X520) - 1))
+ {
+ switch (name->oid.p[2]) {
+ case X520_COMMON_NAME:
+ svn_stringbuf_appendcstr(buf, "CN=");
+ break;
+
+ case X520_COUNTRY:
+ svn_stringbuf_appendcstr(buf, "C=");
+ break;
+
+ case X520_LOCALITY:
+ svn_stringbuf_appendcstr(buf, "L=");
+ break;
+
+ case X520_STATE:
+ svn_stringbuf_appendcstr(buf, "ST=");
+ break;
+
+ case X520_ORGANIZATION:
+ svn_stringbuf_appendcstr(buf, "O=");
+ break;
+
+ case X520_ORG_UNIT:
+ svn_stringbuf_appendcstr(buf, "OU=");
+ break;
+
+ default:
+ temp = apr_psprintf(scratch_pool, "0x%02X=", name->oid.p[2]);
+ svn_stringbuf_appendcstr(buf, temp);
+ break;
+ }
}
- } else if (memcmp(name->oid.p, OID_PKCS9, 8) == 0) {
- switch (name->oid.p[8]) {
- case PKCS9_EMAIL:
- svn_stringbuf_appendcstr(buf, "emailAddress=");
- break;
-
- default:
- temp = apr_psprintf(scratch_pool, "0x%02X=", name->oid.p[8]);
- svn_stringbuf_appendcstr(buf, temp);
- break;
+ else if (starts_with(&name->oid, OID_PKCS9, sizeof(OID_PKCS9) - 1))
+ {
+ switch (name->oid.p[8]) {
+ case PKCS9_EMAIL:
+ svn_stringbuf_appendcstr(buf, "emailAddress=");
+ break;
+
+ default:
+ temp = apr_psprintf(scratch_pool, "0x%02X=", name->oid.p[8]);
+ svn_stringbuf_appendcstr(buf, temp);
+ break;
+ }
}
- } else
+ else
svn_stringbuf_appendcstr(buf, "\?\?=");
utf8_value = x509name_to_utf8_string(name, scratch_pool);
@@ -996,7 +1028,7 @@ x509parse_get_hostnames(apr_array_header
/* no SAN then get the hostname from the CommonName on the cert */
while (name != NULL)
{
- if (memcmp(name->oid.p, OID_X520, 2) == 0)
+ if (starts_with(&name->oid, OID_X520, sizeof(OID_X520) - 1))
{
if (name->oid.p[2] == X520_COMMON_NAME)
{
@@ -1155,7 +1187,7 @@ svn_x509_parse_cert(svn_x509_certinfo_t
*/
SVN_ERR(x509_get_alg(&p, end, &crt->sig_oid2));
- if (memcmp(crt->sig_oid1.p, crt->sig_oid2.p, 9) != 0)
+ if (!oids_equal(&crt->sig_oid1, &crt->sig_oid2))
return svn_error_create(SVN_ERR_X509_CERT_SIG_MISMATCH, NULL, NULL);
SVN_ERR(x509_get_sig(&p, end, &crt->sig));
Modified: subversion/branches/svn-auth-x509/subversion/tests/libsvn_subr/x509-test.c
URL: http://svn.apache.org/viewvc/subversion/branches/svn-auth-x509/subversion/tests/libsvn_subr/x509-test.c?rev=1618575&r1=1618574&r2=1618575&view=diff
==============================================================================
--- subversion/branches/svn-auth-x509/subversion/tests/libsvn_subr/x509-test.c (original)
+++ subversion/branches/svn-auth-x509/subversion/tests/libsvn_subr/x509-test.c Mon Aug 18 10:27:15 2014
@@ -359,6 +359,35 @@ static struct x509_test cert_tests[] = {
"*.example.com, *.foo.example.com, *.bar.example.com, zig-zag.example.com",
"3525fb617c232fdc738d736c1cbd5d97b19b51e4"
},
+ /* Cert with the signature algorithm OID set to sha1WithRSA instead of
+ * sha1WithRSAEncryption. Both have the same meaning but the sha1WithRSA
+ * doesn't seem to be used anymore and is shorter */
+ { "MIIDgDCCAmygAwIBAgIBATAJBgUrDgMCHQUAMEUxCzAJBgNVBAYTAkFVMRMwEQYD"
+ "VQQIFApTb21lLVN0YXRlMSEwHwYDVQQKFBhJbnRlcm5ldCBXaWRnaXRzIFB0eSBM"
+ "dGQwHhcNMTQwODE4MDk1OTQ1WhcNMTUwODE4MDk1OTQ1WjBNMQswCQYDVQQGEwJV"
+ "SzEQMA4GA1UECBQHRW5nbGFuZDESMBAGA1UEBxQJU2hlZmZpZWxkMRgwFgYDVQQD"
+ "FA93d3cuZXhhbXBsZS5jb20wggEiMA0GCSqGSIb3DQEBAQUAA4IBDwAwggEKAoIB"
+ "AQCkvtieKg33RSzhn5JMDPPRlDS8Q16CN96A4lLI9YrJCy33z46PrbR2mq2hOz5l"
+ "MdgbAaRF0MUGhcKv4msJ0bsWhkybaSBAVgnoC7ObQWPNF7ppMzUjeDAlUBXNfheR"
+ "ZcgcgGWqUkoB1uUMhvmVuPrzvxn+WCwyoP6zQCviYLsR8AygGQgdhV6c9wJ/x9HS"
+ "MRUvUOeo7SCmx9GK5Hc11QV2K3rwKXABeAxXNzbyQe7hFfQYCI2SB5s3bEnhIvg7"
+ "BG0BQmoprHjXWBftc0+msKQTFw7+jZ21NsfwGoPonuVsCOJjJ51jp2oKqk3b1GGc"
+ "DEmmMQ0JtqfHO5a7JACBaHbTAgMBAAGjezB5MAkGA1UdEwQCMAAwLAYJYIZIAYb4"
+ "QgENBB8WHU9wZW5TU0wgR2VuZXJhdGVkIENlcnRpZmljYXRlMB0GA1UdDgQWBBSo"
+ "jICtcIgZL6OCCB5BJ5PGf1UIyTAfBgNVHSMEGDAWgBT5KQMLMylrXSQvhMtONHZc"
+ "22Jm9TAJBgUrDgMCHQUAA4IBAQCvCJ4i2kRzSRhnlDxd0UbQtytVIJFFJlfREPTM"
+ "j8+VqqtCVyPSX8T5NU+HCiEmhVrTlm/W0i8ygJXr8izyIMGRqbyhn2M9b8hAY6Jl"
+ "0edztu/FV/YHsJbPznWkXWpMMaXDEX4wI329f5odccIbB5VSaaoAdKZ6Ne4nf6oV"
+ "95KRFWkXoYjm24TnpALsNnK1Kjjed6h5ApB+IANOpXYFbGcsfbuKhWbFd2nd6t5U"
+ "NpUcv4H9Tgdl6KgrfsbQtAeouWCgoiNzrul8FOaQTdJLZfCsjuE+IkGpM+DX8PiF"
+ "5M41EqkSKia8sChFIln+lkRY41OWP9uQ1VXCfdRIzOnXWh9U",
+ "C=UK, ST=England, L=Sheffield, CN=www.example.com",
+ "C=AU, ST=Some-State, O=Internet Widgits Pty Ltd",
+ "2014-08-18T09:59:45.000000Z",
+ "2015-08-18T09:59:45.000000Z",
+ "www.example.com",
+ "0e0869961d508b13bb22aa8da675b2e9951c0e70"
+ },
{ NULL }
};