Uploaded image for project: 'JDK'
  1. JDK
  2. JDK-8271266

TLS: X509KeyManagerImpl can prematurely select wrong alias from keystore

    XMLWordPrintable

Details

    Description

      ADDITIONAL SYSTEM INFORMATION :
      Generic; tested with OpenJDK 16.0.1 64-bit on Windows 10

      A DESCRIPTION OF THE PROBLEM :
      When multiple certificates are available in the keystore, a wrong one can be prematurely chosen depending on the order of SignatureScheme entries in peerRequestedCertSignSchemes. Incorrect extended key usage of the certificates is then not taken into account.

      STEPS TO FOLLOW TO REPRODUCE THE PROBLEM :
      Consider the following scenario:
      * Server S1 allows or requires TLS client authentication
      * Server S2 has a keystore with two entries:
        1) Alias "signing", type EC, with Extended Key Usage 1.3.6.1.5.5.7.3.3 (code signing)
        2) Alias "tls", type RSA, with Extended Key Usage 1.3.6.1.5.5.7.3.2 (TLS client authentication)
      S1 issues a HTTPS request to S2 (TLS 1.3). S2 will attempt to produce a client certificate, but can choose the "signing" key, causing a handshake failure.

      The reproducing instructions are in the README.md in the project root.

      EXPECTED VERSUS ACTUAL BEHAVIOR :
      EXPECTED -
      The "tls" key should be selected for the TLS client authentication. The "signing" key is intended for some other application-specific purpose.
      ACTUAL -
      The handshake flow eventually reaches sun.security.ssl.CertificateMessage.T13CertificateProducer.produce(ConnectionContext, HandshakeMessage), which calls CertificateMessage.T13CertificateProducer.onProduceCertificate(ClientHandshakeContext, HandshakeMessage) because we're in client mode. This then calls CertificateMessage.T13CertificateProducer.choosePossession(HandshakeContext, ClientHelloMessage).

      Let's assume that hc.peerRequestedCertSignSchemes contains the following list:
      #0 = "ED25519"
      #1 = "ED448"
      #2 = "ECDSA_SECP256R1_SHA256" <-- matches APPROXIMATELY with "signing" key
      #3 = "ECDSA_SECP384R1_SHA384"
      #4 = "ECDSA_SECP521R1_SHA512"
      #5 = "RSA_PSS_RSAE_SHA256" <-- matches PERFECTLY with "tls" key
      #6 = "RSA_PSS_RSAE_SHA384"
      #7 = "RSA_PSS_RSAE_SHA512"

      The choosePossession() algorithm will try to create a possession for each of them in this sequence. #0 and #1 have no matching keys, so 509Authentication.valueOf(ss) will be null for them and so they will be skipped. #2 does have an approximately matching key though: the flow will go to X509Authentication.X509PossessionGenerator.createClientPossession(ClientHandshakeContext, String), then X509KeyManagerImpl.chooseClientAlias(String[], Principal[], Socket) and then X509KeyManagerImpl.chooseAlias(List<KeyType>, Principal[], CheckType, AlgorithmConstraints, List<SNIServerName>, String)

      chooseAlias() will call List<EntryStatus> results = getAliases(...) and return the best available match from its results.

      getAliases() loops over each keystore entry and checks how closely it matches what we're looking for. For #2 above, it discovers that the "signing" entry is a suitable EC key and eventually returns it with CheckResult.EXTENSION_MISMATCH, because it indeed does not have a compatible extended key usage.

      However, the caller chooseAlias() then concludes that that's good enough. The "signing" key is therefore selected and used in the client authentication certificate. S1 sees that its Extended Key Usage is invalid for TLS client authentication and throws an exception.

      The root cause is twofold:
      1) chooseAlias() makes a decision based on the current SignatureScheme provided to it (from one of the peerRequestedCertSignSchemes) and discards the closeness information of the match in doing so (only the alias is returned)
      2) Any alias return by chooseAlias() is immediately accepted as the final answer by choosePossession(), which then stops the search process, so a potentially even better match further down the line will not be considered.

      ---------- BEGIN SOURCE ----------
      Whether this problem occurs or not depends on the relative order of the peerRequestedCertSignSchemes entries and the key type of the unsuitable keystore entry. In this example, if entries #2 and #5 were to be swapped, the RSA_PSS_RSAE_SHA256 SignatureScheme would be tried as #2, chooseAlias() would find a match with the "tls" keystore entry and return it to choosePossession(), which accepts it and produces the correct certificate. For this problem to occur there must also be multiple keystore entries, with different SignatureSchemes and the SignatureScheme matching the unsuitable entry must be tried before the correct one.

      A quick fix would be to add an extra test in chooseAlias() at the very end, where it handles imperfect matches: in case of EXTENSION_MISMATCH, return null instead, causing another scheme to be tried that may be better. This would not change any public interfaces and a certificate with wrong extended key usage should be rejected anyway. Extended key usage is optional, but if it is present it must be correct.

      A more elaborate solution would be to try all schemes with all keys. As soon as there's a perfect match, use it; otherwise keep going and gather everything, then pick the best one. However, X509KeyManager is a public interface, so modifying it to return X509KeyManagerImpl.EntryStatus instead of String would break compatibility.
      ---------- END SOURCE ----------

      CUSTOMER SUBMITTED WORKAROUND :
      Workarounds:
      * keep only one entry in the keystore used for TLS, keep the other ones in a dedicated keystore
      * for the other certificates, ensure that they are not trusted by the peer, so that getAliases() rejects them for TLS purposes


      Attachments

        1. messages_18eab24.log
          755 kB
        2. messages.log
          70 kB
        3. TLS_issue_demo.zip
          28 kB

        Issue Links

          Activity

            People

              weijun Weijun Wang
              webbuggrp Webbug Group
              Votes:
              0 Vote for this issue
              Watchers:
              3 Start watching this issue

              Dates

                Created:
                Updated:
                Resolved: