On 3/1/22 2:23 AM, Mike Christie wrote: > On 2/22/22 6:42 AM, Petr Pavlu wrote: >> From: Petr Pavlu <petr.pavlu@xxxxxxxx> >> >> Function iscsi_target_do_login() attempts to cancel a connection when >> a number of login exchanges reaches MAX_LOGIN_PDUS (7). This is done by >> having a local counter and incrementing+checking it as the function >> processes requests in a loop. A problem is that since the login rework in >> back in 2013, the function always processes only a single request and the >> loop is terminated at the end of the first iteration. This means the >> counter reaches only value 1 and any excess number of login requests is >> never rejected. >> >> Fix the problem by introducing iscsi_login.negotiation_exchanges counter >> and update the logic to count exchanges per each login phase as described >> in RFC 7143: >>> 6.2. Text Mode Negotiation: >>> [...] >>> In the Login Phase (see Section 6.3), every stage is a separate >>> negotiation. [...] >>> [...] >>> An iSCSI initiator or target MAY terminate a negotiation that does >>> not terminate within an implementation-specific reasonable time or >>> number of exchanges but SHOULD allow at least six (6) exchanges. >> > > It wasn't clear to me what this fixes. Today, are initiators sending more > than 6 exchanges and if so what happens to the target? Is it crashing or > annoying to user or cause some sort of endless login so we run out of > resources? Or is this more of code cleanup? > > When does this happen and with what initiators? This issue is only something that I noticed while reading through the target code because of some different problem. In that sense, the patch is more a code cleanup. My tests to verify the patch were also artificial (attached). I have now additionally tried some simple examples with sending extensive number of Login requests in a loop to the target and did not observe any immediate problem with running out of resources. A possible alternative might be therefore to remove this logic, not sure. Thanks, Petr
#!/usr/bin/python3 import socket # Test excess number of exchanges during security negotiation. with socket.socket(socket.AF_INET, socket.SOCK_STREAM) as s: s.connect(('192.168.122.30', 3260)) send = ( b'\x43' # 00: -> Immediate marker + Login request b'\x00' # 01: -> Continue security negotiation b'\x00' # 02: VersionMax b'\x00' # 03: VersionMin b'\x00' # 04: TotalAHSLength b'\x00\x00\x8c' # 05: DataSegmentLength b'\x0c\xfd\x37\x00\x00\x01' # 08: ISID -> OUI, SUSE, Qualifier=1 b'\x00\x00' # 14: TSIH b'\x00\x00\x00\x00' # 16: InitiatorTaskTag b'\x00\x00' # 20: CID b'\x00\x00' # 22: Reserved b'\x00\x00\x00\x00' # 24: CmdSN b'\x00\x00\x00\x00' # 28: ExpStatSN b'\x00\x00\x00\x00' # 32: Reserved b'\x00\x00\x00\x00' # 36: Reserved b'\x00\x00\x00\x00' # 40: Reserved b'\x00\x00\x00\x00' # 44: Reserved # 48: Login parameters b'InitiatorName=iqn.1996-04.de.suse:01:6fd07434abf9\x00' b'SessionType=Normal\x00' b'TargetName=iqn.2003-01.org.linux-iscsi.localhost.x8664:sn.8726b6baa6b9\x00' # 188: Padding to 4 bytes b'' # 188: ) for i in range(1, 8): print(f"Send #{i}: {send}") s.sendall(send) data = s.recv(1024) print(f"Response: {data}") assert data[0] == 0x23 # Opcode assert data[36] == (0 if i < 7 else 3) # Status-Class assert data[37] == 0 # Status-Detail print() # Test excess number of exchanges during operation parameter negotiation. with socket.socket(socket.AF_INET, socket.SOCK_STREAM) as s: s.connect(('192.168.122.30', 3260)) send = ( b'\x43' # 00: -> Immediate marker + Login request b'\x81' # 01: -> Transfer to operational parameter negotiation b'\x00' # 02: VersionMax b'\x00' # 03: VersionMin b'\x00' # 04: TotalAHSLength b'\x00\x00\x9c' # 05: DataSegmentLength b'\x0c\xfd\x37\x00\x00\x01' # 08: ISID -> OUI, SUSE, Qualifier=1 b'\x00\x00' # 14: TSIH b'\x00\x00\x00\x00' # 16: InitiatorTaskTag b'\x00\x00' # 20: CID b'\x00\x00' # 22: Reserved b'\x00\x00\x00\x00' # 24: CmdSN b'\x00\x00\x00\x00' # 28: ExpStatSN b'\x00\x00\x00\x00' # 32: Reserved b'\x00\x00\x00\x00' # 36: Reserved b'\x00\x00\x00\x00' # 40: Reserved b'\x00\x00\x00\x00' # 44: Reserved # 48: Login parameters b'InitiatorName=iqn.1996-04.de.suse:01:6fd07434abf9\x00' b'SessionType=Normal\x00' b'TargetName=iqn.2003-01.org.linux-iscsi.localhost.x8664:sn.8726b6baa6b9\x00' b'AuthMethod=None\x00' # 204: Padding to 4 bytes b'' # 204: ) print(f"Send #1: {send}") s.sendall(send) data = s.recv(1024) print(f"Response: {data}") assert data[0] == 0x23 # Opcode assert data[1] == 0x81 # -> Transfer to operational parameter negotiation assert data[36] == 0 # Status-Class assert data[37] == 0 # Status-Detail send = ( b'\x43' # 00: -> Immediate marker + Login request b'\x04' # 01: -> Continue operational parameter negotiation b'\x00' # 02: VersionMax b'\x00' # 03: VersionMin b'\x00' # 04: TotalAHSLength b'\x00\x00\x12' # 05: DataSegmentLength b'\x0c\xfd\x37\x00\x00\x01' # 08: ISID -> OUI, SUSE, Qualifier=1 b'\x00\x00' # 14: TSIH b'\x00\x00\x00\x00' # 16: InitiatorTaskTag b'\x00\x00' # 20: CID b'\x00\x00' # 22: Reserved b'\x00\x00\x00\x00' # 24: CmdSN b'\x00\x00\x00\x00' # 28: ExpStatSN b'\x00\x00\x00\x00' # 32: Reserved b'\x00\x00\x00\x00' # 36: Reserved b'\x00\x00\x00\x00' # 40: Reserved b'\x00\x00\x00\x00' # 44: Reserved # 48: Login parameters b'HeaderDigest=None\x00' # 66: Padding to 4 bytes b'\x00\x00' # 68: ) for i in range(2, 9): print(f"Send #{i}: {send}") s.sendall(send) data = s.recv(1024) print(f"Response: {data}") assert data[0] == 0x23 assert data[36] == (0 if i < 8 else 3) # Status-Class assert data[37] == 0 # Status-Detail