Details

    • Type: Bug Bug
    • Status: Resolved
    • Priority: Blocker Blocker
    • Resolution: Fixed
    • Affects Version/s: 3.1, 3.1.1, 4.0
    • Fix Version/s: 3.1_b42, 3.1.1_b06
    • Component/s: OSGi
    • Labels:
      None
    • Environment:

      Operating System: All
      Platform: All

      Description

      This is a request to provide a way to administratively turn off the OSGi shell.

      The server-config has the following arguments to the JVM which control the OSGi
      shell:

      <jvm-options>-Dosgi.shell.telnet.port=6666</jvm-options>
      <jvm-options>-Dosgi.shell.telnet.maxconn=1</jvm-options>
      <jvm-options>-Dosgi.shell.telnet.ip=127.0.0.1</jvm-options>

      The default-config for instances does not have these arguments, but the shell is
      still started using the default values.

      From Jerome:
      I believe the org.apache.felix.scr.jar would need to be moved out of the
      modules/autostart directory.
      I might be mistaken but if there is no administrative way to turn off
      fileinstall or shell, this is not good, we need to revisit this.

      1. patch.txt
        4 kB
        Sanjeeb Sahoo

        Issue Links

          Activity

          Hide
          Richard S. Hall added a comment -

          From an OSGi perspective, the way to administratively turn off a bundle is to
          stop it or not deploy it. In this case, we're talking about the
          org.apache.felix.shell.remote bundle (in modules/), which provides telnet access
          to the shell.

          Also, it depends on what you mean by "turn off", since if you set the max
          connection to 0, I think this will effectively turn it off. However, to get it
          to not start at all, then we'll have to provide a GF-specific way of doing that.

          As far as I can see, these bundles are started in
          GlassFishActivator.startPostStartupBundles(). So, we'd need to add a
          configuration property to specify the post-startup bundles to be started.

          Show
          Richard S. Hall added a comment - From an OSGi perspective, the way to administratively turn off a bundle is to stop it or not deploy it. In this case, we're talking about the org.apache.felix.shell.remote bundle (in modules/), which provides telnet access to the shell. Also, it depends on what you mean by "turn off", since if you set the max connection to 0, I think this will effectively turn it off. However, to get it to not start at all, then we'll have to provide a GF-specific way of doing that. As far as I can see, these bundles are started in GlassFishActivator.startPostStartupBundles(). So, we'd need to add a configuration property to specify the post-startup bundles to be started.
          Hide
          Tom Mueller added a comment -

          "Turn off" means to at least not listen on the port. Setting maxconn=0 does at
          least prevent access by producing the following output when someone tries to
          connect to the port:

          $ telnet localhost 6666
          Trying 127.0.0.1...
          Connected to localhost.
          Escape character is '^]'.
          Connection refused.
          All possible connections are currently being used.
          Connection closed by foreign host.

          It would seem that Felix could be modified such that if maxconn=0 then it
          wouldn't create the Acceptor at all. If Felix can't be modified that way, then
          maybe the GlassFishActivator.startPostStartupBundles method could check the
          value of that property before starting the bundles.

          Show
          Tom Mueller added a comment - "Turn off" means to at least not listen on the port. Setting maxconn=0 does at least prevent access by producing the following output when someone tries to connect to the port: $ telnet localhost 6666 Trying 127.0.0.1... Connected to localhost. Escape character is '^]'. Connection refused. All possible connections are currently being used. Connection closed by foreign host. It would seem that Felix could be modified such that if maxconn=0 then it wouldn't create the Acceptor at all. If Felix can't be modified that way, then maybe the GlassFishActivator.startPostStartupBundles method could check the value of that property before starting the bundles.
          Hide
          Richard S. Hall added a comment -

          Just so we are using the correct terminology here, this is not Felix, it is the
          Remote Shell, which is a Felix subproject.

          Actually, I am working on a new release for Remote Shell to make it work with
          Gogo so we can use it instead of the old shell. So, I could potentially modify
          it to interpret a max connection count of less than 1 as being disabled.

          However, there is no point in doing this if we are still going to modify the
          startPostStartupBundles() method to avoid starting it if this value is 0. The
          only reason to modify the Remote Shell is if we wanted to avoid modifying our
          launcher or adding a new config property. In OSGi land, Remote Shell behaves in
          a reasonable way, since as I mentioned previously, you would just stop it if you
          wanted to disable it.

          Personally, having these post startup bundles hard coded doesn't seem like a
          good thing to me, so introducing a configuration property to control them seems
          like a good idea and then it solves this issue and solves it for any other
          bundles that we may or may not want to start post startup.

          Show
          Richard S. Hall added a comment - Just so we are using the correct terminology here, this is not Felix, it is the Remote Shell, which is a Felix subproject. Actually, I am working on a new release for Remote Shell to make it work with Gogo so we can use it instead of the old shell. So, I could potentially modify it to interpret a max connection count of less than 1 as being disabled. However, there is no point in doing this if we are still going to modify the startPostStartupBundles() method to avoid starting it if this value is 0. The only reason to modify the Remote Shell is if we wanted to avoid modifying our launcher or adding a new config property. In OSGi land, Remote Shell behaves in a reasonable way, since as I mentioned previously, you would just stop it if you wanted to disable it. Personally, having these post startup bundles hard coded doesn't seem like a good thing to me, so introducing a configuration property to control them seems like a good idea and then it solves this issue and solves it for any other bundles that we may or may not want to start post startup.
          Hide
          Nazrul added a comment -

          We need an option to turn off the Gogo shell. For example,
          <jvm-options>-Dosgi.shell=false</jvm-options>

          Refer to bug 7016890 for customer details.

          Show
          Nazrul added a comment - We need an option to turn off the Gogo shell. For example, <jvm-options>-Dosgi.shell=false</jvm-options> Refer to bug 7016890 for customer details.
          Hide
          Tom Mueller added a comment -

          As a work-around, if you do:

          rm glassfish3/glassfish/modules/shell.jar

          then when you start the domain, the shell doesn't start, although there are several INFO messages in the log about being unable to find shell bundles.

          Also, removing files will cause havoc with the packaging system that expects the files to be there.

          Show
          Tom Mueller added a comment - As a work-around, if you do: rm glassfish3/glassfish/modules/ shell .jar then when you start the domain, the shell doesn't start, although there are several INFO messages in the log about being unable to find shell bundles. Also, removing files will cause havoc with the packaging system that expects the files to be there.
          Hide
          Richard S. Hall added a comment -

          I believe the proper way to handle this is to create a config property to specify the bundles to be started in GlassFishActivator.startPostStartupBundles(). Then this property could just be edited to remove Gogo if you didn't want it.

          Show
          Richard S. Hall added a comment - I believe the proper way to handle this is to create a config property to specify the bundles to be started in GlassFishActivator.startPostStartupBundles(). Then this property could just be edited to remove Gogo if you didn't want it.
          Hide
          Sanjeeb Sahoo added a comment -

          Ideally, we should have a package dedicated for shell related bundles and then user could just uninstall the package instead of having to remove it manually. While we can consider this enhancement in 3.2, I think I can provide a simple solution along the lines of what Richard has said, i.e., to make the list of bundles configurable. If there is necessary approval to do this change, I can do this in 3.1 with little risk.

          Show
          Sanjeeb Sahoo added a comment - Ideally, we should have a package dedicated for shell related bundles and then user could just uninstall the package instead of having to remove it manually. While we can consider this enhancement in 3.2, I think I can provide a simple solution along the lines of what Richard has said, i.e., to make the list of bundles configurable. If there is necessary approval to do this change, I can do this in 3.1 with little risk.
          Hide
          Sanjeeb Sahoo added a comment - - edited

          I have a fix which seems not only reasonable but risk free as well to be applied in 3.1 itself. PFA the patch.

          A new system property called "org.glassfish.additionalBundlesToStart" has been introduced. The value is a list of bundle symbolic names. The default value in code and in domain.xml is
          org.apache.felix.shell, org.apache.felix.gogo.runtime, org.apache.felix.gogo.shell, org.apache.felix.gogo.command, org.apache.felix.shell.remote, org.apache.felix.fileinstall
          To disable remote shell, one has to just remove org.apache.felix.shell.remote,
          from the above list in domain.xml. We have a default value in code so that we can handle an upgraded domain which may not have this value.

          Can admin team review the change to domain.xml?

          Show
          Sanjeeb Sahoo added a comment - - edited I have a fix which seems not only reasonable but risk free as well to be applied in 3.1 itself. PFA the patch. A new system property called "org.glassfish.additionalBundlesToStart" has been introduced. The value is a list of bundle symbolic names. The default value in code and in domain.xml is org.apache.felix.shell, org.apache.felix.gogo.runtime, org.apache.felix.gogo.shell, org.apache.felix.gogo.command, org.apache.felix.shell.remote, org.apache.felix.fileinstall To disable remote shell, one has to just remove org.apache.felix.shell.remote, from the above list in domain.xml. We have a default value in code so that we can handle an upgraded domain which may not have this value. Can admin team review the change to domain.xml?
          Hide
          Sanjeeb Sahoo added a comment -

          Fixed in trunk:
          Sending core/kernel/src/main/java/org/glassfish/kernel/GlassFishActivator.java
          Sending packager/nucleus-base/lib/templates/domain.xml
          Transmitting file data ..
          Committed revision 44954.

          Will keep the bug open until a decision is made about 3.1

          Show
          Sanjeeb Sahoo added a comment - Fixed in trunk: Sending core/kernel/src/main/java/org/glassfish/kernel/GlassFishActivator.java Sending packager/nucleus-base/lib/templates/domain.xml Transmitting file data .. Committed revision 44954. Will keep the bug open until a decision is made about 3.1
          Hide
          Nazrul added a comment -

          In default-config (used by remote instance/cluster), please disable OSGi shell feature by default.

          Show
          Nazrul added a comment - In default-config (used by remote instance/cluster), please disable OSGi shell feature by default.
          Hide
          Tom Mueller added a comment -

          The changes in patch.txt LGTM.

          Show
          Tom Mueller added a comment - The changes in patch.txt LGTM.
          Hide
          Nazrul added a comment -

          We need to review the changes with updated default configuration where OSGi shell is disabled out-of-the-box for default-config.

          Show
          Nazrul added a comment - We need to review the changes with updated default configuration where OSGi shell is disabled out-of-the-box for default-config.
          Hide
          Chris Kasso added a comment -

          Approved for RC3 with the modification that the shell be disabled by default.

          Show
          Chris Kasso added a comment - Approved for RC3 with the modification that the shell be disabled by default.
          Hide
          Sanjeeb Sahoo added a comment -

          Turning off the shell in default-config used by cluster/remote instances :
          Sending packager/nucleus-base/lib/templates/domain.xml
          Transmitting file data .
          Committed revision 44977 to trunk.

          Show
          Sanjeeb Sahoo added a comment - Turning off the shell in default-config used by cluster/remote instances : Sending packager/nucleus-base/lib/templates/domain.xml Transmitting file data . Committed revision 44977 to trunk.
          Hide
          Nazrul added a comment - - edited

          We should turn off OSGi shell in both server-config and default-config by default.

          Show
          Nazrul added a comment - - edited We should turn off OSGi shell in both server-config and default-config by default.
          Hide
          Sanjeeb Sahoo added a comment -

          Having a shell in developer environment is very handy to understand what's going on. Since developers don't set up clusters, I had no reasons to object disabling it in clusters/remote instances. If we have a different domain.xml template for production setup or we change the installer such that it prompts user for enabling shell, I don't see any issues.

          Show
          Sanjeeb Sahoo added a comment - Having a shell in developer environment is very handy to understand what's going on. Since developers don't set up clusters, I had no reasons to object disabling it in clusters/remote instances. If we have a different domain.xml template for production setup or we change the installer such that it prompts user for enabling shell, I don't see any issues.
          Hide
          Nazrul added a comment -

          Please commit the changes for default-config. It will be great if you can join the Bug Swat meeting to discuss the server-config (DAS) changes.

          Show
          Nazrul added a comment - Please commit the changes for default-config. It will be great if you can join the Bug Swat meeting to discuss the server-config (DAS) changes.
          Hide
          Sanjeeb Sahoo added a comment -

          Committing to 3.1-branch:

          Sending core/kernel/src/main/java/org/glassfish/kernel/GlassFishActivator.java
          Sending packager/nucleus-base/lib/templates/domain.xml
          Transmitting file data ..
          Committed revision 45011.

          With this change, the remote shell is disabled in default-config, but enabled in server-config by default.

          There is a separate debate going on about whether the shell should be turned in server-config or not. No conclusion is reached as yet.

          Show
          Sanjeeb Sahoo added a comment - Committing to 3.1-branch: Sending core/kernel/src/main/java/org/glassfish/kernel/GlassFishActivator.java Sending packager/nucleus-base/lib/templates/domain.xml Transmitting file data .. Committed revision 45011. With this change, the remote shell is disabled in default-config, but enabled in server-config by default. There is a separate debate going on about whether the shell should be turned in server-config or not. No conclusion is reached as yet.
          Hide
          Tom Mueller added a comment -

          As an OSGi developer, I can see where the OSGi shell is very important to you. However, for the average Java EE application developer, who may not even know or care what OSGi is (and I expect that this is the majority of the GlassFish users), the OSGi shell is never needed, and they won't even know that they need to turn it off in a production environment. Also, there is only one domain.xml template, i.e., there isn't a "development build" and a "production build" that has different templates, so we only get one choice for the default.

          Show
          Tom Mueller added a comment - As an OSGi developer, I can see where the OSGi shell is very important to you. However, for the average Java EE application developer, who may not even know or care what OSGi is (and I expect that this is the majority of the GlassFish users), the OSGi shell is never needed, and they won't even know that they need to turn it off in a production environment. Also, there is only one domain.xml template, i.e., there isn't a "development build" and a "production build" that has different templates, so we only get one choice for the default.
          Hide
          Sanjeeb Sahoo added a comment -

          Isn't it a design mistake in GlassFish to have one configuration for developers as well as production users?

          Show
          Sanjeeb Sahoo added a comment - Isn't it a design mistake in GlassFish to have one configuration for developers as well as production users?
          Hide
          Tom Mueller added a comment -

          The OGS distribution includes a performance tuner tool that is intended to make a system ready for production use. In other situations where users have requested a more production-ready domain.xml, we have put those changes into performance tuner rather than providing a different domain.xml template. The reason for this is that there isn't any way to really produce a static domain.xml template that addresses every production scenario due to differing memory, CPU, etc. factors in the production environment.

          Maybe one solution to this would be to leave the OSGi console enabled for the DAS in the default domain.xml template, but then modify the performance tuner to disable it as part of what it does. Documentation about disabling it should then be included in the deployment guide also.

          Show
          Tom Mueller added a comment - The OGS distribution includes a performance tuner tool that is intended to make a system ready for production use. In other situations where users have requested a more production-ready domain.xml, we have put those changes into performance tuner rather than providing a different domain.xml template. The reason for this is that there isn't any way to really produce a static domain.xml template that addresses every production scenario due to differing memory, CPU, etc. factors in the production environment. Maybe one solution to this would be to leave the OSGi console enabled for the DAS in the default domain.xml template, but then modify the performance tuner to disable it as part of what it does. Documentation about disabling it should then be included in the deployment guide also.
          Hide
          Richard S. Hall added a comment -

          For me, I am more interested to have the shell enabled for GF developers, not for developers using GF. As a simple example, in Felix for our development builds we have the log level set to DEBUG, but the release process sets it to ERROR. That way the development release provides more debug info by default for us Felix developers than the released version does for OSGi developers. I'd like to treat the OSGi shell in GF similar to this if possible.

          Show
          Richard S. Hall added a comment - For me, I am more interested to have the shell enabled for GF developers, not for developers using GF. As a simple example, in Felix for our development builds we have the log level set to DEBUG, but the release process sets it to ERROR. That way the development release provides more debug info by default for us Felix developers than the released version does for OSGi developers. I'd like to treat the OSGi shell in GF similar to this if possible.
          Hide
          Nazrul added a comment -

          Based on email discussion, we should turn OFF the shell feature for production build due to security reason. Re-opening the issue so that we can make changes for DAS (server-config) also.

          During 3.2, we may make necessary build changes so that developer build will have the shell ON for server-config while the promoted build will have this turned OFF.

          Show
          Nazrul added a comment - Based on email discussion, we should turn OFF the shell feature for production build due to security reason. Re-opening the issue so that we can make changes for DAS (server-config) also. During 3.2, we may make necessary build changes so that developer build will have the shell ON for server-config while the promoted build will have this turned OFF.
          Hide
          Sanjeeb Sahoo added a comment -

          ss141213@Sahoo:/space/ss141213/WS/gf/v3.1-branch$ svn commit -m "GLASSFISH-13006 - Disable remote shell in das in fcs build. This change will not be forward ported to trunk. See JIRA." packager/nucleus-base/
          Sending packager/nucleus-base/lib/templates/domain.xml
          Transmitting file data .
          Committed revision 45043.

          Show
          Sanjeeb Sahoo added a comment - ss141213@Sahoo:/space/ss141213/WS/gf/v3.1-branch$ svn commit -m " GLASSFISH-13006 - Disable remote shell in das in fcs build. This change will not be forward ported to trunk. See JIRA." packager/nucleus-base/ Sending packager/nucleus-base/lib/templates/domain.xml Transmitting file data . Committed revision 45043.
          Hide
          Nazrul added a comment -

          Re-opening the issue since the problem exists in 3.1.1 and 3.2 builds. We need a permanent solution for this given we have a security risk with this.

          Show
          Nazrul added a comment - Re-opening the issue since the problem exists in 3.1.1 and 3.2 builds. We need a permanent solution for this given we have a security risk with this.
          Hide
          Sanjeeb Sahoo added a comment -

          svn commit -m "GLASSFISH-13006: Turn off remote shell in 3.1.1. Trunk will continue to have remote shell enabled until we reach RC candidates." packager/nucleus-base/lib/
          Sending packager/nucleus-base/lib/templates/domain.xml
          Transmitting file data .
          Committed revision 46935.

          Show
          Sanjeeb Sahoo added a comment - svn commit -m " GLASSFISH-13006 : Turn off remote shell in 3.1.1. Trunk will continue to have remote shell enabled until we reach RC candidates." packager/nucleus-base/lib/ Sending packager/nucleus-base/lib/templates/domain.xml Transmitting file data . Committed revision 46935.
          Hide
          scatari added a comment -

          Marking it as fixed as the fix was delivered for b06.

          Show
          scatari added a comment - Marking it as fixed as the fix was delivered for b06.
          Hide
          Sanjeeb Sahoo added a comment -

          This issue is kept open until it is fixed in trunk. We are looking for a better solution in trunk, hence the delay in closing the issue in trunk. Pl. don't mark it resolved for trunk. I ave marked it as 3_1_1-scrubbed, so hopefully it won't show up in 3.1.1 outstanding buglist.

          Show
          Sanjeeb Sahoo added a comment - This issue is kept open until it is fixed in trunk. We are looking for a better solution in trunk, hence the delay in closing the issue in trunk. Pl. don't mark it resolved for trunk. I ave marked it as 3_1_1-scrubbed, so hopefully it won't show up in 3.1.1 outstanding buglist.
          Hide
          Sanjeeb Sahoo added a comment -

          This bug has been fixed in 3.1.1 as well. We are marking this bug resolved and opening a new one per 3.1.1 release manager's request. Pl. contact 3.1.1 release manager for further details about the process.

          Show
          Sanjeeb Sahoo added a comment - This bug has been fixed in 3.1.1 as well. We are marking this bug resolved and opening a new one per 3.1.1 release manager's request. Pl. contact 3.1.1 release manager for further details about the process.
          Hide
          Sanjeeb Sahoo added a comment -

          Linking with the 3.2 specific issue.

          Show
          Sanjeeb Sahoo added a comment - Linking with the 3.2 specific issue.
          Hide
          Sanjeeb Sahoo added a comment -

          I have now disabled the shell on trunk as well, since I have not found time to secure it. Will downgrade the priority of GLASSFISH-16998 now.
          Sending conf/config.properties
          Transmitting file data .
          Committed revision 48678.

          Show
          Sanjeeb Sahoo added a comment - I have now disabled the shell on trunk as well, since I have not found time to secure it. Will downgrade the priority of GLASSFISH-16998 now. Sending conf/config.properties Transmitting file data . Committed revision 48678.

            People

            • Assignee:
              Sanjeeb Sahoo
              Reporter:
              Tom Mueller
            • Votes:
              0 Vote for this issue
              Watchers:
              1 Start watching this issue

              Dates

              • Due:
                Created:
                Updated:
                Resolved: