Derby
  1. Derby
  2. DERBY-5565

Network Server should reject client connections that are not Derby Network Client

    Details

    • Type: Improvement Improvement
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 10.9.1.0
    • Fix Version/s: 10.9.1.0
    • Component/s: Network Server
    • Labels:
      None
    • Issue & fix info:
      High Value Fix, Newcomer, Patch Available, Release Note Needed

      Description

      Since there have been no other network clients besides Derby Network Client tested or supported with Derby since 10.1 and since any protocol based client needs to understand Derby's DRDA extensions, deviations, and stored procedure usage. I think it would be a good idea in 10.9 for Network Server to outright reject any network clients that are not Derby Network Client.

      This would eliminate confusion up front for those that might not be aware that the DB2 Universal JDBC Driver and DB2 Runtime Client are not supported. They would get a clean reasonable error instead of hitting various protocol errors.

      Also it would mean if someone does want to add support for some network client in the future they would at least need to add the one or two lines of code in AppRequester to identify it, which I think would be a good thing.

      I think the code change would not be hard but the biggest impact might be anyone who still runs tests with JCC on trunk would need to disable those tests. There is a separate issue DERBY-4785 that Jayaram is working on to complete remove the JCC related code from the tests and test infrastructure.

      1. releaseNote.html
        5 kB
        Kathey Marsden
      2. JCCDSConnectTest.java
        1 kB
        Kathey Marsden
      3. derby-5565_diff.txt
        6 kB
        Kathey Marsden
      4. derby-5565_diff.txt
        18 kB
        Kathey Marsden
      5. derby.log
        4 kB
        Kathey Marsden

        Activity

        Hide
        Bryan Pendleton added a comment -

        Good idea! Giving a better error message to people who try those old
        client connections will be much nicer.

        I haven't run JCC tests against the trunk in years.

        Show
        Bryan Pendleton added a comment - Good idea! Giving a better error message to people who try those old client connections will be much nicer. I haven't run JCC tests against the trunk in years.
        Hide
        Kathey Marsden added a comment -

        Attached is a patch for this issue, sample derby.log when JCC tries to connect and the program I used to try to connect with JCC.

        If a non-derby client tries to connect, we will throw a requiredValueNotFound SYNTAXRM error and log a more informative error to derby.log:

        Fri Apr 20 14:22:44 PDT 2012 : ERROR UNSUPPORTED CLIENT: Invalid client product id JCC03580, Derby Network Client (DNC)
        is the only supported client Product
        Followed by the SYNTAXRM error.

        Running tests now.

        Show
        Kathey Marsden added a comment - Attached is a patch for this issue, sample derby.log when JCC tries to connect and the program I used to try to connect with JCC. If a non-derby client tries to connect, we will throw a requiredValueNotFound SYNTAXRM error and log a more informative error to derby.log: Fri Apr 20 14:22:44 PDT 2012 : ERROR UNSUPPORTED CLIENT: Invalid client product id JCC03580, Derby Network Client (DNC) is the only supported client Product Followed by the SYNTAXRM error. Running tests now.
        Hide
        Kathey Marsden added a comment -

        It seems the protocol tests use their own product id TST so fail with the error in the log.

        Fri Apr 20 14:38:06 PDT 2012 : ERROR UNSUPPORTED CLIENT: Invalid client product
        id TST01000, Derby Network Client (DNC) is the only supported client Product

        I look and see if that can be changed for the test or if Network server will have to support TST product id as well for the tests.

        Show
        Kathey Marsden added a comment - It seems the protocol tests use their own product id TST so fail with the error in the log. Fri Apr 20 14:38:06 PDT 2012 : ERROR UNSUPPORTED CLIENT: Invalid client product id TST01000, Derby Network Client (DNC) is the only supported client Product I look and see if that can be changed for the test or if Network server will have to support TST product id as well for the tests.
        Hide
        Knut Anders Hatlen added a comment -

        Do you have any idea how the ODBC driver identifies itself? Assuming it will be rejected too, would it be OK to remove the code that's only there to support ODBC now?

        Show
        Knut Anders Hatlen added a comment - Do you have any idea how the ODBC driver identifies itself? Assuming it will be rejected too, would it be OK to remove the code that's only there to support ODBC now?
        Hide
        Kathey Marsden added a comment -

        I beleive it is SQL according to https://collaboration.opengroup.org/dbiop/uploads/40/9358/drda-pid.htm?gpid=453&type=doc&id=9358&fn=drda-pid.htm

        The Network Server code where it branched for ODBC used the shorter exception parameter length as the default and did not identify address it as CCC. The patch does take the CCC references out of AppRequester, eg.

        • static final int CCC_CLIENT = 2; // not yet supported.
        • default:
        • // Default is the max for C clients, since that is more
        • // restricted than for JCC clients. Note, though, that
        • // JCC clients are the only ones supported right now.
        • return Limits.DB2_CCC_MAX_EXCEPTION_PARAM_LENGTH;
          -
        • }

        I am not sure what other code is in there that could or should be stripped out. I think there are some stored procedures for ODBC style metadata. I don't plan to touch them as part of this issue. As they are generic to ODBC and not the DB2 client specifically, I thought it might be good to keep them in case someone ever does want to make an ODBC driver for Derby.

        Show
        Kathey Marsden added a comment - I beleive it is SQL according to https://collaboration.opengroup.org/dbiop/uploads/40/9358/drda-pid.htm?gpid=453&type=doc&id=9358&fn=drda-pid.htm The Network Server code where it branched for ODBC used the shorter exception parameter length as the default and did not identify address it as CCC. The patch does take the CCC references out of AppRequester, eg. static final int CCC_CLIENT = 2; // not yet supported. default: // Default is the max for C clients, since that is more // restricted than for JCC clients. Note, though, that // JCC clients are the only ones supported right now. return Limits.DB2_CCC_MAX_EXCEPTION_PARAM_LENGTH; - } I am not sure what other code is in there that could or should be stripped out. I think there are some stored procedures for ODBC style metadata. I don't plan to touch them as part of this issue. As they are generic to ODBC and not the DB2 client specifically, I thought it might be good to keep them in case someone ever does want to make an ODBC driver for Derby.
        Hide
        Kathey Marsden added a comment -

        Attached is a patch for this issue.
        We now reject all PRDID's that do not start with DNC.
        The protocol tests have been changed to use the DNC10090 PRDID instead of TST01000. This actually alters the code path where there is slightly different behavior in later derby client versions, so that we have to skip an additional dss on connect for the diagnostic information. After this commit I plan to add another protocol test for the client reject but thougth I would get just the changes for the existing tests in first.

        Tests passed. Please review.

        Show
        Kathey Marsden added a comment - Attached is a patch for this issue. We now reject all PRDID's that do not start with DNC. The protocol tests have been changed to use the DNC10090 PRDID instead of TST01000. This actually alters the code path where there is slightly different behavior in later derby client versions, so that we have to skip an additional dss on connect for the diagnostic information. After this commit I plan to add another protocol test for the client reject but thougth I would get just the changes for the existing tests in first. Tests passed. Please review.
        Hide
        Kathey Marsden added a comment -

        Attaching a release note for this issue.

        Show
        Kathey Marsden added a comment - Attaching a release note for this issue.
        Hide
        Kathey Marsden added a comment -

        Resolving this issue for 10.9

        Show
        Kathey Marsden added a comment - Resolving this issue for 10.9
        Hide
        Knut Anders Hatlen added a comment -

        [bulk update] Close all resolved issues that haven't been updated for more than one year.

        Show
        Knut Anders Hatlen added a comment - [bulk update] Close all resolved issues that haven't been updated for more than one year.

          People

          • Assignee:
            Kathey Marsden
            Reporter:
            Kathey Marsden
          • Votes:
            0 Vote for this issue
            Watchers:
            2 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development