Bug 14195 - gnutls_hash errors ignored
Summary: gnutls_hash errors ignored
Alias: None
Product: Samba 4.1 and newer
Classification: Unclassified
Component: Other (show other bugs)
Version: 4.11.2
Hardware: All All
: P5 normal (vote)
Target Milestone: ---
Assignee: Andreas Schneider
QA Contact: Samba QA Contact
Depends on:
Reported: 2019-11-12 23:16 UTC by Andrew Bartlett
Modified: 2021-03-18 22:51 UTC (History)
2 users (show)

See Also:


Note You need to log in before you can comment on or make changes to this bug.
Description Andrew Bartlett 2019-11-12 23:16:24 UTC
We ignore do not return errors from gnutls_hash() and similar in these functions:
 - sign_outgoing_message (MD5, source4 client lib)

 - netlogon_creds_server_init ignores the errors from netlogon_creds_init_128bit() (MD5).  Consequence would be a zero session key server-side.

 - netlogon_creds_server_init ignores errors from netlogon_creds_first_step() -> netlogon_creds_step_crypt().  (AES)

 - netlogon_creds_step_crypt ignores errors from netlogon_creds_aes_encrypt() (AES)

 - SMBsesskeygen_ntv2 ignores errors from gnutls_hmac_fast
 - SMBOWFencrypt_ntv2 ignores errors from gnutls_hmac()
 - both protected because if MD5 disallowed ntv2_owf_gen() would fail first

 - encode_or_decode_arc4_passwd_buffer() returns void so would leave an unencrypted pw buffer in 4.11
 - encode_wkssvc_join_password_buffer() returns void so would leave an unencrypted pw buffer in 4.11
 - E_md5hash() returns void so would leave hash_out unchanged (used for reading legacy password history in S3 code)
 - smb_key_derivation ignores errors from gnutls_hmac_fast() in 4.11

I think we very narrowly avoid a security issue here because:
 - If FIPS mode were enabled then the server would clearly fail to operate well pretty clearly
 - none of the above allows entry to the server via zeroed out buffers.

But I would like this double-checked in 4.11 and fixes made in master.
Comment 1 Andrew Bartlett 2019-11-18 09:23:08 UTC

I think E_md5hash is the only case left still to do.  Can you replace with direct calls to GnuTLS and remove it?

Also, do you agree with my assessment that we don't need a security release for the 4.11 cases?

Comment 2 Andreas Schneider 2019-11-18 09:31:46 UTC
I already have a patch for E_md5hash(). Yes, I don't really see a problem. I agree with you.
Comment 3 Andrew Bartlett 2019-11-19 05:57:07 UTC
Removing the team embargo so the BUG references on the patches make sense.

Thanks for all your help on this!
Comment 5 Andrew Bartlett 2021-03-18 22:51:15 UTC
Fixed in 3db2ca2d and 32e75bb4