You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by bu...@apache.org on 2014/01/11 15:45:34 UTC

[Bug 55988] New: Add parameter useCipherSuitesOrder to JSSE (BIO and NIO) connectors

https://issues.apache.org/bugzilla/show_bug.cgi?id=55988

            Bug ID: 55988
           Summary: Add parameter useCipherSuitesOrder to JSSE (BIO and
                    NIO) connectors
           Product: Tomcat 8
           Version: trunk
          Hardware: All
                OS: All
            Status: NEW
          Severity: enhancement
          Priority: P2
         Component: Connectors
          Assignee: dev@tomcat.apache.org
          Reporter: ognjen.d.blagojevic@gmail.com

Starting with Oracle Java 1.8.0 B108, JSSE supports server-side cipher ordering
[1]. Server-side cipher ordering is useful for enabling Forward Secrecy and for
preventing certain attacks. Appropriate JSSE parameter is called
useCipherSuitesOrder [2].

Is it possible to add that same attribute to Tomcat JSSE connectors?

The problem is that parameter useCipherSuitesOrder is only available starting
with Java 1.8 B108, while Tomcat 8 requires only Java 1.7. Therefore the
proposal is, if Tomcat 8:

(a) runs using Java 1.7 / 1.8 pre-B108, parameter useCipherSuitesOrder would be
ignored, and if 
(b) runs using Java 1.8 B108+, JSSE parameter useCipherSuitesOrder would be
appropriately set.

It might be a precedence to support parameter from non-required version of
Java, albeit -- due to the usefulness of such configuration option -- it might
be worthwhile considering.

Note that similar attribute already exists for APR connector --
SSLHonorCipherOrder.

-Ognjen

[1] https://bugs.openjdk.java.net/browse/JDK-7188657
[2]
http://download.java.net/jdk8/docs/api/javax/net/ssl/SSLParameters.html#setUseCipherSuitesOrder-boolean-

-- 
You are receiving this mail because:
You are the assignee for the bug.

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


[Bug 55988] Add parameter useCipherSuitesOrder to JSSE (BIO and NIO) connectors

Posted by bu...@apache.org.
https://issues.apache.org/bugzilla/show_bug.cgi?id=55988

--- Comment #3 from Christopher Schultz <ch...@christopherschultz.net> ---
APR connector has the "SSLHonorCipherOrder" attribute. Tomcat has a history of
using different SSL-configuration attributes for APR versus other connector
types. I would, however, prefer to make the configuration option more clear to
the user like "useServerCipherSuitesOrder"... as it stands, the option doesn't
really make it clear that the server is in control.

Any objections?

-- 
You are receiving this mail because:
You are the assignee for the bug.

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


[Bug 55988] Add parameter useCipherSuitesOrder to JSSE (BIO and NIO) connectors [PATCH]

Posted by bu...@apache.org.
https://issues.apache.org/bugzilla/show_bug.cgi?id=55988

--- Comment #12 from Christopher Schultz <ch...@christopherschultz.net> ---
(In reply to Ognjen Blagojevic from comment #4)
> No objections. Do I need to provide a new patch with the name you proposed?

If you like my suggestions above, you could make all 3 changes at once and
propose a new patch. That would be nice ;)

-- 
You are receiving this mail because:
You are the assignee for the bug.

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


[Bug 55988] Add parameter useCipherSuitesOrder to JSSE (BIO and NIO) connectors

Posted by bu...@apache.org.
https://issues.apache.org/bugzilla/show_bug.cgi?id=55988

--- Comment #7 from Remy Maucherat <re...@apache.org> ---
Ok, since you asked for it, here's a review:
- I don't think this feature justifies a big blob of ugly code, so this should
wait for Java 8.
- Regardless of what APR may or may not do, it should be a boolean for the Java
connectors, not a String.

-- 
You are receiving this mail because:
You are the assignee for the bug.

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


[Bug 55988] Add parameter useCipherSuitesOrder to JSSE (BIO and NIO) connectors [PATCH]

Posted by bu...@apache.org.
https://issues.apache.org/bugzilla/show_bug.cgi?id=55988

Christopher Schultz <ch...@christopherschultz.net> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
            Summary|Add parameter               |Add parameter
                   |useCipherSuitesOrder to     |useCipherSuitesOrder to
                   |JSSE (BIO and NIO)          |JSSE (BIO and NIO)
                   |connectors                  |connectors [PATCH]

-- 
You are receiving this mail because:
You are the assignee for the bug.

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


[Bug 55988] Add parameter useCipherSuitesOrder to JSSE (BIO and NIO) connectors

Posted by bu...@apache.org.
https://issues.apache.org/bugzilla/show_bug.cgi?id=55988

--- Comment #8 from Christopher Schultz <ch...@christopherschultz.net> ---
(In reply to Remy Maucherat from comment #7)
> Ok, since you asked for it, here's a review:
> - I don't think this feature justifies a big blob of ugly code, so this
> should wait for Java 8.

I'm not sure I'd call this a big ball of ugly code. The only thing that makes
it ugly is the fact that we can't have a compilation dependency on Java 8 --
which is where this feature is actually implemented.

> - Regardless of what APR may or may not do, it should be a boolean for the
> Java connectors, not a String.

While the class member is a String, it's treated as a boolean. Ognjen was
simply following precedent set for example by "clientAuth".

-- 
You are receiving this mail because:
You are the assignee for the bug.

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


[Bug 55988] Add parameter useCipherSuitesOrder to JSSE (BIO and NIO) connectors

Posted by bu...@apache.org.
https://issues.apache.org/bugzilla/show_bug.cgi?id=55988

--- Comment #1 from Ognjen Blagojevic <og...@gmail.com> ---
Created attachment 31198
  --> https://issues.apache.org/bugzilla/attachment.cgi?id=31198&action=edit
Proof of concept patch

Here is initial patch to prove the concept. This patch will always try to set
parameter useCipherSuitesOrder using reflection.

To test it:

(1) Install JDK 1.8.0 EA (must be B108+, tested with B121) [1]
(2) Install Java 7 JCE Unlimited Strength (it also works with JDK 1.8.0 EA) [2]
(3) Apply patch, build Tomcat
(4) Add JSSE Connector configuration to server.xml:

    <Connector port="443" 
               protocol="org.apache.coyote.http11.Http11Protocol"
               SSLEnabled="true"
               maxThreads="150" 
               scheme="https" 
               secure="true"
               clientAuth="false" 
               sslProtocol="TLS" 
               ciphers="TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA384,
                        TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA,
                        TLS_DHE_RSA_WITH_AES_256_CBC_SHA,
                        TLS_DHE_RSA_WITH_AES_128_CBC_SHA,
                        TLS_ECDHE_RSA_WITH_3DES_EDE_CBC_SHA,
                        TLS_RSA_WITH_AES_256_CBC_SHA256,
                        TLS_RSA_WITH_AES_256_CBC_SHA,
                        SSL_RSA_WITH_3DES_EDE_CBC_SHA" />

(5) Start Tomcat. Forward Secrecy is enabled (on all clients that support it)

-Ognjen

[1] https://jdk8.java.net/download.html
[2]
http://www.oracle.com/technetwork/java/javase/downloads/jce-7-download-432124.html

-- 
You are receiving this mail because:
You are the assignee for the bug.

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


[Bug 55988] Add parameter useCipherSuitesOrder to JSSE (BIO and NIO) connectors

Posted by bu...@apache.org.
https://issues.apache.org/bugzilla/show_bug.cgi?id=55988

--- Comment #6 from Ognjen Blagojevic <og...@gmail.com> ---
Just a gentle reminder. If you have some free time to review the patch, it
would be great.

-- 
You are receiving this mail because:
You are the assignee for the bug.

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


[Bug 55988] Add parameter useCipherSuitesOrder to JSSE (BIO and NIO) connectors [PATCH]

Posted by bu...@apache.org.
https://issues.apache.org/bugzilla/show_bug.cgi?id=55988

--- Comment #14 from Christopher Schultz <ch...@christopherschultz.net> ---
Ognjen, if you are still willing to produce a patch, consider writing it
against trunk, which will require Java 8 so won't need the reflection. If we
decide to back-port to Tomcat 8, the reflection can be re-introduced.

Are you still able to update the patch?

(In reply to Ralf Hauser from comment #13)
> Please implement this feature also for non-APR connectors A.S.A.P. - I think
> it is even worthwhile to backport to Tomcat 7!

This enhancement request is specifically targeted towards the non-APR
connectors. The APR connector already supports this capability via the
SSLHonorCipherOrder setting.

-- 
You are receiving this mail because:
You are the assignee for the bug.

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


[Bug 55988] Add parameter useCipherSuitesOrder to JSSE (BIO and NIO) connectors [PATCH]

Posted by bu...@apache.org.
https://issues.apache.org/bugzilla/show_bug.cgi?id=55988

Jens Borgland <je...@gmail.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |jens.borgland@gmail.com

-- 
You are receiving this mail because:
You are the assignee for the bug.

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


[Bug 55988] Add parameter useCipherSuitesOrder to JSSE (BIO and NIO) connectors

Posted by bu...@apache.org.
https://issues.apache.org/bugzilla/show_bug.cgi?id=55988

--- Comment #9 from Remy Maucherat <re...@apache.org> ---
Yes, the reflection code is ugly by design, it's not like it could look better
(although maybe it could use introspection util ?), but the logging shouldn't
be there (if some Java 8 features are to be used, there should be a static Java
8 flag somewhere that would avoid trying the reflection). About the "String"
this is because the attributes are set using the introspection utils
reflection, so that looks ok actually [= like the rest].

I still don't see why this should be added right now, the Java 8 support
doesn't look good overall.

-- 
You are receiving this mail because:
You are the assignee for the bug.

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


[Bug 55988] Add parameter useCipherSuitesOrder to JSSE (BIO and NIO) connectors

Posted by bu...@apache.org.
https://issues.apache.org/bugzilla/show_bug.cgi?id=55988

--- Comment #4 from Ognjen Blagojevic <og...@gmail.com> ---
(In reply to Christopher Schultz from comment #3)
> I would, however, prefer to make the configuration option
> more clear to the user like "useServerCipherSuitesOrder"... as it stands,
> the option doesn't really make it clear that the server is in control.
> 
> Any objections?

No objections. Do I need to provide a new patch with the name you proposed?

-- 
You are receiving this mail because:
You are the assignee for the bug.

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


[Bug 55988] Add parameter useCipherSuitesOrder to JSSE (BIO and NIO) connectors

Posted by bu...@apache.org.
https://issues.apache.org/bugzilla/show_bug.cgi?id=55988

--- Comment #10 from Ognjen Blagojevic <og...@gmail.com> ---
(In reply to Remy Maucherat from comment #9)
> I still don't see why this should be added right now, the Java 8 support
> doesn't look good overall.

Because:

a. is is a useful security feature, and as such it should be subject of
consideration even if the small number of users may benefit from it, and

b. Oracle Java 1.8.0 is released a week ago, so its adoption is only going to
increase, not shrink.

-- 
You are receiving this mail because:
You are the assignee for the bug.

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


[Bug 55988] Add parameter useCipherSuitesOrder to JSSE (BIO and NIO) connectors [PATCH]

Posted by bu...@apache.org.
https://issues.apache.org/bugzilla/show_bug.cgi?id=55988

--- Comment #13 from Ralf Hauser <ha...@acm.org> ---
getting as many clients to choose a forward-secret cipher even if their makers
didn't think of putting forward-secret ciphers highest priority is important in
today's world of massive eaves-dropping.

Please implement this feature also for non-APR connectors A.S.A.P. - I think it
is even worthwhile to backport to Tomcat 7!

-- 
You are receiving this mail because:
You are the assignee for the bug.

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


[Bug 55988] Add parameter useCipherSuitesOrder to JSSE (BIO and NIO) connectors

Posted by bu...@apache.org.
https://issues.apache.org/bugzilla/show_bug.cgi?id=55988

--- Comment #5 from Christopher Schultz <ch...@christopherschultz.net> ---
Not necessary.

-- 
You are receiving this mail because:
You are the assignee for the bug.

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


[Bug 55988] Add parameter useCipherSuitesOrder to JSSE (BIO and NIO) connectors

Posted by bu...@apache.org.
https://issues.apache.org/bugzilla/show_bug.cgi?id=55988

Ognjen Blagojevic <og...@gmail.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #31198|0                           |1
        is obsolete|                            |

--- Comment #2 from Ognjen Blagojevic <og...@gmail.com> ---
Created attachment 31272
  --> https://issues.apache.org/bugzilla/attachment.cgi?id=31272&action=edit
Patch to add useCipherSuitesOrder to BIO and NIO connectors

Fully functional patch. Here is an example how to use it for BIO (with OpenJDK
1.8.0 B108+ and JCE Unlimited Strength):

    <Connector port="443" 
               protocol="org.apache.coyote.http11.Http11Protocol"
               SSLEnabled="true"
               maxThreads="150" scheme="https" secure="true"
               clientAuth="false" sslProtocol="TLS" 
               useCipherSuitesOrder="true"
               ciphers="TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA384,
                        TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA,
                        TLS_DHE_RSA_WITH_AES_256_CBC_SHA,
                        TLS_DHE_RSA_WITH_AES_128_CBC_SHA,
                        SSL_RSA_WITH_3DES_EDE_CBC_SHA" />


To test NIO, just replace Http11Protocol with Http11NioProtocol.

---

You may test Forward Secrecy using 

  https://www.ssllabs.com/ssltest/

It should report "Forward Secrecy -- Yes (with most browsers)"

---

Note: If you try the same with JDK that does not support
javax.net.SSLParameters.setUseCipherSuitesOrder(boolean) method, it will log:

WARNING [main]
org.apache.tomcat.util.net.jsse.JSSESocketFactory.configureUseCipherSuitesOrder
Method setUseCipherSuitesOrder is not supported by the SSL engine

-- 
You are receiving this mail because:
You are the assignee for the bug.

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


[Bug 55988] Add parameter useCipherSuitesOrder to JSSE (BIO and NIO) connectors [PATCH]

Posted by bu...@apache.org.
https://issues.apache.org/bugzilla/show_bug.cgi?id=55988

Ognjen Blagojevic <og...@gmail.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
          Component|Connectors                  |Connectors
            Version|trunk                       |unspecified
            Product|Tomcat 8                    |Tomcat 9
   Target Milestone|----                        |-----

--- Comment #15 from Ognjen Blagojevic <og...@gmail.com> ---
Chris,

(In reply to Christopher Schultz from comment #14)
> Ognjen, if you are still willing to produce a patch, consider writing it
> against trunk, which will require Java 8 so won't need the reflection. If we
> decide to back-port to Tomcat 8, the reflection can be re-introduced.

Ok. I will attach patch for Tomcat 9. As you suggested:

1. Parameter name is useServerCipherSuitesOrder insted of useCipherSuitesOrder.
2. Code is deduplicated / moved to superclass.

To test it:

(1) Install JDK 1.8.0
(2) Install Java 8 JCE Unlimited Strength
(3) Apply patch, build Tomcat
(4) Add JSSE Connector configuration to server.xml:

    <Connector port="443" 
               protocol="org.apache.coyote.http11.Http11NioProtocol"
               SSLEnabled="true"
               maxThreads="150" 
               scheme="https" 
               secure="true"
               clientAuth="false" 
               sslProtocol="TLS" 
               useServerCipherSuitesOrder="true"
               ciphers="TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA384,   
                        TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA,      
                        TLS_DHE_RSA_WITH_AES_256_CBC_SHA,        
                        TLS_DHE_RSA_WITH_AES_128_CBC_SHA,        
                        SSL_RSA_WITH_3DES_EDE_CBC_SHA" 
 />

(5) Start Tomcat. Forward Secrecy is enabled (on all clients that support it)

To test with NIO2, just replace Http11NioProtocol with Http11Nio2Protocol.

-Ognjen

-- 
You are receiving this mail because:
You are the assignee for the bug.

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


[Bug 55988] Add parameter useCipherSuitesOrder to JSSE (BIO and NIO) connectors [PATCH]

Posted by bu...@apache.org.
https://issues.apache.org/bugzilla/show_bug.cgi?id=55988

Ralf Hauser <ha...@acm.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |hauser@acm.org

-- 
You are receiving this mail because:
You are the assignee for the bug.

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


[Bug 55988] Add parameter useCipherSuitesOrder to JSSE (BIO and NIO) connectors [PATCH]

Posted by bu...@apache.org.
https://issues.apache.org/bugzilla/show_bug.cgi?id=55988

--- Comment #11 from Christopher Schultz <ch...@christopherschultz.net> ---
Ognjen, I have a couple of further comments about your proposed patch. I'm
leaning towards adding this to Tomcat 8 but not back-porting unless there is
significant demand.

1. Most of the 2 configureUseCipherSuitesOrder methods is the same. Consider
re-factoring the bulk of that method into a superclass utility method and then
extract the SSLParameters object from either SSLEngine or Socket in the
subclasses.

2. Since this is a security-related configuration, consider failing totally
when server-side ordering is requested but can't be enforced -- e.g. the
reflection fails for any reason. You have it logging a warning but continuing
which I think isn't appropriate in this case.

-- 
You are receiving this mail because:
You are the assignee for the bug.

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


[Bug 55988] Add parameter useCipherSuitesOrder to JSSE (BIO and NIO) connectors [PATCH]

Posted by bu...@apache.org.
https://issues.apache.org/bugzilla/show_bug.cgi?id=55988

--- Comment #16 from Ognjen Blagojevic <og...@gmail.com> ---
Created attachment 32407
  --> https://issues.apache.org/bugzilla/attachment.cgi?id=32407&action=edit
Patch to add useServerCipherSuitesOrder to NIO and NIO2 connectors

-- 
You are receiving this mail because:
You are the assignee for the bug.

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


[Bug 55988] Add parameter useCipherSuitesOrder to JSSE (BIO and NIO) connectors [PATCH]

Posted by bu...@apache.org.
https://issues.apache.org/bugzilla/show_bug.cgi?id=55988

Neale Rudd <ne...@metawerx.net> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |neale@metawerx.net

-- 
You are receiving this mail because:
You are the assignee for the bug.

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