Bug 61210 - When using the Security Manager, Tomcat prints warning about a non-existent file
Summary: When using the Security Manager, Tomcat prints warning about a non-existent file
Status: RESOLVED FIXED
Alias: None
Product: Tomcat 8
Classification: Unclassified
Component: JULI (show other bugs)
Version: 8.5.15
Hardware: PC Linux
: P2 normal (vote)
Target Milestone: ----
Assignee: Tomcat Developers Mailing List
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2017-06-22 21:25 UTC by Coty Sutherland
Modified: 2017-08-21 09:53 UTC (History)
0 users



Attachments
First attempt (2.41 KB, patch)
2017-06-26 17:21 UTC, Coty Sutherland
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Coty Sutherland 2017-06-22 21:25:37 UTC
I noticed that when using the Security Manager in Tomcat you always see the following warning messages on startup (one for every application that's deployed):

~~~
WARNING [localhost-startStop-1] org.apache.juli.ClassLoaderLogManager.readConfiguration Reading /apache-tomcat-8.5.15/webapps/ROOT/WEB-INF/classes/logging.properties is not permitted. See "per context logging" in the default catalina.policy file.
WARNING [localhost-startStop-1] org.apache.juli.ClassLoaderLogManager.readConfiguration Reading /apache-tomcat-8.5.15/webapps/examples/WEB-INF/classes/logging.properties is not permitted. See "per context logging" in the default catalina.policy file.
WARNING [localhost-startStop-1] org.apache.juli.ClassLoaderLogManager.readConfiguration Reading /apache-tomcat-8.5.15/webapps/docs/WEB-INF/classes/logging.properties is not permitted. See "per context logging" in the default catalina.policy file.
~~~

While the message is technically accurate (reading the file is prohibited), it isn't pertinent to the user because the file may not (doesn't in this case) exist in any of these three applications. Is it possible to check that the file exists at this point so that we can only print the message when it's present? Do so would remove an extra warning (that may not be applicable) from the log file and give users a clean vanilla log to start with.
Comment 1 Konstantin Kolinko 2017-06-22 22:29:53 UTC
1. From your logs, you are running Tomcat 8.5.15. I am changing the Version field to match that.

2. Generally, this is a feature.
The message text tells one to look into the catalina.policy file,
and there is a comment there that explains the issue. 

"// Note: To enable per context logging configuration" ...

https://svn.apache.org/viewvc/tomcat/tc8.5.x/tags/TOMCAT_8_5_15/conf/catalina.policy?view=markup#l93


Any ideas how to improve users' experience here?

Allowing to read some random logging.properties files is not an option,
as it is insecure.




- An idea:
Add an explanation of this issue to Documentation and change message text to tell users to read that documentation page as well.

http://tomcat.apache.org/tomcat-8.5-doc/security-manager-howto.html
Comment 2 Mark Thomas 2017-06-22 23:02:56 UTC
I was thinking add a privileged block that tested if the file existed and don't trigger the warning if it doesn't. Note I haven't dug into the code to see hwo easy this would be yet.
Comment 3 Coty Sutherland 2017-06-23 12:02:23 UTC
(In reply to Konstantin Kolinko from comment #1)
> 1. From your logs, you are running Tomcat 8.5.15. I am changing the Version
> field to match that.

I tested with 8.5.x too, apparently I copied the wrong logs.

> 2. Generally, this is a feature.
> The message text tells one to look into the catalina.policy file,
> and there is a comment there that explains the issue. 

Like I said, the message is accurate however the file that it's warning about doesn't exist. This could cause users to see a warning in the log file that needs to be fixed when in fact there is no problem. 
> 
> Any ideas how to improve users' experience here?
> 
> Allowing to read some random logging.properties files is not an option,
> as it is insecure.

I'm not sure what you're after here. I don't want anyone to be able to read the file :) I want the warning message to be conditional based on whether or not the file actually exists.
Comment 4 Coty Sutherland 2017-06-23 12:04:05 UTC
(In reply to Mark Thomas from comment #2)
> I was thinking add a privileged block that tested if the file existed and
> don't trigger the warning if it doesn't. Note I haven't dug into the code to
> see hwo easy this would be yet.

+1, that's what I was hoping for. I haven't played much with privileged blocks, but I can try and mock up a quick patch to do that.
Comment 5 Coty Sutherland 2017-06-26 17:21:22 UTC
Created attachment 35077 [details]
First attempt

Here's my first attempt at checking whether or not the file exists before logging. The problem with this is that the privileged block is still failing checkPermission. I'm not quite sure how to fix it as I modeled my change after some other doPrivileged calls in the same class. The only difference is that I'm returning a value to check later in the readConfiguration method instead of Void. Can anyone point me in the right direction?
Comment 6 Mark Thomas 2017-06-26 18:28:27 UTC
It fails because the call originates in JULI and JULI doesn't have permissions to read the file. All the Privileged block does is stop the security manager also checking that all of the callers up the stack also have permission to read the file.

An alternative approach will be required.
Comment 7 Mark Thomas 2017-06-26 19:39:35 UTC
There is a way to do this.

Hint: Take a look at org.apache.juli.WebappProperties and how it is used.
Comment 8 Mark Thomas 2017-07-05 16:50:51 UTC
Coty, I have a patch for this but I thought you might want to figure this out for yourself. If you want another hint (or just want me to apply my patch), let me know.
Comment 9 Coty Sutherland 2017-07-05 17:31:49 UTC
I've been meaning to circle back to this (and a few others...) but haven't been able to make time just yet. I do recall being a bit confused by your last hint because I couldn't see the correlation between how WebappProperties was used and what I should be doing. Care to drop another hint? If you want to push the patch you have ready, you can commit it and I'll just review what you did and maybe do something similar next time :)
Comment 10 Mark Thomas 2017-07-05 17:40:22 UTC
A slightly bigger hint:

JULI cannot have any external dependencies.
The "Does this file exist?" test needs to happen in a privileged block.
That privileged block needs to be located in a class in a JAR that has full privs (i.e. CATALINA_BASE/lib).
You need a way to call into a that class from JULI.
Comment 11 Mark Thomas 2017-08-21 09:53:45 UTC
It has been a while so I've applied my patch for this.

Fixed in:
- trunk for 9.0.0.M27 onwards
- 8.5.x for 8.5.21 onwards
- 8.0.x for 8.0.47 onwards