The branch, master has been updated via e67845a73c9 ci-images: install diffutils prior to building images via aec2076fa79 lib/util: Delegate constant time memcmp to gnutls_memcmp() via 222e1afc6f9 lib/util: Add test of mem_equal_const_time() via a80d783a341 lib/util: Add test of data_blob_equal_const_time() via 8d7a091adcb lib/util: Reduce sum variable to uint8_t via feb36dbebf1 lib/util: Change function to mem_equal_const_time() via a554e2ce53c lib/util: Change function to data_blob_equal_const_time() via ae6634c7877 auth: Use constant-time memcmp when comparing sensitive buffers via 87f68500ed6 lib/util: Move memcmp_const_time() to util.c via ee29c601b25 tests/krb5/test_ldap.py: Increase maximum threshold for LDAP timeout via 14feb93d481 lib/util: Prefer backtrace_symbols() for internal backtraces via bd09537e219 build: Possibly link against libexecinfo for backtrace_symbols() via df11826a3b3 build: Make build with --disable-fault-hanlding work under --enable-developer from ef1d04762af s3:smbd: Free allocated strings before leaving user_in_netgroup() function
https://git.samba.org/?p=samba.git;a=shortlog;h=master - Log ----------------------------------------------------------------- commit e67845a73c96db885b9724d52857955b51b74632 Author: Uri Simchoni <u...@samba.org> Date: Wed Jun 8 22:20:03 2022 +0300 ci-images: install diffutils prior to building images Ensure the podman image used for generating Samba CI images includes 'diff' utility Signed-off-by: Uri Simchoni <u...@samba.org> Reviewed-by: Andrew Bartlett <abart...@samba.org> Autobuild-User(master): Andrew Bartlett <abart...@samba.org> Autobuild-Date(master): Thu Jun 9 23:48:42 UTC 2022 on sn-devel-184 commit aec2076fa79b853e26b1fe606570f1c4ae94c79b Author: Joseph Sutton <josephsut...@catalyst.net.nz> Date: Wed Jun 8 15:19:58 2022 +1200 lib/util: Delegate constant time memcmp to gnutls_memcmp() gnutls_memcmp() is mostly identical to our own implementation, except that ours will not break if supplied with 4 GiB or more of data. However, using an external function permits us to disclaim responsibility if some CPU/compiler combination happens to invalidate our constant-time guarantee. For reference, gnutls_memcmp() implementation: https://gitlab.com/gnutls/gnutls/-/blob/78d9820de0d2eb2f8088e359779ee7342f5f089e/lib/safe-memfuncs.c#L41-67 Signed-off-by: Joseph Sutton <josephsut...@catalyst.net.nz> Reviewed-by: Andrew Bartlett <abart...@samba.org> commit 222e1afc6f9a49e99ae767d7572dfd16c236148d Author: Joseph Sutton <josephsut...@catalyst.net.nz> Date: Wed May 11 14:06:22 2022 +1200 lib/util: Add test of mem_equal_const_time() Ensure that it gives the correct results for comparing two memory regions. Signed-off-by: Joseph Sutton <josephsut...@catalyst.net.nz> Reviewed-by: Andrew Bartlett <abart...@samba.org> commit a80d783a341fd8b88d73e04bf831b91984f87b73 Author: Joseph Sutton <josephsut...@catalyst.net.nz> Date: Wed May 11 14:05:34 2022 +1200 lib/util: Add test of data_blob_equal_const_time() Ensure that it gives the correct results for comparing two data blobs. Signed-off-by: Joseph Sutton <josephsut...@catalyst.net.nz> Reviewed-by: Andrew Bartlett <abart...@samba.org> commit 8d7a091adcbd4eaa9e5e736413a179c322f6869d Author: Joseph Sutton <josephsut...@catalyst.net.nz> Date: Wed May 11 14:04:25 2022 +1200 lib/util: Reduce sum variable to uint8_t Signed-off-by: Joseph Sutton <josephsut...@catalyst.net.nz> Reviewed-by: Andrew Bartlett <abart...@samba.org> commit feb36dbebf1f0f48f4d9f2549471d355b4ead788 Author: Joseph Sutton <josephsut...@catalyst.net.nz> Date: Wed May 11 12:07:43 2022 +1200 lib/util: Change function to mem_equal_const_time() Since memcmp_const_time() doesn't act as an exact replacement for memcmp(), and its return value is only ever compared with zero, simplify it and emphasize the intention of checking equality by returning a bool instead. Signed-off-by: Joseph Sutton <josephsut...@catalyst.net.nz> Reviewed-by: Andrew Bartlett <abart...@samba.org> commit a554e2ce53cbee584bf3c0944d466cbdf73dd3b2 Author: Joseph Sutton <josephsut...@catalyst.net.nz> Date: Wed May 11 11:39:14 2022 +1200 lib/util: Change function to data_blob_equal_const_time() Since data_blob_cmp_const_time() doesn't act as an exact replacement for data_blob_cmp(), and its return value is only ever compared with zero, simplify it and emphasize the intention of checking equality by returning a bool instead. Signed-off-by: Joseph Sutton <josephsut...@catalyst.net.nz> Reviewed-by: Andrew Bartlett <abart...@samba.org> commit ae6634c78774d2368e815dea650ba71650dd1861 Author: Joseph Sutton <josephsut...@catalyst.net.nz> Date: Thu Feb 17 15:35:42 2022 +1300 auth: Use constant-time memcmp when comparing sensitive buffers This helps to avoid timing attacks. BUG: https://bugzilla.samba.org/show_bug.cgi?id=15010 Signed-off-by: Joseph Sutton <josephsut...@catalyst.net.nz> Reviewed-by: Andrew Bartlett <abart...@samba.org> commit 87f68500ed651f393e2fc6c514ab08b561a60a9b Author: Joseph Sutton <josephsut...@catalyst.net.nz> Date: Tue May 10 15:57:40 2022 +1200 lib/util: Move memcmp_const_time() to util.c This allows it to be used in more places without needing to introduce more dependencies. Signed-off-by: Joseph Sutton <josephsut...@catalyst.net.nz> Reviewed-by: Andrew Bartlett <abart...@samba.org> commit ee29c601b2566831d201735c73e4fbf40319437f Author: Joseph Sutton <josephsut...@catalyst.net.nz> Date: Sat Apr 30 18:03:14 2022 +1200 tests/krb5/test_ldap.py: Increase maximum threshold for LDAP timeout This test often fails because the server takes too long to time out. Signed-off-by: Joseph Sutton <josephsut...@catalyst.net.nz> Reviewed-by: Andrew Bartlett <abart...@samba.org> commit 14feb93d481011765f62614ab49b304e17e4f6fd Author: Andrew Bartlett <abart...@samba.org> Date: Tue Jun 7 15:07:59 2022 +1200 lib/util: Prefer backtrace_symbols() for internal backtraces Backtraces when Samba is in PANIC state are better with backtrace_symbols() than with libunwind on Ubuntu 20.04 x86_64 so move libunwind to a off-by-default option, prompted for if backtrace_symbols() is not available. Based on a request by Fco Javier Felix <ffe...@inode64.com> Signed-off-by: Andrew Bartlett <abart...@samba.org> Reviewed-by: Joseph Sutton <josephsut...@catalyst.net.nz> commit bd09537e21927be82d62984970ac6569f509adcd Author: Andrew Bartlett <abart...@samba.org> Date: Tue Jun 7 15:05:26 2022 +1200 build: Possibly link against libexecinfo for backtrace_symbols() We look for backtrace_symbols() in this library, so we should link against it if we find it. Signed-off-by: Andrew Bartlett <abart...@samba.org> Reviewed-by: Joseph Sutton <josephsut...@catalyst.net.nz> commit df11826a3b35bf7237fdb12ef7eb1ad84f4569f2 Author: Andrew Bartlett <abart...@samba.org> Date: Tue Jun 7 15:03:54 2022 +1200 build: Make build with --disable-fault-hanlding work under --enable-developer Previously this would leave static functions unused, which the compiler will not allow for a developer build. Signed-off-by: Andrew Bartlett <abart...@samba.org> Reviewed-by: Joseph Sutton <josephsut...@catalyst.net.nz> ----------------------------------------------------------------------- Summary of changes: .gitlab-ci-main.yml | 2 +- auth/gensec/schannel.c | 10 +++--- auth/ntlmssp/ntlmssp_ndr.c | 2 +- auth/ntlmssp/ntlmssp_server.c | 10 +++--- auth/ntlmssp/ntlmssp_sign.c | 4 +-- bootstrap/.gitlab-ci.yml | 2 ++ bootstrap/sha1sum.txt | 2 +- lib/util/data_blob.c | 24 +++++++++++++ lib/util/data_blob.h | 6 ++++ lib/util/fault.c | 13 ++++--- lib/util/samba_util.h | 4 +-- lib/util/tests/data_blob.c | 47 +++++++++++++++++++++++++ lib/util/tests/util.c | 22 ++++++++++++ lib/util/util.c | 9 +++++ lib/util/util_str.c | 12 ------- lib/util/wscript | 9 +++++ lib/util/wscript_build | 2 ++ lib/util/wscript_configure | 30 +++++++++++----- libcli/auth/credentials.c | 4 +-- libcli/auth/netlogon_creds_cli.c | 20 +++++------ libcli/auth/ntlm_check.c | 8 ++--- libcli/smb/smb2_signing.c | 2 +- libcli/smb/smbXcli_base.c | 10 +++--- libcli/smb/smb_signing.c | 4 +-- script/autobuild.py | 3 +- source3/librpc/crypto/gse_krb5.c | 4 +-- source3/passdb/machine_account_secrets.c | 18 +++++----- source3/rpc_client/cli_netlogon.c | 16 ++++----- source3/rpc_server/netlogon/srv_netlog_nt.c | 4 +-- source3/rpc_server/samr/srv_samr_chgpasswd.c | 18 +++++----- source3/winbindd/winbindd_dual_srv.c | 20 +++++------ source3/winbindd/winbindd_pam.c | 8 ++--- source4/auth/ntlm/auth_sam.c | 2 +- source4/dsdb/samdb/ldb_modules/password_hash.c | 8 ++--- source4/dsdb/tests/python/large_ldap.py | 4 +-- source4/libcli/raw/smb_signing.c | 2 +- source4/libcli/smb2/signing.c | 2 +- source4/rpc_server/backupkey/dcesrv_backupkey.c | 6 ++-- source4/rpc_server/netlogon/dcerpc_netlogon.c | 4 +-- source4/rpc_server/samr/samr_password.c | 2 +- 40 files changed, 253 insertions(+), 126 deletions(-) Changeset truncated at 500 lines: diff --git a/.gitlab-ci-main.yml b/.gitlab-ci-main.yml index 4bfa2f2ea04..ff8780128bd 100644 --- a/.gitlab-ci-main.yml +++ b/.gitlab-ci-main.yml @@ -42,7 +42,7 @@ variables: # Set this to the contents of bootstrap/sha1sum.txt # which is generated by bootstrap/template.py --render # - SAMBA_CI_CONTAINER_TAG: a9f12c7712cfccf1bf957d6976dc87917524b55e + SAMBA_CI_CONTAINER_TAG: 34eff4df0b3dbbfabcd74d5c50c357a6faa280d5 # # We use the ubuntu1804 image as default as # it matches what we have on sn-devel-184. diff --git a/auth/gensec/schannel.c b/auth/gensec/schannel.c index 6ebbe8f3179..9860559668f 100644 --- a/auth/gensec/schannel.c +++ b/auth/gensec/schannel.c @@ -592,7 +592,7 @@ static NTSTATUS netsec_incoming_packet(struct schannel_state *state, uint8_t *confounder = NULL; uint32_t confounder_ofs = 0; uint8_t seq_num[8]; - int ret; + bool ret; const uint8_t *sign_data = NULL; size_t sign_length = 0; NTSTATUS status; @@ -649,8 +649,8 @@ static NTSTATUS netsec_incoming_packet(struct schannel_state *state, return NT_STATUS_ACCESS_DENIED; } - ret = memcmp(checksum, sig->data+16, checksum_length); - if (ret != 0) { + ret = mem_equal_const_time(checksum, sig->data+16, checksum_length); + if (!ret) { dump_data_pw("calc digest:", checksum, checksum_length); dump_data_pw("wire digest:", sig->data+16, checksum_length); return NT_STATUS_ACCESS_DENIED; @@ -665,8 +665,8 @@ static NTSTATUS netsec_incoming_packet(struct schannel_state *state, ZERO_ARRAY(checksum); - ret = memcmp(seq_num, sig->data+8, 8); - if (ret != 0) { + ret = mem_equal_const_time(seq_num, sig->data+8, 8); + if (!ret) { dump_data_pw("calc seq num:", seq_num, 8); dump_data_pw("wire seq num:", sig->data+8, 8); return NT_STATUS_ACCESS_DENIED; diff --git a/auth/ntlmssp/ntlmssp_ndr.c b/auth/ntlmssp/ntlmssp_ndr.c index c8b16ccd413..ea5d6f0f5a0 100644 --- a/auth/ntlmssp/ntlmssp_ndr.c +++ b/auth/ntlmssp/ntlmssp_ndr.c @@ -31,7 +31,7 @@ do { \ if (!NDR_ERR_CODE_IS_SUCCESS(__ndr_err)) { \ return ndr_map_error2ntstatus(__ndr_err); \ } \ - if (memcmp(r->Signature, "NTLMSSP\0", 8)) {\ + if (!mem_equal_const_time(r->Signature, "NTLMSSP\0", 8)) { \ return NT_STATUS_INVALID_PARAMETER; \ } \ return NT_STATUS_OK; \ diff --git a/auth/ntlmssp/ntlmssp_server.c b/auth/ntlmssp/ntlmssp_server.c index e077c2f7379..6a27db1b7d4 100644 --- a/auth/ntlmssp/ntlmssp_server.c +++ b/auth/ntlmssp/ntlmssp_server.c @@ -1047,7 +1047,7 @@ static NTSTATUS ntlmssp_server_postauth(struct gensec_security *gensec_security, if (ntlmssp_state->new_spnego) { gnutls_hmac_hd_t hmac_hnd = NULL; uint8_t mic_buffer[NTLMSSP_MIC_SIZE] = { 0, }; - int cmp; + bool cmp; int rc; rc = gnutls_hmac_init(&hmac_hnd, @@ -1095,9 +1095,9 @@ static NTSTATUS ntlmssp_server_postauth(struct gensec_security *gensec_security, } gnutls_hmac_deinit(hmac_hnd, mic_buffer); - cmp = memcmp(request.data + NTLMSSP_MIC_OFFSET, - mic_buffer, NTLMSSP_MIC_SIZE); - if (cmp != 0) { + cmp = mem_equal_const_time(request.data + NTLMSSP_MIC_OFFSET, + mic_buffer, NTLMSSP_MIC_SIZE); + if (!cmp) { DEBUG(1,("%s: invalid NTLMSSP_MIC for " "user=[%s] domain=[%s] workstation=[%s]\n", __func__, @@ -1112,7 +1112,7 @@ static NTSTATUS ntlmssp_server_postauth(struct gensec_security *gensec_security, ZERO_ARRAY(mic_buffer); - if (cmp != 0) { + if (!cmp) { return NT_STATUS_INVALID_PARAMETER; } } diff --git a/auth/ntlmssp/ntlmssp_sign.c b/auth/ntlmssp/ntlmssp_sign.c index 89f1aa04f7a..11e5930a8de 100644 --- a/auth/ntlmssp/ntlmssp_sign.c +++ b/auth/ntlmssp/ntlmssp_sign.c @@ -291,7 +291,7 @@ NTSTATUS ntlmssp_check_packet(struct ntlmssp_state *ntlmssp_state, if (ntlmssp_state->neg_flags & NTLMSSP_NEGOTIATE_NTLM2) { if (local_sig.length != sig->length || - memcmp(local_sig.data, sig->data, sig->length) != 0) { + !mem_equal_const_time(local_sig.data, sig->data, sig->length)) { DEBUG(5, ("BAD SIG NTLM2: wanted signature of\n")); dump_data(5, local_sig.data, local_sig.length); @@ -304,7 +304,7 @@ NTSTATUS ntlmssp_check_packet(struct ntlmssp_state *ntlmssp_state, } } else { if (local_sig.length != sig->length || - memcmp(local_sig.data + 8, sig->data + 8, sig->length - 8) != 0) { + !mem_equal_const_time(local_sig.data + 8, sig->data + 8, sig->length - 8)) { DEBUG(5, ("BAD SIG NTLM1: wanted signature of\n")); dump_data(5, local_sig.data, local_sig.length); diff --git a/bootstrap/.gitlab-ci.yml b/bootstrap/.gitlab-ci.yml index 053dd17f9e2..626e0103410 100644 --- a/bootstrap/.gitlab-ci.yml +++ b/bootstrap/.gitlab-ci.yml @@ -13,6 +13,8 @@ SAMBA_CI_IS_BROKEN_IMAGE: "no" SAMBA_CI_TEST_JOB: "samba-o3" before_script: + # install prerequisites + - dnf install -qy diffutils # Ensure we are generating correct the container - uname -a - cat /etc/os-release diff --git a/bootstrap/sha1sum.txt b/bootstrap/sha1sum.txt index e862191aa72..0a3797326be 100644 --- a/bootstrap/sha1sum.txt +++ b/bootstrap/sha1sum.txt @@ -1 +1 @@ -a9f12c7712cfccf1bf957d6976dc87917524b55e +34eff4df0b3dbbfabcd74d5c50c357a6faa280d5 diff --git a/lib/util/data_blob.c b/lib/util/data_blob.c index da1730dccf5..677f7c19211 100644 --- a/lib/util/data_blob.c +++ b/lib/util/data_blob.c @@ -21,6 +21,7 @@ #include "replace.h" #include "attr.h" #include "data_blob.h" +#include "lib/util/samba_util.h" const DATA_BLOB data_blob_null = { NULL, 0 }; @@ -129,6 +130,29 @@ _PUBLIC_ int data_blob_cmp(const DATA_BLOB *d1, const DATA_BLOB *d2) return ret; } +/** +check if two data blobs are equal, where the time taken should not depend on the +contents of either blob. +**/ +_PUBLIC_ bool data_blob_equal_const_time(const DATA_BLOB *d1, const DATA_BLOB *d2) +{ + bool ret; + if (d1->data == NULL && d2->data != NULL) { + return false; + } + if (d1->data != NULL && d2->data == NULL) { + return false; + } + if (d1->length != d2->length) { + return false; + } + if (d1->data == d2->data) { + return true; + } + ret = mem_equal_const_time(d1->data, d2->data, d1->length); + return ret; +} + /** print the data_blob as hex string **/ diff --git a/lib/util/data_blob.h b/lib/util/data_blob.h index 7a0dc3b0014..44376b70776 100644 --- a/lib/util/data_blob.h +++ b/lib/util/data_blob.h @@ -86,6 +86,12 @@ check if two data blobs are equal **/ _PUBLIC_ int data_blob_cmp(const DATA_BLOB *d1, const DATA_BLOB *d2); +/** +check if two data blobs are equal, where the time taken should not depend on the +contents of either blob. +**/ +_PUBLIC_ bool data_blob_equal_const_time(const DATA_BLOB *d1, const DATA_BLOB *d2); + /** print the data_blob as hex string **/ diff --git a/lib/util/fault.c b/lib/util/fault.c index 90552a391b6..3b1d10dce89 100644 --- a/lib/util/fault.c +++ b/lib/util/fault.c @@ -64,6 +64,7 @@ _PUBLIC_ void fault_setup_disable(void) } +#if !defined(HAVE_DISABLE_FAULT_HANDLING) /******************************************************************* report a fault ********************************************************************/ @@ -91,7 +92,7 @@ static void sig_fault(int sig) { fault_report(sig); } - +#endif /******************************************************************* setup our fault handlers ********************************************************************/ @@ -221,9 +222,13 @@ _PUBLIC_ void smb_panic(const char *why) void log_stack_trace(void) { #ifdef HAVE_LIBUNWIND - /* Try to use libunwind before any other technique since on ia64 - * libunwind correctly walks the stack in more circumstances than - * backtrace. + /* + * --with-libunwind is required to use libunwind, the + * backtrace_symbols() code below is the default. + * + * This code is available because a previous version of this + * comment asserted that on ia64 libunwind correctly walks the + * stack in more circumstances than backtrace. */ unw_cursor_t cursor; unw_context_t uc; diff --git a/lib/util/samba_util.h b/lib/util/samba_util.h index ca185909997..ac185cc06c5 100644 --- a/lib/util/samba_util.h +++ b/lib/util/samba_util.h @@ -321,9 +321,9 @@ _PUBLIC_ bool conv_str_u64(const char * str, uint64_t * val); * * @param[in] n The length of the memory to comapre. * - * @return 0 when the memory regions are equal, 0 if not. + * @return true when the memory regions are equal, false if not. */ -_PUBLIC_ int memcmp_const_time(const void *s1, const void *s2, size_t n); +_PUBLIC_ bool mem_equal_const_time(const void *s1, const void *s2, size_t n); /** * @brief Build up a string buffer, handle allocation failure diff --git a/lib/util/tests/data_blob.c b/lib/util/tests/data_blob.c index a3b2db6e604..e1e8129a259 100644 --- a/lib/util/tests/data_blob.c +++ b/lib/util/tests/data_blob.c @@ -76,6 +76,52 @@ static bool test_cmp(struct torture_context *tctx) return true; } +static bool test_equal_const_time(struct torture_context *tctx) +{ + const char *test_string = "foobarfoo"; + + DATA_BLOB null = data_blob_const(NULL, 0); + DATA_BLOB foobar = data_blob_const(test_string, 6); + DATA_BLOB bar = data_blob_const(test_string + 3, 3); + + /* These data blobs both contain 'foo', but at different addresses. */ + DATA_BLOB foo_same = data_blob_const(test_string, 3); + DATA_BLOB foo_other = data_blob_const(test_string + 6, 3); + + /* Test all equality combinations behave as expected. */ + torture_assert(tctx, data_blob_equal_const_time(&null, &null), "null == null"); + torture_assert(tctx, !data_blob_equal_const_time(&null, &foobar), "null != 'foobar'"); + torture_assert(tctx, !data_blob_equal_const_time(&null, &bar), "null != 'bar'"); + torture_assert(tctx, !data_blob_equal_const_time(&null, &foo_same), "null != 'foo'"); + torture_assert(tctx, !data_blob_equal_const_time(&null, &foo_other), "null != 'foo'"); + + torture_assert(tctx, !data_blob_equal_const_time(&foobar, &null), "'foobar' != null"); + torture_assert(tctx, data_blob_equal_const_time(&foobar, &foobar), "'foobar' == 'foobar'"); + torture_assert(tctx, !data_blob_equal_const_time(&foobar, &bar), "'foobar' != 'bar'"); + torture_assert(tctx, !data_blob_equal_const_time(&foobar, &foo_same), "'foobar' != 'foo'"); + torture_assert(tctx, !data_blob_equal_const_time(&foobar, &foo_other), "'foobar' != 'foo'"); + + torture_assert(tctx, !data_blob_equal_const_time(&foo_same, &null), "'foo' != null"); + torture_assert(tctx, !data_blob_equal_const_time(&foo_same, &foobar), "'foo' != 'foobar'"); + torture_assert(tctx, !data_blob_equal_const_time(&foo_same, &bar), "'foo' != 'bar'"); + torture_assert(tctx, data_blob_equal_const_time(&foo_same, &foo_same), "'foo' == 'foo'"); + torture_assert(tctx, data_blob_equal_const_time(&foo_same, &foo_other), "'foo' == 'foo'"); + + torture_assert(tctx, !data_blob_equal_const_time(&foo_other, &null), "'foo' != null"); + torture_assert(tctx, !data_blob_equal_const_time(&foo_other, &foobar), "'foo' != 'foobar'"); + torture_assert(tctx, !data_blob_equal_const_time(&foo_other, &bar), "'foo' != 'bar'"); + torture_assert(tctx, data_blob_equal_const_time(&foo_other, &foo_same), "'foo' == 'foo'"); + torture_assert(tctx, data_blob_equal_const_time(&foo_other, &foo_other), "'foo' == 'foo'"); + + torture_assert(tctx, !data_blob_equal_const_time(&bar, &null), "'bar' != null"); + torture_assert(tctx, !data_blob_equal_const_time(&bar, &foobar), "'bar' != 'foobar'"); + torture_assert(tctx, data_blob_equal_const_time(&bar, &bar), "'bar' == 'bar'"); + torture_assert(tctx, !data_blob_equal_const_time(&bar, &foo_same), "'bar' != 'foo'"); + torture_assert(tctx, !data_blob_equal_const_time(&bar, &foo_other), "'bar' != 'foo'"); + + return true; +} + static bool test_hex_string(struct torture_context *tctx) { DATA_BLOB a = data_blob_string_const("\xC\xA\xF\xE"); @@ -117,6 +163,7 @@ struct torture_suite *torture_local_util_data_blob(TALLOC_CTX *mem_ctx) torture_suite_add_simple_test(suite, "zero", test_zero);; torture_suite_add_simple_test(suite, "clear", test_clear); torture_suite_add_simple_test(suite, "cmp", test_cmp); + torture_suite_add_simple_test(suite, "equal_const_time", test_equal_const_time); torture_suite_add_simple_test(suite, "hex string", test_hex_string); torture_suite_add_simple_test(suite, "append_NULL_0", test_append_NULL_0); torture_suite_add_simple_test(suite, "append_empty_0", test_append_empty_0); diff --git a/lib/util/tests/util.c b/lib/util/tests/util.c index 03f62974c3f..0b69e215f82 100644 --- a/lib/util/tests/util.c +++ b/lib/util/tests/util.c @@ -426,6 +426,25 @@ done: return ret; } +static bool test_mem_equal_const_time(struct torture_context *tctx) +{ + const char *test_string = "abcdabcd"; + + torture_assert(tctx, mem_equal_const_time("", "", 0), + "zero-length comparison failed"); + + torture_assert(tctx, mem_equal_const_time(test_string, test_string, 8), + "comparison with equal pointers failed"); + + torture_assert(tctx, mem_equal_const_time(test_string, test_string + 4, 4), + "comparison with non-equal pointers failed"); + + torture_assert(tctx, !mem_equal_const_time("abcd", "efgh", 4), + "comparison with different values failed"); + + return true; +} + static bool test_smb_strtoul_errno_check(struct torture_context *tctx) { const char *number = "123"; @@ -608,6 +627,9 @@ struct torture_suite *torture_local_util(TALLOC_CTX *mem_ctx) torture_suite_add_simple_test(suite, "directory_create_or_exist", test_directory_create_or_exist); + torture_suite_add_simple_test(suite, + "mem_equal_const_time", + test_mem_equal_const_time); torture_suite_add_simple_test(suite, "smb_strtoul(l) errno", test_smb_strtoul_errno_check); diff --git a/lib/util/util.c b/lib/util/util.c index c066406d320..8c2a74fe5f3 100644 --- a/lib/util/util.c +++ b/lib/util/util.c @@ -36,6 +36,7 @@ #include "samba_util.h" #include "lib/util/select.h" #include <libgen.h> +#include <gnutls/gnutls.h> #ifdef HAVE_SYS_PRCTL_H #include <sys/prctl.h> @@ -1097,6 +1098,14 @@ _PUBLIC_ size_t ascii_len_n(const char *src, size_t n) return len; } +_PUBLIC_ bool mem_equal_const_time(const void *s1, const void *s2, size_t n) +{ + /* Ensure we won't overflow the unsigned index used by gnutls. */ + SMB_ASSERT(n <= UINT_MAX); + + return gnutls_memcmp(s1, s2, n) == 0; +} + struct anonymous_shared_header { union { size_t length; diff --git a/lib/util/util_str.c b/lib/util/util_str.c index b5ba3fb716b..721e7cc8644 100644 --- a/lib/util/util_str.c +++ b/lib/util/util_str.c @@ -305,18 +305,6 @@ _PUBLIC_ bool set_boolean(const char *boolean_string, bool *boolean) return false; } -_PUBLIC_ int memcmp_const_time(const void *s1, const void *s2, size_t n) -{ - const uint8_t *p1 = s1, *p2 = s2; - size_t i, sum = 0; - - for (i = 0; i < n; i++) { - sum |= (p1[i] ^ p2[i]); - } - - return sum != 0; -} - _PUBLIC_ void talloc_asprintf_addbuf(char **ps, const char *fmt, ...) { va_list ap; diff --git a/lib/util/wscript b/lib/util/wscript index 6f711adcd0c..425f659a2ed 100644 --- a/lib/util/wscript +++ b/lib/util/wscript @@ -2,6 +2,15 @@ def options(opt): ''' This is a bit strange, but disable is the flag, not enable. ''' opt.add_option('--disable-fault-handling', action='store_true', dest='disable_fault_handling', help=('disable the fault handlers'), default=False) + # We do not want libunwind by default (backtrace_symbols() in + # glibc is better) but allow (eg) IA-64 to build with it where it + # might be better (per old comment in fault.c) + opt.samba_add_onoff_option('libunwind', + default=None, + help='''Use libunwind instead of the default backtrace_symbols() + from libc, for example on IA-64 where it might give a better + backtrace.''') + opt.add_option('--with-systemd', help=("Enable systemd integration"), action='store_true', dest='enable_systemd') diff --git a/lib/util/wscript_build b/lib/util/wscript_build index 08a551038c8..e3fa3295b46 100644 --- a/lib/util/wscript_build +++ b/lib/util/wscript_build @@ -84,6 +84,7 @@ bld.SAMBA_SUBSYSTEM('smb-panic', replace samba-debug LIBUNWIND + execinfo ''', local_include=False) @@ -117,6 +118,7 @@ bld.SAMBA_SUBSYSTEM('samba-util-core', tini smb_strtox smb-panic + gnutls ''', public_deps='systemd systemd-daemon sys_rw', local_include=False) diff --git a/lib/util/wscript_configure b/lib/util/wscript_configure index d4450d25b98..fbaeb095dd4 100644 --- a/lib/util/wscript_configure +++ b/lib/util/wscript_configure @@ -1,23 +1,35 @@ #!/usr/bin/env python -from waflib import Logs, Options +from waflib import Logs, Options, Errors import os, sys if Options.options.disable_fault_handling: conf.DEFINE('HAVE_DISABLE_FAULT_HANDLING',1) -# backtrace could be in libexecinfo or in libc +# backtrace could be in libexecinfo or in libc. +# This is our preferred backtrace handler (more useful output than libunwind as at Ubuntu 20.04 x86_64) conf.CHECK_FUNCS_IN('backtrace backtrace_symbols', 'execinfo', checklibc=True, headers='execinfo.h') conf.CHECK_HEADERS('execinfo.h') conf.SET_TARGET_TYPE('LIBUNWIND', 'EMPTY') -if conf.check_cfg(package='libunwind-generic', - args='--cflags --libs', - msg='Checking for libunwind', - uselib_store='LIBUNWIND', - mandatory=False): - if conf.CHECK_HEADERS('libunwind.h'): - conf.SET_TARGET_TYPE('LIBUNWIND', 'SYSLIB') +if Options.options.with_libunwind: + if conf.check_cfg(package='libunwind-generic', + args='--cflags --libs', + msg='Checking for libunwind', + uselib_store='LIBUNWIND', + mandatory=False): + if conf.CHECK_HEADERS('libunwind.h'): + conf.SET_TARGET_TYPE('LIBUNWIND', 'SYSLIB') + else: + raise Errors.WafError('--with-libunwind specified but libunwind not found') +elif Options.options.with_libunwind == None: + if not conf.CONFIG_SET('HAVE_BACKTRACE_SYMBOLS') \ + and not Options.options.disable_fault_handling: + raise Errors.WafError( +'''backtrace_symbols() not found but +--with-libunwind not specified. +Use --without-libunwind to build without internal backtrace support or +--disable-fault-handling to totally defer fault handling to the OS.''') conf.CHECK_STRUCTURE_MEMBER('struct statvfs', 'f_frsize', define='HAVE_FRSIZE', headers='sys/statvfs.h') diff --git a/libcli/auth/credentials.c b/libcli/auth/credentials.c index 23339d98bfa..a7f56e75e9e 100644 --- a/libcli/auth/credentials.c +++ b/libcli/auth/credentials.c @@ -659,7 +659,7 @@ bool netlogon_creds_client_check(struct netlogon_creds_CredentialState *creds, -- Samba Shared Repository