public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: "Laszlo Ersek" <lersek@redhat.com>
To: edk2-devel-groups-io <devel@edk2.groups.io>
Cc: "Jiaxin Wu" <jiaxin.wu@intel.com>,
	"Maciej Rabeda" <maciej.rabeda@linux.intel.com>,
	"Philippe Mathieu-Daudé" <philmd@redhat.com>,
	"Siyuan Fu" <siyuan.fu@intel.com>
Subject: [PATCH v2 1/6] NetworkPkg/IScsiDxe: re-set session-level authentication state before login
Date: Tue, 29 Jun 2021 18:33:32 +0200	[thread overview]
Message-ID: <20210629163337.14120-2-lersek@redhat.com> (raw)
In-Reply-To: <20210629163337.14120-1-lersek@redhat.com>

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@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>
Reviewed-by: Maciej Rabeda <maciej.rabeda@linux.intel.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---

Notes:
    v2:
    - pick up R-b's [Phil, Maciej]

 NetworkPkg/IScsiDxe/IScsiProto.c | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/NetworkPkg/IScsiDxe/IScsiProto.c b/NetworkPkg/IScsiDxe/IScsiProto.c
index 6983f0fa5973..69d1b39dbb1f 100644
--- a/NetworkPkg/IScsiDxe/IScsiProto.c
+++ b/NetworkPkg/IScsiDxe/IScsiProto.c
@@ -401,38 +401,55 @@ IScsiGetIp6NicInfo (
   if (Ip6ModeData.GroupTable!= NULL) {
     FreePool (Ip6ModeData.GroupTable);
   }
   if (Ip6ModeData.RouteTable!= NULL) {
     FreePool (Ip6ModeData.RouteTable);
   }
   if (Ip6ModeData.NeighborCache!= NULL) {
     FreePool (Ip6ModeData.NeighborCache);
   }
   if (Ip6ModeData.PrefixTable!= NULL) {
     FreePool (Ip6ModeData.PrefixTable);
   }
   if (Ip6ModeData.IcmpTypeList!= NULL) {
     FreePool (Ip6ModeData.IcmpTypeList);
   }
 
   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
+  )
+{
+}
+
 /**
   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;
   ISCSI_CONNECTION  *Conn;
   VOID              *Tcp;
@@ -454,38 +471,39 @@ IScsiSessionLogin (
   //
   CopyMem (Session->Isid, Session->ConfigData->SessionConfigData.IsId, 6);
 
   RetryCount = 0;
 
   do {
     //
     // Create a connection for the session.
     //
     Conn = IScsiCreateConnection (Session);
     if (Conn == NULL) {
       return EFI_OUT_OF_RESOURCES;
     }
 
     IScsiAttatchConnection (Session, Conn);
 
     //
     // Login through the newly created connection.
     //
+    IScsiSessionResetAuthData (Session);
     Status = IScsiConnLogin (Conn, Session->ConfigData->SessionConfigData.ConnectTimeout);
     if (EFI_ERROR (Status)) {
       IScsiConnReset (Conn);
       IScsiDetatchConnection (Conn);
       IScsiDestroyConnection (Conn);
     }
 
     if (Status != EFI_TIMEOUT) {
       break;
     }
 
     RetryCount++;
   } while (RetryCount <= Session->ConfigData->SessionConfigData.ConnectRetryCount);
 
   if (!EFI_ERROR (Status)) {
     Session->State = SESSION_STATE_LOGGED_IN;
 
     if (!Conn->Ipv6Flag) {
       ProtocolGuid = &gEfiTcp4ProtocolGuid;
-- 
2.19.1.3.g30247aa5d201



  reply	other threads:[~2021-06-29 16:33 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-06-29 16:33 [PATCH v2 0/6] NetworkPkg/IScsiDxe: support SHA256 in CHAP Laszlo Ersek
2021-06-29 16:33 ` Laszlo Ersek [this message]
2021-06-29 16:33 ` [PATCH v2 2/6] NetworkPkg/IScsiDxe: add horizontal whitespace to IScsiCHAP files Laszlo Ersek
2021-06-29 16:33 ` [PATCH v2 3/6] NetworkPkg/IScsiDxe: distinguish "maximum" and "selected" CHAP digest sizes Laszlo Ersek
2021-06-29 16:33 ` [PATCH v2 4/6] NetworkPkg/IScsiDxe: support multiple hash algorithms for CHAP Laszlo Ersek
2021-06-29 16:33 ` [PATCH v2 5/6] NetworkPkg/IScsiDxe: support SHA256 in CHAP Laszlo Ersek
2021-06-29 16:33 ` [PATCH v2 6/6] NetworkPkg: introduce the NETWORK_ISCSI_MD5_ENABLE feature test macro Laszlo Ersek
2021-06-29 18:44 ` [edk2-devel] [PATCH v2 0/6] NetworkPkg/IScsiDxe: support SHA256 in CHAP Maciej Rabeda
2021-06-30 20:33   ` Laszlo Ersek

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=20210629163337.14120-2-lersek@redhat.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