wbond / certvalidator

Python library for validating X.509 certificates and paths
MIT License
107 stars 31 forks source link

URLError and socket.error are expected, but only partially handled #19

Open atmenta opened 4 years ago

atmenta commented 4 years ago

If revocation mode is not "soft-fail", URLError and socket.error are reraised from context.retrieve_crls:

https://github.com/wbond/certvalidator/blob/5bc5c390c1955195507c23db91b8926bb03f7385/certvalidator/context.py#L467-L473

and from context.retrieve_ocsps:

https://github.com/wbond/certvalidator/blob/5bc5c390c1955195507c23db91b8926bb03f7385/certvalidator/context.py#L506-L512

Callers (validate.verify_crl, validate.verify_ocsp_response, and finally validate._validate_path) don't handle these errors, so they may escape from certvalidator, which is probably not intentional and is not documented.

wbond commented 4 years ago

If the revocation mode is in hard mode, as opposed to soft-fail, I would expect the calling code to handle these. Since a hard failure was requested, the calling code will have to decide what to do since the revocation code failed due to an error with HTTP.

I believe the fix for these is to add :raises: entries.

atmenta commented 4 years ago

If the revocation mode is in hard mode, as opposed to soft-fail, I would expect the calling code to handle these.

I agree, the calling code should be prepared to handle these exceptions. Documenting that those exceptions may be raised could help.

An other solution could be catching those (and other "low level") exceptions, (e.g. TypeError raised by ocsp_client.fetch) and raise a more certvalidator specific exception (e.g. PathValidationError) instead of them.

Since a hard failure was requested, the calling code will have to decide what to do since the revocation code failed due to an error with HTTP.

I would argue that certvalidator should do its best effort to verify revocation status even if revocation mode is "hard-fail" or "require", and let the calling code manage only those errors which are not manageable by itself. For example, if there are multiple potential sources of revocation status (e.g. OCSP and CRL, or multiple OCSP responders/CRLs) and checking one of them fails due to any error, checking other sources could be attempted. If revocation checking based on an other source of information succeeds, the whole process could be considered successful, and no error should be raised in the direction of the caller code. Failure should be reported only if obtaining revocation status from all known sources of revocation information was attempted, but failed.

Currently, if URLError or socket.error is raised during fetching an OCSP response, CRL checking is not even attempted, since these exceptions fall through certvalidator. This behavior is problematic not only because not all sources of revocation information have been checked, but also since currently there is no way to tell certvalidator which source of revocation it should prefer.