On 22/07/2014 09:40, Rémy Maucherat wrote: > 2014-07-21 19:57 GMT+02:00 Mark Thomas <ma...@apache.org>: > >> On 20 July 2014 23:40:50 CEST, Tim Whittington <t...@apache.org> wrote: >>> This doesn’t look like it’ll work as expected on IBM JDKs (which do >>> s/^TLS_/SSL_/ on all the TLS era cipher suite names). >>> >>> Also a big -0 for importing the brokenness that is the openssl ciphers >>> syntax (seriously, I have to recite >>> HIGH:!aNULL:!eNULL:!EXPORT:!DES:!RC4:!MD5 to do something sensible?). >>> (I get the consistency argument internally (JSSE/APR connectors) and >>> externally (Apache, nginx etc.), but meh). >>> >>> I’m not a fan of the closed-set approach either, but I haven’t got a >>> better option that doesn’t involve magic cipher-suite name parsing >>> unfortunately (both put you in an arms race with new suites for >>> different reasons). >> >> This seems like a good point to add that I've been looking in to this and >> the current mapping doesn't look right. It references cipher suites that >> aren't in the standard names doc provided in the JRE docs and it also fails >> to reference many suites that are in it. >> >> I'm less concerned about keeping up with new cipher suites. We can write >> unit tests to catch those. >> >> Different cipher names in different JREs is going to be problematic. >> >> This is currently top if my to do list when I get back to work next week. >> My current view of this feature is buggy with maintenance issues but I also >> believe all these to be fixable/manageable. >> >> I might find some time to play with this this week so don't be surprised >> if you see the odd commit from me in this area. >> > > We plan to do maintenance as needed on it too.
I'm beginning to think that this feature is more effort than it is worth. The expected behaviour of this code is that for any given specification: - passing it through OpenSSL and mapping the resulting ciphers to those supported by the current JRE; and - passing it through this parser gives the same set of JSSE ciphers in the same order. Every time this doesn't happen we have a potential security issue since a weaker than intended cipher may be enabled. The incorrect handling of "ALL" that I have just fixed is an obvious example of such an issue. The OpenSSL specification syntax is non-trivial and the actual behaviour of OpenSSL does not always agree with the documentation. The differences in supported cipher suites between OpenSSL and JSSE add further complications to the unit tests. The current implementation was committed without any unit tests. The unit tests I have added this week show that the code does have bugs including nasty security issues and there are still quite a few unit tests required to provide a reasonable degree of confidence that the specification parsing is working correctly. I have spent pretty much all my time so far this week working on this - most of it investigating anomalies thrown up by the unit tests. I still think that this code is fixable and the unit tests will go a long way to making it maintainable and highlighting when OpenSSL and/or Java add support for a new cipher suite. The question at this point is is it worth continuing? The stated original aim of this code was to provide a better set of default cipher suites for JSSE. There are easier ways to do that. If we do keep this code then I'd like to see all mention of it removed from the docs and the connector changes that make use of it reverted to make absolutely sure that no-one makes use of this code until we are happy that it behaves as intended. Mark --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
