esl / escalus

An XMPP client library in Erlang for conveniently testing XMPP servers
Apache License 2.0
129 stars 76 forks source link

Scram -PLUS variants #227

Closed NelsonVides closed 4 years ago

NelsonVides commented 4 years ago

I made some mess with Github's interface, so anyway, here's the PR again, this time organized and code refactored 🎉

Following the instructions from @michalwski in https://github.com/esl/escalus/pull/223#issuecomment-604364171, I tested with MetronomeIM as follows:

()1> Spec =
        [{username, <<"escalus_vides">>},
         {server, <<"lightwitch.org">>},
         {host, <<"meaveen.lightwitch.org">>},
         {resource, <<"res1">>},
         {password, <<"THIS_IS_NOT_MY_REAL_PASSWORD">>},
         {carbons, false},
         {stream_management, false},
         {starttls, required},
         {auth, {escalus_auth, auth_sasl_scram_sha256_plus}}].
()2> {ok, Client, Features} = escalus_connection:start(Specs).

And connected successfully.

The disclaimers: note that this works only with fast_tls as the underlying TLS driver, as OTP's TLS doesn't have as of now an API to get the underlying message. In fact, fast_tls doesn't support it either, the code to do so is on a dev branch in my fork, for which a PR to the main repo is going as well; and this works only with OpenSSL, as other native SSL drivers like BoringSSL or LibreSSL have different API's to get this data that I have not explored.

It is worth reviewing the changes commit by commit, as I've tried to present them in a granular way that could be easily edited or reverted, and with commit messages explaining what each of them is doing.

When it comes to that other short-lived PR, https://github.com/esl/escalus/pull/225, I've decided together with @janciesla8818 that escalus shouldn't decide this, as escalus is only a dumb client for testing. The default should be plain, to reduce load in amoc and improve logs in testing, as there's no hashing, and if anything else is needed, it should be explicitly selected for the user in it's configuration by specifying it for example as {escalus_auth_method, <<"SCRAM-SHA-256-PLUS">>} or {auth, {escalus_auth, auth_sasl_scram_sha256_plus}}, given a desired authentication method.

Another change that might seem unrelated is a specific commit making more symmetric the definition and implementation of the escalus_connection behaviour across the modules that implement it, that's why I mention checking commits more specifically.

Moving forward with https://github.com/esl/MongooseIM/issues/2442 @Neustradamus 😉