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.
signature.asc
Description: OpenPGP digital signature
