Bug 15756 - Replace `crypt` module in python/samba/netcmd/user/readpasswords/common.py
Summary: Replace `crypt` module in python/samba/netcmd/user/readpasswords/common.py
Status: RESOLVED FIXED
Alias: None
Product: Samba 4.1 and newer
Classification: Unclassified
Component: Python (show other bugs)
Version: 4.20.1
Hardware: All All
: P5 normal (vote)
Target Milestone: ---
Assignee: Jule Anger
QA Contact: Samba QA Contact
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2024-11-26 13:49 UTC by Andreas Schneider
Modified: 2025-02-17 15:52 UTC (History)
3 users (show)

See Also:


Attachments
patch for 4.21 (27.77 KB, patch)
2024-12-21 06:38 UTC, Douglas Bagnall
asn: review+
dbagnall: ci-passed+
Details
patch for 4.20 which we probably don't need (27.74 KB, patch)
2024-12-21 06:55 UTC, Douglas Bagnall
dbagnall: ci-passed-
Details
patch for 4.21 (30.53 KB, patch)
2025-01-20 08:22 UTC, Andreas Schneider
no flags Details
4.21 patch with fix, with added Signed-off-by (30.61 KB, patch)
2025-01-22 09:12 UTC, Douglas Bagnall
asn: review+
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Andreas Schneider 2024-11-26 13:49:51 UTC
In python/samba/netcmd/user/readpasswords/common.py we use the `crypt` module, which has been removed in Python 3.13. See

https://docs.python.org/3/library/crypt.html
Comment 1 Andreas Schneider 2024-11-26 13:54:17 UTC
[57(443)/57 at 5m45s] samba.tests.samba_tool.user_virtualCryptSHA_userPassword(ad_dc:local)
/usr/lib64/python3.13/unittest/case.py:597: RuntimeWarning: TestResult has no addDuration method
  warnings.warn("TestResult has no addDuration method",
UNEXPECTED(failure): samba.tests.samba_tool.user_virtualCryptSHA_userPassword.samba.tests.samba_tool.user_virtualCryptSHA_userPassword.UserCmdCryptShaTestCaseUserPassword.test_no_gpg_both_hashes_no_rounds(ad_dc:local)
REASON: Exception: Exception: Traceback (most recent call last):
  File "/tmp/samba-testbase/b1/samba-o3/bin/python/samba/tests/samba_tool/user_virtualCryptSHA_userPassword.py", line 29, in test_no_gpg_both_hashes_no_rounds
    out = self._get_password("virtualCryptSHA256,virtualCryptSHA512")
  File "/tmp/samba-testbase/b1/samba-o3/bin/python/samba/tests/samba_tool/user_virtualCryptSHA_base.py", line 78, in _get_password
    self.assertCmdSuccess(result,
    ~~~~~~~~~~~~~~~~~~~~~^^^^^^^^
                          out,
                          ^^^^
                          err,
                          ^^^^
                          "Ensure getpassword runs")
                          ^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/tmp/samba-testbase/b1/samba-o3/bin/python/samba/tests/samba_tool/base.py", line 103, in assertCmdSuccess
    self.assertIsNone(exit, msg=msg.replace("\n]\n", "\n] \n"))
    ~~~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
AssertionError: -1 is not None : exit[-1] stdout[] stderr[ERROR: Virtual attribute 'virtualCryptSHA256' not supported: crypt required
]: Ensure getpassword runs
Comment 2 Samba QA Contact 2024-12-20 08:00:05 UTC
This bug was referenced in samba master:

833455c7f9f71583d567e3a53e854567cd8c3b0b
1edb12f79593d0b2aac36d5acdaaae6f495772f6
c7597380b479208e33a403211cec9b3c7bd3f034
5f365e71c1fa8cdc533159283a5977164b5d39f2
88e3c82d88a68cf972f8189e1c3718698b49974a
5636d30c0959fd4a211ee7b8d1b267dcdbf0b963
405187d2ef4920a9a284649c9c3287f5844d5180
4af4dd8135e8edbe2a16cfdfc7ded8c145c82e98
552053b6445611ecef6ac4c11c55ebf92f03571d
929f4d0ca57129d493839169ff5b43ae06c0b051
8b84282008dc372d67ba01c8fe256ef756c3dcfb
Comment 3 Douglas Bagnall 2024-12-21 06:38:12 UTC
Created attachment 18518 [details]
patch for 4.21

Patch for 4.21 which we will need if distros are going to combine Samba 4.21 and Python 3.13. 

The bootstrap part of the patch is dropped because 4.21 never had the python3-crypt_r package.
Comment 4 Douglas Bagnall 2024-12-21 06:55:32 UTC
Created attachment 18519 [details]
patch for 4.20 which we probably don't need

The 4.20 patch is maybe unnecessary, because 4.20 users are likely to have an older Python too.

The CI pipeline fails in two places: 
https://gitlab.com/samba-team/devel/samba/-/pipelines/1598508981

1. -O3 compilation of vfs_ceph on centos 8.
2. crypt(3) on centos7 seems to return ENOENT in a case we expect to succeed.

I am not investigating further.
Comment 5 Douglas Bagnall 2024-12-21 06:59:46 UTC
(In reply to Douglas Bagnall from comment #4)

> 2. crypt(3) on centos7 seems to return ENOENT in a case we expect to succeed.

I remember now we warn ourselves about this behaviour in the patch itself!

+	 * RHEL 7 (which does not use libcrypt / libxcrypt) returns a
+	 * non-NULL pointer from crypt_r() on success but (always?)
+	 * sets errno during internal processing in the NSS crypto
+	 * subsystem.
+	 *
+	 * By preferring crypt_rn we avoid the 'return non-NULL but
+	 * set-errno' that we otherwise cannot tell apart from the
+	 * RHEL 7 behaviour.
+	 */
Comment 6 Andreas Schneider 2025-01-07 10:15:57 UTC
Comment on attachment 18518 [details]
patch for 4.21

lgtm
Comment 7 Andreas Schneider 2025-01-07 10:16:22 UTC
Jule, please add the patch to 4.21. Thanks!
Comment 8 Jule Anger 2025-01-17 13:01:58 UTC
Re-assigning to Douglas.
Please add missing cherry picked lines and one missing Signed-off-by tag. Thanks!
Comment 9 Douglas Bagnall 2025-01-17 20:31:51 UTC
Thanks for catching that, Jule. 

The patch will change anyway, because it turns out to be bad (bug 15784).
Comment 10 Andreas Schneider 2025-01-20 08:22:04 UTC
Created attachment 18533 [details]
patch for 4.21
Comment 11 Douglas Bagnall 2025-01-22 09:12:06 UTC
Created attachment 18535 [details]
4.21 patch with fix, with added Signed-off-by

Thanks Andreas. It looks good, but Jule also noticed I missed a signed-off-by, which I have added to this version.
Comment 12 Andreas Schneider 2025-01-22 11:53:51 UTC
Comment on attachment 18535 [details]
4.21 patch with fix, with added Signed-off-by

lgtm
Comment 13 Andreas Schneider 2025-01-22 11:54:15 UTC
Jule, please apply the patch to 4.21. Thanks
Comment 14 Jule Anger 2025-01-23 14:04:06 UTC
Thanks. Pushed to autobuild-v4-21-test.
Comment 15 Samba QA Contact 2025-01-23 15:16:13 UTC
This bug was referenced in samba v4-21-test:

0dccda38f27b3bbda5d2a4de588a333ff554651a
9428e088b1f1e38a5f95933b29197c0ef3775c62
07b6b34178186fba6b101979c6a6c8d85e6e88c5
bbd30dc1d5def60dcac33d95af75bdc1211f1eb7
48297d181718e6b8450a320d6e82b70279aea9ac
4004f475c4dc72127a127dd897df42b9b8dcf018
0cc9a50269e3cfce0d1e3a4fc8f3c0a2e38db3c0
332cd9f58615127ed1d14179e5785228e80a32b9
399d56fe13acb7c25458d695bee3fe7520ac96ff
720ecf666f2bbdbe758eaae5312bcb1df5513b2f
Comment 16 Jule Anger 2025-02-03 14:42:40 UTC
Closing out bug report.

Thanks!
Comment 17 Samba QA Contact 2025-02-17 15:52:56 UTC
This bug was referenced in samba v4-21-stable (Release samba-4.21.4):

0dccda38f27b3bbda5d2a4de588a333ff554651a
9428e088b1f1e38a5f95933b29197c0ef3775c62
07b6b34178186fba6b101979c6a6c8d85e6e88c5
bbd30dc1d5def60dcac33d95af75bdc1211f1eb7
48297d181718e6b8450a320d6e82b70279aea9ac
4004f475c4dc72127a127dd897df42b9b8dcf018
0cc9a50269e3cfce0d1e3a4fc8f3c0a2e38db3c0
332cd9f58615127ed1d14179e5785228e80a32b9
399d56fe13acb7c25458d695bee3fe7520ac96ff
720ecf666f2bbdbe758eaae5312bcb1df5513b2f