RESOLVED FIXED 230526
[Curl] Show TLS connection information in the inspector
https://bugs.webkit.org/show_bug.cgi?id=230526
Summary [Curl] Show TLS connection information in the inspector
Takashi Komori
Reported 2021-09-20 17:49:30 PDT
Created attachment 438761 [details] Added information in the network security tab. Add TLS version and cipher name in the network security tab.
Attachments
Added information in the network security tab. (406.82 KB, image/png)
2021-09-20 17:49 PDT, Takashi Komori
no flags
Patch (4.82 KB, patch)
2021-09-20 18:29 PDT, Takashi Komori
no flags
Patch (4.79 KB, patch)
2021-09-21 02:19 PDT, Takashi Komori
no flags
Patch (4.95 KB, patch)
2021-09-22 13:31 PDT, Takashi Komori
no flags
Patch for landing (4.95 KB, patch)
2021-09-22 20:00 PDT, Takashi Komori
no flags
Takashi Komori
Comment 1 2021-09-20 18:29:58 PDT
Fujii Hironori
Comment 2 2021-09-20 19:01:46 PDT
Comment on attachment 438765 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=438765&action=review > Source/WebCore/platform/network/curl/CurlContext.cpp:875 > + m_tlsConnectionInfo = makeUnique<TLSConnectionInfo>(); Why is m_tlsConnectionInfo needed? What's the problem if setting to networkLoadMetrics.tlsProtocol directly?
Fujii Hironori
Comment 3 2021-09-20 19:05:14 PDT
Comment on attachment 438765 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=438765&action=review > Source/WebCore/platform/network/curl/CurlContext.h:322 > + WTF_MAKE_FAST_ALLOCATED; I think it'd better to use struct and WTF_MAKE_STRUCT_FAST_ALLOCATED.
Fujii Hironori
Comment 4 2021-09-20 19:16:14 PDT
Comment on attachment 438765 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=438765&action=review > Source/WebCore/platform/network/curl/OpenSSLHelper.cpp:329 > +String getTLSCipherName(const SSL* ssl) You shouldn't use 'get' for these functions (Bug 227766). However, OpenSSLHelper.cpp has more 'get'. Sooner or later, they should be renamed.
Takashi Komori
Comment 5 2021-09-21 02:19:21 PDT
Takashi Komori
Comment 6 2021-09-21 02:20:52 PDT
(In reply to Fujii Hironori from comment #2) > Comment on attachment 438765 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=438765&action=review > > > Source/WebCore/platform/network/curl/CurlContext.cpp:875 > > + m_tlsConnectionInfo = makeUnique<TLSConnectionInfo>(); > > Why is m_tlsConnectionInfo needed? What's the problem if setting to > networkLoadMetrics.tlsProtocol directly? A CurlRequest gets metrics in CurlRequest::didReceiveHeader and CurlRequest::didCompleteTransfer but in didCompleteTransfer, curl_easy_getinfo can't obtain SSL pointer. This is because when the connection ends, libcurl clears ssl context in the curl handle internally. Therefore, we should keep TLS information in m_tlsConnectionInfo when didReceiveHeader is called.
Takashi Komori
Comment 7 2021-09-21 02:21:55 PDT
(In reply to Fujii Hironori from comment #3) > Comment on attachment 438765 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=438765&action=review > > > Source/WebCore/platform/network/curl/CurlContext.h:322 > > + WTF_MAKE_FAST_ALLOCATED; > > I think it'd better to use struct and WTF_MAKE_STRUCT_FAST_ALLOCATED. Fixed.
Takashi Komori
Comment 8 2021-09-21 02:22:26 PDT
(In reply to Fujii Hironori from comment #4) > Comment on attachment 438765 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=438765&action=review > > > Source/WebCore/platform/network/curl/OpenSSLHelper.cpp:329 > > +String getTLSCipherName(const SSL* ssl) > > You shouldn't use 'get' for these functions (Bug 227766). However, > OpenSSLHelper.cpp has more 'get'. Sooner or later, they should be renamed. Removed 'get' from function names.
EWS
Comment 9 2021-09-22 12:00:15 PDT
Tools/Scripts/svn-apply failed to apply attachment 438797 [details] to trunk. Please resolve the conflicts and upload a new patch.
Takashi Komori
Comment 10 2021-09-22 13:31:03 PDT
Takashi Komori
Comment 11 2021-09-22 13:34:52 PDT
Fix the conflict of the patch of bug 226982.
Takashi Komori
Comment 12 2021-09-22 20:00:53 PDT
Created attachment 439005 [details] Patch for landing
EWS
Comment 13 2021-09-22 20:28:13 PDT
Committed r282904 (242032@main): <https://commits.webkit.org/242032@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 439005 [details].
Radar WebKit Bug Importer
Comment 14 2021-09-22 20:29:22 PDT
Note You need to log in before you can comment on or make changes to this bug.