Xuelei/I inadvertently left off security-dev in a later discussion, so cc'ing here on some main points.

On 11/4/2016 4:33 PM, Xuelei Fan wrote:
As there is a preMaster, may not need the debug-only
debugProtocolVersion class field.  It can be extracted from preMaster in
the print() implementation.

On 11/6/2016 12:51 AM, Bradford Wetmore wrote:
> What if it's not extractible?

Xuelei wrote:

> We know the version for the ClientKeyExchange message generation
> (client side).  For the receiving/server side, no idea about how to
> get the version.  Maybe, we can just dump "version is not
> extractable"?

For the client, the clientKeyExchange protocol version field for the message is actually set in the KeyGenerator, so RSAClientKeyExchange.protocolVersion may or may not be what is sent over the wire. That is: RSAClientKeyExchange.protocolVersion is only a guess, and may not be accurate and will confuse any debug analysis.

For the server side, I would expect the same: if it's not extractable we could output some currentVersion, but again it's only a guess and would confuse things.

So IMHO, we should not look at this.protocolVersion for debug if the preMaster is not extractable:

    void print(PrintStream s) throws IOException {
+       String version = "protocol version not available";
+
+       byte[] ba = preMaster.getEncoded();
+       if (ba != null && ba.length >= 2) {
+           version = ProtocolVersion.valueOf(ba[0], ba[1]).name;
+       }
+
        s.println("*** ClientKeyExchange, RSA PreMasterSecret, " +
-                                         protocolVersion);
+                                         version);

Final update:

https://bugs.openjdk.java.net/browse/JDK-8169229
http://cr.openjdk.java.net/~wetmore/8169229/webrev.01

I'll run it through JPRT, but I'll mark as noreg-trivial.

Brad


>>>>> On 11/5/2016 6:17 AM, Bradford Wetmore wrote:
>>>>>>
>>>>>> https://bugs.openjdk.java.net/browse/JDK-8169229
>>>>>> http://cr.openjdk.java.net/~wetmore/8169229/webrev.00/
>>>>>>
>>>>>> Please review this minor bug fix.  Our RSAClientKeyExchange isn't
>>>>>> properly outputing the RSA PreMasterSecret field.
>>>>>>
>>>>>> Thanks,
>>>>>>
>>>>>> Brad



Reply via email to