Derby
  1. Derby
  2. DERBY-2796

Obscure error messages when using SSL in various combinations

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 10.3.1.4
    • Fix Version/s: 10.3.1.4, 10.4.1.3
    • Component/s: None
    • Labels:
      None
    • Bug behavior facts:
      Security

      Description

      I ran clients with various ssl configurations on their urls and startup options against servers with various ssl configurations. I will attach an html file recording my results. I feel that many of the error conditions raised diagnostics which were too obscure to be helpful. I think this will be burdensome to tech support.

      1. DERBY-2796-code.diff
        40 kB
        Bernt M. Johnsen
      2. DERBY-2796-code.stat
        2 kB
        Bernt M. Johnsen
      3. DERBY-2796-code-v2.diff
        41 kB
        Bernt M. Johnsen
      4. DERBY-2796-code-v2.stat
        2 kB
        Bernt M. Johnsen
      5. DERBY-2796-docs.diff
        0.8 kB
        Bernt M. Johnsen
      6. DERBY-2796-docs.stat
        0.0 kB
        Bernt M. Johnsen
      7. DERBY-2796-docs.zip
        2 kB
        Bernt M. Johnsen
      8. DERBY-2796-fix-sslexception.diff
        22 kB
        Bernt M. Johnsen
      9. DERBY-2796-fix-sslexception.stat
        1.0 kB
        Bernt M. Johnsen
      10. ssltest.html
        4 kB
        Rick Hillegas

        Issue Links

          Activity

          Hide
          Rick Hillegas added a comment -

          Attaching ssltest.html, which describes my findings.

          Show
          Rick Hillegas added a comment - Attaching ssltest.html, which describes my findings.
          Hide
          Bernt M. Johnsen added a comment -

          Thanks for your input.

          1) When you have a plaintext server/client communication with an ssl peer, the plaintext side will just see garbage. That garbage, may be SSL-encoded communication or it might be something else. It is hard to give meaningful error-messages and it would require a major rewrite of the DRDA code, since this is detected way donw in the call stack. I think documentation is the proper solution here for the time being.

          2) The SSLException "javax.net.ssl.SSLException: Unrecognized SSL message, plaintext connection?" I think is ok, but it might be good to get rid of the stack trace. We have to propagate the text out to the user, since that is the only clue the user might get of what went wrong. Is this an doc issue too?

          Show
          Bernt M. Johnsen added a comment - Thanks for your input. 1) When you have a plaintext server/client communication with an ssl peer, the plaintext side will just see garbage. That garbage, may be SSL-encoded communication or it might be something else. It is hard to give meaningful error-messages and it would require a major rewrite of the DRDA code, since this is detected way donw in the call stack. I think documentation is the proper solution here for the time being. 2) The SSLException "javax.net.ssl.SSLException: Unrecognized SSL message, plaintext connection?" I think is ok, but it might be good to get rid of the stack trace. We have to propagate the text out to the user, since that is the only clue the user might get of what went wrong. Is this an doc issue too?
          Hide
          Bernt M. Johnsen added a comment -

          Fixing the doc typo.

          Show
          Bernt M. Johnsen added a comment - Fixing the doc typo.
          Hide
          Bernt M. Johnsen added a comment -

          The attachment belonged to DERBY-2795 and was therefore romved here.

          Show
          Bernt M. Johnsen added a comment - The attachment belonged to DERBY-2795 and was therefore romved here.
          Hide
          Bernt M. Johnsen added a comment -

          The plan was that the network server refactoring (DERBY-2412) should clean up the exception handling so that the javax.net.ssl.SSLException stack trace should be fixed then.

          Show
          Bernt M. Johnsen added a comment - The plan was that the network server refactoring ( DERBY-2412 ) should clean up the exception handling so that the javax.net.ssl.SSLException stack trace should be fixed then.
          Hide
          Bernt M. Johnsen added a comment -

          Suggestion:

          I'll try and get rid of the SSLException stack trace and change the two obscure error messages to e.g.:

          ERROR 58009: A network protocol error was encountered and the connection has been terminated: A PROTOCOL Data Stream Syntax Error was detected. Reason: 0x3. Plaintext connection to an SSL enabled server?

          and

          Invalid reply header from network server: Invalid string . Plaintext connection to an SSL enabled server?

          -------------------------------------------------------
          There is also another case: If you run an ssl-enabled client against a plaintext server, you will get the following on the server console output:

          Execution failed because of a Distributed Protocol Error: DRDA_Proto_SYNTAXRM; CODPNT arg = 0; Error Code Value = 3
          org.apache.derby.impl.drda.DRDAProtocolException: Execution failed because of a Distributed Protocol Error: DRDA_Proto_SYNTAXRM; CODPNT arg = 0; Error Code Value = 3
          at org.apache.derby.impl.drda.DRDAConnThread.throwSyntaxrm(DRDAConnThread.java:468)
          at org.apache.derby.impl.drda.DDMReader.readDssHeader(DDMReader.java:348)
          at org.apache.derby.impl.drda.DRDAConnThread.exchangeServerAttributes(DRDAConnThread.java:1024)
          at org.apache.derby.impl.drda.DRDAConnThread.sessionInitialState(DRDAConnThread.java:618)
          at org.apache.derby.impl.drda.DRDAConnThread.run(DRDAConnThread.java:264)

          The text "Execution failed because of a Distributed Protocol Error: DRDA_Proto_SYNTAXRM; CODPNT arg = 0; Error Code Value = 3"

          should be changed to "Execution failed because of a Distributed Protocol Error: DRDA_Proto_SYNTAXRM; CODPNT arg = 0; Error Code Value = 3. SSL connection attempt to plaintext server?
          --------------------------------------------------
          An finally: The docs should state that a plaintext server or client has no way to know whether the ther side is an SSL enabled derby client/server or some prgram using a totally different protocol, and thus the error messages you get might seem a bit awkward.

          Show
          Bernt M. Johnsen added a comment - Suggestion: I'll try and get rid of the SSLException stack trace and change the two obscure error messages to e.g.: ERROR 58009: A network protocol error was encountered and the connection has been terminated: A PROTOCOL Data Stream Syntax Error was detected. Reason: 0x3. Plaintext connection to an SSL enabled server? and Invalid reply header from network server: Invalid string . Plaintext connection to an SSL enabled server? ------------------------------------------------------- There is also another case: If you run an ssl-enabled client against a plaintext server, you will get the following on the server console output: Execution failed because of a Distributed Protocol Error: DRDA_Proto_SYNTAXRM; CODPNT arg = 0; Error Code Value = 3 org.apache.derby.impl.drda.DRDAProtocolException: Execution failed because of a Distributed Protocol Error: DRDA_Proto_SYNTAXRM; CODPNT arg = 0; Error Code Value = 3 at org.apache.derby.impl.drda.DRDAConnThread.throwSyntaxrm(DRDAConnThread.java:468) at org.apache.derby.impl.drda.DDMReader.readDssHeader(DDMReader.java:348) at org.apache.derby.impl.drda.DRDAConnThread.exchangeServerAttributes(DRDAConnThread.java:1024) at org.apache.derby.impl.drda.DRDAConnThread.sessionInitialState(DRDAConnThread.java:618) at org.apache.derby.impl.drda.DRDAConnThread.run(DRDAConnThread.java:264) The text "Execution failed because of a Distributed Protocol Error: DRDA_Proto_SYNTAXRM; CODPNT arg = 0; Error Code Value = 3" should be changed to "Execution failed because of a Distributed Protocol Error: DRDA_Proto_SYNTAXRM; CODPNT arg = 0; Error Code Value = 3. SSL connection attempt to plaintext server? -------------------------------------------------- An finally: The docs should state that a plaintext server or client has no way to know whether the ther side is an SSL enabled derby client/server or some prgram using a totally different protocol, and thus the error messages you get might seem a bit awkward.
          Hide
          Bernt M. Johnsen added a comment -

          A patch to fix the unneeded SSLException stack trace to the console. Solved by starting the ssl handshake immediately after the socket creation. DRDA_NoIO needed an extra argument to communicate why connect did not succeed.

          Show
          Bernt M. Johnsen added a comment - A patch to fix the unneeded SSLException stack trace to the console. Solved by starting the ssl handshake immediately after the socket creation. DRDA_NoIO needed an extra argument to communicate why connect did not succeed.
          Hide
          Bernt M. Johnsen added a comment -

          Complete code fix attached which should solved all the issues mentioned by Rick.

          Show
          Bernt M. Johnsen added a comment - Complete code fix attached which should solved all the issues mentioned by Rick.
          Hide
          Bernt M. Johnsen added a comment -

          Uploaded patch which adds a note about possible protocol error messages when mixing plaintext and ssl.

          Show
          Bernt M. Johnsen added a comment - Uploaded patch which adds a note about possible protocol error messages when mixing plaintext and ssl.
          Hide
          Bernt M. Johnsen added a comment -

          Uploaded patch which adds a note about possible protocol error messages when mixing plaintext and ssl.

          Show
          Bernt M. Johnsen added a comment - Uploaded patch which adds a note about possible protocol error messages when mixing plaintext and ssl.
          Hide
          Bernt M. Johnsen added a comment -

          New version of code patch fixing testconnection master file.

          Show
          Bernt M. Johnsen added a comment - New version of code patch fixing testconnection master file.
          Hide
          Dag H. Wanvik added a comment -

          The patch looks good to me; the hint in the error messages will be
          indeed helpful to understand what's going on if the user has
          misconfigured.

          I notice that you had to remove localized versions of J131 "A PROTOCOL
          Data Stream Syntax Error was detected. Reason: 0x

          {0}

          ." since this is
          now extended. For these locales this message will then fall back to
          English.

          I did not run any tests to verify these changes.

          Nits:

          • NetworkServerControlImpl
          • Modified lines > 80
          • Spurious blank diffs (lines 2288-2290)

          I see you have updated the user docs and releaseNote.html of
          DERBY-2108 accordingly, good! Some small comments on the latter:

          > Summary of Change - SSL/TLS implemented for client/server
          > communication.

          I would make the title more descriptive:

          "Summary of Change - Encryption of data traffic between client and
          server is now supported via SSL/TLS."

          or some such.

          > Rationale for Change - The messages had to be extended due to more
          > failure scenarios when connecting a client to a Derby server.

          I think this is the rationale for the changed error messages, not for
          the issue's feature change which is introduction of SSL support.

          I think the rationale here would be something like:

          "Encryption of data traffic between client and server is a desired
          security feature for Derby."

          Another change not mentioned is that localized versions of J131 will
          now fall back to English. You may want to add that, perhaps.

          Show
          Dag H. Wanvik added a comment - The patch looks good to me; the hint in the error messages will be indeed helpful to understand what's going on if the user has misconfigured. I notice that you had to remove localized versions of J131 "A PROTOCOL Data Stream Syntax Error was detected. Reason: 0x {0} ." since this is now extended. For these locales this message will then fall back to English. I did not run any tests to verify these changes. Nits: NetworkServerControlImpl Modified lines > 80 Spurious blank diffs (lines 2288-2290) I see you have updated the user docs and releaseNote.html of DERBY-2108 accordingly, good! Some small comments on the latter: > Summary of Change - SSL/TLS implemented for client/server > communication. I would make the title more descriptive: "Summary of Change - Encryption of data traffic between client and server is now supported via SSL/TLS." or some such. > Rationale for Change - The messages had to be extended due to more > failure scenarios when connecting a client to a Derby server. I think this is the rationale for the changed error messages, not for the issue's feature change which is introduction of SSL support. I think the rationale here would be something like: "Encryption of data traffic between client and server is a desired security feature for Derby." Another change not mentioned is that localized versions of J131 will now fall back to English. You may want to add that, perhaps.
          Hide
          Bernt M. Johnsen added a comment -

          Thanks Dag. I won't have time to address your issues with the patch before the deadline (which is in 2 minutes), so I really hope there will be an RC2 for this issue to make it into 10.3.

          Show
          Bernt M. Johnsen added a comment - Thanks Dag. I won't have time to address your issues with the patch before the deadline (which is in 2 minutes), so I really hope there will be an RC2 for this issue to make it into 10.3.
          Hide
          Bernt M. Johnsen added a comment -

          Code: Committed revision 548301 on trunk.
          Docs: Committed revision 548304 on trunk.

          Show
          Bernt M. Johnsen added a comment - Code: Committed revision 548301 on trunk. Docs: Committed revision 548304 on trunk.
          Hide
          Bernt M. Johnsen added a comment -

          Code: Committed revision 548322 on 10.3 branch.
          Docs: Committed revision 548321 on 10.3 branch.

          Show
          Bernt M. Johnsen added a comment - Code: Committed revision 548322 on 10.3 branch. Docs: Committed revision 548321 on 10.3 branch.
          Hide
          Myrna van Lunteren added a comment -

          Unchecking 10.3.0.0, The version number of trunk was bumped with revision 547808, the changes went into trunk at revision 548301, that was 10.4.

          Show
          Myrna van Lunteren added a comment - Unchecking 10.3.0.0, The version number of trunk was bumped with revision 547808, the changes went into trunk at revision 548301, that was 10.4.

            People

            • Assignee:
              Bernt M. Johnsen
              Reporter:
              Rick Hillegas
            • Votes:
              0 Vote for this issue
              Watchers:
              0 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development