RFR: 8218021: Have jarsigner preserve posix permission attributes

[Weijun Wang]

---------------------
src/java.base/share/classes/sun/security/provider/certpath/DistributionPointFetcher.java
@@ -248,7 +248,7 @@ private static X509CRL getCRL(URIName name) throws CertStoreException {
    debug.println("Trying to fetch CRL from DP " + uri);        
    }        

    Event.report("event.crl.check", uri.toString());        
    Event.report(Event.ReporterCategory.CRLCHECK,"event.crl.check", uri.toString());        
 
Please add a whitespace after CRLCHECK.
---------------------
src/java.base/share/classes/sun/security/provider/certpath/OCSP.java
@@ -234,7 +234,7 @@ static OCSPResponse check(List<CertId> certIds, URI responderURI,
    debug.println("connecting to OCSP service at: " + url);        
    }        

    Event.report("event.ocsp.check", url.toString());        
    Event.report(Event.ReporterCategory.CRLCHECK,"event.ocsp.check", url.toString());        
 
White space after CRLCHECK.
--------------------- 
src/jdk.jartool/share/classes/jdk/security/jarsigner/JarSigner.java
    int perms = JUZFA.getPosixPerms(ze);        
    if (!posixPermsDetected && perms != -1) {        
        posixPermsDetected = true;        
        Event.report(Event.ReporterCategory.POSIXPERMS, "true");        
 
The method signature is "report(category, type, values...)". There is no need to define a type or value here but I feel like more normal to call 'report(POSIXPERMS, "detected")' because the 2nd argument is type instead of value.
---------------------
src/jdk.jartool/share/classes/sun/security/tools/jarsigner/Main.java
@@ -1054,7 +1063,7 @@ private void displayMessagesAndResult(boolean isSigning) {
    notYetValidCert || chainNotValidated || hasExpiredCert ||        
    hasUnsignedEntry || signerSelfSigned || (legacyAlg != 0) ||        
    (disabledAlg != 0) || aliasNotInStore || notSignedByAlias ||        
    tsaChainNotValidated || permsDetected ||        
 
I'd prefer move the permsDetected check and the if block for it out of this big block, since it's not a fatal warning and just informational. You can move it into "if (hasExpiringCert....".
---------------------
src/jdk.jartool/share/classes/sun/security/tools/jarsigner/Main.java
@@ -1805,6 +1820,7 @@ void signJar(String jarName, String alias)
        fos.close();        
    }        

    Event.clearReportListener(Event.ReporterCategory.POSIXPERMS);        
 
Maybe in a final block?
---------------------
test/jdk/sun/security/util/Resources/Usages.java
@@ -75,7 +75,7 @@
    "(?m)rb[ \\n]*\\.getString[ \\n]*\\([ \\n]*\"(.*?)\"\\)");        

    static Pattern EVENT_OCSP_CRL = Pattern.compile(        
            "Event\\.report\\(.*,\"(.*?)\",");        
 
If you agree to add a whitespace in the calls in OSCP.java and DistributionFetcher.java, you might want to add a whitespace (or "\s*") here as well.

Thanks,
Max


> On Jun 30, 2020, at 9:51 PM, Seán Coffey <sean.coffey at oracle.com> wrote:
> 
> Thanks Lance.
> 
> During the CSR review, a suggestion was made to have jarsigner preserve such attributes by default. Warnings about these attributes will also be added during signing and verify operations (if detected).
> 
> webrev: https://cr.openjdk.java.net/~coffeys/webrev.8218021.v4/webrev/
> 
> regards,
> Sean.
> 
> On 22/06/2020 17:17, Lance Andersen wrote:
>> HI Sean,
>> 
>> Looks OK based on our exchanges.  Thank you for your time on this one!
>> 
>> Best
>> Lance
>> 
>>> On Jun 22, 2020, at 7:22 AM, Seán Coffey <sean.coffey at oracle.com> wrote:
>>> 
>>> Thanks Lance.
>>> 
>>> I've updated the patch with some extra offline feedback from yourself and Max.
>>> A new warning is printed with use of the new flag. A warning is also printed when file posix permissions are detected on resources being signed. Test updated for that also.
>>> 
>>> https://cr.openjdk.java.net/~coffeys/webrev.8218021.v3/webrev/
>>> 
>>> regards,
>>> Sean.
>>> 
>>> On 12/06/2020 17:05, Lance Andersen wrote:
>>>> Hi Sean,
>>>> 
>>>> I think your changes look fine so all good FMPOV.
>>>> 
>>>> Best
>>>> Lance
>>>> 
>>>>> On Jun 12, 2020, at 6:21 AM, Seán Coffey <sean.coffey at oracle.com> wrote:
>>>>> 
>>>>> Hi,
>>>>> 
>>>>> I'd like to reboot this jarsigner enhancement request[1]. I've removed the problem references to zip file name extensions. Instead, there's a new JDK implementation specific jarsigner option: -keepposixperms
>>>>> 
>>>>> https://bugs.openjdk.java.net/browse/JDK-8218021
>>>>> https://cr.openjdk.java.net/~coffeys/webrev.8218021.v2/webrev/
>>>>> 
>>>>> regards,
>>>>> Sean.
>>>>> 
>>>>> [1] http://mail.openjdk.java.net/pipermail/security-dev/2020-January/021141.html
>>>>> 
>>>> 
>>>> <oracle_sig_logo.gif>
>>>> 
>>>> Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037
>>>> Oracle Java Engineering 
>>>> 1 Network Drive 
>>>> Burlington, MA 01803
>>>> Lance.Andersen at oracle.com
>>>> 
>>>> 
>>>> 
>> 
>> <oracle_sig_logo.gif>
>> 
>> Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037
>> Oracle Java Engineering 
>> 1 Network Drive 
>> Burlington, MA 01803
>> Lance.Andersen at oracle.com
>> 
>> 
>>