refactor ocsp request parsing and generation to support only one cert (#4439)
* refactor ocsp request parsing and generation to support only one cert
* small doc change
* notimplementederror
diff --git a/docs/x509/ocsp.rst b/docs/x509/ocsp.rst
index afbb2ef..80abf16 100644
--- a/docs/x509/ocsp.rst
+++ b/docs/x509/ocsp.rst
@@ -97,8 +97,7 @@
>>> from cryptography.x509 import ocsp
>>> ocsp_req = ocsp.load_der_ocsp_request(der_ocsp_req)
- >>> for request in ocsp_req:
- ... print(request.serial_number)
+ >>> print(ocsp_req.serial_number)
872625873161273451176241581705670534707360122361
@@ -113,10 +112,10 @@
objects.
- .. method:: add_request(cert, issuer, algorithm)
+ .. method:: add_certificate(cert, issuer, algorithm)
Adds a request using a certificate, issuer certificate, and hash
- algorithm.
+ algorithm. This can only be called once.
:param cert: The :class:`~cryptography.x509.Certificate` whose validity
is being checked.
@@ -141,15 +140,16 @@
>>> from cryptography.hazmat.backends import default_backend
>>> from cryptography.hazmat.primitives import serialization
- >>> from cryptography.hazmat.primitives.hashes import SHA256
+ >>> from cryptography.hazmat.primitives.hashes import SHA1
>>> from cryptography.x509 import load_pem_x509_certificate, ocsp
>>> cert = load_pem_x509_certificate(pem_cert, default_backend())
>>> issuer = load_pem_x509_certificate(pem_issuer, default_backend())
>>> builder = ocsp.OCSPRequestBuilder()
- >>> builder = builder.add_request(cert, issuer, SHA256())
+ >>> # SHA1 is in this example because RFC 5019 mandates its use.
+ >>> builder = builder.add_certificate(cert, issuer, SHA1())
>>> req = builder.build()
>>> base64.b64encode(req.public_bytes(serialization.Encoding.DER))
- b'MF8wXTBbMFkwVzANBglghkgBZQMEAgEFAAQgn3BowBaoh77h17ULfkX6781dUDPD82Taj8wO1jZWhZoEINxPgjoQth3w7q4AouKKerMxIMIuUG4EuWU2pZfwih52AgI/IA=='
+ b'MEMwQTA/MD0wOzAJBgUrDgMCGgUABBRAC0Z68eay0wmDug1gfn5ZN0gkxAQUw5zz/NNGCDS7zkZ/oHxb8+IIy1kCAj8g'
Interfaces
@@ -159,24 +159,8 @@
.. versionadded:: 2.4
- An ``OCSPRequest`` is an iterable containing one or more
- :class:`~cryptography.x509.ocsp.Request` objects.
-
- .. method:: public_bytes(encoding)
-
- :param encoding: The encoding to use. Only
- :attr:`~cryptography.hazmat.primitives.serialization.Encoding.DER`
- is supported.
-
- :return bytes: The serialized OCSP request.
-
-.. class:: Request
-
- .. versionadded:: 2.4
-
- A ``Request`` contains several attributes that create a unique identifier
- for a certificate whose status is being checked. It may also contain
- additional extensions (currently unsupported).
+ An ``OCSPRequest`` is an object containing information about a certificate
+ whose status is being checked.
.. attribute:: issuer_key_hash
@@ -205,3 +189,11 @@
:type: int
The serial number of the certificate to check.
+
+ .. method:: public_bytes(encoding)
+
+ :param encoding: The encoding to use. Only
+ :attr:`~cryptography.hazmat.primitives.serialization.Encoding.DER`
+ is supported.
+
+ :return bytes: The serialized OCSP request.
diff --git a/src/cryptography/hazmat/backends/openssl/backend.py b/src/cryptography/hazmat/backends/openssl/backend.py
index cfd7c89..64d26af 100644
--- a/src/cryptography/hazmat/backends/openssl/backend.py
+++ b/src/cryptography/hazmat/backends/openssl/backend.py
@@ -1439,16 +1439,16 @@
ocsp_req = self._lib.OCSP_REQUEST_new()
self.openssl_assert(ocsp_req != self._ffi.NULL)
ocsp_req = self._ffi.gc(ocsp_req, self._lib.OCSP_REQUEST_free)
- for cert, issuer, algorithm in builder._requests:
- evp_md = self._lib.EVP_get_digestbyname(
- algorithm.name.encode("ascii"))
- self.openssl_assert(evp_md != self._ffi.NULL)
- certid = self._lib.OCSP_cert_to_id(
- evp_md, cert._x509, issuer._x509
- )
- self.openssl_assert(certid != self._ffi.NULL)
- onereq = self._lib.OCSP_request_add0_id(ocsp_req, certid)
- self.openssl_assert(onereq != self._ffi.NULL)
+ cert, issuer, algorithm = builder._request
+ evp_md = self._lib.EVP_get_digestbyname(
+ algorithm.name.encode("ascii"))
+ self.openssl_assert(evp_md != self._ffi.NULL)
+ certid = self._lib.OCSP_cert_to_id(
+ evp_md, cert._x509, issuer._x509
+ )
+ self.openssl_assert(certid != self._ffi.NULL)
+ onereq = self._lib.OCSP_request_add0_id(ocsp_req, certid)
+ self.openssl_assert(onereq != self._ffi.NULL)
return _OCSPRequest(self, ocsp_req)
def elliptic_curve_exchange_algorithm_supported(self, algorithm, curve):
diff --git a/src/cryptography/hazmat/backends/openssl/ocsp.py b/src/cryptography/hazmat/backends/openssl/ocsp.py
index 38e871e..dd66e36 100644
--- a/src/cryptography/hazmat/backends/openssl/ocsp.py
+++ b/src/cryptography/hazmat/backends/openssl/ocsp.py
@@ -4,23 +4,28 @@
from __future__ import absolute_import, division, print_function
-import operator
-
from cryptography import utils
from cryptography.exceptions import UnsupportedAlgorithm
from cryptography.hazmat.backends.openssl.decode_asn1 import (
_asn1_integer_to_int, _asn1_string_to_bytes, _obj2txt
)
from cryptography.hazmat.primitives import serialization
-from cryptography.x509.ocsp import OCSPRequest, Request, _OIDS_TO_HASH
+from cryptography.x509.ocsp import OCSPRequest, _OIDS_TO_HASH
-@utils.register_interface(Request)
-class _Request(object):
- def __init__(self, backend, ocsp_request, request):
+@utils.register_interface(OCSPRequest)
+class _OCSPRequest(object):
+ def __init__(self, backend, ocsp_request):
+ if backend._lib.OCSP_request_onereq_count(ocsp_request) > 1:
+ raise NotImplementedError(
+ 'OCSP request contains more than one request'
+ )
self._backend = backend
self._ocsp_request = ocsp_request
- self._request = request
+ self._request = self._backend._lib.OCSP_request_onereq_get0(
+ self._ocsp_request, 0
+ )
+ self._backend.openssl_assert(self._request != self._backend._ffi.NULL)
self._cert_id = self._backend._lib.OCSP_onereq_get0_id(self._request)
self._backend.openssl_assert(self._cert_id != self._backend._ffi.NULL)
@@ -74,23 +79,6 @@
"Signature algorithm OID: {0} not recognized".format(oid)
)
-
-@utils.register_interface(OCSPRequest)
-class _OCSPRequest(object):
- def __init__(self, backend, ocsp_request):
- self._backend = backend
- self._ocsp_request = ocsp_request
-
- def __len__(self):
- return self._backend._lib.OCSP_request_onereq_count(self._ocsp_request)
-
- def _request(self, idx):
- request = self._backend._lib.OCSP_request_onereq_get0(
- self._ocsp_request, idx
- )
- self._backend.openssl_assert(request != self._backend._ffi.NULL)
- return _Request(self._backend, self._ocsp_request, request)
-
def public_bytes(self, encoding):
if encoding is not serialization.Encoding.DER:
raise ValueError(
@@ -101,19 +89,3 @@
res = self._backend._lib.i2d_OCSP_REQUEST_bio(bio, self._ocsp_request)
self._backend.openssl_assert(res > 0)
return self._backend._read_mem_bio(bio)
-
- def __iter__(self):
- for i in range(len(self)):
- yield self._request(i)
-
- def __getitem__(self, idx):
- if isinstance(idx, slice):
- start, stop, step = idx.indices(len(self))
- return [self._request(i) for i in range(start, stop, step)]
- else:
- idx = operator.index(idx)
- if idx < 0:
- idx += len(self)
- if not 0 <= idx < len(self):
- raise IndexError
- return self._request(idx)
diff --git a/src/cryptography/x509/ocsp.py b/src/cryptography/x509/ocsp.py
index 0567197..c3225da 100644
--- a/src/cryptography/x509/ocsp.py
+++ b/src/cryptography/x509/ocsp.py
@@ -27,10 +27,13 @@
class OCSPRequestBuilder(object):
- def __init__(self, requests=[]):
- self._requests = requests
+ def __init__(self, request=None):
+ self._request = request
- def add_request(self, cert, issuer, algorithm):
+ def add_certificate(self, cert, issuer, algorithm):
+ if self._request is not None:
+ raise ValueError("Only one certificate can be added to a request")
+
allowed_hashes = (
hashes.SHA1, hashes.SHA224, hashes.SHA256,
hashes.SHA384, hashes.SHA512
@@ -45,45 +48,18 @@
):
raise TypeError("cert and issuer must be a Certificate")
- return OCSPRequestBuilder(self._requests + [(cert, issuer, algorithm)])
+ return OCSPRequestBuilder((cert, issuer, algorithm))
def build(self):
from cryptography.hazmat.backends.openssl.backend import backend
- if len(self._requests) == 0:
- raise ValueError("You must add a request before building")
+ if self._request is None:
+ raise ValueError("You must add a certificate before building")
return backend.create_ocsp_request(self)
@six.add_metaclass(abc.ABCMeta)
class OCSPRequest(object):
- @abc.abstractmethod
- def __iter__(self):
- """
- Iteration of Requests
- """
-
- @abc.abstractmethod
- def __len__(self):
- """
- Number of Requests inside the OCSPRequest object
- """
-
- @abc.abstractmethod
- def __getitem__(self, idx):
- """
- Returns a Request or range of Requests
- """
-
- @abc.abstractmethod
- def public_bytes(self, encoding):
- """
- Serializes the request to DER
- """
-
-
-@six.add_metaclass(abc.ABCMeta)
-class Request(object):
@abc.abstractproperty
def issuer_key_hash(self):
"""
@@ -107,3 +83,8 @@
"""
The serial number of the cert whose status is being checked
"""
+ @abc.abstractmethod
+ def public_bytes(self, encoding):
+ """
+ Serializes the request to DER
+ """
diff --git a/tests/x509/test_ocsp.py b/tests/x509/test_ocsp.py
index 709ef6f..3e6ac9c 100644
--- a/tests/x509/test_ocsp.py
+++ b/tests/x509/test_ocsp.py
@@ -46,64 +46,26 @@
with pytest.raises(ValueError):
ocsp.load_der_ocsp_request(b"invalid")
- def test_load_request_one_item(self):
+ def test_load_request(self):
req = _load_data(
os.path.join("x509", "ocsp", "req-sha1.der"),
ocsp.load_der_ocsp_request,
)
- assert len(req) == 1
- assert req[0].issuer_name_hash == (b"8\xcaF\x8c\x07D\x8d\xf4\x81\x96"
- b"\xc7mmLpQ\x9e`\xa7\xbd")
- assert req[0].issuer_key_hash == (b"yu\xbb\x84:\xcb,\xdez\t\xbe1"
- b"\x1bC\xbc\x1c*MSX")
- assert isinstance(req[0].hash_algorithm, hashes.SHA1)
- assert req[0].serial_number == int(
+ assert req.issuer_name_hash == (b"8\xcaF\x8c\x07D\x8d\xf4\x81\x96"
+ b"\xc7mmLpQ\x9e`\xa7\xbd")
+ assert req.issuer_key_hash == (b"yu\xbb\x84:\xcb,\xdez\t\xbe1"
+ b"\x1bC\xbc\x1c*MSX")
+ assert isinstance(req.hash_algorithm, hashes.SHA1)
+ assert req.serial_number == int(
"98D9E5C0B4C373552DF77C5D0F1EB5128E4945F9", 16
)
- def test_load_request_multiple_items(self):
- req = _load_data(
- os.path.join("x509", "ocsp", "req-multi-sha1.der"),
- ocsp.load_der_ocsp_request,
- )
- assert len(req) == 2
- assert req[0].issuer_name_hash == (b"8\xcaF\x8c\x07D\x8d\xf4\x81\x96"
- b"\xc7mmLpQ\x9e`\xa7\xbd")
- assert req[0].issuer_key_hash == (b"yu\xbb\x84:\xcb,\xdez\t\xbe1"
- b"\x1bC\xbc\x1c*MSX")
- assert isinstance(req[0].hash_algorithm, hashes.SHA1)
- assert req[0].serial_number == int(
- "98D9E5C0B4C373552DF77C5D0F1EB5128E4945F9", 16
- )
- assert req[1].issuer_name_hash == (b"8\xcaF\x8c\x07D\x8d\xf4\x81\x96"
- b"\xc7mmLpQ\x9e`\xa7\xbd")
- assert req[1].issuer_key_hash == (b"yu\xbb\x84:\xcb,\xdez\t\xbe1"
- b"\x1bC\xbc\x1c*MSX")
- assert isinstance(req[1].hash_algorithm, hashes.SHA1)
- assert req[1].serial_number == int(
- "98D9E5C0B4C373552DF77C5D0F1EB5128E4945F0", 16
- )
-
- def test_iter(self):
- req = _load_data(
- os.path.join("x509", "ocsp", "req-multi-sha1.der"),
- ocsp.load_der_ocsp_request,
- )
- for request in req:
- assert isinstance(request, ocsp.Request)
-
- def test_indexing_ocsp_request(self):
- req = _load_data(
- os.path.join("x509", "ocsp", "req-multi-sha1.der"),
- ocsp.load_der_ocsp_request,
- )
- assert req[1].serial_number == req[-1].serial_number
- assert len(req[0:2]) == 2
- assert req[1:2][0].serial_number == int(
- "98D9E5C0B4C373552DF77C5D0F1EB5128E4945F0", 16
- )
- with pytest.raises(IndexError):
- req[10]
+ def test_load_request_two_requests(self):
+ with pytest.raises(NotImplementedError):
+ _load_data(
+ os.path.join("x509", "ocsp", "req-multi-sha1.der"),
+ ocsp.load_der_ocsp_request,
+ )
def test_invalid_hash_algorithm(self):
req = _load_data(
@@ -111,7 +73,7 @@
ocsp.load_der_ocsp_request,
)
with pytest.raises(UnsupportedAlgorithm):
- req[0].hash_algorithm
+ req.hash_algorithm
def test_serialize_request(self):
req_bytes = load_vectors_from_file(
@@ -134,6 +96,13 @@
class TestOCSPRequestBuilder(object):
+ def test_add_two_certs(self):
+ cert, issuer = _cert_and_issuer()
+ builder = ocsp.OCSPRequestBuilder()
+ builder = builder.add_certificate(cert, issuer, hashes.SHA1())
+ with pytest.raises(ValueError):
+ builder.add_certificate(cert, issuer, hashes.SHA1())
+
def test_create_ocsp_request_no_req(self):
builder = ocsp.OCSPRequestBuilder()
with pytest.raises(ValueError):
@@ -143,37 +112,24 @@
cert, issuer = _cert_and_issuer()
builder = ocsp.OCSPRequestBuilder()
with pytest.raises(ValueError):
- builder.add_request(cert, issuer, hashes.MD5())
+ builder.add_certificate(cert, issuer, hashes.MD5())
def test_create_ocsp_request_invalid_cert(self):
cert, issuer = _cert_and_issuer()
builder = ocsp.OCSPRequestBuilder()
with pytest.raises(TypeError):
- builder.add_request(b"notacert", issuer, hashes.SHA1())
+ builder.add_certificate(b"notacert", issuer, hashes.SHA1())
with pytest.raises(TypeError):
- builder.add_request(cert, b"notacert", hashes.SHA1())
+ builder.add_certificate(cert, b"notacert", hashes.SHA1())
def test_create_ocsp_request(self):
cert, issuer = _cert_and_issuer()
builder = ocsp.OCSPRequestBuilder()
- builder = builder.add_request(cert, issuer, hashes.SHA1())
+ builder = builder.add_certificate(cert, issuer, hashes.SHA1())
req = builder.build()
serialized = req.public_bytes(serialization.Encoding.DER)
assert serialized == base64.b64decode(
b"MEMwQTA/MD0wOzAJBgUrDgMCGgUABBRAC0Z68eay0wmDug1gfn5ZN0gkxAQUw5zz"
b"/NNGCDS7zkZ/oHxb8+IIy1kCAj8g"
)
-
- def test_create_ocsp_request_two_reqs(self):
- builder = ocsp.OCSPRequestBuilder()
- cert, issuer = _cert_and_issuer()
- builder = builder.add_request(cert, issuer, hashes.SHA1())
- builder = builder.add_request(cert, issuer, hashes.SHA1())
- req = builder.build()
- serialized = req.public_bytes(serialization.Encoding.DER)
- assert serialized == base64.b64decode(
- b"MIGDMIGAMH4wPTA7MAkGBSsOAwIaBQAEFEALRnrx5rLTCYO6DWB+flk3SCTEBBTD"
- b"nPP800YINLvORn+gfFvz4gjLWQICPyAwPTA7MAkGBSsOAwIaBQAEFEALRnrx5rLT"
- b"CYO6DWB+flk3SCTEBBTDnPP800YINLvORn+gfFvz4gjLWQICPyA="
- )