RESOLVED FIXED 191647
[Curl] Add API for CertificateInfo.
https://bugs.webkit.org/show_bug.cgi?id=191647
Summary [Curl] Add API for CertificateInfo.
Basuke Suzuki
Reported 2018-11-14 12:40:48 PST
It will be used by Server Trust Evaluation UI.
Attachments
PATCH (18.80 KB, patch)
2018-11-16 18:36 PST, Basuke Suzuki
no flags
Basuke Suzuki
Comment 1 2018-11-16 18:36:51 PST
Alex Christensen
Comment 2 2018-11-16 20:27:47 PST
Comment on attachment 355170 [details] PATCH View in context: https://bugs.webkit.org/attachment.cgi?id=355170&action=review > Source/WebKit/Shared/API/c/curl/WKCertificateInfoCurl.cpp:46 > + auto data = reinterpret_cast<WKDataRef>(WKArrayGetItemAtIndex(certificateChainRef, i)); I'm not sure you want to make an API that takes PEM encoded certificates as WKDataRefs. I think of PEM encoded certificates as strings that need to be parsed into data. Does libcurl have a way to just give the raw certificate data to it, or does it only take PEM encoded data?
Basuke Suzuki
Comment 3 2018-11-16 21:03:34 PST
(In reply to Alex Christensen from comment #2) > Comment on attachment 355170 [details] > PATCH > > View in context: > https://bugs.webkit.org/attachment.cgi?id=355170&action=review > > > Source/WebKit/Shared/API/c/curl/WKCertificateInfoCurl.cpp:46 > > + auto data = reinterpret_cast<WKDataRef>(WKArrayGetItemAtIndex(certificateChainRef, i)); > > I'm not sure you want to make an API that takes PEM encoded certificates as > WKDataRefs. I think of PEM encoded certificates as strings that need to be > parsed into data. Does libcurl have a way to just give the raw certificate > data to it, or does it only take PEM encoded data? curl doesn't have any support for PEM contents. We have to use OpenSSL to decode information from them. Also it can be DER format (binary) or PEM format (TEXT w/ separator and base64 contents). In that sense, it should be treated as opaque data.
Alex Christensen
Comment 4 2018-11-16 22:42:06 PST
Would you want to use the same API for DER or PEM Certificates? Could you add a test with a DER certificate?
Alex Christensen
Comment 5 2018-11-16 22:44:05 PST
Would you want to be able to compare certificates of different encodings and find them to be equal?
Basuke Suzuki
Comment 6 2018-11-16 23:40:35 PST
(In reply to Alex Christensen from comment #4) > Would you want to use the same API for DER or PEM Certificates? Could you > add a test with a DER certificate? (In reply to Alex Christensen from comment #5) > Would you want to be able to compare certificates of different encodings and > find them to be equal? This patch is for CertificateInfo which is just a container for certificates and it doesn't care what it hold inside. The requested tests will be handled by verifier tests, which will be included in https://bugs.webkit.org/show_bug.cgi?id=187679 ([Curl] Add allowSpecificHTTPSCertificateForHost support.)
Basuke Suzuki
Comment 7 2018-11-16 23:46:17 PST
Sorry, this is wrong. The openssl api only allows PEM format. DER must be converted to PEM. There's no encoding mix-up here. Only PEM. But even if it is a PEM format, which is consisted by printable ascii characters, the contents is just base64 encoded binary with header and footer. There's no text parsing to get information from this chunk of data. I think treating this as a Data is better than String.
Alex Christensen
Comment 8 2018-11-17 07:39:02 PST
There *is* parsing, but it's probably between libcurl and openssl to do so. The network uses certificate bytes from finding and decoding the base64 info. It's true that the string here can only be ASCII, but it is a string that needs to be parsed by something before it can be used on the network. I'm ok with it being a data instead of a string because it's always ascii, but I want that to be an informed choice.
Basuke Suzuki
Comment 9 2018-11-17 13:32:26 PST
(In reply to Alex Christensen from comment #8) > There *is* parsing, but it's probably between libcurl and openssl to do so. I meant the word “parsing” as text processing, such as JSON parsing. Because certificate is in a binary shape, I want to use the term “decoding”. Anyway, those are completely task of OpenSSL’s task, neither libcurl nor WebKit curl layer care about its content detail so that there are no parsing or decoding. If an app want to display those information, that is the task of the app to fetch information using OpenSSL api. That’s why I want to treat it as opaque. That’s the reason I think WKDataRef is appropriate for this API. > The network uses certificate bytes from finding and decoding the base64 > info. It's true that the string here can only be ASCII, but it is a string > that needs to be parsed by something before it can be used on the network. > I'm ok with it being a data instead of a string because it's always ascii, > but I want that to be an informed choice. I am not clear what you request me. What should I show you ?
Alex Christensen
Comment 10 2018-11-19 17:35:59 PST
Comment on attachment 355170 [details] PATCH r=me
WebKit Commit Bot
Comment 11 2018-11-19 18:02:42 PST
Comment on attachment 355170 [details] PATCH Clearing flags on attachment: 355170 Committed r238387: <https://trac.webkit.org/changeset/238387>
WebKit Commit Bot
Comment 12 2018-11-19 18:02:44 PST
All reviewed patches have been landed. Closing bug.
Radar WebKit Bug Importer
Comment 13 2018-11-19 18:03:38 PST
Note You need to log in before you can comment on or make changes to this bug.