On 7/20/21 10:28 PM, Vladislav Bolkhovitin wrote:

On 7/18/21 3:21 PM, Hannes Reinecke wrote:

quoted

On 7/17/21 9:22 AM, Sagi Grimberg wrote:

quoted

quoted

Implement NVMe-oF In-Band authentication. This patch adds two new
fabric options 'dhchap_key' to specify the PSK

pre-shared-key.

Also, we need a sysfs knob to rotate the key that will trigger
re-authentication or even a simple controller(s-plural) reset, so this
should go beyond just the connection string.

Yeah, re-authentication currently is not implemented. I first wanted to
get this patchset out such that we can settle on the userspace interface
(both from host and target).
I'll have to think on how we should handle authentication; one of the
really interesting cases would be when one malicious admin will _just_
send a 'negotiate' command to the controller. As per spec the controller
will be waiting for an 'authentication receive' command to send a
'challenge' payload back to the host. But that will never come, so as it
stands currently the controller is required to abort the connection.
Not very nice.

Yes, in this case after some reasonable timeout (I would suggest 10-15
seconds) the controller expected to abort connection and clean up all
allocated resources.

To handle DoS possibility to make too many such "orphan" negotiations,
hence consume all controller memory, some additional handling is needed.
For simplicity as a first step I would suggest to have a global limit on
number of currently being authenticated connections.

[...]

quoted

quoted

quoted

+    chap->key = nvme_auth_extract_secret(ctrl->opts->dhchap_secret,
+                         &chap->key_len);
+    if (IS_ERR(chap->key)) {
+        ret = PTR_ERR(chap->key);
+        chap->key = NULL;
+        return ret;
+    }
+
+    if (key_hash == 0)
+        return 0;
+
+    hmac_name = nvme_auth_hmac_name(key_hash);
+    if (!hmac_name) {
+        pr_debug("Invalid key hash id %d\n", key_hash);
+        return -EKEYREJECTED;
+    }

Why does the user influence the hmac used? isn't that is driven
by the susbsystem?

I don't think that the user should choose in this level.

That is another weirdness of the spec.
The _secret_ will be hashed with a specific function, and that function
is stated in the transport representation.
(Cf section "DH-HMAC-CHAP Security Requirements").
This is _not_ the hash function used by the authentication itself, which
will be selected by the protocol.
So it's not the user here, but rather the transport specification of the
key which selects the hash algorithm.

Yes, good catch. It looks as a minor errata material to specify that
hash function here is implementation specific.

I would suggest to just hardcode SHA512 here. Users don't have to be
confused by this.

Sure, can do. My reasoning was that the target absolutely has to support
the hash functions specified in the PSK, so that will be a safe bet to
choose for the hash function in the protocol itself.
(Any other hash function _might_ not be preset on the target.)
But if the PSK does not specify a hash the target need to pick one; and
for that of course we can use SHA512.

Cheers,

Hannes
-- 
Dr. Hannes Reinecke		           Kernel Storage Architect
hare@suse.de			                  +49 911 74053 688
SUSE Software Solutions Germany GmbH, Maxfeldstr. 5, 90409 Nürnberg
HRB 36809 (AG Nürnberg), GF: Felix Imendörffer