-
Notifications
You must be signed in to change notification settings - Fork 318
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Improved verify #1779
Improved verify #1779
Conversation
…ort in better detail on the certs. Ensured proper returns of False (revoked) True (good) None (unknown) throughout the methods.
Build appears to be failing in test-cli, I don't think it's related to anything in this PR. test-python locally passes fine. |
This appears to be an issue with lint. Try running flake8. The specific error in Travis-CI is: --> Linting Python files |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good! Will merge. Thank you
Redoing this completely since I broke the last one.
This PR addresses a few issues in the CLI verification process, and #1079
Stacked exceptions were yielding lots of tracebacks
Rather than using exceptions for non-error failure states, I've updated the code to use return values (False = revoked, True = good, None = unknown). Exceptions were left in place where they are for unexpected errors (parse errors, http errors, etc.)
Verifying a large number of certificates was causing slowdowns as the code was pulling the same CRL over and over for certs with the same CRL.
I implemented an extremely simple cache so that an individual CRL is only downloaded once. This works fine for the CLI where the code is loaded fresh each time but it could be a problem if the verify methods are used by the running web app, as the cache has no update/expire feature. Currently, verification is only in use by the CLI code.
Better logging
Added debug logging output in case anyone needs to troubleshoot the process. Also, parse the cert in verify() and pass it to crl_verify() and ocsp_cerify() so they both have access to cert data for logging purposes.
Tests, flake8 and docstrings have been updated since the previous PR and master is merged in.