Ocsp bind to cert

[akhait]

This change is 

[tomato42]

This pull request introduces 1 alert when merging 55d120b into af46665 - view on LGTM.com

new alerts:

• 1 for Unused import

---

Comment posted by LGTM.com

[tomato42]

Reviewed 3 of 3 files at r1, 2 of 2 files at r2, 1 of 1 files at r3.
Reviewable status: all files reviewed, 6 unresolved discussions (waiting on @akhait)

a discussion (no related file):
negative tests (incorrect issuer certificate, incorrect server certificate, incorrect OCSP signer certificate) are missing, please add them

---

---

tlslite/ocsp.py, line 41 at r1 (raw file):

        tuple([0x2a, 0x86, 0x48, 0x86, 0xf7, 0xd, 0x2, 0x5]): 'md5',
        tuple([0x2b, 0xe, 0x3, 0x2, 0x1a]): 'sha1',
        tuple([0x60, 0x86, 0x48, 0x1, 0x65, 0x3, 0x4, 0x2, 0x4]): 'sha256',

looks like a typo for sha224

---

tlslite/ocsp.py, line 43 at r1 (raw file):

        tuple([0x60, 0x86, 0x48, 0x1, 0x65, 0x3, 0x4, 0x2, 0x4]): 'sha256',
        tuple([0x60, 0x86, 0x48, 0x1, 0x65, 0x3, 0x4, 0x2, 0x2]): 'sha384',
        tuple([0x60, 0x86, 0x48, 0x1, 0x65, 0x3, 0x4, 0x2, 0x1]): 'sha256',

nit: typically they are ordered sha256, sha384, sha512

---

tlslite/ocsp.py, line 70 at r1 (raw file):

        issuer_name = issuer_cert.subject

        alg = self._hash_algs_OIDs[tuple(self.cert_hash_alg)]

error handling missing

---

unit_tests/test_tlslite_ocsp.py, line 362 at r1 (raw file):

        server_cert_x509.parseBinary(server_cert)
        issuer_cert_x509 = X509()
        issuer_cert_x509.parseBinary(issuer_cert)

that server_cert and issues_cert is duplicated, wouldn't placing it in setUpClass() or setUp() be cleaner?

---

unit_tests/test_tlslite_ocsp.py, line 364 at r1 (raw file):

        issuer_cert_x509.parseBinary(issuer_cert)
        self.assertGreater(singleRespCnt, 0)
        for i in range(singleRespCnt):

why not for singleResp in resp.responses:?

[akhait]

Reviewable status: 2 of 4 files reviewed, 6 unresolved discussions (waiting on @tomato42 and @akhait)

---

unit_tests/test_tlslite_ocsp.py, line 362 at r1 (raw file):

Previously, tomato42 (Hubert Kario) wrote…

that server_cert and issues_cert is duplicated, wouldn't placing it in setUpClass() or setUp() be cleaner?

since I added tests for incorrect issuer and server certificates , those aren't the same in all tests no more. Should I still add server_cert and issuer_cert in setUpClass() and just re-define them in negative tests or leave it as it is?

[tomato42]

Reviewed 1 of 1 files at r4, 1 of 1 files at r5.
Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @akhait)

---

tlslite/ocsp.py, line 72 at r4 (raw file):

            alg = self._hash_algs_OIDs[tuple(self.cert_hash_alg)]
        except KeyError as e:
            raise KeyError("Unknown hash algorithm: {0}".format(

this should raise a tlslite native exception, something like TLSIllegalParameterException

---

unit_tests/test_tlslite_ocsp.py, line 362 at r1 (raw file):

Previously, akhait wrote…

since I added tests for incorrect issuer and server certificates , those aren't the same in all tests no more. Should I still add server_cert and issuer_cert in setUpClass() and just re-define them in negative tests or leave it as it is?

yes, they can be re-defined (or simply not used) in negative tests

[tomato42]

Reviewed 2 of 2 files at r6.
Reviewable status: complete! all files reviewed, all discussions resolved

[tomato42]

Looks good, thank you!