Bug#825786: tomcat8: postinst script overwrites file permissions in /etc

[Markus Koschany]

On 29.07.2016 12:48, Emmanuel Bourg wrote:
> Le 29/07/2016 à 11:20, Markus Koschany a écrit :
> 
>> If we keep root:tomcat8 then I think 640 is sensible and appropriate.
> 
> There is a downside though, our package will become unusable with the
> IDEs since local users will no longer be able to read the configuration.
> I don't think we can satisfy both use cases (production server and local
> development server) with a single configuration. Securing the default
> configuration is more important, so we may have to add an extra
> CATALINA_BASE directory somewhere under /var/lib suitable for developers
> (maybe in tomcat8-user?).

I also think the production server use case is the more important one.
Currently tomcat-users.xml is already 640 by the way. However if we want
to keep the current permissions then it doesn't make much sense to chown
the files to root:tomcat8 because they are world-readable and Tomcat is
able to read them anyway. So I am in favor of keeping the tomcat8 group
but using 640 permissions for improved security.

I agree with your last sentence. We could create a /var/lib/tomcat8-user
directory just for development purposes with all files world-readable
and other modifications tailored for developers. That would satisfy both
use cases. This should be discussed in another bug report though.

>> 1. Do not override file permissions for custom files in /etc/tomcat8 any
>> longer. Be explicit instead and only change them for known Debian files.
> 
> Good. The script no longer checks the existence of the files, I suspect
> this may break if tomcat-users.xml is removed by the user for example.
> For the policy.d we can chmod the whole directory, no need to whitelist
> the files.

Yes, the check removal was intentional because we didn't check the other
files for existence too. I feel it is kind of superfluous because they
are created on upgrade again.

In regard of policy.d I believe we should be consistent. The whole point
for doing this change is that we don't want to override _additional_
files in /etc/tomcat8. We don't want to surprise users and break their
custom setups. We cannot know beforehand if someone has created another
policy/random/whatever file in policy.d. So just play it safe here. I
think this part of the patch is very sane.



>> 2. Make /var/lib/tomcat8/conf a real directory and remove the symlink.
>>    Instead symlink all Debian files from /etc/tomcat8 into
>> /var/lib/tomcat8/conf
>>
>> 3. Remove /etc/tomcat8/Catalina and move it into
>> /var/lib/tomcat8/conf/Catalina
> 
> I'm not fond of this change, this is more complicated and will break the
> expectation that files added to /etc/tomcat8/ are automatically seen as
> part of the Tomcat conf directory. This may break existing
> installations, for example when a SSL connector references a keystore
> relatively to the CATALINA_BASE directory. Currently I have something
> like this on my servers:
> 
>     <Connector port="443" protocol="HTTP/1.1"
>                SSLEnabled="true"
>                scheme="https"
>                secure="true"
>                clientAuth="false"
>                sslProtocol="TLS"
>                keystoreFile="conf/keystore.p12"
>                keystorePass="secret"
>                keystoreType="PKCS12"/>
> 
> Here the keystore.p12 file is located in /etc/tomcat8 and automatically
> available at /var/lib/tomcat8/conf/keystore.p12. With the proposed
> changes the server.xml file will have to be updated with an absolute path.

I'm not sure if I have understood you correctly here. Did you see the
part of the patch that preserves all custom files in /etc/tomcat8 and
moves them to /var/lib/tomcat8/conf? (tomcat8.preinst)

Your keystore.p12 file would be located in
/var/lib/tomcat8/conf/keystore.p12 and this is exactly relative to
CATALINA_BASE. You can remove the old /etc/tomcat8/keystore.p12 file by
hand but this has no implications on your setup and the server should
continue to work.

> If we were to implement this I think it should happen in the next
> tomcat9 package instead.

I have tested this patch and both upgrades and new installations work
flawlessly. In my opinion there is no technical reason why this
shouldn't be implemented for Stretch.


>> The stable patch only implements point 1 that should address the issue
>> described in this bug report.
> 
> Ok, the stable patch shouldn't change the permissions to 640 though.

Fine with me.


-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 949 bytes
Desc: OpenPGP digital signature
URL: <http://lists.alioth.debian.org/pipermail/pkg-java-maintainers/attachments/20160729/bc8e29c4/attachment.sig>