[HARMONY-6684] Implementation of TLS DHE_RSA support is broken

• Type: Bug
• Status: Open
• Priority: Major
• Resolution: Unresolved
• Affects Version/s: 5.0M15
• Component/s: Classlib
• Environment: Android / Gingerbread
• Estimated Complexity: Novice
• Assignee: Unassigned
• Reporter: Hristo Bojinov
• Created: 17/May/11 23:13
• Updated: 17/May/11 23:13

Description

The issue was found in Android/Gingerbread, however visual inspection of the trunk code shows the same issue is still present.

The problem is in the xnet ClientHandshakeImpl handling of DHE_RSA cipher suites. The problematic code is this:

if (clientCert != null
470 && serverCert != null
471 && (session.cipherSuite.keyExchange == CipherSuite.KeyExchange_DHE_RSA
472 || session.cipherSuite.keyExchange == CipherSuite.KeyExchange_DHE_DSS)) {
473 PublicKey client_pk = clientCert.certs[0].getPublicKey();
474 PublicKey server_pk = serverCert.certs[0].getPublicKey();
475 if (client_pk instanceof DHKey
476 && server_pk instanceof DHKey) {
477 if (((DHKey) client_pk).getParams().getG().equals(
478 ((DHKey) server_pk).getParams().getG())
479 && ((DHKey) client_pk).getParams().getP()
480 .equals(((DHKey) server_pk).getParams().getG()))

{ 481 // client cert message DH public key parameters 482 // matched those specified by the 483 // server in its certificate, 484 clientKeyExchange = new ClientKeyExchange(); // empty 485 }

486 }
487 } else

{ 488 clientKeyExchange = new ClientKeyExchange( 489 ((DHPublicKey) key).getY()); 490 }

First of all, note that getP() is compared to getG()---a typo on line 480. The bigger problem is that when the public key used is RSA, clientKeyExchange is left uninitizalized (null), which causes an exception in the following code a bit down (when dereferencing clientKeyExchange in the conditional, line 509):

508 // fixed DH parameters
509 if (clientCert != null && !clientKeyExchange.isEmpty()) {
510 // Certificate verify

The way to test this is to limit the enabled cipher suites to just 0x33 (TLS_DHE_RSA_WITH_AES_128_CBC_SHA), and try to use this library on the client, with a server that requires client authentication.

A possible fix is to add an "else" statement at line 486, which will handle non-DH public keys (RSA, in particular). It can simply be a copy of the code in 488-489, using the locally generated DHPublicKey varaible "key".