Conversation
…VEL=2 compatibility (cherry picked from commit cfaac47)
… lookup (cherry picked from commit f7c1347)
…inking (cherry picked from commit 2fc8fba)
(cherry picked from commit 8a63e04)
The Link attribute was recovered by querying for an object of the complementary type sharing the creating object's _owner and initial_date. initial_date is a whole-second epoch integer and both halves of a pair are given the same value, so two key pairs created by one owner inside the same second were indistinguishable and .first() returned an arbitrary one. A KMIP client that encrypts under public key A and later asks for A's linked private key could therefore be handed B's. Nothing fails at creation time; it surfaces as a decryption failure against data that is already written. Record the pairing on both objects once SQLAlchemy has assigned identifiers, which is the only moment the relationship is known for certain. The old heuristic is kept for rows created before the column existed, but it now refuses to guess when more than one candidate matches, and logs instead. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A missing comma in TLS12AuthenticationSuite._default_cipher_suites made Python concatenate two entries into one unknown token, so ECDHE-RSA-AES256-GCM-SHA384 was never actually enabled. OpenSSL drops unrecognised tokens silently rather than raising, so a peer restricted to forward-secret GCM suites negotiated AES-128 instead, with nothing logged. The two duplicated ECDHE-ECDSA entries introduced alongside it are dropped. Link.linked_object_identifier used six.string_types, and 0.11.0 removed the Python 2 compatibility shims, so setting a string identifier raised NameError — on exactly the path a client takes to resolve a key pair. The Query and Link tests still asserted the old behaviour: an empty supported-object-type list (which a client reads as "this server supports nothing") and NotImplementedError from the Link factory. Adds a test that asserts the RSA GCM suites survive OpenSSL's own parsing. Asserting the cipher string cannot catch this: a malformed entry is still a string, which is how the missing comma went unnoticed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
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
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.
Three related fixes, found while getting PyKMIP to interoperate with a real KMIP client (Veeam Backup & Replication 13). Each is a gap that stops a standards-conforming client from completing a normal asymmetric key workflow.
1. The Link attribute is unimplemented
There are three stubs and no implementation: no
Linkclass inattributes.py,AttributeValueFactory.create_attribute_value()raisesNotImplementedError, and_get_attribute_from_managed_object()returnsNone. A client that callsCreateKeyPairand then asks the public key for its linked private key gets nothing, so it cannot resolve the pair.This adds the
Linkclass with KMIP read/write serialization, the factory method, and the engine read path.The pairing is stored, not inferred
The tempting implementation is to recover the relationship by querying for an object of the complementary type sharing the creating object's
_ownerandinitial_date. That is unsound:initial_dateis a whole-second epoch integer and_process_create_key_pairgives both halves of a pair the same value, so two key pairs created by one owner within the same second are indistinguishable and the query returns an arbitrary one.A client that encrypts under public key A and later asks for A's linked private key is then handed B's. Nothing fails at creation time; it surfaces as a decryption failure against data that is already written.
So the pairing is recorded on both objects immediately after the commit that assigns their identifiers — the only moment it is known for certain.
Compatibility — guidance welcome
This adds
managed_objects.link_id. There is no migration machinery in the project andmetadata.create_all()does not add columns to existing tables, so an existing database raises:For SQLite the upgrade is:
I am happy to rework this if you would rather not carry a schema change, but the inference-based alternative is unsound for the reason above, so I did not want to propose it as the default.
Objects created before the column existed still resolve through the old heuristic, which now refuses to guess when more than one candidate matches and logs a warning rather than returning an arbitrary key.
Incidentally
The
Linksetter usedsix.string_types, so setting a string identifier raisedNameErroron exactly this path now that the Python 2 shims are gone. It usesstr. (This overlaps in spirit with #739, which trims moresixusage — happy to defer to that if it lands first.)2. Query reports no supported object types
_process_queryreturns an empty list forQUERY_OBJECTS, so the Operation list is populated but Object Types is always empty. Clients reasonably read that as "this server manages no object types" and refuse to proceed; the failure surfaces as a generic registration error with nothing pointing at Query.Now returns the types the engine actually manages: Certificate, Symmetric Key, Public Key, Private Key, Secret Data.
3. No usable GCM cipher suite with an RSA certificate
TLS12AuthenticationSuite._default_cipher_suitesincludes GCM suites only in their ECDHE-ECDSA form, which requires an ECDSA server certificate. Deployed with an RSA certificate — the common case, and whatbin/create_certificates.pyproduces — no GCM suite is available at all.A peer enforcing OpenSSL
SECLEVEL=2offers only forward-secret GCM suites, so the two sides share nothing and the handshake fails inside OpenSSL before any application-level logging. The server logs nothing, which makes this disproportionately hard to diagnose.Adds
ECDHE-RSA-AES256-GCM-SHA384andECDHE-RSA-AES128-GCM-SHA256, and drops twoECDHE-ECDSAentries that were listed twice.(Unrelated and left alone:
DH-DSS-AES256-SHA256appears twice in the existing list.)Tests
SSLContext.set_ciphers()thenget_ciphers()— rather than by comparing the cipher string. String equality cannot catch a malformed entry, because OpenSSL drops tokens it does not recognise instead of raising.test_query_1_*and Link factory tests asserted the previous behaviour (assertIsNone(result.object_types),NotImplementedError) and are updated.Full unit suite: 3363 passed, 35 skipped.
Attribution
The Link implementation, the Query fix and the cipher-suite addition are by Luca Dell'Oca (@dellock6), from his
pykmip-veeamfork; the commits here keep his authorship. Rebasing them onto current master, the stored-pairing change, thesixfix and the test updates are mine.🤖 Generated with Claude Code