[Bug 54353] New: crypt32:cert - testVerifyRevocation() gets unexpected success in Wine on second run
https://bugs.winehq.org/show_bug.cgi?id=54353
Bug ID: 54353 Summary: crypt32:cert - testVerifyRevocation() gets unexpected success in Wine on second run Product: Wine Version: unspecified Hardware: x86-64 OS: Linux Status: NEW Severity: normal Priority: P2 Component: crypt32 Assignee: wine-bugs@winehq.org Reporter: fgouget@codeweavers.com Distribution: ---
crypt32:cert - testVerifyRevocation() gets unexpected success in some Wine configurations:
cert.c:3910: Test failed: success cert.c:3911: Test failed: got 00000000
See https://test.winehq.org/data/patterns.html#crypt32:cert
In fact the test only succeeds on the first run in a given wineprefix. All later runs fail:
$ rm -rf ~/.wineprefix $ ./wine dlls/crypt32/tests/i386-windows/crypt32_test.exe cert 0700:cert:0.615 608 tests executed (0 marked as todo, 0 as flaky, 0 failures), 0 skipped.
$ ./wine dlls/crypt32/tests/i386-windows/crypt32_test.exe cert cert.c:3910: Test failed: success cert.c:3911: Test failed: got 00000000 078c:cert:0.242 608 tests executed (0 marked as todo, 0 as flaky, 2 failures), 0 skipped. # Running the test again gets the same failures
This is why the test.winehq.org only has failures for my fg_deb64-wow32 test configuration: on my desktop I run the wow64 tests first, and then the wow32 tests in the same wineprefix.
A bisect shows that these failures were introduced by the commit below:
commit 67de946ca9bdc8f39adffca57a336165936e62e2 (refs/bisect/bad) Author: Paul Gofman pgofman@codeweavers.com Date: Wed Jan 11 14:00:48 2023 -0600
cryptnet: Check cached revocation status in verify_cert_revocation().
That is, before this commit, running the test twice in a row was not an issue.
https://bugs.winehq.org/show_bug.cgi?id=54353
François Gouget fgouget@codeweavers.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Keywords| |regression, source, | |testcase Regression SHA1| |67de946ca9bdc8f39adffca57a3 | |36165936e62e2
https://bugs.winehq.org/show_bug.cgi?id=54353
François Gouget fgouget@codeweavers.com changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |pgofman@codeweavers.com
https://bugs.winehq.org/show_bug.cgi?id=54353
--- Comment #1 from Paul Gofman pgofman@codeweavers.com --- I've looked into this. The core issue is pre-existing before the blamed commit, although that one made it in seen in repeated test by extending the scope of caching (before the blamed commit it was only affecting CRL revocation checking and we just don't have a test which would hit the issue).
The reason is that there are two checks performed by the test for the same certificate: one without issuer certificate provided in check parameters and another one with that one. Caching uses serial number of the cert being checked as id in cache, so once second check succeeded the cached result is matched for the first check (which should fail).
I see two issues here:
1. Using cert's serial is wrong, we need to hash full certificate contents to identify the cert for the purpose of cache ID (this part does not relate to test failure though);
2. We also need to hash all the request parameters which may influence the check result (I think currently that is only issuer certificate in request parameters).
I will send patches soon.
https://bugs.winehq.org/show_bug.cgi?id=54353
--- Comment #2 from Paul Gofman pgofman@codeweavers.com --- Should be fixed by 66b1a4f333e0687922062cb859bd73e5545608bd.
https://bugs.winehq.org/show_bug.cgi?id=54353
Hans Leidekker hans@meelstraat.net changed:
What |Removed |Added ---------------------------------------------------------------------------- Resolution|--- |FIXED Status|NEW |RESOLVED Fixed by SHA1| |66b1a4f333e0687922062cb859b | |d73e5545608bd
--- Comment #3 from Hans Leidekker hans@meelstraat.net --- Fixed with 66b1a4f333e0687922062cb859bd73e5545608bd.
https://bugs.winehq.org/show_bug.cgi?id=54353
Alexandre Julliard julliard@winehq.org changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|RESOLVED |CLOSED
--- Comment #4 from Alexandre Julliard julliard@winehq.org --- Closing bugs fixed in 8.1.
https://bugs.winehq.org/show_bug.cgi?id=54353
Michael Stefaniuc mstefani@winehq.org changed:
What |Removed |Added ---------------------------------------------------------------------------- Target Milestone|--- |8.0.x
https://bugs.winehq.org/show_bug.cgi?id=54353
Michael Stefaniuc mstefani@winehq.org changed:
What |Removed |Added ---------------------------------------------------------------------------- Target Milestone|8.0.x |---
--- Comment #5 from Michael Stefaniuc mstefani@winehq.org --- Removing the 8.0.x milestone from bug fixes included in 8.0.1.
-
WineHQ Bugzilla