You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@subversion.apache.org by rh...@apache.org on 2013/10/25 12:56:22 UTC

svn commit: r1535676 - /subversion/trunk/subversion/libsvn_ra_serf/util.c

Author: rhuijben
Date: Fri Oct 25 10:56:22 2013
New Revision: 1535676

URL: http://svn.apache.org/r1535676
Log:
Following up on r1534149, extract the hostname verification from the
certificate detail processing and perform this before bailing for the no
reported failure case.

* subversion/libsvn_ra_serf/util.c
  (ssl_server_cert): Revert regression since r1535659, where a proper root
    certificate higher in the chain (via Windows or openssl config) would
    break the hostname check. Tweak logic to properly handle more incomplete
    certificate variants.

Modified:
    subversion/trunk/subversion/libsvn_ra_serf/util.c

Modified: subversion/trunk/subversion/libsvn_ra_serf/util.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_ra_serf/util.c?rev=1535676&r1=1535675&r2=1535676&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_ra_serf/util.c (original)
+++ subversion/trunk/subversion/libsvn_ra_serf/util.c Fri Oct 25 10:56:22 2013
@@ -271,21 +271,70 @@ ssl_server_cert(void *baton, int failure
   svn_auth_iterstate_t *state;
   const char *realmstring;
   apr_uint32_t svn_failures;
-  apr_hash_t *issuer, *subject, *serf_cert;
-  apr_array_header_t *san;
+  apr_hash_t *issuer;
+  apr_hash_t *subject = NULL;
+  apr_hash_t *serf_cert = NULL;
   void *creds;
   int found_matching_hostname = 0;
 
-  if (!failures && ! conn->server_cert_failures)
+  svn_failures = (ssl_convert_serf_failures(failures)
+      | conn->server_cert_failures);
+
+  if (serf_ssl_cert_depth(cert) == 0)
+    {
+      /* If the depth is 0, the hostname must match the certificate.
+
+      ### This should really be handled by serf, which should pass an error
+          for this case, but that has backwards compatibility issues. */
+      apr_array_header_t *san;
+
+      serf_cert = serf_ssl_cert_certificate(cert, scratch_pool);
+
+      san = svn_hash_gets(serf_cert, "subjectAltName");
+      /* Try to find matching server name via subjectAltName first... */
+      if (san) {
+          int i;
+          for (i = 0; i < san->nelts; i++) {
+              const char *s = APR_ARRAY_IDX(san, i, const char*);
+              if (apr_fnmatch(s, conn->session->session_url.hostname,
+                  APR_FNM_PERIOD | APR_FNM_CASE_BLIND) == APR_SUCCESS)
+              {
+                  found_matching_hostname = 1;
+                  break;
+              }
+          }
+      }
+
+      /* Match server certificate CN with the hostname of the server */
+      if (!found_matching_hostname)
+        {
+          const char *hostname = NULL;
+
+          subject = serf_ssl_cert_subject(cert, scratch_pool);
+
+          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)
+          {
+              svn_failures |= SVN_AUTH_SSL_CNMISMATCH;
+          }
+      }
+    }
+
+  if (!svn_failures)
     return SVN_NO_ERROR;
 
   /* Extract the info from the certificate */
-  subject = serf_ssl_cert_subject(cert, scratch_pool);
+  if (! subject)
+    subject = serf_ssl_cert_subject(cert, scratch_pool);
   issuer = serf_ssl_cert_issuer(cert, scratch_pool);
-  serf_cert = serf_ssl_cert_certificate(cert, scratch_pool);
+  if (! serf_cert)
+    serf_cert = serf_ssl_cert_certificate(cert, scratch_pool);
 
   cert_info.hostname = svn_hash_gets(subject, "CN");
-  san = svn_hash_gets(serf_cert, "subjectAltName");
   cert_info.fingerprint = svn_hash_gets(serf_cert, "sha1");
   if (! cert_info.fingerprint)
     cert_info.fingerprint = apr_pstrdup(scratch_pool, "<unknown>");
@@ -298,9 +347,6 @@ ssl_server_cert(void *baton, int failure
   cert_info.issuer_dname = convert_organisation_to_str(issuer, scratch_pool);
   cert_info.ascii_cert = serf_ssl_cert_export(cert, scratch_pool);
 
-  svn_failures = (ssl_convert_serf_failures(failures)
-                  | conn->server_cert_failures);
-
   /* Handle any non-server certs. */
   if (serf_ssl_cert_depth(cert) > 0)
     {
@@ -353,31 +399,6 @@ ssl_server_cert(void *baton, int failure
       return APR_SUCCESS;
     }
 
-  /* Try to find matching server name via subjectAltName first... */
-  if (san) {
-      int i;
-      for (i = 0; i < san->nelts; i++) {
-          char *s = APR_ARRAY_IDX(san, i, char*);
-          if (apr_fnmatch(s, conn->session->session_url.hostname,
-                          APR_FNM_PERIOD | APR_FNM_CASE_BLIND) == APR_SUCCESS)
-            {
-              found_matching_hostname = 1;
-              cert_info.hostname = s;
-              break;
-            }
-      }
-  }
-
-  /* Match server certificate CN with the hostname of the server */
-  if (!found_matching_hostname && cert_info.hostname)
-    {
-      if (apr_fnmatch(cert_info.hostname, conn->session->session_url.hostname,
-                      APR_FNM_PERIOD | APR_FNM_CASE_BLIND) == APR_FNM_NOMATCH)
-        {
-          svn_failures |= SVN_AUTH_SSL_CNMISMATCH;
-        }
-    }
-
   svn_auth_set_parameter(conn->session->wc_callbacks->auth_baton,
                          SVN_AUTH_PARAM_SSL_SERVER_FAILURES,
                          &svn_failures);