Always tamper the signature in the data area #118357
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR fixes a flaky test issue in X509 certificate revocation tests by changing how signatures are tampered with during testing. The change addresses a race condition where tampering with byte 5 of ECDSA signatures could occasionally produce structurally invalid DER, causing OpenSSL to report different errors than expected.
Key changes:
- Modified signature tampering to target the second-to-last byte instead of byte 5
- Ensures tampering always affects signature data rather than DER structure
- Provides consistent test behavior across different cryptographic backends
|
Tagging subscribers to this area: @dotnet/area-system-security, @bartonjs, @vcsjones |
|
/azp run runtime-libraries-coreclr outerloop |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
Looks good to me, I just want to see it green in CI. |
While the outerloop run itself wasn't green, all of the failures are known (per Build Analysis) and none of the Linux failures were in crypto (manual inspection). |
When tests such as System.Security.Cryptography.X509Certificates.Tests.RevocationTests.DynamicRevocationTests.RevokeEndEntityWithInvalidRevocationSignature ran on EC-DSA with secp384r1, there is a roughly 50% chance that flipping all of the bits in signature[5] will produce a signature value that is not structurally valid DER by making the first 9 bits of an INTEGER all have the same value.
OpenSSL bubbles up this error differently from "it is structurally valid, but the signature is wrong", which makes our tests fail.
While there is some value in making these sorts of tamper look the same on Windows and Linux, the most important tamper is "the data was tampered with, invalidating the unchanged signature". That's the scenario these tests are trying to test (by making the signature wrong), so instead do the tamper at the second-to-last byte of the signature, which is always going to be in the "data" segment of a signature, never "structure" (for any known algorithm).
Fixes #117243.