Skip to content
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

python-bareos: use TLS-PSK from core ssl module (available since Python >= 3.13) #1756

Merged

Conversation

joergsteffens
Copy link
Member

@joergsteffens joergsteffens commented Mar 28, 2024

The ssl module in Python 3.13 (cpython) added support for TLS-PSK. This changes uses TLS-PSK from this core module, if the functionality is available. If not, it still fails back to sslpsk and than to unencrypted.

Thank you for contributing to the Bareos Project!

Backport of PR #0000 to bareos-2x (remove this line if this is no backport; for backport use cherry-pick -x)

Please check

  • Short description and the purpose of this PR is present above this paragraph
  • Your name is present in the AUTHORS file (optional)

If you have any questions or problems, please give a comment in the PR.

Helpful documentation and best practices

Checklist for the reviewer of the PR (will be processed by the Bareos team)

Make sure you check/merge the PR using devtools/pr-tool to have some simple automated checks run and a proper changelog record added.

General
  • Is the PR title usable as CHANGELOG entry?
  • Purpose of the PR is understood
  • Commit descriptions are understandable and well formatted
  • Check backport line
  • Required backport PRs have been created
Source code quality
  • Source code changes are understandable
  • Variable and function names are meaningful
  • Code comments are correct (logically and spelling)
  • Required documentation changes are present and part of the PR
Tests
  • Decision taken that a test is required (if not, then remove this paragraph)
  • The choice of the type of test (unit test or systemtest) is reasonable
  • Testname matches exactly what is being tested
  • On a fail, output of the test leads quickly to the origin of the fault

@joergsteffens joergsteffens force-pushed the dev/joergs/master/python313-tlspsk branch 2 times, most recently from 7ec4e77 to 07b6579 Compare March 28, 2024 17:52
@bruno-at-bareos bruno-at-bareos self-assigned this Apr 2, 2024
@joergsteffens joergsteffens force-pushed the dev/joergs/master/python313-tlspsk branch from 07b6579 to 457285c Compare April 2, 2024 14:18
@arogge arogge modified the milestones: 23.1.0, 24.0.0 Apr 2, 2024
@joergsteffens joergsteffens force-pushed the dev/joergs/master/python313-tlspsk branch from 457285c to e04c7ff Compare April 2, 2024 19:54
except ImportError:
warnings.warn(
"Connection encryption via TLS-PSK is not available "
"(TLS-PSK is not available in the ssl module and the extra module sslpsk is not installed)."
Copy link
Contributor

@bruno-at-bareos bruno-at-bareos Apr 3, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I found the warning too long on terminal output.

We may want to fix also the additional warning.warn

/bareos/git/b-at-bareos/python-bareos/bareos/bsock/lowlevel.py:60: UserWarning: Connection encryption via TLS-PSK is not available (TLS-PSK is not available in the ssl module and the extra module sslpsk is not installed).
  warnings.warn(
test_fileset (test_show.PythonBareosShowTest.test_fileset)
Filesets are stored in the database, ... ok
test_show_resources (test_show.PythonBareosShowTest.test_show_resources)
show resources in bconsole ... ok

----------------------------------------------------------------------
Ran 2 tests in 4.268s

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've shorted the warning.

Copy link
Contributor

@bruno-at-bareos bruno-at-bareos left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice work, please see comments

@bruno-at-bareos
Copy link
Contributor

One more thing we would like to treat as next LTS python will be not compatible with sslpsk library

with python >= 3.10 you can install sslpsk but it will failed when tried to used.

We certainly want after the first import do another try except block to catch this missing property, and then failed due to incompatible python/sslpsk mix.

======================================================================
ERROR: test_login_tls_tls_require (test_tlspsk.PythonBareosTlsPskTest.test_login_tls_tls_require)
console: tls, director: tls     => login
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/bareos/git/build/systemtests/tests/python-bareos/test_tlspsk.py", line 225, in test_login_tls_tls_require
    director = bareos.bsock.DirectorConsole(
               ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/bareos/git/b-at-bareos/python-bareos/bareos/bsock/directorconsole.py", line 200, in __init__
    self.connect(
  File "/bareos/git/b-at-bareos/python-bareos/bareos/bsock/lowlevel.py", line 180, in connect
    return self.__connect()
           ^^^^^^^^^^^^^^^^
  File "/bareos/git/b-at-bareos/python-bareos/bareos/bsock/lowlevel.py", line 198, in __connect
    self.__connect_tls_psk()
  File "/bareos/git/b-at-bareos/python-bareos/bareos/bsock/lowlevel.py", line 284, in __connect_tls_psk
    self.socket = sslpsk.wrap_socket(
                  ^^^^^^^^^^^^^^^^^^^
  File "/usr/lib64/python3.11/site-packages/sslpsk/sslpsk.py", line 106, in wrap_socket
    _ssl_set_psk_client_callback(sock, cb)
  File "/usr/lib64/python3.11/site-packages/sslpsk/sslpsk.py", line 73, in _ssl_set_psk_client_callback
    ssl_id = _sslpsk.sslpsk_set_psk_client_callback(_sslobj(sock))
                                                    ^^^^^^^^^^^^^
  File "/usr/lib64/python3.11/site-packages/sslpsk/sslpsk.py", line 55, in _sslobj
    return sock._sslobj._sslobj
           ^^^^^^^^^^^^^^^^^^^^
AttributeError: '_ssl._SSLSocket' object has no attribute '_sslobj'

@joergsteffens
Copy link
Member Author

One more thing we would like to treat as next LTS python will be not compatible with sslpsk library

with python >= 3.10 you can install sslpsk but it will failed when tried to used.

We certainly want after the first import do another try except block to catch this missing property, and then failed due to incompatible python/sslpsk mix.

...
AttributeError: '_ssl._SSLSocket' object has no attribute '_sslobj'

You mean an extra check in the tests?

@joergsteffens
Copy link
Member Author

One more thing we would like to treat as next LTS python will be not compatible with sslpsk library
with python >= 3.10 you can install sslpsk but it will failed when tried to used.
We certainly want after the first import do another try except block to catch this missing property, and then failed due to incompatible python/sslpsk mix.

...
AttributeError: '_ssl._SSLSocket' object has no attribute '_sslobj'

You mean an extra check in the tests?

I'm not sure how to cover this. The combination is broken, so the test should fail. I can do try/except AttributeError block around the connect, but still I should raise the problem after it has occurred. It seams, there exists a PR, that would solve these issues (drbild/sslpsk#28), but it seams, as it will not get merged.

Copy link
Contributor

@bruno-at-bareos bruno-at-bareos left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While there's certainly room for improvement, we want to merge this as it is now.
If someone decide to sponsor development around sslpsk forks like sslpsk3 we may then have the resources to even make it better.

python-bareos/bareos/bsock/lowlevel.py Show resolved Hide resolved
self.socket = context.wrap_socket(client_socket, server_side=False)
else:
try:
self.socket = sslpsk.wrap_socket(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So here with python 3.11 and sslpsk from github there a deprecate warning

test_execute_external_command (test_filedaemon.PythonBareosFiledaemonTest.test_execute_external_command) ... DeprecationWarning: ssl.wrap_socket() is deprecated, use SSLContext.wrap_socket()
DeprecationWarning: ssl.PROTOCOL_TLSv1_2 is deprecated

@bruno-at-bareos
Copy link
Contributor

So to resume the situation, for those who tried to install sslpsk from pypi and got the following error

AttributeError: '_ssl._SSLSocket' object has no attribute '_sslobj'

You have to remove and install sslpsk from github source.

@BareosBot BareosBot force-pushed the dev/joergs/master/python313-tlspsk branch from 04d009b to 6cff0b2 Compare April 8, 2024 14:55
@BareosBot BareosBot merged commit 04f4538 into bareos:master Apr 8, 2024
@joergsteffens joergsteffens deleted the dev/joergs/master/python313-tlspsk branch April 15, 2024 14:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants