Fix SASL SCRAM implementation

* Add SCRAM_DIGEST_SIZE macro for maximum possible digest size. It will
  avoid new buffer overflow errors when new digests are added.
* Fix buffer overflow in sasl_scram(). Buffers were allocated for SHA1
  digest size.
* Fix bug with handler re-registration when a SASL SCRAM mechanism
  fails.

src/auth.c

@@ -82,9 +82,9 @@ static int _handle_digestmd5_challenge(xmpp_conn_t *const conn,
 static int _handle_digestmd5_rspauth(xmpp_conn_t *const conn,
                                      xmpp_stanza_t *const stanza,
                                      void *const userdata);
-static int _handle_scram_sha1_challenge(xmpp_conn_t *const conn,
+static int _handle_scram_challenge(xmpp_conn_t *const conn,
-                                        xmpp_stanza_t *const stanza,
+                                   xmpp_stanza_t *const stanza,
-                                        void *const userdata);
+                                   void *const userdata);
 static char *_make_scram_init_msg(xmpp_conn_t *const conn);
 
 static int _handle_missing_features_sasl(xmpp_conn_t *const conn,
@@ -440,18 +440,18 @@ struct scram_user_data {
 };
 
 /* handle the challenge phase of SCRAM-SHA-1 auth */
-static int _handle_scram_sha1_challenge(xmpp_conn_t *const conn,
+static int _handle_scram_challenge(xmpp_conn_t *const conn,
-                                        xmpp_stanza_t *const stanza,
+                                   xmpp_stanza_t *const stanza,
-                                        void *const userdata)
+                                   void *const userdata)
 {
     char *text;
     char *response;
     xmpp_stanza_t *auth;
     xmpp_stanza_t *authdata;
     const char *name;
-    const char *scram_name;
     char *challenge;
     struct scram_user_data *scram_ctx = (struct scram_user_data *)userdata;
+    int rc;
 
     name = xmpp_stanza_get_name(stanza);
     xmpp_debug(conn->ctx, "xmpp", "handle %s (challenge) called for %s",
@@ -491,14 +491,24 @@ static int _handle_scram_sha1_challenge(xmpp_conn_t *const conn,
         xmpp_send(conn, auth);
         xmpp_stanza_release(auth);
 
+        rc = 1; /* Keep handler */
     } else {
-        scram_name = scram_ctx->alg->scram_name;
+        /*
+         * Free scram_ctx after calling _handle_sasl_result(). If authentication
+         * fails, we want to try other mechanism which may be different SCRAM
+         * mechanism. If we freed scram_ctx before the function, _auth() would
+         * be able to allocate new scram_ctx object with the same address and
+         * handler_add() would consider new SCRAM handler as duplicate, because
+         * current handler is not removed yet. As result, libstrophe wouldn't
+         * handle incoming challenge stanza.
+         */
+        rc = _handle_sasl_result(conn, stanza,
+                                 (void *)scram_ctx->alg->scram_name);
         xmpp_free(conn->ctx, scram_ctx->scram_init);
         xmpp_free(conn->ctx, scram_ctx);
-        return _handle_sasl_result(conn, stanza, (void *)scram_name);
     }
 
-    return 1;
+    return rc;
 
 err_release_auth:
     xmpp_stanza_release(auth);
@@ -694,8 +704,8 @@ static void _auth(xmpp_conn_t *const conn)
         xmpp_stanza_add_child(auth, authdata);
         xmpp_stanza_release(authdata);
 
-        handler_add(conn, _handle_scram_sha1_challenge, XMPP_NS_SASL, NULL,
+        handler_add(conn, _handle_scram_challenge, XMPP_NS_SASL, NULL, NULL,
-                    NULL, (void *)scram_ctx);
+                    (void *)scram_ctx);
 
         xmpp_send(conn, auth);
         xmpp_stanza_release(auth);

src/sasl.c

@@ -21,7 +21,6 @@
 #include "ostypes.h"
 #include "sasl.h"
 #include "md5.h"
-#include "sha1.h"
 #include "scram.h"
 #include "rand.h"
 #include "util.h"
@@ -379,7 +378,7 @@ char *sasl_digest_md5(xmpp_ctx_t *ctx,
     return response;
 }
 
-/** generate auth response string for the SASL SCRAM-SHA-1 mechanism */
+/** generate auth response string for the SASL SCRAM mechanism */
 char *sasl_scram(xmpp_ctx_t *ctx,
                  const struct hash_alg *alg,
                  const char *challenge,
@@ -387,8 +386,8 @@ char *sasl_scram(xmpp_ctx_t *ctx,
                  const char *jid,
                  const char *password)
 {
-    uint8_t key[SHA1_DIGEST_SIZE];
+    uint8_t key[SCRAM_DIGEST_SIZE];
-    uint8_t sign[SHA1_DIGEST_SIZE];
+    uint8_t sign[SCRAM_DIGEST_SIZE];
     char *r = NULL;
     char *s = NULL;
     char *i = NULL;
@@ -439,7 +438,8 @@ char *sasl_scram(xmpp_ctx_t *ctx,
         goto out_sval;
     }
 
-    response_len = 39 + strlen(r);
+    /* "c=biws," + r + ",p=" + sign_b64 + '\0' */
+    response_len = 7 + strlen(r) + 3 + ((alg->digest_size + 2) / 3 * 4) + 1;
     response = xmpp_alloc(ctx, response_len);
     if (!response) {
         goto out_auth;
@@ -454,11 +454,12 @@ char *sasl_scram(xmpp_ctx_t *ctx,
     SCRAM_ClientSignature(alg, key, (uint8_t *)auth, strlen(auth), sign);
     SCRAM_ClientProof(alg, sign, key, sign);
 
-    sign_b64 = xmpp_base64_encode(ctx, sign, sizeof(sign));
+    sign_b64 = xmpp_base64_encode(ctx, sign, alg->digest_size);
     if (!sign_b64) {
         goto out_response;
     }
 
+    /* Check for buffer overflow */
     if (strlen(response) + strlen(sign_b64) + 3 + 1 > response_len) {
         xmpp_free(ctx, sign_b64);
         goto out_response;

src/scram.c

@@ -75,7 +75,7 @@ static void crypto_HMAC(const struct hash_alg *alg,
     uint8_t key_pad[HMAC_BLOCK_SIZE];
     uint8_t key_ipad[HMAC_BLOCK_SIZE];
     uint8_t key_opad[HMAC_BLOCK_SIZE];
-    uint8_t sha_digest[SHA512_DIGEST_SIZE];
+    uint8_t sha_digest[SCRAM_DIGEST_SIZE];
     int i;
     union common_hash_ctx ctx;
 
@@ -148,7 +148,7 @@ void SCRAM_ClientKey(const struct hash_alg *alg,
                      uint32_t i,
                      uint8_t *key)
 {
-    uint8_t salted[SHA512_DIGEST_SIZE];
+    uint8_t salted[SCRAM_DIGEST_SIZE];
 
     /* XXX: Normalize(password) is omitted */
 
@@ -163,7 +163,7 @@ void SCRAM_ClientSignature(const struct hash_alg *alg,
                            size_t len,
                            uint8_t *sign)
 {
-    uint8_t stored[SHA512_DIGEST_SIZE];
+    uint8_t stored[SCRAM_DIGEST_SIZE];
 
     alg->hash(ClientKey, alg->digest_size, stored);
     crypto_HMAC(alg, stored, alg->digest_size, AuthMessage, len, sign);

src/scram.h

@@ -1,5 +1,5 @@
 /* scram.h
- * strophe XMPP client library -- SCRAM-SHA1 helper functions
+ * strophe XMPP client library -- SCRAM helper functions
  *
  * Copyright (C) 2013 Dmitry Podgorny <pasis.ua@gmail.com>
  *
@@ -10,7 +10,7 @@
  */
 
 /** @file
- *  SCRAM-SHA1 helper functions.
+ *  SCRAM helper functions.
  */
 
 #ifndef __LIBSTROPHE_SCRAM_H__
@@ -19,6 +19,10 @@
 /* make sure the stdint.h types are available */
 #include "ostypes.h"
 
+/* Maximum possible digest size. Used for buffers allocation. */
+#include "sha512.h"
+#define SCRAM_DIGEST_SIZE SHA512_DIGEST_SIZE
+
 struct hash_alg {
     const char *scram_name;
     int mask;