Skip to content

Strip the CRL server from the certificates again #67

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
May 16, 2025

Conversation

Darsstar
Copy link
Contributor

Changed the gha-setup.sh as well to make it work in Git Bash on Windows.

@Gsantomaggio
Copy link
Member

Thank you @Darsstar

  .ci/ubuntu/gha-setup.sh

error:

Traceback (most recent call last):
  File "/.../rabbitmq/rabbitmq-amqp-python-client/examples/tls/tls_example.py", line 237, in <module>
    main()
  File "/.../rabbitmq/rabbitmq-amqp-python-client/examples/tls/tls_example.py", line 158, in main
    connection = create_connection(environment)
                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/.../rabbitmq/rabbitmq-amqp-python-client/examples/tls/tls_example.py", line 78, in create_connection
    connection.dial()
  File "/.../rabbitmq/rabbitmq-amqp-python-client/rabbitmq_amqp_python_client/connection.py", line 189, in dial
    self._ssl_domain.set_trusted_ca_db(ca_cert)
  File "/.../rabbitmq/rabbitmq-amqp-python-client/rabbitmq_amqp_python_client/qpid/proton/_transport.py", line 886, in set_trusted_ca_db
    return self._check(
           ^^^^^^^^^^^^
  File "/.../rabbitmq/rabbitmq-amqp-python-client/rabbitmq_amqp_python_client/qpid/proton/_transport.py", line 829, in _check
    raise exc("SSL failure.", err)
rabbitmq_amqp_python_client.qpid.proton._exceptions.SSLException: ('SSL failure.', -1)
 ca_cert_file exists: True
 client_cert exists: True
 client_key exists: True

@Darsstar Darsstar force-pushed the certificate-fixes-for-windows branch 4 times, most recently from dce87c5 to bd4ca14 Compare May 15, 2025 11:57
@Gsantomaggio
Copy link
Member

It fails on this check:
🚫 MAC the domain seems to be NULL:
mac_ko

✅ Linux OK:

linux_ok

@Darsstar
Copy link
Contributor Author

That... But... How...?

else:
return PosixSslConfigurationContext(
ca_cert=".ci/certs/ca_certificate.pem",
client_cert=PosixClientCert(
client_cert=".ci/certs/client_localhost_certificate.pem",
client_key=".ci/certs/client_localhost_key.pem",
),
)

should be executed on Mac.

which means that in

self._ssl_domain = SSLDomain(SSLDomain.MODE_CLIENT)
assert self._ssl_domain
if isinstance(self._conf_ssl_context, PosixSslConfigurationContext):
ca_cert = self._conf_ssl_context.ca_cert
elif isinstance(self._conf_ssl_context, WinSslConfigurationContext):
ca_cert = self._win_store_to_cert(self._conf_ssl_context.ca_store)
else:
typing_extensions.assert_never(self._conf_ssl_context)
self._ssl_domain.set_trusted_ca_db(ca_cert)

constructing the SSLDomain goes wrong, I think

But then why is

def __init__(self, mode: int) -> None:
self._domain = pn_ssl_domain(mode)
if self._domain is None:
raise SSLUnavailable()

not raising an exception?!?!

I don't have access to a Mac, so untill I get a shower/poop/walk epifanie I am stumped.

PS. the screenshots show

"""
return self._check(
pn_ssl_domain_set_trusted_ca_db(self._domain, certificate_db)
)

@Darsstar
Copy link
Contributor Author

Oh, wait, I confused null and None. Proton Qpid C code is where we should be looking apparently...
I should have waited 1min before pressing "Comment"...

@Darsstar Darsstar force-pushed the certificate-fixes-for-windows branch from bd4ca14 to 200d3cd Compare May 15, 2025 14:46
@Darsstar
Copy link
Contributor Author

Darsstar commented May 15, 2025

@Gsantomaggio I think you should now get an exception earlier.

https://stackoverflow.com/questions/44979947/python-qpid-proton-for-mac-using-amqps seems relevant

@Gsantomaggio
Copy link
Member

Gsantomaggio commented May 15, 2025

thank you @Darsstar

https://stackoverflow.com/questions/44979947/python-qpid-proton-for-mac-using-amqps seems relevant

Following this ^^^, it works! On the main branch, too.

Instead of spending hours on this, I had to Google :)!

I will update the documentation.

@Gsantomaggio
Copy link
Member

Closed in favour of #68

@Darsstar
Copy link
Contributor Author

Please reopen and merge, this is still required on Windows.

@Gsantomaggio Gsantomaggio reopened this May 16, 2025
@Gsantomaggio Gsantomaggio merged commit fe4e468 into rabbitmq:main May 16, 2025
1 check passed
@Gsantomaggio
Copy link
Member

Please reopen and merge, this is still required on Windows.

ops sorry!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants