dovecot-2.2: auth: More error checking and cleanups to SCRAM-SHA-1.

dovecot at dovecot.org dovecot at dovecot.org
Wed Oct 3 16:57:43 EEST 2012


details:   http://hg.dovecot.org/dovecot-2.2/rev/559718e321a2
changeset: 15186:559718e321a2
user:      Timo Sirainen <tss at iki.fi>
date:      Wed Oct 03 16:57:28 2012 +0300
description:
auth: More error checking and cleanups to SCRAM-SHA-1.

diffstat:

 src/auth/mech-scram-sha1.c       |  65 ++++++-------------------------
 src/auth/password-scheme-scram.c |  81 +++++++++++++++++++++++++++++++--------
 src/auth/password-scheme.h       |   4 +
 3 files changed, 81 insertions(+), 69 deletions(-)

diffs (278 lines):

diff -r 62a14c9ae6c4 -r 559718e321a2 src/auth/mech-scram-sha1.c
--- a/src/auth/mech-scram-sha1.c	Wed Oct 03 05:15:11 2012 +0300
+++ b/src/auth/mech-scram-sha1.c	Wed Oct 03 16:57:28 2012 +0300
@@ -19,10 +19,9 @@
 #include "str.h"
 #include "strfuncs.h"
 #include "strnum.h"
+#include "password-scheme.h"
 #include "mech.h"
 
-/* SCRAM hash iteration count. RFC says it SHOULD be at least 4096 */
-#define SCRAM_ITERATE_COUNT 4096
 /* s-nonce length */
 #define SCRAM_SERVER_NONCE_LEN 64
 
@@ -43,8 +42,8 @@
 	buffer_t *proof;
 
 	/* stored */
-	buffer_t *stored_key;
-	buffer_t *server_key;
+	unsigned char stored_key[SHA1_RESULTLEN];
+	unsigned char server_key[SHA1_RESULTLEN];
 };
 
 static const char *get_scram_server_first(struct scram_auth_request *request,
@@ -75,7 +74,6 @@
 {
 	struct hmac_context ctx;
 	const char *auth_message;
-	unsigned char server_key[SHA1_RESULTLEN];
 	unsigned char server_signature[SHA1_RESULTLEN];
 	string_t *str;
 
@@ -83,7 +81,7 @@
 			request->server_first_message, ",",
 			request->client_final_message_without_proof, NULL);
 
-	hmac_init(&ctx, request->server_key->data, request->server_key->used,
+	hmac_init(&ctx, request->server_key, sizeof(request->server_key),
 		  &hash_method_sha1);
 	hmac_update(&ctx, auth_message, strlen(auth_message));
 	hmac_final(&ctx, server_signature);
@@ -195,7 +193,7 @@
 			request->server_first_message, ",",
 			request->client_final_message_without_proof, NULL);
 
-	hmac_init(&ctx, request->stored_key->data, request->stored_key->used,
+	hmac_init(&ctx, request->stored_key, sizeof(request->stored_key),
 		  &hash_method_sha1);
 	hmac_update(&ctx, auth_message, strlen(auth_message));
 	hmac_final(&ctx, client_signature);
@@ -209,68 +207,31 @@
 	safe_memset(client_key, 0, sizeof(client_key));
 	safe_memset(client_signature, 0, sizeof(client_signature));
 
-	return memcmp(stored_key, request->stored_key->data,
-		      request->stored_key->used) == 0;
+	return memcmp(stored_key, request->stored_key, sizeof(stored_key)) == 0;
 }
 
 static void credentials_callback(enum passdb_result result,
 				 const unsigned char *credentials, size_t size,
 				 struct auth_request *auth_request)
 {
-	const char *const *fields;
-	size_t len;
-	unsigned int iter;
-	const char *salt;
 	struct scram_auth_request *request =
 		(struct scram_auth_request *)auth_request;
+	const char *salt, *error;
+	unsigned int iter_count;
 
 	switch (result) {
 	case PASSDB_RESULT_OK:
-		fields = t_strsplit(t_strndup(credentials, size), ",");
-
-		if (str_array_length(fields) != 4) {
+		if (scram_sha1_scheme_parse(credentials, size, &iter_count,
+					    &salt, request->stored_key,
+					    request->server_key, &error) < 0) {
 			auth_request_log_info(auth_request, "scram-sha-1",
-					      "Invalid passdb entry");
-			auth_request_fail(auth_request);
-			break;
-		}
-
-		if (str_to_uint(fields[0], &iter) < 0 || (iter < 4096) ||
-		    (iter > INT_MAX)) {
-			auth_request_log_info(auth_request, "scram-sha-1",
-					      "Invalid iteration count");
-			auth_request_fail(auth_request);
-			break;
-		}
-
-		salt = fields[1];
-
-		len = strlen(fields[2]);
-		request->stored_key = buffer_create_dynamic(request->pool,
-					MAX_BASE64_DECODED_SIZE(len));
-		if (base64_decode(fields[2], len, NULL,
-				  request->stored_key) < 0) {
-			auth_request_log_info(auth_request, "scram-sha-1",
-					      "Invalid base64 encoding"
-					      "of StoredKey in passdb");
-			auth_request_fail(auth_request);
-			break;
-		}
-
-		len = strlen(fields[3]);
-		request->server_key = buffer_create_dynamic(request->pool,
-					MAX_BASE64_DECODED_SIZE(len));
-		if (base64_decode(fields[3], len, NULL,
-				  request->server_key) < 0) {
-			auth_request_log_info(auth_request, "scram-sha-1",
-					      "Invalid base64 encoding"
-					      "of ServerKey in passdb");
+					      "%s", error);
 			auth_request_fail(auth_request);
 			break;
 		}
 
 		request->server_first_message = p_strdup(request->pool,
-			get_scram_server_first(request, iter, salt));
+			get_scram_server_first(request, iter_count, salt));
 
 		auth_request_handler_reply_continue(auth_request,
 					request->server_first_message,
diff -r 62a14c9ae6c4 -r 559718e321a2 src/auth/password-scheme-scram.c
--- a/src/auth/password-scheme-scram.c	Wed Oct 03 05:15:11 2012 +0300
+++ b/src/auth/password-scheme-scram.c	Wed Oct 03 16:57:28 2012 +0300
@@ -18,8 +18,11 @@
 #include "str.h"
 #include "password-scheme.h"
 
-/* SCRAM hash iteration count. RFC says it SHOULD be at least 4096 */
-#define SCRAM_ITERATE_COUNT 4096
+/* SCRAM allowed iteration count range. RFC says it SHOULD be at least 4096 */
+#define SCRAM_MIN_ITERATE_COUNT 4096
+#define SCRAM_MAX_ITERATE_COUNT INT_MAX
+
+#define SCRAM_DEFAULT_ITERATE_COUNT 4096
 
 static void Hi(const unsigned char *str, size_t str_size,
 	       const unsigned char *salt, size_t salt_size, unsigned int i,
@@ -47,30 +50,73 @@
 	}
 }
 
-/* password string format: iter,salt,stored_key,server_key */
+int scram_sha1_scheme_parse(const unsigned char *credentials, size_t size,
+			    unsigned int *iter_count_r, const char **salt_r,
+			    unsigned char stored_key_r[],
+			    unsigned char server_key_r[], const char **error_r)
+{
+	const char *const *fields;
+	buffer_t *buf;
+
+	/* password string format: iter,salt,stored_key,server_key */
+	fields = t_strsplit(t_strndup(credentials, size), ",");
+
+	if (str_array_length(fields) != 4) {
+		*error_r = "Invalid SCRAM-SHA-1 passdb entry format";
+		return -1;
+	}
+	if (str_to_uint(fields[0], iter_count_r) < 0 ||
+	    *iter_count_r < SCRAM_MIN_ITERATE_COUNT ||
+	    *iter_count_r > SCRAM_MAX_ITERATE_COUNT) {
+		*error_r = "Invalid SCRAM-SHA-1 iteration count in passdb";
+		return -1;
+	}
+	*salt_r = fields[1];
+
+	buf = buffer_create_dynamic(pool_datastack_create(), SHA1_RESULTLEN);
+	if (base64_decode(fields[2], strlen(fields[2]), NULL, buf) < 0 ||
+	    buf->used != SHA1_RESULTLEN) {
+		*error_r = "Invalid SCRAM-SHA-1 StoredKey in passdb";
+		return -1;
+	}
+	memcpy(stored_key_r, buf->data, SHA1_RESULTLEN);
+
+	buffer_set_used_size(buf, 0);
+	if (base64_decode(fields[3], strlen(fields[3]), NULL, buf) < 0 ||
+	    buf->used != SHA1_RESULTLEN) {
+		*error_r = "Invalid SCRAM-SHA-1 ServerKey in passdb";
+		return -1;
+	}
+	memcpy(server_key_r, buf->data, SHA1_RESULTLEN);
+	return 0;
+}
 
 int scram_sha1_verify(const char *plaintext, const char *user ATTR_UNUSED,
 		      const unsigned char *raw_password, size_t size,
-		      const char **error_r ATTR_UNUSED)
+		      const char **error_r)
 {
 	struct hmac_context ctx;
-	string_t *str;
-	const char *const *fields;
-	int iter;
+	const char *salt_base64;
+	unsigned int iter_count;
 	const unsigned char *salt;
 	size_t salt_len;
 	unsigned char salted_password[SHA1_RESULTLEN];
 	unsigned char client_key[SHA1_RESULTLEN];
 	unsigned char stored_key[SHA1_RESULTLEN];
+	unsigned char calculated_stored_key[SHA1_RESULTLEN];
+	unsigned char server_key[SHA1_RESULTLEN];
+	int ret;
 
-	fields = t_strsplit(t_strndup(raw_password, size), ",");
-	iter = atoi(fields[0]);
-	salt = buffer_get_data(t_base64_decode_str(fields[1]), &salt_len);
-	str = t_str_new(strlen(fields[2]));
+	if (scram_sha1_scheme_parse(raw_password, size, &iter_count,
+				    &salt_base64, stored_key,
+				    server_key, error_r) < 0)
+		return -1;
+
+	salt = buffer_get_data(t_base64_decode_str(salt_base64), &salt_len);
 
 	/* FIXME: credentials should be SASLprepped UTF8 data here */
 	Hi((const unsigned char *)plaintext, strlen(plaintext), salt, salt_len,
-	   iter, salted_password);
+	   iter_count, salted_password);
 
 	/* Calculate ClientKey */
 	hmac_init(&ctx, salted_password, sizeof(salted_password),
@@ -79,14 +125,15 @@
 	hmac_final(&ctx, client_key);
 
 	/* Calculate StoredKey */
-	sha1_get_digest(client_key, sizeof(client_key), stored_key);
-	base64_encode(stored_key, sizeof(stored_key), str);
+	sha1_get_digest(client_key, sizeof(client_key), calculated_stored_key);
+	ret = memcmp(stored_key, calculated_stored_key,
+		     sizeof(stored_key)) == 0 ? 1 : 0;
 
 	safe_memset(salted_password, 0, sizeof(salted_password));
 	safe_memset(client_key, 0, sizeof(client_key));
 	safe_memset(stored_key, 0, sizeof(stored_key));
 
-	return strcmp(fields[2], str_c(str)) == 0 ? 1 : 0;
+	return ret;
 }
 
 void scram_sha1_generate(const char *plaintext, const char *user ATTR_UNUSED,
@@ -103,12 +150,12 @@
 	random_fill(salt, sizeof(salt));
 
 	str = t_str_new(MAX_BASE64_ENCODED_SIZE(sizeof(salt)));
-	str_printfa(str, "%i,", SCRAM_ITERATE_COUNT);
+	str_printfa(str, "%d,", SCRAM_DEFAULT_ITERATE_COUNT);
 	base64_encode(salt, sizeof(salt), str);
 
 	/* FIXME: credentials should be SASLprepped UTF8 data here */
 	Hi((const unsigned char *)plaintext, strlen(plaintext), salt,
-	   sizeof(salt), SCRAM_ITERATE_COUNT, salted_password);
+	   sizeof(salt), SCRAM_DEFAULT_ITERATE_COUNT, salted_password);
 
 	/* Calculate ClientKey */
 	hmac_init(&ctx, salted_password, sizeof(salted_password),
diff -r 62a14c9ae6c4 -r 559718e321a2 src/auth/password-scheme.h
--- a/src/auth/password-scheme.h	Wed Oct 03 05:15:11 2012 +0300
+++ b/src/auth/password-scheme.h	Wed Oct 03 16:57:28 2012 +0300
@@ -85,6 +85,10 @@
 		 const unsigned char *raw_password, size_t size,
 		 const char **error_r);
 
+int scram_sha1_scheme_parse(const unsigned char *credentials, size_t size,
+			    unsigned int *iter_count_r, const char **salt_r,
+			    unsigned char stored_key_r[],
+			    unsigned char server_key_r[], const char **error_r);
 int scram_sha1_verify(const char *plaintext, const char *user ATTR_UNUSED,
 		      const unsigned char *raw_password, size_t size,
 		      const char **error_r ATTR_UNUSED);


More information about the dovecot-cvs mailing list