Bug 1221896 - tomcat.service loads /etc/sysconfig/tomcat without shell expansion
Summary: tomcat.service loads /etc/sysconfig/tomcat without shell expansion
Status: CLOSED ERRATA
Alias: None
Product: Red Hat Enterprise Linux 7
Classification: Red Hat
Component: tomcat   
(Show other bugs)
Version: 7.1
Hardware: x86_64
OS: Linux
medium
medium
Target Milestone: rc
: ---
Assignee: Coty Sutherland
QA Contact: fgoldefu
Lucie Maňásková
URL:
Whiteboard:
Keywords:
Depends On:
Blocks: 1203710 1298191 1313485 1293636
TreeView+ depends on / blocked
 
Reported: 2015-05-15 07:46 UTC by Anthony Symons
Modified: 2017-05-24 02:50 UTC (History)
13 users (show)

Fixed In Version:
Doc Type: Release Note
Doc Text:
Tomcat can now use shell expansion in configuration files within the new `conf.d` directory Previously, the `/etc/sysconfig/tomcat` and `/etc/tomcat/tomcat.conf` files were loaded without shell expansion, causing the application to terminate unexpectedly. This update provides a mechanism for using shell expansion in the Tomcat configuration files by adding a new configuration directory, `/etc/tomcat/conf.d`. Any files placed in the new directory may now include shell variables.
Story Points: ---
Clone Of:
: 1293636 (view as bug list)
Environment:
Last Closed: 2016-11-03 21:08:39 UTC
Type: Bug
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---


Attachments (Terms of Use)
proposed patch (5.79 KB, text/plain)
2016-02-03 20:15 UTC, Coty Sutherland
no flags Details


External Trackers
Tracker ID Priority Status Summary Last Updated
Red Hat Product Errata RHSA-2016:2599 normal SHIPPED_LIVE Moderate: tomcat security, bug fix, and enhancement update 2016-11-03 12:12:12 UTC

Description Anthony Symons 2015-05-15 07:46:24 UTC
Description of problem:
Breaking changes have been made to the startup architecture of tomcat 7 between the tomcat-7.0.42-8.el7_0.noarch and tomcat-7.0.54-1.el7.noarch versions.

Version-Release number of selected component (if applicable):
tomcat-7.0.54-1.el7.noarch

How reproducible:
Every time

Steps to Reproduce:
1. Add the following config line to /etc/sysconfig/tomcat:

JAVA_OPTS="$JAVA_OPTS -Duser.language=en -Dfile.encoding=UTF-8 -Xms128M -Xmx512M -XX:PermSize=64M -XX:MaxPermSize=512M -Djavax.xml.transform.TransformerFactory=com.sun.org.apache.xalan.internal.xsltc.trax.TransformerFactoryImpl"
JAVA_OPTS="-Duser.language=en -Dfile.encoding=UTF-8 -Xms128M -Xmx512M -XX:PermSize=64M -XX:MaxPermSize=512M -Djavax.xml.transform.TransformerFactory=com.sun.org.apache.xalan.internal.xsltc.trax.TransformerFactoryImpl"

2. Try to start tomcat.
3. Observe the message logs:


Actual results:

May 13 11:41:27 brilliant server: arguments used: start
May 13 11:41:27 brilliant server: Error: Could not find or load main class $JAVA_OPTS
May 13 11:41:27 brilliant systemd: tomcat.service: main process exited, code=exited, status=1/FAILURE
May 13 11:41:27 brilliant server: Java virtual machine used: /usr/lib/jvm/jre/bin/java
May 13 11:41:27 brilliant server: classpath used: /usr/share/tomcat/bin/bootstrap.jar:/usr/share/tomcat/bin/tomcat-juli.jar:/usr/share/java/commons-d
aemon.jar
May 13 11:41:27 brilliant server: main class used: org.apache.catalina.startup.Bootstrap
May 13 11:41:27 brilliant server: flags used: $JAVA_OPTS -Duser.language=en -Dfile.encoding=UTF-8 -Xms128M -Xmx512M -XX:PermSize=64M -XX:MaxPermSize=
512M -Djavax.xml.transform.TransformerFactory=com.sun.org.apache.xalan.internal.xsltc.trax.TransformerFactoryImpl -Duser.language=en -Dfile.encoding=
UTF-8 -Xms128M -Xmx512M -XX:PermSize=64M -XX:MaxPermSize=512M -Djavax.xml.transform.TransformerFactory=com.sun.org.apache.xalan.internal.xsltc.trax.T
ransformerFactoryImpl
May 13 11:41:27 brilliant server: options used: -Dcatalina.base=/usr/share/tomcat -Dcatalina.home=/usr/share/tomcat -Djava.endorsed.dirs= -Djava.io.t
mpdir=/var/cache/tomcat/temp -Djava.util.logging.config.file=/usr/share/tomcat/conf/logging.properties -Djava.util.logging.manager=org.apache.juli.Cl
assLoaderLogManager
May 13 11:41:27 brilliant server: arguments used: stop
May 13 11:41:27 brilliant server: Error: Could not find or load main class $JAVA_OPTS
May 13 11:41:27 brilliant systemd: tomcat.service: control process exited, code=exited status=1
May 13 11:41:27 brilliant systemd: Unit tomcat.service entered failed state.


Expected results:

App starts


Additional info:

The 7.0.42-8 /lib/systemd/system/tomcat.service contains these new lines, and no longer calls /usr/sbin/tomcat-sysd start

EnvironmentFile=/etc/tomcat/tomcat.conf
Environment="NAME="
EnvironmentFile=-/etc/sysconfig/tomcat

At a glance this looks OK, except the tomcat start script /usr/libexec/tomcat/server sources /usr/libexec/tomcat/preamble which contains: 

#!/bin/bash
<snip>
# Get the tomcat config (use this for environment specific settings)
if [ -z "${TOMCAT_CFG_LOADED}" ]; then
  if [ -z "${TOMCAT_CFG}" ]; then
    TOMCAT_CFG="/etc/tomcat/tomcat.conf"
  fi
  . $TOMCAT_CFG
fi

Therefore it is expected that shell expansion should work in the config files, and does when the file is not loaded prematurely by systemd.

On 3rd party software vendor advise we added some configuration to these files and for readability we included comments and shell expansion to append different arguments. EG:

# You can pass some parameters to java here if you wish to
JAVA_OPTS="-Xminf0.1 -Xmaxf0.3"

<snip> 

# set memory and other java options
JAVA_OPTS="$JAVA_OPTS -Duser.language=en -Dfile.encoding=UTF-8 -Xms128M -Xmx512M -XX:PermSize=64M -XX:MaxPermSize=512M -Djavax.xml.transform.TransformerFactory=com.sun.org.apache.xalan.internal.xsltc.trax.TransformerFactoryImpl"
JAVA_OPTS="-Duser.language=en -Dfile.encoding=UTF-8 -Xms128M -Xmx512M -XX:PermSize=64M -XX:MaxPermSize=512M -Djavax.xml.transform.TransformerFactory=com.sun.org.apache.xalan.internal.xsltc.trax.TransformerFactoryImpl"

After testing patches in our test environment this application broke because the systemd EnvironmentFile= argument does not expand $JAVA_OPTS by design.

I would suggest that moving the config file load to tomcat.service is not the correct approach. It is a fair assumption that shell expansion is valid in these config files as the tomcat start shell script does this. 3rd party apps are breaking when configured this way. If commented out the files are still loaded under bash and then things do work. 

The fix would be to remove the 3 breaking Environment lines from /lib/systemd/system/tomcat.service

Comment 2 Anthony Symons 2015-05-16 01:30:26 UTC
Cut/paste fail in the initial 'steps to reproduce' section, I wrote:

JAVA_OPTS="$JAVA_OPTS -Duser.language=en -Dfile.encoding=UTF-8 -Xms128M -Xmx512M -XX:PermSize=64M -XX:MaxPermSize=512M -Djavax.xml.transform.TransformerFactory=com.sun.org.apache.xalan.internal.xsltc.trax.TransformerFactoryImpl"
JAVA_OPTS="-Duser.language=en -Dfile.encoding=UTF-8 -Xms128M -Xmx512M -XX:PermSize=64M -XX:MaxPermSize=512M -Djavax.xml.transform.TransformerFactory=com.sun.org.apache.xalan.internal.xsltc.trax.TransformerFactoryImpl"

should be just:

JAVA_OPTS="$JAVA_OPTS -Duser.language=en -Dfile.encoding=UTF-8 -Xms128M -Xmx512M -XX:PermSize=64M -XX:MaxPermSize=512M -Djavax.xml.transform.TransformerFactory=com.sun.org.apache.xalan.internal.xsltc.trax.TransformerFactoryImpl"

(is there a way to edit the initial description for accuracy?)

Comment 13 Ivan Afonichev 2015-11-25 11:07:47 UTC
As proposed solution you can set 
TOMCAT_CFG_LOADED=0
and TOMCAT_CFG=<your file with your java_opts extensions>

I believe we should rely on systemd environmentfile load for generic configuration, but provide an option to load addtional custom conviguration via shell.
So we are going to add something like $TOMCAT_SHELL_CFG which will be sourced in tomcat-preamble script.

Comment 14 Anthony Symons 2015-11-26 06:30:34 UTC
Thanks for looking at this, and discussing it internally (I presume).

I cant see comments 3 to 12 inclusive, but from Ivans comment I dont think this is a good solution. From my perspective as a user the app fails to start when I do something that Is expected to work and does on other operating systems. 

With this proposed solution, the app still breaks in the same way. I would then need to understand that I need to put my config in another place just to get shell expansion. By the time I understand this, I might as well have just not used shell expansion in the first place. 

Perhaps comments at the top of /etc/sysconfig/tomcat explaining not to use shell syntax would be better?

Its not ideal, but it does not complicate further the already complicated bootstrap process by introducing more files which can contain more bits of config. Less is more IMHO.

The apps which broke at my institution when the Tomcat 7.0.42-8 updates were installed have since been configured without the need for shell expansion and are working. We would not use this proposed fix preferring to keep our config locations standard.

As I see it your best options in my order of preference are:

1) dont load the environment from systemd
2) modify systemd to provide a way to source its environment via a shell, and use that from tomcat.service
3) do nothing but document this shortcoming clearly at top of /etc/sysconfig/tomcat and anywhere guides or FAQ produced by redhat on this subject.

Comment 15 Martijn Pepping 2015-12-22 08:23:56 UTC
Exact the same behavior occurs when using /etc/tomcat/tomcat.conf instead of sytemd /etc/sysconfig/tomcat (and when using CATALINA_OPTS instead of JAVA_OPTS).

Comment 16 Coty Sutherland 2016-01-06 21:14:49 UTC
The sysconfig file is there to allow overrides (not additions to the variables) of the variables provided by the conf file and is loaded last (by systemd). The text of the sysconfig provided by RHEL 7 should be updated to reflect that and in Fedora the sysconfig looks like this http://pkgs.fedoraproject.org/cgit/rpms/tomcat.git/tree/tomcat-8.0.sysconfig?h=f23. However, with the conf file defined in the EnvironmentFile directive it is being loaded twice; once by systemd and then sourced by the preamble. Variables can be present in the conf file as long as sysconfig overrides them so that systemd doesn't try and load them, but if they aren't overridden systemd fails to start the service. I'm not sure why the conf file is loaded twice, but if I remove the EnvironmentFile directive (as stated above) which loads the conf file, then shell expansion works fine and everything is happy as long as sysconfig doesn't contain any variables requiring expansion.

I think that removing the EnvironmentFile for the conf file is the best move (assuming that sysconfig doesn't have variables) and then documenting that sysconfig is solely there to override the conf and doesn't support expansion. This is the smallest change that would still work without breaking the systemd design. I don't really want to the sysconfig load into the preamble or elsewhere because then we're not really using the systemd unit for anything (doing that is the same as the sysV scripts). I'm only proposing removing the conf file because its already being loaded in the preamble. An alternative to that might be to use the TOMCAT_CFG variable (not adding a new variable and config combination) to relocate the config for each instance, if you require shell expansion.

I'll continue looking into this and discussing internally, but I wanted to provide an update for some feedback from those involved here.

Comment 17 Martijn Pepping 2016-01-17 12:55:31 UTC
(In reply to Coty Sutherland from comment #16)
[..]
> I think that removing the EnvironmentFile for the conf file is the best move
> (assuming that sysconfig doesn't have variables) and then documenting that
> sysconfig is solely there to override the conf and doesn't support
> expansion. This is the smallest change that would still work without
> breaking the systemd design. 
[..]

Agree, removing the EnvironmentFile param results in the desired behavior and doesn't impact systemd behavior.

Comment 18 Ivan Afonichev 2016-01-17 19:02:32 UTC
It is usual practice to add coment at the top of default config that it is not recommended to edit it. User is encouraged to add and edit his custom configs in special conf.d folder. So as an option we may add /etc/tomcat/conf.d folder and add shell code to source conf files from it.

Comment 19 Coty Sutherland 2016-01-26 14:06:41 UTC
(In reply to Ivan Afonichev from comment #18)
> configs in special conf.d folder. So as an option we may add
> /etc/tomcat/conf.d folder and add shell code to source conf files from it.

I think that would be OK also, but that doesn't affect the EnvironmentFile removal or update to sysconfig. So I'm still proposing that we pull in the sysconfig file from fc23's tomcat package (which is better documented for this issue) and removing EnvironmentFile for now. I'm also open to adding a conf.d dir, if you think that would help here. I'm of the mind that modifying the tomcat.conf would be sufficient though.

Comment 20 Ivan Afonichev 2016-02-02 15:00:01 UTC
I believe we should source some default file using systemd. But let users source another one(s) using shell scripts.
Maybe we should set EnvironmentFile=/etc/sysconfig/tomcat  with all contents of current /etc/tomcat/tomcat.conf and left /etc/tomcat/tomcat.conf empty with comment "Feel free to add your overrides here. Shell invocations will work here."

Comment 21 Coty Sutherland 2016-02-03 20:15 UTC
Created attachment 1120897 [details]
proposed patch

I took Ivan's last comment and made a patch from it. I think that moving the default configuration into sysconfig and leaving the tomcat.conf for overrides with shell expansion further decouples the tomcat source from the service unit configuration. I amended the messages at the top of sysconfig/tomcat and tomcat.conf to further document the shell expansion issue. I think this is a good solution for now, but feedback is welcome.

Comment 22 Matthew Harmsen 2016-03-02 22:35:25 UTC
Please note that per https://bugzilla.redhat.com/show_bug.cgi?id=1311771#c11 Bugzilla Bug #1311771 - Tomcat 8.0.32 update breaks FreeIPA and Dogtag installations, an error in the F22, F23, and F24 packages occurred.

The problematic change was reverted in http://koji.fedoraproject.org/koji/buildinfo?buildID=741084 tomcat-8.0.32-4.fc25 which fixed the problem:

    * https://bugzilla.redhat.com/show_bug.cgi?id=1311771#c10
    * https://bugzilla.redhat.com/show_bug.cgi?id=1311771#c11

Comment 23 Matthew Harmsen 2016-03-03 18:08:12 UTC
Tested out the following F24 package:

    * http://koji.fedoraproject.org/koji/buildinfo?buildID=741325
      tomcat-8.0.32-4.fc24

Everything works as advertised!

Thanks,
-- Matt

Comment 24 Coty Sutherland 2016-05-16 16:41:02 UTC
The fix for this should align with Fedora's resolution of adding a conf.d directory for shell expansion needs while continuing to closely maintain the systemd unit. That solution was the commit here http://pkgs.fedoraproject.org/cgit/rpms/tomcat.git/commit/?id=7d21a72

Comment 30 errata-xmlrpc 2016-11-03 21:08:39 UTC
Since the problem described in this bug report should be
resolved in a recent advisory, it has been closed with a
resolution of ERRATA.

For information on the advisory, and where to find the updated
files, follow the link below.

If the solution does not work for you, open a new bug report.

https://rhn.redhat.com/errata/RHSA-2016-2599.html


Note You need to log in before you can comment on or make changes to this bug.