Derby
  1. Derby
  2. DERBY-3532

Invalid & possibly skipped authentication handling when shutting down the network server.

    Details

    • Type: Bug Bug
    • Status: Open
    • Priority: Critical Critical
    • Resolution: Unresolved
    • Affects Version/s: 10.4.1.3, 10.5.1.1
    • Fix Version/s: None
    • Component/s: Network Server
    • Urgency:
      Normal
    • Bug behavior facts:
      Security

      Description

      In NetworkServerControlImpl.checkShutdownPrivileges() code fetches the internal authentication service to perform user authentication.

      However if no such authentication service is found (null is returned) then authentication is bypassed, this has the potential of being a security hole.

      The discussion in DERBY-2109 indicated that even with authentication NONE, there is still an internal authentication service, thus null is not a valid return when getting the internal authentication service. A secure fail safe system would be to not bypass authentication if null is returned.

      I tried removing the check for null in the method and that lead to NullPointerExceptions. This means that something wrong is going on and very possibly no authentication checks are actually being made when shutting down the network server.

      The null return might be due to checking the authentication after Derby has been shutdown.

      1. ReproDerby3532.java
        2 kB
        Kathey Marsden
      2. ReproDerby3532.java
        2 kB
        Lily Wei
      3. DERBY-3532.diff
        3 kB
        Lily Wei

        Issue Links

          Activity

          Hide
          Tiago R. Espinha added a comment -

          Triaged for 10.5.2.
          Checked High Value Fix.
          Bumped urgency to Urgent.

          This seems like something we should fix for 10.5.2.

          Show
          Tiago R. Espinha added a comment - Triaged for 10.5.2. Checked High Value Fix. Bumped urgency to Urgent. This seems like something we should fix for 10.5.2.
          Hide
          Kathey Marsden added a comment -

          Changing to normal urgency. It is not a regression and nobody seems to be working on it so it won't make it into 10.5.2. I agree we should prioritize it for the next release.

          Show
          Kathey Marsden added a comment - Changing to normal urgency. It is not a regression and nobody seems to be working on it so it won't make it into 10.5.2. I agree we should prioritize it for the next release.
          Hide
          Kristian Waagan added a comment -

          Changed urgency to normal, as described in the previous comment.

          Show
          Kristian Waagan added a comment - Changed urgency to normal, as described in the previous comment.
          Hide
          Kathey Marsden added a comment -

          Lily told me that she got a NPE in ServerPropertiesTest when she commented out the check Dan referenced. Lily, please try this program with your modified server to see if it pops the bug.

          Show
          Kathey Marsden added a comment - Lily told me that she got a NPE in ServerPropertiesTest when she commented out the check Dan referenced. Lily, please try this program with your modified server to see if it pops the bug.
          Hide
          Lily Wei added a comment -

          I cannot reproduce AuthenticationService(auth) is null with the previous ReproDerby3532. I am able to reproduce AuthenticationService(auth) == null with the new repro file.
          When network service server is shutting down using embedded data source, network server does not know shutdown has been call. Hence, network server did not boot to fill the removed AuthenticationService.
          I am not sure what Derby can do at this case. It will be nice if derbynet.ServerPropertiesTest and management.InactiveManagementMBeanTest does not try to shutdown network server with embedded data source.

          Show
          Lily Wei added a comment - I cannot reproduce AuthenticationService(auth) is null with the previous ReproDerby3532. I am able to reproduce AuthenticationService(auth) == null with the new repro file. When network service server is shutting down using embedded data source, network server does not know shutdown has been call. Hence, network server did not boot to fill the removed AuthenticationService. I am not sure what Derby can do at this case. It will be nice if derbynet.ServerPropertiesTest and management.InactiveManagementMBeanTest does not try to shutdown network server with embedded data source.
          Hide
          Kathey Marsden added a comment -

          Lily said:
          I am not sure what Derby can do at this case. It will be nice if derbynet.ServerPropertiesTest and management.InactiveManagementMBeanTest does not try to shutdown network server with embedded data source.

          It is a little confusing with the different shutdowns going on.
          The tests are not shutting down network srever with the embedded driver but rather using it to shutdown the embedded engine which network server is using. So when we reproduce the bug the sequence is in a single JVM:
          1) start network server
          2) shutdown the embedded engine (not network server) with the embedded driver (either DriverManager.getConnection("jdbc:derby:;shutdown=true) or with EmbeddedDataSource as the latest repro.
          3) Shutdown network server. Since network server relies on the embedded engine to manage authentication and it is now gone the findService() call is null.
          I am sure also there are other network server cleanup issues in this scenario.

          In our first attempt at ReproDerby3532 in step 2 we shutdown the embedded engine with the client driver. In this case Network Server is aware that the engine is being shutdown because it came in through the client. Network Server catches the XJ015 error cleans up some of the network server threads and sessions and threads and restarts the embedded engine.
          This happens in DRDAConnThread.writeSQLCARD(). The cleanup and engine restart occurs in the poorly named NetworkServerControl.startNetworkServer() which cleans up network server if needed and starts the embedded engine.

          One option is to document that users should not shutdown the embedded engine with the embedded driver while network server is running and as you suggest, change here
          change the tests to shut down the engine with the client driver.

          I wonder instead if it would work and be worth it to put a lightweight check in network server for each client request to see if the embedded engine is running and if it is not, do the cleanup and start the embedded engine. Then we would always find the authentication service on shutdown and would be able to handle any other cleanup issues as well.

          Thoughts?

          Show
          Kathey Marsden added a comment - Lily said: I am not sure what Derby can do at this case. It will be nice if derbynet.ServerPropertiesTest and management.InactiveManagementMBeanTest does not try to shutdown network server with embedded data source. It is a little confusing with the different shutdowns going on. The tests are not shutting down network srever with the embedded driver but rather using it to shutdown the embedded engine which network server is using. So when we reproduce the bug the sequence is in a single JVM: 1) start network server 2) shutdown the embedded engine (not network server) with the embedded driver (either DriverManager.getConnection("jdbc:derby:;shutdown=true) or with EmbeddedDataSource as the latest repro. 3) Shutdown network server. Since network server relies on the embedded engine to manage authentication and it is now gone the findService() call is null. I am sure also there are other network server cleanup issues in this scenario. In our first attempt at ReproDerby3532 in step 2 we shutdown the embedded engine with the client driver. In this case Network Server is aware that the engine is being shutdown because it came in through the client. Network Server catches the XJ015 error cleans up some of the network server threads and sessions and threads and restarts the embedded engine. This happens in DRDAConnThread.writeSQLCARD(). The cleanup and engine restart occurs in the poorly named NetworkServerControl.startNetworkServer() which cleans up network server if needed and starts the embedded engine. One option is to document that users should not shutdown the embedded engine with the embedded driver while network server is running and as you suggest, change here change the tests to shut down the engine with the client driver. I wonder instead if it would work and be worth it to put a lightweight check in network server for each client request to see if the embedded engine is running and if it is not, do the cleanup and start the embedded engine. Then we would always find the authentication service on shutdown and would be able to handle any other cleanup issues as well. Thoughts?
          Hide
          Lily Wei added a comment -

          I am hesitating to decide having lightweight check in network server for each client request to see if the embedded engine is running. If there is any good suggestion, I will be happy to try.
          I am trying to fix the test problem in junit regarding when network service server is shutting down it is using an embedded data source. First try, I change DriverManagerConnector.shutEngine and hope to shutengine with network server URL instead of embedded URL. However, if I use jdbcclient.getUrlBase(), I will get error like:
          Caused by: org.apache.derby.client.am.SqlException: The URL 'jdbc:derby://localhost:1527/' is not properly formed.
          Second try, If I take out the extra "/" from JDBCClient.DERBYNETCLIENT, I will get error:
          Caused by: java.sql.SQLException: Database '/localhost:1527/' not found.
          Is there any particular reason we put comment "Always shutsdown using the embedded URL thus this method will not work in a remote testing environment." in DriverManagerConnector.shutEngine()?" I think the writer is reading my mind now. I am open to any suggestion.

          Show
          Lily Wei added a comment - I am hesitating to decide having lightweight check in network server for each client request to see if the embedded engine is running. If there is any good suggestion, I will be happy to try. I am trying to fix the test problem in junit regarding when network service server is shutting down it is using an embedded data source. First try, I change DriverManagerConnector.shutEngine and hope to shutengine with network server URL instead of embedded URL. However, if I use jdbcclient.getUrlBase(), I will get error like: Caused by: org.apache.derby.client.am.SqlException: The URL 'jdbc:derby://localhost:1527/' is not properly formed. Second try, If I take out the extra "/" from JDBCClient.DERBYNETCLIENT, I will get error: Caused by: java.sql.SQLException: Database '/localhost:1527/' not found. Is there any particular reason we put comment "Always shutsdown using the embedded URL thus this method will not work in a remote testing environment." in DriverManagerConnector.shutEngine()?" I think the writer is reading my mind now. I am open to any suggestion.
          Hide
          Kathey Marsden added a comment -

          You got the exception org.apache.derby.client.am.SqlException: The URL 'jdbc:derby://localhost:1527/' is not properly formed
          It seems it is missing the shutdown. The url should be getURLBase() + ";shutdown=true'

          The comment is mysterious because
          1) We do not I think support remote testing with JUnit.
          2) If we did, the embedded driver would not be available, just the client, so you wouldn't be able to shutdown using the embedded driver.

          I think it is ok to change

          Show
          Kathey Marsden added a comment - You got the exception org.apache.derby.client.am.SqlException: The URL 'jdbc:derby://localhost:1527/' is not properly formed It seems it is missing the shutdown. The url should be getURLBase() + ";shutdown=true' The comment is mysterious because 1) We do not I think support remote testing with JUnit. 2) If we did, the embedded driver would not be available, just the client, so you wouldn't be able to shutdown using the embedded driver. I think it is ok to change
          Hide
          Lily Wei added a comment -

          Thank you, Kathey. I made the change with your suggestion.
          It is worth noting that Network Server will automatically restart Derby embedded if it detects in test environment. And, this is why management.InactiveManagementMBeanTest failed with the change. As Kathey pointed out, users can shutdown the embedded engine from client with ClientDataSource but not with DriverManager.getConnection(url) (please see DerbyRepro3532.java)
          If we allow ClientDataSource to shut down embedded engine, authentication service could be null. If DriverManager.getConnection(url) can not shutdown the engine, some tests will failed. i.e. management.InactiveManagementMBeanTest will still have embedded engine running since network server restart it. Maybe we should not allow ClientDataSource to shut down embedded engine and make it too heavy of check that can impact performance. Any suggestion is welcome.

          Show
          Lily Wei added a comment - Thank you, Kathey. I made the change with your suggestion. It is worth noting that Network Server will automatically restart Derby embedded if it detects in test environment. And, this is why management.InactiveManagementMBeanTest failed with the change. As Kathey pointed out, users can shutdown the embedded engine from client with ClientDataSource but not with DriverManager.getConnection(url) (please see DerbyRepro3532.java) If we allow ClientDataSource to shut down embedded engine, authentication service could be null. If DriverManager.getConnection(url) can not shutdown the engine, some tests will failed. i.e. management.InactiveManagementMBeanTest will still have embedded engine running since network server restart it. Maybe we should not allow ClientDataSource to shut down embedded engine and make it too heavy of check that can impact performance. Any suggestion is welcome.
          Hide
          Dag H. Wanvik added a comment -

          Not sure if I understand this entirely, but as I read this thread, it is in some connection modes possible to shut down the engine underneath the network server, in other modes it is not possible?
          If so, it seems wrong that it should be possible to take down the engine when the server is running. It seems to me, the correct sequence is to first take down the server, then the engine.

          Show
          Dag H. Wanvik added a comment - Not sure if I understand this entirely, but as I read this thread, it is in some connection modes possible to shut down the engine underneath the network server, in other modes it is not possible? If so, it seems wrong that it should be possible to take down the engine when the server is running. It seems to me, the correct sequence is to first take down the server, then the engine.
          Hide
          Kathey Marsden added a comment -

          Dag said:
          >it is in some connection modes possible to shut down the
          engine underneath the network server, in other modes it is not possible?

          Yes currently from the client you can shutdown the engine remotely with ClientDataSource but not with ClientDriver/DriverManager. The
          two should be the same. The question is whether to to disable this capability for ClientDataSource or enable it for ClientDriver. Disabling functionality of course has the risk of regressing someone that is using it.

          If running in the same JVM you can also shutdown the engine with EmbeddedDriver or EmbeddedDataSource. I think for these two we just should document that it is ill advised. I don't think we can prevent it.

          Show
          Kathey Marsden added a comment - Dag said: >it is in some connection modes possible to shut down the engine underneath the network server, in other modes it is not possible? Yes currently from the client you can shutdown the engine remotely with ClientDataSource but not with ClientDriver/DriverManager. The two should be the same. The question is whether to to disable this capability for ClientDataSource or enable it for ClientDriver. Disabling functionality of course has the risk of regressing someone that is using it. If running in the same JVM you can also shutdown the engine with EmbeddedDriver or EmbeddedDataSource. I think for these two we just should document that it is ill advised. I don't think we can prevent it.
          Hide
          Lily Wei added a comment -

          I create DERBY-4345 for the community to decide whether to disable the capability for ClientDataSource to shutdown embedded engine or enable ClientDriver for such capability.

          At this point, maybe we could document and suggest users should not shutdown the embedded engine with the embedded driver while network server is running. Otherwise, Network Server failed to catch XJ015 error cleans up network threads and sessions and restarts the embedded engine. Therefore, they might not have a authentication handle.

          Show
          Lily Wei added a comment - I create DERBY-4345 for the community to decide whether to disable the capability for ClientDataSource to shutdown embedded engine or enable ClientDriver for such capability. At this point, maybe we could document and suggest users should not shutdown the embedded engine with the embedded driver while network server is running. Otherwise, Network Server failed to catch XJ015 error cleans up network threads and sessions and restarts the embedded engine. Therefore, they might not have a authentication handle.
          Hide
          Dag H. Wanvik added a comment -

          I think we have now established that one can shut down the engine from the client using both ClientDataSource and the DeriverManager (see DERBY-4345). The server detects this. The question is what should we do, if anything, about the fact that using the embedded driver (EmbeddedDataSource or driver manager), we can shut down the engine underneath the server.

          Kathey> I wonder instead if it would work and be worth it to put a
          Kathey> lightweight check in network server for each client request to
          Kathey> see if the embedded engine is running and if it is not, do the
          Kathey> cleanup and start the embedded engine. Then we would always
          Kathey> find the authentication service on shutdown and would be able
          Kathey> to handle any other cleanup issues as well.

          I think it would be good if the network server were able to detect the shutdown and cleanup/reboot the engine, so we get the same behavior using embedded and client shutdown. Would it be costly to have such a check?

          Show
          Dag H. Wanvik added a comment - I think we have now established that one can shut down the engine from the client using both ClientDataSource and the DeriverManager (see DERBY-4345 ). The server detects this. The question is what should we do, if anything, about the fact that using the embedded driver (EmbeddedDataSource or driver manager), we can shut down the engine underneath the server. Kathey> I wonder instead if it would work and be worth it to put a Kathey> lightweight check in network server for each client request to Kathey> see if the embedded engine is running and if it is not, do the Kathey> cleanup and start the embedded engine. Then we would always Kathey> find the authentication service on shutdown and would be able Kathey> to handle any other cleanup issues as well. I think it would be good if the network server were able to detect the shutdown and cleanup/reboot the engine, so we get the same behavior using embedded and client shutdown. Would it be costly to have such a check?
          Hide
          Kathey Marsden added a comment -

          Unchecking HVF. Looks like this is a sticky issue with possible compatibility concerns

          Show
          Kathey Marsden added a comment - Unchecking HVF. Looks like this is a sticky issue with possible compatibility concerns

            People

            • Assignee:
              Unassigned
              Reporter:
              Daniel John Debrunner
            • Votes:
              0 Vote for this issue
              Watchers:
              1 Start watching this issue

              Dates

              • Created:
                Updated:

                Development