RFR (XS) : 8133489: Better messaging for PKIX path validation matching

[Xuelei Fan]

Looks good to me.

Thanks,
Xuelei

On 6/20/2019 8:45 AM, Seán Coffey wrote:
> Thanks for the review Xuelei,
> 
> edits made:
> 
> --- a/src/java.base/share/classes/java/security/cert/X509CertSelector.java
> +++ b/src/java.base/share/classes/java/security/cert/X509CertSelector.java
> @@ -2115,8 +2115,11 @@
>               if (certSubjectKeyID == null ||
>                       !Arrays.equals(subjectKeyID, certSubjectKeyID)) {
>                   if (debug != null) {
> -                    debug.println("X509CertSelector.match: "
> -                        + "subject key IDs don't match");
> +                    debug.println("X509CertSelector.match: subject key 
> IDs " +
> +                        "don't match\nX509CertSelector.match: 
> subjectKeyID: " +
> +                        Arrays.toString(subjectKeyID) +
> +                        "\nX509CertSelector.match: certSubjectKeyID: " +
> +                        Arrays.toString(certSubjectKeyID));
>                   }
>                   return false;
> 
> regards,
> Sean.
> 
> On 20/06/2019 15:14, Xuelei Fan wrote:
>> On 6/20/2019 5:56 AM, Seán Coffey wrote:
>>> A simple debugging enhancement to print out subjectkey ID details 
>>> when mismatch is encountered. I encountered a DER encoding issue with 
>>> an application server team a good while back and needed such a patch 
>>> to debug the issue correctly. I added -Djava.security.debug=certpath 
>>> to a testcase which tests this functionality. Sample output :
>>>
>>> certpath: X509CertSelector.match: subject key IDs don't match
>>> certpath: 509CertSelector.match: subjectKeyID: [4, 20, -12, -2, 115, 
>>> 79, -15, 106, 114, -58, 102, 43, 32, 26, 120, -76, -33, 50, -45, -56, 
>>> -16, -38]
>>> certpath: 509CertSelector.match: certSubjectKeyID: [4, 20, -111, 93, 
>>> -48, -86, -39, 59, -128, -118, 45, -10, 126, -76, -115, 126, -99, 
>>> -106, -116, 107, 124, -63]
>>>
>>> regards,
>>> Sean.
>>>
>>> diff --git 
>>> a/src/java.base/share/classes/java/security/cert/X509CertSelector.java b/src/java.base/share/classes/java/security/cert/X509CertSelector.java 
>>>
>>> --- 
>>> a/src/java.base/share/classes/java/security/cert/X509CertSelector.java
>>> +++ 
>>> b/src/java.base/share/classes/java/security/cert/X509CertSelector.java
>>> @@ -2117,6 +2117,10 @@
>>>                   if (debug != null) {
>>>                       debug.println("X509CertSelector.match: "
>>>                           + "subject key IDs don't match");
>>> +                    debug.println("509CertSelector.match:" +
>>> +                        " subjectKeyID: " + 
>>> Arrays.toString(subjectKeyID));
>>> +                    debug.println("509CertSelector.match:" +
>>> +                        " certSubjectKeyID: " + 
>>> Arrays.toString(certSubjectKeyID));
>>>                   }
>>>                   return false;
>>>               }
>> Is it a typo "509CertSelector" -> "X509CertSelector"?
>>
>> I may use one call to debug.println() in case the information are 
>> separated in multi-thread environment.
>>
>> Otherwise, looks good to me.
>>
>> Thanks,
>> Xuelei
>>
>>> diff --git 
>>> a/test/jdk/java/security/cert/CertPathBuilder/selfIssued/KeyUsageMatters.java 
>>> b/test/jdk/java/security/cert/CertPathBuilder/selfIssued/KeyUsageMatters.java 
>>>
>>> --- 
>>> a/test/jdk/java/security/cert/CertPathBuilder/selfIssued/KeyUsageMatters.java 
>>>
>>> +++ 
>>> b/test/jdk/java/security/cert/CertPathBuilder/selfIssued/KeyUsageMatters.java 
>>>
>>> @@ -29,13 +29,13 @@
>>>
>>>   /**
>>>    * @test
>>> - * @bug 6852744
>>> + * @bug 6852744 8133489
>>>    * @summary PIT b61: PKI test suite fails because self signed 
>>> certificates
>>>    *          are being rejected
>>>    * @modules java.base/sun.security.util
>>> - * @run main/othervm KeyUsageMatters subca
>>> - * @run main/othervm KeyUsageMatters subci
>>> - * @run main/othervm KeyUsageMatters alice
>>> + * @run main/othervm -Djava.security.debug=certpath KeyUsageMatters 
>>> subca
>>> + * @run main/othervm -Djava.security.debug=certpath KeyUsageMatters 
>>> subci
>>> + * @run main/othervm -Djava.security.debug=certpath KeyUsageMatters 
>>> alice
>>>    * @author Xuelei Fan
>>>    */
>>>