From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by mx.groups.io with SMTP id smtpd.web11.5765.1624009539927966646 for ; Fri, 18 Jun 2021 02:45:40 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@redhat.com header.s=mimecast20190719 header.b=Am/hY838; spf=pass (domain: redhat.com, ip: 170.10.133.124, mailfrom: philmd@redhat.com) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1624009539; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=lMoWTvMYaqHRQdTSWJypaSliiO3x1uOJ8xDt6Jdlvhg=; b=Am/hY838gPyZVZSrZc1QkMP76YvsjaifBEm5wn31+tWt5QrNsnDcjla24f8eji0azJep/d zyAo5vtsULTvnezAQ8StMTtqlPVRJW4wVXW/mW7XBesv2+jiYHqM8PQSrIcMX3norm8J9u /I33oi0GsLPHSPKGinIU6zuw2NRz5T4= Received: from mail-wm1-f69.google.com (mail-wm1-f69.google.com [209.85.128.69]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-534-dC0skphnN0inRxVuowUcPQ-1; Fri, 18 Jun 2021 05:45:36 -0400 X-MC-Unique: dC0skphnN0inRxVuowUcPQ-1 Received: by mail-wm1-f69.google.com with SMTP id z25-20020a1c4c190000b029019f15b0657dso3482163wmf.8 for ; Fri, 18 Jun 2021 02:45:35 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:subject:to:cc:references:from:message-id:date :user-agent:mime-version:in-reply-to:content-language :content-transfer-encoding; bh=lMoWTvMYaqHRQdTSWJypaSliiO3x1uOJ8xDt6Jdlvhg=; b=BiwNTW1DqWo8hnVjy7Dwn29l9xcUqFccBasA/n+MivF8sX3rAqbnKnUQ+OQTzGGM2g +mLSAOFjM4ax40HusyMbcCLVmFPaqDjRhrIrCF/phZrGnG7KhVWPwoX5PkDxF6rmt6tT o7CUeIGeSmdyYq0olNoEFb42chB4m+OzOV4E69RIiP+H9CDm7f1k6B0L1S4tIr/8c1n5 +aSaZQeZm60JpQu9/ZHxUcRcGXRjnQF6bFRf4uHV1AKHuOrFKYSdUT+Jce+AAoRBzFFF ng+Ar25isiVxXVD9YY6fTUk0nZTnrUwU7/pLVzEvph3EYS9b4iI+JOkL4Mhy7VQuI26J 0WkQ== X-Gm-Message-State: AOAM533P78wnIzu6Ris3OD0+VimPPs2wB4VVHkRr7jd4Sz/JjtYFHv4g ZYuShP1ggtTgnyVG0Lsnu8x3+5d53KFjFqsN1m45yb9Ew9gqosxdpBaE4MqZ9Fc6R26dQp923oc 9Or8DZPx9Bh7sgA== X-Received: by 2002:a5d:6109:: with SMTP id v9mr11657137wrt.0.1624009534834; Fri, 18 Jun 2021 02:45:34 -0700 (PDT) X-Google-Smtp-Source: ABdhPJwMhE0tv2xUvElL2zTv6Cn32sp8YtTx3kbx/L68H+mtgB/zlBLKhIt/0CAnZ0kFYKeOBbn2QQ== X-Received: by 2002:a5d:6109:: with SMTP id v9mr11657109wrt.0.1624009534605; Fri, 18 Jun 2021 02:45:34 -0700 (PDT) Return-Path: Received: from [192.168.1.36] (93.red-83-35-24.dynamicip.rima-tde.net. [83.35.24.93]) by smtp.gmail.com with ESMTPSA id p16sm8209730wrs.52.2021.06.18.02.45.33 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Fri, 18 Jun 2021 02:45:34 -0700 (PDT) Subject: Re: [PATCH 1/6] NetworkPkg/IScsiDxe: re-set session-level authentication state before login To: Laszlo Ersek , edk2-devel-groups-io Cc: Jiaxin Wu , Maciej Rabeda , Siyuan Fu References: <20210608130652.2434-1-lersek@redhat.com> <20210608130652.2434-2-lersek@redhat.com> From: =?UTF-8?B?UGhpbGlwcGUgTWF0aGlldS1EYXVkw6k=?= Message-ID: Date: Fri, 18 Jun 2021 11:45:33 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.10.1 MIME-Version: 1.0 In-Reply-To: <20210608130652.2434-2-lersek@redhat.com> Authentication-Results: relay.mimecast.com; auth=pass smtp.auth=CUSA124A263 smtp.mailfrom=philmd@redhat.com X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 8bit On 6/8/21 3:06 PM, Laszlo Ersek wrote: > RFC 7143 explains that a single iSCSI session may use multiple TCP > connections. The first connection established is called the leading > connection. The login performed on the leading connection is called the > leading login. Before the session is considered full-featured, the leading > login must succeed. Further (non-leading) connections can be associated > with the session later. > > (It's unclear to me from RFC 7143 whether the non-leading connections > require individual (non-leading) logins as well, but that particular > question is irrelevant from the perspective of this patch; see below.) > > The data model in IScsiDxe exhibits some confusion, regarding connection / > session association: > > - On one hand, the "ISCSI_SESSION.Conns" field is a *set* (it has type > LIST_ENTRY), and accordingly, connections can be added to, and removed > from, a session, with the IScsiAttatchConnection() and > IScsiDetatchConnection() functions. > > - On the other hand, ISCSI_MAX_CONNS_PER_SESSION has value 1, therefore no > session will ever use more than 1 connection at a time (refer to > instances of "Session->MaxConnections" in > "NetworkPkg/IScsiDxe/IScsiProto.c"). > > This one-to-many confusion between ISCSI_SESSION and ISCSI_CONNECTION is > very visible in the CHAP logic, where the progress of the authentication > is maintained *per connection*, in the "ISCSI_CONNECTION.AuthStep" field > (with values such as ISCSI_AUTH_INITIAL, ISCSI_CHAP_STEP_ONE, etc), but > the *data* for the authentication are maintained *per session*, in the > "AuthType" and "AuthData" fields of ISCSI_SESSION. Clearly, this makes no > sense if multiple connections are eligible for logging in. > > Knowing that IScsiDxe uses only one connection per session (put > differently: knowing that any connection is a leading connection, and any > login is a leading login), there is no functionality bug. But the data > model is still broken: "AuthType", "AuthData", and "AuthStep" should be > maintained at the *same* level -- be it "session-level" or "(leading) > connection-level". > > Fixing this data model bug is more than what I'm signing up for. However, > I do need to add one function, in preparation for multi-hash support: > whenever a new login is attempted (put differently: whenever the leading > login is re-attempted), which always happens with a fresh connection, the > session-level authentication data needs to be rewound to a sane initial > state. > > Introduce the IScsiSessionResetAuthData() function. Call it from the > central -- session-level -- IScsiSessionLogin() function, just before the > latter calls the -- connection-level -- IScsiConnLogin() function. > > Right now, do nothing in IScsiSessionResetAuthData(); so functionally > speaking, the patch is a no-op. > > Cc: Jiaxin Wu > Cc: Maciej Rabeda > Cc: Philippe Mathieu-Daudé > Cc: Siyuan Fu > Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=3355 > Signed-off-by: Laszlo Ersek > --- > NetworkPkg/IScsiDxe/IScsiProto.c | 18 ++++++++++++++++++ > 1 file changed, 18 insertions(+) Reviewed-by: Philippe Mathieu-Daude