Re: RFR [11] 8207846: Generalize the jdk.net.includeInExceptions security property
Thanks for the review Sean, > On 23 Jul 2018, at 16:58, Sean Mullan wrote: > ... >> http://cr.openjdk.java.net/~chegar/8207846/webrev.00/ > > A few nits and wording suggestions in the java.security file: > > "By default, several exception messages do not include potentially sensitive > information such as file names, host names, or port numbers." > > I think the following sounds a bit better: > > "By default, exception messages should not include potentially sensitive > information such as file names, host names, or port numbers." > > Also, the 2nd and 3rd sentences basically say the same thing. I would remove > the 2nd sentence. > > "The categories, to enable enhanced exception message information, are:" > > I would remove ", to enable enhanced exception message information," since it > seems redundant (and I believe is grammatically incorrect). > > hostInfo - IOExceptions thrown by java.net.Socket and also the ... > > Remove "also" (not really necessary). Agreed. Here’s where this ended up. # # Enhanced exception message information # # By default, exception messages should not include potentially sensitive # information such as file names, host names, or port numbers. This property # accepts one or more comma separated values, each of which represents a # category of enhanced exception message information to enable. Values are # case-insensitive. Leading and trailing whitespaces, surrounding each value, # are ignored. Unknown values are ignored. # # The categories are: # # hostInfo - IOExceptions thrown by java.net.Socket and the socket types in the # java.nio.channels package will contain enhanced exception # message information # # The property setting in this file can be overridden by a system property of # the same name, with the same syntax and possible values. # #jdk.includeInExceptions=hostInfo -Chris
Re: RFR [11] 8207846: Generalize the jdk.net.includeInExceptions security property
On 7/23/18 6:09 AM, Chris Hegarty wrote: After given this some more thought, I now think that I gave in to the comment to change whitespace handing too easy. While maybe not consistent, with the already inconsistent, whitespace handling in java.security, I think ( for this particular case ) the original - trim leading and trailing - is the right thing to do. It avoids your above scenario where someone accidentally adds a leading space, which could be difficult to debug/find without a warning - which we should avoid if possible. Thanks for making that change. I’d like to re-propose the original webrev for consideration ( whitespace handling is the only change ): http://cr.openjdk.java.net/~chegar/8207846/webrev.00/ A few nits and wording suggestions in the java.security file: "By default, several exception messages do not include potentially sensitive information such as file names, host names, or port numbers." I think the following sounds a bit better: "By default, exception messages should not include potentially sensitive information such as file names, host names, or port numbers." Also, the 2nd and 3rd sentences basically say the same thing. I would remove the 2nd sentence. "The categories, to enable enhanced exception message information, are:" I would remove ", to enable enhanced exception message information," since it seems redundant (and I believe is grammatically incorrect). hostInfo - IOExceptions thrown by java.net.Socket and also the ... Remove "also" (not really necessary). --Sean
Re: RFR [11] 8207846: Generalize the jdk.net.includeInExceptions security property
Sean, > On 20 Jul 2018, at 18:07, Sean Mullan wrote: > > On 7/20/18 11:08 AM, Chris Hegarty wrote: >> This is ambiguous, and needs to be clarified. Surely, it is >> better to use the same wording as the serial filter: >> "Whitespace is significant and is considered part of the value." > > Kind of on the fence on that one. If this were a general property/value > format, I would agree, but these values are fixed and not potentially > complicated expressions. Sure, they are simple strings consisting of only printable characters. > For example, does this mean: > > jdk.includeInExceptions=hostInfo,jarInfo > > and > > jdk.includeInExceptions=hostInfo, jarInfo > > are different? And I assume the latter " jarInfo" would be ignored? > > If you are strongly in favor of this, I would highly recommend logging a > warning when there is an unknown value, otherwise typos and such would be > hard to detect (although this doesn't necessarily need to be in the > specification). My original whitespace handling avoided the potential issue where a rogue lead or trailing whitespace accidentally found its way into a value, therefore avoiding the scenario above ( and potentially emitting a warning ). Whitespace is significant and should be specified as such, since ... jdk.includeInExceptions=“host Info” IS NOT equal to jdk.includeInExceptions=“hostInfo” and system property values can contain spaces jdk.includeInExceptions=“foo bar,hostInfo,jar Info,” After given this some more thought, I now think that I gave in to the comment to change whitespace handing too easy. While maybe not consistent, with the already inconsistent, whitespace handling in java.security, I think ( for this particular case ) the original - trim leading and trailing - is the right thing to do. It avoids your above scenario where someone accidentally adds a leading space, which could be difficult to debug/find without a warning - which we should avoid if possible. I’d like to re-propose the original webrev for consideration ( whitespace handling is the only change ): http://cr.openjdk.java.net/~chegar/8207846/webrev.00/ -Chris.
Re: RFR [11] 8207846: Generalize the jdk.net.includeInExceptions security property
On 7/20/18 11:08 AM, Chris Hegarty wrote: This is ambiguous, and needs to be clarified. Surely, it is better to use the same wording as the serial filter: "Whitespace is significant and is considered part of the value." Kind of on the fence on that one. If this were a general property/value format, I would agree, but these values are fixed and not potentially complicated expressions. For example, does this mean: jdk.includeInExceptions=hostInfo,jarInfo and jdk.includeInExceptions=hostInfo, jarInfo are different? And I assume the latter " jarInfo" would be ignored? If you are strongly in favor of this, I would highly recommend logging a warning when there is an unknown value, otherwise typos and such would be hard to detect (although this doesn't necessarily need to be in the specification). --Sean
Re: RFR [11] 8207846: Generalize the jdk.net.includeInExceptions security property
> On 20 Jul 2018, at 16:15, Roger Riggs wrote: > > Hi Chris, > > The updated text is fine. > Thanks for your consideration. Updated webrev as discussed. http://cr.openjdk.java.net/~chegar/8207846/webrev.01/ -Chris.
Re: RFR [11] 8207846: Generalize the jdk.net.includeInExceptions security property
Hi Chris, The updated text is fine. Thanks for your consideration. The overlapping of configure options between java.security, net.properties, etc. and command line options is something to keep an eye on. Roger On 7/20/18 11:08 AM, Chris Hegarty wrote: Roger, On 20 Jul 2018, at 15:36, Roger Riggs wrote: Hi Chris, It is important to be clear about how whitespace is treated and within the java.security file there are other uses that explicitly define how whitespace is used. Right, and the usages are already inconsistent. Nothing we can do about that now. I am more concerned about how command line properties are understood and used how we have to document them. Allowing whitespace quickly gets bogged down in how shells handle quotes, telling people they have to quote them and when/whether you have to quote the quotes. You cannot disallow whitespace, simple ignore them or consider them part of the value. Having a consistent treatment of command line and security properties keeps the story simple and easier to support. This file is already inconsistent, trimming happens in some cases. Whitespaces are either trimmed, ignored, or considered as like any other character. The jdk.serialFilter property had the same issue and is explicit in the java.security file that spaces are just another character and are not treated specially. This is a reasonable position. Its a slippery slope, if we start compensating/ignoring whitespace in some properties then we will have to keep explaining how some are treated differently. I would keep the original non-whitespace description. Original: "This property may be set to one or more values, separated by commas, and with no white-space” This is ambiguous, and needs to be clarified. Surely, it is better to use the same wording as the serial filter: "Whitespace is significant and is considered part of the value." Case-insensistive compares are another slippery slope but make a bit more sense for usability. The complete updated text: # # Enhanced exception message information # # By default, several exception messages do not include potentially sensitive # information such as file names, host names, or port numbers. This property may # be used to enable categories of enhanced information in exception messages. # The property accepts one or more comma separated values, each of which # represents a category of enhanced exception message information to enable. # Values are case-insensitive. Whitespace is significant and is considered part # of the value. Unknown values are ignored. # # The categories, to enable enhanced exception message information, are: # # hostInfo - IOExceptions thrown by java.net.Socket and also the socket types # in the java.nio.channels package will contain enhanced exception # message information # # The property setting in this file can be overridden by a system property of # the same name, with the same syntax and possible values. # #jdk.includeInExceptions=hostInfo -Chris.
Re: RFR [11] 8207846: Generalize the jdk.net.includeInExceptions security property
Roger, > On 20 Jul 2018, at 15:36, Roger Riggs wrote: > > Hi Chris, > > It is important to be clear about how whitespace is treated and within the > java.security file > there are other uses that explicitly define how whitespace is used. Right, and the usages are already inconsistent. Nothing we can do about that now. > I am more concerned about how command line properties are understood and used > how we have to document them. > Allowing whitespace quickly gets bogged down in how shells handle quotes, > telling people they have to > quote them and when/whether you have to quote the quotes. You cannot disallow whitespace, simple ignore them or consider them part of the value. > Having a consistent treatment of command line and security properties keeps > the > story simple and easier to support. This file is already inconsistent, trimming happens in some cases. Whitespaces are either trimmed, ignored, or considered as like any other character. > The jdk.serialFilter property had the same issue and is explicit in the > java.security file > that spaces are just another character and are not treated specially. This is a reasonable position. > Its a slippery slope, if we start compensating/ignoring whitespace in some > properties > then we will have to keep explaining how some are treated differently. > I would keep the original non-whitespace description. Original: "This property may be set to one or more values, separated by commas, and with no white-space” This is ambiguous, and needs to be clarified. Surely, it is better to use the same wording as the serial filter: "Whitespace is significant and is considered part of the value." > Case-insensistive compares are another slippery slope but make a bit more > sense for usability. The complete updated text: # # Enhanced exception message information # # By default, several exception messages do not include potentially sensitive # information such as file names, host names, or port numbers. This property may # be used to enable categories of enhanced information in exception messages. # The property accepts one or more comma separated values, each of which # represents a category of enhanced exception message information to enable. # Values are case-insensitive. Whitespace is significant and is considered part # of the value. Unknown values are ignored. # # The categories, to enable enhanced exception message information, are: # # hostInfo - IOExceptions thrown by java.net.Socket and also the socket types # in the java.nio.channels package will contain enhanced exception # message information # # The property setting in this file can be overridden by a system property of # the same name, with the same syntax and possible values. # #jdk.includeInExceptions=hostInfo -Chris.
Re: RFR [11] 8207846: Generalize the jdk.net.includeInExceptions security property
Hi Chris, It is important to be clear about how whitespace is treated and within the java.security file there are other uses that explicitly define how whitespace is used. I am more concerned about how command line properties are understood and used how we have to document them. Allowing whitespace quickly gets bogged down in how shells handle quotes, telling people they have to quote them and when/whether you have to quote the quotes. Having a consistent treatment of command line and security properties keeps the story simple and easier to support. The jdk.serialFilter property had the same issue and is explicit in the java.security file that spaces are just another character and are not treated specially. Its a slippery slope, if we start compensating/ignoring whitespace in some properties then we will have to keep explaining how some are treated differently. I would keep the original non-whitespace description. Case-insensistive compares are another slippery slope but make a bit more sense for usability. $.02, Roger On 7/20/18 10:18 AM, Chris Hegarty wrote: Roger, On 20 Jul 2018, at 14:52, Roger Riggs wrote: Hi Chris, It is very unusual for the processing of system properties to do *any* parsing except for delimiters including removing spaces, etc. It complicates the handling and sets a bad precedent that makes it more complex for users and developers to know how to set property values. The whitespace trimming should be removed. The addition of the whitespace trimming was to clarify the intent of the existing wording regarding whitespace. It was not intended to set a precedent, good or bad. Regardless of any subjective opinion, whitespace are allowable within system property values, so it’s a matter of whether or not we want to deal with them explicitly or just leave it as an implementation detail. I’m ok to drop the trimming if you feel strongly about it. But just to note that this now diverges even more from the original. -Chris. $.02, Roger http://cr.openjdk.java.net/~chegar/8207846/webrev.00/ -Chris. P.S. It appears that jtreg does not support quoted system property values with spaces on the @run line. I’ll file an issue against jtreg for this.
Re: RFR [11] 8207846: Generalize the jdk.net.includeInExceptions security property
Roger, > On 20 Jul 2018, at 14:52, Roger Riggs wrote: > > Hi Chris, > > It is very unusual for the processing of system properties to do *any* > parsing except for delimiters > including removing spaces, etc. It complicates the handling and sets a bad > precedent > that makes it more complex for users and developers to know how to set > property values. > The whitespace trimming should be removed. The addition of the whitespace trimming was to clarify the intent of the existing wording regarding whitespace. It was not intended to set a precedent, good or bad. Regardless of any subjective opinion, whitespace are allowable within system property values, so it’s a matter of whether or not we want to deal with them explicitly or just leave it as an implementation detail. I’m ok to drop the trimming if you feel strongly about it. But just to note that this now diverges even more from the original. -Chris. > $.02, Roger > >> http://cr.openjdk.java.net/~chegar/8207846/webrev.00/ >> >> -Chris. >> >> P.S. It appears that jtreg does not support quoted system property values >> with spaces on the @run line. I’ll file an issue against jtreg for this. >> >
Re: RFR [11] 8207846: Generalize the jdk.net.includeInExceptions security property
Hi Chris, It is very unusual for the processing of system properties to do *any* parsing except for delimiters including removing spaces, etc. It complicates the handling and sets a bad precedent that makes it more complex for users and developers to know how to set property values. The whitespace trimming should be removed. $.02, Roger On 7/20/18 7:38 AM, Chris Hegarty wrote: JDK-8204233 added a new security property, `jdk.net.includeInExceptions`, to include additional, potentially security sensitive, information in exception detail messages in the networking area. The property accepts a comma separated list of values that specifies the particular type of extra detail information to add. Since its addition, in JDK 11, further uses have arisen to include additional, potentially security sensitive, information in exception detail messages in other areas, namely the java.util.jar APIs. See JDK-8205525, and http://mail.openjdk.java.net/pipermail/core-libs-dev/2018-July/054284.html Given that this mechanism will likely be used more generally across different parts of the platform, it seem prudent to rename the property to be less area-specific, thus allowing for additional argument values to be specified, like for example `jarPath`. The following are the suggested changes to the java.security file: $ hg extdiff -p diff -o -C1 src/java.base/share/conf/security/java.security *** 1062,1074 # ! # Enhanced exception message text # ! # By default, socket exception messages do not include potentially sensitive ! # information such as hostnames or port numbers. This property may be set to one ! # or more values, separated by commas, and with no white-space. Each value ! # represents a category of enhanced information. Currently, the only category defined ! # is "hostInfo" which enables more detailed information in the IOExceptions ! # thrown by java.net.Socket and also the socket types in the java.nio.channels package. ! # The setting in this file can be overridden by a system property of the same name ! # and with the same syntax and possible values. ! #jdk.net.includeInExceptions=hostInfo --- 1062,1084 + + # + # Enhanced exception message information + # + # By default, several exception messages do not include potentially sensitive + # information such as file names, host names, or port numbers. This property may + # be used to enable categories of enhanced information in exception messages. + # The property accepts one or more comma separated values, each of which + # represents a category of enhanced exception message information to enable. + # Values are case-insensitive. Leading and trailing whitespaces, surrounding + # each value, are ignored. Unknown values are ignored. + # + # The categories, to enable enhanced exception message information, are: + # + # hostInfo - IOExceptions thrown by java.net.Socket and also the socket types + # in the java.nio.channels package will contain enhanced exception + # message information # ! # The property setting in this file can be overridden by a system property of ! # the same name, with the same syntax and possible values. # ! #jdk.includeInExceptions=hostInfo Full webrev: http://cr.openjdk.java.net/~chegar/8207846/webrev.00/ -Chris. P.S. It appears that jtreg does not support quoted system property values with spaces on the @run line. I’ll file an issue against jtreg for this.
Re: RFR [11] 8207846: Generalize the jdk.net.includeInExceptions security property
On 20/07/2018 12:38, Chris Hegarty wrote: JDK-8204233 added a new security property, `jdk.net.includeInExceptions`, to include additional, potentially security sensitive, information in exception detail messages in the networking area. The property accepts a comma separated list of values that specifies the particular type of extra detail information to add. : Full webrev: http://cr.openjdk.java.net/~chegar/8207846/webrev.00/ This looks good. For the follow-on jarPath then I think we should look at moving the parsing to somewhere in java.base so that it can be used by other code in the module. -Alan
RE: RFR [11] 8207846: Generalize the jdk.net.includeInExceptions security property
Hi, I scanned all the changes we did to exception messages in our internal VM, see below. We print paths and sockets in a row of places, but also other information. It's wide spread, while most is in java.base. We plan to contribute these messages in the near future. Thus it'll be useful if the property can be reused whenever some of this information is rated security relevant. Best regards, Goetz. java/io/FileOutputStream.java java/io/RandomAccessFile.java path java.base/unix/native/libjava/UnixFileSystem_md.c java.base/windows/native/libjava/WinNTFileSystem_md.c path java/util/jar/Manifest.java java/util/jar/Attributes.java path, position in file java/util/Timer.java print the errornous values (delay, period, time) jdk/internal/org/objectweb/asm/MethodWriter.java length of too long bytecode code libverify/check_code.c position of error, errroneous types java.base/share/native/libzip/zip_util.c path to zipped file java.base/share/native/libzip/zlib/inflate.c zip version information java.base/unix/native/libnet/Inet6AddressImpl.c java.base/share/native/libnet/net_util.c java.base/unix/native/libnet/PlainDatagramSocketImpl.c java.base/unix/native/libnet/PlainSocketImpl.c java.base/unix/native/libnet/SocketInputStream.c java.base/unix/native/libnet/SocketOutputStream.c java.base/windows/native/libnet/DualStackPlainDatagramSocketImpl.c java.base/windows/native/libnet/DualStackPlainSocketImpl.c Information about sockets java.base/unix/native/libnio/ch/DatagramChannelImpl.c java.base/unix/native/libnio/ch/DatagramDispatcher.c socket java.base/unix/native/libnio/ch/FileChannelImpl.c size, protection codes of memory, file, java.desktop/unix/native/libawt_xawt/awt/awt_GraphicsEnv.c more detailed information about DISPLAY jdk.httpserver/share/classes/sun/net/httpserver/ServerImpl.java inet address jdk.zipfs/share/classes/jdk/nio/zipfs/ZipFileSystem.java path > -Original Message- > From: core-libs-dev [mailto:core-libs-dev-boun...@openjdk.java.net] On > Behalf Of Chris Hegarty > Sent: Freitag, 20. Juli 2018 13:38 > To: core-libs-dev ; Security Dev OpenJDK > > Subject: RFR [11] 8207846: Generalize the jdk.net.includeInExceptions > security property > > JDK-8204233 added a new security property, `jdk.net.includeInExceptions`, > to include additional, potentially security sensitive, information in > exception detail messages in the networking area. The property accepts a > comma separated list of values that specifies the particular type of > extra detail information to add. > > Since its addition, in JDK 11, further uses have arisen to include > additional, potentially security sensitive, information in exception > detail messages in other areas, namely the java.util.jar APIs. See > JDK-8205525, and http://mail.openjdk.java.net/pipermail/core-libs- > dev/2018-July/054284.html > > Given that this mechanism will likely be used more generally across > different parts of the platform, it seem prudent to rename the property > to be less area-specific, thus allowing for additional argument values > to be specified, like for example `jarPath`. > > The following are the suggested changes to the java.security file: > > $ hg extdiff -p diff -o -C1 src/java.base/share/conf/security/java.security > *** 1062,1074 > > # > ! # Enhanced exception message text > # > ! # By default, socket exception messages do not include potentially sensitive > ! # information such as hostnames or port numbers. This property may be set > to one > ! # or more values, separated by commas, and with no white-space. Each > value > ! # represents a category of enhanced information. Currently, the only > category defined > ! # is "hostInfo" which enables more detailed information in the IOExceptions > ! # thrown by java.net.Socket and also the socket types in the > java.nio.channels package. > ! # The setting in this file can be overridden by a system property of the > same > name > ! # and with the same syntax and possible values. > ! #jdk.net.includeInExceptions=hostInfo > --- 1062,1084 > > + > + # > + # Enhanced exception message information > + # > + # By default, several exception messages do not include potentially > sensitive > + # information such as file names, host names, or port numbers. This > property may > + # be used to enable categories of enhanced information in exception > messages. > + # The property accepts one or more comma separated values, each of > which > + # represents a category of enhanced exception message information to > enable. > + # Values are case-insensitive. Leading and trailing whitespaces, surrounding > + # each value, are ignored. Unknown values are ignored. > + # > + # The categories, to enable enhanced exception message information, are: > + # > + # hostInfo - IOExceptions thrown by java.net.Socket and also the socket > types > + # in the java.nio.channels package will contain enhanced > exception > + #
