From ec2aecb4e5cea99a088cc65b8806810171bfdc0d Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Tue, 1 Feb 2022 10:52:27 +0100 Subject: [PATCH 1/3] selftest/quick: add smb2.session We run the quicktest on each linux distro as part of samba-o3 builds. We should make sure smb2 signing/enctyption works on all of them and all different system libraries. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14968 Signed-off-by: Stefan Metzmacher Reviewed-by: Andreas Schneider (cherry picked from commit 68e62962b08497da8359ddbe4324443818c05cd1) --- selftest/quick | 1 + 1 file changed, 1 insertion(+) diff --git a/selftest/quick b/selftest/quick index 0e79f1020bf1..6700180c2c2a 100644 --- a/selftest/quick +++ b/selftest/quick @@ -33,6 +33,7 @@ rpc.join rpc.handles rpc.echo smb.signing +smb2.session drs.unit samba4.blackbox.dbcheck.dc # This needs to be here to get testing of crypt_r() -- 2.25.1 From 17a0b6cbebfb4e9efd0550ca35d1f59ce47fdf55 Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Mon, 31 Jan 2022 20:33:43 +0100 Subject: [PATCH 2/3] libcli/smb: fix error checking in smb2_signing_decrypt_pdu() invalid ptext_len When the ptext_size != m_total check fails, we call this: status = gnutls_error_to_ntstatus(rc, NT_STATUS_INTERNAL_ERROR); goto out; As rc is 0 at that point we'll exit smb2_signing_decrypt_pdu() with NT_STATUS_OK, but without copying the decrypted data back into the callers buffer. Which leads to strange errors in the caller. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14968 Signed-off-by: Stefan Metzmacher Reviewed-by: Andreas Schneider (cherry picked from commit 99182af4ab5a3413311e27c2a193e09babceb01c) --- libcli/smb/smb2_signing.c | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/libcli/smb/smb2_signing.c b/libcli/smb/smb2_signing.c index d036fd95918a..c808f0bda155 100644 --- a/libcli/smb/smb2_signing.c +++ b/libcli/smb/smb2_signing.c @@ -773,9 +773,16 @@ NTSTATUS smb2_signing_decrypt_pdu(struct smb2_signing_key *decryption_key, ctext_size, ptext, &ptext_size); - if (rc < 0 || ptext_size != m_total) { + if (rc < 0) { + TALLOC_FREE(ptext); + TALLOC_FREE(ctext); + status = gnutls_error_to_ntstatus(rc, NT_STATUS_INTERNAL_ERROR); + goto out; + } + if (ptext_size != m_total) { TALLOC_FREE(ptext); TALLOC_FREE(ctext); + rc = GNUTLS_E_SHORT_MEMORY_BUFFER; status = gnutls_error_to_ntstatus(rc, NT_STATUS_INTERNAL_ERROR); goto out; } -- 2.25.1 From af75c0ce7af1192d9347dc4e61daa6c09610d8b2 Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Mon, 31 Jan 2022 20:33:43 +0100 Subject: [PATCH 3/3] libcli/smb: let smb2_signing_decrypt_pdu() cope with gnutls_aead_cipher_decrypt() ptext_len bug The initial implementation of gnutls_aead_cipher_decrypt() had a bug and used: *ptext_len = ctext_len; instead of: *ptext_len = ctext_len - tag_size; This got fixed with gnutls 3.5.2. As we only require gnutls 3.4.7 we need to cope with this... BUG: https://bugzilla.samba.org/show_bug.cgi?id=14968 Signed-off-by: Stefan Metzmacher Reviewed-by: Andreas Schneider Autobuild-User(master): Stefan Metzmacher Autobuild-Date(master): Wed Feb 2 18:29:08 UTC 2022 on sn-devel-184 (cherry picked from commit 735f3d7dde3daf5d0af2e8a1de60422b88663992) --- libcli/smb/smb2_signing.c | 15 +++++++++++++++ wscript_configure_system_gnutls | 3 +++ 2 files changed, 18 insertions(+) diff --git a/libcli/smb/smb2_signing.c b/libcli/smb/smb2_signing.c index c808f0bda155..7411a3314aab 100644 --- a/libcli/smb/smb2_signing.c +++ b/libcli/smb/smb2_signing.c @@ -779,6 +779,21 @@ NTSTATUS smb2_signing_decrypt_pdu(struct smb2_signing_key *decryption_key, status = gnutls_error_to_ntstatus(rc, NT_STATUS_INTERNAL_ERROR); goto out; } +#ifdef HAVE_GNUTLS_AEAD_CIPHER_DECRYPT_PTEXT_LEN_BUG + /* + * Note that gnutls before 3.5.2 had a bug and returned + * *ptext_len = ctext_len, instead of + * *ptext_len = ctext_len - tag_size + */ + if (ptext_size != ctext_size) { + TALLOC_FREE(ptext); + TALLOC_FREE(ctext); + rc = GNUTLS_E_SHORT_MEMORY_BUFFER; + status = gnutls_error_to_ntstatus(rc, NT_STATUS_INTERNAL_ERROR); + goto out; + } + ptext_size -= tag_size; +#endif /* HAVE_GNUTLS_AEAD_CIPHER_DECRYPT_PTEXT_LEN_BUG */ if (ptext_size != m_total) { TALLOC_FREE(ptext); TALLOC_FREE(ctext); diff --git a/wscript_configure_system_gnutls b/wscript_configure_system_gnutls index 2ec217fb9dc5..64f39b6a1def 100644 --- a/wscript_configure_system_gnutls +++ b/wscript_configure_system_gnutls @@ -35,6 +35,9 @@ conf.CHECK_FUNCS_IN('gnutls_set_default_priority_append', 'gnutls') if (parse_version(gnutls_version) > parse_version('3.6.14')): conf.CHECK_FUNCS_IN('gnutls_aead_cipher_encryptv2', 'gnutls') +if (parse_version(gnutls_version) < parse_version('3.5.2')): + conf.DEFINE('HAVE_GNUTLS_AEAD_CIPHER_DECRYPT_PTEXT_LEN_BUG', 1) + # Check if we have support for crypto policies if conf.CHECK_FUNCS_IN('gnutls_get_system_config_file', 'gnutls'): conf.DEFINE('HAVE_GNUTLS_CRYPTO_POLICIES', 1) -- 2.25.1