From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from smtp-out1.suse.de (smtp-out1.suse.de [195.135.220.28]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id A54D32FAF for ; Wed, 22 Mar 2023 16:43:28 +0000 (UTC) Received: from imap2.suse-dmz.suse.de (imap2.suse-dmz.suse.de [192.168.254.74]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (P-521) server-digest SHA512) (No client certificate requested) by smtp-out1.suse.de (Postfix) with ESMTPS id BC59633D1A; Wed, 22 Mar 2023 16:43:26 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.de; s=susede2_rsa; t=1679503406; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc:cc: mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=0hML6p9VjAVsulxdze1pFl1v7RuVX1tS1vNbrPZcF6E=; b=rGREUI4E4qgdY9G3QFNUUAdX0sZprr7Pv1H+HR9BGkJKAgjJ3iXHy7ZKpD1IxBl5xGbFBh tkWO7JP7NyXze3SJH2DSuZZQU+9XK56RbH6RevYO4uBPeiZjjmDD4PcmOPxVTmsJn9E5vr 4/eCRCLVdFbRu0GkrZPaCw1+yd78PSM= DKIM-Signature: v=1; a=ed25519-sha256; c=relaxed/relaxed; d=suse.de; s=susede2_ed25519; t=1679503406; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc:cc: mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=0hML6p9VjAVsulxdze1pFl1v7RuVX1tS1vNbrPZcF6E=; b=TMbcwjeK5xgKYX/yJxjicv7Q5dYiOy9DYiYDfk1N3bjs9yd+MgWla1f2cjAtqTbYNj/uCv UMD9E8h4Xx+eOXDw== Received: from imap2.suse-dmz.suse.de (imap2.suse-dmz.suse.de [192.168.254.74]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (P-521) server-digest SHA512) (No client certificate requested) by imap2.suse-dmz.suse.de (Postfix) with ESMTPS id 53DCB13416; Wed, 22 Mar 2023 16:43:26 +0000 (UTC) Received: from dovecot-director2.suse.de ([192.168.254.65]) by imap2.suse-dmz.suse.de with ESMTPSA id rFI5ES4wG2RaeAAAMHmgww (envelope-from ); Wed, 22 Mar 2023 16:43:26 +0000 Message-ID: Date: Wed, 22 Mar 2023 17:43:25 +0100 Precedence: bulk X-Mailing-List: kernel-tls-handshake@lists.linux.dev List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.6.1 Subject: Re: [PATCH 15/18] nvmet-tcp: enable TLS handshake upcall Content-Language: en-US To: Sagi Grimberg , Christoph Hellwig Cc: Keith Busch , linux-nvme@lists.infradead.org, Chuck Lever , kernel-tls-handshake@lists.linux.dev References: <20230321124325.77385-1-hare@suse.de> <20230321124325.77385-16-hare@suse.de> <6f69983c-17bd-dd72-b941-95f55db10180@grimberg.me> From: Hannes Reinecke In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit On 3/22/23 16:42, Sagi Grimberg wrote: > >>>> The 'data_ready' call might happen at any time after the 'accept' >>>> call and us calling into userspace. >>>> In particular we have this flow of control: >>>> >>>> 1. Kernel: accept() >>>> 2. Kernel: handshake request >>>> 3. Userspace: read data from socket >>>> 4. Userspace: tls handshake >>>> 5. Kernel: handshake complete >>>> >>>> If the 'data_ready' event occurs between 1. and 3. userspace >>>> wouldn't know that something has happened, and will be sitting there >>>> waiting for data which is already present. >>> >>> Umm, doesn't userspace read from the socket once we trigger the upcall? >>> it should. But I still don't understand what is the difference between >>> us waiking up userspace, from the default sock doing the same? >>> >> No, it doesn't (or, rather, can't). >> After processing 'accept()' (from the kernel code) data might already >> be present (after all, why would we get an 'accept' call otherwise?). >> But the daemon has not been started up (yet); that's only done in >> step 3). But 'data_ready' has already been called, so by the time >> userland is able to do a 'read()' on the socket it won't be seeing >> anything. > > Not sure I understand. if data exists, userspace will read from the > socket and get data, whenever that is. > That's what I thought, too. But then the userspace daemon just sat there doing nothing. >> To be precise: that's the reasoning I've come up with. >> Might be completely wrong, or beside the point. >> But what I _do_ know is that without this call userspace would >> _not_ see any data, and would happily sit there until we close the >> socket due to a timeout (which also why I put in the timeout >> module options ...), and only _then_ see the data. >> But with this call everything works. So there. > > I may be missing something, and maybe its because I haven't looked at > the userspace side yet. But if it calls recvmsg, it should either > consume data, or it should block until it arrives. > > I'm finding it difficult to understand why emulating almost 100% the > default socket .data_ready is what is needed. But I'll try to understand > better. > Oh, I don't claim to have a full understanding, either. I had initially assumed that it would work 'out of the box', without us having to specify a callback. Turns out that it doesn't. Maybe it's enough to call sk->sk_data_ready() after the upcall has been started. But we sure have to do _something_. If you have other solutions I'm all ears... > > >>>> As outlined in the response to the nvme-tcp upcall, on the server >>>> side we _have_ to allow for non-TLS connections (eg. for discovery). >>> >>> But in essence what you are doing is that you allow normal connections >>> for a secured port... >>> >> Correct. > > That is not what the feature is supposed to do. > Well ... it's a difference in interpretation. The NVMe TCP spec just says '... describes whether TLS is supported.'. It does not say 'required'. But there currently is no way how the server could describe 'TLS supported' vs 'TLS required'. >>> btw, why not enforce a psk for the discovery controller (on this port) >>> as well? for secured ports? No one said that we must accept a >>> non-secured discovery connecting host on a secured port. >>> >> Because we can't have a PSK for standard discovery. >> Every discovery controller has the canonical discovery NQN, so every >> PSK will have the same subsystem NQN, and you will have to provide the >> _same_ PSK to every attached storage system. > > Right, because they have the same NQN. Sounds good, what is the problem? > It is much better than to have sectype be nothing more than a > recommendation. > > If sectype tls1.x is defined on a port then every connection to that > port is a tls connection. > See above. Only if we decide to give TSAS a 'TLS required' meaning. With the this patchset it's a 'TLS supported' syntax. >> If we had unique discovery controller NQNs everything would be good, >> but as we don't I guess we have to support normal connections in >> addition to secured ones. > > Its orthogonal. the well-known disc-nqn may be "weaker", but tls > is still enforced. when we add unique disc-nqn, we can use different > psks. > Not only weaker, but _identical_ for each server. >> As I said; maybe it's time to revisit the unique discovery NQN patches; >> with those we should be able to separate the various use-case like >> having a dedicated secured discovery port. > > I don't see why this would be needed. > We don't if we treat TSAS as 'TLS required', true. But that's not what the spec says. And there is also the dodgy statement in section 3.6.1.6: > If a host that supports TLS for NVMe/TCP receives a discovery > log entry indicating that the NVM subsystem uses NVMe/TCP and > does not support TLS, then the host should nonetheless > attempt to establish an NVMe/TCP connection that uses TLS. > which really indicates that the TSAS field is a recommendation only. But really, we shouldn't read too much into what the TSAS field says. With the upcoming TPAR for TLS clarification all these different interpretations should be clarified. The key question, however, will remain: Do we _want_ to support a 'TLS supported' mode for nvmet? I'd say we should, to have compability with older (client) installations. And I guess we have to anyway for secure concatenation (as this _requires_ that we start off unencrypted). So I'd suggest to leave it for now, have the code to support both, and wait for the TPAR to be ratified and then revisit this issue. Cheers, Hannes -- Dr. Hannes Reinecke Kernel Storage Architect hare@suse.de +49 911 74053 688 SUSE Software Solutions GmbH, Maxfeldstr. 5, 90409 Nürnberg HRB 36809 (AG Nürnberg), Geschäftsführer: Ivo Totev, Andrew Myers, Andrew McDonald, Martje Boudien Moerman