|
 |
d15d15 |
From dd65b4f245e318e0d76a213c92b159819c6dae79 Mon Sep 17 00:00:00 2001
|
|
|
d15d15 |
From: Laszlo Ersek <lersek@redhat.com>
|
|
|
d15d15 |
Date: Mon, 26 Apr 2021 20:17:23 +0200
|
|
|
d15d15 |
Subject: [PATCH 03/10] NetworkPkg/IScsiDxe: clean up
|
|
|
d15d15 |
"ISCSI_CHAP_AUTH_DATA.OutChallengeLength"
|
|
|
d15d15 |
MIME-Version: 1.0
|
|
|
d15d15 |
Content-Type: text/plain; charset=UTF-8
|
|
|
d15d15 |
Content-Transfer-Encoding: 8bit
|
|
|
d15d15 |
|
|
|
d15d15 |
RH-Author: Laszlo Ersek <lersek@redhat.com>
|
|
|
d15d15 |
RH-MergeRequest: 3: NetworkPkg/IScsiDxe: fix IScsiHexToBin() security and functionality bugs [rhel-8.4.0.z]
|
|
|
d15d15 |
RH-Commit: [3/10] 93e6e1fa7f093898350a40ec60201f64a8849f3c
|
|
|
d15d15 |
RH-Bugzilla: 1956676
|
|
|
d15d15 |
RH-Acked-by: Philippe Mathieu-Daudé <philmd@redhat.com>
|
|
|
d15d15 |
|
|
|
d15d15 |
The "ISCSI_CHAP_AUTH_DATA.OutChallenge" field is declared as a UINT8 array
|
|
|
d15d15 |
with ISCSI_CHAP_AUTH_MAX_LEN (1024) elements. However, when the challenge
|
|
|
d15d15 |
is generated and formatted, only ISCSI_CHAP_RSP_LEN (16) octets are used
|
|
|
d15d15 |
in the array.
|
|
|
d15d15 |
|
|
|
d15d15 |
Change the array size to ISCSI_CHAP_RSP_LEN, and remove the (now unused)
|
|
|
d15d15 |
ISCSI_CHAP_AUTH_MAX_LEN macro.
|
|
|
d15d15 |
|
|
|
d15d15 |
Remove the "ISCSI_CHAP_AUTH_DATA.OutChallengeLength" field, which is
|
|
|
d15d15 |
superfluous too.
|
|
|
d15d15 |
|
|
|
d15d15 |
Most importantly, explain in a new comment *why* tying the challenge size
|
|
|
d15d15 |
to the digest size (ISCSI_CHAP_RSP_LEN) has always made sense. (See also
|
|
|
d15d15 |
Linux kernel commit 19f5f88ed779, "scsi: target: iscsi: tie the challenge
|
|
|
d15d15 |
length to the hash digest size", 2019-11-06.) For sure, the motivation
|
|
|
d15d15 |
that the new comment now explains has always been there, and has always
|
|
|
d15d15 |
been the same, for IScsiDxe; it's just that now we spell it out too.
|
|
|
d15d15 |
|
|
|
d15d15 |
No change in peer-visible behavior.
|
|
|
d15d15 |
|
|
|
d15d15 |
Cc: Jiaxin Wu <jiaxin.wu@intel.com>
|
|
|
d15d15 |
Cc: Maciej Rabeda <maciej.rabeda@linux.intel.com>
|
|
|
d15d15 |
Cc: Philippe Mathieu-Daudé <philmd@redhat.com>
|
|
|
d15d15 |
Cc: Siyuan Fu <siyuan.fu@intel.com>
|
|
|
d15d15 |
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=3356
|
|
|
d15d15 |
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
|
|
|
d15d15 |
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
|
|
|
d15d15 |
Reviewed-by: Maciej Rabeda <maciej.rabeda@linux.intel.com>
|
|
|
d15d15 |
Upstream: https://bugzilla.tianocore.org/show_bug.cgi?id=3356, c#17...c#22
|
|
|
d15d15 |
---
|
|
|
d15d15 |
NetworkPkg/IScsiDxe/IScsiCHAP.c | 3 +--
|
|
|
d15d15 |
NetworkPkg/IScsiDxe/IScsiCHAP.h | 9 ++++++---
|
|
|
d15d15 |
2 files changed, 7 insertions(+), 5 deletions(-)
|
|
|
d15d15 |
|
|
|
d15d15 |
diff --git a/NetworkPkg/IScsiDxe/IScsiCHAP.c b/NetworkPkg/IScsiDxe/IScsiCHAP.c
|
|
|
d15d15 |
index df3c2eb120..9e192ce292 100644
|
|
|
d15d15 |
--- a/NetworkPkg/IScsiDxe/IScsiCHAP.c
|
|
|
d15d15 |
+++ b/NetworkPkg/IScsiDxe/IScsiCHAP.c
|
|
|
d15d15 |
@@ -122,7 +122,7 @@ IScsiCHAPAuthTarget (
|
|
|
d15d15 |
AuthData->AuthConfig->ReverseCHAPSecret,
|
|
|
d15d15 |
SecretSize,
|
|
|
d15d15 |
AuthData->OutChallenge,
|
|
|
d15d15 |
- AuthData->OutChallengeLength,
|
|
|
d15d15 |
+ ISCSI_CHAP_RSP_LEN, // ChallengeLength
|
|
|
d15d15 |
VerifyRsp
|
|
|
d15d15 |
);
|
|
|
d15d15 |
|
|
|
d15d15 |
@@ -490,7 +490,6 @@ IScsiCHAPToSendReq (
|
|
|
d15d15 |
// CHAP_C=<C>
|
|
|
d15d15 |
//
|
|
|
d15d15 |
IScsiGenRandom ((UINT8 *) AuthData->OutChallenge, ISCSI_CHAP_RSP_LEN);
|
|
|
d15d15 |
- AuthData->OutChallengeLength = ISCSI_CHAP_RSP_LEN;
|
|
|
d15d15 |
IScsiBinToHex (
|
|
|
d15d15 |
(UINT8 *) AuthData->OutChallenge,
|
|
|
d15d15 |
ISCSI_CHAP_RSP_LEN,
|
|
|
d15d15 |
diff --git a/NetworkPkg/IScsiDxe/IScsiCHAP.h b/NetworkPkg/IScsiDxe/IScsiCHAP.h
|
|
|
d15d15 |
index 1fc1d96ea3..35d5d6ec29 100644
|
|
|
d15d15 |
--- a/NetworkPkg/IScsiDxe/IScsiCHAP.h
|
|
|
d15d15 |
+++ b/NetworkPkg/IScsiDxe/IScsiCHAP.h
|
|
|
d15d15 |
@@ -19,7 +19,6 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
|
|
|
d15d15 |
|
|
|
d15d15 |
#define ISCSI_CHAP_ALGORITHM_MD5 5
|
|
|
d15d15 |
|
|
|
d15d15 |
-#define ISCSI_CHAP_AUTH_MAX_LEN 1024
|
|
|
d15d15 |
///
|
|
|
d15d15 |
/// MD5_HASHSIZE
|
|
|
d15d15 |
///
|
|
|
d15d15 |
@@ -59,9 +58,13 @@ typedef struct _ISCSI_CHAP_AUTH_DATA {
|
|
|
d15d15 |
//
|
|
|
d15d15 |
// Auth-data to be sent out for mutual authentication.
|
|
|
d15d15 |
//
|
|
|
d15d15 |
+ // While the challenge size is technically independent of the hashing
|
|
|
d15d15 |
+ // algorithm, it is good practice to avoid hashing *fewer bytes* than the
|
|
|
d15d15 |
+ // digest size. In other words, it's good practice to feed *at least as many
|
|
|
d15d15 |
+ // bytes* to the hashing algorithm as the hashing algorithm will output.
|
|
|
d15d15 |
+ //
|
|
|
d15d15 |
UINT32 OutIdentifier;
|
|
|
d15d15 |
- UINT8 OutChallenge[ISCSI_CHAP_AUTH_MAX_LEN];
|
|
|
d15d15 |
- UINT32 OutChallengeLength;
|
|
|
d15d15 |
+ UINT8 OutChallenge[ISCSI_CHAP_RSP_LEN];
|
|
|
d15d15 |
} ISCSI_CHAP_AUTH_DATA;
|
|
|
d15d15 |
|
|
|
d15d15 |
/**
|
|
|
d15d15 |
--
|
|
|
d15d15 |
2.27.0
|
|
|
d15d15 |
|