RFR: 8211752: JNU_ThrowIOExceptionWithLastErrorAndPath - enhance some IOExceptions with path causing the issue

[Sean Mullan]

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