[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: svn commit: r1565531 - /subversion/trunk/subversion/libsvn_ra_serf/util.c

From: Julian Foad <julianfoad_at_btopenworld.com>
Date: Fri, 14 Feb 2014 14:32:14 +0000 (GMT)

This commit is causing a change of behaviour for me against the Apache svn repo. I think there is an error in this code.

The error I get when trying to run a merge into an svn trunk WC from an svn branch is:

$ svn merge --non-interactive ^/subversion/branches/fsfs-ucsnorm/
/home/julianfoad/src/subversion-c/subversion/svn/util.c:548,
/home/julianfoad/src/subversion-c/subversion/libsvn_client/merge.c:11811,
/home/julianfoad/src/subversion-c/subversion/libsvn_client/merge.c:12465,
/home/julianfoad/src/subversion-c/subversion/libsvn_client/ra.c:474,
/home/julianfoad/src/subversion-c/subversion/libsvn_client/ra.c:453,
/home/julianfoad/src/subversion-c/subversion/libsvn_ra/ra_loader.c:485: (apr_err=SVN_ERR_RA_CANNOT_CREATE_SESSION)
svn: E170013: Unable to connect to a repository at URL 'https://svn.apache.org/repos/asf/subversion/trunk'
/home/julianfoad/src/subversion-c/subversion/libsvn_ra_serf/serf.c:593,
/home/julianfoad/src/subversion-c/subversion/libsvn_ra_serf/options.c:491,
/home/julianfoad/src/subversion-c/subversion/libsvn_ra_serf/util.c:941,
/home/julianfoad/src/subversion-c/subversion/libsvn_ra_serf/util.c:915,
/home/julianfoad/src/subversion-c/subversion/libsvn_ra_serf/util.c:880,
/home/julianfoad/src/subversion-c/subversion/libsvn_ra_serf/util.c:431,
/home/julianfoad/src/subversion-c/subversion/libsvn_ra_serf/util.c:413: (apr_err=SVN_ERR_RA_SERF_SSL_CERT_UNTRUSTED)
svn: E230001: Server SSL certificate verification failed: certificate issued for a different hostname

> Author: breser

> Date: Fri Feb  7 04:32:50 2014
> New Revision: 1565531
>
> URL: http://svn.apache.org/r1565531
> Log:
> ra_serf: Do not compare the CommonName of a certificate if there are
> subjectAltName extensions of the DNS type when validating the certificate
> as per RFC 2818.
>
> * subversion/libsvn_ra_serf/util.c
>   (ssl_server_certificate): add a boolean to know if there were san
>     entries, skip the CN match if there were and move the failure
>     flag outside of the CN match code.
>
> 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=1565531&r1=1565530&r2=1565531&view=diff
> ==============================================================================
> --- subversion/trunk/subversion/libsvn_ra_serf/util.c (original)
> +++ subversion/trunk/subversion/libsvn_ra_serf/util.c Fri Feb  7 04:32:50 2014
> @@ -225,6 +225,7 @@ ssl_server_cert(void *baton, int failure
>       ### 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;
> +      svn_boolean_t found_san_entry;
>
>       serf_cert = serf_ssl_cert_certificate(cert, scratch_pool);
>
> @@ -232,6 +233,7 @@ ssl_server_cert(void *baton, int failure
>       /* Try to find matching server name via subjectAltName first... */
>       if (san) {

Here, "san" is false (no SubjectAltName found), so found_san_entry remains uninitialized...

>           int i;
> +          found_san_entry = san->nelts > 0;
>           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,
> @@ -243,8 +245,11 @@ ssl_server_cert(void *baton, int failure
>           }
>       }
>
> -      /* Match server certificate CN with the hostname of the server */
> -      if (!found_matching_hostname)
> +      /* Match server certificate CN with the hostname of the server iff
> +      * we didn't find any subjectAltName fields and try to match them.
> +      * Per RFC 2818 they are authoritative if present and CommonName
> +      * should be ignored. */
> +      if (!found_matching_hostname && !found_san_entry)
>         {

... and here we skip this block because found_san_entry is -134885336 i.e. "true".

This results in the certificate being considered invalid.

- Julian

>           const char *hostname = NULL;
>
> @@ -253,13 +258,16 @@ 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
> +              && apr_fnmatch(hostname,
> conn->session->session_url.hostname,
> +                            APR_FNM_PERIOD | APR_FNM_CASE_BLIND) ==
> APR_SUCCESS)
>           {
> -              svn_failures |= SVN_AUTH_SSL_CNMISMATCH;
> +            found_matching_hostname = 1;
>           }
>       }
> +
> +      if (!found_matching_hostname)
> +        svn_failures |= SVN_AUTH_SSL_CNMISMATCH;
>     }
>
>   if (!svn_failures)
>
Received on 2014-02-14 15:35:46 CET

This is an archived mail posted to the Subversion Dev mailing list.

This site is subject to the Apache Privacy Policy and the Apache Public Forum Archive Policy.