x509 Verification: Python support for custom ext. policies. #12418
Conversation
b6e74ff to
6f646bf
Compare
|
This is missing interface definitions in x509.pyi and tests for the new API, but should otherwise be functional. Feel free to review the rust implementation in the meantime. |
|
@alex This was named wrong until now. I could swear I already edited the title of this PR before and it changed back to the incorrect one... So hopefully it sticks this time 😄 This is the main PR that adds support for custom extension policies to python. |
581ee05 to
5ef0b92
Compare
|
@alex Do you have any ideas on how to split this up further, or is this one of the rare cases where you guys can accept a large PR? I have thought about it for a bit, but it seems pretty monolithic to me. I'm sure some things could be separated out, but none of the options seem to make much sense to me - e.g. would require to write additional tests for things that are currently tested "organically", etc. Unfortunately I expect this to grow a bit more as more tests are needed. |
b5ec04e to
5ba8e35
Compare
| let py_ext = match ext { | ||
| None => None, | ||
| Some(ext) => parse_cert_ext(py, ext).map_err(|e| { | ||
| make_validation_error(format!("{e} (while converting Extension to Python object)")) |
There was a problem hiding this comment.
Do you maybe have any ideas on how to handle this? This should be the last part that doesn't have coverage. I'm struggling to come up with a way to test this in a sane way, or to prove that this can not occur (and thus make this an unwrap). Suggestions much appreciated.
5ba8e35 to
8aa5eb7
Compare
|
Rebased on main. |
|
Btw, just occurred to me that I can potentially split out the .pyi edits since that doesn't really affect anything else. But it seems weird to me to have the interface in main without the implementation. |
|
Yeah, we should land the Thanks! |
|
Just realized another piece that could be split out: Adding ExtensionPolicy, but without the methods for actually configuring it, so all you can do is create either WebPKI or allow all, and attach it to a PolicyBuilder. That should let us get most of the structure, and then it's just the callbacks to be reviewed as the last piece. I think it roughly splits the PR in two pieces. |
Great idea, will do! |
|
Closing to declutter, as I already created the 2 new PRs. |
This is still missing a lot. Planned for after #12416 and #12417.
Pushing this now so this can be consulted to better understand why some of the changes in #12417 are needed (the version in #12360 is quite outdated by now)