From: Mike Christie <michael.christie@oracle.com>
To: Dmitry Bogdanov <d.bogdanov@yadro.com>,
Martin Petersen <martin.petersen@oracle.com>,
target-devel@vger.kernel.org
Cc: linux-scsi@vger.kernel.org, linux@yadro.com,
Roman Bolshakov <r.bolshakov@yadro.com>,
Konstantin Shelekhin <k.shelekhin@yadro.com>
Subject: Re: [PATCH 2/3] scsi: target: iscsi: extract auth functions
Date: Thu, 30 Sep 2021 13:46:29 -0500 [thread overview]
Message-ID: <c55719fa-9150-b64e-306f-b8b31a405be4@oracle.com> (raw)
In-Reply-To: <20210914100314.492-3-d.bogdanov@yadro.com>
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?
> + }
> +
> + 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?
next prev parent reply other threads:[~2021-09-30 18:46 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-09-14 10:03 [PATCH 0/3] target: iscsi: control authentication per ACL Dmitry Bogdanov
2021-09-14 10:03 ` [PATCH 1/3] scsi: target: iscsi: Add upcast helpers Dmitry Bogdanov
2021-09-14 10:03 ` [PATCH 2/3] scsi: target: iscsi: extract auth functions Dmitry Bogdanov
2021-09-30 18:46 ` Mike Christie [this message]
2021-10-04 7:41 ` Dmitriy Bogdanov
2021-09-14 10:03 ` [PATCH 3/3] target: iscsi: control authentication per ACL Dmitry Bogdanov
2021-09-30 18:52 ` Mike Christie
2021-10-04 7:56 ` Dmitriy Bogdanov
2021-10-05 16:04 ` Mike Christie
2021-10-18 11:48 ` Dmitriy Bogdanov
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-all 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=c55719fa-9150-b64e-306f-b8b31a405be4@oracle.com \
--to=michael.christie@oracle.com \
--cc=d.bogdanov@yadro.com \
--cc=k.shelekhin@yadro.com \
--cc=linux-scsi@vger.kernel.org \
--cc=linux@yadro.com \
--cc=martin.petersen@oracle.com \
--cc=r.bolshakov@yadro.com \
--cc=target-devel@vger.kernel.org \
/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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.