Bug 232120 - REGRESSION (r284650-284661): [iOS15 Sim] imported/w3c/web-platform-tests/content-security-policy/script-src/script-src-multiple-policies-multiple-hashing-algorithms.html is a text failure
Summary: REGRESSION (r284650-284661): [iOS15 Sim] imported/w3c/web-platform-tests/cont...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Kate Cheney
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2021-10-21 17:28 PDT by Eric Hutchison
Modified: 2021-10-27 14:34 PDT (History)
7 users (show)

See Also:


Attachments
Patch (17.68 KB, patch)
2021-10-27 11:41 PDT, Kate Cheney
no flags Details | Formatted Diff | Diff
Patch for landing (17.78 KB, patch)
2021-10-27 13:35 PDT, Kate Cheney
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Eric Hutchison 2021-10-21 17:28:02 PDT
imported/w3c/web-platform-tests/content-security-policy/script-src/script-src-multiple-policies-multiple-hashing-algorithms.html

is a text failure on iOS15 EWS

History: https://results.webkit.org/?suite=layout-tests&test=imported/w3c/web-platform-tests/content-security-policy/script-src/script-src-multiple-policies-multiple-hashing-algorithms.html

Build: https://ews-build.webkit.org/#/builders/68/builds/159

Results: https://ews-build.s3-us-west-2.amazonaws.com/iOS-15-Simulator-WK2-Tests-EWS/r441997-159/results.html

Diff:
--- /Volumes/Data/worker/iOS-15-Simulator-WK2-Tests-EWS/build/layout-test-results/imported/w3c/web-platform-tests/content-security-policy/script-src/script-src-multiple-policies-multiple-hashing-algorithms-expected.txt
+++ /Volumes/Data/worker/iOS-15-Simulator-WK2-Tests-EWS/build/layout-test-results/imported/w3c/web-platform-tests/content-security-policy/script-src/script-src-multiple-policies-multiple-hashing-algorithms-actual.txt
@@ -1,3 +1,3 @@
 
-FAIL Test that script executes if allowed by proper hash values assert_true: expected true got false
+FAIL Test that script executes if allowed by proper hash values assert_unreached: Should not have triggered a security event Reached unreachable code
 
Unable to locally reproduce the text failure.
Comment 1 Radar WebKit Bug Importer 2021-10-21 17:28:57 PDT
<rdar://problem/84529888>
Comment 2 Eric Hutchison 2021-10-21 17:34:11 PDT
Updated test expectations at https://trac.webkit.org/changeset/284659/webkit
Comment 3 Alexey Proskuryakov 2021-10-22 10:13:18 PDT
Test history shows that this isn't just EWS, but a very recent trunk regression.

> Unable to locally reproduce the text failure.

Which revision were you attempting to reproduce with?
Comment 4 Ryan Haddad 2021-10-22 10:19:09 PDT
LayoutTests/TestExpectations:5154:imported/w3c/web-platform-tests/content-security-policy/script-src/ [ DumpJSConsoleLogInStdErr ]

The flaky expectation added in r284659 is overriding this and causing the test to show up as a consistent failure now.
Comment 5 Eric Hutchison 2021-10-22 12:20:34 PDT
(In reply to Ryan Haddad from comment #4)
> LayoutTests/TestExpectations:5154:imported/w3c/web-platform-tests/content-
> security-policy/script-src/ [ DumpJSConsoleLogInStdErr ]
> 
> The flaky expectation added in r284659 is overriding this and causing the
> test to show up as a consistent failure now.

Add expectation: https://trac.webkit.org/changeset/284708/webkit
Comment 6 Kate Cheney 2021-10-27 11:41:41 PDT
Created attachment 442616 [details]
Patch
Comment 7 Brent Fulgham 2021-10-27 11:56:48 PDT
Comment on attachment 442616 [details]
Patch

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

r=me

> Source/WebCore/page/csp/ContentSecurityPolicy.cpp:374
> +    Vector<ContentSecurityPolicyHash> hashes;

If we might have many hashes, we could preallocate hashes for the size of algorithms, but maybe we only ever have a handful.

> Source/WebCore/page/csp/ContentSecurityPolicy.cpp:376
>          ContentSecurityPolicyHash hash = cryptographicDigestForBytes(algorithm, encodedContent.data(), encodedContent.size());

This could be auto
Comment 8 Kate Cheney 2021-10-27 12:35:27 PDT
(In reply to Brent Fulgham from comment #7)
> Comment on attachment 442616 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=442616&action=review
> 
> r=me
> 

Thanks! Going to wait for EWS before landing, in case I missed any failing tests locally.

> > Source/WebCore/page/csp/ContentSecurityPolicy.cpp:374
> > +    Vector<ContentSecurityPolicyHash> hashes;
> 
> If we might have many hashes, we could preallocate hashes for the size of
> algorithms, but maybe we only ever have a handful.
> 

We will only ever have 3 at most because only sha256, sha384, and sha512 are supported. So, probably not too much of an efficiency boost, but I will change it if you think it is important.

> > Source/WebCore/page/csp/ContentSecurityPolicy.cpp:376
> >          ContentSecurityPolicyHash hash = cryptographicDigestForBytes(algorithm, encodedContent.data(), encodedContent.size());
> 
> This could be auto

Will fix.
Comment 9 Kate Cheney 2021-10-27 13:35:50 PDT
Created attachment 442626 [details]
Patch for landing
Comment 10 EWS 2021-10-27 14:34:11 PDT
Committed r284959 (243611@main): <https://commits.webkit.org/243611@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 442626 [details].