Issue Details (XML | Word | Printable)

Key: GLASSFISH-13006
Type: Bug Bug
Status: Resolved Resolved
Resolution: Fixed
Priority: Blocker Blocker
Assignee: Sanjeeb Sahoo
Reporter: Tom Mueller
Votes: 0
Watchers: 1
Operations

If you were logged in you would be able to see more operations.
glassfish

Provide way to turn off OSGi shell

Created: 17/Aug/10 09:55 AM   Updated: 02/Dec/11 07:24 PM  Due: 08/Feb/11   Resolved: 08/Jul/11 07:03 AM
Component/s: OSGi
Affects Version/s: 3.1, 3.1.1, 4.0
Fix Version/s: 3.1_b42, 3.1.1_b06

Time Tracking:
Not Specified

File Attachments: 1. Text File patch.txt (4 kB) 06/Feb/11 11:19 AM - Sanjeeb Sahoo

Environment:

Operating System: All
Platform: All

Issue Links:
Related
 

Issuezilla Id: 13,006
Tags: 3_1-approved 3_1_1-scrubbed
Participants: Chris Kasso, Nazrul, Richard S. Hall, Sanjeeb Sahoo, scatari and Tom Mueller


 Description  « Hide

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.



Richard S. Hall added a comment - 17/Aug/10 10:18 AM

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.


Tom Mueller added a comment - 17/Aug/10 11:53 AM

"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.


Richard S. Hall added a comment - 17/Aug/10 12:18 PM

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.


Nazrul added a comment - 03/Feb/11 02:25 PM

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.


Tom Mueller added a comment - 03/Feb/11 02:31 PM

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.


Richard S. Hall added a comment - 03/Feb/11 02:53 PM

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.


Sanjeeb Sahoo added a comment - 03/Feb/11 07:18 PM

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.


Sanjeeb Sahoo added a comment - 06/Feb/11 11:19 AM - 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?


Sanjeeb Sahoo added a comment - 06/Feb/11 06:37 PM

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


Nazrul added a comment - 06/Feb/11 10:13 PM

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


Tom Mueller added a comment - 07/Feb/11 07:24 AM

The changes in patch.txt LGTM.


Nazrul added a comment - 07/Feb/11 11:11 AM

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


Chris Kasso added a comment - 07/Feb/11 02:43 PM

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


Sanjeeb Sahoo added a comment - 07/Feb/11 05:33 PM

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.


Nazrul added a comment - 08/Feb/11 11:35 AM - edited

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


Sanjeeb Sahoo added a comment - 08/Feb/11 08:34 PM

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.


Nazrul added a comment - 09/Feb/11 12:04 AM

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.


Sanjeeb Sahoo added a comment - 09/Feb/11 12:54 AM

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.


Tom Mueller added a comment - 09/Feb/11 07:40 AM

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.


Sanjeeb Sahoo added a comment - 09/Feb/11 07:45 AM

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


Tom Mueller added a comment - 09/Feb/11 07:54 AM

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.


Richard S. Hall added a comment - 09/Feb/11 07:59 AM

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.


Nazrul added a comment - 09/Feb/11 06:47 PM

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.


Sanjeeb Sahoo added a comment - 10/Feb/11 11:30 AM

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.


Nazrul added a comment - 18/May/11 12:43 PM

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.


Sanjeeb Sahoo added a comment - 18/May/11 09:33 PM

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.


scatari added a comment - 20/Jun/11 01:16 PM

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


Sanjeeb Sahoo added a comment - 20/Jun/11 01:34 PM

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.


Sanjeeb Sahoo added a comment - 08/Jul/11 07:03 AM

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.


Sanjeeb Sahoo added a comment - 08/Jul/11 07:04 AM

Linking with the 3.2 specific issue.


Sanjeeb Sahoo added a comment - 10/Aug/11 05:46 AM

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.