[edk2-devel] [PUBLIC edk2 PATCH v2 03/10] NetworkPkg/IScsiDxe: clean up "ISCSI_CHAP_AUTH_DATA.OutChallengeLength"
Laszlo Ersek
lersek at redhat.com
Tue Jun 8 12:12:52 UTC 2021
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 at intel.com>
Cc: Maciej Rabeda <maciej.rabeda at linux.intel.com>
Cc: Philippe Mathieu-Daudé <philmd at redhat.com>
Cc: Siyuan Fu <siyuan.fu at intel.com>
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=3356
Signed-off-by: Laszlo Ersek <lersek at redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd at redhat.com>
Reviewed-by: Maciej Rabeda <maciej.rabeda at linux.intel.com>
---
NetworkPkg/IScsiDxe/IScsiCHAP.h | 9 ++++++---
NetworkPkg/IScsiDxe/IScsiCHAP.c | 3 +--
2 files changed, 7 insertions(+), 5 deletions(-)
diff --git a/NetworkPkg/IScsiDxe/IScsiCHAP.h b/NetworkPkg/IScsiDxe/IScsiCHAP.h
index 1fc1d96ea3f3..35d5d6ec29e3 100644
--- a/NetworkPkg/IScsiDxe/IScsiCHAP.h
+++ b/NetworkPkg/IScsiDxe/IScsiCHAP.h
@@ -3,39 +3,38 @@
Copyright (c) 2004 - 2018, Intel Corporation. All rights reserved.<BR>
SPDX-License-Identifier: BSD-2-Clause-Patent
**/
#ifndef _ISCSI_CHAP_H_
#define _ISCSI_CHAP_H_
#define ISCSI_AUTH_METHOD_CHAP "CHAP"
#define ISCSI_KEY_CHAP_ALGORITHM "CHAP_A"
#define ISCSI_KEY_CHAP_IDENTIFIER "CHAP_I"
#define ISCSI_KEY_CHAP_CHALLENGE "CHAP_C"
#define ISCSI_KEY_CHAP_NAME "CHAP_N"
#define ISCSI_KEY_CHAP_RESPONSE "CHAP_R"
#define ISCSI_CHAP_ALGORITHM_MD5 5
-#define ISCSI_CHAP_AUTH_MAX_LEN 1024
///
/// MD5_HASHSIZE
///
#define ISCSI_CHAP_RSP_LEN 16
#define ISCSI_CHAP_STEP_ONE 1
#define ISCSI_CHAP_STEP_TWO 2
#define ISCSI_CHAP_STEP_THREE 3
#define ISCSI_CHAP_STEP_FOUR 4
#pragma pack(1)
typedef struct _ISCSI_CHAP_AUTH_CONFIG_NVDATA {
UINT8 CHAPType;
CHAR8 CHAPName[ISCSI_CHAP_NAME_STORAGE];
CHAR8 CHAPSecret[ISCSI_CHAP_SECRET_STORAGE];
CHAR8 ReverseCHAPName[ISCSI_CHAP_NAME_STORAGE];
CHAR8 ReverseCHAPSecret[ISCSI_CHAP_SECRET_STORAGE];
@@ -43,41 +42,45 @@ typedef struct _ISCSI_CHAP_AUTH_CONFIG_NVDATA {
#pragma pack()
///
/// ISCSI CHAP Authentication Data
///
typedef struct _ISCSI_CHAP_AUTH_DATA {
ISCSI_CHAP_AUTH_CONFIG_NVDATA *AuthConfig;
UINT32 InIdentifier;
UINT8 InChallenge[1024];
UINT32 InChallengeLength;
//
// Calculated CHAP Response (CHAP_R) value.
//
UINT8 CHAPResponse[ISCSI_CHAP_RSP_LEN];
//
// Auth-data to be sent out for mutual authentication.
//
+ // While the challenge size is technically independent of the hashing
+ // algorithm, it is good practice to avoid hashing *fewer bytes* than the
+ // digest size. In other words, it's good practice to feed *at least as many
+ // bytes* to the hashing algorithm as the hashing algorithm will output.
+ //
UINT32 OutIdentifier;
- UINT8 OutChallenge[ISCSI_CHAP_AUTH_MAX_LEN];
- UINT32 OutChallengeLength;
+ UINT8 OutChallenge[ISCSI_CHAP_RSP_LEN];
} ISCSI_CHAP_AUTH_DATA;
/**
This function checks the received iSCSI Login Response during the security
negotiation stage.
@param[in] Conn The iSCSI connection.
@retval EFI_SUCCESS The Login Response passed the CHAP validation.
@retval EFI_OUT_OF_RESOURCES Failed to allocate memory.
@retval EFI_PROTOCOL_ERROR Some kind of protocol error occurred.
@retval Others Other errors as indicated.
**/
EFI_STATUS
IScsiCHAPOnRspReceived (
IN ISCSI_CONNECTION *Conn
);
/**
diff --git a/NetworkPkg/IScsiDxe/IScsiCHAP.c b/NetworkPkg/IScsiDxe/IScsiCHAP.c
index df3c2eb1200a..9e192ce292e8 100644
--- a/NetworkPkg/IScsiDxe/IScsiCHAP.c
+++ b/NetworkPkg/IScsiDxe/IScsiCHAP.c
@@ -106,39 +106,39 @@ IScsiCHAPCalculateResponse (
**/
EFI_STATUS
IScsiCHAPAuthTarget (
IN ISCSI_CHAP_AUTH_DATA *AuthData,
IN UINT8 *TargetResponse
)
{
EFI_STATUS Status;
UINT32 SecretSize;
UINT8 VerifyRsp[ISCSI_CHAP_RSP_LEN];
Status = EFI_SUCCESS;
SecretSize = (UINT32) AsciiStrLen (AuthData->AuthConfig->ReverseCHAPSecret);
Status = IScsiCHAPCalculateResponse (
AuthData->OutIdentifier,
AuthData->AuthConfig->ReverseCHAPSecret,
SecretSize,
AuthData->OutChallenge,
- AuthData->OutChallengeLength,
+ ISCSI_CHAP_RSP_LEN, // ChallengeLength
VerifyRsp
);
if (CompareMem (VerifyRsp, TargetResponse, ISCSI_CHAP_RSP_LEN) != 0) {
Status = EFI_SECURITY_VIOLATION;
}
return Status;
}
/**
This function checks the received iSCSI Login Response during the security
negotiation stage.
@param[in] Conn The iSCSI connection.
@retval EFI_SUCCESS The Login Response passed the CHAP validation.
@retval EFI_OUT_OF_RESOURCES Failed to allocate memory.
@@ -474,39 +474,38 @@ IScsiCHAPToSendReq (
IScsiBinToHex (
(UINT8 *) AuthData->CHAPResponse,
ISCSI_CHAP_RSP_LEN,
Response,
&RspLen
);
IScsiAddKeyValuePair (Pdu, ISCSI_KEY_CHAP_RESPONSE, Response);
if (AuthData->AuthConfig->CHAPType == ISCSI_CHAP_MUTUAL) {
//
// CHAP_I=<I>
//
IScsiGenRandom ((UINT8 *) &AuthData->OutIdentifier, 1);
AsciiSPrint (ValueStr, sizeof (ValueStr), "%d", AuthData->OutIdentifier);
IScsiAddKeyValuePair (Pdu, ISCSI_KEY_CHAP_IDENTIFIER, ValueStr);
//
// CHAP_C=<C>
//
IScsiGenRandom ((UINT8 *) AuthData->OutChallenge, ISCSI_CHAP_RSP_LEN);
- AuthData->OutChallengeLength = ISCSI_CHAP_RSP_LEN;
IScsiBinToHex (
(UINT8 *) AuthData->OutChallenge,
ISCSI_CHAP_RSP_LEN,
Challenge,
&ChallengeLen
);
IScsiAddKeyValuePair (Pdu, ISCSI_KEY_CHAP_CHALLENGE, Challenge);
Conn->AuthStep = ISCSI_CHAP_STEP_FOUR;
}
//
// Set the stage transition flag.
//
ISCSI_SET_FLAG (LoginReq, ISCSI_LOGIN_REQ_PDU_FLAG_TRANSIT);
break;
default:
Status = EFI_PROTOCOL_ERROR;
break;
--
2.19.1.3.g30247aa5d201
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#76201): https://edk2.groups.io/g/devel/message/76201
Mute This Topic: https://groups.io/mt/83394109/1813853
Group Owner: devel+owner at edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [edk2-devel-archive at redhat.com]
-=-=-=-=-=-=-=-=-=-=-=-
More information about the edk2-devel-archive
mailing list