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-SCRAM specification #135

Closed
sehoffmann opened this issue Feb 14, 2015 · 55 comments
Closed

WAMP-SCRAM specification #135

sehoffmann opened this issue Feb 14, 2015 · 55 comments

Comments

@sehoffmann
Copy link

As discussed in #128 WAMP-CRA is unsafe in regards to database thefts. A new and better authentication method is required. SCRAM turned out to be very promising.

oberstet already gave a summary (#128 (comment)):

From looking at SCRAM, this claims to provide protection against these attack vectors:

Eavesdropping
Replay
Database Compromise
and also against dictionary attacks and password collisions.

It additionally claims to protect against impersonating a server (after the original server had signed up a user).

Here are some references:

http://en.wikipedia.org/wiki/Salted_Challenge_Response_Authentication_Mechanism
https://tools.ietf.org/html/rfc5802
http://www.mongodb.com/blog/post/improved-password-based-authentication-mongodb-30-scram-explained-part-1
http://www.mongodb.com/blog/post/improved-password-based-authentication-mongodb-30-scram-explained-part-2
The good news is, with the WAMP messages HELLO, CHALLENGE, AUTHENTICATE and WELCOME (and the extensibility built in those messages), we have the required message flow in place without changing WAMP.

However, I don't think it makes sense to put this into WAMP-CRA as it exists.

I think we should come up with a mapping of SCRAM to WAMP: WAMP-SCRAM (wamp-scram)

This should be pretty straight-foward, and we can reuse an already designed, scrutenized scheme.

We also do not need new crypto primitives, as the following will do fine:

SHA256
HMAC
PBKDF2
XOR

@oberstet
Copy link
Member

FWIW, I am +1 on this, and want to come up with a proof-of-concept implementation "soon". That means: AutobahnJS, AutobahnPython and Crossbar.io.

Once we have it, WAMP-SCRAM should always be chosen over WAMP-CRA, as the former is expected to provide more security than the latter.

@Paranaix The most important point is the mapping to WAMP messages. You mentioned in the other thread that reusing existing message exchange would be "misuse", because in WELCOME the client does not yet know which auth method the server chooses, but still has to send some SCRAM specific data already. Yes, that's true. However: the number of required roundtrips for establishing a session does matter in some situations. E.g. imagine widget that tracks activity on old-school Web pages. The user will navigate around, loading new pages all time, which means establishing new WAMP sessions all the time. If the connection runs over TLS, there are a lot round-trips happening even before the WAMP opening handshake begins. Another argument: adding some new meaning to existing message details does not change protocol state machines in existing WAMP implementations. Adding a multi-round CHALLENGE/AUTHENTICATE message exchange in between HELLO and WELCOME does change that. A far more invasive change.

@sehoffmann
Copy link
Author

In the case additional fields are allowed for HELLO and WELCOME, I would opt for them being completely generic. While e.g a succesful WAMP-SCRAM authentication would still require certains properties to be present, additional ones shouldn't cause an error but should be simply ignored. This does not only simplify the implementation but it also makes everything more flexible and e.g allows user code to attach additional information on succesful authentication (for example a session token).

@oberstet
Copy link
Member

@ecorm
Copy link
Contributor

ecorm commented Feb 2, 2018

You might also want to consider the Secure Remote Password protocol.

@oberstet oberstet changed the title WAMP-SCRAM Implementation WAMP-SCRAM Feb 15, 2018
@oberstet oberstet changed the title WAMP-SCRAM WAMP-SCRAM specification Feb 15, 2018
@ecorm
Copy link
Contributor

ecorm commented Feb 21, 2018

If my interpretation of the SCRAM RFC is correct, it should be possible to strengthen standard SCRAM-SHA-1 or SCRAM-SHA-256 by pre-hashing the password client-side with a stronger algorithm such as bcrypt, scrypt, or Argon2.

With SCRAM, the hashing work factor is limited by the client's computing power. This is nice for reducing server workload, but has the downside that the password hash strength is limited by the client. These benchmarks published on browser-computed scrypt and Argon2 hashes (using asm.js code compiled for the browser) tell us that the Javascript implementations are roughly an order of magnitude slower than their native counterparts. What would normally take 100ms on a native implementation would seem to take on the order of 1-2 seconds on an asm.js implementation for an equivalent password hash strength.

SCRAM requires that Unicode username/password strings be normalized via the SASLprep profile (RFC4013) of the stringprep framework (RFC3454). I managed to find the reklatsmasters/saslprep Javascript library, and the author confirmed that it works on modern browsers. I also found eloquent/precis-js, a JavaScript implementation of the PRECIS framework RFC7564 + RFC7613, where PRECIS supersedes stringprep. Alas, RFC7564+RFC7613 have been recently superseded by RFC8264+RFC8265.

SASLprep preserves the case of usernames, whereas PRECIS has an optional profile for mapping the username to lowercase. In the case of SASLprep, it's not clear if the server can still choose to do a case-insensitive comparison of the username in the database.

One way to sidestep normalization would be to require that usernames/passwords can only be 7-bit ASCII. This seems to be permitted under SCRAM RFC 5802, section 2.2, Normalize:

Note that implementations MUST either implement SASLprep or disallow use of non US-ASCII Unicode codepoints in "str".

Another approach might be to adopt only NFKC Unicode normalization for username/password, which is supported on the browser by String.prototype.normalize(), with unorm as a polyfill. However, this approach would ignore characters prohibited by SASLprep, such as non-ASCII spaces, or control characters.

@ecorm
Copy link
Contributor

ecorm commented Feb 21, 2018

I've started working on a WAMP-SCRAM spec Markdown document on my personal fork. I'll let you folks know when there is enough meat in it for review.

@ecorm
Copy link
Contributor

ecorm commented Mar 2, 2018

Here is an initial WAMP-SCRAM draft on my personal fork. I'd like to get some initial feedback before submitting a pull request.

@konsultaner
Copy link
Contributor

@ecorm I really like the initial definition. I will soon start to implement it based on your description and give you feedback if something is not clear. @oberstet Is there a client that already supports WAMP-SCRAM?

@ecorm
Copy link
Contributor

ecorm commented Mar 3, 2018

Thanks for your offer of feedback, @konsultaner .

Someone needs to review the algorithms in my draft and compare it with RFC5802. RFC7677 has test vectors that you can use to check your implementation if you use the same test nonces.

If a client allows you to send arbitrary HELLO.Options, lets you receive arbitrary CHALLENGE.Details, and lets you send arbitrary AUTHENTICATE.Details, you should be able to use that client.

If your client is a web browser, the WebCryptoAPI provides native implementations of PBKDF2, SHA-256 and HMAC if you're in a secure context (i.e. HTTPS).

@ecorm
Copy link
Contributor

ecorm commented Mar 3, 2018

While writing the WAMP-SCRAM spec, I was also studying SRP, and I believe it to have fewer vulnerabilities, especially when used over an unsecure transport. I'd like to write a WAMP-SRP draft, but I cannot give any estimate as to when that will happen.

@oberstet oberstet added this to the spec-new-features milestone Mar 3, 2018
@konsultaner
Copy link
Contributor

konsultaner commented Mar 4, 2018

@ecorm I went through the RFCs to dig deeper into the topic. I found channel binding to be a quite useful feature, but I seem to not find a detailed description on how to implement it. You have a very detailed step by step description on how to generate hashes and messages for SCRAM. It would help people like me (that are not familiar with that topic) a lot if you could add what to use from the underlying TLS handshake in what step. Something like PlusHash(tlsMasterSecret) <- this is just an example and most probably wrong. At the moment I have no idea what to use where.

Is channel binding even possible in a browser? No, right?

The RFC for SCRAM has a sample for sha-1. It would be nice to have this for WAMP-SCRAM and its variants as well to have something to unit test on.

@ecorm
Copy link
Contributor

ecorm commented Mar 4, 2018

@konsultaner I did not dig deeper into channel binding because it's not a feature we were planning to use for our product.

RFC5929 section 4.1 describes exactly how to hash the certificate data for the tls-server-end-point channel binding type. I don't think I can improve on the description in RFC5929, and I won't copy it verbatim in the WAMP-SCRAM spec. What I can do is provide a link/citation to that specific section for convenience.

AUTHENTICATE.Extra.cbind_data is where you put the Base64-encoded channel binding data. AUTHENTICATE.Extra.cbind_data is part of the the computed AuthMessage.

Note that the "SCRAM Algorithms" section is marked as non-normative, because the algorithms are already specified in the SCRAM RFC document.

Is channel binding even possible in a browser? No, right?

https://stackoverflow.com/questions/2402121/within-a-web-browser-is-it-possible-for-javascript-to-obtain-information-about

The RFC for SCRAM has a sample for sha-1. It would be nice to have this for WAMP-SCRAM and its variants as well to have something to unit test on.

RFC7677 has an example for PBKDF2+SHA256. I'll add a "Test Vectors" non-normative section that uses the example from RFC7677.

@konsultaner
Copy link
Contributor

@ecorm Thank you!

@konsultaner
Copy link
Contributor

@ecorm

  • The user associated with authid does not have permission to impersonate the given impersonated_id.
  • Ther server recognizes authid but does not recognize impersonated_id.

Shouldn't they be optional too and be handled like

  • (Optional) The server does not recognize the given authid.

because

  1. no one should be able to find out what an authids permissions are
  2. no one should find out if an authid passed in the impersonated_id is a valid authid

@ecorm
Copy link
Contributor

ecorm commented Mar 5, 2018

@konsultaner

Shouldn't they be optional too and be handled like

When I wrote that, I though that for an unrecognized authid, the server wouldn't even bother checking bullet points 3 & 4.

But upon further thought, an attacker could very be a low-privilege user with a recognized authid that is trying to learn who the admin users are.

I'll make those two conditions optional. Good catch. 👍

@ecorm
Copy link
Contributor

ecorm commented Mar 5, 2018

@konsultaner Wait, now I remember why I did it that way.

The user associated with authid does not have permission to impersonate the given impersonated_id.

I think it's acceptable for a user to know if his/her own authid has permission to impersonate. It's not leaking any permission information about other users.

The server recognizes authid but does not recognize impersonated_id.

In that case, if the server ABORTs, nothing is leaked about the existence of impersonated_id. The attacker doesn't know if it's because he/she doesn't have permission to impersonate impersonated_id, or if it's because impersonated_id doesn't exist.

@konsultaner
Copy link
Contributor

@ecorm

In that case, if the server ABORTs, nothing is leaked about the existence of impersonated_id. The attacker doesn't know if it's because he/she doesn't have permission to impersonate impersonated_id, or if it's because impersonated_id doesn't exist.

Lets say the router will send mock CHALLENGE messages for a wrong authids. If the attacker had an administrative authid (i.e. "root" in the worst case) that most probably is allowed to impersonate users, the attacker could brute force others. The attacker would use "root" as authId and whenever the router response is a CHALLENGE, the impersonated_id is a valid authid.

I know this scenario is not very likely to happen, but it could.

I think it's acceptable for a user to know if his/her own authid has permission to impersonate. It's not leaking any permission information about other users.

I'm not an security expert. Since the ABORT doesn't say why it was aborted, it is probably worth a discussion? I don't know.

@oberstet
Copy link
Member

oberstet commented Mar 5, 2018

The impersonated_id has nothing to do with WAMP-SCRAM, so this should be removed from the proposal here ..

@oberstet
Copy link
Member

oberstet commented Mar 5, 2018

rgd WAMP-CRA renaming of attributes (eg iterations): no, I am -1 on that, as it is a breaking change, and it has no gain ..

@ecorm
Copy link
Contributor

ecorm commented Mar 5, 2018

Instead of aiming to expose all possible knobs and whistles, I'd rather chose sensible defaults

If you're referring to hashing algorithm parameters, it's not feasible to "hard-code" them because they are intended to be re-tuned in the future as hardware performance advances. The exceptions to this might be the "p" parameter of Argon2 and the "r" parameter of scrypt.

WAMP router implementations can certainly choose to provide default algorithm parameters, but that is an implementation detail that is outside the scope of the WAMP spec, IMO.

I'd love for there to be only one "golden" hashing algorithm, but there is no agreement in the industry as to which is the best one. Some prefer older algorithms such as bcrypt or scrypt, because they have been battle-tested for longer. Others prefer bleeding edge such as Argon2. And others prefer the NIST-approved PBKDF2.

Even if WAMP-SCRAM covers four different hashing algorithms, nothing prevents a WAMP implementation from only implementing one or two among them. A WAMP-based application would likely only adopt one hashing algorithm.

Eg this also applies to impersonate. We should hide that (not expose) for now

Since user impersonation can done with other authmethods, I tend to agree that it should not be exposed in WAMP-SCRAM for now. I can lift all the impersonation stuff out of the WAMP spec and place it in a separate document for now, pending discussion/decision in issue #297.

I could do the same for the channel binding feature - lift it out of the "basic" WAMP-SCRAM spec.

rgd WAMP-CRA renaming of attributes: no, as it is a breaking change, and it has not gain ..

The suggestion was not to rename WAMP-CRA attributes, but to rename WAMP-SCRAM's cost attribute to be more in line with WAMP-CRA's iterations.

@meejah
Copy link
Member

meejah commented Mar 5, 2018

Regarding naming: I think it makes sense to keep familiar names for whatever standard (so, e.g. keep "cost" if that's the most-used term). I don't see any gain trying to make the attributes match (especially because they aren't the same -- just a similar concept).

@ecorm I think what @oberstet is getting at is: we should pick our favourite algorithm (for now?) to narrow the choices for users/implementors who don't want to learn about all of them and weigh the tradeoffs. We can always add more later. We can also choose to add algorithm support but still "bless" a best one (i.e. wording in the spec, like "if you don't know how to choose a hash algorithm, use X").

Similar for "cost" etc parameters: we probably should include them as knobs (because they're expected to get bigger), but we should definitely also include sensible defaults. Like "in 2018, cost parameter X is the default and is recommended [because ...]".

The point here, from my perspective, is: someone who knows little about hash algorithms (etc) should be able to turn on "WAMP-SCRAM" and still be provided with secure, sensible defaults and a working system. They definitely should NOT have to read a bunch of stuff and try to choose between pbkdf2, bcrypt, scrypt, argon2, etc etc. even if "advanced" users can go and do that. Any choices that are "definitely bad" should be removed (or not added).

I think we should choose "argon2" as the default, blessed password-hashing algorithm.

@ecorm
Copy link
Contributor

ecorm commented Mar 5, 2018

@meejah

I think we should choose "argon2" as the default, blessed password-hashing algorithm.

At the very least, we should keep PBKDF2-SHA256, because that's the algorithm already specified in the standard SCRAM-SHA-256 (RFC7677), and some applications may be contractually bound to only using crypto functions approved by NIST. PBKDF2 is also implemented natively in the Javascript Web Crypto API (in secure contexts).

Don't forget that there are three "flavors" or Argon2: i, d, and id. I think id is the one generally recommended for password hashing - need to research that.

Perhaps PBKDF2 and Argon2id as the two possible choices, and we forget about bcrypt and scrypt? I personally wouldn't mind getting rid of bcrypt because of its password/key length restrictions. I'm also not fond of scrypt because it's difficult to tune the CPU cost independently of the memory cost.

Like "in 2018, cost parameter X is the default and is recommended [because ...]".

I'm not at all comfortable doing this within the WAMP spec, unless we can cite recommendations from authoritative sources. The general approach is to choose an acceptable run time (say 100ms), and tune the cost parameter on actual hardware until that run time is reached. This will be vastly different between mobile browsers and native desktop clients. The tunable memory parameter of Argon2 will also be vastly different among between hardware platforms.

RFC7677 recommends at least 4096 iterations of PBKDF2-SHA-256 as of November 2015, so we could simply quote that in the WAMP-SCRAM spec. Perhaps there's a similar recommendation in the Argon2 spec?

If the WAMP community wants to provide suggestions for parameters, perhaps it would be best done on a web page or "living document" outside of the WAMP specification (or even a StackOverflow community wiki). It would be even better if benchmark timing results were to accompany those recommendations.

Of course WAMP implementors would be free to provide whatever recommendations they want.

@meejah
Copy link
Member

meejah commented Mar 5, 2018

I'm not at all comfortable doing this within the WAMP spec, unless we can cite recommendations from authoritative sources.

Okay that's a fair point; if we can't find authoritative sources then I guess this should be dropped. Still, giving someone a bunch of parameters without recommendations sounds .. "bad".

From what I've read Argon2id basically rolls up all the same features as both bcrypt and scrypt combined -- leading to a conclusion of "keep PBKDF2 for 'older/wider-support' use-cases and use argon2id as the recommended approach". If we wanted to keep one of bcrypt or scrypt I'd also rather keep scrypt (but also happy to drop both in favour of argon2id)

I see "the WAMP specification" as a "living document" -- couldn't the WAMP community recommend upgrades/changes to the recommendations via pull-requests (for example)? But perhaps you're right that the spec should just leave it up to implementors to make recommendations for their users, as they see fit?

@ecorm
Copy link
Contributor

ecorm commented Mar 5, 2018

I see "the WAMP specification" as a "living document" -- couldn't the WAMP community recommend upgrades/changes to the recommendations via pull-requests (for example)?

It's currently a living document now while it's hosted on GitHub, but that will no longer be the case if/when it becomes an RFC. I picture GitHub as the "develop branch" for the WAMP spec, while the RFC is going to be a "master branch" release.

@meejah
Copy link
Member

meejah commented Mar 5, 2018

@ecorm re "living doc": okay I see what you mean. I would still be in favour of putting recommendations in the spec (with dates and assumptions stated). Ideally these could just be references to already-published recommendations.

For comparison: the scrypt paper includes particular parameters (with a date, and the processor used) but the RFC for scrypt does not.

The argon2 RFC draft includes a rule-based method of selecting the parameters, and includes example parameters for different use-cases (with processor speeds and memory specified).

@ecorm
Copy link
Contributor

ecorm commented Mar 5, 2018

Folks, can I get a thumbs up or down on this comment about restricting the WAMP-SCRAM hashing algorithms to only PBKDF2 and Argon2? The rationale for this restriction is to not overwhelm WAMP users/implementors with four different possible hashing algorithms. If thumbs down, please explain why.

Reason for PBKDF2: Approved by NIST and is a well known mature algorithm. Native implementation available in the Javascript Web Crypto API in secure contexts (i.e. HTTPS websites).

Reason for Argon2: Winner of the Password Hashing Competition and positioned to be the successor to scrypt and bcrypt. It's the default password hashing algorithm in libsodium and is now also supported in PHP. Used by KeePass.

@konsultaner
Copy link
Contributor

konsultaner commented Mar 6, 2018

"keep PBKDF2 for 'older/wider-support' use-cases and use argon2id as the recommended approach"

+1 for PBKDF2-SHA256, because implementers have used it for WAMP-CRA and are familiar with it
+1 for one extra KDF, if Adrgon2id is the best for the WAMP-SCRAM +1 for that

It was not hard to find libs to implement all suggested KDFs, but having no idea how to choose the best one, makes me feel like having to study all of them. Implementers shouldn't have to do it.

rgd WAMP-CRA renaming of attributes (eg iterations): no, I am -1 on that, as it is a breaking change, and it has no gain ..

@oberstet I'm totally with you that renaming is not a good idea. Whats with extra/details in CHALLENGE? Shouldn't that be fixed first? (#97)

@meejah
Copy link
Member

meejah commented Mar 6, 2018

Going through the spec more closely, some notes/questions/comments:

  • already nice and clear, with good links to relevant RFCs 👍

  • is there reasoning behind making the variants their "own" auth-method (e.g. "wamp-scram-pbkdf2") versus having one auth-method ("wamp-scram") with an option ("algorithm=pbkdf2" or similar)?

  • the version_num versus version_str stuff feels odd; could this instead be rolled into an "algorithm" string too? So like "argon2id-13" (for "argon2id" version "1.3") or "bcrypt-2b" (for "bcrypt" version "$2b$")?

  • for nonces, would it simplify implementations if the spec decided "it's a base64-encoded number" instead of just an arbitrary character sequence?

  • gs2_cbind_flag doesn't exactly roll off the tongue; is there a reason for that name?

  • in the table for mapping SCRAM->WAMP messages, the upper-right corner should read "HELLO" (not "WELCOME") I think?

  • there's already "authextra" in HELLO -- does it make sense to put the "nonce", "gs2_cbind_flag" and "impersonation_id" into that instead of additional "top level" HELLO keys? (Or am I just getting confused by implementation details in Autobahn-Python?)

@ecorm
Copy link
Contributor

ecorm commented Mar 7, 2018

@meejah Thanks for the detailed critique! 👍

is there reasoning behind making the variants their "own" auth-method (e.g. "wamp-scram-pbkdf2") versus having one auth-method ("wamp-scram") with an option ("algorithm=pbkdf2" or similar)?

There's already a mechanism for the client announcing its supported authmethods in the HELLO message. I felt it would have been more complicated to have two "levels" of authmethod announcement (one level for wamp-scram, and another level for algorithms), so I essentially flattened it into one level.

Let's say a client announces "authmethods": ["wamp-scram", "wamp-srp"], and both of those authmethods have their own variants. How can the client announce its supported authmethod variants in HELLO.Details without there being an attribute name collision? "wamp-scram-algorithms": ["argon2", "pbkdf2"] and "wamp-srp-algorithms": ["argon2", "pbkdf2"]? How do we manage the HELLO.Details "namespace" for all possible authmethods? Seems complicated to me.

One could ask if the client should even bother announcing its supported wamp-scram variants? If not, is it acceptable for the client to ABORT the authentication exchange once it realizes that it doesn't support the algorithm requested by the server for that particular user?

I we want to place the burden on the server to check that the client supports the required algorithm+version, then I think the authmethod string should be something like wamp-scram-argon2id-13.

the version_num versus version_str stuff feels odd; could this instead be rolled into an "algorithm" string too? So like "argon2id-13" (for "argon2id" version "1.3") or "bcrypt-2b" (for "bcrypt" version "$2b$")?

I wanted to avoid a single version attribute that could be either a string or a number, which is painful to deal with in statically typed languages.

I've seen an Argon2 API (libsodium, I believe) that takes a version argument as an integer. I thought that having the version in its own attribute would alleviate the burden of parsing the version number out of a more complex string.

I now think that the version suffix should be part of the authmethod string.

for nonces, would it simplify implementations if the spec decided "it's a base64-encoded number" instead of just an arbitrary character sequence?

Yes, I think it would simplify things, since base64 output is guaranteed not to contain the illegal , character. Implementations are already going to need base64 for other parts of the algorithm anyway.

gs2_cbind_flag doesn't exactly roll off the tongue; is there a reason for that name?

I almost went with the more readable channel_binding, but then changed my mind to match the terminology used by the SCRAM/SASL RFCs. After seeing your critique, I feel that I want to go back to channel_binding. I now see that Crossbar uses channel_binding for its cryptosign authmethod, so perhaps it's better that WAMP-SCRAM adopts the same attribute name for consistency.

in the table for mapping SCRAM->WAMP messages, the upper-right corner should read "HELLO" (not "WELCOME") I think?

Yes, that's correct. Good catch!

there's already "authextra" in HELLO -- does it make sense to put the "nonce", "gs2_cbind_flag" and "impersonation_id" into that instead of additional "top level" HELLO keys? (Or am I just getting confused by implementation details in Autobahn-Python?)

None of the authmethods that are currently in the WAMP spec make use of authextra (authextra is not found anywhere in the spec). But I now see that Crossbar/Autobahn uses it for its "not-yet-standard" authmethods.

I agree that it would be better for WAMP-SCRAM to follow the same convention for using authextra.

@oberstet
Copy link
Member

oberstet commented Mar 7, 2018

IMO, in general, this is exposing way to many knobs, and letting client-server negotiate details is also an invitation for trouble. Also, @ecorm could you pls file a PR so we can look at the proposed text?

gs2_cbind_flag

If that is supposed to work like for WAMP-cryptosign, then this isn't flag, and the attribute should be name channel_binding

impersonation_id

As mentioned, I am -1 on exposing this

@ecorm
Copy link
Contributor

ecorm commented Mar 7, 2018

@oberstet

IMO, in general, this is exposing way to many knobs

If you're referring to the number of possible algorithms, there seems to be consensus here to reducing the set of algorithms to only PBKDF2 and Argon2id. I've described the rationale for those two in #135 (comment). We could recommend Argon2id as the default, with PBKDF2 also available for those applications that are restricted to NIST-approved crypto functions. Most crypto libraries providing SHA-256 and HMAC also provide PBKDF2, so it wouldn't be that much more work to provide PBKDF2 in addition to Argon2.

As for the version value, this is required to be able to cope with future changes to Argon2 which results in different hashes for the same inputs. If a hypothetical Argon2 v1.4 is released someday to fix a vulnerability, applications will need to be able to continue authenticating against older v1.3 hashes until they can be upgraded to v1.4 hashes.

The other "knobs", cost and memory, need to be tuned for the client platform that will perform the hashing. If we fix those values to the "least common denominator", which would be a mobile web browser using a Javascript crypto library, then we are penalizing the native desktop clients that are able to compute much stronger hashes.

We can take a cue from the libsodium library, and hide the parallel knob and fix it to 1. That would be one less thing for the application developer to worry about. Other than that, I'm sorry, but I don't know what else I can do remove some of the knobs.

and letting client-server negotiate details is also an invitation for trouble.

There is no negotiation involved. Once a password has been hashed using a certain algorithm/version, the client MUST support that same algorithm/version to authenticate the user.

If your concern is with the algorithm and version number being appended to the wamp-scram authmethod string, I can understand how that might be tricky for routers to handle the different combinations when interpreting the HELLO message. I'm starting to think that @meejah's suggestion to have a separate attribute like "algorithm": "argon2id13" might be best.

Also, @ecorm could you pls file a PR so we can look at the proposed text?

I'll do so for the next iteration of my proposal. I wanted to get some initial feedback before submitting a PR. The current proposed text is here.

If that is supposed to work like for WAMP-cryptosign, then this isn't flag

I don't know about WAMP-cryptosign, but I can assure you that the SCRAM RFC calls it "channel binding flag". I need to investigate how WAMP-cryptosign uses its channel_binding parameter to make sure there is no confusion.

impersonation_id As mentioned, I am -1 on exposing this

I already said that I'd take it out of my proposal, pending a resolution to #297.

@meejah
Copy link
Member

meejah commented Mar 8, 2018

For the "algorithm" vs. auth-method name things, I think we should do this:

  • the authmethod is "wamp-scram"
  • there is an "algorithm" (or "scram_hash_algorithm" or similar) field saying which algorithm to use (if unspecified, argon2id-13 is used)
  • there are only two options for "scram_hash_algorithm": argon2id-13 or pbkdf2 (or maybe legacy-pbkdf2 to make it more obviously not-preferred?)
  • all implementations MUST support both algorithms.

Thus: there is no negotiation, the authmethod string is straightforward and it's more-obvious what to choose.

@meejah
Copy link
Member

meejah commented Mar 8, 2018

As for the other knobs, the situation is:

  • cost (or something similar) is definitely needed (for both algorithms)
  • a salt is required (for both algorithms)
  • memory and parallel are only parameters for argon2id

It's tempting to try and get rid of the argon2 params (or roll them up; e.g. "memory is always cost / 4 and parallel is always 2"). But I guess also if you were just forced to learn about "cost" and PBKDF2 vs. Argon2 then you might be surprised that the extra Argon parameters aren't there for some reason :)

@meejah
Copy link
Member

meejah commented Mar 8, 2018

Probably worth noting that in any case it's only "the person running Crossbar" that has to think about the parameters, and they could have "sensible defaults" (decided by the router authors).

@meejah
Copy link
Member

meejah commented Mar 8, 2018

Hmm, argon2_cffi outputs v=19 currently -- so the algorithm should maybe be argon2id-19?

@ecorm
Copy link
Contributor

ecorm commented Mar 10, 2018

@meejah

the authmethod is "wamp-scram"

I agree

there is an "algorithm" (or "scram_hash_algorithm" or similar) field saying which algorithm to use (if unspecified, argon2id-13 is used)

I agree in principal, but prefer to name it the "kdf" field, as it's only one step in the overall algorithm. I agree that the version information should be appended.

all implementations MUST support both algorithms. Thus: there is no negotiation, the authmethod string is straightforward and it's more-obvious what to choose.

I agree about both being mandatory to avoid the need to "negotiate". I'm not comfortable with legacy-pbkdf2 instead of pbkdf2 but what we can do is write non-normative text in the spec that explains why Argon2 might be preferable, and that PBKDF is also supported both those applications that are restricted to algorithms approved by standards bodies.

cost (or something similar) is definitely needed (for both algorithms)
a salt is required (for both algorithms)
memory and parallel are only parameters for argon2id

Now that bcrypt is out of the picture, we can go back to naming the CPU cost iterations. "iterations" is the term used by both the PBKDF2 and Argon2 literature.

It's tempting to try and get rid of the argon2 params (or roll them up; e.g. "memory is always cost / 4 and parallel is always 2"). But I guess also if you were just forced to learn about "cost" and PBKDF2 vs. Argon2 then you might be surprised that the extra Argon parameters aren't there for some reason :)

I'm going to research why libsodium locks the parallelization factor to 1. libsodium is a widely-used crypto library, with bindings to several languages, and it would be a shame to force WAMP-SCRAM implementors to not be able to use it.

Hmm, argon2_cffi outputs v=19 currently -- so the algorithm should maybe be argon2id-19?

19 is the decimal representation of hexadecimal 0x13. 1.3 is the actual, current version number.

@ecorm
Copy link
Contributor

ecorm commented Mar 10, 2018

Okay, I've figured out Argon2's parallelization parameter. On crypto libraries that support multithreading, this will make the library use multiple threads to compute the hash. With iterations and memory being equal, if two cores are used to compute a hash (with p=2), the resulting hash will be stronger (and therefore harder to crack) than a hash with p=1 computed in the same amount of time.

The p parameter can thus be useful to make a multi-cored server compute a stronger hash in a given amount of time. However, for WAMP-SCRAM, the hash has to be computed client-side, and I don't think an application developer should be making assumptions about the availability of multiple cores on their client platforms.

On crypto libraries that don't support multi-threading, if the p parameter is greater than 1, the library will have to perform the parallel work sequentially. On such mono-threaded libraries, the p parameter effectively becomes a multiplier on iterations (and possibly a divider on memory). I think this is why the libsodium library locks the p parameter to 1.

Given that WAMP-SCRAM requires the password hash to be computed by the client, I think it's reasonable to restrict the Argon2 parallellization parameter to 1 for the WAMP-SCRAM authentication method. We can explain this rationale in the spec so that security-savvy folks aren't scratching their heads about why the parallelization parameter is not tunable.

@ecorm
Copy link
Contributor

ecorm commented Mar 10, 2018

Something we need to achieve consensus on, is if we want to stick with the SASLprep username/password normalization algorithm mandated by the SCRAM spec, or just go with simple NFKC normalization, which is supported natively in browser-land Javascript via String.prototype.normalize().

SASLprep goes further than NFKC by dealing with various space characters, control characters, private characters, etc.

I did find the reklatsmasters/saslprep Javascript library (MIT license), and the author confirmed that it works on modern browsers.

For those who are unfamiliar with Unicode normalization, it's to deal with multiple code points that are visually rendered the same way. For example, there are code points for accented letters and there are also accent code points that combine with letter code points to be rendered as an accented character. Without any normalization, it would be possible for an imposter to choose a username that appears identical to another user, yet the unicode bytes are different and the system treats it as a different user.

@ecorm
Copy link
Contributor

ecorm commented Mar 10, 2018

Can I get a thumb's up/down to lock the Argon2 parallelization parameter to 1? The rationale to lock at 1 is due to the hash being computed client-side. See my comment above for more details.

@ecorm
Copy link
Contributor

ecorm commented Mar 10, 2018

Can I get a thumb's up to keep "saslprep" string normalization, as mandated by the SCRAM RFC, or thumb's down to use simple NFKC normalization? See my comment above for more details.

@oberstet
Copy link
Member

requiring saslprep seems safer, and implementation then may or may not catch up with this tighter set -- I seem to remember there is even a slight gap in the Python Autobahn libs rgd treatment of regular expressions for URIs .. Perl having it right;) practically, I think these are really obscure, exotic cases - I don't care much as long as the implementations don't get fuzzed when being exposed to all the flavors. Eg technically requiring UTF8 encoding totally makes sense (there is a state machine to check exactly), but checking for some specific form of Unicode normalization seems crazy to me. Until the Unicode consortium can come up with a well defined, formally complete algorithm to check for some form, it is academic ...

@ecorm ecorm mentioned this issue Mar 13, 2018
@ecorm
Copy link
Contributor

ecorm commented Mar 31, 2018

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

6 participants