search

sahlberg / libnfs

NFS client library
Other
510 stars 200 forks source link

Add RPC-with-TLS support #457

Closed linuxsmiths closed 4 months ago

linuxsmiths commented 4 months ago

This PR adds RPC-with-TLS (RFC 9289) support to libnfs. It depends on the following:

It adds a new mount option - xprtsec=[none,tls,mtls] (similar to Linux NFS client)

I've currently enabled it only on Linux since that's what my target system is. BSD should be easy to make work if someone has a system they can try out. Also, this change only adds client support. I've only made changes to the CMake build files.

Tested to make sure it works fine with reconnects.

Sample command:

./examples/nfs-writefile /mnt/random 'nfs://nfsserver/mnt/testfile?version=3&xprtsec=tls&debug=1'

Performance testing: With AES-128-GCM there's almost no overhead and I could get ~1GBps/core which is same as w/o TLS. This is because kTLS does the encryption while copying data from user to kernel (so there's no extra copy) and encryption is accelerated by the CPU.

sahlberg commented 4 months ago

Very nice. I will review this over the weekend. Until then, can you add a licence boilerplate to the two new files? LGPLv2.1 or compatible. I am fine with BSD licence too.

On Thu, 25 Apr 2024 at 19:58, linuxsmiths @.***> wrote:

This PR adds RPC-with-TLS (RFC 9289) support to libnfs. It depends on the following:

  • GnuTLS version 3.4.6+
  • Linux kernel 5.10+

I've currently enabled it only on Linux since that's what my target system is. BSD should be easy to make work if someone has a system they can try out. Also, this change only adds client support. I've only made changes to the CMake build files.

Tested to make sure it works fine with reconnects.

You can view, comment on, or merge this pull request online at:

https://github.com/sahlberg/libnfs/pull/457 Commit Summary

File Changes

(19 files https://github.com/sahlberg/libnfs/pull/457/files)

Patch Links:

— Reply to this email directly, view it on GitHub https://github.com/sahlberg/libnfs/pull/457, or unsubscribe https://github.com/notifications/unsubscribe-auth/AADY3EGEST5SMGTB3IBY5DDY7DHTRAVCNFSM6AAAAABGYRHAT2VHI2DSMVQWIX3LMV43ASLTON2WKOZSGI3DGMJXG4YDSNY . You are receiving this because you are subscribed to this thread.Message ID: @.***>

linuxsmiths commented 4 months ago

Please have a look at the build failures too. OSX and Windows look like they might just be unrelated environmental issues since they shouldn't be using TLS anyway (and the compile would fail if they did).

Please check the linux failure though.

Please have a look at the build failures too. OSX and Windows look like they might just be unrelated environmental issues since they shouldn't be using TLS anyway (and the compile would fail if they did).

Please check the linux failure though.

I don't see this failure when I try locally.. could it be transient? Have you seen it fail before?

/home/linuxsmiths_libnfs/tls-change1/libnfs# sudo bash -c "cd build/tests;./test_0100_ls_basic.sh" basic ls test Testing nfs-ls on root of export ... [OK] Create a file and verify nfs-ls can see it ... [OK]

linuxsmiths commented 4 months ago

Please have a look at the build failures too. OSX and Windows look like they might just be unrelated environmental issues since they shouldn't be using TLS anyway (and the compile would fail if they did).

Please check the linux failure though.

Btw, did you notice that I haven't update the autobuild files as I'm not familiar with that. Do we plan to support both or just the cmake?

sahlberg commented 4 months ago

Please have a look at the build failures too. OSX and Windows look like they might just be unrelated environmental issues since they shouldn't be using TLS anyway (and the compile would fail if they did). Please check the linux failure though.

Please have a look at the build failures too. OSX and Windows look like they might just be unrelated environmental issues since they shouldn't be using TLS anyway (and the compile would fail if they did). Please check the linux failure though.

I don't see this failure when I try locally.. could it be transient? Have you seen it fail before?

/home/linuxsmiths_libnfs/tls-change1/libnfs# sudo bash -c "cd build/tests;./test_0100_ls_basic.sh" basic ls test Testing nfs-ls on root of export ... [OK] Create a file and verify nfs-ls can see it ... [OK]

I have not seen this failure before but I accept it might be environmental or transient. If it returns I will debug it and fix it.

sahlberg commented 4 months ago

Clicked the wrong button. I meant to resolve the discussion, not close the pull request. Sorry.

It is merged now. Thank you a lot for this contribution. This is great.

sahlberg commented 4 months ago

On Fri, 26 Apr 2024 at 18:40, linuxsmiths @.***> wrote:

Please have a look at the build failures too. OSX and Windows look like they might just be unrelated environmental issues since they shouldn't be using TLS anyway (and the compile would fail if they did).

Please check the linux failure though.

Btw, did you notice that I haven't update the autobuild files as I'm not familiar with that. Do we plan to support both or just the cmake?

I didn't actually go back and check. Don't worry about it. I am much more familiar with autotools than CMake so I can easily add autotool support to it later. No problem. Thankyou a lot for this contribution!

—

Reply to this email directly, view it on GitHub https://github.com/sahlberg/libnfs/pull/457#issuecomment-2078918014, or unsubscribe https://github.com/notifications/unsubscribe-auth/AADY3EAJ2AUF62VTQDQUO6DY7IHJJAVCNFSM6AAAAABGYRHAT2VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDANZYHEYTQMBRGQ . You are receiving this because you commented.Message ID: @.***>