---------------------
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.cof...@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.cof...@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.cof...@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.ander...@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.ander...@oracle.com
>>
>>
>>
