NetworkPkg/IScsiDxe: clean up "ISCSI_CHAP_AUTH_DATA.OutChallengeLength"

The "ISCSI_CHAP_AUTH_DATA.OutChallenge" field is declared as a UINT8 array
with ISCSI_CHAP_AUTH_MAX_LEN (1024) elements. However, when the challenge
is generated and formatted, only ISCSI_CHAP_RSP_LEN (16) octets are used
in the array.

Change the array size to ISCSI_CHAP_RSP_LEN, and remove the (now unused)
ISCSI_CHAP_AUTH_MAX_LEN macro.

Remove the "ISCSI_CHAP_AUTH_DATA.OutChallengeLength" field, which is
superfluous too.

Most importantly, explain in a new comment *why* tying the challenge size
to the digest size (ISCSI_CHAP_RSP_LEN) has always made sense. (See also
Linux kernel commit 19f5f88ed779, "scsi: target: iscsi: tie the challenge
length to the hash digest size", 2019-11-06.) For sure, the motivation
that the new comment now explains has always been there, and has always
been the same, for IScsiDxe; it's just that now we spell it out too.

No change in peer-visible behavior.

Cc: Jiaxin Wu <jiaxin.wu@intel.com>
Cc: Maciej Rabeda <maciej.rabeda@linux.intel.com>
Cc: Philippe Mathieu-Daudé <philmd@redhat.com>
Cc: Siyuan Fu <siyuan.fu@intel.com>
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=3356
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Reviewed-by: Maciej Rabeda <maciej.rabeda@linux.intel.com>
Message-Id: <20210608121259.32451-4-lersek@redhat.com>

diff --git a/NetworkPkg/IScsiDxe/IScsiCHAP.c b/NetworkPkg/IScsiDxe/IScsiCHAP.c
index df3c2eb1200a73626e50f24cabf2cdbdbdfb1570..9e192ce292e87ecce296df12b81ea3819d721dce 100644 (file)
--- a/NetworkPkg/IScsiDxe/IScsiCHAP.c
+++ b/NetworkPkg/IScsiDxe/IScsiCHAP.c
@@ -122,7 +122,7 @@ IScsiCHAPAuthTarget (
              AuthData->AuthConfig->ReverseCHAPSecret,\r
              SecretSize,\r
              AuthData->OutChallenge,\r
-             AuthData->OutChallengeLength,\r
+             ISCSI_CHAP_RSP_LEN,                      // ChallengeLength\r
              VerifyRsp\r
              );\r
 \r
@@ -490,7 +490,6 @@ IScsiCHAPToSendReq (
       // CHAP_C=<C>\r
       //\r
       IScsiGenRandom ((UINT8 *) AuthData->OutChallenge, ISCSI_CHAP_RSP_LEN);\r
-      AuthData->OutChallengeLength = ISCSI_CHAP_RSP_LEN;\r
       IScsiBinToHex (\r
         (UINT8 *) AuthData->OutChallenge,\r
         ISCSI_CHAP_RSP_LEN,\r

diff --git a/NetworkPkg/IScsiDxe/IScsiCHAP.h b/NetworkPkg/IScsiDxe/IScsiCHAP.h
index 1fc1d96ea3f30f2e415c91543a0a77be11b21d0c..35d5d6ec29e3a437cfca8c56c7cf5989a69692d7 100644 (file)
--- a/NetworkPkg/IScsiDxe/IScsiCHAP.h
+++ b/NetworkPkg/IScsiDxe/IScsiCHAP.h
@@ -19,7 +19,6 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
 \r
 #define ISCSI_CHAP_ALGORITHM_MD5  5\r
 \r
-#define ISCSI_CHAP_AUTH_MAX_LEN   1024\r
 ///\r
 /// MD5_HASHSIZE\r
 ///\r
@@ -59,9 +58,13 @@ typedef struct _ISCSI_CHAP_AUTH_DATA {
   //\r
   // Auth-data to be sent out for mutual authentication.\r
   //\r
+  // While the challenge size is technically independent of the hashing\r
+  // algorithm, it is good practice to avoid hashing *fewer bytes* than the\r
+  // digest size. In other words, it's good practice to feed *at least as many\r
+  // bytes* to the hashing algorithm as the hashing algorithm will output.\r
+  //\r
   UINT32                        OutIdentifier;\r
-  UINT8                         OutChallenge[ISCSI_CHAP_AUTH_MAX_LEN];\r
-  UINT32                        OutChallengeLength;\r
+  UINT8                         OutChallenge[ISCSI_CHAP_RSP_LEN];\r
 } ISCSI_CHAP_AUTH_DATA;\r
 \r
 /**\r