[edk2-devel] [PATCH 1/6] NetworkPkg/IScsiDxe: re-set session-level authentication state before login

[Philippe Mathieu-Daudé]

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 <jiaxin.wu at intel.com>
> Cc: Maciej Rabeda <maciej.rabeda at linux.intel.com>
> Cc: Philippe Mathieu-Daudé <philmd at redhat.com>
> Cc: Siyuan Fu <siyuan.fu at intel.com>
> Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=3355
> Signed-off-by: Laszlo Ersek <lersek at redhat.com>
> ---
>  NetworkPkg/IScsiDxe/IScsiProto.c | 18 ++++++++++++++++++
>  1 file changed, 18 insertions(+)

Reviewed-by: Philippe Mathieu-Daude <philmd at redhat.com>



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#76746): https://edk2.groups.io/g/devel/message/76746
Mute This Topic: https://groups.io/mt/83395022/1813853
Group Owner: devel+owner at edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [edk2-devel-archive at redhat.com]
-=-=-=-=-=-=-=-=-=-=-=-