Bug 175147 – [Win] DRT should only allow any https certificate for localhost.

• Status: RESOLVED
• Resolution: FIXED
• Component: Tools / Tests

[Per Arne Vollan]

The same restriction is enforced on macOS.

[Per Arne Vollan]

Created attachment 317142 [details]
Patch

[Per Arne Vollan]

Created attachment 317144 [details]
Patch

[Alexey Proskuryakov]

Comment on attachment 317144 [details]
Patch

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

> Source/WebCore/ChangeLog:3
> +        [Win] DRT should only allow any https certificate for localhost.

Is SSL completely broken on Windows? There is a change in WebCore, what is its impact other than on testing?

[Per Arne Vollan]

(In reply to Alexey Proskuryakov from comment #3)
> Comment on attachment 317144 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=317144&action=review
> 
> > Source/WebCore/ChangeLog:3
> > +        [Win] DRT should only allow any https certificate for localhost.
> 
> Is SSL completely broken on Windows? There is a change in WebCore, what is
> its impact other than on testing?

SSL works on Windows, but SSL tests are failing because the layout test certificate has expired. To work around this, I recently committed a change to allow any https certificate in layout tests, but this was not enough to get SSL tests to pass, since there appears to be a bug in CFNetwork on Windows where the certificate will not be accepted when the certificate chain validation is skipped. This is addressed in this patch, and I believe it only will impact WebKit on Windows clients which allow any https certificate. It actually makes the certificate check a little stricter when any https certificate is allowed, since the certificate chain validation is not skipped with this patch.

[Per Arne Vollan]

I believe this patch fixes ~40 layout tests :)

[Alexey Proskuryakov]

Brent or Alex would be the best reviewers for this change I think.

[Brent Fulgham]

Comment on attachment 317144 [details]
Patch

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

r=me assuming you add the Radar reference I requested.

> Source/WebCore/platform/network/cf/ResourceHandleCFNet.cpp:183
>          CFDictionaryAddValue(sslProps.get(), kCFStreamSSLValidatesCertificateChain, kCFBooleanFalse);

Is this a bug in CFNetwork on Windows? Can you add a <rdar> reference for this, if it is?

> Tools/DumpRenderTree/win/DumpRenderTree.cpp:1217
> +        request->setAllowsAnyHTTPSCertificate();

This is a harmless check, but I'm not sure it's necessary since DRT only runs against servers we specify.

[Per Arne Vollan]

(In reply to Brent Fulgham from comment #7)
> Comment on attachment 317144 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=317144&action=review
> 
> r=me assuming you add the Radar reference I requested.
> 
> > Source/WebCore/platform/network/cf/ResourceHandleCFNet.cpp:183
> >          CFDictionaryAddValue(sslProps.get(), kCFStreamSSLValidatesCertificateChain, kCFBooleanFalse);
> 
> Is this a bug in CFNetwork on Windows? Can you add a <rdar> reference for
> this, if it is?

Yes. I will add a <rdar> reference.

Thanks for reviewing!

[Per Arne Vollan]

Committed r220970: <https://trac.webkit.org/changeset/220970/webkit>

[Radar WebKit Bug Importer]

<rdar://problem/35622830>

[Radar WebKit Bug Importer]

<rdar://problem/35622831>