On Wed, 2012-10-03 at 02:10 +0200, Florian Zeitz wrote:
> Attached is a new export incorporating your feedback.
Committed. Also what do you think about the attached patch? (Compiles,
untested.)
diff -r 3e3ac2c16fa4 src/auth/mech-scram-sha1.c
--- a/src/auth/mech-scram-sha1.c Wed Sep 19 03:13:39 2012 +0200
+++ b/src/auth/mech-scram-sha1.c Wed Oct 03 03:48:46 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 3e3ac2c16fa4 src/auth/password-scheme-scram.c
--- a/src/auth/password-scheme-scram.c Wed Sep 19 03:13:39 2012 +0200
+++ b/src/auth/password-scheme-scram.c Wed Oct 03 03:48:46 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 3e3ac2c16fa4 src/auth/password-scheme.h
--- a/src/auth/password-scheme.h Wed Sep 19 03:13:39 2012 +0200
+++ b/src/auth/password-scheme.h Wed Oct 03 03:48:46 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);
