Disallow X509 certificate serial numbers bigger than 159 bits (#3064) (#3067)
diff --git a/src/cryptography/x509/base.py b/src/cryptography/x509/base.py
index ab70fe7..5c4e3aa 100644
--- a/src/cryptography/x509/base.py
+++ b/src/cryptography/x509/base.py
@@ -425,10 +425,13 @@
raise TypeError('Serial number must be of integral type.')
if self._serial_number is not None:
raise ValueError('The serial number may only be set once.')
- if number < 0:
- raise ValueError('The serial number should be non-negative.')
- if utils.bit_length(number) > 160: # As defined in RFC 5280
- raise ValueError('The serial number should not be more than 160 '
+ if number <= 0:
+ raise ValueError('The serial number should be positive.')
+
+ # ASN.1 integers are always signed, so most significant bit must be
+ # zero.
+ if utils.bit_length(number) >= 160: # As defined in RFC 5280
+ raise ValueError('The serial number should not be more than 159 '
'bits.')
return CertificateBuilder(
self._issuer_name, self._subject_name,
@@ -635,10 +638,13 @@
raise TypeError('Serial number must be of integral type.')
if self._serial_number is not None:
raise ValueError('The serial number may only be set once.')
- if number < 0:
- raise ValueError('The serial number should be non-negative.')
- if utils.bit_length(number) > 160: # As defined in RFC 5280
- raise ValueError('The serial number should not be more than 160 '
+ if number <= 0:
+ raise ValueError('The serial number should be positive')
+
+ # ASN.1 integers are always signed, so most significant bit must be
+ # zero.
+ if utils.bit_length(number) >= 160: # As defined in RFC 5280
+ raise ValueError('The serial number should not be more than 159 '
'bits.')
return RevokedCertificateBuilder(
number, self._revocation_date, self._extensions
diff --git a/tests/test_x509.py b/tests/test_x509.py
index 40efb6d..1ce8c61 100644
--- a/tests/test_x509.py
+++ b/tests/test_x509.py
@@ -1689,12 +1689,55 @@
def test_serial_number_must_be_non_negative(self):
with pytest.raises(ValueError):
- x509.CertificateBuilder().serial_number(-10)
+ x509.CertificateBuilder().serial_number(-1)
+
+ def test_serial_number_must_be_positive(self):
+ with pytest.raises(ValueError):
+ x509.CertificateBuilder().serial_number(0)
+
+ @pytest.mark.requires_backend_interface(interface=RSABackend)
+ @pytest.mark.requires_backend_interface(interface=X509Backend)
+ def test_minimal_serial_number(self, backend):
+ subject_private_key = RSA_KEY_2048.private_key(backend)
+ builder = x509.CertificateBuilder().serial_number(
+ 1
+ ).subject_name(x509.Name([
+ x509.NameAttribute(NameOID.COUNTRY_NAME, u'RU'),
+ ])).issuer_name(x509.Name([
+ x509.NameAttribute(NameOID.COUNTRY_NAME, u'RU'),
+ ])).public_key(
+ subject_private_key.public_key()
+ ).not_valid_before(
+ datetime.datetime(2002, 1, 1, 12, 1)
+ ).not_valid_after(
+ datetime.datetime(2030, 12, 31, 8, 30)
+ )
+ cert = builder.sign(subject_private_key, hashes.SHA256(), backend)
+ assert cert.serial_number == 1
+
+ @pytest.mark.requires_backend_interface(interface=RSABackend)
+ @pytest.mark.requires_backend_interface(interface=X509Backend)
+ def test_biggest_serial_number(self, backend):
+ subject_private_key = RSA_KEY_2048.private_key(backend)
+ builder = x509.CertificateBuilder().serial_number(
+ (1 << 159) - 1
+ ).subject_name(x509.Name([
+ x509.NameAttribute(NameOID.COUNTRY_NAME, u'RU'),
+ ])).issuer_name(x509.Name([
+ x509.NameAttribute(NameOID.COUNTRY_NAME, u'RU'),
+ ])).public_key(
+ subject_private_key.public_key()
+ ).not_valid_before(
+ datetime.datetime(2002, 1, 1, 12, 1)
+ ).not_valid_after(
+ datetime.datetime(2030, 12, 31, 8, 30)
+ )
+ cert = builder.sign(subject_private_key, hashes.SHA256(), backend)
+ assert cert.serial_number == (1 << 159) - 1
def test_serial_number_must_be_less_than_160_bits_long(self):
with pytest.raises(ValueError):
- # 2 raised to the 160th power is actually 161 bits
- x509.CertificateBuilder().serial_number(2 ** 160)
+ x509.CertificateBuilder().serial_number(1 << 159)
def test_serial_number_may_only_be_set_once(self):
builder = x509.CertificateBuilder().serial_number(10)
diff --git a/tests/test_x509_revokedcertbuilder.py b/tests/test_x509_revokedcertbuilder.py
index 5aa9063..bd64b60 100644
--- a/tests/test_x509_revokedcertbuilder.py
+++ b/tests/test_x509_revokedcertbuilder.py
@@ -21,10 +21,37 @@
with pytest.raises(ValueError):
x509.RevokedCertificateBuilder().serial_number(-1)
+ def test_serial_number_must_be_positive(self):
+ with pytest.raises(ValueError):
+ x509.RevokedCertificateBuilder().serial_number(0)
+
+ @pytest.mark.requires_backend_interface(interface=X509Backend)
+ def test_minimal_serial_number(self, backend):
+ revocation_date = datetime.datetime(2002, 1, 1, 12, 1)
+ builder = x509.RevokedCertificateBuilder().serial_number(
+ 1
+ ).revocation_date(
+ revocation_date
+ )
+
+ revoked_certificate = builder.build(backend)
+ assert revoked_certificate.serial_number == 1
+
+ @pytest.mark.requires_backend_interface(interface=X509Backend)
+ def test_biggest_serial_number(self, backend):
+ revocation_date = datetime.datetime(2002, 1, 1, 12, 1)
+ builder = x509.RevokedCertificateBuilder().serial_number(
+ (1 << 159) - 1
+ ).revocation_date(
+ revocation_date
+ )
+
+ revoked_certificate = builder.build(backend)
+ assert revoked_certificate.serial_number == (1 << 159) - 1
+
def test_serial_number_must_be_less_than_160_bits_long(self):
with pytest.raises(ValueError):
- # 2 raised to the 160th power is actually 161 bits
- x509.RevokedCertificateBuilder().serial_number(2 ** 160)
+ x509.RevokedCertificateBuilder().serial_number(1 << 159)
def test_set_serial_number_twice(self):
builder = x509.RevokedCertificateBuilder().serial_number(3)