Bug 198210

Summary: Web Inspector: Network: provide a way to accept alternate types of certificate data
Product: WebKit Reporter: Devin Rousso <drousso>
Component: Web InspectorAssignee: Devin Rousso <drousso>
Status: NEW ---    
Severity: Normal CC: bburg, drousso, ews-watchlist, inspector-bugzilla-changes, joepeck, keith_miller, mark.lam, msaboff, sbarati, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: All   
OS: All   
Attachments:
Description Flags
Patch none

Description Devin Rousso 2019-05-23 21:12:26 PDT
Both the Network agent and `InspectorFrontendHost` use a `CertificateInfo` for encoding/decoding, but other devices/frameworks may not have included the entirety of WebKit (including WebCore), and therefore may only have a smaller subset of the data.  There should be a way to send back other types of certificate data from `Network.getSerializedCertificate` that `InspectorFrontendHost::showCertificate` can then handle.
Comment 1 Devin Rousso 2019-05-23 21:17:00 PDT
Created attachment 370557 [details]
Patch

Rather than create an API for it, this may be a simpler solution for clients using the Security framework.
Comment 2 EWS Watchlist 2019-05-23 21:18:51 PDT
This patch modifies the inspector protocol. Please ensure that any frontend changes appropriately use feature checks for new protocol features.
Comment 3 Devin Rousso 2019-05-24 12:30:17 PDT
Comment on attachment 370557 [details]
Patch

cq-, while we discuss if this is the right way forward
Comment 4 Devin Rousso 2019-05-31 17:01:31 PDT
<rdar://problem/51076339>
Comment 5 Devin Rousso 2019-06-25 09:37:19 PDT
Comment on attachment 370557 [details]
Patch

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

> Source/WebCore/inspector/InspectorFrontendHost.cpp:490
> +    if (type.isEmpty() || type == "WebCore") {

This could use `Inspector::Protocol::InspectorHelpers::getEnumConstantValue(Inspector::Protocol::Security::SerializedCertificateType::WebCore)` instead.

> Source/WebCore/inspector/InspectorFrontendHost.cpp:495
> +    else if (type == "SecTrustRef") {

Ditto (>490).
Comment 6 Brian Burg 2019-08-06 13:41:11 PDT
Comment on attachment 370557 [details]
Patch

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

I am worried about this breaking. How can we test this?

> Source/JavaScriptCore/inspector/protocol/Network.json:209
> +                { "name": "serializedCertificate", "type": "string", "description": "Represents a base64 encoded certificat object." },

Nit: certificate

> Source/WebCore/platform/network/cf/CertificateInfo.h:153
> +static bool decodeSecTrustRefIfAble(Decoder& decoder, RetainPtr<SecTrustRef>& result)

I don't understand the need to shell this out. What's wrong with the existing decodeSecTrustRef?
Comment 7 Devin Rousso 2019-08-29 00:34:45 PDT
Comment on attachment 370557 [details]
Patch

Clearing flags to remove from review queue while we discuss the necessity of this approach.