[jira] [Commented] (GERONIMO-6174) Environment variables being set in bin/geronimo does not account for named server instances

["Russell E Glaue (Commented) (JIRA)" <ji...@apache.org>]

    [ https://issues.apache.org/jira/browse/GERONIMO-6174?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13116601#comment-13116601 ] 

Russell E Glaue commented on GERONIMO-6174:
-------------------------------------------

The temp directory issue is explained in this source:
trunk/framework/modules/geronimo-system/src/main/java/org/apache/geronimo/system/serverinfo/BasicServerInfo.java
{noformat}
    public BasicServerInfo(@ParamAttribute(name = "baseDirectory")String defaultBaseDirectory,
                           @ParamAttribute(name="useSystemProperties") boolean useSystemProperties,
                           @ParamSpecial(type = SpecialAttributeType.bundleContext) BundleContext bundleContext) throws Exception {
        // Before we try the persistent value, we always check the
        // system properties first.  This lets an admin override this
        // on the command line.
        this.baseDirectory = useSystemProperties? System.getProperty(HOME_DIR_SYS_PROP, defaultBaseDirectory): defaultBaseDirectory;

        // force load of server constants
        ServerConstants.getVersion();

        if (baseDirectory == null || baseDirectory.length() == 0) {
            String karafHome = System.getProperty("karaf.home");
            if (karafHome == null) {
                throw new IllegalStateException("NO karaf.home specified");
            }
            this.base = new File(karafHome);
        } else {
            base = new File(baseDirectory);
        }

	if (!base.isDirectory()) {
            throw new IllegalArgumentException("Base directory is not a directory: " + baseDirectory);
        }

	baseURI = base.toURI();
        baseServer = deriveBaseServer(useSystemProperties);
        baseServerURI = baseServer.toURI();
        if (useSystemProperties) {
            System.setProperty(HOME_DIR_SYS_PROP, base.getAbsolutePath());
            System.setProperty(SERVER_DIR_SYS_PROP, baseServer.getAbsolutePath());
        }
	String tmpDir = resolveServerPath(System.getProperty("java.io.tmpdir"));
        System.setProperty("java.io.tmpdir", tmpDir);

        logEnvInfo();
    }
{noformat}

In the source,
1. `BasicServerInfo.base` is set to `System.getProperty("org.apache.geronimo.home.dir")`
2. The code then sets `org.apache.geronimo.server.dir` to one of the following:
2a. `org.apache.geronimo.server.name` ? `BasicServerInfo.base` + `org.apache.geronimo.server.name` : `BasicServerInfo.base`
2b. `org.apache.geronimo.server.dir` ? isAbsolute(`org.apache.geronimo.server.dir`) ? `org.apache.geronimo.server.dir` :`BasicServerInfo.base` + `org.apache.geronimo.server.dir` : `BasicServerInfo.base`

This means that `org.apache.geronimo.server.dir` will be the absolute path to the Named Server Instance OR GERONIMO_HOME
And also means `org.apache.geronimo.home.dir` will ALWAYS be GERONIMO_HOME

So when Geronimo internals point to read-write structures on disk, they should be using `org.apache.geronimo.server.dir`.


So for the issues reported.
1) We have stated that the temp directory is always to be `java.io.tmpdir` for security reasons. BasicServerInfo does nothing to attempt a construct of this path. As such the code is not incorrect.
Resolution for issue #1: The GERONIMO_TMPDIR must be set in the startup environment, which means we need to modify the startup scripts to recognize a named server instance. Else we must not set GERONIMO_TMPDIR, and then change BasicServerInfo to construct the temp directory, which I think is undesirable for the stated security issues.

2a) The `karaf.home` property should be defined from the `org.apache.geronimo.home.dir` property. So using it in BasicServerInfo to set the value of `org.apache.geronimo.home.dir`, if `org.apache.geronimo.home.dir` is not defined, seems odd.

2b) Later on, "framework/modules/geronimo-main/src/main/java/org/apache/geronimo/main/FrameworkLauncher.java" sets `karaf.base` to geronimoBase. However, "geronimo/server/trunk/framework/configs/karaf-framework/src/main/distribution/text/etc/system.properties" uses `karaf.home` for locating the {etc/shell.init.script}
The `karaf.home` and `karaf.base` are forcefully set in "FrameworkLauncher", so they cannot be set in the startup environment.
This would tell me that "geronimo/server/trunk/framework/configs/karaf-framework/src/main/distribution/text/etc/system.properties" needs to be corrected to use `karaf.base` for locating {etc/shell.init.script} and not `karaf.home`.

                
> Environment variables being set in bin/geronimo does not account for named server instances
> -------------------------------------------------------------------------------------------
>
>                 Key: GERONIMO-6174
>                 URL: https://issues.apache.org/jira/browse/GERONIMO-6174
>             Project: Geronimo
>          Issue Type: Bug
>      Security Level: public(Regular issues) 
>          Components: startup/shutdown
>    Affects Versions: 3.0
>         Environment: Linux x86, Red Hat Enterprise Linux Server release 5.4 (Tikanga)
>            Reporter: Russell E Glaue
>            Priority: Trivial
>              Labels: geronimo
>
> I have been able to workaround these two issues by setting properties.
> Questions:
> 1A) But should these properties be dynamically set relative to the named instance directory instead of GERONIMO_HOME?
> 1B) Or do we require the user to explicitly set these in the environment in order to start a named instance?
> 2) Does setting karaf.home in GERONIMO_OPTS as relative to the named instance directory instead of GERONIMO_HOME potentially cause any other issues?
> To produce these issues follow these steps:
> 1. Unpack the latest Geronimo javaee6 bundle as /opt/geronimo3
> 2. Create a Geronimo named instance directory as /opt/geronimo3/gserv1
> 3. Move the directories var, etc, and repository into /opt/geronimo3/gserv1
> 4. Use this start script
> -
> #!/bin/bash
> GHOME=/opt/geronimo3
> GVIRT=gserv1
> # Must change to the server directory in order to work around ActiveMQ lock
> # file conflict issue reported in GERONIMO-5987.
> cd ${GHOME}/${GVIRT}
> # Uncomment for the Workaround of issue #1
> #export GERONIMO_TMPDIR=${GHOME}/${GVIRT}/var/temp
> #
> # Uncomment for the Workaround of issue #2
> #GERONIMO_OPTS="${GERONIMO_OPTS} -Dkaraf.home=${GHOME}/${GVIRT}"
> #
> GERONIMO_OPTS="${GERONIMO_OPTS} -Dorg.apache.geronimo.server.name=${GVIRT}"
> export GERONIMO_OPTS
> ${GHOME}/bin/geronimo run
> -
> 1) GERONIMO_TMPDIR
> On startup, the `bin/geronimo` startup script sets GERONIMO_TMPDIR explicitly as
> $GERONIMO_HOME/var/temp , but the actual temp directory is really
> {org.apache.geronimo.server.dir}/var/temp (or
> $GERONIMO_HOME/{org.apache.geronimo.server.name}/var/temp)
> It does not account for the cases where an instance is being started.
> -
> [user@system gserv1]# ./start.sh 
> Using GERONIMO_HOME:   /opt/geronimo3
> Using GERONIMO_TMPDIR: /opt/geronimo3/var/temp
> Using JRE_HOME:        /usr/jdk1.6.0/jre
> Error launching framework: java.lang.IllegalArgumentException: Invalid temporary directory. The '/opt/geronimo3/var/temp' path does not exist.
> -
> The workaround is to specifically specify GERONIMO_TMPDIR in your environment
> before starting the instance.
> 2) karaf.home
> On startup, the `bin/geronimo` script sets karaf.home explicitly as
> $GERONIMO_HOME . Karaf expects to use {karaf.home}/etc/shell.init.script each
> time a shell session is started (See:
> geronimo/server/trunk/framework/configs/karaf-framework/src/main/distribution/text/etc/system.properties).
> The setting of the karaf.home property in `bin/geronimo` does not account for
> the cases where a Geronimo named instance is being started.
> -
> [root@rglaue7 gserv1]# ./start.sh 
> Using GERONIMO_HOME:   /opt/geronimo3
> Using GERONIMO_TMPDIR: /opt/geronimo3/gserv1/var/temp
> Using JRE_HOME:        /usr/jdk1.6.0/jre
>                                                           
>      ______                       _                       
>     / ____/___  _________  ____  (_)____ ___  ____        
>    / / __ / _ \/ ___/ __ \/ __ \/ // __ `__ \/ __ \  
>   / /_/ //  __/ /  / /_/ / / / / // / / / / / /_/ /       
>   \____/ \___/_/   \____/_/ /_/_//_/ /_/ /_/\____/    
>   Apache Geronimo (3.0-SNAPSHOT)
> Hit '<tab>' for a list of available commands
> and '[cmd] --help' for help on a specific command.
> Hit '<ctrl-d>' or 'osgi:shutdown' to shutdown Geronimo.
> Error in initialization script: /opt/geronimo3/etc/shell.init.script (No such file or directory)
> ... snip ...
> -
> The workaround is to set karaf.home in GERONIMO_OPTS before starting the instance.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira