Comprehensive refactor to modern Python idioms - #27
Draft
kjd wants to merge 17 commits into
Draft
Conversation
kjd
commented
Apr 15, 2026
kjd
commented
Contributor
- Added CLI support
- Removed dependencies on openssl binary
- Added type support, code formatting, etc.
- Removed support for EOL Python versions
- Added Github Action based testing for pull requests
Replace the flat script with a proper Python package structure
(get_trust_anchor/{__init__,__main__,cli}.py) and add a
console_scripts entry point so `pip install` provides a
`get-trust-anchor` command. Also supports `python -m get_trust_anchor`.
Replaces setup.py with pyproject.toml and drops Python 2 support.
See also: #7
Replace Travis CI with GitHub Actions and add pytest test suite Specifcally, remove .travis.yml and add a GitHub Actions workflow that tests across Python 3.9-3.13. Add a pytest suite covering XML parsing, digest hashing, validity filtering, KSK matching, key tag calculation, file I/O, and end-to-end CLI regression against the reference output files.
Reflect the Python 3 requirement, pip-installable package, module invocation, and document all CLI options and output files. Add sections for installation, development, and how the tool works.
Rewrite PKCS7 signature verification to use the cryptography library instead of shelling out to the openssl command line tool. This removes the external tool dependency and works with in-memory data directly, eliminating the need for CA cert temp files. Includes a minimal DER parser to extract signer info from the PKCS7 structure, with full test coverage using dynamically generated certificates and signatures. Also fixes a pre-existing bug where --local-sig checked the wrong path (opts.local instead of opts.local_sig).
Replace pylint and the Makefile with ruff, configured in pyproject.toml. Ruff enforces formatting (double quotes, sorted imports, f-strings, line length) and lints for pycodestyle, pyflakes, isort, pyupgrade, flake8-bugbear, flake8-simplify, and flake8-bandit rules. Remove the Makefile — all development tasks (lint, format, test) are now run directly via ruff and pytest. Add a [dev] optional dependency group for development tooling. Add a lint job to GitHub Actions CI.
Consolidate all test data in one place by moving ksk-as-dnskey.txt and ksk-as-ds.txt from regress/ to tests/fixtures/ and removing the now-empty regress/ directory.
Add options to print DNSKEY or DS records to stdout instead of writing files, so the output can be piped. Both can be used together. Refactor export_ksk into format_records (returns strings) and export_ksk (writes files) to support both output modes. Move all status messages to stderr via a new log() helper so stdout contains only record data when using the print options.
Annotate all function signatures and key variables in cli.py with type hints. Add type aliases (KskDict, TrustAnchorDict, SignerInfoDict) for the dict structures used throughout. Use `from __future__ import annotations` for forward-reference support. Configure ty in pyproject.toml with unresolved-import set to ignore (third-party packages may not be available in all environments). Add ty to the dev dependencies and CI lint job. Fix pre-existing type-safety issues caught by ty: dict.get() returning None fed into call sites, Element.find() returning None accessed unconditionally, and dict slicing in error messages. Bump requires-python to >=3.10 to match union type syntax usage.
Narrow public_key() return types with isinstance checks for RSAPublicKey instead of using type: ignore comments. ty does not support type: ignore directives, and the broad union type returned by public_key() (RSA | DSA | EC | Ed25519 | Ed448 | X25519 | X448) caused errors for the RSA-specific verify() call signature. Also guard against signature_hash_algorithm being None.
- Remove codecs/StringIO imports; use .decode() and fromstring() directly - Use with-statement in write_out_file - Replace manual date parsing with datetime.fromisoformat() - Replace .format() calls with f-strings throughout - Simplify hash type selection, XML attribute access, and control flow - Convert LICENSE to markdown format
- _extract_pkcs7_signer_info: extract signer_serial from IssuerAndSerialNumber to unambiguously identify the signer cert - _parse_pem_bundle (new): split a PEM bundle into individual certs, allowing --root-ca to accept multi-cert files like icannbundle.pem - validate_detached_signature: accept ca_pems list[bytes]; identify signer cert by serial; walk the chain through any intermediates in the bundle up to a trusted CA, verifying each link - Call site: parse PEM text through _parse_pem_bundle before calling validate_detached_signature
Remove the custom _parse_pem_bundle helper and use cryptography.x509.load_pem_x509_certificates (available since cryptography 41.0, already the project minimum) instead. validate_detached_signature now accepts list[x509.Certificate] rather than list[bytes], eliminating the per-iteration x509.load_pem_x509_certificate call inside the chain-walking loop.
Validate full PKCS7 certificate chain
kjd
marked this pull request as draft
April 22, 2026 19:54
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.