RFR JDK-8206925,,Support the certificate_authorities extension

Xuelei Fan xuelei.fan at oracle.com
Fri May 22 17:55:20 UTC 2020


All good comments.  I updated the code and CSR accordingly.
    http://cr.openjdk.java.net/~xuelei/8206925/webrev.05/

On 5/22/2020 8:41 AM, Sean Mullan wrote:
> On 5/15/20 6:11 PM, Xuelei Fan wrote:
>> New webrev: http://cr.openjdk.java.net/~xuelei/8206925/webrev.04/
> 
> * 
> src/java.base/share/classes/sun/security/ssl/CertificateAuthoritiesExtension.java 
> 
> 
>   212                     SSLLogger.warning(
>   213                             "Too much certificate authorities to 
> use " +
>   214                             "the certificate_authorities extension");
> 
> How about: "The number of CAs exceeds the maximum size of the 
> certificate_authorities extension"
> 
> * test/jdk/sun/security/ssl/X509KeyManager/CertificateAuthorities.java
> 
> The test is still using the previous property name: 
> jdk.tls.client.indicateCertificateAuthorities
> 
> * test/jdk/sun/security/ssl/X509TrustManagerImpl/TooMuchCAs.java
> 
> Will this test FAIL if we ever exceed the maximum number of CAs? I think 
> it is important that it does FAIL, as the extension is effectively not 
> working anymore and could cause compatibility issues. I even think we 
> would need to try to think of some way to fix it, either by seeing if 
> some CAs could be excluded - not really sure, hopefully it won't ever 
> happen but we would want to know about it in advance.
> 
Alexey (from azul) and I discussed the idea to control the number of 
CAs.  However, there are still some issues in practice.

"If the certificate authorities can not be fully listed, it cannot be 
used to indicate the peer certificate selection accuracy.  For example, 
client support A, B and C, and is only able to indicate A and B.  If the 
server supports C, the connection cannot be established with this 
extension. This is not the expected behavior.  Maybe, it is no worse 
than without this extension. "

It looks like safer that the extension is not used if the size exceed 
the limit, at least there ARE less compatibility issues.  I have a note 
in the CSR and release note for the behaviors.

The test case, TooMuchCAs, is used to make sure the connection can be 
established when the CAs size exceed the limit (no extension used).

Xuelei


> * src/java.base/share/classes/sun/security/ssl/SSLExtension.java
> 
> Some typos and wording suggestions:
> 
> 750             // Note: Plese be careful to enable this extension in 
> ClientHello.
> 
> Typo: Please
> 
>   757             // untrusted server certificate. If this extension is 
> enabled in
>   758             // the ClientHello handshake message, the server does 
> not send
>   759             // certificate which is not indicated in the 
> extension.  The server
>   760             // will just terminate handshake and close the 
> connection.  Then
> 
> A few more details would be useful above - how about combining the 2 
> sentences as:
> 
> "If this extension is enabled in the ClientHello handshake message, and 
> the server's certificate does not chain back to any of the CAs in the 
> extension, then the server will terminate the handshake and close the 
> connection."
> 
> 761             // there is no chance for client to perform the manually 
> checking.
> 
> "the client"
> s/manually checking/manual check/
> 
> 762             // Therefore, enable this extension in ClientHello may 
> lead to
> 
> s/enable/enabling/
> 
> 767             // maximum TLS record size is 2^14 bytes.  In case of 
> handshake
> 
> s/In case of/If the/
> 
> 768             // message is bigger then maximum TLS record size, it 
> should be
> 
> s/then/than the/
> 
> 770             // implementations does not allow ClientHello message 
> bigger than
> 
> s/does/do/
> s/message/messages/
> 
> 771             // the maximum TLS record size and aborts connection 
> immediately
> 
> s/aborts connection immediately/will immediately abort the connection/
> 
> 772             // with a fatal alert.  Therefore, if the client accepts 
> too much
> 
> s/accepts too much/trusts too many/
> 
> 775             // Further more, if the client accepts too much CAs to 
> meet the
> 776             // size limit of the extension, enabling this extension 
> in client
> 
> s/Further more/Furthermore/
> s/accepts too much CAs to meet the/trusts more CAs such that it exceeds 
> the/
> 
> --Sean
> 
>>
>> On 5/15/2020 5:27 AM, Sean Mullan wrote:
>>> On 5/15/20 1:22 AM, Xuelei Fan wrote:
>>>> Alexey has some good point about the size limit of the extension.  I 
>>>> added more comments about the compatibility impact and interop 
>>>> impact when there is too much CAs to meet the size limits in CSR, 
>>>> source code and release notes.
>>>>
>>>> New webrev: https://bugs.openjdk.java.net/browse/JDK-8244460
>>>>
>>>> I have not had a chance to add a negative test case yet.
>>>
>>> By negative test case, do you mean trying to exceed the maximum 
>>> number of CAs? I agree that would be a good test to add, as we may or 
>>> may not exceed that number some day, but it would be good to know 
>>> when/if we do.
>>>
>> Yes.  I added a new test case in webrev.04 (TooMuchCAs.java).
>>
>> Xuelei
>>
>>> --Sean
>>>
>>>>
>>>> On 5/14/2020 1:38 PM, Sean Mullan wrote:
>>>>> For the CSR, why did you check the binary and behavioral boxes for 
>>>>> compatibility risk?
>>>> I should not check the boxes.  Removed.
>>>>
>>>> Thanks,
>>>> Xuelei
>>>>
>>>>> Otherwise it looks good, and I added my name as Reviewer. I will 
>>>>> review the updated webrev later.
>>>>>
>>>>> Please file and add a link to a docs issue to document the new 
>>>>> system property.
>>>>>
>>>>> --Sean
>>>>>
>>>>> On 5/13/20 5:20 PM, Xuelei Fan wrote:
>>>>>>
>>>>>> On 5/13/2020 2:11 PM, Sean Mullan wrote:
>>>>>>>>>> It is not expected to use this extension regularly.
>>>>>>>>>>
>>>>>>>>>> Please let me know if you still prefer to use 
>>>>>>>>>> "enableCAExtension".
>>>>>>>>>>
>>>>>>>>>>> Also, it is a bit unfortunate that we have to have a system 
>>>>>>>>>>> property to enable it. Can we not enable it based on whether 
>>>>>>>>>>> the configured X509TrustManager.getAcceptedIssuers returns a 
>>>>>>>>>>> non-empty list?
>>>>>>>>>>>
>>>>>>>>>> We can do that on server side, but there are compatibility 
>>>>>>>>>> impact on client behavior if we did it in client side.  See #2 
>>>>>>>>>> in the "Specification" section.
>>>>>>>>>
>>>>>>>>> But doesn't the default JDK PKIX TrustManager throw a fatal 
>>>>>>>>> exception and close the connection if the server's certificate 
>>>>>>>>> cannot be validated? Could we check if the PKIX TrustManager is 
>>>>>>>>> being used?
>>>>>>>>>
>>>>>>>> Yes, the trust manager could throw a fatal exception and close 
>>>>>>>> the connection if the trust cannot be established.  The fallback 
>>>>>>>> mechanism is implemented in the customized trust manager, that 
>>>>>>>> if users accept the cert, the cert is trusted, and no exception 
>>>>>>>> and the handshaking continued.  It is too later to fallback 
>>>>>>>> after the connection closed.
>>>>>>>
>>>>>>>>> If a client wants to accept self-signed or untrusted server 
>>>>>>>>> certificates, I would have expected them to have to use a 
>>>>>>>>> custom X509TrustManager that allows that, and that 
>>>>>>>>> getAcceptedIssuers() should return an empty List. Is that not 
>>>>>>>>> is what is typically done in practice?
>>>>>>>>>
>>>>>>>> Yes, customized trust manager is used to accept users manually 
>>>>>>>> selection.  As the users may also want to accept normal 
>>>>>>>> certificate without manually involved, so getAcceptedIssuers() 
>>>>>>>> should respect those CA as well.
>>>>>>>
>>>>>>> I see. Out of curiosity, have you checked how other 
>>>>>>> implementations handle this extension? For web browsers, they 
>>>>>>> typically give the user the option of proceeding if the server 
>>>>>>> certificate is not trusted. Seems to be a bit of a configuration 
>>>>>>> dilemma as you may want this extension enabled for certain sites 
>>>>>>> that have multiple certificates, but not as a general default 
>>>>>>> because then you wouldn't be able to connect to untrusted sites 
>>>>>>> (at your own risk of course). I wonder if it would have been 
>>>>>>> better for the RFC to allow the server to treat this extension 
>>>>>>> more as a hint, and still return its chain if an acceptable 
>>>>>>> certificate could not be found.
>>>>>> If it is treated as a hint, then it might be better no have this 
>>>>>> extension.
>>>>>>
>>>>>> I checked with browsers, the extension is not present in 
>>>>>> ClientHello. For JDK, I would not expect a lot use of this 
>>>>>> extension in client side. It is just designed to workaround a few 
>>>>>> cases, just as you mentioned above.
>>>>>>
>>>>>> Xuelei



More information about the security-dev mailing list