On 7/17/21 8:30 AM, Sagi Grimberg wrote:

On 7/16/21 4:04 AM, Hannes Reinecke wrote:

quoted

Signed-off-by: Hannes Reinecke <hare@suse.de>
---
  include/linux/nvme.h | 185 ++++++++++++++++++++++++++++++++++++++++++-
  1 file changed, 184 insertions(+), 1 deletion(-)

diff --git a/include/linux/nvme.h b/include/linux/nvme.h
index b7c4c4130b65..7b94abacfd08 100644
--- a/include/linux/nvme.h
+++ b/include/linux/nvme.h

@@ -19,6 +19,7 @@

  #define NVMF_TRSVCID_SIZE    32
  #define NVMF_TRADDR_SIZE    256
  #define NVMF_TSAS_SIZE        256
+#define NVMF_AUTH_HASH_LEN    64
  #define NVME_DISC_SUBSYS_NAME    "nqn.2014-08.org.nvmexpress.discovery"

@@ -1263,6 +1264,8 @@ enum nvmf_capsule_command {

      nvme_fabrics_type_property_set    = 0x00,
      nvme_fabrics_type_connect    = 0x01,
      nvme_fabrics_type_property_get    = 0x04,
+    nvme_fabrics_type_auth_send    = 0x05,
+    nvme_fabrics_type_auth_receive    = 0x06,
  };
  #define nvme_fabrics_type_name(type)   { type, #type }

@@ -1270,7 +1273,9 @@ enum nvmf_capsule_command {

      __print_symbolic(type,                        \
          nvme_fabrics_type_name(nvme_fabrics_type_property_set),    \
          nvme_fabrics_type_name(nvme_fabrics_type_connect),    \
-        nvme_fabrics_type_name(nvme_fabrics_type_property_get))
+        nvme_fabrics_type_name(nvme_fabrics_type_property_get), \
+        nvme_fabrics_type_name(nvme_fabrics_type_auth_send),    \
+        nvme_fabrics_type_name(nvme_fabrics_type_auth_receive))
  /*
   * If not fabrics command, fctype will be ignored.

@@ -1393,6 +1398,182 @@ struct nvmf_property_get_command {

      __u8        resv4[16];
  };
+struct nvmf_auth_send_command {
+    __u8        opcode;
+    __u8        resv1;
+    __u16        command_id;
+    __u8        fctype;
+    __u8        resv2[19];
+    union nvme_data_ptr dptr;
+    __u8        resv3;
+    __u8        spsp0;
+    __u8        spsp1;
+    __u8        secp;
+    __le32        tl;
+    __u8        resv4[12];

Isn't that 16 bytes?
You should add these to the compile time checkers
in _nvme_check_size.

If you say so ... I'll cross-check.

quoted

+
+};
+
+struct nvmf_auth_receive_command {
+    __u8        opcode;
+    __u8        resv1;
+    __u16        command_id;
+    __u8        fctype;
+    __u8        resv2[19];
+    union nvme_data_ptr dptr;
+    __u8        resv3;
+    __u8        spsp0;
+    __u8        spsp1;
+    __u8        secp;
+    __le32        al;
+    __u8        resv4[12];
+};
+
+/* Value for secp */
+enum {
+    NVME_AUTH_DHCHAP_PROTOCOL_IDENTIFIER    = 0xe9,
+};
+
+/* Defined value for auth_type */
+enum {
+    NVME_AUTH_COMMON_MESSAGES    = 0x00,
+    NVME_AUTH_DHCHAP_MESSAGES    = 0x01,
+};
+
+/* Defined messages for auth_id */
+enum {
+    NVME_AUTH_DHCHAP_MESSAGE_NEGOTIATE    = 0x00,
+    NVME_AUTH_DHCHAP_MESSAGE_CHALLENGE    = 0x01,
+    NVME_AUTH_DHCHAP_MESSAGE_REPLY        = 0x02,
+    NVME_AUTH_DHCHAP_MESSAGE_SUCCESS1    = 0x03,
+    NVME_AUTH_DHCHAP_MESSAGE_SUCCESS2    = 0x04,
+    NVME_AUTH_DHCHAP_MESSAGE_FAILURE2    = 0xf0,
+    NVME_AUTH_DHCHAP_MESSAGE_FAILURE1    = 0xf1,
+};
+
+struct nvmf_auth_dhchap_protocol_descriptor {
+    __u8        authid;
+    __u8        rsvd;
+    __u8        halen;
+    __u8        dhlen;
+    __u8        idlist[60];
+};
+
+enum {
+    NVME_AUTH_DHCHAP_AUTH_ID    = 0x01,
+};
+
+enum {
+    NVME_AUTH_DHCHAP_HASH_SHA256    = 0x01,

Maybe s/HASH/HF/ (stands for hash function, which is
a better description).

Or HMAC, as this is what it's used for...

quoted

+    NVME_AUTH_DHCHAP_HASH_SHA384    = 0x02,
+    NVME_AUTH_DHCHAP_HASH_SHA512    = 0x03,
+};
+
+enum {
+    NVME_AUTH_DHCHAP_DHGROUP_NULL    = 0x00,
+    NVME_AUTH_DHCHAP_DHGROUP_2048    = 0x01,
+    NVME_AUTH_DHCHAP_DHGROUP_3072    = 0x02,
+    NVME_AUTH_DHCHAP_DHGROUP_4096    = 0x03,
+    NVME_AUTH_DHCHAP_DHGROUP_6144    = 0x04,
+    NVME_AUTH_DHCHAP_DHGROUP_8192    = 0x05,
+};
+
+union nvmf_auth_protocol {
+    struct nvmf_auth_dhchap_protocol_descriptor dhchap;
+};
+
+struct nvmf_auth_dhchap_negotiate_data {
+    __u8        auth_type;
+    __u8        auth_id;
+    __u8        rsvd[2];
+    __le16        t_id;
+    __u8        sc_c;
+    __u8        napd;
+    union nvmf_auth_protocol auth_protocol[];
+};
+
+struct nvmf_auth_dhchap_challenge_data {
+    __u8        auth_type;
+    __u8        auth_id;
+    __u8        rsvd1[2];
+    __le16        t_id;
+    __u8        hl;
+    __u8        rsvd2;
+    __u8        hashid;
+    __u8        dhgid;
+    __le16        dhvlen;
+    __le32        seqnum;
+    /* 'hl' bytes of challenge value */
+    __u8        cval[];
+    /* followed by 'dhvlen' bytes of DH value */
+};
+
+struct nvmf_auth_dhchap_reply_data {
+    __u8        auth_type;
+    __u8        auth_id;
+    __u8        rsvd1[2];

Maybe __u32 rsvd1? Usually its done this way in the other
headers...

Ah. Right, will fix.

quoted

+    __le16        t_id;
+    __u8        hl;
+    __u8        rsvd2;
+    __u8        cvalid;
+    __u8        rsvd3;
+    __le16        dhvlen;
+    __le32        seqnum;
+    /* 'hl' bytes of response data */
+    __u8        rval[];
+    /* followed by 'hl' bytes of Challenge value */
+    /* followed by 'dhvlen' bytes of DH value */
+};
+
+enum {
+    NVME_AUTH_DHCHAP_RESPONSE_VALID    = (1 << 0),
+};
+
+struct nvmf_auth_dhchap_success1_data {
+    __u8        auth_type;
+    __u8        auth_id;
+    __u8        rsvd1[2];
+    __le16        t_id;
+    __u8        hl;
+    __u8        rsvd2;
+    __u8        rvalid;
+    __u8        rsvd3[7];
+    /* 'hl' bytes of response value if 'rvalid' is set */
+    __u8        rval[];

It really sucks that we have zero-length pointers in
a wire-format struct... but anyways, it is what it is...

Yeah, and that's not the worst of it; the 'reply' structure has _three_ 
zero-length pointers.
Makes me wonder if I should drop them completely...

quoted

+};
+
+struct nvmf_auth_dhchap_success2_data {
+    __u8        auth_type;
+    __u8        auth_id;
+    __u8        rsvd1[2];
+    __le16        t_id;
+    __u8        rsvd2[10];
+};
+
+struct nvmf_auth_dhchap_failure_data {
+    __u8        auth_type;
+    __u8        auth_id;
+    __u8        rsvd1[2];
+    __le16        t_id;
+    __u8        reason_code;
+    __u8        reason_code_explanation;

I'd maybe do those shorter;
rescode
rescode_exp

Ok.

quoted

+};
+
+enum {
+    NVME_AUTH_DHCHAP_FAILURE_REASON_FAILED    = 0x01,
+};
+
+enum {
+    NVME_AUTH_DHCHAP_FAILURE_FAILED            = 0x01,
+    NVME_AUTH_DHCHAP_FAILURE_NOT_USABLE        = 0x02,
+    NVME_AUTH_DHCHAP_FAILURE_CONCAT_MISMATCH    = 0x03,
+    NVME_AUTH_DHCHAP_FAILURE_HASH_UNUSABLE        = 0x04,
+    NVME_AUTH_DHCHAP_FAILURE_DHGROUP_UNUSABLE    = 0x05,
+    NVME_AUTH_DHCHAP_FAILURE_INVALID_PAYLOAD    = 0x06,
+    NVME_AUTH_DHCHAP_FAILURE_INVALID_MESSAGE    = 0x07,

I think the language in the spec is "incorrect", why not
stick with that instead of "invalid"?

Ah. Things got changed so much during development of the spec that I 
seem to have lost track. Will change it.

quoted

+};
+
+
  struct nvme_dbbuf {
      __u8            opcode;
      __u8            flags;

@@ -1436,6 +1617,8 @@ struct nvme_command {

          struct nvmf_connect_command connect;
          struct nvmf_property_set_command prop_set;
          struct nvmf_property_get_command prop_get;
+        struct nvmf_auth_send_command auth_send;
+        struct nvmf_auth_receive_command auth_receive;
          struct nvme_dbbuf dbbuf;
          struct nvme_directive_cmd directive;
      };

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: Felix Imendörffer