Re: [PATCH 3/4] wininet: Combine certificate errors in HTTP_OpenConnection

[Juan Lang]

Hi Piotr,

+            if((lpwhr->hdr.ErrorMask&INTERNET_ERROR_MASK_COMBINED_SEC_CERT)
&& (
+                    res == ERROR_INTERNET_SEC_CERT_DATE_INVALID
+                    || res == ERROR_INTERNET_INVALID_CA
+                    || res == ERROR_INTERNET_SEC_CERT_NO_REV
+                    || res == ERROR_INTERNET_SEC_CERT_REV_FAILED
+                    || res == ERROR_INTERNET_SEC_CERT_REVOKED
+                    || res == ERROR_INTERNET_SEC_INVALID_CERT
+                    || res == ERROR_INTERNET_SEC_CERT_CN_INVALID))
+                res = ERROR_INTERNET_SEC_CERT_ERRORS;

I'm confused.  Is there evidence that native does the same?  This
would be surprising to me, given the existence of
INTERNET_OPTION_SECURITY_FLAGS to mask particular errors.  When are
you trying to accomplish with this patch?  Tests would certainly help.

Thanks,
--Juan

[PATCH 3/4] wininet: Combine certificate errors in HTTP_OpenConnection

[Piotr Caban]

Hi,

On 05/19/10 02:01, Juan Lang wrote:
> +            if((lpwhr->hdr.ErrorMask&INTERNET_ERROR_MASK_COMBINED_SEC_CERT)
> &&  (
> +                    res == ERROR_INTERNET_SEC_CERT_DATE_INVALID
> +                    || res == ERROR_INTERNET_INVALID_CA
> +                    || res == ERROR_INTERNET_SEC_CERT_NO_REV
> +                    || res == ERROR_INTERNET_SEC_CERT_REV_FAILED
> +                    || res == ERROR_INTERNET_SEC_CERT_REVOKED
> +                    || res == ERROR_INTERNET_SEC_INVALID_CERT
> +                    || res == ERROR_INTERNET_SEC_CERT_CN_INVALID))
> +                res = ERROR_INTERNET_SEC_CERT_ERRORS;
>
> I'm confused.  Is there evidence that native does the same?  This
> I'm not sure which of the errors are combined (and if all of them to the 
> same value as is stated in msdn).
> would be surprising to me, given the existence of
> INTERNET_OPTION_SECURITY_FLAGS to mask particular errors.
> I guess this flag was used in past, currently IE is probably temporarily 
> accepting certificates.
>    When are
> you trying to accomplish with this patch?  Tests would certainly help.
> Test can't be easily added to wine because we don't have pages to test 
> certificate errors on. Thanks to this patch IE7 displays certificate 
> validation error page.

I have written a simple test (not to be added to wine) that shows it's 
at least partially correct behavior. This test prints error returned in 
INTERNET_STATUS_REQUEST_COMPLETE callback (you can see the difference on 
WXPPROSP3 machine).
https://testbot.winehq.org/JobDetails.pl?Key=2143&log_204=1#k204

Cheers,
Piotr

[PATCH 3/4] wininet: Combine certificate errors in HTTP_OpenConnection

[Juan Lang]

> Test can't be easily added to wine because we don't have pages to test
> certificate errors on.

That should be addressed, no?  E.g. with a page hosted at winehq.org
with a self-signed certificate?  That'd at least allow us to check the
expected error when the root of a certificate chain is untrusted.
--Juan