[PATCH 18/18] nvmet-tcp: peek icreq before starting TLS

[Sagi Grimberg]

On 3/21/23 14:43, Hannes Reinecke wrote:
> Incoming connection might be either 'normal' NVMe-TCP connections
> starting with icreq or TLS handshakes. To ensure that 'normal'
> connections can still be handled we need to peek the first packet
> and only start TLS handshake if it's not an icreq.

I think that for nvmet, we will want to strictly enforce tsas.sectype.
What are we gaining from allowing this?

And if you insist that we must, then this needs to be an explicit
setting to a permissive mode.

> 
> Signed-off-by: Hannes Reinecke <hare at suse.de>
> ---
>   drivers/nvme/target/tcp.c | 60 +++++++++++++++++++++++++++++++++++++--
>   1 file changed, 58 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/nvme/target/tcp.c b/drivers/nvme/target/tcp.c
> index a69647fb2c81..a328a303c2be 100644
> --- a/drivers/nvme/target/tcp.c
> +++ b/drivers/nvme/target/tcp.c
> @@ -1105,6 +1105,61 @@ static inline bool nvmet_tcp_pdu_valid(u8 type)
>   	return false;
>   }
>   
> +static int nvmet_tcp_try_peek_pdu(struct nvmet_tcp_queue *queue)
> +{
> +	struct nvme_tcp_hdr *hdr = &queue->pdu.cmd.hdr;
> +	int len;
> +	struct kvec iov = {
> +		.iov_base = (u8 *)&queue->pdu + queue->offset,
> +		.iov_len = sizeof(struct nvme_tcp_hdr),
> +	};
> +	char cbuf[CMSG_LEN(sizeof(char))] = {};
> +	unsigned char ctype;
> +	struct cmsghdr *cmsg;
> +	struct msghdr msg = {
> +		.msg_control = cbuf,
> +		.msg_controllen = sizeof(cbuf),
> +		.msg_flags = MSG_PEEK,
> +	};
> +
> +	len = kernel_recvmsg(queue->sock, &msg, &iov, 1,
> +			iov.iov_len, msg.msg_flags);
> +	if (unlikely(len < 0)) {
> +		pr_debug("queue %d peek error %d\n",
> +			 queue->idx, len);
> +		return len;
> +	}
> +
> +	cmsg = (struct cmsghdr *)cbuf;
> +	if (CMSG_OK(&msg, cmsg) &&
> +	    cmsg->cmsg_level == SOL_TLS &&
> +	    cmsg->cmsg_type == TLS_GET_RECORD_TYPE) {
> +		ctype = *((unsigned char *)CMSG_DATA(cmsg));
> +		if (ctype != TLS_RECORD_TYPE_DATA) {
> +			pr_err("queue %d unhandled TLS record %d\n",
> +				queue->idx, ctype);
> +			return -ENOTCONN;
> +		}
> +	}
> +
> +	if (len < sizeof(struct nvme_tcp_hdr)) {
> +		pr_debug("queue %d short read, %d bytes missing\n",
> +			 queue->idx, (int)iov.iov_len - len);
> +		return -EAGAIN;
> +	}
> +	pr_debug("queue %d hdr type %d hlen %d plen %d size %d\n",
> +		 queue->idx, hdr->type, hdr->hlen, hdr->plen,
> +		 (int)sizeof(struct nvme_tcp_icreq_pdu));
> +	if (hdr->type == nvme_tcp_icreq &&
> +	    hdr->hlen == sizeof(struct nvme_tcp_icreq_pdu) &&
> +	    hdr->plen == sizeof(struct nvme_tcp_icreq_pdu)) {
> +		pr_debug("queue %d icreq detected\n",
> +			 queue->idx);
> +		return len;
> +	}
> +	return 0;
> +}
> +
>   static int nvmet_tcp_try_recv_pdu(struct nvmet_tcp_queue *queue)
>   {
>   	struct nvme_tcp_hdr *hdr = &queue->pdu.cmd.hdr;
> @@ -1879,8 +1934,9 @@ static void nvmet_tcp_alloc_queue(struct nvmet_tcp_port *port,
>   
>   	if (queue->state == NVMET_TCP_Q_TLS_HANDSHAKE) {
>   		nvmet_tcp_save_tls_callbacks(queue);
> -		if (!nvmet_tcp_tls_handshake(queue))
> -			return;
> +		if (!nvmet_tcp_try_peek_pdu(queue))

Who guarantees that a payload already exist? Where is the peek resumes
when a payload already exist?

> +			if (!nvmet_tcp_tls_handshake(queue))
> +				return;
>   		nvmet_tcp_restore_tls_callbacks(queue);
>   
>   		/*