[RFC PATCH 0/4] iscsi: chap: introduce support for SHA1 and SHA3-256

[RFC PATCH 0/4] iscsi: chap: introduce support for SHA1 and SHA3-256

[Maurizio Lombardi]

iSCSI with the Challenge-Handshake Authentication Protocol is not FIPS compliant.
This is due to the fact that CHAP currently uses MD5 as the only supported
digest algorithm and MD5 is not allowed by FIPS.

When FIPS mode is enabled on the target server, the CHAP authentication
won't work because the target driver will be prevented from using the MD5 module.

Given that CHAP is agnostic regarding the algorithm it uses, this
patchset introduce support for two new alternatives: SHA1 and SHA3-256.

SHA1 has already its own assigned value for its use in CHAP, as reported by IANA:
https://www.iana.org/assignments/ppp-numbers/ppp-numbers.xml#ppp-numbers-9
On the other hand the use of SHA1 on FIPS-enabled systems has been deprecated
and therefore it's not a vialable long term option.

We could consider introducing a more modern hash algorithm like SHA3-256, as
this patchset does.

A pull request for the open-iscsi initiator side implementation has been
submitted here:
https://github.com/open-iscsi/open-iscsi/pull/170

Maurizio Lombardi (4):
  target-iscsi: CHAP: add support to SHA1 and SHA3-256 hash functions
  target-iscsi: remove unneeded function
  target-iscsi: tie the challenge length to the hash digest size
  target-iscsi: rename some variables to avoid confusion.

 drivers/target/iscsi/iscsi_target_auth.c | 218 ++++++++++++++---------
 drivers/target/iscsi/iscsi_target_auth.h |  13 +-
 2 files changed, 147 insertions(+), 84 deletions(-)

-- 
Maurizio Lombardi

[PATCH 1/4] target-iscsi: CHAP: add support to SHA1 and SHA3-256 hash functions

[Maurizio Lombardi]

This patches modifies the chap_server_compute_hash() function
to make it agnostic to the choice of hash algorithm that is used.
It also adds support to two new hash algorithms: SHA1 and SHA3-256

Signed-off-by: Maurizio Lombardi <mlombard@redhat.com>
---
 drivers/target/iscsi/iscsi_target_auth.c | 135 +++++++++++++++++------
 drivers/target/iscsi/iscsi_target_auth.h |   9 +-
 2 files changed, 108 insertions(+), 36 deletions(-)

diff --git a/drivers/target/iscsi/iscsi_target_auth.c b/drivers/target/iscsi/iscsi_target_auth.c
index 51ddca2033e0..3d1e94333835 100644
--- a/drivers/target/iscsi/iscsi_target_auth.c
+++ b/drivers/target/iscsi/iscsi_target_auth.c
@@ -46,9 +46,22 @@ static int chap_gen_challenge(
 	return 0;
 }
 
+static int chap_test_algorithm(const char *name)
+{
+	struct crypto_shash *tfm;
+
+	tfm = crypto_alloc_shash(name, 0, 0);
+	if (IS_ERR(tfm))
+		return -1;
+
+	crypto_free_shash(tfm);
+	return 0;
+}
+
 static int chap_check_algorithm(const char *a_str)
 {
 	char *tmp, *orig, *token;
+	int r = CHAP_DIGEST_UNKNOWN;
 
 	tmp = kstrdup(a_str, GFP_KERNEL);
 	if (!tmp) {
@@ -72,13 +85,33 @@ static int chap_check_algorithm(const char *a_str)
 
 		if (!strncmp(token, "5", 1)) {
 			pr_debug("Selected MD5 Algorithm\n");
-			kfree(orig);
-			return CHAP_DIGEST_MD5;
+			if (chap_test_algorithm("md5") < 0) {
+				pr_err("failed to allocate md5 algo\n");
+				continue;
+			}
+			r = CHAP_DIGEST_MD5;
+			goto out;
+		} else if (!strncmp(token, "6", 1)) {
+			pr_debug("Selected SHA1 Algorithm\n");
+			if (chap_test_algorithm("sha1") < 0) {
+				pr_err("failed to allocate sha1 algo\n");
+				continue;
+			}
+			r = CHAP_DIGEST_SHA;
+			goto out;
+		} else if (!strncmp(token, "7", 1)) {
+			pr_debug("Selected SHA3-256 Algorithm\n");
+			if (chap_test_algorithm("sha3-256") < 0) {
+				pr_err("failed to allocate sha3-256 algo\n");
+				continue;
+			}
+			r = CHAP_DIGEST_SHA3_256;
+			goto out;
 		}
 	}
 out:
 	kfree(orig);
-	return CHAP_DIGEST_UNKNOWN;
+	return r;
 }
 
 static void chap_close(struct iscsi_conn *conn)
@@ -94,7 +127,7 @@ static struct iscsi_chap *chap_server_open(
 	char *aic_str,
 	unsigned int *aic_len)
 {
-	int ret;
+	int digest_type;
 	struct iscsi_chap *chap;
 
 	if (!(auth->naf_flags & NAF_USERID_SET) ||
@@ -109,17 +142,19 @@ static struct iscsi_chap *chap_server_open(
 		return NULL;
 
 	chap = conn->auth_protocol;
-	ret = chap_check_algorithm(a_str);
-	switch (ret) {
+	digest_type = chap_check_algorithm(a_str);
+	switch (digest_type) {
 	case CHAP_DIGEST_MD5:
-		pr_debug("[server] Got CHAP_A=5\n");
-		/*
-		 * Send back CHAP_A set to MD5.
-		*/
-		*aic_len = sprintf(aic_str, "CHAP_A=5");
-		*aic_len += 1;
-		chap->digest_type = CHAP_DIGEST_MD5;
-		pr_debug("[server] Sending CHAP_A=%d\n", chap->digest_type);
+		chap->digest_size = MD5_SIGNATURE_SIZE;
+		chap->digest_name = "md5";
+		break;
+	case CHAP_DIGEST_SHA:
+		chap->digest_size = SHA_SIGNATURE_SIZE;
+		chap->digest_name = "sha1";
+		break;
+	case CHAP_DIGEST_SHA3_256:
+		chap->digest_size = SHA3_256_SIGNATURE_SIZE;
+		chap->digest_name = "sha3-256";
 		break;
 	case CHAP_DIGEST_UNKNOWN:
 	default:
@@ -128,6 +163,11 @@ static struct iscsi_chap *chap_server_open(
 		return NULL;
 	}
 
+	pr_debug("[server] Got CHAP_A=%d\n", digest_type);
+	*aic_len = sprintf(aic_str, "CHAP_A=%d", digest_type);
+	*aic_len += 1;
+	pr_debug("[server] Sending CHAP_A=%d\n", digest_type);
+
 	/*
 	 * Set Identifier.
 	 */
@@ -146,7 +186,7 @@ static struct iscsi_chap *chap_server_open(
 	return chap;
 }
 
-static int chap_server_compute_md5(
+static int chap_server_compute_hash(
 	struct iscsi_conn *conn,
 	struct iscsi_node_auth *auth,
 	char *nr_in_ptr,
@@ -155,12 +195,13 @@ static int chap_server_compute_md5(
 {
 	unsigned long id;
 	unsigned char id_as_uchar;
-	unsigned char digest[MD5_SIGNATURE_SIZE];
-	unsigned char type, response[MD5_SIGNATURE_SIZE * 2 + 2];
+	unsigned char type;
 	unsigned char identifier[10], *challenge = NULL;
 	unsigned char *challenge_binhex = NULL;
-	unsigned char client_digest[MD5_SIGNATURE_SIZE];
-	unsigned char server_digest[MD5_SIGNATURE_SIZE];
+	unsigned char *digest = NULL;
+	unsigned char *response = NULL;
+	unsigned char *client_digest = NULL;
+	unsigned char *server_digest = NULL;
 	unsigned char chap_n[MAX_CHAP_N_SIZE], chap_r[MAX_RESPONSE_LENGTH];
 	size_t compare_len;
 	struct iscsi_chap *chap = conn->auth_protocol;
@@ -168,13 +209,33 @@ static int chap_server_compute_md5(
 	struct shash_desc *desc = NULL;
 	int auth_ret = -1, ret, challenge_len;
 
+	digest = kzalloc(chap->digest_size, GFP_KERNEL);
+	if (!digest) {
+		pr_err("Unable to allocate the digest buffer\n");
+		goto out;
+	}
+
+	response = kzalloc(chap->digest_size * 2 + 2, GFP_KERNEL);
+	if (!response) {
+		pr_err("Unable to allocate the response buffer\n");
+		goto out;
+	}
+
+	client_digest = kzalloc(chap->digest_size, GFP_KERNEL);
+	if (!client_digest) {
+		pr_err("Unable to allocate the client_digest buffer\n");
+		goto out;
+	}
+
+	server_digest = kzalloc(chap->digest_size, GFP_KERNEL);
+	if (!server_digest) {
+		pr_err("Unable to allocate the server_digest buffer\n");
+		goto out;
+	}
+
 	memset(identifier, 0, 10);
 	memset(chap_n, 0, MAX_CHAP_N_SIZE);
 	memset(chap_r, 0, MAX_RESPONSE_LENGTH);
-	memset(digest, 0, MD5_SIGNATURE_SIZE);
-	memset(response, 0, MD5_SIGNATURE_SIZE * 2 + 2);
-	memset(client_digest, 0, MD5_SIGNATURE_SIZE);
-	memset(server_digest, 0, MD5_SIGNATURE_SIZE);
 
 	challenge = kzalloc(CHAP_CHALLENGE_STR_LEN, GFP_KERNEL);
 	if (!challenge) {
@@ -219,18 +280,18 @@ static int chap_server_compute_md5(
 		pr_err("Could not find CHAP_R.\n");
 		goto out;
 	}
-	if (strlen(chap_r) != MD5_SIGNATURE_SIZE * 2) {
+	if (strlen(chap_r) != chap->digest_size * 2) {
 		pr_err("Malformed CHAP_R\n");
 		goto out;
 	}
-	if (hex2bin(client_digest, chap_r, MD5_SIGNATURE_SIZE) < 0) {
+	if (hex2bin(client_digest, chap_r, chap->digest_size) < 0) {
 		pr_err("Malformed CHAP_R\n");
 		goto out;
 	}
 
 	pr_debug("[server] Got CHAP_R=%s\n", chap_r);
 
-	tfm = crypto_alloc_shash("md5", 0, 0);
+	tfm = crypto_alloc_shash(chap->digest_name, 0, 0);
 	if (IS_ERR(tfm)) {
 		tfm = NULL;
 		pr_err("Unable to allocate struct crypto_shash\n");
@@ -271,15 +332,15 @@ static int chap_server_compute_md5(
 		goto out;
 	}
 
-	bin2hex(response, server_digest, MD5_SIGNATURE_SIZE);
-	pr_debug("[server] MD5 Server Digest: %s\n", response);
+	bin2hex(response, server_digest, chap->digest_size);
+	pr_debug("[server] %s Server Digest: %s\n", hash_name, response);
 
-	if (memcmp(server_digest, client_digest, MD5_SIGNATURE_SIZE) != 0) {
-		pr_debug("[server] MD5 Digests do not match!\n\n");
+	if (memcmp(server_digest, client_digest, chap->digest_size) != 0) {
+		pr_debug("[server] %s Digests do not match!\n\n", hash_name);
 		goto out;
 	} else
-		pr_debug("[server] MD5 Digests match, CHAP connection"
-				" successful.\n\n");
+		pr_debug("[server] %s Digests match, CHAP connection"
+				" successful.\n\n", hash_name);
 	/*
 	 * One way authentication has succeeded, return now if mutual
 	 * authentication is not enabled.
@@ -393,7 +454,7 @@ static int chap_server_compute_md5(
 	/*
 	 * Convert response from binary hex to ascii hext.
 	 */
-	bin2hex(response, digest, MD5_SIGNATURE_SIZE);
+	bin2hex(response, digest, chap->digest_size);
 	*nr_out_len += sprintf(nr_out_ptr + *nr_out_len, "CHAP_R=0x%s",
 			response);
 	*nr_out_len += 1;
@@ -405,6 +466,10 @@ static int chap_server_compute_md5(
 		crypto_free_shash(tfm);
 	kfree(challenge);
 	kfree(challenge_binhex);
+	kfree(digest);
+	kfree(response);
+	kfree(server_digest);
+	kfree(client_digest);
 	return auth_ret;
 }
 
@@ -419,7 +484,9 @@ static int chap_got_response(
 
 	switch (chap->digest_type) {
 	case CHAP_DIGEST_MD5:
-		if (chap_server_compute_md5(conn, auth, nr_in_ptr,
+	case CHAP_DIGEST_SHA:
+	case CHAP_DIGEST_SHA3_256:
+		if (chap_server_compute_hash(conn, auth, nr_in_ptr,
 				nr_out_ptr, nr_out_len) < 0)
 			return -1;
 		return 0;
diff --git a/drivers/target/iscsi/iscsi_target_auth.h b/drivers/target/iscsi/iscsi_target_auth.h
index d5600ac30b53..8b10f935675a 100644
--- a/drivers/target/iscsi/iscsi_target_auth.h
+++ b/drivers/target/iscsi/iscsi_target_auth.h
@@ -7,13 +7,16 @@
 #define CHAP_DIGEST_UNKNOWN	0
 #define CHAP_DIGEST_MD5		5
 #define CHAP_DIGEST_SHA		6
+#define CHAP_DIGEST_SHA3_256	7
 
 #define CHAP_CHALLENGE_LENGTH	16
 #define CHAP_CHALLENGE_STR_LEN	4096
-#define MAX_RESPONSE_LENGTH	64	/* sufficient for MD5 */
+#define MAX_RESPONSE_LENGTH	128	/* sufficient for SHA3 256 */
 #define	MAX_CHAP_N_SIZE		512
 
 #define MD5_SIGNATURE_SIZE	16	/* 16 bytes in a MD5 message digest */
+#define SHA_SIGNATURE_SIZE	20	/* 20 bytes in a SHA1 message digest */
+#define SHA3_256_SIGNATURE_SIZE	32	/* 32 bytes in a SHA3 256 message digest */
 
 #define CHAP_STAGE_CLIENT_A	1
 #define CHAP_STAGE_SERVER_AIC	2
@@ -28,9 +31,11 @@ extern u32 chap_main_loop(struct iscsi_conn *, struct iscsi_node_auth *, char *,
 				int *, int *);
 
 struct iscsi_chap {
-	unsigned char	digest_type;
 	unsigned char	id;
 	unsigned char	challenge[CHAP_CHALLENGE_LENGTH];
+	unsigned int	challenge_len;
+	unsigned char	*digest_name;
+	unsigned int	digest_size;
 	unsigned int	authenticate_target;
 	unsigned int	chap_state;
 } ____cacheline_aligned;
-- 
Maurizio Lombardi

[PATCH 2/4] target-iscsi: remove unneeded function

[Maurizio Lombardi]

The digest type validy is already checked by chap_server_open(), therefore
we can remove the chap_got_response() function.

Signed-off-by: Maurizio Lombardi <mlombard@redhat.com>
---
 drivers/target/iscsi/iscsi_target_auth.c | 26 +-----------------------
 1 file changed, 1 insertion(+), 25 deletions(-)

diff --git a/drivers/target/iscsi/iscsi_target_auth.c b/drivers/target/iscsi/iscsi_target_auth.c
index 3d1e94333835..77dac8086927 100644
--- a/drivers/target/iscsi/iscsi_target_auth.c
+++ b/drivers/target/iscsi/iscsi_target_auth.c
@@ -473,30 +473,6 @@ static int chap_server_compute_hash(
 	return auth_ret;
 }
 
-static int chap_got_response(
-	struct iscsi_conn *conn,
-	struct iscsi_node_auth *auth,
-	char *nr_in_ptr,
-	char *nr_out_ptr,
-	unsigned int *nr_out_len)
-{
-	struct iscsi_chap *chap = conn->auth_protocol;
-
-	switch (chap->digest_type) {
-	case CHAP_DIGEST_MD5:
-	case CHAP_DIGEST_SHA:
-	case CHAP_DIGEST_SHA3_256:
-		if (chap_server_compute_hash(conn, auth, nr_in_ptr,
-				nr_out_ptr, nr_out_len) < 0)
-			return -1;
-		return 0;
-	default:
-		pr_err("Unknown CHAP digest type %d!\n",
-				chap->digest_type);
-		return -1;
-	}
-}
-
 u32 chap_main_loop(
 	struct iscsi_conn *conn,
 	struct iscsi_node_auth *auth,
@@ -515,7 +491,7 @@ u32 chap_main_loop(
 		return 0;
 	} else if (chap->chap_state == CHAP_STAGE_SERVER_AIC) {
 		convert_null_to_semi(in_text, *in_len);
-		if (chap_got_response(conn, auth, in_text, out_text,
+		if (chap_server_compute_hash(conn, auth, in_text, out_text,
 				out_len) < 0) {
 			chap_close(conn);
 			return 2;
-- 
Maurizio Lombardi

[PATCH 3/4] target-iscsi: tie the challenge length to the hash digest size

[Maurizio Lombardi]

Signed-off-by: Maurizio Lombardi <mlombard@redhat.com>
---
 drivers/target/iscsi/iscsi_target_auth.c | 37 +++++++++++++++++-------
 drivers/target/iscsi/iscsi_target_auth.h |  4 +--
 2 files changed, 28 insertions(+), 13 deletions(-)

diff --git a/drivers/target/iscsi/iscsi_target_auth.c b/drivers/target/iscsi/iscsi_target_auth.c
index 77dac8086927..976c8c73d261 100644
--- a/drivers/target/iscsi/iscsi_target_auth.c
+++ b/drivers/target/iscsi/iscsi_target_auth.c
@@ -25,16 +25,21 @@ static int chap_gen_challenge(
 	unsigned int *c_len)
 {
 	int ret;
-	unsigned char challenge_asciihex[CHAP_CHALLENGE_LENGTH * 2 + 1];
+	unsigned char *challenge_asciihex;
 	struct iscsi_chap *chap = conn->auth_protocol;
 
-	memset(challenge_asciihex, 0, CHAP_CHALLENGE_LENGTH * 2 + 1);
+	challenge_asciihex = kzalloc(chap->challenge_len * 2 + 1, GFP_KERNEL);
+	if (!challenge_asciihex)
+		return -ENOMEM;
 
-	ret = get_random_bytes_wait(chap->challenge, CHAP_CHALLENGE_LENGTH);
+	memset(chap->challenge, 0, MAX_CHAP_CHALLENGE_LEN);
+
+	ret = get_random_bytes_wait(chap->challenge, chap->challenge_len);
 	if (unlikely(ret))
-		return ret;
+		goto out;
+
 	bin2hex(challenge_asciihex, chap->challenge,
-				CHAP_CHALLENGE_LENGTH);
+				chap->challenge_len);
 	/*
 	 * Set CHAP_C, and copy the generated challenge into c_str.
 	 */
@@ -43,7 +48,10 @@ static int chap_gen_challenge(
 
 	pr_debug("[%s] Sending CHAP_C=0x%s\n\n", (caller) ? "server" : "client",
 			challenge_asciihex);
-	return 0;
+
+out:
+	kfree(challenge_asciihex);
+	return ret;
 }
 
 static int chap_test_algorithm(const char *name)
@@ -163,6 +171,9 @@ static struct iscsi_chap *chap_server_open(
 		return NULL;
 	}
 
+	/* Tie the challenge length to the digest size */
+	chap->challenge_len = chap->digest_size;
+
 	pr_debug("[server] Got CHAP_A=%d\n", digest_type);
 	*aic_len = sprintf(aic_str, "CHAP_A=%d", digest_type);
 	*aic_len += 1;
@@ -326,21 +337,23 @@ static int chap_server_compute_hash(
 	}
 
 	ret = crypto_shash_finup(desc, chap->challenge,
-				 CHAP_CHALLENGE_LENGTH, server_digest);
+				 chap->challenge_len, server_digest);
 	if (ret < 0) {
 		pr_err("crypto_shash_finup() failed for challenge\n");
 		goto out;
 	}
 
 	bin2hex(response, server_digest, chap->digest_size);
-	pr_debug("[server] %s Server Digest: %s\n", hash_name, response);
+	pr_debug("[server] %s Server Digest: %s\n",
+		chap->digest_name, response);
 
 	if (memcmp(server_digest, client_digest, chap->digest_size) != 0) {
-		pr_debug("[server] %s Digests do not match!\n\n", hash_name);
+		pr_debug("[server] %s Digests do not match!\n\n",
+			chap->digest_name);
 		goto out;
 	} else
 		pr_debug("[server] %s Digests match, CHAP connection"
-				" successful.\n\n", hash_name);
+				" successful.\n\n", chap->digest_name);
 	/*
 	 * One way authentication has succeeded, return now if mutual
 	 * authentication is not enabled.
@@ -406,7 +419,9 @@ static int chap_server_compute_hash(
 	 * initiator must not match the original CHAP_C generated by
 	 * the target.
 	 */
-	if (!memcmp(challenge_binhex, chap->challenge, CHAP_CHALLENGE_LENGTH)) {
+	if (challenge_len == chap->challenge_len &&
+				!memcmp(challenge_binhex, chap->challenge,
+				challenge_len)) {
 		pr_err("initiator CHAP_C matches target CHAP_C, failing"
 		       " login attempt\n");
 		goto out;
diff --git a/drivers/target/iscsi/iscsi_target_auth.h b/drivers/target/iscsi/iscsi_target_auth.h
index 8b10f935675a..e22840fea903 100644
--- a/drivers/target/iscsi/iscsi_target_auth.h
+++ b/drivers/target/iscsi/iscsi_target_auth.h
@@ -9,7 +9,7 @@
 #define CHAP_DIGEST_SHA		6
 #define CHAP_DIGEST_SHA3_256	7
 
-#define CHAP_CHALLENGE_LENGTH	16
+#define MAX_CHAP_CHALLENGE_LEN	32
 #define CHAP_CHALLENGE_STR_LEN	4096
 #define MAX_RESPONSE_LENGTH	128	/* sufficient for SHA3 256 */
 #define	MAX_CHAP_N_SIZE		512
@@ -32,7 +32,7 @@ extern u32 chap_main_loop(struct iscsi_conn *, struct iscsi_node_auth *, char *,
 
 struct iscsi_chap {
 	unsigned char	id;
-	unsigned char	challenge[CHAP_CHALLENGE_LENGTH];
+	unsigned char	challenge[MAX_CHAP_CHALLENGE_LEN];
 	unsigned int	challenge_len;
 	unsigned char	*digest_name;
 	unsigned int	digest_size;
-- 
Maurizio Lombardi

[PATCH 4/4] target-iscsi: rename some variables to avoid confusion.

[Maurizio Lombardi]

This patch renames some variables in chap_server_compute_hash()
to make it harder to confuse the initiator's challenge with
the target's challenge when the mutual chap authentication is used.

Signed-off-by: Maurizio Lombardi <mlombard@redhat.com>
---
 drivers/target/iscsi/iscsi_target_auth.c | 40 ++++++++++++------------
 1 file changed, 20 insertions(+), 20 deletions(-)

diff --git a/drivers/target/iscsi/iscsi_target_auth.c b/drivers/target/iscsi/iscsi_target_auth.c
index 976c8c73d261..7ccef7b1c60b 100644
--- a/drivers/target/iscsi/iscsi_target_auth.c
+++ b/drivers/target/iscsi/iscsi_target_auth.c
@@ -207,8 +207,8 @@ static int chap_server_compute_hash(
 	unsigned long id;
 	unsigned char id_as_uchar;
 	unsigned char type;
-	unsigned char identifier[10], *challenge = NULL;
-	unsigned char *challenge_binhex = NULL;
+	unsigned char identifier[10], *initiatorchg = NULL;
+	unsigned char *initiatorchg_binhex = NULL;
 	unsigned char *digest = NULL;
 	unsigned char *response = NULL;
 	unsigned char *client_digest = NULL;
@@ -218,7 +218,7 @@ static int chap_server_compute_hash(
 	struct iscsi_chap *chap = conn->auth_protocol;
 	struct crypto_shash *tfm = NULL;
 	struct shash_desc *desc = NULL;
-	int auth_ret = -1, ret, challenge_len;
+	int auth_ret = -1, ret, initiatorchg_len;
 
 	digest = kzalloc(chap->digest_size, GFP_KERNEL);
 	if (!digest) {
@@ -248,15 +248,15 @@ static int chap_server_compute_hash(
 	memset(chap_n, 0, MAX_CHAP_N_SIZE);
 	memset(chap_r, 0, MAX_RESPONSE_LENGTH);
 
-	challenge = kzalloc(CHAP_CHALLENGE_STR_LEN, GFP_KERNEL);
-	if (!challenge) {
+	initiatorchg = kzalloc(CHAP_CHALLENGE_STR_LEN, GFP_KERNEL);
+	if (!initiatorchg) {
 		pr_err("Unable to allocate challenge buffer\n");
 		goto out;
 	}
 
-	challenge_binhex = kzalloc(CHAP_CHALLENGE_STR_LEN, GFP_KERNEL);
-	if (!challenge_binhex) {
-		pr_err("Unable to allocate challenge_binhex buffer\n");
+	initiatorchg_binhex = kzalloc(CHAP_CHALLENGE_STR_LEN, GFP_KERNEL);
+	if (!initiatorchg_binhex) {
+		pr_err("Unable to allocate initiatorchg_binhex buffer\n");
 		goto out;
 	}
 	/*
@@ -391,7 +391,7 @@ static int chap_server_compute_hash(
 	 * Get CHAP_C.
 	 */
 	if (extract_param(nr_in_ptr, "CHAP_C", CHAP_CHALLENGE_STR_LEN,
-			challenge, &type) < 0) {
+			initiatorchg, &type) < 0) {
 		pr_err("Could not find CHAP_C.\n");
 		goto out;
 	}
@@ -400,28 +400,28 @@ static int chap_server_compute_hash(
 		pr_err("Could not find CHAP_C.\n");
 		goto out;
 	}
-	challenge_len = DIV_ROUND_UP(strlen(challenge), 2);
-	if (!challenge_len) {
+	initiatorchg_len = DIV_ROUND_UP(strlen(initiatorchg), 2);
+	if (!initiatorchg_len) {
 		pr_err("Unable to convert incoming challenge\n");
 		goto out;
 	}
-	if (challenge_len > 1024) {
+	if (initiatorchg_len > 1024) {
 		pr_err("CHAP_C exceeds maximum binary size of 1024 bytes\n");
 		goto out;
 	}
-	if (hex2bin(challenge_binhex, challenge, challenge_len) < 0) {
+	if (hex2bin(initiatorchg_binhex, initiatorchg, initiatorchg_len) < 0) {
 		pr_err("Malformed CHAP_C\n");
 		goto out;
 	}
-	pr_debug("[server] Got CHAP_C=%s\n", challenge);
+	pr_debug("[server] Got CHAP_C=%s\n", initiatorchg);
 	/*
 	 * During mutual authentication, the CHAP_C generated by the
 	 * initiator must not match the original CHAP_C generated by
 	 * the target.
 	 */
-	if (challenge_len == chap->challenge_len &&
-				!memcmp(challenge_binhex, chap->challenge,
-				challenge_len)) {
+	if (initiatorchg_len == chap->challenge_len &&
+				!memcmp(initiatorchg_binhex, chap->challenge,
+				initiatorchg_len)) {
 		pr_err("initiator CHAP_C matches target CHAP_C, failing"
 		       " login attempt\n");
 		goto out;
@@ -453,7 +453,7 @@ static int chap_server_compute_hash(
 	/*
 	 * Convert received challenge to binary hex.
 	 */
-	ret = crypto_shash_finup(desc, challenge_binhex, challenge_len,
+	ret = crypto_shash_finup(desc, initiatorchg_binhex, initiatorchg_len,
 				 digest);
 	if (ret < 0) {
 		pr_err("crypto_shash_finup() failed for ma challenge\n");
@@ -479,8 +479,8 @@ static int chap_server_compute_hash(
 	kzfree(desc);
 	if (tfm)
 		crypto_free_shash(tfm);
-	kfree(challenge);
-	kfree(challenge_binhex);
+	kfree(initiatorchg);
+	kfree(initiatorchg_binhex);
 	kfree(digest);
 	kfree(response);
 	kfree(server_digest);
-- 
Maurizio Lombardi

Re: [RFC PATCH 0/4] iscsi: chap: introduce support for SHA1 and SHA3-256

[Christoph Hellwig]

On Thu, Aug 29, 2019 at 05:59:25PM +0200, Maurizio Lombardi wrote:
> iSCSI with the Challenge-Handshake Authentication Protocol is not FIPS compliant.
> This is due to the fact that CHAP currently uses MD5 as the only supported
> digest algorithm and MD5 is not allowed by FIPS.
> 
> When FIPS mode is enabled on the target server, the CHAP authentication
> won't work because the target driver will be prevented from using the MD5 module.
> 
> Given that CHAP is agnostic regarding the algorithm it uses, this
> patchset introduce support for two new alternatives: SHA1 and SHA3-256.
> 
> SHA1 has already its own assigned value for its use in CHAP, as reported by IANA:
> https://www.iana.org/assignments/ppp-numbers/ppp-numbers.xml#ppp-numbers-9
> On the other hand the use of SHA1 on FIPS-enabled systems has been deprecated
> and therefore it's not a vialable long term option.
> 
> We could consider introducing a more modern hash algorithm like SHA3-256, as
> this patchset does.

But we'll need IANA assignments and IETF consensus before adding new
algorithms to ensure we have interoperable implementations.

Adding Dave Black who has helped with IANA interaction in NVMe recently.

RE: [RFC PATCH 0/4] iscsi: chap: introduce support for SHA1 and SHA3-256

[Black, David]

Christoph,

> Adding Dave Black who has helped with IANA interaction in NVMe recently.

I see my cue ... please keep me cc:'d on this conversation, as I'm not on either of the mailing lists.

> But we'll need IANA assignments and IETF consensus before adding new
> algorithms to ensure we have interoperable implementations.

In reverse order ...

-- IETF Consensus: 

My sense of the IETF view on secure hashes is that MD5 and SHA1 are broken, whereas the SHA2 algorithms are proving to be longer-lived (more resistant to attack) than expected, and the SHA3 algorithms are fine.

That suggests that registration of codepoints for both SHA2 and SHA3 would be a good thing to do, as opposed to only SHA3.  I'd suggest starting with either SHA-256 or SHA-512/256 (both are SHA2 hashes) in addition to SHA3-256, as all three have the same 256-bit output size.

Figuring out exactly what should be done here (e.g., which SHA2 variant to register) would benefit from some discussion at IETF.  I would start with the Security Area's saag@ietf.org mailing list.  In addition, as iSCSI falls within IETF's Transport Area, the Transport Area Directors ought to be looped in beforehand.  Fortunately, publication of an RFC is not necessary, because ...

-- IANA assignments

... the Registration Procedure for PPP Authentication Algorithms is Expert Review.   The long version of what that means is in Section 4.5 of RFC 8126: https://tools.ietf.org/html/rfc8126#section-4.5.  The short version is that a request for allocation of these codepoints is submitted to IANA, whose designated expert then makes a decision.  It's probably a good idea for that request to state that the intended usage is iSCSI, and say that it's ok to restrict the resulting registrations solely to use by iSCSI.

As Christoph notes, I've helped with IANA interactions at NVMe, and would be likewise willing to help here.  My name is attached to the SHA1 registration, so it would make sense for me to ask for the SHA2 and SHA3 registrations, and I know a number of the people who will be involved in ensuring that the proverbial "right thing" happens, e.g., the Transport Area Directors.

Thanks, --David

> -----Original Message-----
> From: Christoph Hellwig <hch@infradead.org>
> Sent: Tuesday, September 3, 2019 3:00 AM
> To: Maurizio Lombardi
> Cc: cleech@redhat.com; mchristi@redhat.com; linux-scsi@vger.kernel.org;
> target-devel@vger.kernel.org; Black, David
> Subject: Re: [RFC PATCH 0/4] iscsi: chap: introduce support for SHA1 and
> SHA3-256
> 
> 
> [EXTERNAL EMAIL]
> 
> On Thu, Aug 29, 2019 at 05:59:25PM +0200, Maurizio Lombardi wrote:
> > iSCSI with the Challenge-Handshake Authentication Protocol is not FIPS
> compliant.
> > This is due to the fact that CHAP currently uses MD5 as the only supported
> > digest algorithm and MD5 is not allowed by FIPS.
> >
> > When FIPS mode is enabled on the target server, the CHAP authentication
> > won't work because the target driver will be prevented from using the
> MD5 module.
> >
> > Given that CHAP is agnostic regarding the algorithm it uses, this
> > patchset introduce support for two new alternatives: SHA1 and SHA3-256.
> >
> > SHA1 has already its own assigned value for its use in CHAP, as reported by IANA:
> > https://www.iana.org/assignments/ppp-numbers/ppp-numbers.xml#ppp-numbers-9
> > On the other hand the use of SHA1 on FIPS-enabled systems has been deprecated
> > and therefore it's not a vialable long term option.
> >
> > We could consider introducing a more modern hash algorithm like SHA3-256, as
> > this patchset does.
> 
> But we'll need IANA assignments and IETF consensus before adding new
> algorithms to ensure we have interoperable implementations.
> 
> Adding Dave Black who has helped with IANA interaction in NVMe recently.

Re: [RFC PATCH 0/4] iscsi: chap: introduce support for SHA1 and SHA3-256

[Maurizio Lombardi]

Hello David,

first of all, thank for your reply and your offer to help with this.
We appreciate this a lot.


Dne 4.9.2019 v 01:59 Black, David napsal(a):
> Christoph,
> 
>> Adding Dave Black who has helped with IANA interaction in NVMe recently.
> 
> I see my cue ... please keep me cc:'d on this conversation, as I'm not on either of the mailing lists.
> 
>> But we'll need IANA assignments and IETF consensus before adding new
>> algorithms to ensure we have interoperable implementations.
> 
> In reverse order ...
> 
> -- IETF Consensus: 
> 
> My sense of the IETF view on secure hashes is that MD5 and SHA1 are broken, whereas the SHA2 algorithms are proving to be longer-lived (more resistant to attack) than expected, and the SHA3 algorithms are fine.
> 
> That suggests that registration of codepoints for both SHA2 and SHA3 would be a good thing to do, as opposed to only SHA3.  I'd suggest starting with either SHA-256 or SHA-512/256 (both are SHA2 hashes) in addition to SHA3-256, as all three have the same 256-bit output size.

Agree. Having SHA-256 would make sense.

> 
> Figuring out exactly what should be done here (e.g., which SHA2 variant to register) would benefit from some discussion at IETF.  I would start with the Security Area's saag@ietf.org mailing list.  In addition, as iSCSI falls within IETF's Transport Area, the Transport Area Directors ought to be looped in beforehand.  Fortunately, publication of an RFC is not necessary, because ...

Ok, I am going to send an email for the SAAG mailing list to see what they think about it.

> 
> -- IANA assignments
> 
> ... the Registration Procedure for PPP Authentication Algorithms is Expert Review.   The long version of what that means is in Section 4.5 of RFC 8126: https://tools.ietf.org/html/rfc8126#section-4.5.  The short version is that a request for allocation of these codepoints is submitted to IANA, whose designated expert then makes a decision.  It's probably a good idea for that request to state that the intended usage is iSCSI, and say that it's ok to restrict the resulting registrations solely to use by iSCSI.
> 
> As Christoph notes, I've helped with IANA interactions at NVMe, and would be likewise willing to help here.  My name is attached to the SHA1 registration, so it would make sense for me to ask for the SHA2 and SHA3 registrations, and I know a number of the people who will be involved in ensuring that the proverbial "right thing" happens, e.g., the Transport Area Directors.

Thank you very much for the help!
Maurizio Lombardi