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

WAMP CRA password salt is sent base64 encoded #385

Open
oberstet opened this issue Oct 24, 2021 · 5 comments
Open

WAMP CRA password salt is sent base64 encoded #385

oberstet opened this issue Oct 24, 2021 · 5 comments

Comments

@oberstet
Copy link
Member

Pls see:

Proposal: change text in ap_authentication_cra.md from

The CHALLENGE.Details.salt|string is the password salt in use.

to

The CHALLENGE.Details.salt|string is the password salt in use as a base64-encoded string.

@om26er
Copy link
Contributor

om26er commented Oct 24, 2021

I am not sure if that is the correct change to fix the linked issue.

The issue is that some WAMP implementations (like Nexus) use the PBKDF2 derived key bytes to sign the challenge, which is broken (at least with Crossbar). They should rather encode those bytes to base64 string AND then get a byte representation of that string to sign the challenge. At this stage I am not sure if that is a bug in Crossbar ?

So we need to clarify what should clients do.

@oberstet
Copy link
Member Author

sorry, I don't understand ^

but I of course agree: we should improve the spec text.

rgd "bug in crossbar": that would probably mean a bug in AB Py, J and JS as well. not sure. I think it is easier to look at this as a spec bug;)

if there is a bug in crossbar, we should first have a unit test that demonstrates the bug ...

PBKDF2 derived key bytes to sign the challenge, which is broken (at least with Crossbar).

that's what crossbar does?

https://github.com/crossbario/autobahn-python/blob/a35f22eeaafca7568f1deb35c4a1b82ae78f77d4/autobahn/twisted/wamp.py#L851

here are the helpers:

https://github.com/crossbario/autobahn-python/blob/a35f22eeaafca7568f1deb35c4a1b82ae78f77d4/autobahn/wamp/auth.py#L587

@oberstet
Copy link
Member Author

oberstet commented Oct 24, 2021

actually, it looks like we really could make good use of some salt-specific unit tests .. don't see much here:

https://github.com/crossbario/crossbar/blob/93f3931eb3fb34bae59c6a8d3d4bcbbef7131d67/crossbar/router/test/test_authorize.py#L92
https://github.com/crossbario/autobahn-python/blob/a35f22eeaafca7568f1deb35c4a1b82ae78f77d4/autobahn/wamp/test/test_wamp_auth.py#L105

but it looks plausible to add some salt-specific test code to above. does Nexus have respective tests? if so, we could copy and add their test vectors and vice versa. that way, interop is ensure by test coverage

@om26er
Copy link
Contributor

om26er commented Oct 24, 2021

I'll look into coming up with a test case, sure. Let me try to explain again, as my previous reply is confusing even for me :-D

This call (auth.derive_key()) returns base64 encoded key
https://github.com/crossbario/autobahn-python/blob/a35f22eeaafca7568f1deb35c4a1b82ae78f77d4/autobahn/twisted/wamp.py#L854

Then auth.compute_wcs() internally calls key.encode('utf-8') AND NOT base64.b64decode(key) when signing the challenge.

What's not clear is and should be cleared in the spec document: Should the challenge be signed with the base64 key OR should it actually be signed with the raw bytes of the key.

Crossbar (and all autobahn libraries) does the former, Nexus does the latter

@oberstet
Copy link
Member Author

oberstet commented Oct 25, 2021

sounds cool! thanks!

Should the challenge be signed with the base64 key OR should it actually be signed with the raw bytes of the key.

yes, it should be signed using the binary key (secret), and this is what crossbar/autobahn is doing:

key = self._secret.encode('utf8')

above is using utf8, since here the data self._secret is read from the node config to configure the secret to be used

Should the challenge be signed with the base64 key OR should it actually be signed with the raw bytes of the key.

the signature must be created using PBKDF2. this algo does not talk about strings, but bytes - hence the raw key and raw salt must be used when computing the signature

"""
WAMP-CRA allows the use of salted passwords following the PBKDF2 key derivation scheme. With salted passwords, the password itself is never stored, but only a key derived from the password and a password salt. This derived key is then practically working as the new shared secret.
"""

only for wire transfer the question of encoding pops up. and this text is indeed missing, hence

The CHALLENGE.Details.salt|string is the password salt in use as a base64-encoded string.

anyways, I still don't quite understand the problem ... hence I think before changing any code in AB or CB, we should add unit test coverage for salt-based WAMP-CRA as I still think everything is good (but I might be wrong and too dumb to realize the bug or what ..)


from cryptography.hazmat.primitives.kdf.pbkdf2 import PBKDF2HMAC

https://github.com/crossbario/autobahn-python/blob/a35f22eeaafca7568f1deb35c4a1b82ae78f77d4/autobahn/wamp/auth.py#L475

https://cryptography.io/en/latest/hazmat/primitives/key-derivation-functions/#cryptography.hazmat.primitives.kdf.pbkdf2.PBKDF2HMAC

salt (bytes) – A salt. Secure values 1 are 128-bits (16 bytes) or longer and randomly generated.

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

No branches or pull requests

2 participants