Bug 105467 – [WK2 NetworkProcess] Client doesn't receive SSL certificates

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKit2

[Alexey Proskuryakov]

SSL certificate chain is not serialized with NSURLResponse.

<rdar://problem/12890242>

[Alexey Proskuryakov]

Created attachment 180233 [details]
proposed fix

[WebKit Review Bot]

Attachment 180233 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/ChangeLog', u'Source/WebCor..." exit_code: 1
Source/WebKit2/WebProcess/Network/WebResourceLoader.h:81:  The parameter name "certificateInfo" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/WebKit/mac/ChangeLog:1:  ChangeLog entry has no bug number  [changelog/bugnumber] [5]
Total errors found: 2 in 15 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Brady Eidson]

Comment on attachment 180233 [details]
proposed fix

View in context: https://bugs.webkit.org/attachment.cgi?id=180233&action=review

> Source/WebKit2/NetworkProcess/NetworkResourceLoader.cpp:155
> -    send(Messages::WebResourceLoader::DidReceiveResponse(response));
> +    send(Messages::WebResourceLoader::DidReceiveResponse(response, PlatformCertificateInfo(response)));

I would much rather see us keep the cert chain encapsulated by the response, and just encode it in the ResourceResponse encoder when it exists.

> Source/WebKit2/WebProcess/Network/WebResourceLoader.cpp:84
> -void WebResourceLoader::didReceiveResponse(const WebCore::ResourceResponse& response)
> +void WebResourceLoader::didReceiveResponse(const ResourceResponse& response, const PlatformCertificateInfo& certificateInfo)
>  {
>      LOG(Network, "(WebProcess) WebResourceLoader::didReceiveResponse for '%s'. Status %d.", m_coreLoader->url().string().utf8().data(), response.httpStatusCode());
> -    m_coreLoader->didReceiveResponse(response);
> +    ResourceResponse responseCopy(response);
> +    responseCopy.setCertificateChain(certificateInfo.certificateChain());
> +    m_coreLoader->didReceiveResponse(responseCopy);

Then none of this would be required.

[Alexey Proskuryakov]

Created attachment 180249 [details]
updated patch

Discussed this with Brady in person.

I think that the right design is for ResourceRequest and ResourceResponse serializers to only preserve essential lightweight data. Parts that we rarely need can be sent over IPC as separate arguments. Another example of this is how we shouldn't serialize form data with each ResourceRequest.

Another consideration is that we should really think about only passing certificate chain data on demand, not with ResourceResponse.

As a result of this discussion, I renamed didReceiveResponse to didReceiveResponseWithCertificateInfo, so that it doesn't look confusingly similar to other didReceiveResponse functions.

[WebKit Review Bot]

Attachment 180249 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/ChangeLog', u'Source/WebCor..." exit_code: 1
Source/WebKit2/WebProcess/Network/WebResourceLoader.h:81:  The parameter name "certificateInfo" adds no information, so it should be removed.  [readability/parameter_name] [5]
Total errors found: 1 in 15 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Brady Eidson]

Yes, please fix the stylebot complaint.  :)

[Alexey Proskuryakov]

Fixed <http://trac.webkit.org/changeset/138206>.