nvme: testcases for TLS support #158
Conversation
tests/nvme/059
Outdated
| return 1 | ||
| fi | ||
|
|
||
| systemctl start tlshd |
There was a problem hiding this comment.
I think you need to check that it exists as a dependency
There was a problem hiding this comment.
Maybe also check the version of ktls-utils?
Or just explain in a comment if you have any expectations from it?
There was a problem hiding this comment.
Yeah, good point. Will check what we can do here.
There was a problem hiding this comment.
According to "man systemctl" "EXIT STATUS" section, systemctl command returns exit status "4" for "no such unit". So it would work to check if "systemctl status tlshd" command's exist status is 4 or not.
I use Fedora, and needed to install "ktls-utils" package to run the test case. It would be the better to mention the word "ktls-utils" in the SKIP_REASONS message to help users to understand what is missing.
tests/nvme/059
Outdated
| _nvmet_target_setup --blkdev file --tls | ||
|
|
||
| # Test unencrypted connection | ||
| echo "Test unencrypted connection w/ tls not required" |
There was a problem hiding this comment.
umm, looks pretty useless...
There was a problem hiding this comment.
Don't think so. This is testing the 'not required' setting in nvmet, which should accept both TLS and non-TLS connections even if TLS is enabled on the target.
tests/nvme/059
Outdated
| echo "WARNING: connection is not encrypted" | ||
| fi | ||
|
|
||
| _nvme_disconnect_subsys |
There was a problem hiding this comment.
is there any room to test passing explicit keys and private keyrings to this test?
There was a problem hiding this comment.
I'd rather not do that here. This is for testing the 'default' case, where PSKs are pre-populated in the keyring and the connection picks up the keys automatically. Explicit keys and keyrings are really just for testing.
But we should have a separate testcase for that, true.
There was a problem hiding this comment.
The commit bc544f8 introduces the --concat option of _nvme_connect_subsys(), but it is not used anywhere. Do we need this commit in this PR? If it is a preparation for the next PR, I suggest to move this commit to that PR.
|
@hreinecke Thanks for rebasing the series. I ran the test case in my environment using the kernel v6.13 and the latest nvme-cli (2.10.2-77-gb4628c3, with libnvme 1.11.1-48-gacc19fc), but it fails. 059.full file left logs as follows: kernel message was as follows: I'm not sure if this catches a kernel bug. Still the test case may need improvement, or I may be missing something. If you have any insights about this failure, please let me know. |
Okay, I'll fix it up. |
It would if I had pushed the testcase for secure concatenation... |
_check_ctrl_tls() need to redirect stderr to /dev/null, not stdout (as it does now on two occasions). Will be fixing up the testcase. |
|
@hreinecke Thanks for updating the patches. Question, which kernel should I use to run the test case? nvme/059 failure looks like this. Also, nvme/060 run terminated in the middle. Kernel reported a BUG. |
|
I tried the updated branch hreinecke/tls.v3 branch, using v6.14-rc1 kernel and "[PATCHv14 00/10] nvme: implement secure concatenation" series. Still I observed the nvme/059 failure and the "BUG: KASAN: slab-out-of-bounds" at nvme/060. |
|
@hreinecke I took a closer look, and succeeded to make nvme/059 and nvme/060 on my test node. I fell in two pit-falls: 1) I did not use the latest tlshd and ktls-utils, and 2) I used v6.14-rcX kernels. I used linux-block/block-6.14 branch kernel with your patch series ""[PATCHv14 00/10] nvme: implement secure concatenation", then I managed to make the test cases pass. To make the nvme/060 pass, I needed several fixes in nvme/060. Please refer to this commit. I also created some test cases for blktests common scripts to,
My patches are available here. Please take a looks and see if it makes sense. If you like it, I can post the whole series to linux-nvme and linux-block lists for wider review. Or, I can post my changes separately, and after they got settled to the master branch, then you can rebase this series later on. Please let me know your preference on the next action. |
|
FYI, I ran nvme/059 with my blktests patch on top of your blktests patches, using the kernel v6.14-rc3 and the patch "[PATCHv14 00/10] nvme: implement secure concatenation". Then I got the kernel message below, the the test hanged. |
Maybe you should follow that advise? |
|
But yeah, it clearly shouldn't crash. I'll check. |
|
But this looks again like the infamous page reference issue. If you have compound pages the page reference is only set on the first page, not the following ones. So if you then walk the pages one-by-one, and release the page reference on each page you see this error. |
|
For reference:
And this should have been prevented by the call to 'sendpage_ok()' in nvme_tcp_try_send_data(). Hmm. |
To start TLS-encrypted connections. Signed-off-by: Hannes Reinecke <[email protected]>
Add --tls option to _create_nvmet_subsystem and allow to specify the tls requirements in _create_nvmet_port. Signed-off-by: Hannes Reinecke <[email protected]>
To check that the test system has a specific systemctl unit, introduce the new helper function _have_systemctl_unit. Signed-off-by: Shin'ichiro Kawasaki <[email protected]> Signed-off-by: Hannes Reinecke <[email protected]>
TCP connections can be encrypted using in-kernel TLS, so add a testcase to exercise the various combinations. Signed-off-by: Hannes Reinecke <[email protected]>
To start secure concatenation the option '--concat' has to be passed to the 'nvme connect' command. Signed-off-by: Hannes Reinecke <[email protected]>
NVMe-TCP has a 'secure concatenation' mode, where the TLS PSK is generated from the secret negotiated by the DH-HMAC-CHAP authentication, and the TLS connection is started after authentication. Signed-off-by: Hannes Reinecke <[email protected]>
Turns out there's an issue with 6.14 (cf my post to linux-nvme just now). Doesn't seem to be related to the concat patches (as it's happening with the stock kernel, too), but you never know. |
This pull request adds two new testcases for nvme TLS support, one for 'plain' TLS with TLS PSKs, and the other one for testing 'secure concatenation' where TLS is started after DH-HMAC-CHAP authentication.