On 12/11/18 10:38 AM, Baesken, Matthias wrote:
File paths are, in general, always something that demands extra scrutiny
as it can be the source of security issues (privacy leaks, traversal
attacks, etc). It's not just me that thinks that way, you can do a
search on the Internet and find lots of references.
...
It might be perfectly fine and your usage might be ok too. But I'll need
some time to evaluate it further. I am not familiar with the code in
this part of the JDK.
I would also see if you can think about the security issues as well.
Where do these paths come from? What are the application use cases that
invoke these lower methods? From application code or elsewhere? Are
relative paths used? Are paths containing ".." canonicalized? Are they
arbitrary paths or a fixed set of paths? Do they ever contain sensitive
information like home directory user names, etc?
Once we understand if there are any security issues, then we can decide
if allowing that via a system property is acceptable or not, and if so
the security risks that we would have to document for that property.
Hello, the file paths potentially printed in the enhanced exceptions in
*canonicalize0, *createFileExclusively,
Java_java_io_WinNTFileSystem_canonicalizeWithPrefix0 could potentially come
from more or less arbitrary paths.
That to me, is an issue that warrants much more careful analysis before
deciding whether to proceed with this fix.
This means, in some cases, there is a chance that information (like
user-ids/user-names often found in paths below HOME dirs )
might be in the printed paths that people do not want to have in log files or
other output containing the exception messages.
For such potentially sensitive info in exception messages, we have already the
jdk.includeInExceptions
security property, see the java.security file :
1064 # Enhanced exception message information
1065 #
1066 # By default, exception messages should not include potentially sensitive
1067 # information such as file names, host names, or port numbers. This
property
1068 # accepts one or more comma separated values, each of which represents a
1069 # category of enhanced exception message information to enable. Values are
1070 # case-insensitive. Leading and trailing whitespaces, surrounding each
value,
1071 # are ignored. Unknown values are ignored.
I think this approach fits well for 8211752 (and is used already for some other
categories).
I am concerned that a single property controls whether or not
potentially sensitive pathnames appear in Exceptions.
I created an updated webrev, it uses the jdk.includeInExceptions security
property
(had to do it via JNI because it did not work from the static class
initializers of UnixFileSystem/WinNTFileSystem) :
http://cr.openjdk.java.net/~mbaesken/webrevs/8211752.2/
These exceptions are generated from a very low level part of the native
JDK Windows or Unix FileSystem implementations. That is a concern. The
previous usages of this property were more focused and confined to
smaller parts of the code resulting in fewer code paths to analyze.
I think we need to take a step back. I think we need to reconsider
whether the jdk.includeInExceptions security property is appropriate for
this type of enhancement.
Therefore, I oppose this change as-is. I'm happy to participate in a
more involved discussion where we start with use cases and motivation
before jumping to solutions.
Thanks,
Sean
