You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by ma...@apache.org on 2015/08/10 17:20:35 UTC

svn commit: r1695111 - /tomcat/trunk/java/org/apache/tomcat/util/net/jsse/openssl/OpenSSLCipherConfigurationParser.java

Author: markt
Date: Mon Aug 10 15:20:34 2015
New Revision: 1695111

URL: http://svn.apache.org/r1695111
Log:
Fix DEFAULT

Modified:
    tomcat/trunk/java/org/apache/tomcat/util/net/jsse/openssl/OpenSSLCipherConfigurationParser.java

Modified: tomcat/trunk/java/org/apache/tomcat/util/net/jsse/openssl/OpenSSLCipherConfigurationParser.java
URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/tomcat/util/net/jsse/openssl/OpenSSLCipherConfigurationParser.java?rev=1695111&r1=1695110&r2=1695111&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/tomcat/util/net/jsse/openssl/OpenSSLCipherConfigurationParser.java (original)
+++ tomcat/trunk/java/org/apache/tomcat/util/net/jsse/openssl/OpenSSLCipherConfigurationParser.java Mon Aug 10 15:20:34 2015
@@ -485,13 +485,12 @@ public class OpenSSLCipherConfigurationP
         addListAlias(SRP, filterByKeyExchange(allCiphers, Collections.singleton(KeyExchange.SRP)));
         initialized = true;
         // Despite what the OpenSSL docs say, DEFAULT also excludes SSLv2
-        addListAlias(DEFAULT, parse("ALL:!EXPORT:!eNULL:!aNULL:!SSLv2"));
+        addListAlias(DEFAULT, parse("ALL:!eNULL:!aNULL:!SSLv2"));
         // COMPLEMENTOFDEFAULT is also not exactly as defined by the docs
         Set<Cipher> complementOfDefault = filterByKeyExchange(all, new HashSet<>(Arrays.asList(KeyExchange.EDH,KeyExchange.EECDH)));
         complementOfDefault = filterByAuthentication(complementOfDefault, Collections.singleton(Authentication.aNULL));
         complementOfDefault.removeAll(aliases.get(eNULL));
         complementOfDefault.addAll(aliases.get(Constants.SSL_PROTO_SSLv2));
-        complementOfDefault.addAll(aliases.get(EXPORT));
         addListAlias(COMPLEMENTOFDEFAULT, complementOfDefault);
     }
 



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


Re: svn commit: r1695111 - /tomcat/trunk/java/org/apache/tomcat/util/net/jsse/openssl/OpenSSLCipherConfigurationParser.java

Posted by Mark Thomas <ma...@apache.org>.
On 10/08/2015 18:38, Mark Thomas wrote:
> On 10/08/2015 17:17, Christopher Schultz wrote:
>> Mark,
>>
>> On 8/10/15 11:20 AM, markt@apache.org wrote:
>>> Author: markt
>>> Date: Mon Aug 10 15:20:34 2015
>>> New Revision: 1695111
>>>
>>> URL: http://svn.apache.org/r1695111
>>> Log:
>>> Fix DEFAULT
>>>
>>> Modified:
>>>     tomcat/trunk/java/org/apache/tomcat/util/net/jsse/openssl/OpenSSLCipherConfigurationParser.java
>>>
>>> Modified: tomcat/trunk/java/org/apache/tomcat/util/net/jsse/openssl/OpenSSLCipherConfigurationParser.java
>>> URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/tomcat/util/net/jsse/openssl/OpenSSLCipherConfigurationParser.java?rev=1695111&r1=1695110&r2=1695111&view=diff
>>> ==============================================================================
>>> --- tomcat/trunk/java/org/apache/tomcat/util/net/jsse/openssl/OpenSSLCipherConfigurationParser.java (original)
>>> +++ tomcat/trunk/java/org/apache/tomcat/util/net/jsse/openssl/OpenSSLCipherConfigurationParser.java Mon Aug 10 15:20:34 2015
>>> @@ -485,13 +485,12 @@ public class OpenSSLCipherConfigurationP
>>>          addListAlias(SRP, filterByKeyExchange(allCiphers, Collections.singleton(KeyExchange.SRP)));
>>>          initialized = true;
>>>          // Despite what the OpenSSL docs say, DEFAULT also excludes SSLv2
>>> -        addListAlias(DEFAULT, parse("ALL:!EXPORT:!eNULL:!aNULL:!SSLv2"));
>>> +        addListAlias(DEFAULT, parse("ALL:!eNULL:!aNULL:!SSLv2"));
>>
>> Do I misunderstand the above? Have you added EXPORT ciphers back into
>> the DEFAULT ciphers? That really should not be the default. Recent
>> versions of OpenSSL don't include EXPORT in the "ALL" (I think) unless
>> specifically requested, but older versions still include them, so we
>> ought to be very careful and explicitly exclude them.
> 
> We are aiming to mimic OpenSSL behaviour and - with the current code -
> we are doing that for trunk and 1.0.2.
> 
> OpenSSL trunk includes EXPORT in DEFAULT whereas 1.0.2 does not.
> 
> Bizarrely, prior to this change the Tomcat code included it in 8.0.x
> (which is tested with 1.0.2) and excluded it with trunk (which is tested
> with OpenSSL trunk).
> 
> This change was triggered by restoring the experimental export ciphers
> in trunk as OpenSSL 1.0.2 still defines them.
> 
> I don't think we are ever going to be able to exactly mimic OpenSSL but
> absent a test case that shows Tomcat doing something that a) OpenSSl
> doesn't do for a given configuration string and b) is really stupid I'm
> not going to be too concerned.
> 
> I'm more concerned about having consistent behaviour of this feature
> between Tomcat 8.0.x and Tomcat trunk.

Just as a follow-up I'm trying to construct the OpenSSL unit tests so
they run and pass with both OpenSSL trunk and 1.0.2

Mark


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


Re: svn commit: r1695111 - /tomcat/trunk/java/org/apache/tomcat/util/net/jsse/openssl/OpenSSLCipherConfigurationParser.java

Posted by Mark Thomas <ma...@apache.org>.
On 10/08/2015 17:17, Christopher Schultz wrote:
> Mark,
> 
> On 8/10/15 11:20 AM, markt@apache.org wrote:
>> Author: markt
>> Date: Mon Aug 10 15:20:34 2015
>> New Revision: 1695111
>>
>> URL: http://svn.apache.org/r1695111
>> Log:
>> Fix DEFAULT
>>
>> Modified:
>>     tomcat/trunk/java/org/apache/tomcat/util/net/jsse/openssl/OpenSSLCipherConfigurationParser.java
>>
>> Modified: tomcat/trunk/java/org/apache/tomcat/util/net/jsse/openssl/OpenSSLCipherConfigurationParser.java
>> URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/tomcat/util/net/jsse/openssl/OpenSSLCipherConfigurationParser.java?rev=1695111&r1=1695110&r2=1695111&view=diff
>> ==============================================================================
>> --- tomcat/trunk/java/org/apache/tomcat/util/net/jsse/openssl/OpenSSLCipherConfigurationParser.java (original)
>> +++ tomcat/trunk/java/org/apache/tomcat/util/net/jsse/openssl/OpenSSLCipherConfigurationParser.java Mon Aug 10 15:20:34 2015
>> @@ -485,13 +485,12 @@ public class OpenSSLCipherConfigurationP
>>          addListAlias(SRP, filterByKeyExchange(allCiphers, Collections.singleton(KeyExchange.SRP)));
>>          initialized = true;
>>          // Despite what the OpenSSL docs say, DEFAULT also excludes SSLv2
>> -        addListAlias(DEFAULT, parse("ALL:!EXPORT:!eNULL:!aNULL:!SSLv2"));
>> +        addListAlias(DEFAULT, parse("ALL:!eNULL:!aNULL:!SSLv2"));
> 
> Do I misunderstand the above? Have you added EXPORT ciphers back into
> the DEFAULT ciphers? That really should not be the default. Recent
> versions of OpenSSL don't include EXPORT in the "ALL" (I think) unless
> specifically requested, but older versions still include them, so we
> ought to be very careful and explicitly exclude them.

We are aiming to mimic OpenSSL behaviour and - with the current code -
we are doing that for trunk and 1.0.2.

OpenSSL trunk includes EXPORT in DEFAULT whereas 1.0.2 does not.

Bizarrely, prior to this change the Tomcat code included it in 8.0.x
(which is tested with 1.0.2) and excluded it with trunk (which is tested
with OpenSSL trunk).

This change was triggered by restoring the experimental export ciphers
in trunk as OpenSSL 1.0.2 still defines them.

I don't think we are ever going to be able to exactly mimic OpenSSL but
absent a test case that shows Tomcat doing something that a) OpenSSl
doesn't do for a given configuration string and b) is really stupid I'm
not going to be too concerned.

I'm more concerned about having consistent behaviour of this feature
between Tomcat 8.0.x and Tomcat trunk.

Mark

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


Re: svn commit: r1695111 - /tomcat/trunk/java/org/apache/tomcat/util/net/jsse/openssl/OpenSSLCipherConfigurationParser.java

Posted by Christopher Schultz <ch...@christopherschultz.net>.
Mark,

On 8/10/15 11:20 AM, markt@apache.org wrote:
> Author: markt
> Date: Mon Aug 10 15:20:34 2015
> New Revision: 1695111
> 
> URL: http://svn.apache.org/r1695111
> Log:
> Fix DEFAULT
> 
> Modified:
>     tomcat/trunk/java/org/apache/tomcat/util/net/jsse/openssl/OpenSSLCipherConfigurationParser.java
> 
> Modified: tomcat/trunk/java/org/apache/tomcat/util/net/jsse/openssl/OpenSSLCipherConfigurationParser.java
> URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/tomcat/util/net/jsse/openssl/OpenSSLCipherConfigurationParser.java?rev=1695111&r1=1695110&r2=1695111&view=diff
> ==============================================================================
> --- tomcat/trunk/java/org/apache/tomcat/util/net/jsse/openssl/OpenSSLCipherConfigurationParser.java (original)
> +++ tomcat/trunk/java/org/apache/tomcat/util/net/jsse/openssl/OpenSSLCipherConfigurationParser.java Mon Aug 10 15:20:34 2015
> @@ -485,13 +485,12 @@ public class OpenSSLCipherConfigurationP
>          addListAlias(SRP, filterByKeyExchange(allCiphers, Collections.singleton(KeyExchange.SRP)));
>          initialized = true;
>          // Despite what the OpenSSL docs say, DEFAULT also excludes SSLv2
> -        addListAlias(DEFAULT, parse("ALL:!EXPORT:!eNULL:!aNULL:!SSLv2"));
> +        addListAlias(DEFAULT, parse("ALL:!eNULL:!aNULL:!SSLv2"));

Do I misunderstand the above? Have you added EXPORT ciphers back into
the DEFAULT ciphers? That really should not be the default. Recent
versions of OpenSSL don't include EXPORT in the "ALL" (I think) unless
specifically requested, but older versions still include them, so we
ought to be very careful and explicitly exclude them.

-chris