Add python 3.12 and 3.13 support by blink1073 · Pull Request #707 · OpenKMIP/PyKMIP

blink1073 · 2023-10-04T16:50:14Z

blink1073 · 2023-10-04T16:52:19Z

The traceback on my fork is:

kmip/tests/integration/conftest.py:49: _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ kmip/pie/client.py:173: inopenself.proxy.open()
kmip/services/kmip_client.py:285: inopensix.reraise(*last_error)
.tox/integration/lib/python3.9/site-packages/six.py:719: inreraiseraisevaluekmip/services/kmip_client.py:274: inopenself.socket.connect((self.host, self.port))
/opt/hostedtoolcache/Python/3.9.18/x64/lib/python3.9/ssl.py:1376: inconnectself._real_connect(addr, False)
/opt/hostedtoolcache/Python/3.9.18/x64/lib/python3.9/ssl.py:1367: in_real_connectself.do_handshake()
__ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ self=<ssl.SSLSocket [closed] fd=-1, family=AddressFamily.AF_INET, type=SocketKind.SOCK_STREAM, proto=0>block=False@_sslcopydocdefdo_handshake(self, block=False):
self._check_connected()
timeout=self.gettimeout()
try:
iftimeout==0.0andblock:
self.settimeout(None)
>self._sslobj.do_handshake()
Essl.SSLCertVerificationError: [SSL: CERTIFICATE_VERIFY_FAILED] certificateverifyfailed: self-signedcertificate (_ssl.c:1129)
/opt/hostedtoolcache/Python/3.9.18/x64/lib/python3.9/ssl.py:1343: SSLCertVerificationError_ERRORatsetupofTestProxyKmipClientIntegration.test_x509_certificate_register_get_destroy_request=<SubRequest'simple'for<TestCaseFunctiontest_asymmetric_key_pair_create_get_destroy>>
@pytest.fixture(scope="class")
defsimple(request):
config=request.config.getoption("--config")
client=pclient.ProxyKmipClient(config=config)
>client.open()
kmip/tests/integration/conftest.py:49: _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ kmip/pie/client.py:173: inopenself.proxy.open()
kmip/services/kmip_client.py:285: inopensix.reraise(*last_error)
.tox/integration/lib/python3.9/site-packages/six.py:719: inreraiseraisevaluekmip/services/kmip_client.py:274: inopenself.socket.connect((self.host, self.port))
/opt/hostedtoolcache/Python/3.9.18/x64/lib/python3.9/ssl.py:1376: inconnectself._real_connect(addr, False)
/opt/hostedtoolcache/Python/3.9.18/x64/lib/python3.9/ssl.py:1367: in_real_connectself.do_handshake()
__ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ self=<ssl.SSLSocket [closed] fd=-1, family=AddressFamily.AF_INET, type=SocketKind.SOCK_STREAM, proto=0>block=False@_sslcopydocdefdo_handshake(self, block=False):
self._check_connected()
timeout=self.gettimeout()
try:
iftimeout==0.0andblock:
self.settimeout(None)
>self._sslobj.do_handshake()
Essl.SSLCertVerificationError: [SSL: CERTIFICATE_VERIFY_FAILED] certificateverifyfailed: self-signedcertificate (_ssl.c:1129)
/opt/hostedtoolcache/Python/3.9.18/x64/lib/python3.9/ssl.py:1343: SSLCertVerificationError

+ context.check_hostname = False
+ if self.certfile:
+ context.load_cert_chain(self.certfile, self.keyfile)
+ self.socket = context.wrap_socket(


+ keyfile = self.config.settings.get('key_path')
+ context.load_cert_chain(certfile, keyfile=keyfile)
+
+ self._socket = context.wrap_socket(


s-t-e-v-e-n-k · 2024-03-28T03:37:54Z

The problem with your mocking is that you can't mock ssl.SSLContext.wrap_socket because the code constructs an SSLContext object and then calls wrap_socket on that, so you're not mocking the right thing.

I've edited the test changes for your patch (and not your code changes, which looks good) thusly:

@@ -210,9 +210,9 @@ class TestKmipServer(testtools.TestCase)
# Test that in ideal cases no errors are generated and the right
# log messages are.
with mock.patch('socket.socket') as socket_mock:
- with mock.patch('ssl.wrap_socket') as ssl_mock:
+ with mock.patch('ssl.SSLContext') as ssl_mock:
socket_mock.return_value = a_mock
- ssl_mock.return_value = b_mock
+ ssl_mock.return_value.wrap_socket.return_value = b_mock
manager_mock.assert_not_called()
monitor_mock.assert_not_called()

And I've confirmed the tests pass with Python 3.9-3.12.

This reverts commit 4618b1d.

blink1073 · 2024-03-29T16:44:40Z

Thanks @s-t-e-v-e-n-k, new build in progress on my fork.

blink1073 · 2024-03-29T16:47:55Z

Still no joy: ssl.SSLCertVerificationError: [SSL: CERTIFICATE_VERIFY_FAILED] certificate verify failed: self-signed certificate (_ssl.c:1133)

avikivity · 2024-06-02T19:11:33Z

@blink1073 were you able to make this work?

blink1073 · 2024-06-03T11:31:42Z

No, I haven't been able to figure out the testing portion.

petere · 2024-07-24T21:52:02Z

 def _create_socket(self, sock):
- self.socket = ssl.wrap_socket(
+ purpose = ssl.Purpose.SERVER_AUTH
+ context = ssl.create_default_context(purpose=purpose, capath=self.ca_certs)


This should be cafile=... instead of capath=..., because the underlying configuration parameter points to a file.

petere · 2024-07-24T21:53:03Z

- self._socket = ssl.wrap_socket(
+ purpose = ssl.Purpose.CLIENT_AUTH
+ capath = self.config.settings.get('ca_path')
+ context = ssl.create_default_context(purpose=purpose, capath=capath)


This should be cafile=capath instead of capath=capath, because the underlying configuration parameter points to a file. (The naming is unfortunately inconsistent between the Python ssl module and pykmip.)

petere · 2024-07-24T22:03:01Z

This no longer pays attention to the configuration settings self.cert_reqs and self.ssl_versions. That's probably okay in practice, since the defaults are good, but these settings are documented, so they should work.
One option might be to not use ssl.create_default_context() and instead use ssl.SSLContext directly and apply these settings. Like
context=ssl.SSLContext(self.ssl_version) context.verify_mode=self.cert_reqsifself.ca_certs: context.load_verify_locations(self.ca_certs)
This is just about as much code as before but it reproduces the previous behavior more precisely.

petere · 2024-07-24T22:06:50Z

Similar to the client, we should make sure that this change does not lose any previously working configuration settings. I see for example that ssl_version=self.auth_suite.protocol and ciphers=self.auth_suite.ciphers don't get used anymore.

petere · 2024-07-24T22:13:07Z

- self._socket = ssl.wrap_socket(
+ purpose = ssl.Purpose.CLIENT_AUTH
+ capath = self.config.settings.get('ca_path')
+ context = ssl.create_default_context(purpose=purpose, capath=capath)


You must set
context.verify_mode=ssl.CERT_REQUIRED
after creating context. This matches what the previous code did. Without this, the server won't ask the client to send a certificate, and without a client certificate this will later fail at higher levels (such as
PyKMIP/kmip/services/server/session.py
Line 144 in 4d3b5a5
certificate=auth.get_certificate_from_connection(
).

blink1073 · 2024-07-25T00:07:12Z

Thanks @petere! I believe I've addressed your feedback.

petere · 2024-07-26T09:56:51Z

+ context.verify_mode = self.cert_reqs
+ if self.ca_certs:
+ context.load_verify_locations(self.ca_certs)
+ context.check_hostname = False


This line doesn't appear to be needed. Probably left over from a previous version that used create_default_context().

petere · 2024-07-26T10:00:53Z

+ if self.ca_certs:
+ context.load_verify_locations(self.ca_certs)
+ context.check_hostname = False
+ if self.certfile:


I'm testing combinations of "forgetting" to specify certfile or keyfile in the configuration file. With this code, if you specify certfile but not keyfile, you'll get an exception from load_cert_chain. If you specify keyfile but not certfile, this code is skipped and you'll get whatever the SSL stack ends up deciding. The old module-level wrap_socket had an additional check
ifkeyfileandnotcertfile: raiseValueError("certfile must be specified")
to prevent this. Maybe this would be good to have here as well.

petere · 2024-07-26T10:03:10Z


- self._socket = ssl.wrap_socket(
+ cafile = self.config.settings.get('ca_path')
+ context = ssl.SSLContext(self.ssl_version)


This should be
context=ssl.SSLContext(self.auth_suite.protocol)
(wrongly copy-pasted from client code?)

petere · 2024-07-26T10:04:46Z

+ cafile = self.config.settings.get('ca_path')
+ context = ssl.SSLContext(self.ssl_version)
+ context.verify_mode = ssl.CERT_REQUIRED
+ if auth_suite_ciphers:


This should be self.auth_suite.ciphers. I'm not sure exactly what the relationship between that and auth_suite_ciphers is, but this one crashes.

petere · 2024-07-26T10:08:23Z

+ context.load_verify_locations(cafile)
+ certfile = self.config.settings.get('certificate_path')
+
+ if certfile:


As with the client, it's worth checking here about how different combinations of not specifying certificate or key behave. For example, the old module-level wrap_socket() had a check
ifserver_sideandnotcertfile: raiseValueError("certfile must be specified for server-side ""operations")
so omitting the certificate would have been a hard error.

I think what I had in mind was like
if certfile: keyfile = self.config.settings.get('key_path') context.load_cert_chain(certfile, keyfile=keyfile) + else:+ raise ValueError("certfile must be specified for server-side operations")

I instead let the context raise an appropriate error. This has the benefit of letting us mock the context behavior in the tests.

from OpenKMIP#707

blink1073 · 2024-11-14T21:40:44Z

@petere is there anything else you'd like me to try?

petere · 2024-11-27T15:51:19Z

Other than the single comment I just added (which I just got my looking at my local copy, but it's been a while since I played with this), I am content with this version.

How we get from that to getting this merged and released, I don't know.

…rap-socket

blink1073 · 2024-12-02T12:22:58Z

This is now ready for review. cc @arp102.

blink1073 · 2024-12-02T12:23:12Z

The workflows are all passing on my fork: https://github.com/blink1073/PyKMIP/actions/runs/12119149288?pr=1

tomamoto · 2025-05-09T19:39:12Z

@arp102 Are you able to merge this PR, and if not, what else is needed? Getting 3.12 support in master branch would be great, and seeing a new release to pypi would be even nicer.

wdelgenio · 2025-07-10T21:14:34Z

@arp102 @BourgoisMickael @petere @PeterHamilton
Do any one of you have permissions to merge this fix? This one is needed for anyone who is running python3.12 or newer and is a complete showstopper.

BourgoisMickael · 2025-07-11T09:46:44Z

@arp102 @BourgoisMickael @petere @PeterHamilton Do any one of you have permissions to merge this fix? This one is needed for anyone who is running python3.12 or newer and is a complete showstopper.

I'm not related to the project I just want to see it merged

wdelgenio · 2025-07-14T19:06:09Z

Looks like this repo is no longer maintained, might be time for a fork or a switch to a different library.

(cherry picked from commit b76e74e)

Steven Silvester added 10 commits October 4, 2023 09:59

Add python 3.12 support
433ab3e

fix usage
f16f36d

fix usage
c90654c

cleanup
61c5528

fix cert handling
06f3a1f

add capath
22fc83c

try loading default certs
519cbdc

cleanup
ec91215

fixups
a8e44f3

cleanup
eea62bd

github-advanced-securityAI found potential problems Oct 4, 2023
View reviewed changes

Use all original options
4618b1d

blink1073 added 2 commits March 29, 2024 11:42

Revert "Use all original options"
c3a5dbc
This reverts commit 4618b1d.

Apply fix
5a02eb7

petere suggested changes Jul 24, 2024
View reviewed changes

address review
2897e40

blink1073 requested review from petere July 25, 2024 00:07

petere suggested changes Jul 26, 2024
View reviewed changes

blink1073 added 3 commits July 26, 2024 08:19

address review
cc035f4

fix client
d206639

lint
777c70d

oittaa mentioned this pull request Jul 28, 2024
ssl.PROTOCOL_TLS, ssl.PROTOCOL_TLSv1_2, and so on are deprecated #719
Open

petere added a commit to petere/PyKMIP that referenced this pull request Jul 29, 2024
Fix Python 3.12 support
01c3a9f
from OpenKMIP#707

remove extra check
3da6f8d

blink1073 requested a review from petere July 29, 2024 18:37

blink1073 added 10 commits November 30, 2024 13:40

address review
80faaf6

Merge branch 'master' of github.com:OpenKMIP/PyKMIP into remove-ssl-w…
452693a
…rap-socket

fix test
4b781ff

address docs error
529e123

leave validation to the context
da34f31

mock load_cert_chain
1b46974

fix wrap
8352de5

fix stop test
042378b

fix stop test
65c6993

add python 3.13 support
5b736ad

blink1073 changed the title ~~Add python 3.12 support~~Add python 3.12 and 3.13 supportDec 2, 2024

blink1073 marked this pull request as ready for review December 2, 2024 12:21

blink1073 mentioned this pull request Dec 3, 2024
DRIVERS-2743 Add support for Python 3.12 and Python 3.13 mongodb-labs/drivers-evergreen-tools#555
Merged

blink1073 mentioned this pull request Apr 21, 2025
Add python 3.12 and 3.13 support mongodb-forks/PyKMIP#1
Merged

BourgoisMickael approved these changes May 28, 2025
View reviewed changes

artemgavrilov mentioned this pull request Jun 6, 2025
Use ubuntu 24.04 in main CI workflows percona/postgres#402
Merged

ymeiron mentioned this pull request Aug 28, 2025
Fixed compatability with Python 3.12 #733
Closed

ymeiron added a commit to ymeiron/PyKMIP that referenced this pull request Aug 28, 2025
Brought in line withOpenKMIP#707
b76e74e

phlogistonjohn pushed a commit to phlogistonjohn/PyKMIP that referenced this pull request Nov 3, 2025
Brought in line withOpenKMIP#707
aecb9cc
(cherry picked from commit b76e74e)

Uh oh!

Conversation

blink1073 commented Oct 4, 2023• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

blink1073 commented Oct 4, 2023

Uh oh!

Check failure

Check failure

s-t-e-v-e-n-k commented Mar 28, 2024

Uh oh!

blink1073 commented Mar 29, 2024

Uh oh!

blink1073 commented Mar 29, 2024

Uh oh!

avikivity commented Jun 2, 2024

Uh oh!

blink1073 commented Jun 3, 2024

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

blink1073 commented Jul 25, 2024

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

blink1073 commented Nov 14, 2024

Uh oh!

petere commented Nov 27, 2024

Uh oh!

blink1073 commented Dec 2, 2024

Uh oh!

blink1073 commented Dec 2, 2024

Uh oh!

tomamoto commented May 9, 2025

Uh oh!

wdelgenio commented Jul 10, 2025

Uh oh!

BourgoisMickael commented Jul 11, 2025

blink1073 commented Oct 4, 2023•
edited
Loading