[RFR] 8205525 : Improve exception messages during manifest parsing of jar archives
Baesken, Matthias
matthias.baesken at sap.com
Mon Sep 10 08:21:09 UTC 2018
> I think it would be enough to drop the privileged section and just return
> "filename" as is. (without conveting to a file object).
OK, any objections on this ?
> I also agree with Sean that the common parts of the new
> src/java.base/share/classes/sun/security/util/SecurityProperties.java should
> be aligned with
> src/java.base/share/classes/sun/net/util/SocketExceptions.java (That is,
> SocketExceptions calling SecurityProperties)
OK I will include this in the next webrev .
Thanks, Matthias
> -----Original Message-----
> From: Langer, Christoph
> Sent: Montag, 10. September 2018 10:04
> To: Baesken, Matthias <matthias.baesken at sap.com>; Wang Weijun
> <weijun.wang at oracle.com>; Sean Mullan <sean.mullan at oracle.com>
> Cc: security-dev at openjdk.java.net; core-libs-dev at openjdk.java.net
> Subject: RE: [RFR] 8205525 : Improve exception messages during manifest
> parsing of jar archives
>
> Hi Matthias,
>
> I think it would be enough to drop the privileged section and just return
> "filename" as is. (without conveting to a file object).
>
> I also agree with Sean that the common parts of the new
> src/java.base/share/classes/sun/security/util/SecurityProperties.java should
> be aligned with
> src/java.base/share/classes/sun/net/util/SocketExceptions.java (That is,
> SocketExceptions calling SecurityProperties)
>
> Best regards
> Christoph
>
>
> > -----Original Message-----
> > From: core-libs-dev <core-libs-dev-bounces at openjdk.java.net> On Behalf
> > Of Baesken, Matthias
> > Sent: Montag, 10. September 2018 09:53
> > To: Wang Weijun <weijun.wang at oracle.com>; Sean Mullan
> > <sean.mullan at oracle.com>
> > Cc: security-dev at openjdk.java.net; core-libs-dev at openjdk.java.net
> > Subject: [CAUTION] RE: [RFR] 8205525 : Improve exception messages
> during
> > manifest parsing of jar archives
> >
> > Hello are you fine with changing from file.getAbsolutePath() to
> > file.getName() ?
> >
> >
> >
> http://cr.openjdk.java.net/~mbaesken/webrevs/8205525.8/src/java.base/s
> > hare/classes/java/util/jar/Manifest.java.frames.html
> >
> >
> > 206 static String getErrorPosition(String filename, final int lineNumber) {
> > 207 if (filename == null || !jarPathInExceptionText) {
> > 208 return "line " + lineNumber;
> > 209 }
> > 210
> > 211 final File file = new File(filename);
> > 212 return AccessController.doPrivileged(new
> PrivilegedAction<String>()
> > {
> > 213 public String run() {
> > 214 return "manifest of " + file.getName() + ":" + lineNumber;
> > 215 }
> >
> >
> > Best regards, Matthias
> >
> >
> > > -----Original Message-----
> > > From: Wang Weijun <weijun.wang at oracle.com>
> > > Sent: Samstag, 8. September 2018 17:43
> > > To: Sean Mullan <sean.mullan at oracle.com>
> > > Cc: Baesken, Matthias <matthias.baesken at sap.com>; Alan Bateman
> > > <Alan.Bateman at oracle.com>; Chris Hegarty
> <chris.hegarty at oracle.com>;
> > > security-dev at openjdk.java.net; core-libs-dev at openjdk.java.net
> > > Subject: Re: [RFR] 8205525 : Improve exception messages during
> manifest
> > > parsing of jar archives
> > >
> > > Thinking about this again. Looks like the absolute path is not necessary.
> > Even
> > > if there are multiple files using the same name, they will be in different
> > > directories, no matter absolute or relative. Suppose the jarPath info is
> used
> > > for debugging purpose mostly like the developer can find out what the
> > > current working directory is and can find the files. *Matthias*: Is the
> > absolute
> > > path really necessary? Are you working on actual case?
> > >
> > > As for the possible global effect of a security property, maybe we can
> > > emphasis that this is both a security property and system property, and if
> > it’s
> > > just for one time use, it’s better to use a system property.
> > >
> > > BTW, does the existing value “hostInfo” of the property have a similar
> > > problem?
> > >
> > > Thanks
> > > Max
> > >
> > > >> 在 2018年9月8日，21:42，Sean Mullan <sean.mullan at oracle.com>
> 写
> > > 道：
> > > >>
> > > >> On 9/7/18 7:58 PM, Weijun Wang wrote:
> > > >> In my understanding, the author deliberately wants to show the
> > absolute
> > > paths when there are multiple jar files with the same name (Ex: a jar
> hell).
> > > >
> > > > If you are very familiar with a particular application and understand the
> > risks
> > > associated with running it, then I agree that is ok. But security properties
> > > apply to all applications using that JDK, and so I don't always think it is
> > > practical for an admin to understand every type of application that may
> be
> > > using that JDK and whether or not enabling this property presents a risk.
> > > >
> > > >> Maybe we can add some more detail in the java.security so an admin
> > > knows what exact impact it has.
> > > >
> > > > It would be a slippery slope in my opinion. You would have to say
> > > something like: "enabling this property may allow untrusted code running
> > > under a SecurityManager to access sensitive information such as the
> > > user.home system property even if it has not been granted permission to
> > do
> > > so."
> > > >
> > > > I think we should consider not allowing this property to take effect if a
> > > SecurityManager is running.
> > > >
> > > > --Sean
> > > >
> > > >> --Max
> > > >>> On Sep 8, 2018, at 3:41 AM, Sean Mullan <sean.mullan at oracle.com>
> > > wrote:
> > > >>>
> > > >>> On 8/29/18 10:01 AM, Baesken, Matthias wrote:
> > > >>>> Hi Max, thanks for your input .
> > > >>>> I created another webrev , this contains now the suggested
> > > SecurityProperties class :
> > > >>>> http://cr.openjdk.java.net/~mbaesken/webrevs/8205525.6/
> > > >>>
> > > >>> java/util/jar/Attributes.java
> > > >>>
> > > >>> 469 return AccessController.doPrivileged(new
> > > PrivilegedAction<String>() {
> > > >>> 470 public String run() {
> > > >>> 471 return file.getAbsolutePath() + ":" + lineNumber;
> > > >>> 472 }
> > > >>> 473 });
> > > >>>
> > > >>> I have a serious concern with the code above.
> > > >>>
> > > >>> With this change, untrusted code running under a SecurityManager
> > > could potentially create a JarFile on a non-absolute path ex: "foo.jar", and
> > > (somehow) cause an IOException to be thrown which would then reveal
> > the
> > > absolute path of where the jar was located, and thus could reveal
> sensitive
> > > details about the system (ex: the user's home directory). It could still be
> > hard
> > > to exploit, since it would have to be a known jar that exists, and you
> would
> > > then need to cause an IOException to be thrown, but it's still theoretically
> > > possible.
> > > >>>
> > > >>> In short, this is a more general issue in that it may allow untrusted
> code
> > > to access something it couldn't have previously. new
> > > JarFile("foo.jar").getName() returns "foo.jar", and not the absolute path.
> > > >>>
> > > >>> Granted this can only be done if the security property is enabled, but
> I
> > > believe this is not something administrators should have to know about
> > their
> > > environment and account for when enabling this property.
> > > >>>
> > > >>> The above code should be changed to return only what the caller
> > > provides to JarFile, which is the name of the file (without the full path).
> > > >>>
> > > >>> --Sean
>
More information about the core-libs-dev
mailing list