Qpid
  1. Qpid
  2. QPID-4352

Java client logs key_store_password/trust_store_password from connection url at debug

    Details

    • Type: Bug Bug
    • Status: Resolved
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 0.14, 0.16, 0.18
    • Fix Version/s: 0.19
    • Component/s: Java Client
    • Labels:
      None

      Description

      When run in DEBUG, the Qpid client logs the trust store/key store passwords to the log. This could present a security issue.

      main 2012-09-29 22:32:54,558 DEBUG [apache.qpid.client.AMQConnection] Connection(1):amqp://guest:********@test/?brokerlist='tcp://localhost:15671?trust_store_password='password'&trust_store='test-profiles/test_resources/ssl/java_client_truststore.jks'&ssl_verify_hostname='true'&ssl='true'&key_store_password='password'&key_store='test-profiles/test_resources/ssl/java_client_keystore.jks''
      

      The code should be changed to mask these passwords in the same fashion as the client's password. This change was made by QPID-1208.

        Activity

        Hide
        Keith Wall added a comment -

        Patch applied.

        Hi Robbie, could you review this commit?

        Show
        Keith Wall added a comment - Patch applied. Hi Robbie, could you review this commit?
        Hide
        Robbie Gemmell added a comment -

        Hi Keith,

        I noticed that the connection factory was affected by the changes due to its use of the AMQConnectionURL toString method.

        I have updated it to work around the issue, updated SSLTest so that it uses factories and thus exposes the issue, and updated a couple of tests that were affected by my change due to their horrible manipulation of connection URLs post-creation.

        Can you review my changes?

        (I later plan to look into removing the ability to create an AMQConnectionFactory with a ConnectionURL object instead of a String as it seems no real good comes from having the ability, but potentially lots of hideousness and lack of testing does).

        Show
        Robbie Gemmell added a comment - Hi Keith, I noticed that the connection factory was affected by the changes due to its use of the AMQConnectionURL toString method. I have updated it to work around the issue, updated SSLTest so that it uses factories and thus exposes the issue, and updated a couple of tests that were affected by my change due to their horrible manipulation of connection URLs post-creation. Can you review my changes? (I later plan to look into removing the ability to create an AMQConnectionFactory with a ConnectionURL object instead of a String as it seems no real good comes from having the ability, but potentially lots of hideousness and lack of testing does).
        Hide
        Keith Wall added a comment -

        Hi Robbie, I've reviewed the change and have no comments.

        Show
        Keith Wall added a comment - Hi Robbie, I've reviewed the change and have no comments.

          People

          • Assignee:
            Keith Wall
            Reporter:
            Keith Wall
          • Votes:
            0 Vote for this issue
            Watchers:
            2 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development