[RFR] 8205525 : Improve exception messages during manifest parsing of jar archives (original) (raw)
Baesken, Matthias matthias.baesken at sap.com
Fri Aug 24 14:52:59 UTC 2018
- Previous message: [RFR] 8205525 : Improve exception messages during manifest parsing of jar archives
- Next message: [RFR] 8205525 : Improve exception messages during manifest parsing of jar archives
- Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]
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 theincludeInExceptions
>>>> 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.
- Previous message: [RFR] 8205525 : Improve exception messages during manifest parsing of jar archives
- Next message: [RFR] 8205525 : Improve exception messages during manifest parsing of jar archives
- Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]