Re: [PATCH 1/2] scsi: target: fix hexadecimal CHAP_I handling

Lee Duncan <[email protected]> Fri, 5 Jun 2026 09:02:54 -0700
Newsgroups org.kernel.vger.target-devel,org.kernel.vger.linux-scsi
Message-ID <CAPj3X_VQerGwtz2MheM9ESFe9KpZHAsy2UceADTHgJeEekyU9A@mail.gmail.com>
On Fri, Jun 5, 2026 at 5:37 AM David Disseldorp <[email protected]> wrote:
>
> A mutual CHAP handshake requires target processing of an initiator-sent
> CHAP_I identifier. The RFC 3720 specification states:
>
>   11.1.4.  Challenge Handshake Authentication Protocol (CHAP)
>   ...
>   CHAP_A=<A> CHAP_I=<I> CHAP_C=<C>
>   ...
>   Where N, (A,A1,A2), I, C, and R are (correspondingly) the Name,
>   Algorithm, Identifier, Challenge, and Response as defined in
>   [RFC1994], N is a text string, A,A1,A2, and I are numbers
>
> CHAP_I parsing currently calls extract_param(), which returns the
> @identifier string (stripped of any 0b/0B or 0x/0X prefix) and a
> @type which indicates DECIMAL, HEX or BASE64 encoding (based on any
> stripped prefix).
>
> Any HEX encoded CHAP_I string is further processed via:
>   ret = kstrtoul(&identifier[2], 0, &id);
> This is incorrect for two reasons:
> * The @identifier string has already been stripped of the 0x/0X prefix,
>   so skipping the first two bytes omits part of the number.
> * The kstrtoul() call specifies a base of 0, which will see
>   &identifier[2] parsed as a decimal, unless a '0x' or (octal) '0' is
>   erroneously present at that offset.
>
> Fix this by passing the (zero-offset) identifier string to kstrtoul()
> along with a base=16 parameter. Also add an explicit error handler for
> BASE64 encoding.
>
> Hex-encoded CHAP_I handling can be testing using the libiscsi EncodedI
> test linked below.
>
> Reported-by: Sashiko (gemini/gemini-3.1-pro-preview)
> Link: https://sashiko.dev/#/patchset/20260521151121.808477-1-hossu.alexandru%40gmail.com
> Link: https://github.com/sahlberg/libiscsi/pull/473
> Fixes: c3fb804c12bad ("scsi: target: fix hexadecimal CHAP_I handling")
> Signed-off-by: David Disseldorp <[email protected]>
> ---
>  drivers/target/iscsi/iscsi_target_auth.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/target/iscsi/iscsi_target_auth.c b/drivers/target/iscsi/iscsi_target_auth.c
> index a3ad2d244dbee..5858cc3089796 100644
> --- a/drivers/target/iscsi/iscsi_target_auth.c
> +++ b/drivers/target/iscsi/iscsi_target_auth.c
> @@ -438,9 +438,11 @@ static int chap_server_compute_hash(
>         }
>
>         if (type == HEX)
> -               ret = kstrtoul(&identifier[2], 0, &id);
> +               ret = kstrtoul(identifier, 16, &id);
> +       else if (type == DECIMAL)
> +               ret = kstrtoul(identifier, 10, &id);
>         else
> -               ret = kstrtoul(identifier, 0, &id);
> +               ret = -EINVAL;
>
>         if (ret < 0) {
>                 pr_err("kstrtoul() failed for CHAP identifier: %d\n", ret);
> --
> 2.51.0
>
>

Reviewed-by: Lee Duncan <[email protected]>