[PATCH 0/8] smb: client: More crypto library conversions

Eric Biggers posted 8 patches 3 months, 4 weeks ago
fs/smb/client/Kconfig         |   7 +-
fs/smb/client/cifsencrypt.c   | 201 +++++++++++++---------------------
fs/smb/client/cifsfs.c        |   4 -
fs/smb/client/cifsglob.h      |   3 -
fs/smb/client/cifsproto.h     |  10 +-
fs/smb/client/link.c          |  31 +-----
fs/smb/client/sess.c          |   2 +-
fs/smb/client/smb2misc.c      |  53 ++-------
fs/smb/client/smb2proto.h     |   8 +-
fs/smb/client/smb2transport.c | 164 +++++----------------------
10 files changed, 131 insertions(+), 352 deletions(-)
[PATCH 0/8] smb: client: More crypto library conversions
Posted by Eric Biggers 3 months, 4 weeks ago
This series converts fs/smb/client/ to access SHA-512, HMAC-SHA256, MD5,
and HMAC-MD5 using the library APIs instead of crypto_shash.

This simplifies the code significantly.  It also slightly improves
performance, as it eliminates unnecessary overhead.

Tested with Samba with all SMB versions, with mfsymlinks in the mount
options, 'server min protocol = NT1' and 'server signing = required' in
smb.conf, and doing a simple file data and symlink verification test.
That seems to cover all the modified code paths.

However, with SMB 1.0 I get "CIFS: VFS: SMB signature verification
returned error = -13", regardless of whether this series is applied or
not.  Presumably, testing that case requires some other setting I
couldn't find.

Regardless, these are straightforward conversions and all the actual
crypto is exactly the same as before, as far as I can tell.

Eric Biggers (8):
  smb: client: Use SHA-512 library for SMB3.1.1 preauth hash
  smb: client: Use HMAC-SHA256 library for key generation
  smb: client: Use HMAC-SHA256 library for SMB2 signature calculation
  smb: client: Use MD5 library for M-F symlink hashing
  smb: client: Use MD5 library for SMB1 signature calculation
  smb: client: Use HMAC-MD5 library for NTLMv2
  smb: client: Remove obsolete crypto_shash allocations
  smb: client: Consolidate cmac(aes) shash allocation

 fs/smb/client/Kconfig         |   7 +-
 fs/smb/client/cifsencrypt.c   | 201 +++++++++++++---------------------
 fs/smb/client/cifsfs.c        |   4 -
 fs/smb/client/cifsglob.h      |   3 -
 fs/smb/client/cifsproto.h     |  10 +-
 fs/smb/client/link.c          |  31 +-----
 fs/smb/client/sess.c          |   2 +-
 fs/smb/client/smb2misc.c      |  53 ++-------
 fs/smb/client/smb2proto.h     |   8 +-
 fs/smb/client/smb2transport.c | 164 +++++----------------------
 10 files changed, 131 insertions(+), 352 deletions(-)


base-commit: 67029a49db6c1f21106a1b5fcdd0ea234a6e0711
-- 
2.51.0
Re: [PATCH 0/8] smb: client: More crypto library conversions
Posted by Eric Biggers 3 months, 3 weeks ago
On Sat, Oct 11, 2025 at 06:57:30PM -0700, Eric Biggers wrote:
> This series converts fs/smb/client/ to access SHA-512, HMAC-SHA256, MD5,
> and HMAC-MD5 using the library APIs instead of crypto_shash.
> 
> This simplifies the code significantly.  It also slightly improves
> performance, as it eliminates unnecessary overhead.
> 
> Tested with Samba with all SMB versions, with mfsymlinks in the mount
> options, 'server min protocol = NT1' and 'server signing = required' in
> smb.conf, and doing a simple file data and symlink verification test.
> That seems to cover all the modified code paths.
> 
> However, with SMB 1.0 I get "CIFS: VFS: SMB signature verification
> returned error = -13", regardless of whether this series is applied or
> not.  Presumably, testing that case requires some other setting I
> couldn't find.
> 
> Regardless, these are straightforward conversions and all the actual
> crypto is exactly the same as before, as far as I can tell.
> 
> Eric Biggers (8):
>   smb: client: Use SHA-512 library for SMB3.1.1 preauth hash
>   smb: client: Use HMAC-SHA256 library for key generation
>   smb: client: Use HMAC-SHA256 library for SMB2 signature calculation
>   smb: client: Use MD5 library for M-F symlink hashing
>   smb: client: Use MD5 library for SMB1 signature calculation
>   smb: client: Use HMAC-MD5 library for NTLMv2
>   smb: client: Remove obsolete crypto_shash allocations
>   smb: client: Consolidate cmac(aes) shash allocation

As requested off-list, here are some (micro)benchmark results for this
series.  The CPU was AMD Ryzen 9 9950X.  The server was Samba running on
localhost.  Message signing was enabled.  A valid username and password
were given in the mount options.  The "Improvement" column is the
percent change in throughput (reciprocal cycles):

           Microbenchmark               Before      After   Improvement
           ==============               ======      =====   ===========

    1. Total cycles spent in             44548      20081      122%
    smb311_update_preauth_hash()
    during SMB 3.1.1 mount
    (4 calls total)

    2. setup_ntlmv2_rsp() cycles         31777      22231       43%

    3. Total cycles spent in             17802      22876      -22%
    generate_key()
    during SMB 3.1.1 mount
    (3 calls total)

    4. Total cycles spent in            205110      87204      135%
    smb2_calc_signature()
    during SMB 2.0 mount
    (26 calls & 3316 bytes total)

    5. Total cycles spent in          22689767   21043125        8%
    smb2_calc_signature()
    reading 10MB file using SMB 2.0
    (316 calls & 10031077 bytes total)

    6. Total cycles spent in             56803      37840       50%
    cifs_calc_signature()
    during SMB 1.0 mount
    (18 calls & 1551 bytes total)

    7. Total cycles spent in          52669066   51974573        1%
    cifs_calc_signature()
    reading 10MB file using SMB 1.0
    (336 calls & 10021426 bytes total)

    8. parse_mf_symlink() cycles          7654       4902       56%

Note: case 3 regressed because the "cmac(aes)" allocation moved from
smb311_update_preauth_hash (case 1) to generate_key (case 3).  Excluding
that allocation, generate_key got faster.  Likewise, the sum of cases 1,
2, and 3 (which all occurred at mount time) got faster.

There was a greater speedup in signature calculation than I expected.
It's probably because many SMB messages are short (especially the ones
exchanged at mount time), and also because the old code allocated new
crypto_shash objects more frequently than I had thought.  The SMB 2.0
code allocated a new "hmac(sha256)" crypto_shash for every other message
signed.  That overhead is all gone after switching to the library.

TLDR: all SMB crypto calculations (SHA-512, HMAC-SHA256, MD5, HMAC-MD5)
affected by this series are faster now.  The library APIs fix the
unnecessary overhead that the traditional crypto API has.  Of course,
it's also a lot easier to use as well.

- Eric
Re: [PATCH 0/8] smb: client: More crypto library conversions
Posted by Steve French 3 months, 3 weeks ago
> with SMB 1.0 I get "CIFS: VFS: SMB signature verification
> returned error = -13",

If testing SMB1 to Samba the server disabled signing unless I set
  "server signing = mandatory"
in smb.conf.  But with that, signing with your patches worked fine even to SMB1

Were you testing to Samba with SMB1?

On Mon, Oct 13, 2025 at 10:44 PM Eric Biggers <ebiggers@kernel.org> wrote:
>
> On Sat, Oct 11, 2025 at 06:57:30PM -0700, Eric Biggers wrote:
> > This series converts fs/smb/client/ to access SHA-512, HMAC-SHA256, MD5,
> > and HMAC-MD5 using the library APIs instead of crypto_shash.
> >
> > This simplifies the code significantly.  It also slightly improves
> > performance, as it eliminates unnecessary overhead.
> >
> > Tested with Samba with all SMB versions, with mfsymlinks in the mount
> > options, 'server min protocol = NT1' and 'server signing = required' in
> > smb.conf, and doing a simple file data and symlink verification test.
> > That seems to cover all the modified code paths.
> >
> > However, with SMB 1.0 I get "CIFS: VFS: SMB signature verification
> > returned error = -13", regardless of whether this series is applied or
> > not.  Presumably, testing that case requires some other setting I
> > couldn't find.
> >
> > Regardless, these are straightforward conversions and all the actual
> > crypto is exactly the same as before, as far as I can tell.
> >
> > Eric Biggers (8):
> >   smb: client: Use SHA-512 library for SMB3.1.1 preauth hash
> >   smb: client: Use HMAC-SHA256 library for key generation
> >   smb: client: Use HMAC-SHA256 library for SMB2 signature calculation
> >   smb: client: Use MD5 library for M-F symlink hashing
> >   smb: client: Use MD5 library for SMB1 signature calculation
> >   smb: client: Use HMAC-MD5 library for NTLMv2
> >   smb: client: Remove obsolete crypto_shash allocations
> >   smb: client: Consolidate cmac(aes) shash allocation
>
> As requested off-list, here are some (micro)benchmark results for this
> series.  The CPU was AMD Ryzen 9 9950X.  The server was Samba running on
> localhost.  Message signing was enabled.  A valid username and password
> were given in the mount options.  The "Improvement" column is the
> percent change in throughput (reciprocal cycles):
>
>            Microbenchmark               Before      After   Improvement
>            ==============               ======      =====   ===========
>
>     1. Total cycles spent in             44548      20081      122%
>     smb311_update_preauth_hash()
>     during SMB 3.1.1 mount
>     (4 calls total)
>
>     2. setup_ntlmv2_rsp() cycles         31777      22231       43%
>
>     3. Total cycles spent in             17802      22876      -22%
>     generate_key()
>     during SMB 3.1.1 mount
>     (3 calls total)
>
>     4. Total cycles spent in            205110      87204      135%
>     smb2_calc_signature()
>     during SMB 2.0 mount
>     (26 calls & 3316 bytes total)
>
>     5. Total cycles spent in          22689767   21043125        8%
>     smb2_calc_signature()
>     reading 10MB file using SMB 2.0
>     (316 calls & 10031077 bytes total)
>
>     6. Total cycles spent in             56803      37840       50%
>     cifs_calc_signature()
>     during SMB 1.0 mount
>     (18 calls & 1551 bytes total)
>
>     7. Total cycles spent in          52669066   51974573        1%
>     cifs_calc_signature()
>     reading 10MB file using SMB 1.0
>     (336 calls & 10021426 bytes total)
>
>     8. parse_mf_symlink() cycles          7654       4902       56%
>
> Note: case 3 regressed because the "cmac(aes)" allocation moved from
> smb311_update_preauth_hash (case 1) to generate_key (case 3).  Excluding
> that allocation, generate_key got faster.  Likewise, the sum of cases 1,
> 2, and 3 (which all occurred at mount time) got faster.
>
> There was a greater speedup in signature calculation than I expected.
> It's probably because many SMB messages are short (especially the ones
> exchanged at mount time), and also because the old code allocated new
> crypto_shash objects more frequently than I had thought.  The SMB 2.0
> code allocated a new "hmac(sha256)" crypto_shash for every other message
> signed.  That overhead is all gone after switching to the library.
>
> TLDR: all SMB crypto calculations (SHA-512, HMAC-SHA256, MD5, HMAC-MD5)
> affected by this series are faster now.  The library APIs fix the
> unnecessary overhead that the traditional crypto API has.  Of course,
> it's also a lot easier to use as well.
>
> - Eric
>


-- 
Thanks,

Steve
Re: [PATCH 0/8] smb: client: More crypto library conversions
Posted by Eric Biggers 3 months, 3 weeks ago
On Fri, Oct 17, 2025 at 11:12:58AM -0500, Steve French wrote:
> > with SMB 1.0 I get "CIFS: VFS: SMB signature verification
> > returned error = -13",
> 
> If testing SMB1 to Samba the server disabled signing unless I set
>   "server signing = mandatory"
> in smb.conf.  But with that, signing with your patches worked fine even to SMB1
> 
> Were you testing to Samba with SMB1?

As per my cover letter, these are the settings I used:

    Tested with Samba with all SMB versions, with mfsymlinks in the
    mount options, 'server min protocol = NT1' and 'server signing =
    required' in smb.conf, and doing a simple file data and symlink
    verification test.  That seems to cover all the modified code paths.

This was with Samba 4.23.1.

I just tried 'server signing = mandatory' too (just in case that's
different from "required"), and I still get the error.

Anyway, it's not related to my patchset, as it happens regardless of it.
I also tried some much older kernels, and it still happens there too.

- Eric
Re: [PATCH 0/8] smb: client: More crypto library conversions
Posted by Enzo Matsumiya 3 months, 3 weeks ago
Hi Eric,

On 10/11, Eric Biggers wrote:
>This series converts fs/smb/client/ to access SHA-512, HMAC-SHA256, MD5,
>and HMAC-MD5 using the library APIs instead of crypto_shash.
>
>This simplifies the code significantly.  It also slightly improves
>performance, as it eliminates unnecessary overhead.
>
>Tested with Samba with all SMB versions, with mfsymlinks in the mount
>options, 'server min protocol = NT1' and 'server signing = required' in
>smb.conf, and doing a simple file data and symlink verification test.
>That seems to cover all the modified code paths.
>
>However, with SMB 1.0 I get "CIFS: VFS: SMB signature verification
>returned error = -13", regardless of whether this series is applied or
>not.  Presumably, testing that case requires some other setting I
>couldn't find.
>
>Regardless, these are straightforward conversions and all the actual
>crypto is exactly the same as before, as far as I can tell.

I think the overall series looks good and do a great cleanup.

Just a minor nit about fips_enabled: since it's now being handled
explicitly (rather than an error on cifs_alloc_hash() currently), I
think it makes sense to move the check to mount code path when
'sectype == NTLMv2' (I don't particularly care about SMB1, but
something similar can be done for 'smb1 && sign' cases I guess).

>Eric Biggers (8):
>  smb: client: Use SHA-512 library for SMB3.1.1 preauth hash
>  smb: client: Use HMAC-SHA256 library for key generation
>  smb: client: Use HMAC-SHA256 library for SMB2 signature calculation
>  smb: client: Use MD5 library for M-F symlink hashing
>  smb: client: Use MD5 library for SMB1 signature calculation
>  smb: client: Use HMAC-MD5 library for NTLMv2
>  smb: client: Remove obsolete crypto_shash allocations
>  smb: client: Consolidate cmac(aes) shash allocation
>
> fs/smb/client/Kconfig         |   7 +-
> fs/smb/client/cifsencrypt.c   | 201 +++++++++++++---------------------
> fs/smb/client/cifsfs.c        |   4 -
> fs/smb/client/cifsglob.h      |   3 -
> fs/smb/client/cifsproto.h     |  10 +-
> fs/smb/client/link.c          |  31 +-----
> fs/smb/client/sess.c          |   2 +-
> fs/smb/client/smb2misc.c      |  53 ++-------
> fs/smb/client/smb2proto.h     |   8 +-
> fs/smb/client/smb2transport.c | 164 +++++----------------------
> 10 files changed, 131 insertions(+), 352 deletions(-)
>
>
>base-commit: 67029a49db6c1f21106a1b5fcdd0ea234a6e0711
>-- 
>2.51.0


Cheers,

Enzo
Re: [PATCH 0/8] smb: client: More crypto library conversions
Posted by Eric Biggers 3 months, 3 weeks ago
On Mon, Oct 13, 2025 at 11:44:37AM -0300, Enzo Matsumiya wrote:
> Hi Eric,
> 
> On 10/11, Eric Biggers wrote:
> > This series converts fs/smb/client/ to access SHA-512, HMAC-SHA256, MD5,
> > and HMAC-MD5 using the library APIs instead of crypto_shash.
> > 
> > This simplifies the code significantly.  It also slightly improves
> > performance, as it eliminates unnecessary overhead.
> > 
> > Tested with Samba with all SMB versions, with mfsymlinks in the mount
> > options, 'server min protocol = NT1' and 'server signing = required' in
> > smb.conf, and doing a simple file data and symlink verification test.
> > That seems to cover all the modified code paths.
> > 
> > However, with SMB 1.0 I get "CIFS: VFS: SMB signature verification
> > returned error = -13", regardless of whether this series is applied or
> > not.  Presumably, testing that case requires some other setting I
> > couldn't find.
> > 
> > Regardless, these are straightforward conversions and all the actual
> > crypto is exactly the same as before, as far as I can tell.
> 
> I think the overall series looks good and do a great cleanup.
> 
> Just a minor nit about fips_enabled: since it's now being handled
> explicitly (rather than an error on cifs_alloc_hash() currently), I
> think it makes sense to move the check to mount code path when
> 'sectype == NTLMv2' (I don't particularly care about SMB1, but
> something similar can be done for 'smb1 && sign' cases I guess).

For MD5 message signing and NTLMv2, this series keeps the fips_enabled
checks where they were before.  That is, they're inserted where the
calls to cifs_alloc_hash() were before.  I think moving them to earlier
locations is best done in later patches, as it's not obvious (at least
to me) exactly how to do that.

I spent a while reading the code again, and this is what I came up with:

- For MD5 message signing: cifs_enable_signing() is probably the right
  place to disallow it.  However, it's called regardless of the signing
  algorithm.  So it needs a parameter passed in that tells it that the
  signing algorithm will be MD5 (or equivalently the dialect is SMB1).

- For NTLMv2: select_sec() and SMB2_select_sec() are where the
  authentication protocol is selected and are probably the right places
  to disallow NTLMv2 and RawNTLMSSP.  However, those are two places, not
  one.  We'd have to remember to put the fips_enabled check in both.

The nice thing about keeping the fips_enabled checks next to the actual
uses of the algorithms is that it ensures they stay in sync.  So maybe
we should just stay with that here.

- Eric
Re: [PATCH 0/8] smb: client: More crypto library conversions
Posted by Ard Biesheuvel 3 months, 3 weeks ago
On Sun, 12 Oct 2025 at 03:59, Eric Biggers <ebiggers@kernel.org> wrote:
>
> This series converts fs/smb/client/ to access SHA-512, HMAC-SHA256, MD5,
> and HMAC-MD5 using the library APIs instead of crypto_shash.
>
> This simplifies the code significantly.  It also slightly improves
> performance, as it eliminates unnecessary overhead.
>
> Tested with Samba with all SMB versions, with mfsymlinks in the mount
> options, 'server min protocol = NT1' and 'server signing = required' in
> smb.conf, and doing a simple file data and symlink verification test.
> That seems to cover all the modified code paths.
>
> However, with SMB 1.0 I get "CIFS: VFS: SMB signature verification
> returned error = -13", regardless of whether this series is applied or
> not.  Presumably, testing that case requires some other setting I
> couldn't find.
>
> Regardless, these are straightforward conversions and all the actual
> crypto is exactly the same as before, as far as I can tell.
>
> Eric Biggers (8):
>   smb: client: Use SHA-512 library for SMB3.1.1 preauth hash
>   smb: client: Use HMAC-SHA256 library for key generation
>   smb: client: Use HMAC-SHA256 library for SMB2 signature calculation
>   smb: client: Use MD5 library for M-F symlink hashing
>   smb: client: Use MD5 library for SMB1 signature calculation
>   smb: client: Use HMAC-MD5 library for NTLMv2
>   smb: client: Remove obsolete crypto_shash allocations
>   smb: client: Consolidate cmac(aes) shash allocation
>

Acked-by: Ard Biesheuvel <ardb@kernel.org>