Geronimo
  1. Geronimo
  2. GERONIMO-4896

Commands to a Secure JMX Connector require the SSL keyStorePassword to be specified on command line

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 2.1.5, 2.2, 3.0.0
    • Fix Version/s: 2.1.5, 2.2.1, 3.0.0, Wish List
    • Component/s: None
    • Security Level: public (Regular issues)
    • Labels:
      None

      Description

      To my knowledge, it is not possible to run a Geronimo command (e.g. deploy.sh deploy or gsh geronimo/stop-server) to a server with a secure JMX Connector (running SSL, without specifying the following Java system properties on the command line:
      javax.net.ssl.keyStore and javax.net.ssl.keyStorePassword

      For example:

      export GERONIMO_HOME=~/target/geronimo-jetty6-javaee5-2.2-SNAPSHOT
      export JAVA_OPTS="-Djavax.net.ssl.keyStore=$GERONIMO_HOME/var/security/keystores/geronimo-default -Djavax.net.ssl.keyStorePassword=secret -Djavax.net.ssl.trustStore=$GERONIMO_HOME/var/security/keystores/geronimo-default -Djavax.net.ssl.trustStorePassword=secret"
      $GERONIMO_HOME/bin/deploy.sh -u system -p manager --secure list-modules --stopped
      

      javax.net.ssl.keyStorePassword causes a problem, since this means the keyStorePassword is available, in-the-clear, to someone inspecting executing processes. For example while a deploy command was active, someone could run 'ps auxww | grep deployer.jar' and discover the keyStorePassword for the KeyStore.

      Geronimo should provide a mechanism, whereby users can specify the keyStorePassword without making that secret available to anyone inspecting processes running on the current system. Ideally, the password could be encrypted/obfuscated within a file (just as passwords can be encrypted/obfuscated in var/config/config.xml).

      1. 4896_21.patch
        18 kB
        Ashish Jain
      2. 4896_updated_21.patch
        7 kB
        Ashish Jain
      3. 4896_updated.patch
        4 kB
        Ashish Jain
      4. 4896.patch
        2 kB
        Ashish Jain
      5. JavaAgent.jar
        2 kB
        Ashish Jain
      6. JvmOpts.java
        0.8 kB
        Ashish Jain

        Activity

        Hide
        Rex Wang added a comment -

        closing it

        Show
        Rex Wang added a comment - closing it
        Hide
        Ashish Jain added a comment -

        Thanks a lot Shawn for your continuous suggestions on code style, api's used and the coding standards.

        Show
        Ashish Jain added a comment - Thanks a lot Shawn for your continuous suggestions on code style, api's used and the coding standards.
        Hide
        Shawn Jiang added a comment -

        The new patch looks good, All the patches have been applied to 21, 22, trunk.

        thanks Ashish.

        Show
        Shawn Jiang added a comment - The new patch looks good, All the patches have been applied to 21, 22, trunk. thanks Ashish.
        Hide
        Ashish Jain added a comment -

        Hi Shawn,

        Since the patch has been modified so that it no longer throws Null pointer exception therefore I do not see where I can add the new message.
        I have also modified my environment so that indentation are proper. kindly verify and suggest if it needs further improvements.

        Thanks
        Ashish

        Show
        Ashish Jain added a comment - Hi Shawn, Since the patch has been modified so that it no longer throws Null pointer exception therefore I do not see where I can add the new message. I have also modified my environment so that indentation are proper. kindly verify and suggest if it needs further improvements. Thanks Ashish
        Hide
        Shawn Jiang added a comment -

        1, I can't find any code related to this in the patch.

        the message should be more user friendly. I will modify the message and upload a new fix for this. "

        2, please conform the coding standards[1], especially the Indentation in your patches. You could import the code format rules if you are using Eclipse as the dev tool according to [2]

        [1]http://cwiki.apache.org/GMOxDEV/coding-standards.html
        [2]http://cwiki.apache.org/GMOxDEV/developing-geronimo-in-eclipse.html

        Show
        Shawn Jiang added a comment - 1, I can't find any code related to this in the patch. the message should be more user friendly. I will modify the message and upload a new fix for this. " 2, please conform the coding standards [1] , especially the Indentation in your patches. You could import the code format rules if you are using Eclipse as the dev tool according to [2] [1] http://cwiki.apache.org/GMOxDEV/coding-standards.html [2] http://cwiki.apache.org/GMOxDEV/developing-geronimo-in-eclipse.html
        Hide
        Ashish Jain added a comment -

        Uploading a new patch for g21 after incorporating the comments made earlier.The name of the file is 4896_21.patch. This modified version
        also sets the default location of keyStore and trustStore password,

        Kindly review and apply.

        Thanks
        Ashish

        Show
        Ashish Jain added a comment - Uploading a new patch for g21 after incorporating the comments made earlier.The name of the file is 4896_21.patch. This modified version also sets the default location of keyStore and trustStore password, Kindly review and apply. Thanks Ashish
        Hide
        Ashish Jain added a comment -

        Hi Shawn,

        Thanks for your comments. I will modify the patch as suggested by you. On the default KEYSTORE_TRUSTSTORE_PASSWORD_FILE GERONIMO-5146 takes care of that. I will
        further make one more addition through which the default password are picked automatically by the server w/o any need for user to specify any parameter. User will only have to
        specify the system properties if he is using custom keystore, truststore.

        Thanks
        Ashish

        Show
        Ashish Jain added a comment - Hi Shawn, Thanks for your comments. I will modify the patch as suggested by you. On the default KEYSTORE_TRUSTSTORE_PASSWORD_FILE GERONIMO-5146 takes care of that. I will further make one more addition through which the default password are picked automatically by the server w/o any need for user to specify any parameter. User will only have to specify the system properties if he is using custom keystore, truststore. Thanks Ashish
        Hide
        Shawn Jiang added a comment -

        It would be appreciated if you can

        1, create patch based on current latest code.
        2, use 4 space instead of tab as the indent in the patch.
        3, provide both 21, 22, trunk patches.

        Thanks.

        Show
        Shawn Jiang added a comment - It would be appreciated if you can 1, create patch based on current latest code. 2, use 4 space instead of tab as the indent in the patch. 3, provide both 21, 22, trunk patches. Thanks.
        Hide
        Shawn Jiang added a comment -

        Ashish,

        I committed the patch to 21,22,trunk. Since you are working to improve the message. You might also want to replace current BufferedReader based parser with Properties like following.

        Properties props = new Properties();
        FileInputStream in = new FileInputStream(System.getProperty(KEYSTORE_TRUSTSTORE_PASSWORD_FILE));
        props .load(in);
        in.close();

        //read username , password from the props.

        ....

        I'm also wondering if we should provide a default KEYSTORE_TRUSTSTORE_PASSWORD_FILE under geronimo_home/var/security so that the users could know what's the format they should follow to compose the file.

        Show
        Shawn Jiang added a comment - Ashish, I committed the patch to 21,22,trunk. Since you are working to improve the message. You might also want to replace current BufferedReader based parser with Properties like following. Properties props = new Properties(); FileInputStream in = new FileInputStream(System.getProperty(KEYSTORE_TRUSTSTORE_PASSWORD_FILE)); props .load(in); in.close(); //read username , password from the props. .... I'm also wondering if we should provide a default KEYSTORE_TRUSTSTORE_PASSWORD_FILE under geronimo_home/var/security so that the users could know what's the format they should follow to compose the file.
        Hide
        Ashish Jain added a comment -

        I think you are correct the message should be more user friendly. I will modify the message and upload a new fix for this. Thanks a lot for your comment .

        Show
        Ashish Jain added a comment - I think you are correct the message should be more user friendly. I will modify the message and upload a new fix for this. Thanks a lot for your comment .
        Hide
        Chi Runhua added a comment -

        User deserves better description or instructions instead of an exception when the original command line doesn't work properly on the new release.

        I'd like to see description like the following aside with the exception:

        The usage of javax.net.ssl.keyStorePassword and javax.net.ssl.trustStorePassword in a command line has been deprecated. 
        Use org.apache.geronimo.keyStoreTrustStorePasswordFile property to specify the location of passwords of the trustStore and keystore. 
        

        Any comments?

        Show
        Chi Runhua added a comment - User deserves better description or instructions instead of an exception when the original command line doesn't work properly on the new release. I'd like to see description like the following aside with the exception: The usage of javax.net.ssl.keyStorePassword and javax.net.ssl.trustStorePassword in a command line has been deprecated. Use org.apache.geronimo.keyStoreTrustStorePasswordFile property to specify the location of passwords of the trustStore and keystore. Any comments?
        Hide
        Ashish Jain added a comment -

        I think there is still some tweaking required to the patch. The patch uploaded does not allow the users to use the conventional way of setting the password and locations in plain text.
        Do we want this option be available to the users which is as follows.
        export JAVA_OPTS="-Djavax.net.ssl.keyStore=$GERONIMO_HOME/var/security/keystores/geronimo-default -Djavax.net.ssl.keyStorePassword=secret -Djavax.net.ssl.trustStore=$GERONIMO_HOME/var/security/keystores/geronimo-default -Djavax.net.ssl.trustStorePassword=secret"
        or else we want to completely disable the usage of plain text passwords??

        Show
        Ashish Jain added a comment - I think there is still some tweaking required to the patch. The patch uploaded does not allow the users to use the conventional way of setting the password and locations in plain text. Do we want this option be available to the users which is as follows. export JAVA_OPTS="-Djavax.net.ssl.keyStore=$GERONIMO_HOME/var/security/keystores/geronimo-default -Djavax.net.ssl.keyStorePassword=secret -Djavax.net.ssl.trustStore=$GERONIMO_HOME/var/security/keystores/geronimo-default -Djavax.net.ssl.trustStorePassword=secret" or else we want to completely disable the usage of plain text passwords??
        Hide
        Ashish Jain added a comment -

        The previous patch was only for deployer.The new patch 4896_updated_21.patch takes care of shutdown as well. Please verify thanks.

        Show
        Ashish Jain added a comment - The previous patch was only for deployer.The new patch 4896_updated_21.patch takes care of shutdown as well. Please verify thanks.
        Hide
        Ashish Jain added a comment -

        Uploading a new patch following has been incorporated

        1) Read encrypted passwords from a file.
        2) If using default keystore and truststore than no need to specify location of keystore or truststore.
        3) In case a custom location than you need to specify the correct location through JAVA_OPTS.
        4) Specify the file location of the password file using the following system property
        org.apache.geronimo.keyStoreTrustStorePasswordFile. This will make geronoimo find the file
        for password.

        The deploy tool encrypted utility "deploy.bat encrypt" has been used to encrypt password. Refer GERONIMO-3003.

        Show
        Ashish Jain added a comment - Uploading a new patch following has been incorporated 1) Read encrypted passwords from a file. 2) If using default keystore and truststore than no need to specify location of keystore or truststore. 3) In case a custom location than you need to specify the correct location through JAVA_OPTS. 4) Specify the file location of the password file using the following system property org.apache.geronimo.keyStoreTrustStorePasswordFile. This will make geronoimo find the file for password. The deploy tool encrypted utility "deploy.bat encrypt" has been used to encrypt password. Refer GERONIMO-3003 .
        Hide
        Kevan Miller added a comment -

        I like this a lot better than a java agent... A few things:

        1) Thoughts on defaulting JAVA_OPTS to the default $GERONIMO_HOME/var/security locations? Only requiring JAVA_OPTS if you have a non-standard configuration.
        2) Manually inputing the passwords may be an issue for some environments. Perhaps allow optional properties to specify files w/ passwords? e.g. -Dorg.apache.geronimo.keyStorePasswordFile=<user-defined-location>, etc

        Show
        Kevan Miller added a comment - I like this a lot better than a java agent... A few things: 1) Thoughts on defaulting JAVA_OPTS to the default $GERONIMO_HOME/var/security locations? Only requiring JAVA_OPTS if you have a non-standard configuration. 2) Manually inputing the passwords may be an issue for some environments. Perhaps allow optional properties to specify files w/ passwords? e.g. -Dorg.apache.geronimo.keyStorePasswordFile=<user-defined-location>, etc
        Hide
        Ashish Jain added a comment -

        Uploading a patch.
        Using this approach you only need to specify keystore location and truststore location. something as follows

        set
        JAVA_OPTS=-Djavax.net.ssl.keyStore=%GERONIMO_HOME%/var/security/keystore
        s/geronimo-default
        -Djavax.net.ssl.trustStore=%GERONIMO_HOME%/var/security/keystores/geroni
        mo-default

        Once done run a test as follows
        deploy.bat --secure list-modules

        you will be prompted for truststore password and keystore password.

        Show
        Ashish Jain added a comment - Uploading a patch. Using this approach you only need to specify keystore location and truststore location. something as follows set JAVA_OPTS=-Djavax.net.ssl.keyStore=%GERONIMO_HOME%/var/security/keystore s/geronimo-default -Djavax.net.ssl.trustStore=%GERONIMO_HOME%/var/security/keystores/geroni mo-default Once done run a test as follows deploy.bat --secure list-modules you will be prompted for truststore password and keystore password.
        Hide
        Ashish Jain added a comment -

        Can someone help review and provide comments over the patch?

        Show
        Ashish Jain added a comment - Can someone help review and provide comments over the patch?
        Hide
        Ashish Jain added a comment -

        Uploading a jar file JavaAgent.jar. Add this to lib directory of geronimo installation. Modify the deploy.bat as follows
        %_RUNJAVA% %JAVA_OPTS% %GERONIMO_OPTS% -Dorg.apache.geronimo.home.dir="%GERONIMO_HOME%" -Djava.io.tmpdir="%GERONIMO_TMPDIR%" -javaagent:<LOCATION_OF_JAVA_AGENT_JAR> -jar %_JARFILE% %CMD_LINE_ARGS%

        Here LOCATION_OF_JAVA_AGENT_JAR= Exact location of JavaAgent.jar.

        Show
        Ashish Jain added a comment - Uploading a jar file JavaAgent.jar. Add this to lib directory of geronimo installation. Modify the deploy.bat as follows %_RUNJAVA% %JAVA_OPTS% %GERONIMO_OPTS% -Dorg.apache.geronimo.home.dir="%GERONIMO_HOME%" -Djava.io.tmpdir="%GERONIMO_TMPDIR%" -javaagent:<LOCATION_OF_JAVA_AGENT_JAR> -jar %_JARFILE% %CMD_LINE_ARGS% Here LOCATION_OF_JAVA_AGENT_JAR= Exact location of JavaAgent.jar.
        Hide
        Ashish Jain added a comment -

        A hello world implementation of java agent.

        Show
        Ashish Jain added a comment - A hello world implementation of java agent.
        Hide
        Ashish Jain added a comment -

        This can be achieved by creating a new java agent which will take care of setting the properties before the main class for the deployer is invoked.
        The implementation can be provided in anyway as suggested by the community. As of now I have written a hello world implementation where I
        have hardcoded all the values. Please advice if this seems to be a viable option and what implementation is acceptable??

        Just uploading the agent class for reference.

        Show
        Ashish Jain added a comment - This can be achieved by creating a new java agent which will take care of setting the properties before the main class for the deployer is invoked. The implementation can be provided in anyway as suggested by the community. As of now I have written a hello world implementation where I have hardcoded all the values. Please advice if this seems to be a viable option and what implementation is acceptable?? Just uploading the agent class for reference.
        Hide
        David Jencks added a comment -

        Not getting into 2.2.

        Show
        David Jencks added a comment - Not getting into 2.2.

          People

          • Assignee:
            Shawn Jiang
            Reporter:
            Kevan Miller
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development