Skip to contentSkip to conversation

scsi: target: iscsi: Use crypto library API

PATCH Applied Eric Biggers 3 participants 7 messages Oct 1 – Oct 24

Subsystems (3)
SCSI Crypto API SCSI target
lore
Share & tools
lore

Eric Biggers

author
#0 lore raw

PATCH 1/1 scsi: target: iscsi: Use crypto library API

Make iscsi_target_auth.c call into the crypto library directly instead of going through crypto_shash. This is simpler and much more efficient, as it avoids overhead of the crypto_shash layer. The hash computation itself also no longer has error codes that need to be checked.

For simplicity, still do a memory allocation per operation to buffer the data (which is typically less than 50 bytes here) so that the one-shot digest functions can be used. It's still much more efficient than doing the super slow crypto_alloc_shash().

Signed-off-by Eric Biggers <[email protected]>
---
 drivers/target/iscsi/Kconfig             |   7 +-
 drivers/target/iscsi/iscsi_target_auth.c | 192 +++++++++--------------
 drivers/target/iscsi/iscsi_target_auth.h |   8 +-
 3 files changed, 78 insertions(+), 129 deletions(-)
drivers/target/iscsi/Kconfig +5 -2
index 70d76f3dd693..44bbf1f32859 100644
@@ -3,8 +3,11 @@ config ISCSI_TARGET
 	tristate "SCSI Target Mode Stack"
 	depends on INET
 	select CRC32
-	select CRYPTO
-	select CRYPTO_HASH
+	select CRYPTO_LIB_MD5
+	select CRYPTO_LIB_SHA1
+	select CRYPTO_LIB_SHA256
+	select CRYPTO_LIB_SHA3
+	select CRYPTO_LIB_UTILS
 	help
 	Say M to enable the SCSI target mode stack. A SCSI target mode stack
 	is software that makes local storage available over a storage network
drivers/target/iscsi/iscsi_target_auth.c +71 -121

275 lines not on this page · raw message

drivers/target/iscsi/iscsi_target_auth.h +2 -6
index ceb9b7754770..7ed288936af2 100644
@@ -15,11 +15,6 @@
 #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 SHA1_SIGNATURE_SIZE	20	/* 20 bytes in a SHA1 message digest */
-#define SHA256_SIGNATURE_SIZE	32	/* 32 bytes in a SHA256 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
 #define CHAP_STAGE_CLIENT_NR	3

@@ -36,7 +31,8 @@ struct iscsi_chap {
 	unsigned char	id;
 	unsigned char	challenge[MAX_CHAP_CHALLENGE_LEN];
 	unsigned int	challenge_len;
-	unsigned char	*digest_name;
+	const char	*digest_name;
+	int		digest_type;
 	unsigned int	digest_size;
 	unsigned int	authenticate_target;
 	unsigned int	chap_state;

base-commit: 551c722f40809618230001baccf219193e22fc5a

Signature · 1 line
2.56.0
Reviewed-by on v1 1/1
#1 lore raw

This looks like a worthwhile clean-up and doesn't trigger any libiscsi chap test regressions.

Reviewed-by David Disseldorp <[email protected]>

I do have one minor concern:

Quoted 16 lines · Eric Biggers

Make iscsi_target_auth.c call into the crypto library directly instead of going through crypto_shash. This is simpler and much more efficient, as it avoids overhead of the crypto_shash layer. The hash computation itself also no longer has error codes that need to be checked.

For simplicity, still do a memory allocation per operation to buffer the data (which is typically less than 50 bytes here) so that the one-shot digest functions can be used. It's still much more efficient than doing the super slow crypto_alloc_shash().

Signed-off-by Eric Biggers <[email protected]>
---
 drivers/target/iscsi/Kconfig             |   7 +-
 drivers/target/iscsi/iscsi_target_auth.c | 192 +++++++++--------------
 drivers/target/iscsi/iscsi_target_auth.h |   8 +-
 3 files changed, 78 insertions(+), 129 deletions(-)

...

drivers/target/iscsi/iscsi_target_auth.c static int chap_check_algorithm(const char *a_str) · line 147

Quoted 8 lines
drivers/target/iscsi/iscsi_target_auth.c +1 -2
@@ -113,17 +143,17 @@ static int chap_check_algorithm(const char *a_str)
 		if (kstrtol(token, 10, &digest_type))
 			continue;
 
-		digest_name = chap_get_digest_name(digest_type);
-		if (!digest_name)
+		if (fips_enabled && digest_type == CHAP_DIGEST_MD5)
 			continue;

Could e.g. SHA1 also require fips filtering here in future? If so, would it make sense to track the fips compat status (preferably at build time) in chap_digest_algs?

author
#2 lore raw
Quoted 1 line · David Disseldorp

Could e.g. SHA1 also require fips filtering here in future?

It might.

Quoted 2 lines

If so, would it make sense to track the fips compat status (preferably at build time) in chap_digest_algs?

Let's wait until there's actually more than one before generalizing it.

- Eric

#3 lore raw
Quoted 9 lines · Eric Biggers
Quoted 1 line · David Disseldorp

Could e.g. SHA1 also require fips filtering here in future?

It might.

Quoted 2 lines

If so, would it make sense to track the fips compat status (preferably at build time) in chap_digest_algs?

Let's wait until there's actually more than one before generalizing it.

Fair enough. If you're okay with having the extra caller to update / audit following any fips compliance changes then I think it makes sense to merge this as-is.

author
#4 lore raw
In reply to Eric Biggers
Quoted 11 lines · Eric Biggers

Make iscsi_target_auth.c call into the crypto library directly instead of going through crypto_shash. This is simpler and much more efficient, as it avoids overhead of the crypto_shash layer. The hash computation itself also no longer has error codes that need to be checked.

For simplicity, still do a memory allocation per operation to buffer the data (which is typically less than 50 bytes here) so that the one-shot digest functions can be used. It's still much more efficient than doing the super slow crypto_alloc_shash().

Signed-off-by Eric Biggers <[email protected]>

Martin, could you take this via the scsi tree?

Thanks,

- Eric

maintainer Applied
#5 lore raw
In reply to Eric Biggers

Eric,

Quoted 5 lines

Make iscsi_target_auth.c call into the crypto library directly instead of going through crypto_shash. This is simpler and much more efficient, as it avoids overhead of the crypto_shash layer. The hash computation itself also no longer has error codes that need to be checked.

Applied to 7.4/scsi-staging, thanks!

Signature · 1 line
Martin K. Petersen
maintainer Applied
#6 lore raw
In reply to Eric Biggers
Quoted 11 lines · Eric Biggers

Make iscsi_target_auth.c call into the crypto library directly instead of going through crypto_shash. This is simpler and much more efficient, as it avoids overhead of the crypto_shash layer. The hash computation itself also no longer has error codes that need to be checked.

For simplicity, still do a memory allocation per operation to buffer the data (which is typically less than 50 bytes here) so that the one-shot digest functions can be used. It's still much more efficient than doing the super slow crypto_alloc_shash().

[...]

Applied to 7.4/scsi-queue, thanks!

[1/1] scsi: target: iscsi: Use crypto library API

      https://git.kernel.org/mkp/scsi/c/b633249f5883
Signature · 1 line
Martin K. Petersen