Uploaded image for project: 'Harmony'
  1. Harmony
  2. HARMONY-6684

Implementation of TLS DHE_RSA support is broken

    XMLWordPrintableJSON

    Details

    • Type: Bug
    • Status: Open
    • Priority: Major
    • Resolution: Unresolved
    • Affects Version/s: 5.0M15
    • Fix Version/s: None
    • Component/s: Classlib
    • Labels:
      None
    • Environment:
      Android / Gingerbread
    • Estimated Complexity:
      Novice

      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".

        Attachments

          Activity

            People

            • Assignee:
              Unassigned
              Reporter:
              hristo Hristo Bojinov
            • Votes:
              0 Vote for this issue
              Watchers:
              1 Start watching this issue

              Dates

              • Created:
                Updated:

                Time Tracking

                Estimated:
                Original Estimate - 2h
                2h
                Remaining:
                Remaining Estimate - 2h
                2h
                Logged:
                Time Spent - Not Specified
                Not Specified