RFR JDK-8236039: JSSE Client does not accept status_request extension in CertificateRequest messages for TLS 1.3

[Jamil Nimeh]

Hi Xuelei, I backed out my change and went solely with the approach you 
suggested below.  It works in that it allows the handshake to proceed.  
I notice these debug log messages though during consumption of the CR 
message from the server:

javax.net.ssl|DEBUG|01|main|2020-01-04 15:02:26.636 
PST|SSLExtensions.java:135|Ignore unknown or unsupported extension (
"unknown extension (5)": {

}
...
javax.net.ssl|DEBUG|01|main|2020-01-04 15:02:26.656 
PST|CertificateRequest.java:926|Consuming CertificateRequest handshake 
message (
"CertificateRequest": {
   "certificate_request_context": "",
   "extensions": [
     "unknown extension (5)": {

     },
     "unknown extension (18)": {

     },
     "signature_algorithms (13)": {
       "signature schemes": [rsa_pss_rsae_sha256, 
ecdsa_secp256r1_sha256, ed25519, rsa_pss_rsae_sha384, 
rsa_pss_rsae_sha512, rsa_pkcs1_sha256, rsa_pkcs1_sha384, 
rsa_pkcs1_sha512, ecdsa_secp384r1_sha384, ecdsa_secp521r1_sha512, 
rsa_pkcs1_sha1, ecdsa_sha1]
     },
     "unknown extension (47)": {
       0000: 00 29 00 27 30 25 31 10   30 0E 06 03 55 04 0A 0C  
.).'0%1.0...U...
       0010: 07 54 65 73 74 50 4B 49   31 11 30 0F 06 03 55 04  
.TestPKI1.0...U.
       0020: 03 0C 08 54 65 73 74 52   6F 6F 74                 ...TestRoot
     }
   ]
}
)

In the debug logs, it seems like we shouldn't call a status_request 
extension an "unknown extension" because we know what it is and can 
parse it.  Having the entry in SSLExtension for CR_STATUS_REQUEST and a 
simple do-nothing consumer gets rid of the status_request/unknown 
messages and has it render in the printing of the CR message properly.

While we're talking about the debug logs we have other extensions that 
we know about and have entries in SSLExtension, but they have no 
per-message registration.  You can see in the CR above that 18 
(signed_certificate_timestamp) and 47 (certificate_authorities) are 
extensions that we were aware of enough to put basic entries in 
SSLExtension for, but just didn't put full consumer/producer support in 
there.

It might be nice to use the display approach that we have for an 
unsupported extension for those extensions we at least know about, but 
rather than say "unknown extension" at the start of the printing at 
least give the name.  Let me see if I can make that work without it 
being too invasive.

--Jamil

On 1/4/20 9:27 AM, Xuelei Fan wrote:
> CertStatusExtension.java
> ------------------------
> 1239             // The consuming happens in server side only.
> typo? server -> client
>
> It wold be nice to add a debug log that this extension get ignored.  
> But may not need to define this consumer as it is not supported.
>
> SSLExtension.java
> -----------------
> As this fix does not implement this feature yet, is it possible to 
> just define it without the on-load consumer?
>
> Otherwise, it looks fine to me.
>
> BTW, this issue reminders me a common problem: if an extension is 
> supported in a handshake message, we need to know all other handshake 
> messages that could use the extension, and define an SSLExtension enum 
> for them.  Otherwise, a similar issue could happen.  I think it would 
> be challenge to know that in practice.
>
> So I was just wondering, could we just release the check in the 
> SSLExtensions.java implementation (from line 71 to 95)?  If the 
> extension for the specific handshake type is not defined, just ignore 
> it, except for ServerHello?  I think the behavior complies to the TLS 
> 1.3 protocol.
>
> if (SSLExtension.isConsumable(extId) &&
>     SSLExtension.valueOf(handshakeType, extId) == null) {
>     ...
> -   } else {
> +   } else if (handshakeType == SSLHandshake.SERVER_HELLO) {
>         throw hm.handshakeContext.conContext.fatal(
>               Alert.UNSUPPORTED_EXTENSION,
>               "extension (" + extId +
>               ") should not be presented in " + handshakeType.name);
> +   } else {
> +       isSupported = false;
> +       // debug log to ignore unknown extension for handshakeType
>     }
> }
>
> Xuelei
>
> On 1/3/2020 10:06 AM, Jamil Nimeh wrote:
>> Hi All, the golang folks have been running into an issue where our 
>> JSSE client treats the status_request extension in a 
>> CertificateRequest message from a golang server as an unknown 
>> extension and alerts.  This quick fix will allow the client to read 
>> and accept the extension and proceed.  I believe you need golang 
>> 1.13.x to see this take place.
>>
>> This fix does not implement client-side OCSP stapling.  That will be 
>> an RFE for another day.
>>
>> Bug: https://bugs.openjdk.java.net/browse/JDK-8236039
>>
>> Webrev: https://cr.openjdk.java.net/~jnimeh/reviews/8236039/webrev.01/
>>
>> --Jamil
>>