verification: add skeleton for revocation checking#14501
verification: add skeleton for revocation checking#14501
Conversation
There was a problem hiding this comment.
I'd appreciate thoughts on the general approach here with revocation checker ownership. I assume that we eventually want to instantiate RevocationChecker for OCSP, so I patterned everything after the existing Subject type. It feels a little bit clunky.
There was a problem hiding this comment.
It's not clear to me that the RevocationCheckerOwner is actually doing any work here? It only ever lives on the stack AFAICT, which suggests it shouldn't be needed.
There was a problem hiding this comment.
Maybe there's a better name for it, but RevocationCheckerOwner helps us by holding the concrete PyCrlRevocationChecker, which we then can borrow a CrlRevocationChecker from to pass to cryptography-x509-verification. Another way we could do this is by holding a plain PyCrlRevocationChecker, but we'd have to figure something out in the future to accept multiple RevocationChecker impls. I originally wanted to directly borrow a CrlRevocationChecker out of the PyAny but couldn't find a way to; I'm not very handy with pyo3 😅
There was a problem hiding this comment.
Scratch the above, I had an idea that makes everything simpler and allows for custom Python revocation checkers :)
| class RevocationChecker(metaclass=abc.ABCMeta): | ||
| """ | ||
| An interface for revocation checkers. | ||
| """ |
There was a problem hiding this comment.
So right now the idea is that we won't actually have a Python API you can implement for revocation checking, it'll be handled internally and these are more markers, is that the idea?
There was a problem hiding this comment.
Correct, sorry that this isn't more clear—I experimented with having an extensible Python API but it was a bit complicated to shim over considering that the implementation for CRL is going to be in Rust. I can sketch out a shim if you'd like? see latest diff which supports Python-extensible revocation checkers
There was a problem hiding this comment.
It's not clear to me that the RevocationCheckerOwner is actually doing any work here? It only ever lives on the stack AFAICT, which suggests it shouldn't be needed.
2 participants
This is an outline of the CRL revocation API that was discussed in #10393. I'd like to get some feedback on the general approach before we go further and fill in the revocation algorithm.