Bug 197800 - Add a unit test for client certificate authentication
Summary: Add a unit test for client certificate authentication
Status: REOPENED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Alex Christensen
URL:
Keywords: InRadar
Depends on: 197887 197989
Blocks:
  Show dependency treegraph
 
Reported: 2019-05-10 15:04 PDT by Alex Christensen
Modified: 2019-05-17 11:25 PDT (History)
15 users (show)

See Also:


Attachments
Patch (42.07 KB, patch)
2019-05-10 16:02 PDT, Alex Christensen
no flags Details | Formatted Diff | Diff
Patch (41.75 KB, patch)
2019-05-10 16:06 PDT, Alex Christensen
no flags Details | Formatted Diff | Diff
Patch (41.68 KB, patch)
2019-05-10 18:23 PDT, Alex Christensen
no flags Details | Formatted Diff | Diff
Patch (42.45 KB, patch)
2019-05-14 08:16 PDT, Alex Christensen
no flags Details | Formatted Diff | Diff
patch (42.52 KB, patch)
2019-05-16 14:29 PDT, Alex Christensen
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Alex Christensen 2019-05-10 15:04:05 PDT
Add a unit test for client certificate authentication
Comment 1 Alex Christensen 2019-05-10 16:02:46 PDT
Created attachment 369609 [details]
Patch
Comment 2 Build Bot 2019-05-10 16:04:03 PDT
Attachment 369609 [details] did not pass style-queue:


ERROR: Tools/TestWebKitAPI/TCPServer.cpp:44:  Declaration has space between type name and * in X509 *cert  [whitespace/declaration] [3]
Total errors found: 1 in 11 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 3 Alex Christensen 2019-05-10 16:06:28 PDT
Created attachment 369610 [details]
Patch
Comment 4 Alex Christensen 2019-05-10 18:23:05 PDT
Created attachment 369630 [details]
Patch
Comment 5 youenn fablet 2019-05-13 16:55:59 PDT
Comment on attachment 369630 [details]
Patch

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

> Tools/TestWebKitAPI/TCPServer.cpp:282
> +    EXPECT_TRUE(static_cast<size_t>(bytesRead) < sizeof(buffer));

These tests seem a bit strange since they are not test per-se but test infra.
It looks like it should be ASSERT(...)

> Tools/TestWebKitAPI/Tests/WebKitCocoa/Challenge.mm:43
> +    RetainPtr<SecCertificateRef> certificate = adoptCF(SecCertificateCreateWithData(nullptr, (__bridge CFDataRef)[NSData dataWithBytes:certificateBytes.data() length:certificateBytes.size()]));

auto

> Tools/TestWebKitAPI/Tests/WebKitCocoa/Challenge.mm:45
> +    Vector<uint8_t> privateKeyBytes = TestWebKitAPI::TCPServer::testPrivateKey();

auto here and below.

> Tools/TestWebKitAPI/Tests/WebKitCocoa/Challenge.mm:58
> +    RetainPtr<SecIdentityRef> identity = adoptCF(SecIdentityCreate(kCFAllocatorDefault, certificate.get(), privateKey.get()));

auto

> Tools/TestWebKitAPI/Tests/WebKitCocoa/Challenge.mm:157
> +    EXPECT_EQ([delegate challengeCount], 2ull);

We could check that we first receive a server trust challenge and then a client certificate challenge.

> Tools/TestWebKitAPI/Tests/WebKitCocoa/PDFLinkReferrer.mm:79
> +        Vector<uint8_t> request = TCPServer::read(socket);

auto

> Tools/TestWebKitAPI/Tests/WebKitCocoa/PDFLinkReferrer.mm:82
> +        const char* currentLine = reinterpret_cast<char*>(request.data());

auto
Comment 6 Alex Christensen 2019-05-14 08:16:13 PDT
Created attachment 369849 [details]
Patch
Comment 7 WebKit Commit Bot 2019-05-14 08:49:41 PDT
Comment on attachment 369849 [details]
Patch

Clearing flags on attachment: 369849

Committed r245281: <https://trac.webkit.org/changeset/245281>
Comment 8 WebKit Commit Bot 2019-05-14 08:49:42 PDT
All reviewed patches have been landed.  Closing bug.
Comment 9 Radar WebKit Bug Importer 2019-05-14 08:50:19 PDT
<rdar://problem/50768065>
Comment 10 Aakash Jain 2019-05-14 10:09:37 PDT
This broke the API Test TestWebKitAPI.Challenge.ClientCertificate, as reported by EWS for API test.
Comment 11 WebKit Commit Bot 2019-05-14 11:06:34 PDT
Re-opened since this is blocked by bug 197887
Comment 12 Alex Christensen 2019-05-16 14:29:06 PDT
Created attachment 370070 [details]
patch
Comment 13 WebKit Commit Bot 2019-05-16 16:08:53 PDT
Comment on attachment 370070 [details]
patch

Clearing flags on attachment: 370070

Committed r245418: <https://trac.webkit.org/changeset/245418>
Comment 14 WebKit Commit Bot 2019-05-16 16:08:56 PDT
All reviewed patches have been landed.  Closing bug.
Comment 15 Truitt Savell 2019-05-17 08:19:50 PDT
It looks like the changes in https://trac.webkit.org/changeset/245418/webkit

has caused API test TestWebKitAPI.Challenge.ClientCertificate to crash on Mojave.

Build:
https://build.webkit.org/builders/Apple%20Mojave%20Release%20WK2%20%28Tests%29/builds/4171
Comment 16 WebKit Commit Bot 2019-05-17 10:19:34 PDT
Re-opened since this is blocked by bug 197989
Comment 17 Shawn Roberts 2019-05-17 11:25:14 PDT
Rolled out in https://trac.webkit.org/changeset/245468/webkit 

Causing API crashes on testers