Re: [PATCH v5 2/3] scsi: target: iscsi: extract auth functions

[Lee Duncan <lduncan@suse.com>]

On 5/23/22 02:59, Dmitry Bogdanov wrote:
> Create functions that answers simple questions:
> whether authentication is required, what credentials, whether
> connection is autenticated.
> 
> Reviewed-by: Roman Bolshakov <r.bolshakov@yadro.com>
> Reviewed-by: Konstantin Shelekhin <k.shelekhin@yadro.com>
> Reviewed-by: Mike Christie <michael.christie@oracle.com>
> Signed-off-by: Dmitry Bogdanov <d.bogdanov@yadro.com>
> ---
>   drivers/target/iscsi/iscsi_target_nego.c | 140 +++++++++++++++--------
>   1 file changed, 92 insertions(+), 48 deletions(-)
> 
> diff --git a/drivers/target/iscsi/iscsi_target_nego.c b/drivers/target/iscsi/iscsi_target_nego.c
> index d853bacf1cfc..f06f16d63fe6 100644
> --- a/drivers/target/iscsi/iscsi_target_nego.c
> +++ b/drivers/target/iscsi/iscsi_target_nego.c
> @@ -94,6 +94,31 @@ int extract_param(
>   	return 0;
>   }
>   
> +static struct iscsi_node_auth *iscsi_get_node_auth(struct iscsit_conn *conn)
> +{
> +	struct iscsi_portal_group *tpg;
> +	struct iscsi_node_acl *nacl;
> +	struct se_node_acl *se_nacl;
> +
> +	if (conn->sess->sess_ops->SessionType)
> +		return &iscsit_global->discovery_acl.node_auth;
> +
> +	se_nacl = conn->sess->se_sess->se_node_acl;
> +	if (!se_nacl) {
> +		pr_err("Unable to locate struct se_node_acl for CHAP auth\n");
> +		return NULL;
> +	}
> +
> +	if (se_nacl->dynamic_node_acl) {
> +		tpg = to_iscsi_tpg(se_nacl->se_tpg);
> +		return &tpg->tpg_demo_auth;
> +	}
> +
> +	nacl = to_iscsi_nacl(se_nacl);
> +
> +	return &nacl->node_auth;
> +}
> +
>   static u32 iscsi_handle_authentication(
>   	struct iscsit_conn *conn,
>   	char *in_buf,
> @@ -102,38 +127,11 @@ static u32 iscsi_handle_authentication(
>   	int *out_length,
>   	unsigned char *authtype)
>   {
> -	struct iscsit_session *sess = conn->sess;
>   	struct iscsi_node_auth *auth;
> -	struct iscsi_node_acl *nacl;
> -	struct iscsi_portal_group *tpg;
> -	struct se_node_acl *se_nacl;
> -
> -	if (!sess->sess_ops->SessionType) {
> -		/*
> -		 * For SessionType=Normal
> -		 */
> -		se_nacl = conn->sess->se_sess->se_node_acl;
> -		if (!se_nacl) {
> -			pr_err("Unable to locate struct se_node_acl for"
> -					" CHAP auth\n");
> -			return -1;
> -		}
> -
> -		if (se_nacl->dynamic_node_acl) {
> -			tpg = to_iscsi_tpg(se_nacl->se_tpg);
> -
> -			auth = &tpg->tpg_demo_auth;
> -		} else {
> -			nacl = to_iscsi_nacl(se_nacl);
>   
> -			auth = &nacl->node_auth;
> -		}
> -	} else {
> -		/*
> -		 * For SessionType=Discovery
> -		 */
> -		auth = &iscsit_global->discovery_acl.node_auth;
> -	}
> +	auth = iscsi_get_node_auth(conn);
> +	if (!auth)
> +		return -1;
>   
>   	if (strstr("CHAP", authtype))
>   		strcpy(conn->sess->auth_type, "CHAP");
> @@ -813,6 +811,37 @@ static int iscsi_target_do_authentication(
>   	return 0;
>   }
>   
> +static bool iscsi_conn_auth_required(struct iscsit_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;
> +	}
> +
> +	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 iscsit_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);
> +
> +		if (auth_required) {
> +			if (!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;
> +			}
>   
> -		if (conn->tpg->tpg_attrib.authentication &&
> -		    !login->auth_complete)
> -			return 0;
> +			if (!login->auth_complete)
> +				return 0;
>   
> -		if (strncmp(param->value, NONE, 4) && !login->auth_complete)
> -			return 0;
> +			if (strncmp(param->value, NONE, 4) &&
> +			    !login->auth_complete)
> +				return 0;
> +		}
>   
>   		if ((login_req->flags & ISCSI_FLAG_LOGIN_NEXT_STAGE1) &&
>   		    (login_req->flags & ISCSI_FLAG_LOGIN_TRANSIT)) {
> @@ -904,6 +937,18 @@ static int iscsi_target_handle_csg_zero(
>   	return iscsi_target_do_authentication(conn, login);
>   }
>   
> +static bool iscsi_conn_authenticated(struct iscsit_conn *conn,
> +				     struct iscsi_login *login)
> +{
> +	if (!iscsi_conn_auth_required(conn))
> +		return true;
> +
> +	if (login->auth_complete)
> +		return true;
> +
> +	return false;
> +}
> +
>   static int iscsi_target_handle_csg_one(struct iscsit_conn *conn, struct iscsi_login *login)
>   {
>   	int ret;
> @@ -947,11 +992,10 @@ static int iscsi_target_handle_csg_one(struct iscsit_conn *conn, struct iscsi_lo
>   		return -1;
>   	}
>   
> -	if (!login->auth_complete &&
> -	     conn->tpg->tpg_attrib.authentication) {
> +	if (!iscsi_conn_authenticated(conn, login)) {
>   		pr_err("Initiator is requesting CSG: 1, has not been"
> -			 " successfully authenticated, and the Target is"
> -			" enforcing iSCSI Authentication, login failed.\n");
> +		       " successfully authenticated, and the 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;

Reviewed-by: Lee Duncan <lduncan@suse.com>