Thank you Xuelei for looking into this!
On 26.04.2016 17:33, Xuelei Fan wrote:
jdk.tls.handleCertReqAuthoritesOverflow is a little bit long.
jdk.tls.server.overFlowAuthorites?
Yes, I agree.
Changed to a shorter property name.
May not need to define the "none" property value.
Using enum for HAO_NONE, HAO_EMPTY and HAO_TRUNC?
Changed to enum.
Have to keep NONE, though, as we need to represent it in the enum somehow.
line 1920: Overflow should be rare, what about print only when overflow?
It the indication of overflow is printed only when it has happened.
Please find the updated version of webrev:
http://cr.openjdk.java.net/~igerasim/8154947/02/webrev/
With kind regards,
Ivan
Xuelei
On 4/26/2016 9:56 PM, Seán Coffey wrote:
Looks like a fair approach to solving this issue Ivan. A few comments
from me :
typo : authoririesOverflow --> authoritiesOverflow
typo : handleAuthoritesOverflow --> handleAuthoritiesOverflow
typo : jdk.tls.handleCertReqAuthoritesOverflow -->
jdk.tls.handleCertReqAuthoritiesOverflow
+ throw new RuntimeException("Value of " + prop
+ + " must be one of '" + HAO_NONE + "', '"
+ + HAO_EMPTY + "', '" + HAO_TRUNC + "'");
I think it would be good to print the value of s in above exception
also. something like + ". Received: \"" + s + "\"");
==
s.println("Cert Authorities:" + (authoririesOverflow ? " (overflow)" :
""));
I would also be good to indicate the handleAuthoritiesOverflow string
value in above printing *if* authoritiesOverflow turns out to be true.
We should be able to determine from the next message printed - but no
harm to future proof.
Maybe :
s.println("Cert Authorities:" + (authoritiesOverflow ? " (overflow" +
"[" + handleAuthoritiesOverflow + "])" : ""));
Regards,
Sean.
On 26/04/2016 11:57, Ivan Gerasimov wrote:
Here's a modified version of the fix.
Instead of a boolean-type property, a string-type property is introduced.
It is used to specify the strategy to use, if we encounter the
overflow during filling the list of authorities.
The default strategy is to throw an exception (just like the currently
implemented behavior.)
It can also be set to the values 'empty' or 'truncate', which will
make the server to send an empty or truncated list upon overflow.
Would you please help review it?
http://cr.openjdk.java.net/~igerasim/8154947/01/webrev/
With kind regards,
Ivan
On 22.04.2016 20:09, Ivan Gerasimov wrote:
Hello everyone!
During TLS handshake, a server may be required to send a
CertificateRequest, which contains a list of authorities.
If the list happens to be too long, the server is throwing an
exception, indicating an overflow.
It may be convenient to be able to just drop the list altogether, and
let the client to choose a certificate randomly.
In certain situation this may be more preferable that just block
communication.
Would you please help review a patch, which introduces an
command-line option that controls this behavior of the server?
If the approach is approved, I'll file a CCC request for that option.
BUGURL: https://bugs.openjdk.java.net/browse/JDK-8154947
WEBREV: http://cr.openjdk.java.net/~igerasim/8154947/00/webrev/
With the proposed fix all the security-related regression tests,
including the modified one, passed on all supported platforms.
With kind regards,
Ivan
