Bug 142461 – [Cocoa] Add an option to treat certificate chains with SHA1-signed certificates as insecure

• Component: WebKit API
• Status: RESOLVED FIXED

Add a WKWebViewConfiguration option to make the view treat certificate chains where the signature hash algorithm for a non-root certificate is SHA-1 as insecure for purposes of -[WKWebView containsInsecureContent].

[Radar WebKit Bug Importer]

<rdar://problem/20086546>

Created attachment 248202 [details]
Add _treatsSHA1SignedCertificatesAsInsecure property to WKWebViewConfiguration

[WebKit Commit Bot]

Attachment 248202 [details] did not pass style-queue:


ERROR: Source/WebCore/platform/spi/cocoa/SecuritySPI.h:35:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Source/WebCore/platform/spi/cocoa/SecuritySPI.h:36:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Source/WebCore/platform/spi/cocoa/SecuritySPI.h:37:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Source/WebCore/platform/spi/cocoa/SecuritySPI.h:38:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Source/WebCore/platform/spi/cocoa/SecuritySPI.h:39:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Source/WebCore/platform/spi/cocoa/SecuritySPI.h:40:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Source/WebCore/platform/spi/cocoa/SecuritySPI.h:41:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Source/WebCore/platform/spi/cocoa/SecuritySPI.h:42:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Source/WebCore/platform/spi/cocoa/SecuritySPI.h:43:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Source/WebCore/platform/spi/cocoa/SecuritySPI.h:49:  The parameter name "certificate" adds no information, so it should be removed.  [readability/parameter_name] [5]
Total errors found: 10 in 15 files


If any of these errors are false positives, please file a bug against check-webkit-style.

Committed most of attachment 248202 [details] in <http://trac.webkit.org/r181317>, but left out the OS X part. I am going to turn that on soon.

Finished landing this in <http://trac.webkit.org/r181455>.

[Michael Catanzaro]

So it looks like these commits ensure that Apple port users get some warning if a SHA1 certificate is used in the chain for the main resource (without blocking anything). Kudos for that change. But it allows SHA1 certificates in subresources without any warning. Surely that is not the desired behavior?

Instead of adding the second hasInsecureContent parameter to PageLoadState::didCommitLoad (which should only be called for the main resource), it would probably be better to add a new member function to PageLoadState for this.

Aside #1: hasInsecureContent is not an accurate name for the new parameter to PageLoadState::didCommitLoad: the only reason it could be true is if the certificate uses a SHA1 signature, so hasInsecureContent will usually be *false* if the certificate is insecure. We should fix that. I don't have a great alternative name, but one possibility is hasDubiouslySecureContent, since "dubious" is often used to indicate that the security is acceptable (we're not going to block the connection) but not good. This isn't a great name either, but it's much better, since this code cannot ever block any content.

Aside #2: I'm working on a patch to replace treatsSHA1SignedCertificatesAsInsecure in WebPageConfiguration with a platform-specific object (my current working name for it is WebCore::CertificateInfo::DubiousnessConfiguration) since (a) the curl and soup ports will never use it, and (b) WebPageConfiguration is used to hold a few very important objects, not preferences (c) that is one highly-specific certificate check out of many, and WebPageProxy is not the right layer for it to be in. (Alternatively we could use conditional compilation for it, but that only solves (a) and not (b) or (c).)

(In reply to comment #6)

Thanks for the comment and the new bugs you’ve filed, Michael!