[RFR] 8205525 : Improve exception messages during manifest parsing of jar archives
Baesken, Matthias
matthias.baesken at sap.com
Fri Aug 24 14:52:59 UTC 2018
Hello, I created another webrev :
http://cr.openjdk.java.net/~mbaesken/webrevs/8205525.5/
- use now jarPath instead of jarpath in the java security file
- moved the parsing of the property to a more general location (separate class jdk/internal/util/SecuritySystemPropertyHelper.java )
- read now from the InputStream and check for error conditions *before* setting jarFilename in the Manifest constructor
Best regards , Matthias
> -----Original Message-----
> From: Sean Mullan <sean.mullan at oracle.com>
> Sent: Freitag, 10. August 2018 17:39
> To: Baesken, Matthias <matthias.baesken at sap.com>; Chris Hegarty
> <chris.hegarty at oracle.com>; Alan Bateman <Alan.Bateman at oracle.com>
> Cc: core-libs-dev at openjdk.java.net; security Dev OpenJDK <security-
> dev at openjdk.java.net>
> Subject: Re: [RFR] 8205525 : Improve exception messages during manifest
> parsing of jar archives
>
> I need more time to finish my review but here are some initial comments:
>
> - java.security
>
> 1079 # jarpath - enables more detailed information in the IOExceptions
> thrown
>
> Use "jarPath" to be consistent with "hostInfo".
>
> 1080 # by java.util.jar.Attributes and java.util.jar.Manifest
>
> and java.util.jar.JarFile
>
> - Manifest.java
>
> 57 private String jarFilename = null;
>
> Not necessary to set it to null, as that is the default.
>
> 82 Manifest(InputStream is, String jarFilename) throws IOException {
> 83 this.jarFilename = jarFilename;
> 84 read(is);
> 85 }
>
> Read from the InputStream and check for error conditions *before*
> setting jarFilename (switch lines 83 & 84). This is a general best
> practice but can also protect against finalizer attacks. See JSCG 7-3
> for more info:
> https://www.oracle.com/technetwork/java/seccodeguide-139067.html#7
>
> - Attributes.java
>
> As Chris and Alan mentioned, you should move the parsing of the property
> to a more general location so it can be used by other code that uses
> this property.
>
> --Sean
>
> On 8/8/18 11:00 AM, Sean Mullan wrote:
> > Cross-posting to security-dev since this fix is security related and
> > should also be reviewed there.
> >
> > --Sean
> >
> > On 8/7/18 11:00 AM, Baesken, Matthias wrote:
> >> Ping .... , any reviews / comments ?
> >>
> >> Thanks , Matthias
> >>
> >>> -----Original Message-----
> >>> From: Baesken, Matthias
> >>> Sent: Dienstag, 31. Juli 2018 12:28
> >>> To: 'Chris Hegarty' <chris.hegarty at oracle.com>; Alan Bateman
> >>> <Alan.Bateman at oracle.com>
> >>> Cc: core-libs-dev at openjdk.java.net; Lindenmaier, Goetz
> >>> <goetz.lindenmaier at sap.com>; Langer, Christoph
> >>> <christoph.langer at sap.com>
> >>> Subject: RE: [RFR] 8205525 : Improve exception messages during
> manifest
> >>> parsing of jar archives
> >>>
> >>> Hello ,
> >>> looks like the generalization of the `includeInExceptions`
> >>> security property
> >>> is now in jdk/jdk after
> >>>
> >>> "8207846: Generalize the jdk.net.includeInExceptions security property"
> >>>
> >>> was added, great news and thanks to Chris for driving this !
> >>>
> >>> I use this security property now as well , and updated the change :
> >>>
> >>> http://cr.openjdk.java.net/~mbaesken/webrevs/8205525.3/
> >>>
> >>> I updated the CSR as well :
> >>>
> >>> https://bugs.openjdk.java.net/browse/JDK-8207768
> >>>
> >>>
> >>> Best regards, Matthias
> >>>
> >>>
> >>>
> >>>> -----Original Message-----
> >>>> From: Chris Hegarty <chris.hegarty at oracle.com>
> >>>> Sent: Donnerstag, 19. Juli 2018 14:54
> >>>> To: Alan Bateman <Alan.Bateman at oracle.com>; Baesken, Matthias
> >>>> <matthias.baesken at sap.com>
> >>>> Cc: core-libs-dev at openjdk.java.net; Lindenmaier, Goetz
> >>>> <goetz.lindenmaier at sap.com>
> >>>> Subject: Re: [RFR] 8205525 : Improve exception messages during
> manifest
> >>>> parsing of jar archives
> >>>>
> >>>>
> >>>>> On 19 Jul 2018, at 11:46, Alan Bateman <Alan.Bateman at oracle.com>
> >>>> wrote:
> >>>>>
> >>>>> On 19/07/2018 09:07, Baesken, Matthias wrote:
> >>>>>> Hello, in the meantime I prepared a CSR :
> >>>>>>
> >>>>>> https://bugs.openjdk.java.net/browse/JDK-8207768
> >>>>>>
> >>>>>>
> >>>>>>> jdk.includeInExceptions expands the scope. That might be okay but
> we
> >>>>>>> will need to re-visit jdk.net.includeInExceptions and also move the
> >>>>>>> support to somewhere in jdk.internal so that it can be used by
> other
> >>>>>>> code in java.base.
> >>>>>> Is there some support code for " jdk.net.includeInExceptions "
> >>>>>> or do
> >>>> you just mean the name of the property ?
> >>>>>>
> >>>>> Chris is right that it's a bit premature to submit a CSR while the
> >>>>> question
> >>> on
> >>>> whether to rename the existing security property is on the table. I
> >>>> have no
> >>>> objection to doing that.
> >>>>
> >>>> I filed the following issue to generalize the `includeInExceptions`
> >>>> security
> >>>> property:
> >>>> https://bugs.openjdk.java.net/browse/JDK-8207846
> >>>>
> >>>> I would like to resolve 8207846 first, then 8205525 can propose to
> >>>> add the
> >>>> new argument value, `jarPath`.
> >>>>
> >>>> -Chris.
More information about the core-libs-dev
mailing list