You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@subversion.apache.org by da...@apache.org on 2015/05/10 17:54:22 UTC

svn commit: r1678571 - /subversion/trunk/subversion/libsvn_subr/cmdline.c

Author: danielsh
Date: Sun May 10 15:54:22 2015
New Revision: 1678571

URL: http://svn.apache.org/r1678571
Log:
* subversion/libsvn_subr/cmdline.c
  (trust_server_cert_non_interactive): Fix false-positive acceptance of
    certificates with multiple failures of which some but not all were
    designated acceptable.

Modified:
    subversion/trunk/subversion/libsvn_subr/cmdline.c

Modified: subversion/trunk/subversion/libsvn_subr/cmdline.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_subr/cmdline.c?rev=1678571&r1=1678570&r2=1678571&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_subr/cmdline.c (original)
+++ subversion/trunk/subversion/libsvn_subr/cmdline.c Sun May 10 15:54:22 2015
@@ -538,19 +538,20 @@ trust_server_cert_non_interactive(svn_au
                                   apr_pool_t *pool)
 {
   struct trust_server_cert_non_interactive_baton *b = baton;
+  apr_uint32_t non_ignored_failures;
   *cred_p = NULL;
 
-  if (failures == 0 ||
-      (b->trust_server_cert_unknown_ca &&
-       (failures & SVN_AUTH_SSL_UNKNOWNCA)) ||
-      (b->trust_server_cert_cn_mismatch &&
-       (failures & SVN_AUTH_SSL_CNMISMATCH)) ||
-      (b->trust_server_cert_expired &&
-       (failures & SVN_AUTH_SSL_EXPIRED)) ||
-      (b->trust_server_cert_not_yet_valid &&
-        (failures & SVN_AUTH_SSL_NOTYETVALID)) ||
-      (b->trust_server_cert_other_failure &&
-        (failures & SVN_AUTH_SSL_OTHER)))
+  /* Mask away bits we are instructed to ignore. */
+  non_ignored_failures = failures & ~(
+        (b->trust_server_cert_unknown_ca ? SVN_AUTH_SSL_UNKNOWNCA : 0)
+      | (b->trust_server_cert_cn_mismatch ? SVN_AUTH_SSL_CNMISMATCH : 0)
+      | (b->trust_server_cert_expired ? SVN_AUTH_SSL_EXPIRED : 0)
+      | (b->trust_server_cert_not_yet_valid ? SVN_AUTH_SSL_NOTYETVALID : 0)
+      | (b->trust_server_cert_other_failure ? SVN_AUTH_SSL_OTHER : 0)
+  );
+
+  /* If no failures remain, accept the certificate. */
+  if (non_ignored_failures == 0)
     {
       *cred_p = apr_pcalloc(pool, sizeof(**cred_p));
       (*cred_p)->may_save = FALSE;



Re: 1.9.0-beta1 may accept invalid certificates

Posted by Branko Čibej <br...@wandisco.com>.
On 10.05.2015 21:23, Stefan Sperling wrote:
> On Sun, May 10, 2015 at 04:05:16PM +0000, Daniel Shahaf wrote:
>> Subversion 1.9.0-beta1 may accept invalid SSL certificates presented by
>> servers in certain conditions: if both --non-interactive and --trust-foo
>> were passed, and the certificate has two failures, both the 'foo'
>> failure and some other failure.
>>
>> In this context, a 'failure' corresponds to one of the 1.9.x cmdline
>> client's --trust-* option flags.
>>
>> This issue is not present in any GA release (1.8.x or earlier) and will
>> not be present in 1.9.0 final.
>>
>> Daniel
>> (handling this publicly since it doesn't affect any GA release; normally
>> we handle security issues privately)
>>
> Sorry! I think I wrote this... oops.
>
> And thank you very much for catching it before dot zero GA!

Yup. FWIW, RC-1 has the same problem, but RC2 will not (assuming we all
vote for the backport).

-- Brane


Re: 1.9.0-beta1 may accept invalid certificates (was: svn commit: r1678571 - /subversion/trunk/subversion/libsvn_subr/cmdline.c)

Posted by Stefan Sperling <st...@elego.de>.
On Sun, May 10, 2015 at 04:05:16PM +0000, Daniel Shahaf wrote:
> Subversion 1.9.0-beta1 may accept invalid SSL certificates presented by
> servers in certain conditions: if both --non-interactive and --trust-foo
> were passed, and the certificate has two failures, both the 'foo'
> failure and some other failure.
> 
> In this context, a 'failure' corresponds to one of the 1.9.x cmdline
> client's --trust-* option flags.
> 
> This issue is not present in any GA release (1.8.x or earlier) and will
> not be present in 1.9.0 final.
> 
> Daniel
> (handling this publicly since it doesn't affect any GA release; normally
> we handle security issues privately)
> 

Sorry! I think I wrote this... oops.

And thank you very much for catching it before dot zero GA!

> danielsh@apache.org wrote on Sun, May 10, 2015 at 15:54:22 -0000:
> > Author: danielsh
> > Date: Sun May 10 15:54:22 2015
> > New Revision: 1678571
> > 
> > URL: http://svn.apache.org/r1678571
> > Log:
> > * subversion/libsvn_subr/cmdline.c
> >   (trust_server_cert_non_interactive): Fix false-positive acceptance of
> >     certificates with multiple failures of which some but not all were
> >     designated acceptable.

1.9.0-beta1 may accept invalid certificates (was: svn commit: r1678571 - /subversion/trunk/subversion/libsvn_subr/cmdline.c)

Posted by Daniel Shahaf <d....@daniel.shahaf.name>.
Subversion 1.9.0-beta1 may accept invalid SSL certificates presented by
servers in certain conditions: if both --non-interactive and --trust-foo
were passed, and the certificate has two failures, both the 'foo'
failure and some other failure.

In this context, a 'failure' corresponds to one of the 1.9.x cmdline
client's --trust-* option flags.

This issue is not present in any GA release (1.8.x or earlier) and will
not be present in 1.9.0 final.

Daniel
(handling this publicly since it doesn't affect any GA release; normally
we handle security issues privately)

danielsh@apache.org wrote on Sun, May 10, 2015 at 15:54:22 -0000:
> Author: danielsh
> Date: Sun May 10 15:54:22 2015
> New Revision: 1678571
> 
> URL: http://svn.apache.org/r1678571
> Log:
> * subversion/libsvn_subr/cmdline.c
>   (trust_server_cert_non_interactive): Fix false-positive acceptance of
>     certificates with multiple failures of which some but not all were
>     designated acceptable.