public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: "Maciej Rabeda" <maciej.rabeda@linux.intel.com>
To: Laszlo Ersek <lersek@redhat.com>,
	edk2-devel-groups-io <devel@edk2.groups.io>
Cc: "Jiaxin Wu" <jiaxin.wu@intel.com>,
	"Philippe Mathieu-Daudé" <philmd@redhat.com>,
	"Siyuan Fu" <siyuan.fu@intel.com>
Subject: Re: [PATCH 4/6] NetworkPkg/IScsiDxe: support multiple hash algorithms for CHAP
Date: Fri, 11 Jun 2021 13:54:22 +0200	[thread overview]
Message-ID: <b6227a00-1fdc-7133-8fd2-cba0a2408a10@linux.intel.com> (raw)
In-Reply-To: <20210608130652.2434-5-lersek@redhat.com>

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.
> +  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.
>   
>     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;


  reply	other threads:[~2021-06-11 11:54 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 [this message]
2021-06-22 15:57     ` Laszlo Ersek
2021-06-25 14:56       ` [edk2-devel] " Maciej Rabeda
2021-06-28 14:44         ` 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=b6227a00-1fdc-7133-8fd2-cba0a2408a10@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