Re: [PATCH 3/6] NetworkPkg/IScsiDxe: distinguish "maximum" and "selected" CHAP digest sizes

["Philippe Mathieu-Daudé" <philmd@redhat.com>]

Hi Laszlo,

On 6/8/21 3:06 PM, Laszlo Ersek wrote:
> IScsiDxe uses the ISCSI_CHAP_RSP_LEN macro for expressing the size of the
> digest (16) that it solely supports at this point (MD5).
> ISCSI_CHAP_RSP_LEN is used for both (a) *allocating* digest-related
> buffers (binary buffers and hex encodings alike), and (b) *processing*
> binary digest buffers (comparing them, filling them, reading them).
> 
> In preparation for adding other hash algorithms, split purpose (a) from
> purpose (b). For purpose (a) -- buffer allocation --, introduce
> ISCSI_CHAP_MAX_DIGEST_SIZE. For purpose (b) -- processing --, rely on
> MD5_DIGEST_SIZE from <BaseCryptLib.h>.

Matter of taste probably, I'd rather see this patch split in 2, as you
identified. (b) first then (a). Regardless:
Reviewed-by: Philippe Mathieu-Daude <philmd@redhat.com>

> Distinguishing these purposes is justified because purpose (b) --
> processing -- must depend on the hashing algorithm negotiated between
> initiator and target, while for purpose (a) -- allocation --, using the
> maximum supported digest size is suitable. For now, because only MD5 is
> supported, introduce ISCSI_CHAP_MAX_DIGEST_SIZE *as* MD5_DIGEST_SIZE.
> 
> Note that the argument for using the digest size as the size of the
> outgoing challenge (in case mutual authentication is desired by the
> initiator) remains in place. Because of this, the above two purposes are
> distinguished for the "ISCSI_CHAP_AUTH_DATA.OutChallenge" field as well.
> 
> This patch is functionally a no-op, just yet.
> 
> 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 | 17 +++++++++------
>  NetworkPkg/IScsiDxe/IScsiCHAP.c | 22 ++++++++++----------
>  2 files changed, 22 insertions(+), 17 deletions(-)