Fix segfault in X509.set_serial_number for negative serials

[dylanpulver]

Problem

X509.set_serial_number() crashes the interpreter when passed a negative serial number:

from OpenSSL import crypto
crypto.X509().set_serial_number(-1)   # SIGSEGV

Reproduced on 26.4.0 from PyPI and on current main.

Cause

The hex string was built with hex(serial)[2:]. For negative values hex() emits -0x1, so the slice strips -0 and leaves "x1", which BN_hex2bn cannot parse. The failure went undetected because the guard was:

_openssl_assert(result != _ffi.NULL)

BN_hex2bn returns an int (characters consumed, 0 on error), not a pointer, so comparing it against _ffi.NULL is always true. The still-NULL BIGNUM was then handed to BN_to_ASN1_INTEGER, which dereferences it.

Fix

Format the digits with f"{serial:x}" — BN_hex2bn accepts a leading - — and check the return value against 0, as documented. get_serial_number() already round-trips negative values (BN_bn2hex emits -FF, which int(..., 16) parses), so this restores set/get symmetry rather than introducing new behaviour.

Test

test_negative_serial_number covers -1, -255 and -(2**128 + 1). Reverting the f"{serial:x}" change makes it die with SIGSEGV; correcting only the != 0 guard makes it fail with Error instead. Both changed lines are executed by the test.

A changelog entry is added under a new 26.5.0 (UNRELEASED) section.

Noting it since it is the obvious first question: this API is already deprecated in favour of cryptography's CertificateBuilder. It is still shipped and callable, and the failure mode is an interpreter crash rather than an exception, which is why it seemed worth two lines. Happy to close if you would rather leave the deprecated surface alone.

---

Prepared with assistance from Claude Code.

[reaperhulk]

Thanks for the submission. I think we need to fix this, but another potential option would be to error on any negative integer. This has the advantage of being RFC 5280 compliant, but the disadvantage of changing pyOpenSSL's (wrong) behavior. @alex what do you think?

[alex]

I'm pro just rejecting it All that is necessary for evil to succeed is for good people to do nothing.

…

On Tue, Sep 1, 2026, 11:53 AM Paul Kehrer ***@***.***> wrote: *reaperhulk* left a comment (pyca/pyopenssl#1534) <#1534 (comment)> Thanks for the submission. I think we need to fix this, but another potential option would be to error on any negative integer. This has the advantage of being RFC 5280 compliant, but the disadvantage of changing pyOpenSSL's (wrong) behavior. @alex <https://github.com/alex> what do you think? — Reply to this email directly, view it on GitHub <#1534?email_source=notifications&email_token=AAAAGBAHMTWSXFOQHNARAV35M3WH7A5CNFSNUABFM5UWIORPF5TWS5BNNB2WEL2JONZXKZKDN5WW2ZLOOQXTKNBZGY3DIOBWGM3KM4TFMFZW63VHNVSW45DJN5XKKZLWMVXHJLDGN5XXIZLSL5RWY2LDNM#issuecomment-5496648636>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAAAGBBKWTGWRVG67BNONGD5M3WH7AVCNFSNUABEKJSXA33TNF2G64TZHMYTKNZXHAYDKOJ3JFZXG5LFHM2TGMJTGUZTOMRRGOQXMAQ> . Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS <https://github.com/notifications/mobile/ios/AAAAGBB7GCK3HFAURAQFVET5M3WH7A5CNFSNUABFM5UWIORPF5TWS5BNNB2WEL2JONZXKZKDN5WW2ZLOOQXTKNBZGY3DIOBWGM3KM4TFMFZW63VHNVSW45DJN5XKKZLWMVXHJKTGN5XXIZLSL5UW64Y> and Android <https://github.com/notifications/mobile/android/AAAAGBA5OELTPEY5HWU44VT5M3WH7A5CNFSNUABFM5UWIORPF5TWS5BNNB2WEL2JONZXKZKDN5WW2ZLOOQXTKNBZGY3DIOBWGM3KM4TFMFZW63VHNVSW45DJN5XKKZLWMVXHJLTGN5XXIZLSL5QW4ZDSN5UWI>. Download it today! You are receiving this because you were mentioned.Message ID: ***@***.***>

[dylanpulver]

Happy to switch it to rejecting — RFC 5280 4.1.2.2 is unambiguous that the serial must be a positive integer, and it makes the failure loud instead of a segfault.

One fact worth weighing before I push it, since it is the asymmetry the change creates. get_serial_number already returns negative values: BN_bn2hex emits a leading - and line 1036 does int(hexstring_serial, 16). So a certificate parsed from the wire with a negative serial reads back fine today, and after this change it could not be round-tripped through set_serial_number. Non-conformant certificates like that do exist, and anyone re-serialising one would go from a working path to an exception.

If that is acceptable — and I can see the argument that it should be, since the value was never conformant — say the word and I will replace the patch with a ValueError on serial < 0, keep the regression test for the crash, and drop the negative round-trip test. If you would rather not change behaviour for existing callers, the current patch fixes the segfault without touching what is accepted.

Either version is fine by me; it is your call on the compatibility trade-off.

[reaperhulk]

It's fine to not round trip. Let's just reject.

[dylanpulver]

Done — pushed as a follow-up commit rather than a rewrite so the change is visible in the diff.

set_serial_number now raises ValueError for a negative serial, and the changelog entry moved to the backward-incompatible section since a value that previously round-tripped is now refused.

The test asserts the rejection for -1, -255 and -(2**128 + 1), and also that a serial set earlier survives a rejected call. Removing just the new guard makes it fail with crypto.Error instead of ValueError, so it pins the exception type rather than only the absence of the crash.

tests/test_crypto.py is 172 passed locally on Python 3.14; ruff 0.14.5 check and format --check are clean, though I could not find a pinned ruff version in the repo so that is just the version I ran. I have not run mypy, the docs build, or the pypy and Windows legs.

[reaperhulk]

Let's remove this comment.

[reaperhulk]

One outstanding request + need to rebase the changelog.

[dylanpulver]

Both done.

• Comment removed, as asked. I also trimmed the BN_hex2bn one next to it from three lines to two — it still records the fact the fix turns on (the call returns a count, not a pointer, so != _ffi.NULL was always true), but it was longer than the line it explains.
• Changelog rebased on main. Worth noting one thing I changed rather than carried over: the branch had moved the heading to 26.5.0 (UNRELEASED), and main is at 26.4.1. I kept main's heading — a bugfix PR should not be moving the version — and the entry sits in Backward-incompatible changes alongside the two new entries from main, untouched.

pytest tests/test_crypto.py is 172 passed. Removing the serial < 0 guard fails test_negative_serial_number, so it is pinned to the change rather than passing either way.