Update JSON handling for TLS keys

[martin-gpy]

As per Hannes:

JSON handling needs to be updated with the latest TLS changes upstream, which introduced a 'tls_configured_key' in addition to the existing 'tls_key' to differentiate between the TLS key used by the current connection (the 'tls_key' attribute) and the TLS key specified on the command line (the 'tls_configured_key' attribute).

With that change we can format the JSON configuration correctly, and only specify the TLS key in the JSON configuration if it had been configured on the command line.

[martin-gpy]

There are also other problems here. It seems the TLS identity version is hardcoded to 0 in the JSON TLS handling:

https://github.com/linux-nvme/libnvme/blob/master/src/nvme/json.c#L52

[igaw]

The specs states under 3.6.1.3 TLS PSK and PSK Identity Derivation

The retained PSK is derived from the configured PSK (refer to section 3.6.1.4). The configured PSK shall
be destroyed as soon as the retained PSK is generated and stored.

The current code is actually doing this, it outputs the retained PSK to the JSON file after a successful nvme connect.

# nvme check-tls-key -i -I 1 -c nqn.io-1 -n nqn.2014-08.org.nvmexpress:uuid:befdec4c-2234-11b2-a85c-ca77c773af36 -d NVMeTLSkey-1:01:PMmRzMi+sRVtOSc6qpz8DaNszOufkp/+P1pp+M6r6/cGUjM9: 
# nvme connect -t tcp -a 192.168.154.148 -s 4420 -q nqn.2014-08.org.nvmexpress:uuid:befdec4c-2234-11b2-a85c-ca77c773af36 -n nqn.io-1 --tls --tls_key=811559380 -O
connecting to device: nvme1
[
  {
    "hostnqn":"nqn.2014-08.org.nvmexpress:uuid:2cd2c43b-a90a-45c1-a8cd-86b33ab273b6",
    "hostid":"2cd2c43b-a90a-45c1-a8cd-86b33ab273b6",
    "subsystems":[
      {
        "nqn":"nqn.io-1",
        "ports":[
          {
            "transport":"tcp",
            "traddr":"192.168.154.144",
            "trsvcid":"4421",
            "dhchap_key":"none"
          },
          {
            "transport":"tcp",
            "traddr":"192.168.154.144",
            "trsvcid":"4420",
            "dhchap_key":"none"
          }
        ]
      }
    ]
  },
  {
    "hostnqn":"nqn.2014-08.org.nvmexpress:uuid:2cd2c43b-a90a-45c1-a8cd-86b33ab273b5",
    "hostid":"2cd2c43b-a90a-45c1-a8cd-86b33ab273b5",
    "subsystems":[
      {
        "nqn":"nqn.io-1",
        "ports":[
          {
            "transport":"tcp",
            "traddr":"192.168.154.144",
            "trsvcid":"4421",
            "dhchap_key":"none"
          },
          {
            "transport":"tcp",
            "traddr":"192.168.154.144",
            "host_iface":"enp52s0f3u1",
            "trsvcid":"4420",
            "dhchap_key":"none"
          }
        ]
      }
    ]
  },
  {
    "hostnqn":"nqn.2014-08.org.nvmexpress:uuid:befdec4c-2234-11b2-a85c-ca77c773af36",
    "hostid":"befdec4c-2234-11b2-a85c-ca77c773af36",
    "subsystems":[
      {
        "nqn":"nqn.io-1",
        "ports":[
          {
            "transport":"tcp",
            "traddr":"192.168.154.144",
            "trsvcid":"4421",
            "dhchap_key":"none"
          }
        ]
      }
    ]
  },
  {
    "hostnqn":"nqn.2014-08.org.nvmexpress:uuid:befdec4c-2234-11b2-a85c-ca77c773af36",
    "hostid":"2cd2c43b-a90a-45c1-a8cd-86b33ab273b6",
    "subsystems":[
      {
        "nqn":"nqn.io-1",
        "ports":[
          {
            "transport":"tcp",
            "traddr":"192.168.154.148",
            "trsvcid":"4420",
            "dhchap_key":"none",
            "tls":true,
            "tls_key":"NVMeTLSkey-1:01:Hhc5sFjwSZ6w5hPY19tqprajYtuYci3tN+Z2wGViDk3rpSR+:"
          }
        ]
      }
    ]
  }
]

[igaw]

The TCP spec states

That means the PSK interchange format contains the configured PSK and thus as reported we should not store the retained PSK.

Also did a bit of digging in the RFCs on HMACs

• https://datatracker.ietf.org/doc/html/rfc8446
• https://datatracker.ietf.org/doc/html/rfc9257
• https://datatracker.ietf.org/doc/html/rfc2104

to figure out what we actually need to store in the JSON file. The PSK interchange format tells us which version (currently hard coded to 1 and which HMAC is used. But let's make this future proof (see linux-nvme/nvme-cli#1669).

https://en.wikipedia.org/wiki/HMAC

[igaw]

With that change we can format the JSON configuration correctly, and only specify the TLS key in the JSON configuration if it had been configured on the command line.

I can't really follow here, where is this tls_configured_key?

Anyway, it is technically impossible to produce a 'correct' JSON output file for the connect case when the key is already in the keystore, because we only store retained keys in the store. The only way to generate the 'correct' JSON output is when the configured key is present during the connect command. But this leads to the problem that key is likely to be stored in the shell history.

[hreinecke]

With that change we can format the JSON configuration correctly, and only specify the TLS key in the JSON configuration if it had been configured on the command line.

I can't really follow here, where is this tls_configured_key?

Anyway, it is technically impossible to produce a 'correct' JSON output file for the connect case when the key is already in the keystore, because we only store retained keys in the store. The only way to generate the 'correct' JSON output is when the configured key is present during the connect command. But this leads to the problem that key is likely to be stored in the shell history.

'tls_configured_key' is the key specified on the nvme-cli commandline via '--tls_key'. Which makes it a 'retained' key in Spec parlance.

[hreinecke]

As mentioned, I really want to avoid having to specify the TLS key in interchange format on the commandline. That will allow any unprivileged process to read the key data and cause a security nightmare.
And also, as mentioned elsewhere, the '--tls_key' option really is for debugging only, and should be discouraged from normal operations. Specifying the key on the commandline will make key rotating impossible, and should only be used in exceptional circumstances, not during normal operations.

[hreinecke]

So, my recommendation would be this:

• Continue to require all TLS keys to be present in the keystore before calling 'nvme connect' (or 'nvme discover')
• Retain the -tls_key option to refer to keys via a key ID
• Export the 'tls' setting to the JSON configuration if any non-zero value is given in the 'tls_key' sysfs attribute
• If a non-zero value is given in the 'tls_configured_key' sysfs attribute (which reflects the value specified via '-tls_key' once the connection is established) export the specified key in PSK interchange format in the JSON configuration.

[hreinecke]

But that means we're doing things correctly already, and nothing is to be done.

[igaw]

But that means we're doing things correctly already, and nothing is to be done.

Yep, this is what I also thought. Some parts of the refactoring/extension in #894 makes sense. Maybe even the nvme_ctrl_set_tls_key etc part, just not the nvme-cli command line change.

[igaw]

Just verfied that nvme tls export and nvme tls import do not do any operations on the keys, such as deriving a new key when inserting a key into the keystore. So all good there.

Though when loading the JSON file, the key listed under tls_key will be used as configured PSK thus on every load of the JSON file a new key will be generated.