please update to work with github.com/pyca/bcrypt

[danstender]

The implementation of bcrypt of pyca isn't fully compatible with py-bcrypt, but lacks bcrypt.kdf(). Thus it isn't working together with asyncssh. Could you update asyncssh/public_key.py somehow to catch that? As a matter of fact the actual pypi/bcrypt became the newer/the other library in the meanwhile.

[ronf]

As you said, the PyCA version of bcrypt claims to be backward compatible with py-bcrypt, but this is not the case. Without an implementation of bcrypt_pbkdf(), there's no way I can support this version in AsyncSSH. It looks like there's already an issue open around adding KDF support on the PyCA version, but it doesn't appear to be there yet.

About the best I could do at this point is to detect that the wrong version of bcrypt is installed and not use it, but that would mean encrypted OpenSSH private keys would not be supported (like when no version of bcrypt is installed).

[ronf]

Ok - I've gone ahead and put in a check to make sure the bcrypt KDF function exists before enabling bcrypt support in the public_key module. If a version of bcrypt is installed which doesn't have this function, AsyncSSH won't use it. This is available in AsyncSSH 1.5.5, which I just released.

[danstender]

That's great, thanks. We weren't too careful with checking it over and just replaced py-bcrypt with pyca/bcrypt and that move broke some packages in Debian (like asyncssh). And nobody knows when this function gets reimplemented.
I expect the tests keep breaking due to KeyExportError, could be this worked around somewhow? (haven't got deeper into asyncssh, yet)

[ronf]

The tests are supposed to be conditional based on the presence of bcrypt as well, but it looks like there's a separate test for bcrypt availability there, so it's going to need the same change I put in public_key.

For now, AsyncSSH should work ok with the wrong bcrypt installed, but I think you're right that the tests will probably still fail in that case, with a key import/export error rather than the error about kdf() being missing.

I've got some other stuff to do tomorrow, but I'll try to get a fix for the tests checked in soon.

[danstender]

Great, thanks in advance. When the package builds all right with tests, maybe that extra feature could be spared in an environment where py-bcrypt isn't available for some time, until full backward compatibility is reached. However you might consider to drop the dependency against py-bcrypt, anyway in the long run. We've dropped it in Debian completely because there were some security problems indicated for that library: https://bugs.debian.org/796853

[ronf]

Yeah, thanks - once there is an alternative version of bcrypt available which provide support for kdf(), I’d be happy to change my dependency from py-bcrypt to that (preferably to the PyCA version to avoid conflicting dependencies).

In the meantime, I’m not too worried about the “wraparound bug”. It only applies in cases where a password larger than 254 characters is used, which seems extremely unlikely. This is also completely under the control of the AsyncSSH user, since bcrypt is only used to encrypt and decrypt local private keys being passed in to AsyncSSH.

Regarding the change to test for bcrypt compatibility, I could remove it at that point, but there are enough alternate versions of bcrypt floating around that it's probably best to keep it. It's extremely cheap - it happens only at import time, and it's basically just one extra check for the presence of kdf() in the package:

try: 
    import bcrypt
    bcrypt_available = hasattr(bcrypt, 'kdf')
except ImportError: # pragma: no cover
    bcrypt_available = False

Previously the first assignment here was a "bcrypt_available = True".

[ronf]

The updated check for bcrypt having the "kdf" function in the test code is now checked into the "develop" branch. I'll get it moved over to "master" and into a new release soon.

[ronf]

The fixes above are now released in AsyncSSH 1.5.6.

[reaperhulk]

pyca/bcrypt has been rewritten and will have kdf in the next release (3.0.0) which should be out sometime today :)

[ronf]

Terrific! The already related AsyncSSH should work with PyCA's bcrypt now that this is available.

In addition, I've checked in a change in setup.py in the "develop" branch to make PyCA's bcrypt 3.0.0 be an optional dependency replacing py-bcrypt. I'll roll this into the next release.