RFR [13] JDK-4919790 : Errors in alert ssl message does not reflect the actual certificate status

[Jamil Nimeh]

On 2/14/2019 10:24 AM, Sean Mullan wrote:
> On 2/11/19 2:32 PM, Xuelei Fan wrote:
>> Hi,
>>
>> Could I get the update reviewed?
>>     http://cr.openjdk.java.net/~xuelei/4919790/webrev.00/
>
> 721                     alert = Alert.UNSUPPORTED_CERTIFCATE;
>
> Can we fix this typo while we are cleaning this up? 
> s/CERTIFCATE/CERTIFICATE/
>
> Also, I was a bit curious about these lines (not part of your fix):
>
>  711                 if (reason == BasicReason.REVOKED) {
>  712                     alert = chc.staplingActive ?
>  713                             Alert.BAD_CERT_STATUS_RESPONSE :
>  714                             Alert.CERTIFICATE_REVOKED;
>
> If a certificate is revoked, why would you set the alert status to 
> BAD_CERT_STATUS_RESPONSE if stapling is enabled?
I think that was something I did back when we did OCSP stapling in JDK 
9.  IIRC, RFC 6066 says that clients receiving stapled responses must 
check them and alert with bad_certificate_status_response "if the 
response is not satisfactory" (their words, not mine).  Back then I 
interpreted "unsatisfactory" to mean both structurally wrong or 
revoked.  Perhaps that was too broad an interpretation.

It's probably worth seeing how a couple other clients handle this case.  
I know I had tested with them back in pre-JDK9 days, but I don't recall 
the results.  I might have some old packet captures lying about still 
also, I just need to dig them up.
>
> Also, bug needs a noreg label.
>
> --Sean
>
>> It had been a while that the SunJSSE provider use certificate_unknown 
>> or certificate_revoked (or bad_certificate_status_response for OCSP 
>> stapling) as the certificate issues alert.  Other certificate alert 
>> like certificate_expired are not used.
>>
>> The bug was reported in JDK 6.  With the introducing of 
>> CertPathValidatorException.BasicReason in JDK 7. Now we can handle 
>> the alert more accuracy.
>>
>> Note: please don't rely on the certificate alert type for application 
>> development.  The alert type may be changed and different per the 
>> provider preference.
>>
>> No new regression test as the update is simple and straightforward.
>>
>> Thanks,
>> Xuelei