Skip to content

Commit

Permalink
Add clientChallenge SASL mechanism method
Browse files Browse the repository at this point in the history 
This is to prevent the `onChallenge` method from being monkey patched for the
SCRAM-SHA1 auth mechanism, which causes issues when reconnecting.

An alternative fix would be to call `registerSASLMechanisms` in the
`reset` method to undo the patch, but I've opted for this approach since
the patch itself is a code smell.
  • Loading branch information
@jcbrand
jcbrand committed Mar 16, 2021
1 parent 918b65e commit ae0131a
Show file tree
Hide file tree
Showing 5 changed files with 84 additions and 62 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@
## Version 1.4.2 - (Unreleased)

* Update optional NodeJS-specific dependency xmldom to version 0.5.0 which includes a security fix.
* Add `clientChallenge` SASL mechanism method to avoid having to monkey patch `onChallenge`,
which prevents reconnection when using SCRAM-SHA1.

## Version 1.4.1 - (2020-12-02)

Expand Down
2 changes: 1 addition & 1 deletion src/core.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2704,7 +2704,7 @@ Strophe.Connection = class Connection {
'mechanism': this._sasl_mechanism.mechname
});
if (this._sasl_mechanism.isClientFirst) {
const response = this._sasl_mechanism.onChallenge(this, null);
const response = this._sasl_mechanism.clientChallenge(this);
request_auth_exchange.t(btoa(response));
}
this.send(request_auth_exchange.tree());
Expand Down
114 changes: 57 additions & 57 deletions src/sasl-sha1.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,71 +17,71 @@ export default class SASLSHA1 extends SASLMechanism {
return connection.authcid !== null;
}

onChallenge (connection, challenge, test_cnonce) {
const cnonce = test_cnonce || MD5.hexdigest("" + (Math.random() * 1234567890));
let auth_str = "n=" + utils.utf16to8(connection.authcid);
auth_str += ",r=";
auth_str += cnonce;
connection._sasl_data.cnonce = cnonce;
connection._sasl_data["client-first-message-bare"] = auth_str;
auth_str = "n,," + auth_str;
onChallenge (connection, challenge) { // eslint-disable-line class-methods-use-this
let nonce, salt, iter, Hi, U, U_old, i, k;
let responseText = "c=biws,";
let authMessage = `${connection._sasl_data["client-first-message-bare"]},${challenge},`;
const cnonce = connection._sasl_data.cnonce;
const attribMatch = /([a-z]+)=([^,]+)(,|$)/;

this.onChallenge = (connection, challenge) => {
let nonce, salt, iter, Hi, U, U_old, i, k;
let responseText = "c=biws,";
let authMessage = `${connection._sasl_data["client-first-message-bare"]},${challenge},`;
const cnonce = connection._sasl_data.cnonce;
const attribMatch = /([a-z]+)=([^,]+)(,|$)/;
while (challenge.match(attribMatch)) {
const matches = challenge.match(attribMatch);
challenge = challenge.replace(matches[0], "");
switch (matches[1]) {
case "r":
nonce = matches[2];
break;
case "s":
salt = matches[2];
break;
case "i":
iter = matches[2];
break;
}
}

while (challenge.match(attribMatch)) {
const matches = challenge.match(attribMatch);
challenge = challenge.replace(matches[0], "");
switch (matches[1]) {
case "r":
nonce = matches[2];
break;
case "s":
salt = matches[2];
break;
case "i":
iter = matches[2];
break;
}
}
if (nonce.substr(0, cnonce.length) !== cnonce) {
connection._sasl_data = {};
return connection._sasl_failure_cb();
}

if (nonce.substr(0, cnonce.length) !== cnonce) {
connection._sasl_data = {};
return connection._sasl_failure_cb();
}
responseText += "r=" + nonce;
authMessage += responseText;

responseText += "r=" + nonce;
authMessage += responseText;
salt = atob(salt);
salt += "\x00\x00\x00\x01";

salt = atob(salt);
salt += "\x00\x00\x00\x01";
const pass = utils.utf16to8(connection.pass);
Hi = U_old = SHA1.core_hmac_sha1(pass, salt);
for (i=1; i<iter; i++) {
U = SHA1.core_hmac_sha1(pass, SHA1.binb2str(U_old));
for (k = 0; k < 5; k++) {
Hi[k] ^= U[k];
}
U_old = U;
}
Hi = SHA1.binb2str(Hi);

const pass = utils.utf16to8(connection.pass);
Hi = U_old = SHA1.core_hmac_sha1(pass, salt);
for (i=1; i<iter; i++) {
U = SHA1.core_hmac_sha1(pass, SHA1.binb2str(U_old));
for (k = 0; k < 5; k++) {
Hi[k] ^= U[k];
}
U_old = U;
}
Hi = SHA1.binb2str(Hi);
const clientKey = SHA1.core_hmac_sha1(Hi, "Client Key");
const serverKey = SHA1.str_hmac_sha1(Hi, "Server Key");
const clientSignature = SHA1.core_hmac_sha1(SHA1.str_sha1(SHA1.binb2str(clientKey)), authMessage);
connection._sasl_data["server-signature"] = SHA1.b64_hmac_sha1(serverKey, authMessage);

const clientKey = SHA1.core_hmac_sha1(Hi, "Client Key");
const serverKey = SHA1.str_hmac_sha1(Hi, "Server Key");
const clientSignature = SHA1.core_hmac_sha1(SHA1.str_sha1(SHA1.binb2str(clientKey)), authMessage);
connection._sasl_data["server-signature"] = SHA1.b64_hmac_sha1(serverKey, authMessage);
for (k = 0; k < 5; k++) {
clientKey[k] ^= clientSignature[k];
}
responseText += ",p=" + btoa(SHA1.binb2str(clientKey));
return responseText;
}

for (k = 0; k < 5; k++) {
clientKey[k] ^= clientSignature[k];
}
responseText += ",p=" + btoa(SHA1.binb2str(clientKey));
return responseText;
}
clientChallenge (connection, test_cnonce) { // eslint-disable-line class-methods-use-this
const cnonce = test_cnonce || MD5.hexdigest("" + (Math.random() * 1234567890));
let auth_str = "n=" + utils.utf16to8(connection.authcid);
auth_str += ",r=";
auth_str += cnonce;
connection._sasl_data.cnonce = cnonce;
connection._sasl_data["client-first-message-bare"] = auth_str;
auth_str = "n,," + auth_str;
return auth_str;
}
}
24 changes: 22 additions & 2 deletions src/sasl.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -89,8 +89,11 @@ export default class SASLMechanism {
}

/** PrivateFunction: onChallenge
* Called by protocol implementation on incoming challenge. If client is
* first (isClientFirst === true) challenge will be null on the first call.
* Called by protocol implementation on incoming challenge.
*
* By deafult, if the client is expected to send data first (isClientFirst === true),
* this method is called with `challenge` as null on the first call,
* unless `clientChallenge` is overridden in the relevant subclass.
*
* Parameters:
* (Strophe.Connection) connection - Target Connection.
Expand All @@ -103,6 +106,23 @@ export default class SASLMechanism {
throw new Error("You should implement challenge handling!");
}

/** PrivateFunction: clientChallenge
* Called by the protocol implementation if the client is expected to send
* data first in the authentication exchange (i.e. isClientFirst === true).
*
* Parameters:
* (Strophe.Connection) connection - Target Connection.
*
* Returns:
* (String) Mechanism response.
*/
clientChallenge (connection) {
if (!this.isClientFirst) {
throw new Error("clientChallenge should not be called if isClientFirst is false!");
}
return this.onChallenge(connection);
}

/** PrivateFunction: onFailure
* Protocol informs mechanism implementation about SASL failure.
*/
Expand Down
4 changes: 2 additions & 2 deletions tests/tests.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -670,7 +670,7 @@ define([
ok(saslsha1.test(conn), "SHA-1 is enabled by default.");
// test taken from example section on:
// URL: http://tools.ietf.org/html/rfc5802#section-5
let response = saslsha1.onChallenge(conn, null, "fyko+d2lbbFgONRv9qkxdawL");
let response = saslsha1.clientChallenge(conn, "fyko+d2lbbFgONRv9qkxdawL");
equal(response, "n,,n=user,r=fyko+d2lbbFgONRv9qkxdawL", "checking first auth challenge");

response = saslsha1.onChallenge(conn, "r=fyko+d2lbbFgONRv9qkxdawL3rfcNHYJY1ZVvWVs7j,s=QSXCR+Q6sek8bf92,i=4096");
Expand All @@ -685,7 +685,7 @@ define([
ok(sasl_external.test(conn), "EXTERNAL is enabled by default.");
sasl_external.onStart(conn);

let response = sasl_external.onChallenge(conn, null);
let response = sasl_external.clientChallenge(conn);
equal(response, conn.authzid,
"Response to EXTERNAL auth challenge should be authzid if different authcid was passed in.");
sasl_external.onSuccess();
Expand Down

0 comments on commit ae0131a

Please sign in to comment.