Cbest/multi identities - #10
Merged
Merged
Conversation
… present Raise AmbiguousCertificateError when multiple identities are present and one has not been specified
Dual key pairs live in the platform stores as often as in files -- AD/Exchange key archival puts both halves in CurrentUser\MY, and keychains collect them the same way -- but the store constructors had nothing to select on. WinCert and MacCert carried only a CN, a label, and a thumbprint, so a store holding both halves under one subject raised AmbiguousCertificateError with no way out except an exact thumbprint, which is machine-specific and changes at renewal. Both dataclasses now carry the parsed certificate and its CertInfo, and a shared _CertDetails mixin gives them (and P12Identity) identical key_usage / extended_key_usage accessors, so one predicate reads the same across all three surfaces. from_windows_cert_store, from_macos_keychain, their build_*_ssl_context counterparts, and the select_* helpers all take the usage selectors, which are recorded on the SourceRef so reload() re-selects. The usage vocabulary moved from _pkcs12 to _select, which now owns selection for every surface; both selectors share matches_usages so they cannot drift. Behavior change: store selectors intersect instead of overriding. Passing name= with thumbprint= used to ignore the name; now every selector must match, as the PKCS#12 ones do. macOS needed no new plumbing -- _certificate_details already parsed the DER and discarded everything but the CN and thumbprint. Windows reads the DER off a declared CERT_CONTEXT, then checks the parsed certificate's SHA-1 against the thumbprint crypt32 reported separately: a wrong struct offset yields plausible garbage rather than an error, so a mismatch drops to certificate=None and the record keeps selecting by name and thumbprint as before. The extraction flow is tested off-Windows against a mirrored struct; the real layout is asserted by the new opt-in suite. That suite (HTTPX_PKI_WINSTORE_TESTS=1, mirroring the macOS keychain gate, and enabled for Windows CI runners) imports a throwaway dual pair into CurrentUser\MY with Import-PfxCertificate -Exportable and removes it after. It is the first time the crypt32 enumeration and export have run in CI at all. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017X8iUVVmXpS2rWcWmELzLM
ctypes.get_last_error() returns a signed int, so every NTE_* HRESULT arrives with its high bit read as a sign: NTE_BAD_KEY_STATE (0x8009000B) -- what Windows returns when a key was imported without the exportable flag, and the most likely reason an export fails at all -- came back as -0x7ff6fff5 and matched nothing in NON_EXPORTABLE_ERRORS. The message fell through to the generic "PFX export failed (Windows error -0x7ff6fff5)", which reads like a library fault rather than a fixable import setting. Codes are now normalized to unsigned before comparing or formatting, the known set also covers the CNG counterpart NTE_NOT_SUPPORTED, and the message names the flags that mark a key exportable at import time -- along with the TPM and smart-card case, where no flag will help and a key file is the way out. The set moved to module scope and _raise_export_error lost its no-cover pragma: the mapping is pure, so it is now tested off Windows in both the signed and unsigned spelling of every code. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017X8iUVVmXpS2rWcWmELzLM
Building a session from the macOS keychain emitted "PKCS#12 bundle could not be parsed as DER, falling back to parsing as BER" on every construction. The cause is an explicitly encoded ASN.1 DEFAULT: DER requires a field equal to its default to be omitted, and MacData.iterations carries DEFAULT 1, so writing it anyway is valid BER but not DER. Reproduced locally by encoding that field the same way -- both load_pkcs12 and load_key_and_certificates warn identically, so this predates identity selection. The bytes are generated by the Security framework, consumed in-process, and never written down, so there is nothing a caller could do about the encoding. The filter is installed only in material_from_store_export, and only for that message, so files a caller supplies still warn -- there, re-exporting the file is a real fix. cryptography says the fallback "may become an exception". If it does, the generic load failure would blame an invalid password that this library generated itself, so the store path now raises an error naming the encoding instead. make_pkcs12 grew strict_der=False to write that shape, which is what makes any of this testable off macOS -- and is worth having for downstream suites that need to cope with platform exports too. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017X8iUVVmXpS2rWcWmELzLM
|
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
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
2 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.
Add support for PKCS#12 certificates containing multiple identities (file/windows/macos)
Explicitly does not add PEM support for the same despite tooling now being available - follow up branch needed
Raise AmbiguousCertificateError when multiple identities are present and one has not been specified