subversion-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From bre...@apache.org
Subject svn commit: r1658631 - in /subversion/trunk/subversion: include/svn_x509.h libsvn_subr/x509.h libsvn_subr/x509info.c libsvn_subr/x509parse.c tests/libsvn_subr/x509-test.c
Date Tue, 10 Feb 2015 08:12:58 GMT
Author: breser
Date: Tue Feb 10 08:12:58 2015
New Revision: 1658631

URL: http://svn.apache.org/r1658631
Log:
Resolve some edge cases of parsing X.509 certificates.

Multiple attributes of the a Subject or Issuer with the same object id.
Overflow of an object id segment.
Leading zeros in an object id segment.

* subversion/include/svn_x509.h
  (SVN_X509_OID_COMMON_NAME, SVN_X509_OID_COUNTRY, SVN_X509_OID_LOCALITY,
   SVN_X509_OID_STATE, SVN_X509_OID_ORGANIZATION, SVN_X509_OID_ORG_UNIT,
   SVN_X509_OID_EMAIL): Change from string representations to DER encoded
    object ids.
  (svn_x509_name_attr_t): New opaque struct for storing an attribute
    object id and value pair.
  (svn_x509_name_attr_dup, svn_x509_name_attr_get_oid,
   svn_x509_name_attr_get_value, svn_x509_certinfo_get_subject_attrs,
   svn_x509_certinfo_get_issuer_attrs, svn_x509_oid_to_string): New functions.
  (svn_x509_certinfo_get_subject_oids, svn_x509_certinfo_get_issuer_oids):
    Remove.

* subversion/libsvn_subr/x509.h
  (svn_x509_name_attr_t): Add struct.
  (svn_x509_certinfo_t): Remove issuer_oids and subject_oids members and
    make issuer and subject members into an array.

* subversion/libsvn_subr/x509info.c
  (svn_x509_name_attr_dup, deep_copy_name_attrs,
   svn_x509_certinfo_get_subject_attrs,
   svn_x509_certinfo_get_issuer_attrs): New functions.
  (deep_copy_hash, svn_x509_certinfo_get_subject_oids,
   svn_x509_certinfo_get_subject_attr, svn_x509_certinfo_get_issuer_oids,
   svn_x509_certinfo_get_issuer_attr): Remove functions.
  (svn_x509_certinfo_dup): Update to reflect changes to the certinfo
    struct.
  (asn1_oid): Use the DER encoding of the object id and not a string of dotted
    decimal values for the oid.
  (CONSTANT_PAIR): Convenience macro.
  (asn1_oids): Adjust for the changes to the struct.
  (svn_x509_oid_to_string): Moved from asn1_oid_to_strin() and deal with
    overflows and leading zeros.
  (oid_string_to_asn1_oid): Rename to ...
  (oid_to_asn1_oid): ... and adjust to use DER oids instead of strings.
  (oid_string_to_best_label): Rename to ...
  (oid_to_best_label): ... and adjust to use DER oids instead of strings.
  (get_dn, svn_x509_certinfo_get_subject, svn_x509_certinfo_get_issuer):
    Adjust to use the new array of svn_x509_name_attr_t instead of an array and
    hash for the attributes.

* subversion/libsvn_subr/x509parse.c
  (asn1_oid_to_string): Moved to svn_x509_oid_to_string().
  (x509_name_to_certinfo): Adjust to reflect the change to the certinfo struct.
  (x509parse_get_cn): New function to retrieve the common name.
  (x509parse_get_hostnames): Use x509parse_get_cn().
  (svn_x509_parse_cert): Reflect changes to certinfo struct.

* subversion/tests/libsvn_subr/x509-test.c
  (cert_tests): Add tests for edge cases repaired by this commit.
  (compare_oids, compare_results): Update to reflect the API changes.
  (broken_cert_tests, test_x509_parse_cert_broken, test_funcs): Disable the XFAIL
    test setup since we have none now.



Modified:
    subversion/trunk/subversion/include/svn_x509.h
    subversion/trunk/subversion/libsvn_subr/x509.h
    subversion/trunk/subversion/libsvn_subr/x509info.c
    subversion/trunk/subversion/libsvn_subr/x509parse.c
    subversion/trunk/subversion/tests/libsvn_subr/x509-test.c

Modified: subversion/trunk/subversion/include/svn_x509.h
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/include/svn_x509.h?rev=1658631&r1=1658630&r2=1658631&view=diff
==============================================================================
--- subversion/trunk/subversion/include/svn_x509.h (original)
+++ subversion/trunk/subversion/include/svn_x509.h Tue Feb 10 08:12:58 2015
@@ -38,13 +38,13 @@
 extern "C" {
 #endif
 
-#define SVN_X509_OID_COMMON_NAME  "2.5.4.3"
-#define SVN_X509_OID_COUNTRY      "2.5.4.6"
-#define SVN_X509_OID_LOCALITY     "2.5.4.7"
-#define SVN_X509_OID_STATE        "2.5.4.8"
-#define SVN_X509_OID_ORGANIZATION "2.5.4.10"
-#define SVN_X509_OID_ORG_UNIT     "2.5.4.11"
-#define SVN_X509_OID_EMAIL        "1.2.840.113549.1.9.1"
+#define SVN_X509_OID_COMMON_NAME  "\x55\x04\x03"
+#define SVN_X509_OID_COUNTRY      "\x55\x04\x06"
+#define SVN_X509_OID_LOCALITY     "\x55\x04\x07"
+#define SVN_X509_OID_STATE        "\x55\x04\x08"
+#define SVN_X509_OID_ORGANIZATION "\x55\x04\x0A"
+#define SVN_X509_OID_ORG_UNIT     "\x55\x04\x0B"
+#define SVN_X509_OID_EMAIL        "\x2A\x86\x48\x86\xF7\x0D\x01\x09\x01"
 
 /**
  * Representation of parsed certificate info.
@@ -54,6 +54,13 @@ extern "C" {
 typedef struct svn_x509_certinfo_t svn_x509_certinfo_t;
 
 /**
+ * Representation of an atttribute in an X.509 name (e.g. Subject or Issuer)
+ *
+ * @since New in 1.9.
+ */
+typedef struct svn_x509_name_attr_t svn_x509_name_attr_t;
+
+/**
  * Parse x509 @a der certificate data from @a buf with length @a
  * buflen and return certificate information in @a *certinfo,
  * allocated in @a result_pool.
@@ -77,6 +84,32 @@ svn_x509_parse_cert(svn_x509_certinfo_t
                     apr_pool_t *scratch_pool);
 
 /**
+ * Returns a deep copy of the @a attr, allocated in @a result_pool.
+ * May use @a scratch_pool for temporary allocations.
+ * @since New in 1.9.
+ */
+svn_x509_name_attr_t *
+svn_x509_name_attr_dup(const svn_x509_name_attr_t *attr,
+                       apr_pool_t *result_pool,
+                       apr_pool_t *scratch_pool);
+
+/**
+ * Returns the OID of @a attr as encoded in the certificate.  The
+ * length of the OID will be set in @a len.
+ * @since New in 1.9.
+ */
+const unsigned char *
+svn_x509_name_attr_get_oid(const svn_x509_name_attr_t *attr, apr_size_t *len);
+
+/**
+ * Returns the value of @a attr as a UTF-8 C string.
+ * @since New in 1.9.
+ */
+const char *
+svn_x509_name_attr_get_value(const svn_x509_name_attr_t *attr);
+
+
+/**
  * Returns a deep copy of @a certinfo, allocated in @a result_pool.
  * May use @a scratch_pool for temporary allocations.
  * @since New in 1.9.
@@ -95,25 +128,13 @@ svn_x509_certinfo_get_subject(const svn_
                               apr_pool_t *result_pool);
 
 /**
- * Returns a list of the the object IDs of the attributes available
- * for the subject in the @a certinfo.  The oids in the list are C
- * strings with dot separated integers.
+ * Returns a list of the attributes for the subject in the @a certinfo.
+ * Each member of the list is of type svn_x509_name_attr_t.
  *
  * @since New in 1.9.
  */
 const apr_array_header_t *
-svn_x509_certinfo_get_subject_oids(const svn_x509_certinfo_t *certinfo);
-
-/**
- * Returns the value of the attribute with the object ID specified in
- * @a oid of the subject from @a certinfo.  @a oid is a string of dot
- * separated integers.
- *
- * @since New in 1.9.
- */
-const char *
-svn_x509_certinfo_get_subject_attr(const svn_x509_certinfo_t *certinfo,
-                                   const char *oid);
+svn_x509_certinfo_get_subject_attrs(const svn_x509_certinfo_t *certinfo);
 
 /**
  * Returns the cerficiate issuer DN from @a certinfo.
@@ -124,25 +145,13 @@ svn_x509_certinfo_get_issuer(const svn_x
                              apr_pool_t *result_pool);
 
 /**
- * Returns a list of the the object IDs of the attributes available
- * for the issuer in the @a certinfo.  The oids in the list are C
- * strings with dot separated integers.
+ * Returns a list of the attributes for the issuer in the @a certinfo.
+ * Each member of the list is of type svn_x509_name_attr_t.
  *
  * @since New in 1.9.
  */
 const apr_array_header_t *
-svn_x509_certinfo_get_issuer_oids(const svn_x509_certinfo_t *certinfo);
-
-/**
- * Returns the value of the attribute with the object ID specified in
- * @a oid of the issuer from @a certinfo.  @a oid is a string of dot
- * separated integers.
- *
- * @since New in 1.9.
- */
-const char *
-svn_x509_certinfo_get_issuer_attr(const svn_x509_certinfo_t *certinfo,
-                                  const char *oid);
+svn_x509_certinfo_get_issuer_attrs(const svn_x509_certinfo_t *certinfo);
 
 /**
  * Returns the start of the certificate validity period from @a certinfo.
@@ -175,6 +184,17 @@ svn_x509_certinfo_get_digest(const svn_x
 const apr_array_header_t *
 svn_x509_certinfo_get_hostnames(const svn_x509_certinfo_t *certinfo);
 
+/**
+ * Given an @a oid return a null-terminated C string representation.
+ * For example an OID with the bytes "\x2A\x86\x48\x86\xF7\x0D\x01\x09\x01"
+ * would be converted to the string "1.2.840.113549.1.9.1".  Returns
+ * NULL if the @oid can't be represented as a string.
+ *
+ * @since New in 1.9. */
+const char *
+svn_x509_oid_to_string(const unsigned char *oid, apr_size_t oid_len,
+                       apr_pool_t *scratch_pool, apr_pool_t *result_pool);
+
 #ifdef __cplusplus
 }
 #endif

Modified: subversion/trunk/subversion/libsvn_subr/x509.h
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_subr/x509.h?rev=1658631&r1=1658630&r2=1658631&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_subr/x509.h (original)
+++ subversion/trunk/subversion/libsvn_subr/x509.h Tue Feb 10 08:12:58 2015
@@ -108,15 +108,19 @@ typedef struct _x509_cert {
 } x509_cert;
 
 
+struct svn_x509_name_attr_t {
+  unsigned char *oid;
+  apr_size_t oid_len;
+  const char *utf8_value;
+};
+
 /*
  * Certificate info, returned from the parser
  */
 struct svn_x509_certinfo_t
 {
-  apr_array_header_t *issuer_oids;
-  apr_hash_t *issuer;
-  apr_array_header_t *subject_oids;
-  apr_hash_t *subject;
+  apr_array_header_t *issuer;
+  apr_array_header_t *subject;
   apr_time_t valid_from;
   apr_time_t valid_to;
   svn_checksum_t *digest;

Modified: subversion/trunk/subversion/libsvn_subr/x509info.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_subr/x509info.c?rev=1658631&r1=1658630&r2=1658631&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_subr/x509info.c (original)
+++ subversion/trunk/subversion/libsvn_subr/x509info.c Tue Feb 10 08:12:58 2015
@@ -34,6 +34,33 @@
 
 
 
+svn_x509_name_attr_t *
+svn_x509_name_attr_dup(const svn_x509_name_attr_t *attr,
+                       apr_pool_t *result_pool,
+                       apr_pool_t *scratch_pool)
+{
+  svn_x509_name_attr_t *result = apr_palloc(result_pool, sizeof(*result));
+  result->oid_len = attr->oid_len;
+  result->oid = apr_palloc(result_pool, result->oid_len);
+  memcpy(result->oid, attr->oid, result->oid_len);
+  result->utf8_value = apr_pstrdup(result_pool, attr->utf8_value);
+
+  return result;
+}
+
+const unsigned char *
+svn_x509_name_attr_get_oid(const svn_x509_name_attr_t *attr, apr_size_t *len)
+{
+  *len = attr->oid_len;
+  return attr->oid;
+}
+
+const char *
+svn_x509_name_attr_get_value(const svn_x509_name_attr_t *attr)
+{
+  return attr->utf8_value;
+}
+
 /* Array elements are assumed to be nul-terminated C strings. */
 static apr_array_header_t *
 deep_copy_array(apr_array_header_t *s, apr_pool_t *result_pool)
@@ -56,31 +83,24 @@ deep_copy_array(apr_array_header_t *s, a
   return d;
 }
 
-/* Hash key and value are assumed to be nul-terminated C strings. */
-static apr_hash_t *deep_copy_hash(apr_hash_t *s,
-                                  apr_pool_t *scratch_pool, 
-                                  apr_pool_t *result_pool)
+/* Copy an array with elements that are svn_x509_name_attr_t's */
+static apr_array_header_t *
+deep_copy_name_attrs(apr_array_header_t *s, apr_pool_t *result_pool)
 {
-  apr_hash_t *d;
-  apr_hash_index_t *i;
+  int i;
+  apr_array_header_t *d;
 
   if (!s)
     return NULL;
 
-  d = apr_hash_make(result_pool);
-  i = apr_hash_first(scratch_pool, s);
+  d = apr_array_copy(result_pool, s);
 
-  /* Make a deep copy of the hash keys and values. */
-  while (i)
+  /* Make a deep copy of the svn_x509_name_attr_t's in the array. */
+  for (i = 0; i < s->nelts; ++i)
     {
-      const void *key;
-      void *val;
-      apr_ssize_t klen;
-
-      apr_hash_this(i, &key, &klen, &val);
-      apr_hash_set(d, apr_pstrndup(result_pool, key, klen), klen,
-                   apr_pstrdup(result_pool, val));
-      i = apr_hash_next(i);
+      APR_ARRAY_IDX(d, i, const svn_x509_name_attr_t *) =
+        svn_x509_name_attr_dup(APR_ARRAY_IDX(s, i, svn_x509_name_attr_t *),
+                               result_pool, result_pool);
     }
 
   return d;
@@ -92,10 +112,8 @@ svn_x509_certinfo_dup(const svn_x509_cer
                       apr_pool_t *scratch_pool)
 {
   svn_x509_certinfo_t *result = apr_palloc(result_pool, sizeof(*result));
-  result->subject_oids = deep_copy_array(certinfo->subject_oids, result_pool);
-  result->subject = deep_copy_hash(certinfo->subject, scratch_pool, result_pool);
-  result->issuer_oids = deep_copy_array(certinfo->issuer_oids, result_pool);
-  result->issuer = deep_copy_hash(certinfo->issuer, scratch_pool, result_pool);
+  result->subject = deep_copy_name_attrs(certinfo->subject, result_pool);
+  result->issuer = deep_copy_name_attrs(certinfo->issuer, result_pool);
   result->valid_from = certinfo->valid_from;
   result->valid_to = certinfo->valid_to;
   result->digest = svn_checksum_dup(certinfo->digest, result_pool);
@@ -105,49 +123,134 @@ svn_x509_certinfo_dup(const svn_x509_cer
 }
 
 typedef struct asn1_oid {
-  const char *oid_string;
+  const unsigned char *oid;
+  const ptrdiff_t oid_len;
   const char *short_label;
   const char *long_label;
 } asn1_oid;
 
+#define CONSTANT_PAIR(c) (unsigned char *)(c), sizeof((c)) - 1
+
 static const asn1_oid asn1_oids[] = {
-  { SVN_X509_OID_COMMON_NAME,  "CN", "commonName" },
-  { SVN_X509_OID_COUNTRY,      "C",  "countryName" },
-  { SVN_X509_OID_LOCALITY,     "L",  "localityName" },
-  { SVN_X509_OID_STATE,        "ST", "stateOrProvinceName" },
-  { SVN_X509_OID_ORGANIZATION, "O",  "organizationName" },
-  { SVN_X509_OID_ORG_UNIT,     "OU", "organizationalUnitName"},
-  { SVN_X509_OID_EMAIL,        NULL, "emailAddress" },
+  { CONSTANT_PAIR(SVN_X509_OID_COMMON_NAME),  "CN", "commonName" },
+  { CONSTANT_PAIR(SVN_X509_OID_COUNTRY),      "C",  "countryName" },
+  { CONSTANT_PAIR(SVN_X509_OID_LOCALITY),     "L",  "localityName" },
+  { CONSTANT_PAIR(SVN_X509_OID_STATE),        "ST", "stateOrProvinceName" },
+  { CONSTANT_PAIR(SVN_X509_OID_ORGANIZATION), "O",  "organizationName" },
+  { CONSTANT_PAIR(SVN_X509_OID_ORG_UNIT),     "OU", "organizationalUnitName"},
+  { CONSTANT_PAIR(SVN_X509_OID_EMAIL),        NULL, "emailAddress" },
   { NULL },
 };
 
-static const asn1_oid *oid_string_to_asn1_oid(const char *oid_string)
+/* Given an OID return a null-terminated C string representation.
+ * For example an OID with the bytes "\x2A\x86\x48\x86\xF7\x0D\x01\x09\x01"
+ * would be converted to the string "1.2.840.113549.1.9.1". */
+const char *
+svn_x509_oid_to_string(const unsigned char *oid, apr_size_t oid_len,
+                       apr_pool_t *scratch_pool, apr_pool_t *result_pool)
 {
-  const asn1_oid *oid;
+  svn_stringbuf_t *out = svn_stringbuf_create_empty(result_pool);
+  const unsigned char *p = oid;
+  const unsigned char *end = p + oid_len;
+  const char *temp;
+
+  while (p != end) {
+    if (p == oid)
+      {
+        /* Handle decoding the first two values of the OID.  These values
+         * are encoded by taking the first value and adding 40 to it and
+         * adding the result to the second value, then placing this single
+         * value in the first byte of the output.  This is unambiguous since
+         * the first value is apparently limited to 0, 1 or 2 and the second
+         * is limited to 0 to 39. */
+        temp = apr_psprintf(scratch_pool, "%d.%d", *p / 40, *p % 40);
+        p++;
+      }
+    else if (*p < 128)
+      {
+        /* The remaining values if they're less than 128 are just 
+         * the number one to one encoded */
+        temp = apr_psprintf(scratch_pool, ".%d", *p);
+        p++;
+      }
+    else
+      {
+        /* Values greater than 128 are encoded as a series of 7 bit values
+         * with the left most bit set to indicate this encoding with the
+         * last octet missing the left most bit to finish out the series.. */
+        unsigned int collector = 0;
+        svn_boolean_t dot = FALSE;
+
+        do {
+          if (collector == 0 && *p == 0x80)
+            {
+              /* include leading zeros in the string representation
+                 technically not legal, but this seems nicer than just
+                 returning NULL */
+              if (!dot)
+                {
+                  svn_stringbuf_appendbyte(out, '.');
+                  dot = TRUE;
+                }
+              svn_stringbuf_appendbyte(out, '0');
+            }
+          else if (collector > UINT_MAX >> 7)
+            {
+              /* overflow */
+              return NULL;
+            }
+          collector = collector << 7 | (*(p++) & 0x7f);
+        } while (p != end && *p > 127);
+        if (collector > UINT_MAX >> 7)
+          return NULL; /* overflow */
+        collector = collector << 7 | *(p++);
+        temp = apr_psprintf(scratch_pool, "%s%d", dot ? "" : ".", collector);
+      }
+    svn_stringbuf_appendcstr(out, temp);
+  }
 
-  for (oid = asn1_oids; oid->oid_string; oid++)
+  if (svn_stringbuf_isempty(out))
+    return NULL;
+
+  return out->data;
+}
+
+static const asn1_oid *oid_to_asn1_oid(unsigned char *oid, apr_size_t oid_len)
+{
+  const asn1_oid *entry;
+
+  for (entry = asn1_oids; entry->oid; entry++)
     {
-      if (strcmp(oid_string, oid->oid_string) == 0)
-        return oid;
+      if (oid_len == entry->oid_len &&
+          memcmp(oid, entry->oid, oid_len) == 0)
+        return entry;
     }
 
   return NULL;
 }
 
-static const char *oid_string_to_best_label(const char *oid_string)
+static const char *oid_to_best_label(unsigned char *oid, apr_size_t oid_len,
+                                     apr_pool_t *result_pool)
 {
-  const asn1_oid *oid = oid_string_to_asn1_oid(oid_string);
+  const asn1_oid *entry = oid_to_asn1_oid(oid, oid_len);
 
-  if (oid)
+  if (entry)
     {
-      if (oid->short_label)
-        return oid->short_label;
+      if (entry->short_label)
+        return entry->short_label;
 
-      if (oid->long_label)
-        return oid->long_label;
+      if (entry->long_label)
+        return entry->long_label;
+    }
+  else
+    {
+      const char *oid_string = svn_x509_oid_to_string(oid, oid_len,
+                                                      result_pool, result_pool);
+      if (oid_string)
+        return oid_string;
     }
 
-  return oid_string;
+  return "??";
 }
 
 /*
@@ -156,23 +259,22 @@ static const char *oid_string_to_best_la
  * If CN is not NULL, return any common name in CN
  */
 static const char *
-get_dn(apr_array_header_t *oids,
-       apr_hash_t *hash,
+get_dn(apr_array_header_t *name,
        apr_pool_t *result_pool)
 {
   svn_stringbuf_t *buf = svn_stringbuf_create_empty(result_pool);
   int n;
 
-  for (n = 0; n < oids->nelts; n++)
+  for (n = 0; n < name->nelts; n++)
     {
-      const char *field = APR_ARRAY_IDX(oids, n, const char *);
+      const svn_x509_name_attr_t *attr = APR_ARRAY_IDX(name, n, svn_x509_name_attr_t *);
 
       if (n > 0)
         svn_stringbuf_appendcstr(buf, ", ");
 
-      svn_stringbuf_appendcstr(buf, oid_string_to_best_label(field));
+      svn_stringbuf_appendcstr(buf, oid_to_best_label(attr->oid, attr->oid_len, result_pool));
       svn_stringbuf_appendbyte(buf, '=');
-      svn_stringbuf_appendcstr(buf, svn_hash_gets(hash, field));
+      svn_stringbuf_appendcstr(buf, attr->utf8_value);
     }
 
   return buf->data;
@@ -182,40 +284,26 @@ const char *
 svn_x509_certinfo_get_subject(const svn_x509_certinfo_t *certinfo,
                               apr_pool_t *result_pool)
 {
-  return get_dn(certinfo->subject_oids, certinfo->subject, result_pool);
+  return get_dn(certinfo->subject, result_pool);
 }
 
 const apr_array_header_t *
-svn_x509_certinfo_get_subject_oids(const svn_x509_certinfo_t *certinfo)
+svn_x509_certinfo_get_subject_attrs(const svn_x509_certinfo_t *certinfo)
 {
-  return certinfo->subject_oids;
-}
-
-const char *
-svn_x509_certinfo_get_subject_attr(const svn_x509_certinfo_t *certinfo,
-                                   const char *oid)
-{
-  return svn_hash_gets(certinfo->subject, oid);
+  return certinfo->subject;
 }
 
 const char *
 svn_x509_certinfo_get_issuer(const svn_x509_certinfo_t *certinfo,
                              apr_pool_t *result_pool)
 {
-  return get_dn(certinfo->issuer_oids, certinfo->issuer, result_pool);
+  return get_dn(certinfo->issuer, result_pool);
 }
 
 const apr_array_header_t *
-svn_x509_certinfo_get_issuer_oids(const svn_x509_certinfo_t *certinfo)
-{
-  return certinfo->issuer_oids;
-}
-
-const char *
-svn_x509_certinfo_get_issuer_attr(const svn_x509_certinfo_t *certinfo,
-                                  const char *oid)
+svn_x509_certinfo_get_issuer_attrs(const svn_x509_certinfo_t *certinfo)
 {
-  return svn_hash_gets(certinfo->issuer, oid);
+  return certinfo->issuer;
 }
 
 apr_time_t
@@ -241,3 +329,4 @@ svn_x509_certinfo_get_hostnames(const sv
 {
   return certinfo->hostnames;
 }
+

Modified: subversion/trunk/subversion/libsvn_subr/x509parse.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_subr/x509parse.c?rev=1658631&r1=1658630&r2=1658631&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_subr/x509parse.c (original)
+++ subversion/trunk/subversion/libsvn_subr/x509parse.c Tue Feb 10 08:12:58 2015
@@ -903,90 +903,27 @@ x509name_to_utf8_string(const x509_name
   return nul_escape(utf8_string, result_pool);
 }
 
-/* Given an OID return a null-terminated C string representation in *RESULT.
- * For example an OID with the bytes "\x2A\x86\x48\x86\xF7\x0D\x01\x09\x01"
- * would be converted to the string "1.2.840.113549.1.9.1". */
 static svn_error_t *
-asn1_oid_to_string(const char **result, const x509_buf *oid,
-                   apr_pool_t *scratch_pool, apr_pool_t *result_pool)
-{
-  svn_stringbuf_t *out = svn_stringbuf_create_empty(result_pool);
-  const unsigned char *p = oid->p;
-  const unsigned char *end = p + oid->len;
-  const char *temp;
-
-  *result = NULL;
-
-  if (oid->tag != ASN1_OID)
-    return svn_error_create(SVN_ERR_ASN1_UNEXPECTED_TAG, NULL, NULL);
-
-  if (oid->len <= 0)
-    return svn_error_create(SVN_ERR_ASN1_OUT_OF_DATA, NULL, NULL);
-
-  while (p != end) {
-    if (p == oid->p)
-      {
-        /* Handle decoding the first two values of the OID.  These values
-         * are encoded by taking the first value and adding 40 to it and
-         * adding the result to the second value, then placing this single
-         * value in the first byte of the output.  This is unambiguous since
-         * the first value is apparently limited to 0, 1 or 2 and the second
-         * is limited to 0 to 39. */
-        temp = apr_psprintf(scratch_pool, "%d.%d", *p / 40, *p % 40);
-        p++;
-      }
-    else if (*p < 128)
-      {
-        /* The remaining values if they're less than 128 are just 
-         * the number one to one encoded */
-        temp = apr_psprintf(scratch_pool, ".%d", *p);
-        p++;
-      }
-    else
-      {
-        /* Values greater than 128 are encoded as a series of 7 bit values
-         * with the left most bit set to indicate this encoding with the
-         * last octet missing the left most bit to finish out the series.. */
-        int collector = 0;
-
-        do {
-          collector = collector << 7 | (*(p++) & 0x7f);
-        } while (p != end && *p > 127);
-        collector = collector << 7 | *(p++);
-        temp = apr_psprintf(scratch_pool, ".%d", collector);
-      }
-    svn_stringbuf_appendcstr(out, temp);
-  }
-
-  *result = out->data;
-  return SVN_NO_ERROR;
-}
-
-static svn_error_t *
-x509_name_to_certinfo(apr_array_header_t **oids,
-                      apr_hash_t **hash,
+x509_name_to_certinfo(apr_array_header_t **result,
                       const x509_name *dn,
                       apr_pool_t *scratch_pool,
                       apr_pool_t *result_pool)
 {
   const x509_name *name = dn;
 
-  *oids = apr_array_make(result_pool, 6, sizeof(const char *));
-  *hash = apr_hash_make(result_pool);
+  *result = apr_array_make(result_pool, 6, sizeof(svn_x509_name_attr_t *));
 
   while (name != NULL) {
-    const char *oid_string;
-    const char *utf8_value;
+    svn_x509_name_attr_t *attr = apr_palloc(result_pool, sizeof(svn_x509_name_attr_t));
 
-    SVN_ERR(asn1_oid_to_string(&oid_string, &name->oid,
-                               scratch_pool, result_pool));
-    APR_ARRAY_PUSH(*oids, const char *) = oid_string;
-    utf8_value = x509name_to_utf8_string(name, result_pool);
-    if (utf8_value)
-      svn_hash_sets(*hash, oid_string, utf8_value);
-    else
+    attr->oid_len = name->oid.len;
+    attr->oid = apr_palloc(result_pool, attr->oid_len);
+    memcpy(attr->oid, name->oid.p, attr->oid_len);
+    attr->utf8_value = x509name_to_utf8_string(name, result_pool);
+    if (!attr->utf8_value)
       /* this should never happen */
-      svn_hash_sets(*hash, oid_string, "??");
+      attr->utf8_value = apr_pstrdup(result_pool, "??");
+    APR_ARRAY_PUSH(*result, const svn_x509_name_attr_t *) = attr;
 
     name = name->next;
   }
@@ -1028,6 +965,23 @@ is_hostname(const char *str)
   return TRUE;
 }
 
+static const char *
+x509parse_get_cn(apr_array_header_t *subject)
+{
+  int i;
+
+  for (i = 0; i < subject->nelts; ++i)
+    {
+      const svn_x509_name_attr_t *attr = APR_ARRAY_IDX(subject, i, const svn_x509_name_attr_t
*);
+      if (equal(attr->oid, attr->oid_len,
+                SVN_X509_OID_COMMON_NAME, sizeof(SVN_X509_OID_COMMON_NAME) - 1))
+        return attr->utf8_value;
+    }
+
+  return NULL;
+}
+
+
 static void
 x509parse_get_hostnames(svn_x509_certinfo_t *ci, x509_cert *crt,
                         apr_pool_t *result_pool, apr_pool_t *scratch_pool)
@@ -1058,8 +1012,7 @@ x509parse_get_hostnames(svn_x509_certinf
       /* no SAN then get the hostname from the CommonName on the cert */
       const char *utf8_value;
 
-      utf8_value = svn_x509_certinfo_get_subject_attr(ci,
-                      SVN_X509_OID_COMMON_NAME);
+      utf8_value = x509parse_get_cn(ci->subject);
 
       if (utf8_value && is_hostname(utf8_value))
         {
@@ -1220,11 +1173,11 @@ svn_x509_parse_cert(svn_x509_certinfo_t
   ci = apr_pcalloc(result_pool, sizeof(*ci));
 
   /* Get the subject name */
-  SVN_ERR(x509_name_to_certinfo(&ci->subject_oids, &ci->subject, &crt->subject,
+  SVN_ERR(x509_name_to_certinfo(&ci->subject, &crt->subject,
                                 scratch_pool, result_pool));
 
   /* Get the issuer name */
-  SVN_ERR(x509_name_to_certinfo(&ci->issuer_oids, &ci->issuer, &crt->issuer,
+  SVN_ERR(x509_name_to_certinfo(&ci->issuer, &crt->issuer,
                                 scratch_pool, result_pool));
 
   /* Copy the validity range */

Modified: subversion/trunk/subversion/tests/libsvn_subr/x509-test.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/tests/libsvn_subr/x509-test.c?rev=1658631&r1=1658630&r2=1658631&view=diff
==============================================================================
--- subversion/trunk/subversion/tests/libsvn_subr/x509-test.c (original)
+++ subversion/trunk/subversion/tests/libsvn_subr/x509-test.c Tue Feb 10 08:12:58 2015
@@ -500,6 +500,98 @@ static struct x509_test cert_tests[] = {
     NULL,
     "99302ca2824f585a117bb41302a388daa0519765"
   },
+  /* certificate with subject that includes an attribute that has an
+   * object id that has leading zeros.  This isn't technically legal
+   * but a simplistic parser might parser it the same as an object
+   * id that doesn't have a leading zero.  In this case the object id
+   * with a leading zero could parse to the same object id as the
+   * Common Name.  Make sure we don't treat it as such. */
+  { "MIIDDjCCAfYCAQEwDQYJKoZIhvcNAQEFBQAwRTELMAkGA1UEBhMCQVUxEzARBgNV"
+    "BAgTClNvbWUtU3RhdGUxITAfBgNVBAoTGEludGVybmV0IFdpZGdpdHMgUHR5IEx0"
+    "ZDAeFw0xNTAxMjcwNzQ5MDhaFw0xNjAxMjcwNzQ5MDhaMFUxCzAJBgNVBAYTAlVT"
+    "MRMwEQYDVQQIEwpXYXNoaW5ndG9uMRMwEQYDVQQHEwpOb3J0aCBCZW5kMRwwGgYE"
+    "VQSAAxMSbm90YWNuLmV4YW1wbGUuY29tMIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8A"
+    "MIIBCgKCAQEAvXCJv0gr9d3GNYiukPrbse0FdXmuBx2mPf665WyZVHk9JiPnDcb2"
+    "ng8gHLgJe8izou6I0vN2iJgy91rUPvX9zA3qVhml+cboVY2jHCPWo/v5PQsXAgLV"
+    "5gVjp2POn3N0O1xcS1yNe249LkP0Di3kAMp5gkzdprm3fD3JDW1Q+ocQylnbjzG0"
+    "FtNQSUJLITvPXjR7ny46Fci2mv8scHOvlEXTK5/2RoBaoK2jWQimqGfFj1sr1vqZ"
+    "Wcb6NAdZso64Xg1V6CWX8zymlA7gAhTQWveq+ovUWcXpmR8aj9pYNuy0aZW3BANz"
+    "N6L0G7OZiVUvvzpfnn0V3Z/sR/iQs7q3nQIDAQABMA0GCSqGSIb3DQEBBQUAA4IB"
+    "AQACZwruCiesCRkT08AtHl0WQnQui58e9/7En+iqxNQO6+fx84SfWGcUFYZtvzdO"
+    "KkHNTs06km+471OjLSDcotRkdqO1JxQCkNxbrPat7T6FrO9n2JFivx6eijRqK/jB"
+    "cBYW92dK4BfXU4+FyeB2OIpyPjuqLU2j7S5p7qNU50i/1J7Qt669nXeaPINIfZdW"
+    "sDjjWkFR1VOgXS/zeu/GOxlQFmmcde+X/qkFI+L352VX7Ktf95j4ms4vG2yZgNfe"
+    "jbNb9a7LMcqlop/PlX5WBGv8GGKUNZO0LvukFYOULf1oL8VQsN0x/gRHGC7m9kVM"
+    "3hojWZDXAY4mYqdBCRX7/gkt",
+    "C=US, ST=Washington, L=North Bend, 2.5.4.03=notacn.example.com",
+    "2.5.4.6 2.5.4.8 2.5.4.7 2.5.4.03",
+    "C=AU, ST=Some-State, O=Internet Widgits Pty Ltd",
+    "2.5.4.6 2.5.4.8 2.5.4.10",
+    "2015-01-27T07:49:08.000000Z",
+    "2016-01-27T07:49:08.000000Z",
+    NULL,
+    "6f24b834ba00fb4ef863df63b8fbeddab25e4838"
+  },
+  /* certificate with subject that includes an attribute that has an
+   * object id that has an overflow such that it calculates to
+   * the same object id as the Common Name (2.5.4.3).  OpenSSL
+   * with its bignum support shows this as 2.5.4.2361183241434822606851.
+   * It would be wrong to display this as a Common Name to the user. */
+  { "MIIDGTCCAgECAQEwDQYJKoZIhvcNAQEFBQAwRTELMAkGA1UEBhMCQVUxEzARBgNV"
+    "BAgTClNvbWUtU3RhdGUxITAfBgNVBAoTGEludGVybmV0IFdpZGdpdHMgUHR5IEx0"
+    "ZDAeFw0xNTAxMjcwODMxNDNaFw0xNjAxMjcwODMxNDNaMGAxCzAJBgNVBAYTAlVT"
+    "MRMwEQYDVQQIEwpXYXNoaW5ndG9uMRMwEQYDVQQHEwpOb3J0aCBCZW5kMScwJQYN"
+    "VQSCgICAgICAgICAAxMUb3ZlcmZsb3cuZXhhbXBsZS5jb20wggEiMA0GCSqGSIb3"
+    "DQEBAQUAA4IBDwAwggEKAoIBAQDHL1e8zSPyRND3tI42Vqca2FoCiWn881Czv2ct"
+    "tGFwyjUM8R1yHXEP+doS9KN9L29xRWZRxyCQ18S+QbjNQCh6Ay22qnkBu0uPdVB6"
+    "iIVKiW9RzU8dZSFMnveUZYLloG12kK++ooJGIstTJwkI8Naw1X1D29gZaY9oSKAc"
+    "Gs5c92po61RoetB744dUfUbAXi8eEd4ShdsdnCoswpEI4WTLdYLZ/cH/sU1a5Djm"
+    "cAfEBzZSOseEQSG7Fa/HvHyW+jDNnKG2r73M45TDcXAunSFcAYl1ioBaRwwdcTbK"
+    "SMGORThIX5UwpJDZI5sTVmTTRuCjbMxXXki/g9fTYD6mlaavAgMBAAEwDQYJKoZI"
+    "hvcNAQEFBQADggEBABvZSzFniMK4lqJcubzzk410NqZQEDBxdNZTNGrQYIDV8fDU"
+    "LLoQ2/2Y6kOQbx8r3RNcaJ6JtJeVqAq05It9oR5lMJFA2r0YMl4eB2V6o35+eaKY"
+    "FXrJzwx0rki2mX+iKsgRbJTv6mFb4I7vny404WKHNgYIfB8Z5jgbwWgrXH9M6BMb"
+    "FL9gZHMmU+6uqvCPYeIIZaAjT4J4E9322gpcumI9KGVApmbQhi5lC1hBh+eUprG7"
+    "4Brl9GeCLSTnTTf4GHIpqaUsKMtJ1sN/KJGwEB7Z4aszr80P5/sjHXOyqJ78tx46"
+    "pwH7/Fx0pM7nZjJVGvcxGBBOMeKy/o2QUVvEYPU=",
+    "C=US, ST=Washington, L=North Bend, \?\?=overflow.example.com",
+    "2.5.4.6 2.5.4.8 2.5.4.7 \?\?",
+    "C=AU, ST=Some-State, O=Internet Widgits Pty Ltd",
+    "2.5.4.6 2.5.4.8 2.5.4.10",
+    "2015-01-27T08:31:43.000000Z",
+    "2016-01-27T08:31:43.000000Z",
+    NULL,
+    "c1f063daf23e402fe58bab1a3fa2ba05c1106158"
+  },
+  /* certificate with multiple common names, make sure this behaves
+   * the same way as serf. */
+  { "MIIDJjCCAg4CAQEwDQYJKoZIhvcNAQEFBQAwRTELMAkGA1UEBhMCQVUxEzARBgNV"
+    "BAgTClNvbWUtU3RhdGUxITAfBgNVBAoTGEludGVybmV0IFdpZGdpdHMgUHR5IEx0"
+    "ZDAeFw0xNTAxMjExNzUwMDZaFw0xNjAxMjExNzUwMDZaMG0xCzAJBgNVBAYTAlVT"
+    "MRMwEQYDVQQIEwpXYXNoaW5ndG9uMRMwEQYDVQQHEwpOb3J0aCBCZW5kMRkwFwYD"
+    "VQQDExBnb29kLmV4YW1wbGUuY29tMRkwFwYDVQQDExBldmlsLmV4YW1wbGUuY29t"
+    "MIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEA5pfrXkiiDGCWSYhMQNHJ"
+    "gNBLEBNcFzsGpW8i6rMKVephwG7p4VqIvc0pSsmpD9IYuIxxq/2E2cziaTWyqCBp"
+    "hKKipqt8eMcu6u45LduHGiCcnN7rHORbQZTdvwzTmiVN1eI1oCVejB4zgHNkHUko"
+    "DyaALCHGRz8l7Qq6hSbiOnhH1qlscIIEsgQEyDlMZpbsWVTQKPxluhtgqVEn7wPN"
+    "qScrf2evq050NuNYYFzCmuqOGKq2gKbD/BlUqCNmEM2JPg/bdcAQxFCf0HcvDiS9"
+    "e29suMKWZAzJkbzrWhlDMG1Xt5c7dd82PcGwnL//Q7muE57luCw38Gp2vQQ3/Uki"
+    "vQIDAQABMA0GCSqGSIb3DQEBBQUAA4IBAQBry9wfxYia/dCSKvDXOBKUgWFQtI8j"
+    "7vYHuouTvIb5m6b62kiUdtuaVKi3jnUbHUFohOi/6o+HIwbXSgz5CbiLjgUvONBU"
+    "BLekaguIYX9tTmg+vhWchcmVMHufj6HdQkzWtyojSQD9GjHGInNDG102KlN1cdL8"
+    "jGTrru4vnef+xA24EvYPdcS2+H2yYH0THL3JPKo1GtO4NCEGWQbS6Ygwcy+BQpbU"
+    "TBIWhlbleuCalB8qhWyijcHeszT7mFR0CarEaSLeZj6FaQpZB636iHuELmxcgiFw"
+    "j3r3QZyAMEGvPPBPKYSTgmol31pX9LYvuFGA9ADQ2in/n9WdMfYzFzOn",
+    "C=US, ST=Washington, L=North Bend, "
+    "CN=good.example.com, CN=evil.example.com",
+    "2.5.4.6 2.5.4.8 2.5.4.7 2.5.4.3 2.5.4.3",
+    "C=AU, ST=Some-State, O=Internet Widgits Pty Ltd",
+    "2.5.4.6 2.5.4.8 2.5.4.10",
+    "2015-01-21T17:50:06.000000Z",
+    "2016-01-21T17:50:06.000000Z",
+    "good.example.com",
+    "9693f17e59205f41ca2e14450d151b945651b2d7"
+  },
   { NULL }
 };
 
@@ -593,10 +685,16 @@ compare_oids(const char *expected,
   buf = svn_stringbuf_create_empty(pool);
   for (i = 0; i < actual->nelts; ++i)
     {
-      const char *oid = APR_ARRAY_IDX(actual, i, const char*);
+      apr_size_t len;
+      const svn_x509_name_attr_t *attr = APR_ARRAY_IDX(actual, i, const svn_x509_name_attr_t
*);
+      const void *oid = svn_x509_name_attr_get_oid(attr, &len);
+      const char *oid_string = svn_x509_oid_to_string(oid, len, pool, pool);
       if (i > 0)
         svn_stringbuf_appendbyte(buf, ' ');
-      svn_stringbuf_appendcstr(buf, oid);
+      if (oid_string)
+        svn_stringbuf_appendcstr(buf, oid_string);
+      else
+        svn_stringbuf_appendcstr(buf, "??");
     }
 
   if (strcmp(expected, buf->data))
@@ -626,7 +724,7 @@ compare_results(struct x509_test *xt,
                              "expected '%s', got '%s'", xt->subject,
                              xt->subject, v);
 
-  SVN_ERR(compare_oids(xt->subject_oids, svn_x509_certinfo_get_subject_oids(certinfo),
+  SVN_ERR(compare_oids(xt->subject_oids, svn_x509_certinfo_get_subject_attrs(certinfo),
                        xt->subject, pool));
 
   v = svn_x509_certinfo_get_issuer(certinfo, pool);
@@ -639,7 +737,7 @@ compare_results(struct x509_test *xt,
                              "expected '%s', got '%s'", xt->subject,
                              xt->issuer, v);
 
-  SVN_ERR(compare_oids(xt->issuer_oids, svn_x509_certinfo_get_issuer_oids(certinfo),
+  SVN_ERR(compare_oids(xt->issuer_oids, svn_x509_certinfo_get_issuer_attrs(certinfo),
                        xt->subject, pool));
 
   SVN_ERR(compare_dates(xt->valid_from,
@@ -700,39 +798,9 @@ test_x509_parse_cert(apr_pool_t *pool)
   return SVN_NO_ERROR;
 }
 
+#if 0
 static struct x509_test broken_cert_tests[] = {
-  /* certificate with subject that includes an attribute that has a
-   * object id that has and overflow such that it calculates to
-   * the same object id as the Common Name (2.5.4.3).  OpenSSL
-   * with its bignum support shows this as 2.5.4.2361183241434822606851.
-   * It would be wrong to display this as a Common Name to the user. */
-  { "MIIDGTCCAgECAQEwDQYJKoZIhvcNAQEFBQAwRTELMAkGA1UEBhMCQVUxEzARBgNV"
-    "BAgTClNvbWUtU3RhdGUxITAfBgNVBAoTGEludGVybmV0IFdpZGdpdHMgUHR5IEx0"
-    "ZDAeFw0xNTAxMjcwODMxNDNaFw0xNjAxMjcwODMxNDNaMGAxCzAJBgNVBAYTAlVT"
-    "MRMwEQYDVQQIEwpXYXNoaW5ndG9uMRMwEQYDVQQHEwpOb3J0aCBCZW5kMScwJQYN"
-    "VQSCgICAgICAgICAAxMUb3ZlcmZsb3cuZXhhbXBsZS5jb20wggEiMA0GCSqGSIb3"
-    "DQEBAQUAA4IBDwAwggEKAoIBAQDHL1e8zSPyRND3tI42Vqca2FoCiWn881Czv2ct"
-    "tGFwyjUM8R1yHXEP+doS9KN9L29xRWZRxyCQ18S+QbjNQCh6Ay22qnkBu0uPdVB6"
-    "iIVKiW9RzU8dZSFMnveUZYLloG12kK++ooJGIstTJwkI8Naw1X1D29gZaY9oSKAc"
-    "Gs5c92po61RoetB744dUfUbAXi8eEd4ShdsdnCoswpEI4WTLdYLZ/cH/sU1a5Djm"
-    "cAfEBzZSOseEQSG7Fa/HvHyW+jDNnKG2r73M45TDcXAunSFcAYl1ioBaRwwdcTbK"
-    "SMGORThIX5UwpJDZI5sTVmTTRuCjbMxXXki/g9fTYD6mlaavAgMBAAEwDQYJKoZI"
-    "hvcNAQEFBQADggEBABvZSzFniMK4lqJcubzzk410NqZQEDBxdNZTNGrQYIDV8fDU"
-    "LLoQ2/2Y6kOQbx8r3RNcaJ6JtJeVqAq05It9oR5lMJFA2r0YMl4eB2V6o35+eaKY"
-    "FXrJzwx0rki2mX+iKsgRbJTv6mFb4I7vny404WKHNgYIfB8Z5jgbwWgrXH9M6BMb"
-    "FL9gZHMmU+6uqvCPYeIIZaAjT4J4E9322gpcumI9KGVApmbQhi5lC1hBh+eUprG7"
-    "4Brl9GeCLSTnTTf4GHIpqaUsKMtJ1sN/KJGwEB7Z4aszr80P5/sjHXOyqJ78tx46"
-    "pwH7/Fx0pM7nZjJVGvcxGBBOMeKy/o2QUVvEYPU=",
-    "C=US, ST=Washington, L=North Bend, \?\?=overflow.example.com",
-    "2.5.4.6 2.5.4.8 2.5.4.7 2.5.4.3",
-    "C=AU, ST=Some-State, O=Internet Widgits Pty Ltd",
-    "2.5.4.6 2.5.4.8 2.5.4.10",
-    "2015-01-27T08:31:43.000000Z",
-    "2016-01-27T08:31:43.000000Z",
-    NULL,
-    "c1f063daf23e402fe58bab1a3fa2ba05c1106158"
-  },
-  { NULL }
+ { NULL }
 };
 
 static svn_error_t *
@@ -761,6 +829,7 @@ test_x509_parse_cert_broken(apr_pool_t *
 
   return SVN_NO_ERROR;
 }
+#endif
 
 /* The test table.  */
 
@@ -771,8 +840,8 @@ static struct svn_test_descriptor_t test
     SVN_TEST_NULL,
     SVN_TEST_PASS2(test_x509_parse_cert,
                    "test svn_x509_parse_cert"),
-    SVN_TEST_XFAIL2(test_x509_parse_cert_broken,
-                    "test broken certs"),
+/*    SVN_TEST_XFAIL2(test_x509_parse_cert_broken,
+                    "test broken certs"), */
     SVN_TEST_NULL
   };
 



Mime
View raw message