Re: [PATCH 2/3] scsi: target: iscsi: extract auth functions
From: Mike Christie <michael.christie@oracle.com>
Date: 2021-09-30 18:46:42
Also in:
target-devel
On 9/14/21 5:03 AM, Dmitry Bogdanov wrote:
+bool iscsi_conn_auth_required(struct iscsi_conn *conn)
+{
+ struct se_node_acl *se_nacl;
+
+ if (conn->sess->sess_ops->SessionType) {
+ /*
+ * For SessionType=Discovery
+ */
+ return conn->tpg->tpg_attrib.authentication;
+ }
+ /*
+ * For SessionType=Normal
+ */
+ se_nacl = conn->sess->se_sess->se_node_acl;
+ if (!se_nacl) {
+ pr_debug("Unknown ACL %s is trying to connect\n",
+ se_nacl->initiatorname);
+ return true;Before the patch, if we didn't have an ACL we would go by conn->tpg->tpg_attrib.authentication. But with the patch, if we don't have an ACL, then it looks like we always require authentication which I don't think is right. Is the code above supposed to return the value of conn->tpg->tpg_attrib.authentication?
quoted hunk ↗ jump to hunk
+ } + + if (se_nacl->dynamic_node_acl) { + pr_debug("Dynamic ACL %s is trying to connect\n", + se_nacl->initiatorname); + return conn->tpg->tpg_attrib.authentication; + } + + pr_debug("Known ACL %s is trying to connect\n", + se_nacl->initiatorname); + return conn->tpg->tpg_attrib.authentication; +} + static int iscsi_target_handle_csg_zero( struct iscsi_conn *conn, struct iscsi_login *login)@@ -874,22 +903,26 @@ static int iscsi_target_handle_csg_zero( return -1; if (!iscsi_check_negotiated_keys(conn->param_list)) { - if (conn->tpg->tpg_attrib.authentication && - !strncmp(param->value, NONE, 4)) { - pr_err("Initiator sent AuthMethod=None but" - " Target is enforcing iSCSI Authentication," - " login failed.\n"); - iscsit_tx_login_rsp(conn, ISCSI_STATUS_CLS_INITIATOR_ERR, - ISCSI_LOGIN_STATUS_AUTH_FAILED); - return -1; - } + bool auth_required = iscsi_conn_auth_required(conn); +
In __iscsi_target_login_thread we have:
if (conn->sess)
conn->sess->se_sess->sup_prot_ops =
conn->conn_transport->iscsit_get_sup_prot_ops(conn);
before we call:
iscsi_target_start_negotiation -> iscsi_target_do_login- > iscsi_target_handle_csg_zero
and hit the code above.
If conn->sess can be NULL then iscsi_conn_auth_required will crash.
However, I can't tell how conn->sess can be NULL in that code path. Is the conn->sess
check in __iscsi_target_login_thread not needed?