Fix #52093: openssl_csr_sign silently truncates $serial#7209
Fix #52093: openssl_csr_sign silently truncates $serial#7209cmb69 wants to merge 3 commits intophp:PHP-7.4from
Conversation
We must not silently truncate integer arguments passed to a function. In this case we could use `ASN1_INTEGER_set_int64()` on LLP64 architectures, but that might be regarded a BC break; instead we choose to raise a warning regarding the truncation for now.
|
Hmm, we must skip the tests on architectures where no truncation occurs. Not sure about the best skipif clause – maybe just run on Windows x64? |
|
Using ASN1_INTEGER_set_int64() seems better here, and I don't think that counts as a BC break. If I understand correctly, then this will already work fine on platforms where long is 64-bit, so effectively this means that currently 64-bit Windows and 64-bit Linux behave differently, which seems like a plain bug. |
We actually use `ASN1_INTEGER_set_int64()`[1] on 64bit Windows which is the only LLP64 platform we support, and where we can be reasonably sure that nobody uses OpenSSL 1.0 anymore (the official Windows dependencies ship OppenSSL 1.1 for years). We also adapt the test to actually verify the serial number. [1] <https://www.openssl.org/docs/man1.1.0/man3/ASN1_INTEGER_set_int64.html#HISTORY>
|
Fine! I switched to using |
|
@cmb69 we are getting reports that this crashes on LibreSSL 3.3 builds whereas previously we never had issues with LibreSSL compat before... I'm trying to get to the bottom of this now. :) |
|
Found it 6724d5d4c2c502b09 sorry for the noise! |
3 participants
We must not silently truncate integer arguments passed to a function.
In this case we could use
ASN1_INTEGER_set_int64()on LLP64architectures, but that might be regarded a BC break; instead we choose
to raise a warning regarding the truncation for now.