Bug 187679

Summary: [Curl] Allow request to have white list of certificate chains for specific host.
Product: WebKit Reporter: Basuke Suzuki <Basuke.Suzuki>
Component: PlatformAssignee: Nobody <webkit-unassigned>
Status: RESOLVED INVALID    
Severity: Normal CC: achristensen, Basuke.Suzuki, Hironori.Fujii
Priority: P2    
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
See Also: https://bugs.webkit.org/show_bug.cgi?id=191646
Bug Depends on: 187611, 191647    
Bug Blocks: 191646    

Description Basuke Suzuki 2018-07-14 12:18:03 PDT
Curl port doesn't have implementation of allowSpecificHTTPSCertificateForHost right now. This patch adds support for that on WebCore curl network layer and interfaces in WebKit network layer.
Comment 1 Alex Christensen 2018-11-17 07:36:32 PST
We intend to remove allowSpecificHTTPSCertificateForHost in favor of decidePolicyForAuthenticationChallenge.  Please don't add another one.  If you want to allow an invalid certificate, please have the application just say so.
Comment 2 Basuke Suzuki 2018-11-18 20:29:25 PST
So without this api, how does app handle insecure certificate? What is decidePolicyForAuthenticationChallenge?
Comment 3 Alex Christensen 2018-11-26 09:24:16 PST
When a TLS handshake happens, the app is asked whether it wants to continue or not.  The default behavior is to continue if the CA is trusted and everything checks out, but when asked the app has the ability to say continue even though it is not a good certificate.
Comment 4 Alex Christensen 2018-11-26 09:27:09 PST
In our API, the asking is done through decidePolicyForAuthenticationChallenge, and the challenge would have this: https://developer.apple.com/documentation/foundation/nsurlauthenticationmethodservertrust?language=objc

Challenges are also used for client certificate authentication.  I'm not sure how curl supports that.
Comment 5 Basuke Suzuki 2018-11-27 12:34:36 PST
I cannot get any information about `decidePolicyForAuthenticationChallenge` from Apple document or WebKit repository, but does that mean 
 URLSession's URLSession:task:didReceiveChallenge:completionHandler: delegete method?

Let me guess the underlying implementation of Safari based on above delegate.  

1) First time connection hit a invalid certificate(s), it invokes server trust evaluation through the authentication manager to ask the client app to validate this invalid certificate(s).

2) The app will ask user to trust or not to trust.

3) If a user choose to trust the certificate, it stores the certificate for a host in app's storage for further reference and call completionHandler with dispositon of useCredential for server trust.

4) When next communication to the same host happens, again it invoke server trust evaluation to the app and the app compares certificates with its permitted certificate list. If it's in the list, it allow communication by calling completion handler immediately. If not, go to 2).

Is this correct? There's a huge communication overhead via IPC calls with this way.
Comment 6 Alex Christensen 2018-11-27 12:40:42 PST
That is correct, except it does this for every TLS handshake, whether it has valid or invalid certificates.
Comment 7 Basuke Suzuki 2018-11-27 12:50:20 PST
(In reply to Alex Christensen from comment #6)
> That is correct, except it does this for every TLS handshake, whether it has
> valid or invalid certificates.

Ah, got it. Thanks for clarification. We'll change our implementation.
Comment 8 Basuke Suzuki 2018-11-27 16:15:49 PST
Unfortunately there's no way to continue curl connection once it detect verification error. It has to be restarted from the beginning.

Our initial idea is that CurlContext has global exceptional list for each (host, certificates) pair to allow exception while validating in a callback of OpenSSL to prevent validation error reported to libcurl.

To ignore verification error not using above way, it is possible to disable validation for specific session, but it doesn't check the received certificate. Any certificate sent from the server is accepted for that connection. It should be ignore specific certificates for the host.

Other idea is that a request can have local exceptional certificate list. Then when we get a ServerTrustRequest and the user allow to communicate to server for the certificate, the request will be restarted from the beginning with the exceptional certificates. It's a little bit complecated than original, but it may work.

Any way, Curl port needs some sort of allowSpecificHTTPSCertificateForHost feature in WebKit layer to support Server Trust Evaluation. Can we reopen this bug?
Comment 9 Alex Christensen 2018-11-27 16:19:33 PST
How do you intend to support client certificates?  Do you intend to not support them?  Will you need to say up front what client certs to use, or will you want to be able to fetch them as needed?
Comment 10 Basuke Suzuki 2018-11-27 17:07:22 PST
ClientCertificate is not the high priority for our platform, but WinCairo has supported it when we came to this platform. It requires to set up front.
Comment 11 Basuke Suzuki 2018-11-27 17:09:36 PST
Better idea will be sending a patch to libcurl for adding a callback to allow policy decision to the client. I will open that PR to them. But it takes time. We promise we will replace the implementation once libcurl has this feature.
Comment 12 Alex Christensen 2018-11-27 20:07:06 PST
Wow, that would be great.  Another approach could be to listen for handshake failures and when one happens because of the cert, ask the client if it's ok to proceed and retry if they say it's ok.

Client certificates might not be a priority right now, but they may become a priority.
Comment 13 Fujii Hironori 2018-11-27 21:01:44 PST
FWIW, theoretically it is possible to post decidePolicyForAuthenticationChallenge callback while verify_callback is called by blocking the curl thread.

(In reply to Basuke Suzuki from comment #8)
> To ignore verification error not using above way, it is possible to disable
> validation for specific session, but it doesn't check the received
> certificate. Any certificate sent from the server is accepted for that
> connection. It should be ignore specific certificates for the host.

This is a bad idea. User agents shouldn't continue a handshake and receive data without user consent of accepting the invalid cert.
Comment 14 Basuke Suzuki 2018-11-28 10:26:53 PST
(In reply to Fujii Hironori from comment #13)
> FWIW, theoretically it is possible to post
> decidePolicyForAuthenticationChallenge callback while verify_callback is
> called by blocking the curl thread.

We can pause it, but then entire curl thread may be blocked. Currently there's no way to pause specific curl communication while curl is trying to connect to the ssl server. There is a similar situation while getting a header via curl callback and pausing from such callback is officially allowed without blocking entire thread. The idea above to send a patch to libcurl may be similar approach.


> (In reply to Basuke Suzuki from comment #8)
> > To ignore verification error not using above way, it is possible to disable
> > validation for specific session, but it doesn't check the received
> > certificate. Any certificate sent from the server is accepted for that
> > connection. It should be ignore specific certificates for the host.
> 
> This is a bad idea. User agents shouldn't continue a handshake and receive
> data without user consent of accepting the invalid cert.

Of cause this must happen after user's confirmation. This describes how to ignore invalid certificate after user allow us to communicate to the server. But still, as I said in the last sentence, ignoring entire validation for the host is not good idea. It should be checked with (host, certificates) pair which is allowed by the user.
Comment 15 Basuke Suzuki 2018-11-28 10:48:52 PST
(In reply to Alex Christensen from comment #12)
> Wow, that would be great.  Another approach could be to listen for handshake
> failures and when one happens because of the cert, ask the client if it's ok
> to proceed and retry if they say it's ok.

Yes, that is ideal, but we cannot pause the communication while validation is ongoing (our code is in OpenSSL callback which is called by libcurl). Once verification complete and has error, then curl cancel the communication before giving us a chance to modify the situation. A patch is required if we really do pause and continue.


> Client certificates might not be a priority right now, but they may become a
> priority.

Agreed.
Comment 16 Basuke Suzuki 2018-11-29 12:56:38 PST
Are you okay to reopen this bug? > Alex
Comment 17 Basuke Suzuki 2018-11-30 13:04:56 PST
Alex, please try us this again to implement server trust evaluation, not globally allowing the exeption, but per request base.
Comment 18 Alex Christensen 2018-12-03 13:53:44 PST
We want only decreasing use of allowSpecificHTTPSCertificateForHost in WebKit trunk.  If you need it temporarily in your downstream port, keep it there.  If you are adding an implementation that asks the client per-cert-chain, re-open and upload a patch for review.
Comment 19 Basuke Suzuki 2019-04-01 10:41:13 PDT
Because Server Trust Evaluation was implemented with different way, this is no longer required.