[PATCH v3 0/2] MR11546: winhttp: Handle incorrect handle state when querying WINHTTP_OPTION_SERVER_CERT_CHAIN_CONTEXT.
-- v3: winhttp: Use chain established in netconn_verify_cert() for WINHTTP_OPTION_SERVER_CERT_CHAIN_CONTEXT. winhttp: Handle incorrect handle state when querying WINHTTP_OPTION_SERVER_CERT_CHAIN_CONTEXT. https://gitlab.winehq.org/wine/wine/-/merge_requests/11546
From: Paul Gofman <pgofman@codeweavers.com> --- dlls/winhttp/session.c | 6 ++++++ dlls/winhttp/tests/winhttp.c | 31 +++++++++++++++++++++++++++++++ 2 files changed, 37 insertions(+) diff --git a/dlls/winhttp/session.c b/dlls/winhttp/session.c index 85e3c3f18d1..eb02c5f00bb 100644 --- a/dlls/winhttp/session.c +++ b/dlls/winhttp/session.c @@ -923,6 +923,12 @@ static BOOL request_query_option( struct object_header *hdr, DWORD option, void chainPara.RequestedUsage.Usage.rgpszUsageIdentifier = server_auth; if (!validate_buffer( buffer, buflen, sizeof(cert_chain) )) return FALSE; + if (!request->server_cert) + { + SetLastError( ERROR_WINHTTP_INCORRECT_HANDLE_STATE ); + *(CERT_CHAIN_CONTEXT **)buffer = NULL; + return FALSE; + } if (!CertGetCertificateChain(NULL, request->server_cert, NULL, NULL, &chainPara, 0, NULL, &cert_chain)) return FALSE; *(CERT_CHAIN_CONTEXT **)buffer = (CERT_CHAIN_CONTEXT *)cert_chain; diff --git a/dlls/winhttp/tests/winhttp.c b/dlls/winhttp/tests/winhttp.c index f639877d2ab..4caf2abee38 100644 --- a/dlls/winhttp/tests/winhttp.c +++ b/dlls/winhttp/tests/winhttp.c @@ -1099,6 +1099,9 @@ static void test_secure_connection(void) CERT_CONTEXT *cert; WINHTTP_CERTIFICATE_INFO info; WINHTTP_SECURITY_INFO secinfo; + PCCERT_CHAIN_CONTEXT chain; + CERT_CHAIN_POLICY_PARA chain_policy = { .cbSize = sizeof(chain_policy) }; + CERT_CHAIN_POLICY_STATUS policy_status = { .cbSize = sizeof(policy_status) }; char buffer[32]; ses = WinHttpOpen(L"winetest", 0, NULL, NULL, 0); @@ -1152,9 +1155,25 @@ static void test_secure_connection(void) WinHttpCloseHandle(req); + size = sizeof(chain); + chain = (void *)0xdeadbeef; + SetLastError(0xdeadbeef); + ret = WinHttpQueryOption(ses, WINHTTP_OPTION_SERVER_CERT_CHAIN_CONTEXT, &chain, &size); + ok(!ret, "unexpected success.\n"); + todo_wine ok(GetLastError() == ERROR_WINHTTP_INCORRECT_HANDLE_TYPE, "got error %lu.\n", GetLastError()); + ok(chain == (void *)0xdeadbeef, "got %p.\n", chain); + req = WinHttpOpenRequest(con, NULL, NULL, NULL, NULL, NULL, WINHTTP_FLAG_SECURE); ok(req != NULL, "failed to open a request %lu\n", GetLastError()); + size = sizeof(chain); + chain = (void *)0xdeadbeef; + SetLastError(0xdeadbeef); + ret = WinHttpQueryOption(req, WINHTTP_OPTION_SERVER_CERT_CHAIN_CONTEXT, &chain, &size); + ok(!ret, "unexpected success.\n"); + ok(GetLastError() == ERROR_WINHTTP_INCORRECT_HANDLE_STATE, "got error %lu.\n", GetLastError()); + ok(!chain, "got %p.\n", chain); + flags = 0xdeadbeef; size = sizeof(flags); ret = WinHttpQueryOption(req, WINHTTP_OPTION_SECURITY_FLAGS, &flags, &size); @@ -1198,6 +1217,18 @@ static void test_secure_connection(void) } ok(ret, "failed to send request %lu\n", GetLastError()); + size = sizeof(chain); + chain = (void *)0xdeadbeef; + SetLastError(0xdeadbeef); + ret = WinHttpQueryOption(req, WINHTTP_OPTION_SERVER_CERT_CHAIN_CONTEXT, &chain, &size); + ok(ret, "got error %lu.\n", GetLastError()); + ok(chain && chain != (void *)0xdeadbeef, "got %p.\n", chain); + ret = CertVerifyCertificateChainPolicy(CERT_CHAIN_POLICY_SSL, chain, &chain_policy, &policy_status); + ok(ret, "got error %lu.\n", GetLastError()); + ok(!chain->TrustStatus.dwErrorStatus, "got %#lx.\n", chain->TrustStatus.dwErrorStatus); + ok(!policy_status.dwError, "got %#lx.\n", policy_status.dwError); + CertFreeCertificateChain(chain); + size = sizeof(cert); ret = WinHttpQueryOption(req, WINHTTP_OPTION_SERVER_CERT_CONTEXT, &cert, &size ); ok(ret, "failed to retrieve certificate context %lu\n", GetLastError()); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/11546
From: Paul Gofman <pgofman@codeweavers.com> --- dlls/winhttp/net.c | 9 ++++++--- dlls/winhttp/session.c | 22 ++++++---------------- dlls/winhttp/tests/winhttp.c | 2 ++ dlls/winhttp/winhttp_private.h | 1 + 4 files changed, 15 insertions(+), 19 deletions(-) diff --git a/dlls/winhttp/net.c b/dlls/winhttp/net.c index b0f405a9f52..05922acca00 100644 --- a/dlls/winhttp/net.c +++ b/dlls/winhttp/net.c @@ -81,7 +81,8 @@ static int sock_recv(int fd, void *msg, size_t len, int flags) return ret; } -static DWORD netconn_verify_cert( PCCERT_CONTEXT cert, WCHAR *server, DWORD security_flags, BOOL check_revocation ) +static DWORD netconn_verify_cert( PCCERT_CONTEXT cert, WCHAR *server, DWORD security_flags, BOOL check_revocation, + PCCERT_CHAIN_CONTEXT *ret_chain ) { HCERTSTORE store = cert->hCertStore; BOOL ret; @@ -92,6 +93,7 @@ static DWORD netconn_verify_cert( PCCERT_CONTEXT cert, WCHAR *server, DWORD secu DWORD err = ERROR_SUCCESS; TRACE("verifying %s\n", debugstr_w( server )); + *ret_chain = NULL; chainPara.RequestedUsage.Usage.cUsageIdentifier = 1; chainPara.RequestedUsage.Usage.rgpszUsageIdentifier = server_auth; ret = CertGetCertificateChain( NULL, cert, NULL, store, &chainPara, @@ -169,7 +171,7 @@ static DWORD netconn_verify_cert( PCCERT_CONTEXT cert, WCHAR *server, DWORD secu err = ERROR_WINHTTP_SECURE_INVALID_CERT; } } - CertFreeCertificateChain( chain ); + *ret_chain = chain; } else err = ERROR_WINHTTP_SECURE_CHANNEL_ERROR; @@ -296,6 +298,7 @@ void netconn_release( struct netconn *conn ) free(conn->ssl_read_buf); free(conn->ssl_write_buf); free(conn->extra_buf); + CertFreeCertificateChain(conn->chain); DeleteSecurityContext(&conn->ssl_ctx); } if (conn->socket != -1) @@ -400,7 +403,7 @@ DWORD netconn_secure_connect( struct netconn *conn, WCHAR *hostname, DWORD secur status = QueryContextAttributesW(&ctx, SECPKG_ATTR_REMOTE_CERT_CONTEXT, (void*)&cert); if(status == SEC_E_OK) { - res = netconn_verify_cert(cert, hostname, security_flags, check_revocation); + res = netconn_verify_cert(cert, hostname, security_flags, check_revocation, &conn->chain); CertFreeCertificateContext(cert); if(res != ERROR_SUCCESS) { WARN( "cert verify failed: %lu\n", res ); diff --git a/dlls/winhttp/session.c b/dlls/winhttp/session.c index eb02c5f00bb..8ff858ac084 100644 --- a/dlls/winhttp/session.c +++ b/dlls/winhttp/session.c @@ -912,28 +912,18 @@ static BOOL request_query_option( struct object_header *hdr, DWORD option, void } case WINHTTP_OPTION_SERVER_CERT_CHAIN_CONTEXT: { - const CERT_CHAIN_CONTEXT *cert_chain; + const CERT_CHAIN_CONTEXT *chain; - char oid_server_auth[] = szOID_PKIX_KP_SERVER_AUTH; - char *server_auth[] = { oid_server_auth }; - - CERT_CHAIN_PARA chainPara = { sizeof(chainPara), { 0 } }; - - chainPara.RequestedUsage.Usage.cUsageIdentifier = 1; - chainPara.RequestedUsage.Usage.rgpszUsageIdentifier = server_auth; - - if (!validate_buffer( buffer, buflen, sizeof(cert_chain) )) return FALSE; - if (!request->server_cert) + if (!validate_buffer( buffer, buflen, sizeof(chain) )) return FALSE; + if (!request->netconn || !request->netconn->chain) { SetLastError( ERROR_WINHTTP_INCORRECT_HANDLE_STATE ); *(CERT_CHAIN_CONTEXT **)buffer = NULL; return FALSE; } - if (!CertGetCertificateChain(NULL, request->server_cert, NULL, NULL, &chainPara, 0, NULL, &cert_chain)) return FALSE; - - *(CERT_CHAIN_CONTEXT **)buffer = (CERT_CHAIN_CONTEXT *)cert_chain; - *buflen = sizeof(cert_chain); - + if (!(chain = CertDuplicateCertificateChain( request->netconn->chain ))) return FALSE; + *(const CERT_CHAIN_CONTEXT **)buffer = chain; + *buflen = sizeof(chain); return TRUE; } case WINHTTP_OPTION_SECURITY_CERTIFICATE_STRUCT: diff --git a/dlls/winhttp/tests/winhttp.c b/dlls/winhttp/tests/winhttp.c index 4caf2abee38..ee868e65f55 100644 --- a/dlls/winhttp/tests/winhttp.c +++ b/dlls/winhttp/tests/winhttp.c @@ -6528,6 +6528,8 @@ START_TEST (winhttp) return; } +test_secure_connection(); +return; test_IWinHttpRequest(si.port); test_connection_info(si.port); test_basic_request(si.port, NULL, L"/basic"); diff --git a/dlls/winhttp/winhttp_private.h b/dlls/winhttp/winhttp_private.h index 99f57749856..bf94c650744 100644 --- a/dlls/winhttp/winhttp_private.h +++ b/dlls/winhttp/winhttp_private.h @@ -110,6 +110,7 @@ struct netconn struct hostdata *host; ULONGLONG keep_until; CtxtHandle ssl_ctx; + const CERT_CHAIN_CONTEXT *chain; SecPkgContext_StreamSizes ssl_sizes; char *ssl_read_buf, *ssl_write_buf; char *extra_buf; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/11546
v3: - remove NULL request check; - use 'const CERT_CHAIN_CONTEXT *' in netconn. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/11546#note_147889
Hans Leidekker (@hans) commented about dlls/winhttp/tests/winhttp.c:
return; }
+test_secure_connection(); +return;
Sorry, can't approve that ;-) -- https://gitlab.winehq.org/wine/wine/-/merge_requests/11546#note_147892
participants (3)
-
Hans Leidekker (＠hans) -
Paul Gofman
-
Paul Gofman (＠gofman)