Allow bcrypt authentication

[Laman]

.htpasswd file format used for Trac authentication allows for bcrypt password digest but the current Trac implementation doesn't reflect this (the user is not authenticated). I propose to add this functionality.

[Jun Omae]

Thanks.

1. I think we should use $2b$ rather than $2y$ as a prefix.

Python 3.10.2 (main, Jan 15 2022, 18:02:07) [GCC 9.3.0] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> import crypt
>>> for method in crypt.methods: method, crypt.mksalt(method)
...
(<crypt.METHOD_SHA512>, '$6$CCWJgk75dv3A1NMS')
(<crypt.METHOD_SHA256>, '$5$/3/hmHBny/.nBNu0')
(<crypt.METHOD_BLOWFISH>, '$2b$12$3k8a4Id5P8a/HSqRXWX/P7')
(<crypt.METHOD_MD5>, '$1$m0N4eQqu')
(<crypt.METHOD_CRYPT>, 'dv')

See also: ​https://en.wikipedia.org/wiki/Bcrypt#Versioning_history

2. The added test cases fail on Windows.

[Laman]

1. I guess. Curiosly both my htpasswd and the online htpasswd generator linked from Trac docs output $2y$. And either version got verified by both my crypt and the passlib.

2. Ah, I see. I fixed the passlib version.

But the crypt compatibility function in passlib is deprecated and it proved difficult to replace it, so I instead unified the interface to the passlib verify function.

Tests passed on passlib 1.6.5 and 1.7.4

[Jun Omae]

Thanks for the revised patch. I tried trac.auth.2.diff​ with Python 3.6 - 3.10 on Windows, and got the following failures. I consider verify_hash() should catch exceptions of passlib and return False.

======================================================================
ERROR: test_bcrypt (trac.web.tests.auth.BasicAuthenticationTestCase)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "C:\usr\src\trac\trac.git\trac\web\tests\auth.py", line 210, in test_bcrypt
    self.assertTrue(auth.test('bcrypt', 'bcrypt'))
  File "C:\usr\src\trac\trac.git\trac\web\auth.py", line 363, in test
    return self.verify(password, the_hash)
  File "C:\usr\src\trac\trac.git\trac\util\compat.py", line 33, in verify_hash
    return ctx.verify(secret, the_hash)
  File "C:\usr\src\trac\venv\py310\lib\site-packages\passlib\context.py", line 2347, in verify
    return record.verify(secret, hash, **kwds)
  File "C:\usr\src\trac\venv\py310\lib\site-packages\passlib\utils\handlers.py", line 792, in verify
    return consteq(self._calc_checksum(secret), chk)
  File "C:\usr\src\trac\venv\py310\lib\site-packages\passlib\handlers\bcrypt.py", line 591, in _calc_checksum
    self._stub_requires_backend()
  File "C:\usr\src\trac\venv\py310\lib\site-packages\passlib\utils\handlers.py", line 2254, in _stub_requires_backend
    cls.set_backend()
  File "C:\usr\src\trac\venv\py310\lib\site-packages\passlib\utils\handlers.py", line 2156, in set_backend
    return owner.set_backend(name, dryrun=dryrun)
  File "C:\usr\src\trac\venv\py310\lib\site-packages\passlib\utils\handlers.py", line 2176, in set_backend
    raise default_error
passlib.exc.MissingBackendError: bcrypt: no backends available -- recommend you install one (e.g. 'pip install bcrypt')

======================================================================
ERROR: test_extra_entries_ignored (trac.web.tests.auth.BasicAuthenticationTestCase)
Extra entries and comments are ignored.
----------------------------------------------------------------------
Traceback (most recent call last):
  File "C:\usr\src\trac\trac.git\trac\web\tests\auth.py", line 228, in test_extra_entries_ignored
    self.assertTrue(auth.test('bcrypt', 'bcrypt'))
  File "C:\usr\src\trac\trac.git\trac\web\auth.py", line 363, in test
    return self.verify(password, the_hash)
  File "C:\usr\src\trac\trac.git\trac\util\compat.py", line 33, in verify_hash
    return ctx.verify(secret, the_hash)
  File "C:\usr\src\trac\venv\py310\lib\site-packages\passlib\context.py", line 2347, in verify
    return record.verify(secret, hash, **kwds)
  File "C:\usr\src\trac\venv\py310\lib\site-packages\passlib\utils\handlers.py", line 792, in verify
    return consteq(self._calc_checksum(secret), chk)
  File "C:\usr\src\trac\venv\py310\lib\site-packages\passlib\handlers\bcrypt.py", line 591, in _calc_checksum
    self._stub_requires_backend()
  File "C:\usr\src\trac\venv\py310\lib\site-packages\passlib\utils\handlers.py", line 2254, in _stub_requires_backend
    cls.set_backend()
  File "C:\usr\src\trac\venv\py310\lib\site-packages\passlib\utils\handlers.py", line 2156, in set_backend
    return owner.set_backend(name, dryrun=dryrun)
  File "C:\usr\src\trac\venv\py310\lib\site-packages\passlib\utils\handlers.py", line 2176, in set_backend
    raise default_error
passlib.exc.MissingBackendError: bcrypt: no backends available -- recommend you install one (e.g. 'pip install bcrypt')

[Laman]

Sorry, I don't have an easy access to a Windows machine to test properly.

But the missing bcrypt backend bothers me. If the user sets up bcrypt digest while lacking the support, it is a miscofiguration on his part - and by returning False we just cover the error without guiding him to the cause.

There is a backup ​pure python backend:

The pure-python backend (5) is disabled by default!

That backend is currently too slow to be usable given the number of rounds required for security. That said, if you have no other alternative and need to use it, set the environmental variable PASSLIB_BUILTIN_BCRYPT="enabled" before importing Passlib.

I consider it better to go this way and provide a working, even if suboptimal solution. For other potential crashes, I am again hesitant to just catch and hide them. Are you sure it is the best course?

[Jun Omae]

Replying to Laman:

Sorry, I don't have an easy access to a Windows machine to test properly.

Modifying trac/util/compat.py temporarily like the following, it will be able to test on Linux.

 # Windows doesn't have a crypt module by default.
 try:
-     from crypt import crypt
+     raise ImportError  # from crypt import crypt
 except ImportError:

But the missing bcrypt backend bothers me. If the user sets up bcrypt digest while lacking the support, it is a miscofiguration on his part - and by returning False we just cover the error without guiding him to the cause.

That makes sense.

In proposed changes, log:jomae.git@trunk:t13464, the following warning is printed when the password hash is using bcrypt and bcrypt library is not installed.

Warning: cannot verify password for user "username" (MissingBackendError: no bcrypt backends available -- recommend you install one (e.g. 'pip install bcrypt'))

The changes include:

• allowing sha-256/512 password hash
• contrib/htpasswd.py: add -t method option for hash method of passwords

[Jun Omae]

Fixed in [17619].

[Jun Omae]

Python on macOS supports only crypt. Unit tests failing.

$ python3
Python 3.9.10 (main, Jan 15 2022, 11:48:15)
[Clang 12.0.0 (clang-1200.0.32.29)] on darwin
Type "help", "copyright", "credits" or "license" for more information.
>>> import crypt
>>> crypt.methods
[<crypt.METHOD_CRYPT>]

======================================================================
FAIL: test_bcrypt (trac.web.tests.auth.BasicAuthenticationTestCase.test_bcrypt)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/Users/runner/work/trac/trac/trac/web/tests/auth.py", line 243, in test_bcrypt
    self.assertTrue(auth.test('bcrypt', 'bcrypt'))
AssertionError: False is not true

======================================================================
FAIL: test_extra_entries_ignored (trac.web.tests.auth.BasicAuthenticationTestCase.test_extra_entries_ignored)
Extra entries and comments are ignored.
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/Users/runner/work/trac/trac/trac/web/tests/auth.py", line 281, in test_extra_entries_ignored
    self.assertTrue(auth.test('bcrypt', 'bcrypt'))
AssertionError: False is not true

======================================================================
FAIL: test_sha256 (trac.web.tests.auth.BasicAuthenticationTestCase.test_sha256)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/Users/runner/work/trac/trac/trac/web/tests/auth.py", line 249, in test_sha256
    self.assertTrue(auth.test('sha256', 'sha256'))
AssertionError: False is not true

======================================================================
FAIL: test_sha512 (trac.web.tests.auth.BasicAuthenticationTestCase.test_sha512)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/Users/runner/work/trac/trac/trac/web/tests/auth.py", line 255, in test_sha512
    self.assertTrue(auth.test('sha512', 'sha512'))
AssertionError: False is not true

[Jun Omae]

Fixed in [17622].