From: "Maciej Rabeda" <maciej.rabeda@linux.intel.com>
To: devel@edk2.groups.io, lersek@redhat.com
Cc: "Jiaxin Wu" <jiaxin.wu@intel.com>,
"Philippe Mathieu-Daudé" <philmd@redhat.com>,
"Siyuan Fu" <siyuan.fu@intel.com>
Subject: Re: [edk2-devel] [PATCH 4/6] NetworkPkg/IScsiDxe: support multiple hash algorithms for CHAP
Date: Fri, 25 Jun 2021 16:56:56 +0200 [thread overview]
Message-ID: <b12221f9-a569-a1d7-68fa-5994806a132e@linux.intel.com> (raw)
In-Reply-To: <518cfc75-7816-c8e1-21f6-5ade8cbc9b39@redhat.com>
More comments inline.
On 22-Jun-21 17:57, Laszlo Ersek wrote:
> On 06/11/21 13:54, Rabeda, Maciej wrote:
>> Laszlo,
>>
>> Comments inline.
>>
>> On 08-Jun-21 15:06, Laszlo Ersek wrote:
>>> Introduce the "mChapHash" table, containing the hash algorithms supported
>>> for CHAP. Hash algos listed at the beginning of the table are
>>> preferred by
>>> the initiator.
>>>
>>> In ISCSI_CHAP_STEP_ONE, send such a CHAP_A value that is the
>>> comma-separated, ordered list of algorithm identifiers from "mChapHash".
>>> Pre-format this value string at driver startup, in the new function
>>> IScsiCHAPInitHashList().
>>>
>>> (In IScsiCHAPInitHashList(), also enforce that every hash algo's digest
>>> size fit into ISCSI_CHAP_MAX_DIGEST_SIZE, as the latter controls the
>>> digest, outgoing challenge, and hex *allocations*.)
>>>
>>> In ISCSI_CHAP_STEP_TWO, allow the target to select one of the offered
>>> hash
>>> algorithms, and remember the selection for the later steps. For
>>> ISCSI_CHAP_STEP_THREE, hash the challenge from the target with the
>>> selected hash algo.
>>>
>>> In ISCSI_CHAP_STEP_THREE, send the correctly sized digest to the target.
>>> If the initiator wants mutual authentication, then generate a challenge
>>> with as many bytes as the target's digest will have, in
>>> ISCSI_CHAP_STEP_FOUR.
>>>
>>> In ISCSI_CHAP_STEP_FOUR (i.e., when mutual authentication is required by
>>> the initiator), verify the target's response (digest) with the selected
>>> algorithm.
>>>
>>> Clear the selected hash algorithm before every login (remember that in
>>> IScsiDxe, every login is a leading login).
>>>
>>> There is no peer-observable change from this patch, as it only reworks
>>> the
>>> current MD5 support into the new internal representation.
>>>
>>> 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=3355
>>> Signed-off-by: Laszlo Ersek <lersek@redhat.com>
>>> ---
>>> NetworkPkg/IScsiDxe/IScsiCHAP.h | 55 +++++++
>>> NetworkPkg/IScsiDxe/IScsiCHAP.c | 158 +++++++++++++++++---
>>> NetworkPkg/IScsiDxe/IScsiDriver.c | 2 +
>>> NetworkPkg/IScsiDxe/IScsiProto.c | 3 +
>>> 4 files changed, 195 insertions(+), 23 deletions(-)
>>>
>>> diff --git a/NetworkPkg/IScsiDxe/IScsiCHAP.h
>>> b/NetworkPkg/IScsiDxe/IScsiCHAP.h
>>> index b8811b7580f0..1e5cc0b287ed 100644
>>> --- a/NetworkPkg/IScsiDxe/IScsiCHAP.h
>>> +++ b/NetworkPkg/IScsiDxe/IScsiCHAP.h
>>> @@ -31,47 +31,91 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
>>> #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];
>>> } ISCSI_CHAP_AUTH_CONFIG_NVDATA;
>>> #pragma pack()
>>> +//
>>> +// Typedefs for collecting sets of hash APIs from BaseCryptLib.
>>> +//
>>> +typedef
>>> +UINTN
>>> +(EFIAPI *CHAP_HASH_GET_CONTEXT_SIZE) (
>>> + VOID
>>> + );
>>> +
>>> +typedef
>>> +BOOLEAN
>>> +(EFIAPI *CHAP_HASH_INIT) (
>>> + OUT VOID *Context
>>> + );
>>> +
>>> +typedef
>>> +BOOLEAN
>>> +(EFIAPI *CHAP_HASH_UPDATE) (
>>> + IN OUT VOID *Context,
>>> + IN CONST VOID *Data,
>>> + IN UINTN DataSize
>>> + );
>>> +
>>> +typedef
>>> +BOOLEAN
>>> +(EFIAPI *CHAP_HASH_FINAL) (
>>> + IN OUT VOID *Context,
>>> + OUT UINT8 *HashValue
>>> + );
>>> +
>>> +typedef struct {
>>> + UINT8 Algorithm; //
>>> ISCSI_CHAP_ALGORITHM_*, CHAP_A
>> Any chance to define an enum type for Algorithm? IMHO it brings more
>> meaning to that number and limits the set of values it is expected to have.
> I picked UINT8 intentionally; I wanted to stay close to the definition at
>
> https://www.iana.org/assignments/ppp-numbers/ppp-numbers.xhtml#ppp-numbers-9
>
> which says, "A one octet field". (Hence the reference to "CHAP_A" in the
> comment as well.)
>
> If we want an enum here, then I need to prepend a patch, for first
> replacing the existent ISCSI_CHAP_ALGORITHM_MD5 macro with an enum type
> / constant.
>
> I still like UINT8 for this, but if you strongly prefer the enum, I can
> certainly do it. Please advise :)
Okay, let's stick to UINT8 based on IANA definition.
>
>
>>> + UINT32 DigestSize;
>>> + CHAP_HASH_GET_CONTEXT_SIZE GetContextSize;
>>> + CHAP_HASH_INIT Init;
>>> + CHAP_HASH_UPDATE Update;
>>> + CHAP_HASH_FINAL Final;
>>> +} CHAP_HASH;
>>> +
>>> ///
>>> /// ISCSI CHAP Authentication Data
>>> ///
>>> typedef struct _ISCSI_CHAP_AUTH_DATA {
>>> ISCSI_CHAP_AUTH_CONFIG_NVDATA *AuthConfig;
>>> UINT32 InIdentifier;
>>> UINT8 InChallenge[1024];
>>> UINT32 InChallengeLength;
>>> //
>>> + // The hash algorithm (CHAP_A) that the target selects in
>>> + // ISCSI_CHAP_STEP_TWO.
>>> + //
>>> + CONST CHAP_HASH *Hash;
>>> + //
>>> // Calculated CHAP Response (CHAP_R) value.
>>> //
>>> UINT8
>>> CHAPResponse[ISCSI_CHAP_MAX_DIGEST_SIZE];
>>> //
>>> // 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_MAX_DIGEST_SIZE];
>>> } ISCSI_CHAP_AUTH_DATA;
>>> /**
>>> This function checks the received iSCSI Login Response during the
>>> security
>>> negotiation stage.
>>> @@ -92,20 +136,31 @@ IScsiCHAPOnRspReceived (
>>> This function fills the CHAP authentication information into the
>>> login PDU
>>> during the security negotiation stage in the iSCSI connection login.
>>> @param[in] Conn The iSCSI connection.
>>> @param[in, out] Pdu The PDU to send out.
>>> @retval EFI_SUCCESS All check passed and the
>>> phase-related CHAP
>>> authentication info is filled into
>>> the iSCSI
>>> PDU.
>>> @retval EFI_OUT_OF_RESOURCES Failed to allocate memory.
>>> @retval EFI_PROTOCOL_ERROR Some kind of protocol error occurred.
>>> **/
>>> EFI_STATUS
>>> IScsiCHAPToSendReq (
>>> IN ISCSI_CONNECTION *Conn,
>>> IN OUT NET_BUF *Pdu
>>> );
>>> +/**
>>> + Initialize the CHAP_A=<A1,A2...> *value* string for the entire
>>> driver, to be
>>> + sent by the initiator in ISCSI_CHAP_STEP_ONE.
>>> +
>>> + This function sanity-checks the internal table of supported CHAP
>>> hashing
>>> + algorithms, as well.
>>> +**/
>>> +VOID
>>> +IScsiCHAPInitHashList (
>>> + VOID
>>> + );
>>> #endif
>>> diff --git a/NetworkPkg/IScsiDxe/IScsiCHAP.c
>>> b/NetworkPkg/IScsiDxe/IScsiCHAP.c
>>> index 744824e63d23..f02ada6444ce 100644
>>> --- a/NetworkPkg/IScsiDxe/IScsiCHAP.c
>>> +++ b/NetworkPkg/IScsiDxe/IScsiCHAP.c
>>> @@ -1,148 +1,194 @@
>>> /** @file
>>> This file is for Challenge-Handshake Authentication Protocol (CHAP)
>>> Configuration.
>>> Copyright (c) 2004 - 2018, Intel Corporation. All rights
>>> reserved.<BR>
>>> SPDX-License-Identifier: BSD-2-Clause-Patent
>>> **/
>>> #include "IScsiImpl.h"
>>> +//
>>> +// Supported CHAP hash algorithms, mapped to sets of BaseCryptLib
>>> APIs and
>>> +// macros. CHAP_HASH structures at lower subscripts in the array are
>>> preferred
>>> +// by the initiator.
>>> +//
>>> +STATIC CONST CHAP_HASH mChapHash[] = {
>>> + {
>>> + ISCSI_CHAP_ALGORITHM_MD5,
>>> + MD5_DIGEST_SIZE,
>>> + Md5GetContextSize,
>>> + Md5Init,
>>> + Md5Update,
>>> + Md5Final
>>> + },
>>> +};
>>> +
>>> +//
>>> +// Ordered list of mChapHash[*].Algorithm values. It is formatted for
>>> the
>>> +// CHAP_A=<A1,A2...> value string, by the IScsiCHAPInitHashList()
>>> function. It
>>> +// is sent by the initiator in ISCSI_CHAP_STEP_ONE.
>>> +//
>>> +STATIC CHAR8 mChapHashListString[
>>> + 3 + // UINT8
>>> identifier in
>>> + // decimal
>>> + (1 + 3) * (ARRAY_SIZE (mChapHash) - 1) + // comma
>>> prepended for
>>> + // entries
>>> after the
>>> + // first
>>> + 1 + // extra
>>> character for
>>> + //
>>> AsciiSPrint()
>>> + //
>>> truncation check
>>> + 1 //
>>> terminating NUL
>>> + ];
>>> +
>>> /**
>>> Initiator calculates its own expected hash value.
>>> @param[in] ChapIdentifier iSCSI CHAP identifier sent by
>>> authenticator.
>>> @param[in] ChapSecret iSCSI CHAP secret of the
>>> authenticator.
>>> @param[in] SecretLength The length of iSCSI CHAP secret.
>>> @param[in] ChapChallenge The challenge message sent by
>>> authenticator.
>>> @param[in] ChallengeLength The length of iSCSI CHAP challenge
>>> message.
>>> + @param[in] Hash Pointer to the CHAP_HASH structure
>>> that
>>> + determines the hashing algorithm to
>>> use. The
>>> + caller is responsible for making
>>> Hash point
>>> + to an "mChapHash" element.
>>> @param[out] ChapResponse The calculation of the expected
>>> hash value.
>>> @retval EFI_SUCCESS The expected hash value was
>>> calculatedly
>>> successfully.
>>> @retval EFI_PROTOCOL_ERROR The length of the secret should be
>>> at least
>>> the length of the hash value for
>>> the hashing
>>> algorithm chosen.
>>> - @retval EFI_PROTOCOL_ERROR MD5 hash operation fail.
>>> - @retval EFI_OUT_OF_RESOURCES Fail to allocate resource to
>>> complete MD5.
>>> + @retval EFI_PROTOCOL_ERROR Hash operation fails.
>>> + @retval EFI_OUT_OF_RESOURCES Failure to allocate resource to
>>> complete
>>> + hashing.
>>> **/
>>> EFI_STATUS
>>> IScsiCHAPCalculateResponse (
>>> IN UINT32 ChapIdentifier,
>>> IN CHAR8 *ChapSecret,
>>> IN UINT32 SecretLength,
>>> IN UINT8 *ChapChallenge,
>>> IN UINT32 ChallengeLength,
>>> + IN CONST CHAP_HASH *Hash,
>>> OUT UINT8 *ChapResponse
>>> )
>>> {
>>> - UINTN Md5ContextSize;
>>> - VOID *Md5Ctx;
>>> + UINTN ContextSize;
>>> + VOID *Ctx;
>>> CHAR8 IdByte[1];
>>> EFI_STATUS Status;
>>> if (SecretLength < ISCSI_CHAP_SECRET_MIN_LEN) {
>>> return EFI_PROTOCOL_ERROR;
>>> }
>>> - Md5ContextSize = Md5GetContextSize ();
>>> - Md5Ctx = AllocatePool (Md5ContextSize);
>>> - if (Md5Ctx == NULL) {
>>> + ASSERT (Hash != NULL);
>>> +
>>> + ContextSize = Hash->GetContextSize ();
>>> + Ctx = AllocatePool (ContextSize);
>>> + if (Ctx == NULL) {
>>> return EFI_OUT_OF_RESOURCES;
>>> }
>>> Status = EFI_PROTOCOL_ERROR;
>>> - if (!Md5Init (Md5Ctx)) {
>>> + if (!Hash->Init (Ctx)) {
>>> goto Exit;
>>> }
>>> //
>>> // Hash Identifier - Only calculate 1 byte data (RFC1994)
>>> //
>>> IdByte[0] = (CHAR8) ChapIdentifier;
>>> - if (!Md5Update (Md5Ctx, IdByte, 1)) {
>>> + if (!Hash->Update (Ctx, IdByte, 1)) {
>>> goto Exit;
>>> }
>>> //
>>> // Hash Secret
>>> //
>>> - if (!Md5Update (Md5Ctx, ChapSecret, SecretLength)) {
>>> + if (!Hash->Update (Ctx, ChapSecret, SecretLength)) {
>>> goto Exit;
>>> }
>>> //
>>> // Hash Challenge received from Target
>>> //
>>> - if (!Md5Update (Md5Ctx, ChapChallenge, ChallengeLength)) {
>>> + if (!Hash->Update (Ctx, ChapChallenge, ChallengeLength)) {
>>> goto Exit;
>>> }
>>> - if (Md5Final (Md5Ctx, ChapResponse)) {
>>> + if (Hash->Final (Ctx, ChapResponse)) {
>>> Status = EFI_SUCCESS;
>>> }
>>> Exit:
>>> - FreePool (Md5Ctx);
>>> + FreePool (Ctx);
>>> return Status;
>>> }
>>> /**
>>> The initiator checks the CHAP response replied by target against
>>> its own
>>> calculation of the expected hash value.
>>> @param[in] AuthData iSCSI CHAP authentication data.
>>> @param[in] TargetResponse The response from target.
>>> @retval EFI_SUCCESS The response from target passed
>>> authentication.
>>> @retval EFI_SECURITY_VIOLATION The response from target was not
>>> expected
>>> value.
>>> @retval Others Other errors as indicated.
>>> **/
>>> EFI_STATUS
>>> IScsiCHAPAuthTarget (
>>> IN ISCSI_CHAP_AUTH_DATA *AuthData,
>>> IN UINT8 *TargetResponse
>>> )
>>> {
>>> EFI_STATUS Status;
>>> UINT32 SecretSize;
>>> UINT8 VerifyRsp[ISCSI_CHAP_MAX_DIGEST_SIZE];
>>> Status = EFI_SUCCESS;
>>> SecretSize = (UINT32) AsciiStrLen
>>> (AuthData->AuthConfig->ReverseCHAPSecret);
>>> +
>>> + ASSERT (AuthData->Hash != NULL);
>>> +
>>> Status = IScsiCHAPCalculateResponse (
>>> AuthData->OutIdentifier,
>>> AuthData->AuthConfig->ReverseCHAPSecret,
>>> SecretSize,
>>> AuthData->OutChallenge,
>>> - MD5_DIGEST_SIZE, // ChallengeLength
>>> + AuthData->Hash->DigestSize, // ChallengeLength
>>> + AuthData->Hash,
>>> VerifyRsp
>>> );
>>> - if (CompareMem (VerifyRsp, TargetResponse, MD5_DIGEST_SIZE) != 0) {
>>> + if (CompareMem (VerifyRsp, TargetResponse,
>>> + AuthData->Hash->DigestSize) != 0) {
>>> Status = EFI_SECURITY_VIOLATION;
>>> }
>> Either break like regular function call or leave it in a single line for
>> readability, since UEFI coding standard section 5.1.1 allows for lines
>> up to 120.
>> I opt for the latter, since param break will look ugly, but if it looks
>> better in your editor than a line over 80 chars in size - go for it.
> My mistake, sorry -- I forgot that the "condensed style" that we
> actually prefer in ArmVirtPkg and OvmfPkg is not accepted in other packages.
Making it the other way around. Apart from historical reasons (EDK2
coding style evolving) - why is a different style accepted in other
packages? We have a EDK2 coding style document for a reason.
If it is not perfect, it does not suit our needs or simply hurts our
eyes (which 'param per line' in if statements does to me), we can try to
change it to the "condensed style".
> So, I will insert the line breaks rigorously -- as explained elsewhere,
> my strict 80 chars/line limit is not due to my editor, but to my desire
> to fit two source code windows side-by-side, combined with my
> not-so-great eyesight, and inability to use multiple monitors at the
> same time (or one huge monitor).
>
> I'll fix up the other instances of this same style issue as well.
>
> Thanks!
> Laszlo
>
>
>>> 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.
>>> @retval EFI_PROTOCOL_ERROR Some kind of protocol error occurred.
>>> @retval Others Other errors as indicated.
>>> **/
>>> @@ -150,38 +196,39 @@ EFI_STATUS
>>> IScsiCHAPOnRspReceived (
>>> IN ISCSI_CONNECTION *Conn
>>> )
>>> {
>>> EFI_STATUS Status;
>>> ISCSI_SESSION *Session;
>>> ISCSI_CHAP_AUTH_DATA *AuthData;
>>> CHAR8 *Value;
>>> UINT8 *Data;
>>> UINT32 Len;
>>> LIST_ENTRY *KeyValueList;
>>> UINTN Algorithm;
>>> CHAR8 *Identifier;
>>> CHAR8 *Challenge;
>>> CHAR8 *Name;
>>> CHAR8 *Response;
>>> UINT8 TargetRsp[ISCSI_CHAP_MAX_DIGEST_SIZE];
>>> UINT32 RspLen;
>>> UINTN Result;
>>> + UINTN HashIndex;
>>> ASSERT (Conn->CurrentStage == ISCSI_SECURITY_NEGOTIATION);
>>> ASSERT (Conn->RspQue.BufNum != 0);
>>> Session = Conn->Session;
>>> AuthData = &Session->AuthData.CHAP;
>>> Len = Conn->RspQue.BufSize;
>>> Data = AllocateZeroPool (Len);
>>> if (Data == NULL) {
>>> return EFI_OUT_OF_RESOURCES;
>>> }
>>> //
>>> // Copy the data in case the data spans over multiple PDUs.
>>> //
>>> NetbufQueCopy (&Conn->RspQue, 0, Len, Data);
>>> //
>>> // Build the key-value list from the data segment of the Login
>>> Response.
>>> //
>>> @@ -241,44 +288,54 @@ IScsiCHAPOnRspReceived (
>>> // Transit to CHAP step one.
>>> //
>>> Conn->AuthStep = ISCSI_CHAP_STEP_ONE;
>>> Status = EFI_SUCCESS;
>>> break;
>>> case ISCSI_CHAP_STEP_TWO:
>>> //
>>> // The Target replies with CHAP_A=<A> CHAP_I=<I> CHAP_C=<C>
>>> //
>>> Value = IScsiGetValueByKeyFromList (
>>> KeyValueList,
>>> ISCSI_KEY_CHAP_ALGORITHM
>>> );
>>> if (Value == NULL) {
>>> goto ON_EXIT;
>>> }
>>> Algorithm = IScsiNetNtoi (Value);
>>> - if (Algorithm != ISCSI_CHAP_ALGORITHM_MD5) {
>>> + for (HashIndex = 0; HashIndex < ARRAY_SIZE (mChapHash);
>>> HashIndex++) {
>>> + if (Algorithm == mChapHash[HashIndex].Algorithm) {
>>> + break;
>>> + }
>>> + }
>>> + if (HashIndex == ARRAY_SIZE (mChapHash)) {
>>> //
>>> // Unsupported algorithm is chosen by target.
>>> //
>>> goto ON_EXIT;
>>> }
>>> + //
>>> + // Remember the target's chosen hash algorithm.
>>> + //
>>> + ASSERT (AuthData->Hash == NULL);
>>> + AuthData->Hash = &mChapHash[HashIndex];
>>> Identifier = IScsiGetValueByKeyFromList (
>>> KeyValueList,
>>> ISCSI_KEY_CHAP_IDENTIFIER
>>> );
>>> if (Identifier == NULL) {
>>> goto ON_EXIT;
>>> }
>>> Challenge = IScsiGetValueByKeyFromList (
>>> KeyValueList,
>>> ISCSI_KEY_CHAP_CHALLENGE
>>> );
>>> if (Challenge == NULL) {
>>> goto ON_EXIT;
>>> }
>>> //
>>> // Process the CHAP identifier and CHAP Challenge from Target.
>>> // Calculate Response value.
>>> @@ -289,76 +346,78 @@ IScsiCHAPOnRspReceived (
>>> }
>>> AuthData->InIdentifier = (UINT32) Result;
>>> AuthData->InChallengeLength = (UINT32) sizeof
>>> (AuthData->InChallenge);
>>> Status = IScsiHexToBin (
>>> (UINT8 *) AuthData->InChallenge,
>>> &AuthData->InChallengeLength,
>>> Challenge
>>> );
>>> if (EFI_ERROR (Status)) {
>>> Status = EFI_PROTOCOL_ERROR;
>>> goto ON_EXIT;
>>> }
>>> Status = IScsiCHAPCalculateResponse (
>>> AuthData->InIdentifier,
>>> AuthData->AuthConfig->CHAPSecret,
>>> (UINT32) AsciiStrLen (AuthData->AuthConfig->CHAPSecret),
>>> AuthData->InChallenge,
>>> AuthData->InChallengeLength,
>>> + AuthData->Hash,
>>> AuthData->CHAPResponse
>>> );
>>> //
>>> // Transit to next step.
>>> //
>>> Conn->AuthStep = ISCSI_CHAP_STEP_THREE;
>>> break;
>>> case ISCSI_CHAP_STEP_THREE:
>>> //
>>> // One way CHAP authentication and the target would like to
>>> // authenticate us.
>>> //
>>> Status = EFI_SUCCESS;
>>> break;
>>> case ISCSI_CHAP_STEP_FOUR:
>>> ASSERT (AuthData->AuthConfig->CHAPType == ISCSI_CHAP_MUTUAL);
>>> //
>>> // The forth step, CHAP_N=<N> CHAP_R=<R> is received from Target.
>>> //
>>> Name = IScsiGetValueByKeyFromList (KeyValueList,
>>> ISCSI_KEY_CHAP_NAME);
>>> if (Name == NULL) {
>>> goto ON_EXIT;
>>> }
>>> Response = IScsiGetValueByKeyFromList (
>>> KeyValueList,
>>> ISCSI_KEY_CHAP_RESPONSE
>>> );
>>> if (Response == NULL) {
>>> goto ON_EXIT;
>>> }
>>> - RspLen = MD5_DIGEST_SIZE;
>>> + ASSERT (AuthData->Hash != NULL);
>>> + RspLen = AuthData->Hash->DigestSize;
>>> Status = IScsiHexToBin (TargetRsp, &RspLen, Response);
>>> - if (EFI_ERROR (Status) || RspLen != MD5_DIGEST_SIZE) {
>>> + if (EFI_ERROR (Status) || RspLen != AuthData->Hash->DigestSize) {
>>> Status = EFI_PROTOCOL_ERROR;
>>> goto ON_EXIT;
>>> }
>>> //
>>> // Check the CHAP Name and Response replied by Target.
>>> //
>>> Status = IScsiCHAPAuthTarget (AuthData, TargetRsp);
>>> break;
>>> default:
>>> break;
>>> }
>>> ON_EXIT:
>>> if (KeyValueList != NULL) {
>>> IScsiFreeKeyValueList (KeyValueList);
>>> }
>>> @@ -442,88 +501,141 @@ IScsiCHAPToSendReq (
>>> Session->ConfigData->SessionConfigData.TargetName
>>> );
>>> if (Session->AuthType == ISCSI_AUTH_TYPE_NONE) {
>>> Value = ISCSI_KEY_VALUE_NONE;
>>> ISCSI_SET_FLAG (LoginReq, ISCSI_LOGIN_REQ_PDU_FLAG_TRANSIT);
>>> } else {
>>> Value = ISCSI_AUTH_METHOD_CHAP;
>>> }
>>> IScsiAddKeyValuePair (Pdu, ISCSI_KEY_AUTH_METHOD, Value);
>>> break;
>>> case ISCSI_CHAP_STEP_ONE:
>>> //
>>> // First step, send the Login Request with CHAP_A=<A1,A2...>
>>> key-value
>>> // pair.
>>> //
>>> - AsciiSPrint (ValueStr, sizeof (ValueStr), "%d",
>>> ISCSI_CHAP_ALGORITHM_MD5);
>>> - IScsiAddKeyValuePair (Pdu, ISCSI_KEY_CHAP_ALGORITHM, ValueStr);
>>> + IScsiAddKeyValuePair (Pdu, ISCSI_KEY_CHAP_ALGORITHM,
>>> mChapHashListString);
>>> Conn->AuthStep = ISCSI_CHAP_STEP_TWO;
>>> break;
>>> case ISCSI_CHAP_STEP_THREE:
>>> //
>>> // Third step, send the Login Request with CHAP_N=<N> CHAP_R=<R> or
>>> // CHAP_N=<N> CHAP_R=<R> CHAP_I=<I> CHAP_C=<C> if target
>>> authentication is
>>> // required too.
>>> //
>>> // CHAP_N=<N>
>>> //
>>> IScsiAddKeyValuePair (
>>> Pdu,
>>> ISCSI_KEY_CHAP_NAME,
>>> (CHAR8 *) &AuthData->AuthConfig->CHAPName
>>> );
>>> //
>>> // CHAP_R=<R>
>>> //
>>> + ASSERT (AuthData->Hash != NULL);
>>> BinToHexStatus = IScsiBinToHex (
>>> (UINT8 *) AuthData->CHAPResponse,
>>> - MD5_DIGEST_SIZE,
>>> + AuthData->Hash->DigestSize,
>>> Response,
>>> &RspLen
>>> );
>>> ASSERT_EFI_ERROR (BinToHexStatus);
>>> 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,
>>> MD5_DIGEST_SIZE);
>>> + IScsiGenRandom ((UINT8 *) AuthData->OutChallenge,
>>> + AuthData->Hash->DigestSize);
>> Coding standard. Parameter break.
>>> BinToHexStatus = IScsiBinToHex (
>>> (UINT8 *) AuthData->OutChallenge,
>>> - MD5_DIGEST_SIZE,
>>> + AuthData->Hash->DigestSize,
>>> Challenge,
>>> &ChallengeLen
>>> );
>>> ASSERT_EFI_ERROR (BinToHexStatus);
>>> 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;
>>> }
>>> FreePool (Response);
>>> FreePool (Challenge);
>>> return Status;
>>> }
>>> +
>>> +/**
>>> + Initialize the CHAP_A=<A1,A2...> *value* string for the entire
>>> driver, to be
>>> + sent by the initiator in ISCSI_CHAP_STEP_ONE.
>>> +
>>> + This function sanity-checks the internal table of supported CHAP
>>> hashing
>>> + algorithms, as well.
>>> +**/
>>> +VOID
>>> +IScsiCHAPInitHashList (
>>> + VOID
>>> + )
>>> +{
>>> + CHAR8 *Position;
>>> + UINTN Left;
>>> + UINTN HashIndex;
>>> + CONST CHAP_HASH *Hash;
>>> + UINTN Printed;
>>> +
>>> + Position = mChapHashListString;
>>> + Left = sizeof (mChapHashListString);
>>> + for (HashIndex = 0; HashIndex < ARRAY_SIZE (mChapHash); HashIndex++) {
>>> + Hash = &mChapHash[HashIndex];
>>> +
>>> + //
>>> + // Format the next hash identifier.
>>> + //
>>> + // Assert that we can format at least one non-NUL character, i.e.
>>> that we
>>> + // can progress. Truncation is checked after printing.
>>> + //
>>> + ASSERT (Left >= 2);
>>> + Printed = AsciiSPrint (Position, Left, "%a%d", (HashIndex == 0) ?
>>> "" : ",",
>>> + Hash->Algorithm);
>> Coding standard. Parameter break.
>>> + //
>>> + // There's no way to differentiate between the "buffer filled to
>>> the brim,
>>> + // but not truncated" result and the "truncated" result of
>>> AsciiSPrint().
>>> + // This is why "mChapHashListString" has an extra byte allocated,
>>> and the
>>> + // reason why we use the less-than (rather than the
>>> less-than-or-equal-to)
>>> + // relational operator in the assertion below -- we enforce "no
>>> truncation"
>>> + // by excluding the "completely used up" case too.
>>> + //
>>> + ASSERT (Printed + 1 < Left);
>>> +
>>> + Position += Printed;
>>> + Left -= Printed;
>>> +
>>> + //
>>> + // Sanity-check the digest size for Hash.
>>> + //
>>> + ASSERT (Hash->DigestSize <= ISCSI_CHAP_MAX_DIGEST_SIZE);
>>> + }
>>> +}
>>> diff --git a/NetworkPkg/IScsiDxe/IScsiDriver.c
>>> b/NetworkPkg/IScsiDxe/IScsiDriver.c
>>> index 98b73308c118..485c92972113 100644
>>> --- a/NetworkPkg/IScsiDxe/IScsiDriver.c
>>> +++ b/NetworkPkg/IScsiDxe/IScsiDriver.c
>>> @@ -1763,38 +1763,40 @@ IScsiDriverEntryPoint (
>>> goto Error1;
>>> }
>>> //
>>> // Install the iSCSI Initiator Name Protocol.
>>> //
>>> Status = gBS->InstallProtocolInterface (
>>> &ImageHandle,
>>> &gEfiIScsiInitiatorNameProtocolGuid,
>>> EFI_NATIVE_INTERFACE,
>>> &gIScsiInitiatorName
>>> );
>>> if (EFI_ERROR (Status)) {
>>> goto Error2;
>>> }
>>> //
>>> // Create the private data structures.
>>> //
>>> + IScsiCHAPInitHashList ();
>>> +
>>> mPrivate = AllocateZeroPool (sizeof (ISCSI_PRIVATE_DATA));
>>> if (mPrivate == NULL) {
>>> Status = EFI_OUT_OF_RESOURCES;
>>> goto Error3;
>>> }
>>> InitializeListHead (&mPrivate->NicInfoList);
>>> InitializeListHead (&mPrivate->AttemptConfigs);
>>> //
>>> // Initialize the configuration form of iSCSI.
>>> //
>>> Status = IScsiConfigFormInit
>>> (gIScsiIp4DriverBinding.DriverBindingHandle);
>>> if (EFI_ERROR (Status)) {
>>> goto Error4;
>>> }
>>> //
>>> // Create the Maximum Attempts.
>>> diff --git a/NetworkPkg/IScsiDxe/IScsiProto.c
>>> b/NetworkPkg/IScsiDxe/IScsiProto.c
>>> index 69d1b39dbb1f..e62736bc041f 100644
>>> --- a/NetworkPkg/IScsiDxe/IScsiProto.c
>>> +++ b/NetworkPkg/IScsiDxe/IScsiProto.c
>>> @@ -416,38 +416,41 @@ IScsiGetIp6NicInfo (
>>> return Status;
>>> }
>>> /**
>>> Re-set any stateful session-level authentication information that
>>> is used by
>>> the leading login / leading connection.
>>> (Note that this driver only supports a single connection per
>>> session -- see
>>> ISCSI_MAX_CONNS_PER_SESSION.)
>>> @param[in,out] Session The iSCSI session.
>>> **/
>>> STATIC
>>> VOID
>>> IScsiSessionResetAuthData (
>>> IN OUT ISCSI_SESSION *Session
>>> )
>>> {
>>> + if (Session->AuthType == ISCSI_AUTH_TYPE_CHAP) {
>>> + Session->AuthData.CHAP.Hash = NULL;
>>> + }
>>> }
>>> /**
>>> Login the iSCSI session.
>>> @param[in] Session The iSCSI session.
>>> @retval EFI_SUCCESS The iSCSI session login procedure
>>> finished.
>>> @retval EFI_OUT_OF_RESOURCES Failed to allocate memory.
>>> @retval EFI_NO_MEDIA There was a media error.
>>> @retval Others Other errors as indicated.
>>> **/
>>> EFI_STATUS
>>> IScsiSessionLogin (
>>> IN ISCSI_SESSION *Session
>>> )
>>> {
>>> EFI_STATUS Status;
>
>
>
>
>
next prev parent reply other threads:[~2021-06-25 14:57 UTC|newest]
Thread overview: 22+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-06-08 13:06 [PATCH 0/6] NetworkPkg/IScsiDxe: support SHA256 in CHAP Laszlo Ersek
2021-06-08 13:06 ` [PATCH 1/6] NetworkPkg/IScsiDxe: re-set session-level authentication state before login Laszlo Ersek
2021-06-11 11:30 ` Maciej Rabeda
2021-06-18 9:45 ` Philippe Mathieu-Daudé
2021-06-08 13:06 ` [PATCH 2/6] NetworkPkg/IScsiDxe: add horizontal whitespace to IScsiCHAP files Laszlo Ersek
2021-06-09 10:35 ` Philippe Mathieu-Daudé
2021-06-11 11:32 ` Maciej Rabeda
2021-06-08 13:06 ` [PATCH 3/6] NetworkPkg/IScsiDxe: distinguish "maximum" and "selected" CHAP digest sizes Laszlo Ersek
2021-06-09 10:43 ` Philippe Mathieu-Daudé
2021-06-09 13:46 ` Laszlo Ersek
2021-06-11 11:38 ` Maciej Rabeda
2021-06-08 13:06 ` [PATCH 4/6] NetworkPkg/IScsiDxe: support multiple hash algorithms for CHAP Laszlo Ersek
2021-06-11 11:54 ` Maciej Rabeda
2021-06-22 15:57 ` Laszlo Ersek
2021-06-25 14:56 ` Maciej Rabeda [this message]
2021-06-28 14:44 ` [edk2-devel] " Laszlo Ersek
2021-06-08 13:06 ` [PATCH 5/6] NetworkPkg/IScsiDxe: support SHA256 in CHAP Laszlo Ersek
2021-06-09 10:37 ` Philippe Mathieu-Daudé
2021-06-11 11:54 ` Maciej Rabeda
2021-06-08 13:06 ` [PATCH 6/6] NetworkPkg: introduce the NETWORK_ISCSI_MD5_ENABLE feature test macro Laszlo Ersek
2021-06-11 11:55 ` Maciej Rabeda
2021-06-17 15:51 ` Philippe Mathieu-Daudé
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-list from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=b12221f9-a569-a1d7-68fa-5994806a132e@linux.intel.com \
--to=devel@edk2.groups.io \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox