From 1e1c29dd98fe72f1d15340003294ac9db4ee3008 Mon Sep 17 00:00:00 2001 From: Joseph Sutton Date: Tue, 2 Aug 2022 14:01:59 +1200 Subject: [PATCH 01/41] s3:rpc_server: Fix typo in error message Signed-off-by: Joseph Sutton Reviewed-by: Andreas Schneider Reviewed-by: Andrew Bartlett (cherry picked from commit 6932ccf3ccffbd9ab1907c4fb39b46c971e88d49) --- source3/rpc_server/samr/srv_samr_nt.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/source3/rpc_server/samr/srv_samr_nt.c b/source3/rpc_server/samr/srv_samr_nt.c index 4f8605eb576..ac40cbc418b 100644 --- a/source3/rpc_server/samr/srv_samr_nt.c +++ b/source3/rpc_server/samr/srv_samr_nt.c @@ -5066,7 +5066,7 @@ set_user_info_pw_aes(DATA_BLOB *pw_data, const char *rhost, struct samu *pwd) username = pdb_get_username(pwd); if (username == NULL) { - DBG_WARNING("User unkown\n"); + DBG_WARNING("User unknown\n"); return false; } -- 2.35.0 From 5183d1bb481d697d446b535a6d629c6a78a4bb4c Mon Sep 17 00:00:00 2001 From: Joseph Sutton Date: Tue, 2 Aug 2022 15:19:02 +1200 Subject: [PATCH 02/41] lib:crypto: Zero auth_tag array in encryption test If samba_gnutls_aead_aes_256_cbc_hmac_sha512_encrypt() does not fill the array completely, we may be comparing uninitialised bytes. Signed-off-by: Joseph Sutton Reviewed-by: Andreas Schneider Reviewed-by: Andrew Bartlett (cherry picked from commit f9850c776f81d596ffbd2761c85fe7a72d369bae) --- lib/crypto/tests/test_gnutls_aead_aes_256_cbc_hmac_sha512.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/crypto/tests/test_gnutls_aead_aes_256_cbc_hmac_sha512.c b/lib/crypto/tests/test_gnutls_aead_aes_256_cbc_hmac_sha512.c index 51f125f42d6..bc6a191cd90 100644 --- a/lib/crypto/tests/test_gnutls_aead_aes_256_cbc_hmac_sha512.c +++ b/lib/crypto/tests/test_gnutls_aead_aes_256_cbc_hmac_sha512.c @@ -187,7 +187,7 @@ static void torture_encrypt(void **state) .length = sizeof(salt_data), }; DATA_BLOB ctext; - uint8_t auth_tag[64]; + uint8_t auth_tag[64] = {0}; assert_int_equal(iv.length, 16); -- 2.35.0 From b843b7f7717d70acc71c275f1f48060427455254 Mon Sep 17 00:00:00 2001 From: Joseph Sutton Date: Tue, 2 Aug 2022 15:21:43 +1200 Subject: [PATCH 03/41] s4:torture: Zero samr_UserInfo union in password set test If init_samr_CryptPasswordAES() does not fill the u.info31.password.auth_data array completely, we may be comparing uninitialised bytes. Signed-off-by: Joseph Sutton Reviewed-by: Andreas Schneider Reviewed-by: Andrew Bartlett (cherry picked from commit 03f0e4d55be80a1a6dcc0dba8e6ed74d9da63dc3) --- source4/torture/rpc/samr.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/source4/torture/rpc/samr.c b/source4/torture/rpc/samr.c index 5fe51e51022..de354659067 100644 --- a/source4/torture/rpc/samr.c +++ b/source4/torture/rpc/samr.c @@ -920,6 +920,8 @@ static bool test_SetUserPass_31(struct dcerpc_pipe *p, struct torture_context *t s.in.info = &u; s.in.level = 31; + ZERO_STRUCT(u); + u.info31.password_expired = 0; status = dcerpc_fetch_session_key(p, &session_key); -- 2.35.0 From d6f8217f44d665520e5ca634c71ca6d8cba1aa30 Mon Sep 17 00:00:00 2001 From: Joseph Sutton Date: Tue, 2 Aug 2022 14:34:26 +1200 Subject: [PATCH 04/41] lib:crypto: Check for overflow before filling pauth_tag array Signed-off-by: Joseph Sutton Reviewed-by: Andreas Schneider Reviewed-by: Andrew Bartlett (cherry picked from commit cec59b82f7041a305c228091a84257c28e0818d5) --- lib/crypto/gnutls_aead_aes_256_cbc_hmac_sha512.c | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/lib/crypto/gnutls_aead_aes_256_cbc_hmac_sha512.c b/lib/crypto/gnutls_aead_aes_256_cbc_hmac_sha512.c index a05aa8a323c..fc4d21f4ec5 100644 --- a/lib/crypto/gnutls_aead_aes_256_cbc_hmac_sha512.c +++ b/lib/crypto/gnutls_aead_aes_256_cbc_hmac_sha512.c @@ -124,6 +124,14 @@ samba_gnutls_aead_aes_256_cbc_hmac_sha512_encrypt(TALLOC_CTX *mem_ctx, * TODO: Use gnutls_cipher_encrypt3() */ + if (hmac_size > 64) { + /* + * We don't want to overflow 'pauth_tag', which is 64 bytes in + * size. + */ + return NT_STATUS_INVALID_BUFFER_SIZE; + } + if (plaintext->length + aes_block_size < plaintext->length) { return NT_STATUS_INVALID_BUFFER_SIZE; } -- 2.35.0 From 4e20c0eb91c388ad4a22d5e3450f4f29e2854c9c Mon Sep 17 00:00:00 2001 From: Joseph Sutton Date: Tue, 2 Aug 2022 14:34:55 +1200 Subject: [PATCH 05/41] lib:crypto: Use constant time memory comparison to check HMAC Signed-off-by: Joseph Sutton Reviewed-by: Andreas Schneider Reviewed-by: Andrew Bartlett (cherry picked from commit 121e439e24a9c03ae900ffca1ae1dda8e059008c) --- lib/crypto/gnutls_aead_aes_256_cbc_hmac_sha512.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/crypto/gnutls_aead_aes_256_cbc_hmac_sha512.c b/lib/crypto/gnutls_aead_aes_256_cbc_hmac_sha512.c index fc4d21f4ec5..e0877a03f52 100644 --- a/lib/crypto/gnutls_aead_aes_256_cbc_hmac_sha512.c +++ b/lib/crypto/gnutls_aead_aes_256_cbc_hmac_sha512.c @@ -282,7 +282,7 @@ samba_gnutls_aead_aes_256_cbc_hmac_sha512_decrypt(TALLOC_CTX *mem_ctx, uint8_t padding; size_t i; NTSTATUS status; - int cmp; + bool equal; int rc; if (cdk->length == 0 || ciphertext->length == 0 || @@ -333,8 +333,8 @@ samba_gnutls_aead_aes_256_cbc_hmac_sha512_decrypt(TALLOC_CTX *mem_ctx, } gnutls_hmac_deinit(hmac_hnd, auth_data); - cmp = memcmp(auth_data, auth_tag, sizeof(auth_data)); - if (cmp != 0) { + equal = mem_equal_const_time(auth_data, auth_tag, sizeof(auth_data)); + if (!equal) { return NT_STATUS_DECRYPTION_FAILED; } -- 2.35.0 From 0361db5c783abf5abf944d81b52c802705f36f86 Mon Sep 17 00:00:00 2001 From: Joseph Sutton Date: Wed, 6 Jul 2022 15:36:26 +1200 Subject: [PATCH 06/41] CVE-2021-20251 lib:crypto: Add des_crypt_blob_16() for encrypting data with DES This lets us access single-DES from Python. This function is used in a following commit for encrypting an NT hash to obtain the verifier for a SAMR password change. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14611 Signed-off-by: Joseph Sutton Reviewed-by: Andreas Schneider Reviewed-by: Andrew Bartlett (cherry picked from commit b27a67af0216811d330d8a4c52390cf4fc04b5fd) --- lib/crypto/py_crypto.c | 65 ++++++++++++++++++++++++++++++++++++++++++ lib/crypto/wscript | 2 +- 2 files changed, 66 insertions(+), 1 deletion(-) diff --git a/lib/crypto/py_crypto.c b/lib/crypto/py_crypto.c index ad18d3ada0f..6753d3d8e9c 100644 --- a/lib/crypto/py_crypto.c +++ b/lib/crypto/py_crypto.c @@ -25,6 +25,7 @@ #include #include #include "lib/crypto/gnutls_helpers.h" +#include "libcli/auth/libcli_auth.h" static PyObject *py_crypto_arcfour_crypt_blob(PyObject *module, PyObject *args) { @@ -100,13 +101,77 @@ static PyObject *py_crypto_set_strict_mode(PyObject *module) Py_RETURN_NONE; } +static PyObject *py_crypto_des_crypt_blob_16(PyObject *self, PyObject *args) +{ + PyObject *py_data = NULL; + uint8_t *data = NULL; + Py_ssize_t data_size; + + PyObject *py_key = NULL; + uint8_t *key = NULL; + Py_ssize_t key_size; + + uint8_t result[16]; + + bool ok; + int ret; + + ok = PyArg_ParseTuple(args, "SS", + &py_data, &py_key); + if (!ok) { + return NULL; + } + + ret = PyBytes_AsStringAndSize(py_data, + (char **)&data, + &data_size); + if (ret != 0) { + return NULL; + } + + ret = PyBytes_AsStringAndSize(py_key, + (char **)&key, + &key_size); + if (ret != 0) { + return NULL; + } + + if (data_size != 16) { + return PyErr_Format(PyExc_ValueError, + "Expected data size of 16 bytes; got %zd", + data_size); + } + + if (key_size != 14) { + return PyErr_Format(PyExc_ValueError, + "Expected key size of 14 bytes; got %zd", + key_size); + } + + ret = des_crypt112_16(result, data, key, + SAMBA_GNUTLS_ENCRYPT); + if (ret != 0) { + return PyErr_Format(PyExc_RuntimeError, + "des_crypt112_16() failed: %d", + ret); + } + + return PyBytes_FromStringAndSize((const char *)result, + sizeof(result)); +} + static const char py_crypto_arcfour_crypt_blob_doc[] = "arcfour_crypt_blob(data, key)\n" "Encrypt the data with RC4 algorithm using the key"; +static const char py_crypto_des_crypt_blob_16_doc[] = "des_crypt_blob_16(data, key) -> bytes\n" + "Encrypt the 16-byte data with DES using " + "the 14-byte key"; + static PyMethodDef py_crypto_methods[] = { { "arcfour_crypt_blob", (PyCFunction)py_crypto_arcfour_crypt_blob, METH_VARARGS, py_crypto_arcfour_crypt_blob_doc }, { "set_relax_mode", (PyCFunction)py_crypto_set_relax_mode, METH_NOARGS, "Set fips to relax mode" }, { "set_strict_mode", (PyCFunction)py_crypto_set_strict_mode, METH_NOARGS, "Set fips to strict mode" }, + { "des_crypt_blob_16", (PyCFunction)py_crypto_des_crypt_blob_16, METH_VARARGS, py_crypto_des_crypt_blob_16_doc }, {0}, }; diff --git a/lib/crypto/wscript b/lib/crypto/wscript index 78927437e37..acf5cb8e731 100644 --- a/lib/crypto/wscript +++ b/lib/crypto/wscript @@ -81,7 +81,7 @@ def build(bld): bld.SAMBA_PYTHON('python_crypto', source='py_crypto.c', - deps='gnutls talloc', + deps='gnutls talloc LIBCLI_AUTH', realname='samba/crypto.so') bld.SAMBA_BINARY('test_gnutls_aead_aes_256_cbc_hmac_sha512', -- 2.35.0 From 974d9530d69c1a265e4b836a353b7c21b16132dd Mon Sep 17 00:00:00 2001 From: Joseph Sutton Date: Wed, 13 Jul 2022 14:20:59 +1200 Subject: [PATCH 07/41] CVE-2021-20251 lib:crypto: Add md4_hash_blob() for hashing data with MD4 This lets us access MD4, which might not be available in hashlib, from Python. This function is used in a following commit for hashing a password to obtain the verifier for a SAMR password change. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14611 Signed-off-by: Joseph Sutton Reviewed-by: Andreas Schneider Reviewed-by: Andrew Bartlett (cherry picked from commit 17b8d164f69a5ed79d9b7b7fc2f3f84f8ea534c8) --- lib/crypto/py_crypto.c | 35 +++++++++++++++++++++++++++++++++++ 1 file changed, 35 insertions(+) diff --git a/lib/crypto/py_crypto.c b/lib/crypto/py_crypto.c index 6753d3d8e9c..40b0cb9e9c0 100644 --- a/lib/crypto/py_crypto.c +++ b/lib/crypto/py_crypto.c @@ -25,6 +25,7 @@ #include #include #include "lib/crypto/gnutls_helpers.h" +#include "lib/crypto/md4.h" #include "libcli/auth/libcli_auth.h" static PyObject *py_crypto_arcfour_crypt_blob(PyObject *module, PyObject *args) @@ -160,6 +161,36 @@ static PyObject *py_crypto_des_crypt_blob_16(PyObject *self, PyObject *args) sizeof(result)); } +static PyObject *py_crypto_md4_hash_blob(PyObject *self, PyObject *args) +{ + PyObject *py_data = NULL; + uint8_t *data = NULL; + Py_ssize_t data_size; + + uint8_t result[16]; + + bool ok; + int ret; + + ok = PyArg_ParseTuple(args, "S", + &py_data); + if (!ok) { + return NULL; + } + + ret = PyBytes_AsStringAndSize(py_data, + (char **)&data, + &data_size); + if (ret != 0) { + return NULL; + } + + mdfour(result, data, data_size); + + return PyBytes_FromStringAndSize((const char *)result, + sizeof(result)); +} + static const char py_crypto_arcfour_crypt_blob_doc[] = "arcfour_crypt_blob(data, key)\n" "Encrypt the data with RC4 algorithm using the key"; @@ -167,11 +198,15 @@ static const char py_crypto_des_crypt_blob_16_doc[] = "des_crypt_blob_16(data, k "Encrypt the 16-byte data with DES using " "the 14-byte key"; +static const char py_crypto_md4_hash_blob_doc[] = "md4_hash_blob(data) -> bytes\n" + "Hash the data with MD4 algorithm"; + static PyMethodDef py_crypto_methods[] = { { "arcfour_crypt_blob", (PyCFunction)py_crypto_arcfour_crypt_blob, METH_VARARGS, py_crypto_arcfour_crypt_blob_doc }, { "set_relax_mode", (PyCFunction)py_crypto_set_relax_mode, METH_NOARGS, "Set fips to relax mode" }, { "set_strict_mode", (PyCFunction)py_crypto_set_strict_mode, METH_NOARGS, "Set fips to strict mode" }, { "des_crypt_blob_16", (PyCFunction)py_crypto_des_crypt_blob_16, METH_VARARGS, py_crypto_des_crypt_blob_16_doc }, + { "md4_hash_blob", (PyCFunction)py_crypto_md4_hash_blob, METH_VARARGS, py_crypto_md4_hash_blob_doc }, {0}, }; -- 2.35.0 From 4b342e2ba90c5214128cad45cd0d343446d502c5 Mon Sep 17 00:00:00 2001 From: Joseph Sutton Date: Tue, 2 Aug 2022 14:35:19 +1200 Subject: [PATCH 08/41] CVE-2021-20251 lib:crypto: Add Python functions for AES SAMR password change These functions allow us to perform key derivation and AES256 encryption in Python. They will be used in a following commit. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14611 Signed-off-by: Joseph Sutton Reviewed-by: Andreas Schneider Reviewed-by: Andrew Bartlett (cherry picked from commit 4bb9d85fed8498566bdb87baa71a3147806baafc) --- lib/crypto/py_crypto.c | 221 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 221 insertions(+) diff --git a/lib/crypto/py_crypto.c b/lib/crypto/py_crypto.c index 40b0cb9e9c0..11659556884 100644 --- a/lib/crypto/py_crypto.c +++ b/lib/crypto/py_crypto.c @@ -27,6 +27,51 @@ #include "lib/crypto/gnutls_helpers.h" #include "lib/crypto/md4.h" #include "libcli/auth/libcli_auth.h" +#include "libcli/util/pyerrors.h" + +#ifdef HAVE_GNUTLS_PBKDF2 +static bool samba_gnutls_datum_from_PyObject(PyObject *py_obj, + gnutls_datum_t *datum) +{ + uint8_t *data = NULL; + Py_ssize_t size; + + int ret; + + ret = PyBytes_AsStringAndSize(py_obj, + (char **)&data, + &size); + if (ret != 0) { + return false; + } + + datum->data = data; + datum->size = size; + + return true; +} +#endif /* HAVE_GNUTLS_PBKDF2 */ + +static bool samba_DATA_BLOB_from_PyObject(PyObject *py_obj, + DATA_BLOB *blob) +{ + uint8_t *data = NULL; + Py_ssize_t size; + + int ret; + + ret = PyBytes_AsStringAndSize(py_obj, + (char **)&data, + &size); + if (ret != 0) { + return false; + } + + blob->data = data; + blob->length = size; + + return true; +} static PyObject *py_crypto_arcfour_crypt_blob(PyObject *module, PyObject *args) { @@ -191,6 +236,165 @@ static PyObject *py_crypto_md4_hash_blob(PyObject *self, PyObject *args) sizeof(result)); } +static PyObject *py_crypto_sha512_pbkdf2(PyObject *self, PyObject *args) +{ +#ifdef HAVE_GNUTLS_PBKDF2 + PyObject *py_key = NULL; + uint8_t *key = NULL; + gnutls_datum_t key_datum = {0}; + + PyObject *py_salt = NULL; + gnutls_datum_t salt_datum = {0}; + + uint8_t result[16]; + + unsigned iterations = 0; + + bool ok; + int ret; + NTSTATUS status; + + ok = PyArg_ParseTuple(args, "SSI", + &py_key, &py_salt, &iterations); + if (!ok) { + return NULL; + } + + ok = samba_gnutls_datum_from_PyObject(py_key, &key_datum); + if (!ok) { + return NULL; + } + + ok = samba_gnutls_datum_from_PyObject(py_salt, &salt_datum); + if (!ok) { + return NULL; + } + + ret = gnutls_pbkdf2(GNUTLS_MAC_SHA512, + &key_datum, + &salt_datum, + iterations, + result, + sizeof(result)); + BURN_DATA(key); + if (ret < 0) { + status = gnutls_error_to_ntstatus(ret, NT_STATUS_CRYPTO_SYSTEM_INVALID); + PyErr_SetNTSTATUS(status); + return NULL; + } + + return PyBytes_FromStringAndSize((const char *)result, + sizeof(result)); +#else /* HAVE_GNUTLS_PBKDF2 */ + PyErr_SetString(PyExc_NotImplementedError, "gnutls_pbkdf2() is not available"); + return NULL; +#endif /* HAVE_GNUTLS_PBKDF2 */ +} + +static PyObject *py_crypto_aead_aes_256_cbc_hmac_sha512_blob(PyObject *self, PyObject *args) +{ + TALLOC_CTX *ctx = NULL; + + PyObject *py_ciphertext = NULL; + DATA_BLOB ciphertext_blob = {0}; + + PyObject *py_auth_data = NULL; + PyObject *py_result = NULL; + + PyObject *py_plaintext = NULL; + DATA_BLOB plaintext_blob = {0}; + PyObject *py_cek = NULL; + DATA_BLOB cek_blob = {0}; + PyObject *py_key_salt = NULL; + DATA_BLOB key_salt_blob = {0}; + PyObject *py_mac_salt = NULL; + DATA_BLOB mac_salt_blob = {0}; + PyObject *py_iv = NULL; + DATA_BLOB iv_blob = {0}; + + uint8_t auth_data[64]; + + bool ok; + NTSTATUS status; + + ok = PyArg_ParseTuple(args, "SSSSS", + &py_plaintext, + &py_cek, + &py_key_salt, + &py_mac_salt, + &py_iv); + if (!ok) { + return NULL; + } + + /* Create data blobs from the contents of the function parameters. */ + + ok = samba_DATA_BLOB_from_PyObject(py_plaintext, &plaintext_blob); + if (!ok) { + return NULL; + } + + ok = samba_DATA_BLOB_from_PyObject(py_cek, &cek_blob); + if (!ok) { + return NULL; + } + + ok = samba_DATA_BLOB_from_PyObject(py_key_salt, &key_salt_blob); + if (!ok) { + return NULL; + } + + ok = samba_DATA_BLOB_from_PyObject(py_mac_salt, &mac_salt_blob); + if (!ok) { + return NULL; + } + + ok = samba_DATA_BLOB_from_PyObject(py_iv, &iv_blob); + if (!ok) { + return NULL; + } + + ctx = talloc_new(NULL); + if (ctx == NULL) { + return PyErr_NoMemory(); + } + + /* Encrypt the plaintext. */ + status = samba_gnutls_aead_aes_256_cbc_hmac_sha512_encrypt(ctx, + &plaintext_blob, + &cek_blob, + &key_salt_blob, + &mac_salt_blob, + &iv_blob, + &ciphertext_blob, + auth_data); + if (!NT_STATUS_IS_OK(status)) { + PyErr_SetNTSTATUS(status); + talloc_free(ctx); + return NULL; + } + + /* Convert the output into Python 'bytes' objects. */ + py_ciphertext = PyBytes_FromStringAndSize((const char *)ciphertext_blob.data, + ciphertext_blob.length); + talloc_free(ctx); + if (py_ciphertext == NULL) { + return NULL; + } + py_auth_data = PyBytes_FromStringAndSize((const char *)auth_data, + sizeof(auth_data)); + if (py_auth_data == NULL) { + return NULL; + } + + /* Steal ciphertext and auth_data into a new tuple. */ + py_result = Py_BuildValue("(NN)", py_ciphertext, py_auth_data); + + return py_result; +} + + + static const char py_crypto_arcfour_crypt_blob_doc[] = "arcfour_crypt_blob(data, key)\n" "Encrypt the data with RC4 algorithm using the key"; @@ -201,12 +405,29 @@ static const char py_crypto_des_crypt_blob_16_doc[] = "des_crypt_blob_16(data, k static const char py_crypto_md4_hash_blob_doc[] = "md4_hash_blob(data) -> bytes\n" "Hash the data with MD4 algorithm"; +static const char py_crypto_sha512_pbkdf2_doc[] = "sha512_pbkdf2(key, salt, iterations) -> bytes\n" + "Derive a key from an existing one with SHA512 " + "algorithm"; + +static const char py_crypto_aead_aes_256_cbc_hmac_sha512_blob_doc[] = + "aead_aes_256_cbc_hmac_sha512_blob(plaintext, cek, key_salt, " + "mac_salt, iv) -> ciphertext, auth_data\n" + "Encrypt the plaintext with AES256 as specified in " + "[MS-SAMR] 3.2.2.4 AES Cipher Usage"; + static PyMethodDef py_crypto_methods[] = { { "arcfour_crypt_blob", (PyCFunction)py_crypto_arcfour_crypt_blob, METH_VARARGS, py_crypto_arcfour_crypt_blob_doc }, { "set_relax_mode", (PyCFunction)py_crypto_set_relax_mode, METH_NOARGS, "Set fips to relax mode" }, { "set_strict_mode", (PyCFunction)py_crypto_set_strict_mode, METH_NOARGS, "Set fips to strict mode" }, { "des_crypt_blob_16", (PyCFunction)py_crypto_des_crypt_blob_16, METH_VARARGS, py_crypto_des_crypt_blob_16_doc }, { "md4_hash_blob", (PyCFunction)py_crypto_md4_hash_blob, METH_VARARGS, py_crypto_md4_hash_blob_doc }, + { "sha512_pbkdf2", (PyCFunction)py_crypto_sha512_pbkdf2, METH_VARARGS, py_crypto_sha512_pbkdf2_doc }, + { + "aead_aes_256_cbc_hmac_sha512_blob", + (PyCFunction)py_crypto_aead_aes_256_cbc_hmac_sha512_blob, + METH_VARARGS, + py_crypto_aead_aes_256_cbc_hmac_sha512_blob_doc + }, {0}, }; -- 2.35.0 From 7405b08f46595a6e541d0974821517aa2d29cf3f Mon Sep 17 00:00:00 2001 From: Joseph Sutton Date: Mon, 4 Jul 2022 20:48:48 +1200 Subject: [PATCH 09/41] CVE-2021-20251 tests/krb5: Add tests for password lockout race BUG: https://bugzilla.samba.org/show_bug.cgi?id=14611 Signed-off-by: Joseph Sutton Reviewed-by: Andreas Schneider Reviewed-by: Andrew Bartlett (cherry picked from commit 91e2e5616ccd507fcaf097533c5fc25974119c1e) [jsutton@samba.org Fixed conflicts in usage.py, knownfails, and tests.py due to not having claims tests] --- python/samba/tests/krb5/lockout_tests.py | 1088 ++++++++++++++++++ python/samba/tests/krb5/raw_testcase.py | 10 +- python/samba/tests/krb5/rfc4120_constants.py | 1 + python/samba/tests/usage.py | 1 + selftest/flapping.d/ldap-pwd-change-race | 5 + selftest/knownfail_heimdal_kdc | 10 + selftest/knownfail_mit_kdc | 14 + source4/selftest/tests.py | 7 + 8 files changed, 1135 insertions(+), 1 deletion(-) create mode 100755 python/samba/tests/krb5/lockout_tests.py create mode 100644 selftest/flapping.d/ldap-pwd-change-race diff --git a/python/samba/tests/krb5/lockout_tests.py b/python/samba/tests/krb5/lockout_tests.py new file mode 100755 index 00000000000..e49e82a4bd5 --- /dev/null +++ b/python/samba/tests/krb5/lockout_tests.py @@ -0,0 +1,1088 @@ +#!/usr/bin/env python3 +# Unix SMB/CIFS implementation. +# Copyright (C) Stefan Metzmacher 2020 +# Copyright (C) Catalyst.Net Ltd +# +# This program is free software; you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation; either version 3 of the License, or +# (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program. If not, see . +# + +from concurrent import futures +from enum import Enum +from functools import partial +from multiprocessing import Pipe +import os +import sys +import time + +from cryptography.hazmat.backends import default_backend +from cryptography.hazmat.primitives.ciphers.base import Cipher +from cryptography.hazmat.primitives.ciphers import algorithms + +import ldb + +from samba import ( + NTSTATUSError, + dsdb, + generate_random_bytes, + generate_random_password, + ntstatus, + unix2nttime, + werror, +) +from samba.credentials import DONT_USE_KERBEROS, MUST_USE_KERBEROS +from samba.crypto import ( + aead_aes_256_cbc_hmac_sha512_blob, + des_crypt_blob_16, + md4_hash_blob, + sha512_pbkdf2, +) +from samba.dcerpc import lsa, samr +from samba.samdb import SamDB + +from samba.tests import connect_samdb, env_get_var_value, env_loadparm + +from samba.tests.krb5.as_req_tests import AsReqBaseTest +from samba.tests.krb5 import kcrypto +from samba.tests.krb5.kdc_base_test import KDCBaseTest +from samba.tests.krb5.raw_testcase import KerberosCredentials +import samba.tests.krb5.rfc4120_pyasn1 as krb5_asn1 +from samba.tests.krb5.rfc4120_constants import ( + KDC_ERR_CLIENT_REVOKED, + KDC_ERR_PREAUTH_FAILED, + KRB_AS_REP, + KRB_ERROR, + NT_PRINCIPAL, + NT_SRV_INST, +) + +sys.path.insert(0, 'bin/python') +os.environ['PYTHONUNBUFFERED'] = '1' + +global_asn1_print = False +global_hexdump = False + + +class ConnectionResult(Enum): + LOCKED_OUT = 1 + WRONG_PASSWORD = 2 + SUCCESS = 3 + + +def connect_kdc(pipe, + url, + hostname, + username, + password, + domain, + realm, + workstation, + dn, + expect_error=True): + AsReqBaseTest.setUpClass() + as_req_base = AsReqBaseTest() + as_req_base.setUp() + + user_creds = KerberosCredentials() + user_creds.set_username(username) + user_creds.set_password(password) + user_creds.set_domain(domain) + user_creds.set_realm(realm) + user_creds.set_workstation(workstation) + user_creds.set_kerberos_state(DONT_USE_KERBEROS) + + user_name = user_creds.get_username() + cname = as_req_base.PrincipalName_create(name_type=NT_PRINCIPAL, + names=user_name.split('/')) + + krbtgt_creds = as_req_base.get_krbtgt_creds() + krbtgt_supported_etypes = krbtgt_creds.tgs_supported_enctypes + realm = krbtgt_creds.get_realm() + + krbtgt_account = krbtgt_creds.get_username() + sname = as_req_base.PrincipalName_create(name_type=NT_SRV_INST, + names=[krbtgt_account, realm]) + + expected_salt = user_creds.get_salt() + + till = as_req_base.get_KerberosTime(offset=36000) + + kdc_options = krb5_asn1.KDCOptions('postdated') + + preauth_key = as_req_base.PasswordKey_from_creds(user_creds, + kcrypto.Enctype.AES256) + + ts_enc_padata = as_req_base.get_enc_timestamp_pa_data_from_key(preauth_key) + padata = [ts_enc_padata] + + krbtgt_decryption_key = ( + as_req_base.TicketDecryptionKey_from_creds(krbtgt_creds)) + + etypes = as_req_base.get_default_enctypes() + + if expect_error: + expected_error_modes = (KDC_ERR_CLIENT_REVOKED, + KDC_ERR_PREAUTH_FAILED) + else: + expected_error_modes = 0 + + # Remove the LDAP connection. + del type(as_req_base)._ldb + + # Indicate that we're ready. This ensures we hit the right transaction + # lock. + pipe.send_bytes(b'0') + + # Wait for the main process to take out a transaction lock. + if not pipe.poll(timeout=5): + raise AssertionError('main process failed to indicate readiness') + + # Try making a Kerberos AS-REQ to the KDC. This should fail, either due to + # the user's account being locked out or due to using the wrong password. + as_rep, kdc_exchange_dict = as_req_base._test_as_exchange( + cname=cname, + realm=realm, + sname=sname, + till=till, + client_as_etypes=etypes, + expected_error_mode=expected_error_modes, + expected_crealm=realm, + expected_cname=cname, + expected_srealm=realm, + expected_sname=sname, + expected_salt=expected_salt, + etypes=etypes, + padata=padata, + kdc_options=kdc_options, + expected_supported_etypes=krbtgt_supported_etypes, + expected_account_name=user_name, + preauth_key=preauth_key, + ticket_decryption_key=krbtgt_decryption_key, + pac_request=True) + as_req_base.assertIsNotNone(as_rep) + + msg_type = as_rep['msg-type'] + if expect_error and msg_type != KRB_ERROR or ( + not expect_error and msg_type != KRB_AS_REP): + raise AssertionError(f'wrong message type {msg_type}') + + if not expect_error: + return ConnectionResult.SUCCESS + + error_code = as_rep['error-code'] + if error_code == KDC_ERR_CLIENT_REVOKED: + return ConnectionResult.LOCKED_OUT + elif error_code == KDC_ERR_PREAUTH_FAILED: + return ConnectionResult.WRONG_PASSWORD + else: + raise AssertionError(f'wrong error code {error_code}') + + +def connect_ntlm(pipe, + url, + hostname, + username, + password, + domain, + realm, + workstation, + dn): + user_creds = KerberosCredentials() + user_creds.set_username(username) + user_creds.set_password(password) + user_creds.set_domain(domain) + user_creds.set_workstation(workstation) + user_creds.set_kerberos_state(DONT_USE_KERBEROS) + + # Indicate that we're ready. This ensures we hit the right transaction + # lock. + pipe.send_bytes(b'0') + + # Wait for the main process to take out a transaction lock. + if not pipe.poll(timeout=5): + raise AssertionError('main process failed to indicate readiness') + + try: + # Try connecting to SamDB. This should fail, either due to our + # account being locked out or due to using the wrong password. + SamDB(url=url, + credentials=user_creds, + lp=env_loadparm()) + except ldb.LdbError as err: + num, estr = err.args + + if num != ldb.ERR_INVALID_CREDENTIALS: + raise AssertionError(f'connection raised wrong error code ' + f'({err})') + + if f'data {werror.WERR_ACCOUNT_LOCKED_OUT:x},' in estr: + return ConnectionResult.LOCKED_OUT + elif f'data {werror.WERR_LOGON_FAILURE:x},' in estr: + return ConnectionResult.WRONG_PASSWORD + else: + raise AssertionError(f'connection raised wrong error code ' + f'({estr})') + else: + return ConnectionResult.SUCCESS + + +def connect_samr(pipe, + url, + hostname, + username, + password, + domain, + realm, + workstation, + dn): + # Get the user's NT hash. + user_creds = KerberosCredentials() + user_creds.set_password(password) + nt_hash = user_creds.get_nt_hash() + + # Generate a new UTF-16 password. + new_password = generate_random_password(32, 32) + new_password = new_password.encode('utf-16le') + + # Generate the MD4 hash of the password. + new_password_md4 = md4_hash_blob(new_password) + + # Prefix the password with padding so it is 512 bytes long. + new_password_len = len(new_password) + remaining_len = 512 - new_password_len + new_password = bytes(remaining_len) + new_password + + # Append the 32-bit length of the password.. + new_password += int.to_bytes(new_password_len, + length=4, + byteorder='little') + + # Encrypt the password with RC4 and the existing NT hash. + encryptor = Cipher(algorithms.ARC4(nt_hash), + None, + default_backend()).encryptor() + new_password = encryptor.update(new_password) + + # Create a key from the MD4 hash of the new password. + key = new_password_md4[:14] + + # Encrypt the old NT hash with DES to obtain the verifier. + verifier = des_crypt_blob_16(nt_hash, key) + + server = lsa.String() + server.string = hostname + + account = lsa.String() + account.string = username + + nt_password = samr.CryptPassword() + nt_password.data = list(new_password) + + nt_verifier = samr.Password() + nt_verifier.hash = list(verifier) + + conn = samr.samr(f'ncacn_np:{hostname}[krb5,seal,smb2]') + + # Indicate that we're ready. This ensures we hit the right transaction + # lock. + pipe.send_bytes(b'0') + + # Wait for the main process to take out a transaction lock. + if not pipe.poll(timeout=5): + raise AssertionError('main process failed to indicate readiness') + + try: + # Try changing the password. This should fail, either due to our + # account being locked out or due to using the wrong password. + conn.ChangePasswordUser3(server=server, + account=account, + nt_password=nt_password, + nt_verifier=nt_verifier, + lm_change=True, + lm_password=None, + lm_verifier=None, + password3=None) + except NTSTATUSError as err: + num, estr = err.args + + if num == ntstatus.NT_STATUS_ACCOUNT_LOCKED_OUT: + return ConnectionResult.LOCKED_OUT + elif num == ntstatus.NT_STATUS_WRONG_PASSWORD: + return ConnectionResult.WRONG_PASSWORD + else: + raise AssertionError(f'pwd change raised wrong error code ' + f'({num:08X})') + else: + return ConnectionResult.SUCCESS + + +def connect_samr_aes(pipe, + url, + hostname, + username, + password, + domain, + realm, + workstation, + dn): + # Get the user's NT hash. + user_creds = KerberosCredentials() + user_creds.set_password(password) + nt_hash = user_creds.get_nt_hash() + + # Generate a new UTF-16 password. + new_password = generate_random_password(32, 32) + new_password = new_password.encode('utf-16le') + + # Prepend the 16-bit length of the password.. + new_password_len = int.to_bytes(len(new_password), + length=2, + byteorder='little') + new_password = new_password_len + new_password + + server = lsa.String() + server.string = hostname + + account = lsa.String() + account.string = username + + # Derive a key from the user's NT hash. + iv = generate_random_bytes(16) + iterations = 5555 + cek = sha512_pbkdf2(nt_hash, iv, iterations) + + enc_key_salt = (b'Microsoft SAM encryption key ' + b'AEAD-AES-256-CBC-HMAC-SHA512 16\0') + mac_key_salt = (b'Microsoft SAM MAC key ' + b'AEAD-AES-256-CBC-HMAC-SHA512 16\0') + + # Encrypt the new password. + ciphertext, auth_data = aead_aes_256_cbc_hmac_sha512_blob(new_password, + cek, + enc_key_salt, + mac_key_salt, + iv) + + # Create the new password structure + pwd_buf = samr.EncryptedPasswordAES() + pwd_buf.auth_data = list(auth_data) + pwd_buf.salt = list(iv) + pwd_buf.cipher_len = len(ciphertext) + pwd_buf.cipher = list(ciphertext) + pwd_buf.PBKDF2Iterations = iterations + + conn = samr.samr(f'ncacn_np:{hostname}[krb5,seal,smb2]') + + # Indicate that we're ready. This ensures we hit the right transaction + # lock. + pipe.send_bytes(b'0') + + # Wait for the main process to take out a transaction lock. + if not pipe.poll(timeout=5): + raise AssertionError('main process failed to indicate readiness') + + try: + # Try changing the password. This should fail, either due to our + # account being locked out or due to using the wrong password. + conn.ChangePasswordUser4(server=server, + account=account, + password=pwd_buf) + except NTSTATUSError as err: + num, estr = err.args + + if num == ntstatus.NT_STATUS_ACCOUNT_LOCKED_OUT: + return ConnectionResult.LOCKED_OUT + elif num == ntstatus.NT_STATUS_WRONG_PASSWORD: + return ConnectionResult.WRONG_PASSWORD + else: + raise AssertionError(f'pwd change raised wrong error code ' + f'({num:08X})') + else: + return ConnectionResult.SUCCESS + + +def ldap_pwd_change(pipe, + url, + hostname, + username, + password, + domain, + realm, + workstation, + dn): + lp = env_loadparm() + + admin_creds = KerberosCredentials() + admin_creds.guess(lp) + admin_creds.set_username(env_get_var_value('ADMIN_USERNAME')) + admin_creds.set_password(env_get_var_value('ADMIN_PASSWORD')) + admin_creds.set_kerberos_state(MUST_USE_KERBEROS) + + samdb = SamDB(url=url, + credentials=admin_creds, + lp=lp) + + old_utf16pw = f'"{password}"'.encode('utf-16le') + + new_password = generate_random_password(32, 32) + new_utf16pw = f'"{new_password}"'.encode('utf-16le') + + msg = ldb.Message(ldb.Dn(samdb, dn)) + msg['0'] = ldb.MessageElement(old_utf16pw, + ldb.FLAG_MOD_DELETE, + 'unicodePwd') + msg['1'] = ldb.MessageElement(new_utf16pw, + ldb.FLAG_MOD_ADD, + 'unicodePwd') + + # Indicate that we're ready. This ensures we hit the right transaction + # lock. + pipe.send_bytes(b'0') + + # Wait for the main process to take out a transaction lock. + if not pipe.poll(timeout=5): + raise AssertionError('main process failed to indicate readiness') + + # Try changing the user's password. This should fail, either due to the + # user's account being locked out or due to specifying the wrong password. + try: + samdb.modify(msg) + except ldb.LdbError as err: + num, estr = err.args + if num != ldb.ERR_CONSTRAINT_VIOLATION: + raise AssertionError(f'pwd change raised wrong error code ({err})') + + if f'<{werror.WERR_ACCOUNT_LOCKED_OUT:08X}:' in estr: + return ConnectionResult.LOCKED_OUT + elif f'<{werror.WERR_INVALID_PASSWORD:08X}:' in estr: + return ConnectionResult.WRONG_PASSWORD + else: + raise AssertionError(f'pwd change raised wrong error code ' + f'({estr})') + else: + return ConnectionResult.SUCCESS + + +class LockoutTests(KDCBaseTest): + + def setUp(self): + super().setUp() + self.do_asn1_print = global_asn1_print + self.do_hexdump = global_hexdump + + samdb = self.get_samdb() + base_dn = ldb.Dn(samdb, samdb.domain_dn()) + + def modify_attr(attr, value): + if value is None: + value = [] + flag = ldb.FLAG_MOD_DELETE + else: + value = str(value) + flag = ldb.FLAG_MOD_REPLACE + + msg = ldb.Message(base_dn) + msg[attr] = ldb.MessageElement( + value, flag, attr) + samdb.modify(msg) + + res = samdb.search(base_dn, + scope=ldb.SCOPE_BASE, + attrs=['lockoutDuration', + 'lockoutThreshold', + 'msDS-LogonTimeSyncInterval']) + self.assertEqual(1, len(res)) + + # Reset the lockout duration as it was before. + lockout_duration = res[0].get('lockoutDuration', idx=0) + self.addCleanup(modify_attr, 'lockoutDuration', lockout_duration) + + # Set the new lockout duration: locked out accounts now stay locked + # out. + modify_attr('lockoutDuration', 0) + + # Reset the lockout threshold as it was before. + lockout_threshold = res[0].get('lockoutThreshold', idx=0) + self.addCleanup(modify_attr, 'lockoutThreshold', lockout_threshold) + + # Set the new lockout threshold. + self.lockout_threshold = 3 + modify_attr('lockoutThreshold', self.lockout_threshold) + + # Reset the logon time sync interval as it was before. + sync_interval = res[0].get('msDS-LogonTimeSyncInterval', idx=0) + self.addCleanup(modify_attr, + 'msDS-LogonTimeSyncInterval', + sync_interval) + + # Set the new logon time sync interval. Setting it to 0 eliminates the + # need for this attribute to be updated on logon, and thus the + # requirement to take out a transaction. + modify_attr('msDS-LogonTimeSyncInterval', 0) + + # Get the old 'minPwdAge'. + minPwdAge = samdb.get_minPwdAge() + + # Reset the 'minPwdAge' as it was before. + self.addCleanup(samdb.set_minPwdAge, minPwdAge) + + # Set it temporarily to '0'. + samdb.set_minPwdAge('0') + + def assertLocalSamDB(self, samdb): + if samdb.url.startswith('tdb://'): + return + if samdb.url.startswith('mdb://'): + return + + self.fail(f'connection to {samdb.url} is not local!') + + def wait_for_ready(self, pipe, future): + if pipe.poll(timeout=5): + return + + # We failed to read a response from the pipe, so see if the test raised + # an exception with more information. + if future.done(): + exception = future.exception(timeout=0) + if exception is not None: + raise exception + + self.fail('test failed to indicate readiness') + + def test_lockout_transaction_kdc(self): + self.do_lockout_transaction(connect_kdc) + + def test_lockout_transaction_ntlm(self): + self.do_lockout_transaction(connect_ntlm) + + def test_lockout_transaction_samr(self): + self.do_lockout_transaction(connect_samr) + + def test_lockout_transaction_samr_aes(self): + if not self.gnutls_pbkdf2_support: + self.skipTest('gnutls_pbkdf2() is not available') + self.do_lockout_transaction(connect_samr_aes) + + def test_lockout_transaction_ldap_pw_change(self): + self.do_lockout_transaction(ldap_pwd_change) + + # Tests to ensure we can handle the account being renamed. We do not test + # renames with SAMR password changes, because in that case the entire + # process happens inside a transaction, and the password change method only + # receives the account username. By the time it searches for the account, + # it will have already been renamed, and so it will always fail to find the + # account. + + def test_lockout_transaction_rename_kdc(self): + self.do_lockout_transaction(connect_kdc, rename=True) + + def test_lockout_transaction_rename_ntlm(self): + self.do_lockout_transaction(connect_ntlm, rename=True) + + def test_lockout_transaction_rename_ldap_pw_change(self): + self.do_lockout_transaction(ldap_pwd_change, rename=True) + + def test_lockout_transaction_bad_pwd_kdc(self): + self.do_lockout_transaction(connect_kdc, correct_pw=False) + + def test_lockout_transaction_bad_pwd_ntlm(self): + self.do_lockout_transaction(connect_ntlm, correct_pw=False) + + def test_lockout_transaction_bad_pwd_samr(self): + self.do_lockout_transaction(connect_samr, correct_pw=False) + + def test_lockout_transaction_bad_pwd_samr_aes(self): + if not self.gnutls_pbkdf2_support: + self.skipTest('gnutls_pbkdf2() is not available') + self.do_lockout_transaction(connect_samr_aes, correct_pw=False) + + def test_lockout_transaction_bad_pwd_ldap_pw_change(self): + self.do_lockout_transaction(ldap_pwd_change, correct_pw=False) + + def test_bad_pwd_count_transaction_kdc(self): + self.do_bad_pwd_count_transaction(connect_kdc) + + def test_bad_pwd_count_transaction_ntlm(self): + self.do_bad_pwd_count_transaction(connect_ntlm) + + def test_bad_pwd_count_transaction_samr(self): + self.do_bad_pwd_count_transaction(connect_samr) + + def test_bad_pwd_count_transaction_samr_aes(self): + if not self.gnutls_pbkdf2_support: + self.skipTest('gnutls_pbkdf2() is not available') + self.do_bad_pwd_count_transaction(connect_samr_aes) + + def test_bad_pwd_count_transaction_ldap_pw_change(self): + self.do_bad_pwd_count_transaction(ldap_pwd_change) + + def test_bad_pwd_count_transaction_rename_kdc(self): + self.do_bad_pwd_count_transaction(connect_kdc, rename=True) + + def test_bad_pwd_count_transaction_rename_ntlm(self): + self.do_bad_pwd_count_transaction(connect_ntlm, rename=True) + + def test_bad_pwd_count_transaction_rename_ldap_pw_change(self): + self.do_bad_pwd_count_transaction(ldap_pwd_change, rename=True) + + def test_lockout_race_kdc(self): + self.do_lockout_race(connect_kdc) + + def test_lockout_race_ntlm(self): + self.do_lockout_race(connect_ntlm) + + def test_lockout_race_samr(self): + self.do_lockout_race(connect_samr) + + def test_lockout_race_samr_aes(self): + if not self.gnutls_pbkdf2_support: + self.skipTest('gnutls_pbkdf2() is not available') + self.do_lockout_race(connect_samr_aes) + + def test_lockout_race_ldap_pw_change(self): + self.do_lockout_race(ldap_pwd_change) + + def test_logon_without_transaction_ntlm(self): + self.do_logon_without_transaction(connect_ntlm) + + # Tests to ensure that the connection functions work correctly in the happy + # path. + + def test_logon_kdc(self): + self.do_logon(partial(connect_kdc, expect_error=False)) + + def test_logon_ntlm(self): + self.do_logon(connect_ntlm) + + def test_logon_samr(self): + self.do_logon(connect_samr) + + def test_logon_samr_aes(self): + if not self.gnutls_pbkdf2_support: + self.skipTest('gnutls_pbkdf2() is not available') + self.do_logon(connect_samr_aes) + + def test_logon_ldap_pw_change(self): + self.do_logon(ldap_pwd_change) + + # Test that connection without a correct password works. + def do_logon(self, connect_fn): + # Create the user account for testing. + user_creds = self.get_cached_creds(account_type=self.AccountType.USER, + use_cache=False) + user_dn = user_creds.get_dn() + + admin_creds = self.get_admin_creds() + lp = self.get_lp() + + # Get a connection to our local SamDB. + samdb = connect_samdb(samdb_url=lp.samdb_url(), lp=lp, + credentials=admin_creds) + self.assertLocalSamDB(samdb) + + password = user_creds.get_password() + + # Prepare to connect to the server with a valid password. + our_pipe, their_pipe = Pipe(duplex=True) + + # Inform the test function that it may proceed. + our_pipe.send_bytes(b'0') + + result = connect_fn(pipe=their_pipe, + url=f'ldap://{samdb.host_dns_name()}', + hostname=samdb.host_dns_name(), + username=user_creds.get_username(), + password=password, + domain=user_creds.get_domain(), + realm=user_creds.get_realm(), + workstation=user_creds.get_workstation(), + dn=str(user_dn)) + + # The connection should succeed. + self.assertEqual(result, ConnectionResult.SUCCESS) + + # Lock out the account while holding a transaction lock, then release the + # lock. A logon attempt already in progress should reread the account + # details and recognise the account is locked out. The account can + # additionally be renamed within the transaction to ensure that, by using + # the GUID, rereading the account's details still succeeds. + def do_lockout_transaction(self, connect_fn, + rename=False, + correct_pw=True): + # Create the user account for testing. + user_creds = self.get_cached_creds(account_type=self.AccountType.USER, + use_cache=False) + user_dn = user_creds.get_dn() + + admin_creds = self.get_admin_creds() + lp = self.get_lp() + + # Get a connection to our local SamDB. + samdb = connect_samdb(samdb_url=lp.samdb_url(), lp=lp, + credentials=admin_creds) + self.assertLocalSamDB(samdb) + + password = user_creds.get_password() + if not correct_pw: + password = password[:-1] + + # Prepare to connect to the server. + with futures.ProcessPoolExecutor(max_workers=1) as executor: + our_pipe, their_pipe = Pipe(duplex=True) + connect_future = executor.submit( + connect_fn, + pipe=their_pipe, + url=f'ldap://{samdb.host_dns_name()}', + hostname=samdb.host_dns_name(), + username=user_creds.get_username(), + password=password, + domain=user_creds.get_domain(), + realm=user_creds.get_realm(), + workstation=user_creds.get_workstation(), + dn=str(user_dn)) + + # Wait until the test process indicates it's ready. + self.wait_for_ready(our_pipe, connect_future) + + # Take out a transaction. + samdb.transaction_start() + try: + # Lock out the account. We must do it using an actual password + # check like so, rather than directly with a database + # modification, so that the account is also added to the + # auxiliary bad password database. + + old_utf16pw = f'"Secret007"'.encode('utf-16le') # invalid pwd + new_utf16pw = f'"Secret008"'.encode('utf-16le') + + msg = ldb.Message(user_dn) + msg['0'] = ldb.MessageElement(old_utf16pw, + ldb.FLAG_MOD_DELETE, + 'unicodePwd') + msg['1'] = ldb.MessageElement(new_utf16pw, + ldb.FLAG_MOD_ADD, + 'unicodePwd') + + for i in range(self.lockout_threshold): + try: + samdb.modify(msg) + except ldb.LdbError as err: + num, estr = err.args + + # We get an error, but the bad password count should + # still be updated. + self.assertEqual(num, ldb.ERR_OPERATIONS_ERROR) + self.assertEqual('Failed to obtain remote address for ' + 'the LDAP client while changing the ' + 'password', + estr) + else: + self.fail('pwd change should have failed') + + # Ensure the account is locked out. + + res = samdb.search( + user_dn, scope=ldb.SCOPE_BASE, + attrs=['msDS-User-Account-Control-Computed']) + self.assertEqual(1, len(res)) + + uac = int(res[0].get('msDS-User-Account-Control-Computed', + idx=0)) + self.assertTrue(uac & dsdb.UF_LOCKOUT) + + # Now the bad password database has been updated, inform the + # test process that it may proceed. + our_pipe.send_bytes(b'0') + + # Wait one second to ensure the test process hits the + # transaction lock. + time.sleep(1) + + if rename: + # While we're at it, rename the account to ensure that is + # also safe if a race occurs. + msg = ldb.Message(user_dn) + new_username = self.get_new_username() + msg['sAMAccountName'] = ldb.MessageElement( + new_username, + ldb.FLAG_MOD_REPLACE, + 'sAMAccountName') + samdb.modify(msg) + + except Exception: + samdb.transaction_cancel() + raise + + # Commit the local transaction. + samdb.transaction_commit() + + result = connect_future.result(timeout=5) + self.assertEqual(result, ConnectionResult.LOCKED_OUT) + + # Update the bad password count while holding a transaction lock, then + # release the lock. A logon attempt already in progress should reread the + # account details and ensure the bad password count is atomically + # updated. The account can additionally be renamed within the transaction + # to ensure that, by using the GUID, rereading the account's details still + # succeeds. + def do_bad_pwd_count_transaction(self, connect_fn, rename=False): + # Create the user account for testing. + user_creds = self.get_cached_creds(account_type=self.AccountType.USER, + use_cache=False) + user_dn = user_creds.get_dn() + + admin_creds = self.get_admin_creds() + lp = self.get_lp() + + # Get a connection to our local SamDB. + samdb = connect_samdb(samdb_url=lp.samdb_url(), lp=lp, + credentials=admin_creds) + self.assertLocalSamDB(samdb) + + # Prepare to connect to the server with an invalid password. + with futures.ProcessPoolExecutor(max_workers=1) as executor: + our_pipe, their_pipe = Pipe(duplex=True) + connect_future = executor.submit( + connect_fn, + pipe=their_pipe, + url=f'ldap://{samdb.host_dns_name()}', + hostname=samdb.host_dns_name(), + username=user_creds.get_username(), + password=user_creds.get_password()[:-1], # invalid password + domain=user_creds.get_domain(), + realm=user_creds.get_realm(), + workstation=user_creds.get_workstation(), + dn=str(user_dn)) + + # Wait until the test process indicates it's ready. + self.wait_for_ready(our_pipe, connect_future) + + # Take out a transaction. + samdb.transaction_start() + try: + # Inform the test process that it may proceed. + our_pipe.send_bytes(b'0') + + # Wait one second to ensure the test process hits the + # transaction lock. + time.sleep(1) + + # Set badPwdCount to 1. + msg = ldb.Message(user_dn) + now = int(time.time()) + bad_pwd_time = unix2nttime(now) + msg['badPwdCount'] = ldb.MessageElement( + '1', + ldb.FLAG_MOD_REPLACE, + 'badPwdCount') + msg['badPasswordTime'] = ldb.MessageElement( + str(bad_pwd_time), + ldb.FLAG_MOD_REPLACE, + 'badPasswordTime') + if rename: + # While we're at it, rename the account to ensure that is + # also safe if a race occurs. + new_username = self.get_new_username() + msg['sAMAccountName'] = ldb.MessageElement( + new_username, + ldb.FLAG_MOD_REPLACE, + 'sAMAccountName') + samdb.modify(msg) + + # Ensure the account is not yet locked out. + + res = samdb.search( + user_dn, scope=ldb.SCOPE_BASE, + attrs=['msDS-User-Account-Control-Computed']) + self.assertEqual(1, len(res)) + + uac = int(res[0].get('msDS-User-Account-Control-Computed', + idx=0)) + self.assertFalse(uac & dsdb.UF_LOCKOUT) + except Exception: + samdb.transaction_cancel() + raise + + # Commit the local transaction. + samdb.transaction_commit() + + result = connect_future.result(timeout=5) + self.assertEqual(result, ConnectionResult.WRONG_PASSWORD, result) + + # Check that badPwdCount has now increased to 2. + + res = samdb.search(user_dn, + scope=ldb.SCOPE_BASE, + attrs=['badPwdCount']) + self.assertEqual(1, len(res)) + + bad_pwd_count = int(res[0].get('badPwdCount', idx=0)) + self.assertEqual(2, bad_pwd_count) + + # Attempt to log in to the account with an incorrect password, using + # lockoutThreshold+1 simultaneous attempts. We should get three 'wrong + # password' errors and one 'locked out' error, showing that the bad + # password count is checked and incremented atomically. + def do_lockout_race(self, connect_fn): + # Create the user account for testing. + user_creds = self.get_cached_creds(account_type=self.AccountType.USER, + use_cache=False) + user_dn = user_creds.get_dn() + + admin_creds = self.get_admin_creds() + lp = self.get_lp() + + # Get a connection to our local SamDB. + samdb = connect_samdb(samdb_url=lp.samdb_url(), lp=lp, + credentials=admin_creds) + self.assertLocalSamDB(samdb) + + # Prepare to connect to the server with an invalid password, using four + # simultaneous requests. Only three of those attempts should get + # through before the account is locked out. + num_attempts = self.lockout_threshold + 1 + with futures.ProcessPoolExecutor(max_workers=num_attempts) as executor: + connect_futures = [] + our_pipes = [] + for i in range(num_attempts): + our_pipe, their_pipe = Pipe(duplex=True) + our_pipes.append(our_pipe) + + connect_future = executor.submit( + connect_fn, + pipe=their_pipe, + url=f'ldap://{samdb.host_dns_name()}', + hostname=samdb.host_dns_name(), + username=user_creds.get_username(), + password=user_creds.get_password()[:-1], # invalid pw + domain=user_creds.get_domain(), + realm=user_creds.get_realm(), + workstation=user_creds.get_workstation(), + dn=str(user_dn)) + connect_futures.append(connect_future) + + # Wait until the test process indicates it's ready. + self.wait_for_ready(our_pipe, connect_future) + + # Take out a transaction. + samdb.transaction_start() + try: + # Inform the test processes that they may proceed. + for our_pipe in our_pipes: + our_pipe.send_bytes(b'0') + + # Wait one second to ensure the test processes hit the + # transaction lock. + time.sleep(1) + except Exception: + samdb.transaction_cancel() + raise + + # Commit the local transaction. + samdb.transaction_commit() + + lockouts = 0 + wrong_passwords = 0 + for i, connect_future in enumerate(connect_futures): + result = connect_future.result(timeout=5) + if result == ConnectionResult.LOCKED_OUT: + lockouts += 1 + elif result == ConnectionResult.WRONG_PASSWORD: + wrong_passwords += 1 + else: + self.fail(f'process {i} gave an unexpected result ' + f'{result}') + + self.assertEqual(wrong_passwords, self.lockout_threshold) + self.assertEqual(lockouts, num_attempts - self.lockout_threshold) + + # Ensure the account is now locked out. + + res = samdb.search( + user_dn, scope=ldb.SCOPE_BASE, + attrs=['badPwdCount', + 'msDS-User-Account-Control-Computed']) + self.assertEqual(1, len(res)) + + bad_pwd_count = int(res[0].get('badPwdCount', idx=0)) + self.assertEqual(self.lockout_threshold, bad_pwd_count) + + uac = int(res[0].get('msDS-User-Account-Control-Computed', + idx=0)) + self.assertTrue(uac & dsdb.UF_LOCKOUT) + + # Test that logon is possible even while we locally hold a transaction + # lock. This test only works with NTLM authentication; Kerberos + # authentication must take out a transaction to update the logonCount + # attribute, and LDAP and SAMR password changes both take out a transaction + # to effect the password change. NTLM is the only logon method that does + # not require a transaction, and can thus be performed while we're holding + # the lock. + def do_logon_without_transaction(self, connect_fn): + # Create the user account for testing. + user_creds = self.get_cached_creds(account_type=self.AccountType.USER, + use_cache=False) + user_dn = user_creds.get_dn() + + admin_creds = self.get_admin_creds() + lp = self.get_lp() + + # Get a connection to our local SamDB. + samdb = connect_samdb(samdb_url=lp.samdb_url(), lp=lp, + credentials=admin_creds) + self.assertLocalSamDB(samdb) + + password = user_creds.get_password() + + # Prepare to connect to the server with a valid password. + with futures.ProcessPoolExecutor(max_workers=1) as executor: + our_pipe, their_pipe = Pipe(duplex=True) + connect_future = executor.submit( + connect_fn, + pipe=their_pipe, + url=f'ldap://{samdb.host_dns_name()}', + hostname=samdb.host_dns_name(), + username=user_creds.get_username(), + password=password, + domain=user_creds.get_domain(), + realm=user_creds.get_realm(), + workstation=user_creds.get_workstation(), + dn=str(user_dn)) + + # Wait until the test process indicates it's ready. + self.wait_for_ready(our_pipe, connect_future) + + # Take out a transaction. + samdb.transaction_start() + try: + # Inform the test process that it may proceed. + our_pipe.send_bytes(b'0') + + # The connection should succeed, despite our holding a + # transaction. + result = connect_future.result(timeout=5) + self.assertEqual(result, ConnectionResult.SUCCESS) + except Exception: + samdb.transaction_cancel() + raise + + # Commit the local transaction. + samdb.transaction_commit() + + +if __name__ == '__main__': + global_asn1_print = False + global_hexdump = False + import unittest + unittest.main() diff --git a/python/samba/tests/krb5/raw_testcase.py b/python/samba/tests/krb5/raw_testcase.py index a7e3add60b7..7b9444d7425 100644 --- a/python/samba/tests/krb5/raw_testcase.py +++ b/python/samba/tests/krb5/raw_testcase.py @@ -50,6 +50,7 @@ from samba.tests.krb5.rfc4120_constants import ( AD_IF_RELEVANT, AD_WIN2K_PAC, FX_FAST_ARMOR_AP_REQUEST, + KDC_ERR_CLIENT_REVOKED, KDC_ERR_GENERIC, KDC_ERR_POLICY, KDC_ERR_PREAUTH_FAILED, @@ -640,6 +641,13 @@ class RawKerberosTest(TestCaseInTempDir): tkt_sig_support = '0' cls.tkt_sig_support = bool(int(tkt_sig_support)) + gnutls_pbkdf2_support = samba.tests.env_get_var_value( + 'GNUTLS_PBKDF2_SUPPORT', + allow_missing=True) + if gnutls_pbkdf2_support is None: + gnutls_pbkdf2_support = '1' + cls.gnutls_pbkdf2_support = bool(int(gnutls_pbkdf2_support)) + expect_pac = samba.tests.env_get_var_value('EXPECT_PAC', allow_missing=True) if expect_pac is None: @@ -3489,7 +3497,7 @@ class RawKerberosTest(TestCaseInTempDir): expected_patypes += (PADATA_ETYPE_INFO2,) if error_code not in (KDC_ERR_PREAUTH_FAILED, KDC_ERR_SKEW, - KDC_ERR_POLICY): + KDC_ERR_POLICY, KDC_ERR_CLIENT_REVOKED): if sent_fast: expected_patypes += (PADATA_ENCRYPTED_CHALLENGE,) else: diff --git a/python/samba/tests/krb5/rfc4120_constants.py b/python/samba/tests/krb5/rfc4120_constants.py index 7d20093f97d..16527f13593 100644 --- a/python/samba/tests/krb5/rfc4120_constants.py +++ b/python/samba/tests/krb5/rfc4120_constants.py @@ -88,6 +88,7 @@ KDC_ERR_POLICY = 12 KDC_ERR_BADOPTION = 13 KDC_ERR_ETYPE_NOSUPP = 14 KDC_ERR_SUMTYPE_NOSUPP = 15 +KDC_ERR_CLIENT_REVOKED = 18 KDC_ERR_TGT_REVOKED = 20 KDC_ERR_PREAUTH_FAILED = 24 KDC_ERR_PREAUTH_REQUIRED = 25 diff --git a/python/samba/tests/usage.py b/python/samba/tests/usage.py index 15ff97764e0..e6845b52e1b 100644 --- a/python/samba/tests/usage.py +++ b/python/samba/tests/usage.py @@ -113,6 +113,7 @@ EXCLUDE_USAGE = { 'python/samba/tests/krb5/protected_users_tests.py', 'python/samba/tests/krb5/nt_hash_tests.py', 'python/samba/tests/krb5/kpasswd_tests.py', + 'python/samba/tests/krb5/lockout_tests.py', } EXCLUDE_HELP = { diff --git a/selftest/flapping.d/ldap-pwd-change-race b/selftest/flapping.d/ldap-pwd-change-race new file mode 100644 index 00000000000..54ed56c1134 --- /dev/null +++ b/selftest/flapping.d/ldap-pwd-change-race @@ -0,0 +1,5 @@ +# This test currently depends on a race. The password_hash dsdb module +# relinquishes and immediately reacquires a transaction lock, and another +# process may be able to acquire it during the short period of time in which it +# is not held. +^samba.tests.krb5.lockout_tests.samba.tests.krb5.lockout_tests.LockoutTests.test_lockout_race_ldap_pw_change.ad_dc:local diff --git a/selftest/knownfail_heimdal_kdc b/selftest/knownfail_heimdal_kdc index 4ae27eacb09..ef913b04952 100644 --- a/selftest/knownfail_heimdal_kdc +++ b/selftest/knownfail_heimdal_kdc @@ -54,3 +54,13 @@ ^samba.tests.krb5.protected_users_tests.samba.tests.krb5.protected_users_tests.ProtectedUsersTests.test_proxiable_as_protected.ad_dc # ^samba.tests.krb5.protected_users_tests.samba.tests.krb5.protected_users_tests.ProtectedUsersTests.test_samr_change_password_protected.ad_dc +# +# Lockout tests +# +^samba.tests.krb5.lockout_tests.samba.tests.krb5.lockout_tests.LockoutTests.test_bad_pwd_count_transaction_kdc.ad_dc:local +^samba.tests.krb5.lockout_tests.samba.tests.krb5.lockout_tests.LockoutTests.test_bad_pwd_count_transaction_ntlm.ad_dc:local +^samba.tests.krb5.lockout_tests.samba.tests.krb5.lockout_tests.LockoutTests.test_bad_pwd_count_transaction_rename_kdc.ad_dc:local +^samba.tests.krb5.lockout_tests.samba.tests.krb5.lockout_tests.LockoutTests.test_bad_pwd_count_transaction_rename_ntlm.ad_dc:local +^samba.tests.krb5.lockout_tests.samba.tests.krb5.lockout_tests.LockoutTests.test_bad_pwd_count_transaction_samr.ad_dc:local +^samba.tests.krb5.lockout_tests.samba.tests.krb5.lockout_tests.LockoutTests.test_lockout_race_kdc.ad_dc:local +^samba.tests.krb5.lockout_tests.samba.tests.krb5.lockout_tests.LockoutTests.test_lockout_race_ntlm.ad_dc:local diff --git a/selftest/knownfail_mit_kdc b/selftest/knownfail_mit_kdc index 8601da3d79a..6ece892767a 100644 --- a/selftest/knownfail_mit_kdc +++ b/selftest/knownfail_mit_kdc @@ -439,3 +439,17 @@ samba.tests.krb5.as_canonicalization_tests.samba.tests.krb5.as_canonicalization_ ^samba.tests.krb5.kpasswd_tests.samba.tests.krb5.kpasswd_tests.KpasswdTests.test_kpasswd_canonicalize_realm_case.ad_dc ^samba.tests.krb5.kpasswd_tests.samba.tests.krb5.kpasswd_tests.KpasswdTests.test_kpasswd_no_canonicalize_realm_case.ad_dc ^samba.tests.krb5.kpasswd_tests.samba.tests.krb5.kpasswd_tests.KpasswdTests.test_kpasswd_ticket_requester_sid_tgs.ad_dc +# +# Lockout tests +# +^samba.tests.krb5.lockout_tests.samba.tests.krb5.lockout_tests.LockoutTests.test_bad_pwd_count_transaction_kdc.ad_dc:local +^samba.tests.krb5.lockout_tests.samba.tests.krb5.lockout_tests.LockoutTests.test_bad_pwd_count_transaction_ntlm.ad_dc:local +^samba.tests.krb5.lockout_tests.samba.tests.krb5.lockout_tests.LockoutTests.test_bad_pwd_count_transaction_rename_kdc.ad_dc:local +^samba.tests.krb5.lockout_tests.samba.tests.krb5.lockout_tests.LockoutTests.test_bad_pwd_count_transaction_rename_ntlm.ad_dc:local +^samba.tests.krb5.lockout_tests.samba.tests.krb5.lockout_tests.LockoutTests.test_bad_pwd_count_transaction_samr.ad_dc:local +^samba.tests.krb5.lockout_tests.samba.tests.krb5.lockout_tests.LockoutTests.test_lockout_race_kdc.ad_dc:local +^samba.tests.krb5.lockout_tests.samba.tests.krb5.lockout_tests.LockoutTests.test_lockout_race_ntlm.ad_dc:local +^samba.tests.krb5.lockout_tests.samba.tests.krb5.lockout_tests.LockoutTests.test_lockout_transaction_bad_pwd_kdc.ad_dc:local +^samba.tests.krb5.lockout_tests.samba.tests.krb5.lockout_tests.LockoutTests.test_lockout_transaction_kdc.ad_dc:local +^samba.tests.krb5.lockout_tests.samba.tests.krb5.lockout_tests.LockoutTests.test_lockout_transaction_rename_kdc.ad_dc:local +^samba.tests.krb5.lockout_tests.samba.tests.krb5.lockout_tests.LockoutTests.test_logon_kdc.ad_dc:local diff --git a/source4/selftest/tests.py b/source4/selftest/tests.py index 54906ae589c..2da8bcfcdc0 100755 --- a/source4/selftest/tests.py +++ b/source4/selftest/tests.py @@ -1002,6 +1002,8 @@ if ('SAMBA4_USES_HEIMDAL' in config_hash or else: tkt_sig_support = 0 +gnutls_pbkdf2_support = int('HAVE_GNUTLS_PBKDF2' in config_hash) + if 'HAVE_MIT_KRB5_1_20' in config_hash: kadmin_is_tgs = 1 else: @@ -1022,6 +1024,7 @@ krb5_environ = { 'CLAIMS_SUPPORT': claims_support, 'COMPOUND_ID_SUPPORT': compound_id_support, 'TKT_SIG_SUPPORT': tkt_sig_support, + 'GNUTLS_PBKDF2_SUPPORT': gnutls_pbkdf2_support, 'EXPECT_PAC': expect_pac, 'EXPECT_EXTRA_PAC_BUFFERS': extra_pac_buffers, 'CHECK_CNAME': check_cname, @@ -1727,6 +1730,10 @@ planoldpythontestsuite( 'ad_dc', 'samba.tests.krb5.kpasswd_tests', environ=krb5_environ) +planoldpythontestsuite( + 'ad_dc:local', + 'samba.tests.krb5.lockout_tests', + environ=krb5_environ) for env in [ 'vampire_dc', -- 2.35.0 From d5ed3ac24660910191d2720abb8bfc7b8914ecbd Mon Sep 17 00:00:00 2001 From: Andrew Bartlett Date: Tue, 30 Mar 2021 10:51:26 +1300 Subject: [PATCH 10/41] CVE-2021-20251 s4-rpc_server: Use authsam_search_account() to find the user This helps the bad password and audit log handling code as it allows assumptions to be made about the attributes found in the variable "msg", such as that DSDB_SEARCH_SHOW_EXTENDED_DN was used. This ensures we can re-search on the DN via the embedded GUID, which in in turn rename-proof. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14611 Signed-off-by: Andrew Bartlett Reviewed-by: Joseph Sutton Reviewed-by: Andreas Schneider (cherry picked from commit 439f96a2cfe77f6cbf331d965a387512c2db91c6) --- source4/rpc_server/samr/samr_password.c | 40 +++++++++++-------------- 1 file changed, 17 insertions(+), 23 deletions(-) diff --git a/source4/rpc_server/samr/samr_password.c b/source4/rpc_server/samr/samr_password.c index da22ab7484b..09d7501d474 100644 --- a/source4/rpc_server/samr/samr_password.c +++ b/source4/rpc_server/samr/samr_password.c @@ -312,13 +312,7 @@ NTSTATUS dcesrv_samr_ChangePasswordUser3(struct dcesrv_call_state *dce_call, struct ldb_context *sam_ctx = NULL; struct ldb_dn *user_dn = NULL; int ret; - struct ldb_result *res = NULL; - const char * const attrs[] = { "unicodePwd", "dBCSPwd", - "userAccountControl", - "msDS-ResultantPSO", - "msDS-User-Account-Control-Computed", - "badPwdCount", "badPasswordTime", - "objectSid", NULL }; + struct ldb_message *msg = NULL; struct samr_Password *nt_pwd; struct samr_DomInfo1 *dominfo = NULL; struct userPwdChangeFailureInformation *reject = NULL; @@ -356,26 +350,26 @@ NTSTATUS dcesrv_samr_ChangePasswordUser3(struct dcesrv_call_state *dce_call, return NT_STATUS_INVALID_SYSTEM_SERVICE; } - /* we need the users dn and the domain dn (derived from the - user SID). We also need the current lm and nt password hashes - in order to decrypt the incoming passwords */ - ret = dsdb_search(sam_ctx, mem_ctx, &res, - ldb_get_default_basedn(sam_ctx), - LDB_SCOPE_SUBTREE, attrs, - DSDB_SEARCH_SHOW_EXTENDED_DN, - "(&(sAMAccountName=%s)(objectclass=user))", - ldb_binary_encode_string(mem_ctx, r->in.account->string)); - if (ret != LDB_SUCCESS || res->count != 1) { - status = NT_STATUS_NO_SUCH_USER; /* Converted to WRONG_PASSWORD below */ + /* + * We use authsam_search_account() to be consistent with the + * other callers in the bad password and audit log handling + * systems. It ensures we get DSDB_SEARCH_SHOW_EXTENDED_DN. + */ + status = authsam_search_account(mem_ctx, + sam_ctx, + r->in.account->string, + ldb_get_default_basedn(sam_ctx), + &msg); + if (!NT_STATUS_IS_OK(status)) { goto failed; } - user_dn = res->msgs[0]->dn; - user_samAccountName = ldb_msg_find_attr_as_string(res->msgs[0], "samAccountName", NULL); - user_objectSid = samdb_result_dom_sid(res, res->msgs[0], "objectSid"); + user_dn = msg->dn; + user_samAccountName = ldb_msg_find_attr_as_string(msg, "samAccountName", NULL); + user_objectSid = samdb_result_dom_sid(mem_ctx, msg, "objectSid"); status = samdb_result_passwords(mem_ctx, lp_ctx, - res->msgs[0], &nt_pwd); + msg, &nt_pwd); if (!NT_STATUS_IS_OK(status) ) { goto failed; } @@ -491,7 +485,7 @@ failed: /* Only update the badPwdCount if we found the user */ if (NT_STATUS_EQUAL(status, NT_STATUS_WRONG_PASSWORD)) { - authsam_update_bad_pwd_count(sam_ctx, res->msgs[0], ldb_get_default_basedn(sam_ctx)); + authsam_update_bad_pwd_count(sam_ctx, msg, ldb_get_default_basedn(sam_ctx)); } else if (NT_STATUS_EQUAL(status, NT_STATUS_NO_SUCH_USER)) { /* Don't give the game away: (don't allow anonymous users to prove the existence of usernames) */ status = NT_STATUS_WRONG_PASSWORD; -- 2.35.0 From dd4fd0c41966eaae713d5c7e299ab1f7c8c70ea8 Mon Sep 17 00:00:00 2001 From: Gary Lockyer Date: Tue, 16 Mar 2021 10:52:58 +1300 Subject: [PATCH 11/41] CVE-2021-20251 auth4: split samdb_result_msds_LockoutObservationWindow() out samdb_result_msds_LockoutObservationWindow() is split out of samdb_result_effective_badPwdCount() BUG: https://bugzilla.samba.org/show_bug.cgi?id=14611 Signed-off-by: Gary Lockyer Reviewed-by: Joseph Sutton Reviewed-by: Andreas Schneider Reviewed-by: Andrew Bartlett (cherry picked from commit 2087b0cd986b8959b2a402b9a1891472e47ca0b0) --- source4/dsdb/common/util.c | 45 +++++++++++++++++++++++++++----------- 1 file changed, 32 insertions(+), 13 deletions(-) diff --git a/source4/dsdb/common/util.c b/source4/dsdb/common/util.c index 88b05555b96..451495fe4c5 100644 --- a/source4/dsdb/common/util.c +++ b/source4/dsdb/common/util.c @@ -5363,9 +5363,9 @@ int dsdb_create_partial_replica_NC(struct ldb_context *ldb, struct ldb_dn *dn) * This also requires that the domain_msg have (if present): * - lockOutObservationWindow */ -static int dsdb_effective_badPwdCount(const struct ldb_message *user_msg, - int64_t lockOutObservationWindow, - NTTIME now) +int dsdb_effective_badPwdCount(const struct ldb_message *user_msg, + int64_t lockOutObservationWindow, + NTTIME now) { int64_t badPasswordTime; badPasswordTime = ldb_msg_find_attr_as_int64(user_msg, "badPasswordTime", 0); @@ -5412,25 +5412,24 @@ static struct ldb_result *lookup_user_pso(struct ldb_context *sam_ldb, } /* - * Return the effective badPwdCount + * Return the msDS-LockoutObservationWindow for a user message * * This requires that the user_msg have (if present): - * - badPasswordTime - * - badPwdCount * - msDS-ResultantPSO */ -int samdb_result_effective_badPwdCount(struct ldb_context *sam_ldb, - TALLOC_CTX *mem_ctx, - struct ldb_dn *domain_dn, - const struct ldb_message *user_msg) +int64_t samdb_result_msds_LockoutObservationWindow( + struct ldb_context *sam_ldb, + TALLOC_CTX *mem_ctx, + struct ldb_dn *domain_dn, + const struct ldb_message *user_msg) { - struct timeval tv_now = timeval_current(); - NTTIME now = timeval_to_nttime(&tv_now); int64_t lockOutObservationWindow; struct ldb_result *res = NULL; const char *attrs[] = { "msDS-LockoutObservationWindow", NULL }; - + if (domain_dn == NULL) { + smb_panic("domain dn is NULL"); + } res = lookup_user_pso(sam_ldb, mem_ctx, user_msg, attrs); if (res != NULL) { @@ -5446,7 +5445,27 @@ int samdb_result_effective_badPwdCount(struct ldb_context *sam_ldb, samdb_search_int64(sam_ldb, mem_ctx, 0, domain_dn, "lockOutObservationWindow", NULL); } + return lockOutObservationWindow; +} +/* + * Return the effective badPwdCount + * + * This requires that the user_msg have (if present): + * - badPasswordTime + * - badPwdCount + * - msDS-ResultantPSO + */ +int samdb_result_effective_badPwdCount(struct ldb_context *sam_ldb, + TALLOC_CTX *mem_ctx, + struct ldb_dn *domain_dn, + const struct ldb_message *user_msg) +{ + struct timeval tv_now = timeval_current(); + NTTIME now = timeval_to_nttime(&tv_now); + int64_t lockOutObservationWindow = + samdb_result_msds_LockoutObservationWindow( + sam_ldb, mem_ctx, domain_dn, user_msg); return dsdb_effective_badPwdCount(user_msg, lockOutObservationWindow, now); } -- 2.35.0 From 05c2bf83b1b47236b80f74cb2c902f2126611c7b Mon Sep 17 00:00:00 2001 From: Gary Lockyer Date: Wed, 27 Jan 2021 14:24:58 +1300 Subject: [PATCH 12/41] CVE-2021-20251 s4 auth: Prepare to make bad password count increment atomic To ensure that the bad password count is incremented atomically, and that the successful logon accounting data is updated atomically, without always opening a transaction, we will need to make a note of all bad and successful passwords in a side-DB outside the transaction lock. This provides the functions needed for that and hooks them in (future commits will handle errors and use the results). Based on patches by Gary Lockyer BUG: https://bugzilla.samba.org/show_bug.cgi?id=14611 Signed-off-by: Andrew Bartlett Reviewed-by: Joseph Sutton Reviewed-by: Andreas Schneider (cherry picked from commit 408717242aad8adf4551f2394eee2d80a06c7e63) --- source4/auth/sam.c | 187 +++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 187 insertions(+) diff --git a/source4/auth/sam.c b/source4/auth/sam.c index 420b165446a..33f2c561805 100644 --- a/source4/auth/sam.c +++ b/source4/auth/sam.c @@ -31,6 +31,8 @@ #include "libcli/ldap/ldap_ndr.h" #include "param/param.h" #include "librpc/gen_ndr/ndr_winbind_c.h" +#include "lib/dbwrap/dbwrap.h" +#include "cluster/cluster.h" #undef DBGC_CLASS #define DBGC_CLASS DBGC_AUTH @@ -825,6 +827,177 @@ static int authsam_get_user_pso(struct ldb_context *sam_ctx, return LDB_SUCCESS; } +static struct db_context *authsam_get_bad_password_db( + TALLOC_CTX *mem_ctx, + struct ldb_context *sam_ctx) +{ + struct loadparm_context *lp_ctx = NULL; + const char *db_name = "bad_password"; + struct db_context *db_ctx = NULL; + + lp_ctx = ldb_get_opaque(sam_ctx, "loadparm"); + if (lp_ctx == NULL) { + DBG_ERR("Unable to get loadparm_context\n"); + return NULL; + } + + db_ctx = cluster_db_tmp_open(mem_ctx, lp_ctx, db_name, TDB_DEFAULT); + if (db_ctx == NULL) { + DBG_ERR("Unable to open bad password attempts database\n"); + return NULL; + } + return db_ctx; +} + +static NTSTATUS get_object_sid_as_tdb_data( + TALLOC_CTX *mem_ctx, + const struct ldb_message *msg, + struct dom_sid_buf *buf, + TDB_DATA *key) +{ + struct dom_sid *objectsid = NULL; + + /* + * Convert the objectSID to a human readable form to + * make debugging easier + */ + objectsid = samdb_result_dom_sid(mem_ctx, msg, "objectSID"); + if (objectsid == NULL) { + DBG_ERR("Unable to extract objectSID\n"); + return NT_STATUS_INTERNAL_ERROR; + } + dom_sid_str_buf(objectsid, buf); + key->dptr = (unsigned char *)buf->buf; + key->dsize = strlen(buf->buf); + + talloc_free(objectsid); + + return NT_STATUS_OK; +} + +/* + * Add the users objectSID to the bad password attempt database + * to indicate that last authentication failed due to a bad password + */ +static NTSTATUS authsam_set_bad_password_indicator( + struct ldb_context *sam_ctx, + TALLOC_CTX *mem_ctx, + const struct ldb_message *msg) +{ + NTSTATUS status = NT_STATUS_OK; + struct dom_sid_buf buf; + TDB_DATA key = {0}; + TDB_DATA value = {0}; + struct db_context *db = NULL; + + TALLOC_CTX *ctx = talloc_new(mem_ctx); + if (ctx == NULL) { + return NT_STATUS_NO_MEMORY; + } + + db = authsam_get_bad_password_db(ctx, sam_ctx); + if (db == NULL) { + status = NT_STATUS_INTERNAL_ERROR; + goto exit; + } + + status = get_object_sid_as_tdb_data(ctx, msg, &buf, &key); + if (!NT_STATUS_IS_OK(status)) { + goto exit; + } + + status = dbwrap_store(db, key, value, 0); + if (!NT_STATUS_IS_OK(status)) { + DBG_ERR("Unable to store bad password indicator\n"); + } +exit: + talloc_free(ctx); + return status; +} + +/* + * see if the users objectSID is in the bad password attempt database + */ +static NTSTATUS authsam_check_bad_password_indicator( + struct ldb_context *sam_ctx, + TALLOC_CTX *mem_ctx, + bool *exists, + const struct ldb_message *msg) +{ + NTSTATUS status = NT_STATUS_OK; + struct dom_sid_buf buf; + TDB_DATA key = {0}; + struct db_context *db = NULL; + + TALLOC_CTX *ctx = talloc_new(mem_ctx); + if (ctx == NULL) { + return NT_STATUS_NO_MEMORY; + } + + db = authsam_get_bad_password_db(ctx, sam_ctx); + if (db == NULL) { + status = NT_STATUS_INTERNAL_ERROR; + goto exit; + } + + status = get_object_sid_as_tdb_data(ctx, msg, &buf, &key); + if (!NT_STATUS_IS_OK(status)) { + goto exit; + } + + *exists = dbwrap_exists(db, key); +exit: + talloc_free(ctx); + return status; +} + +/* + * Remove the users objectSID to the bad password attempt database + * to indicate that last authentication succeeded. + */ +static NTSTATUS authsam_clear_bad_password_indicator( + struct ldb_context *sam_ctx, + TALLOC_CTX *mem_ctx, + const struct ldb_message *msg) +{ + NTSTATUS status = NT_STATUS_OK; + struct dom_sid_buf buf; + TDB_DATA key = {0}; + struct db_context *db = NULL; + + TALLOC_CTX *ctx = talloc_new(mem_ctx); + if (ctx == NULL) { + return NT_STATUS_NO_MEMORY; + } + + db = authsam_get_bad_password_db(ctx, sam_ctx); + if (db == NULL) { + status = NT_STATUS_INTERNAL_ERROR; + goto exit; + } + + status = get_object_sid_as_tdb_data(ctx, msg, &buf, &key); + if (!NT_STATUS_IS_OK(status)) { + goto exit; + } + + status = dbwrap_delete(db, key); + if (NT_STATUS_EQUAL(NT_STATUS_NOT_FOUND, status)) { + /* + * Ok there was no bad password indicator this is expected + */ + status = NT_STATUS_OK; + } + if (NT_STATUS_IS_ERR(status)) { + DBG_ERR("Unable to delete bad password indicator, %s %s\n", + nt_errstr(status), + get_friendly_nt_error_msg(status)); + } +exit: + talloc_free(ctx); + return status; +} + NTSTATUS authsam_update_bad_pwd_count(struct ldb_context *sam_ctx, struct ldb_message *msg, struct ldb_dn *domain_dn) @@ -894,6 +1067,10 @@ NTSTATUS authsam_update_bad_pwd_count(struct ldb_context *sam_ctx, ret = dsdb_autotransaction_request(sam_ctx, req); talloc_free(req); + + status = authsam_set_bad_password_indicator( + sam_ctx, mem_ctx, msg); + /* Failure is ignored for now */ } done: @@ -1057,12 +1234,19 @@ NTSTATUS authsam_logon_success_accounting(struct ldb_context *sam_ctx, NTTIME now; NTTIME lastLogonTimestamp; bool am_rodc = false; + bool need_db_reread; mem_ctx = talloc_new(msg); if (mem_ctx == NULL) { return NT_STATUS_NO_MEMORY; } + status = authsam_check_bad_password_indicator( + sam_ctx, mem_ctx, &need_db_reread, msg); + if (!NT_STATUS_IS_OK(status)) { + return status; + } + lockoutTime = ldb_msg_find_attr_as_int64(msg, "lockoutTime", 0); dbBadPwdCount = ldb_msg_find_attr_as_int(msg, "badPwdCount", 0); if (interactive_or_kerberos) { @@ -1197,6 +1381,9 @@ NTSTATUS authsam_logon_success_accounting(struct ldb_context *sam_ctx, talloc_free(req); } + status = authsam_clear_bad_password_indicator(sam_ctx, mem_ctx, msg); + /* Failure is ignored for now */ + done: if (ret != LDB_SUCCESS) { DEBUG(0, ("Failed to set badPwdCount and lockoutTime " -- 2.35.0 From b21c696cb14a41c36713c8857294e1e69150d567 Mon Sep 17 00:00:00 2001 From: Andrew Bartlett Date: Tue, 30 Mar 2021 17:57:10 +1300 Subject: [PATCH 13/41] CVE-2021-20251 auth4: Reread the user record if a bad password is noticed. As is, this is pointless, as we need a transaction to make this any less of a race, but this provides the steps towards that goal. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14611 Signed-off-by: Andrew Bartlett Reviewed-by: Joseph Sutton Reviewed-by: Andreas Schneider (cherry picked from commit 7b8e32efc336fb728e0c7e3dd6fbe2ed54122124) --- source4/auth/sam.c | 82 ++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 82 insertions(+) diff --git a/source4/auth/sam.c b/source4/auth/sam.c index 33f2c561805..92c0480ba6a 100644 --- a/source4/auth/sam.c +++ b/source4/auth/sam.c @@ -827,6 +827,68 @@ static int authsam_get_user_pso(struct ldb_context *sam_ctx, return LDB_SUCCESS; } +/* + * Re-read the bad password and successful logon data for a user. + * + * The DN in the passed user record should contain the "objectGUID" in case the + * object DN has changed. + */ +NTSTATUS authsam_reread_user_logon_data( + struct ldb_context *sam_ctx, + TALLOC_CTX *mem_ctx, + const struct ldb_message *user_msg, + struct ldb_message **current) +{ + const struct ldb_val *v = NULL; + struct ldb_result *res = NULL; + uint16_t acct_flags = 0; + const char *attr_name = "msDS-User-Account-Control-Computed"; + + int ret; + + /* + * Re-read the account details, using the GUID in case the DN + * is being changed (this is automatic in LDB because the + * original search also used DSDB_SEARCH_SHOW_EXTENDED_DN) + * + * We re read all the attributes in user_attrs, rather than using a + * subset to ensure that we can reuse existing validation code. + */ + ret = dsdb_search_dn(sam_ctx, + mem_ctx, + &res, + user_msg->dn, + user_attrs, + DSDB_SEARCH_SHOW_EXTENDED_DN); + if (ret != LDB_SUCCESS) { + DBG_ERR("Unable to re-read account control data for %s\n", + ldb_dn_get_linearized(user_msg->dn)); + return NT_STATUS_INTERNAL_ERROR; + } + + /* + * Ensure the account has not been locked out by another request + */ + v = ldb_msg_find_ldb_val(res->msgs[0], attr_name); + if (v == NULL || v->data == NULL) { + DBG_ERR("No %s attribute for %s\n", + attr_name, + ldb_dn_get_linearized(user_msg->dn)); + TALLOC_FREE(res); + return NT_STATUS_INTERNAL_ERROR; + } + acct_flags = samdb_result_acct_flags(res->msgs[0], attr_name); + if (acct_flags & ACB_AUTOLOCK) { + DBG_WARNING( + "Account for user %s was locked out.\n", + ldb_dn_get_linearized(user_msg->dn)); + TALLOC_FREE(res); + return NT_STATUS_ACCOUNT_LOCKED_OUT; + } + *current = res->msgs[0]; + return NT_STATUS_OK; +} + static struct db_context *authsam_get_bad_password_db( TALLOC_CTX *mem_ctx, struct ldb_context *sam_ctx) @@ -1247,6 +1309,26 @@ NTSTATUS authsam_logon_success_accounting(struct ldb_context *sam_ctx, return status; } + if (need_db_reread) { + struct ldb_message *current = NULL; + + /* + * Re-read the account details, using the GUID + * embedded in DN so this is safe against a race where + * it is being renamed. + */ + status = authsam_reread_user_logon_data( + sam_ctx, mem_ctx, msg, ¤t); + if (!NT_STATUS_IS_OK(status)) { + /* + * The re-read can return account locked out, as well + * as an internal error + */ + return status; + } + msg = current; + } + lockoutTime = ldb_msg_find_attr_as_int64(msg, "lockoutTime", 0); dbBadPwdCount = ldb_msg_find_attr_as_int(msg, "badPwdCount", 0); if (interactive_or_kerberos) { -- 2.35.0 From a772560f027078cf9d08f77356b6fd277d0183ab Mon Sep 17 00:00:00 2001 From: Gary Lockyer Date: Tue, 9 Feb 2021 11:59:05 +1300 Subject: [PATCH 14/41] CVE-2021-20251 s4 auth test: Unit tests for source4/auth/sam.c cmocka unit tests for the authsam_reread_user_logon_data in source4/auth/sam.c BUG: https://bugzilla.samba.org/show_bug.cgi?id=14611 Signed-off-by: Gary Lockyer Reviewed-by: Andreas Schneider Reviewed-by: Andrew Bartlett (cherry picked from commit d6cf245b96fb02edb3bcc52733d040d5f03fb918) --- selftest/knownfail.d/auth-sam | 25 + selftest/tests.py | 2 + source4/auth/tests/sam.c | 2746 +++++++++++++++++++++++++++++++++ source4/auth/wscript_build | 11 + 4 files changed, 2784 insertions(+) create mode 100644 selftest/knownfail.d/auth-sam create mode 100644 source4/auth/tests/sam.c diff --git a/selftest/knownfail.d/auth-sam b/selftest/knownfail.d/auth-sam new file mode 100644 index 00000000000..e87e43d509b --- /dev/null +++ b/selftest/knownfail.d/auth-sam @@ -0,0 +1,25 @@ +^samba.unittests.auth.sam.test_reread_account_not_locked.none +^samba.unittests.auth.sam.test_success_accounting_add_control_failed.none +^samba.unittests.auth.sam.test_success_accounting_build_mod_req_failed.none +^samba.unittests.auth.sam.test_success_accounting_commit_failed.none +^samba.unittests.auth.sam.test_success_accounting_ldb_msg_new_failed.none +^samba.unittests.auth.sam.test_success_accounting_ldb_request_failed.none +^samba.unittests.auth.sam.test_success_accounting_ldb_wait_failed.none +^samba.unittests.auth.sam.test_success_accounting_reread_failed.none +^samba.unittests.auth.sam.test_success_accounting_rollback_failed.none +^samba.unittests.auth.sam.test_success_accounting_samdb_rodc_failed.none +^samba.unittests.auth.sam.test_success_accounting_spurious_bad_pwd_indicator.none +^samba.unittests.auth.sam.test_success_accounting_start_txn_failed.none +^samba.unittests.auth.sam.test_success_accounting_update_lastlogon_failed.none +^samba.unittests.auth.sam.test_update_bad_add_control_failed.none +^samba.unittests.auth.sam.test_update_bad_build_mod_request_failed.none +^samba.unittests.auth.sam.test_update_bad_commit_failed.none +^samba.unittests.auth.sam.test_update_bad_get_pso_failed.none +^samba.unittests.auth.sam.test_update_bad_ldb_request_failed.none +^samba.unittests.auth.sam.test_update_bad_ldb_wait_failed.none +^samba.unittests.auth.sam.test_update_bad_no_update_required.none +^samba.unittests.auth.sam.test_update_bad_reread_failed.none +^samba.unittests.auth.sam.test_update_bad_reread_locked_out.none +^samba.unittests.auth.sam.test_update_bad_start_txn_failed.none +^samba.unittests.auth.sam.test_update_bad_txn_cancel_failed.none +^samba.unittests.auth.sam.test_update_bad_update_count_failed.none diff --git a/selftest/tests.py b/selftest/tests.py index a3780da0615..b5e418cca3b 100644 --- a/selftest/tests.py +++ b/selftest/tests.py @@ -446,6 +446,8 @@ plantestsuite("samba.unittests.test_registry_regfio", "none", [os.path.join(bindir(), "default/source3/test_registry_regfio")]) plantestsuite("samba.unittests.test_oLschema2ldif", "none", [os.path.join(bindir(), "default/source4/utils/oLschema2ldif/test_oLschema2ldif")]) +plantestsuite("samba.unittests.auth.sam", "none", + [os.path.join(bindir(), "test_auth_sam")]) if with_elasticsearch_backend: plantestsuite("samba.unittests.mdsparser_es", "none", [os.path.join(bindir(), "default/source3/test_mdsparser_es")] + [configuration]) diff --git a/source4/auth/tests/sam.c b/source4/auth/tests/sam.c new file mode 100644 index 00000000000..b39408c3699 --- /dev/null +++ b/source4/auth/tests/sam.c @@ -0,0 +1,2746 @@ +/* + * Unit tests for source4/auth/sam.c + * + * Copyright (C) Catalyst.NET Ltd 2021 + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see . + * + */ + +/* + * from cmocka.c: + * These headers or their equivalents should be included prior to + * including + * this header file. + * + * #include + * #include + * #include + * + * This allows test applications to use custom definitions of C standard + * library functions and types. + * + */ + +#include +#include +#include +#include +#include +#include +#include + +#include "includes.h" +#include "auth/sam.c" +#include "ldb.h" +#include "ntstatus.h" +#include "librpc/gen_ndr/ndr_security.h" + +/***************************************************************************** + * wrapped functions + * + *****************************************************************************/ +int __wrap_samdb_msg_add_int64( + struct ldb_context *sam_ldb, + TALLOC_CTX *mem_ctx, + struct ldb_message *msg, + const char *attr_name, + int64_t v); +int __real_samdb_msg_add_int64( + struct ldb_context *sam_ldb, + TALLOC_CTX *mem_ctx, + struct ldb_message *msg, + const char *attr_name, + int64_t v); +int __wrap_samdb_msg_add_int64( + struct ldb_context *sam_ldb, + TALLOC_CTX *mem_ctx, + struct ldb_message *msg, + const char *attr_name, + int64_t v) +{ + + int ret; + ret = (int)mock(); + if (ret != LDB_SUCCESS) { + return ret; + } + return __real_samdb_msg_add_int64(sam_ldb, mem_ctx, msg, attr_name, v); +} +/***************************************************************************** + * Mock implementations + *****************************************************************************/ + +static int check_dn(const LargestIntegralType left_value, + const LargestIntegralType right_value) +{ + /* + * We have to cast away const so we can get the linearized form with + * ldb_dn_get_extended_linearized(). + */ + struct ldb_dn *left_dn = (void *)left_value; + struct ldb_dn *right_dn = (void *)right_value; + char *left_dn_string = NULL; + char *right_dn_string = NULL; + bool ok; + + if (left_dn == NULL && right_dn == NULL) { + return true; + } + + if (left_dn != NULL) { + left_dn_string = ldb_dn_get_extended_linearized(NULL, left_dn, 1); + assert_non_null(left_dn_string); + } + + if (right_dn != NULL) { + right_dn_string = ldb_dn_get_extended_linearized(NULL, right_dn, 1); + assert_non_null(right_dn_string); + } + + if (left_dn_string == NULL || right_dn_string == NULL) { + ok = false; + print_error("\"%s\" != \"%s\"\n", + left_dn_string != NULL ? left_dn_string : "", + right_dn_string != NULL ? right_dn_string : ""); + } else { + ok = (strcmp(left_dn_string, right_dn_string) == 0); + if (!ok) { + print_error("\"%s\" != \"%s\"\n", + left_dn_string, + right_dn_string); + } + + } + + TALLOC_FREE(right_dn_string); + TALLOC_FREE(left_dn_string); + + return ok; +} + +int __wrap_dsdb_search_dn(struct ldb_context *ldb, + TALLOC_CTX *mem_ctx, + struct ldb_result **_result, + struct ldb_dn *basedn, + const char * const *attrs, + uint32_t dsdb_flags); +int __wrap_dsdb_search_dn(struct ldb_context *ldb, + TALLOC_CTX *mem_ctx, + struct ldb_result **_result, + struct ldb_dn *basedn, + const char * const *attrs, + uint32_t dsdb_flags) +{ + check_expected(basedn); + + *_result = talloc_steal(mem_ctx, mock_ptr_type(struct ldb_result *)); + + return mock(); +} + +int ldb_transaction_start_ret = LDB_SUCCESS; +bool in_transaction = false; +int ldb_transaction_start(struct ldb_context *ldb) { + assert_false(in_transaction); + if (ldb_transaction_start_ret == LDB_SUCCESS) { + in_transaction = true; + } + return ldb_transaction_start_ret; +} + +int ldb_transaction_cancel_ret = LDB_SUCCESS; +bool transaction_cancelled = false; +int ldb_transaction_cancel(struct ldb_context *ldb) { + assert_true(in_transaction); + if (ldb_transaction_cancel_ret == LDB_SUCCESS) { + in_transaction = false; + transaction_cancelled = true; + } + return ldb_transaction_cancel_ret; +} + +int ldb_transaction_commit_ret = LDB_SUCCESS; +bool transaction_committed = false; +int ldb_transaction_commit(struct ldb_context *ldb) { + assert_true(in_transaction); + if (ldb_transaction_commit_ret == LDB_SUCCESS) { + in_transaction = false; + transaction_committed = true; + } + return ldb_transaction_commit_ret; +} + +NTSTATUS dsdb_update_bad_pwd_count_ret = NT_STATUS_OK; +struct ldb_message *dsdb_update_bad_pwd_count_res = NULL; +NTSTATUS dsdb_update_bad_pwd_count(TALLOC_CTX *mem_ctx, + struct ldb_context *sam_ctx, + struct ldb_message *user_msg, + struct ldb_message *domain_msg, + struct ldb_message *pso_msg, + struct ldb_message **_mod_msg) { + + *_mod_msg = talloc_move(mem_ctx, &dsdb_update_bad_pwd_count_res); + return dsdb_update_bad_pwd_count_ret; +} + +int ldb_build_mod_req_ret = LDB_SUCCESS; +struct ldb_request *ldb_build_mod_req_res = NULL; +int ldb_build_mod_req(struct ldb_request **ret_req, + struct ldb_context *ldb, + TALLOC_CTX *mem_ctx, + const struct ldb_message *message, + struct ldb_control **controls, + void *context, + ldb_request_callback_t callback, + struct ldb_request *parent) +{ + *ret_req = talloc_move(mem_ctx, &ldb_build_mod_req_res); + return ldb_build_mod_req_ret; +} + +int ldb_request_add_control_ret = LDB_SUCCESS; +int ldb_request_add_control(struct ldb_request *req, + const char *oid, + bool critical, + void *data) +{ + return ldb_request_add_control_ret; +} + +int ldb_request_ret = LDB_SUCCESS; +int ldb_request(struct ldb_context *ldb, + struct ldb_request *req) +{ + return ldb_request_ret; +} + +int ldb_wait_ret = LDB_SUCCESS; +int ldb_wait(struct ldb_handle *handle, + enum ldb_wait_type type) +{ + return ldb_wait_ret; +} +bool ldb_msg_new_fail = false; +struct ldb_message *ldb_msg_new(TALLOC_CTX *mem_ctx) +{ + if (ldb_msg_new_fail) { + return NULL; + } else { + return talloc_zero(mem_ctx, struct ldb_message); + } +} + +int samdb_rodc_ret = LDB_SUCCESS; +bool samdb_rodc_res = false; + +int samdb_rodc( + struct ldb_context *sam_ctx, + bool *am_rodc) +{ + + *am_rodc = samdb_rodc_res; + return samdb_rodc_ret; +} + +struct loadparm_context *ldb_get_opaque_ret = NULL; +void *ldb_get_opaque(struct ldb_context *ldb, const char *name) +{ + return ldb_get_opaque_ret; +} + +struct db_context {}; +struct db_context *cluster_db_tmp_open_ret = NULL; +struct db_context *cluster_db_tmp_open( + TALLOC_CTX *mem_ctx, + struct loadparm_context *lp_ctx, + const char *dbbase, + int flags) +{ + return cluster_db_tmp_open_ret; +} + +NTSTATUS dbwrap_store_ret = NT_STATUS_OK; +NTSTATUS dbwrap_store(struct db_context *db, TDB_DATA key, + TDB_DATA data, int flags) +{ + return dbwrap_store_ret; +} +bool dbwrap_exists_ret = true; + +bool dbwrap_exists(struct db_context *db, TDB_DATA key) +{ + return dbwrap_exists_ret; +} + +NTSTATUS dbwrap_delete_ret = NT_STATUS_OK; +NTSTATUS dbwrap_delete(struct db_context *db, TDB_DATA key) +{ + return dbwrap_delete_ret; +} + +/* + * Set the globals used by the mocked functions to a known and consistent state + * + */ +static void init_mock_results(TALLOC_CTX *mem_ctx) +{ + ldb_transaction_start_ret = LDB_SUCCESS; + in_transaction = false; + + ldb_transaction_cancel_ret = LDB_SUCCESS; + transaction_cancelled = false; + + ldb_transaction_commit_ret = LDB_SUCCESS; + transaction_committed = false; + + dsdb_update_bad_pwd_count_ret = NT_STATUS_OK; + dsdb_update_bad_pwd_count_res = NULL; + + ldb_build_mod_req_ret = LDB_SUCCESS; + ldb_build_mod_req_res = NULL; + + ldb_request_add_control_ret = LDB_SUCCESS; + ldb_request_ret = LDB_SUCCESS; + ldb_wait_ret = LDB_SUCCESS; + + ldb_msg_new_fail = false; + + samdb_rodc_ret = LDB_SUCCESS; + samdb_rodc_res = false; + + ldb_get_opaque_ret = loadparm_init(mem_ctx); + + cluster_db_tmp_open_ret = talloc_zero(mem_ctx, struct db_context); + + dbwrap_store_ret = NT_STATUS_OK; + + dbwrap_exists_ret = true; + + dbwrap_delete_ret = NT_STATUS_OK; + +} + +/***************************************************************************** + * Unit test set up and tear down + *****************************************************************************/ +struct context { +}; + +static int setup(void **state) { + struct context *ctx = talloc_zero(NULL, struct context); + init_mock_results(ctx); + + *state = ctx; + return 0; +} + +static int teardown(void **state) { + struct context *ctx = *state; + TALLOC_FREE(ctx); + return 0; +} + +/****************************************************************************** + * Helper functions + ******************************************************************************/ + +/* + * Build the "Original" user details record, i.e. the user being + * authenticated + */ +static struct ldb_message *create_message(TALLOC_CTX *ctx) +{ + + int ret; + struct timeval tv_now = timeval_current(); + NTTIME now = timeval_to_nttime(&tv_now); + + struct ldb_message *msg = ldb_msg_new(ctx); + + assert_non_null(msg); + ret = samdb_msg_add_int(ctx, msg, msg, "badPwdCount", 10); + assert_int_equal(LDB_SUCCESS, ret); + ret = __real_samdb_msg_add_int64(ctx, msg, msg, "badPasswordTime", now); + assert_int_equal(LDB_SUCCESS, ret); + ret = __real_samdb_msg_add_int64(ctx, msg, msg, "lockoutTime", now); + assert_int_equal(LDB_SUCCESS, ret); + return msg; +} + +/* + * Add a binary objectSID from string form to the supplied message + * + * + */ +static void add_sid( + struct ldb_message *msg, + const char *sid_str) +{ + struct ldb_val v; + enum ndr_err_code ndr_err; + struct dom_sid *sid = NULL; + + sid = talloc_zero(msg, struct dom_sid); + assert_non_null(sid); + assert_true(string_to_sid(sid, sid_str)); + ndr_err = ndr_push_struct_blob( + &v, msg, sid, (ndr_push_flags_fn_t)ndr_push_dom_sid); + assert_true(NDR_ERR_CODE_IS_SUCCESS(ndr_err)); + assert_int_equal(0, ldb_msg_add_value(msg, "objectSID", &v, NULL)); +} + +/* + * Build an ldb_result, for the re-reading of a user record + * + * if account_control < 0 then the msDS-User-Account-Control-Computed + * element is not included + * otherwise it is set to the value passed in account_control. + * + */ +static struct ldb_result *build_reread_result( + struct ldb_context *ldb, + TALLOC_CTX *ctx, + int account_control) +{ + struct ldb_message *msg = NULL; + int ret; + + struct ldb_result *res = talloc_zero(ctx, struct ldb_result); + + assert_non_null(res); + res->count = 1; + res->msgs = talloc_array(res, struct ldb_message *, 1); + + msg = create_message(res); + add_sid(msg, "S-1-5-21-2470180966-3899876309-2637894779-1000"); + if (account_control >= 0) { + ret = samdb_msg_add_int( + ldb, + msg, + msg, + "msDS-User-Account-Control-Computed", + account_control); + assert_int_equal(LDB_SUCCESS, ret); + } + + res->msgs[0] = msg; + return res; +} + +/* + * Build a mock domain pso ldb_result + */ +static struct ldb_result *build_domain_pso_result( + struct ldb_context *ldb, + TALLOC_CTX *ctx) +{ + struct ldb_message *msg = NULL; + struct ldb_result *res = talloc_zero(ctx, struct ldb_result); + + assert_non_null(res); + res->count = 1; + res->msgs = talloc_array(res, struct ldb_message *, 1); + assert_non_null(res->msgs); + msg = talloc_zero(res, struct ldb_message); + assert_non_null(msg); + res->msgs[0] = msg; + return res; +} + +/***************************************************************************** + * authsam_reread_user_logon_data unit tests + *****************************************************************************/ +/* + * authsam_reread_user_logon_data unable to re-read the user record. + * + */ +static void test_reread_read_failure(void **state) { + struct ldb_context *ldb = NULL; + struct ldb_message *msg = NULL; + struct ldb_message *cur = NULL; + TALLOC_CTX *ctx = NULL; + size_t before = 0; + size_t after = 0; + NTSTATUS status; + + ctx = talloc_new(*state); + assert_non_null(ctx); + + ldb = ldb_init(ctx, NULL); + assert_non_null(ldb); + + msg = create_message(ctx); + add_sid(msg, "S-1-5-21-2470180966-3899876309-2637894779-1000"); + + msg->dn = ldb_dn_new(ctx, ldb, "CN=User"); + assert_non_null(msg->dn); + + before = talloc_total_size(ctx); + + expect_check(__wrap_dsdb_search_dn, basedn, check_dn, msg->dn); + will_return(__wrap_dsdb_search_dn, NULL); + will_return(__wrap_dsdb_search_dn, LDB_ERR_NO_SUCH_OBJECT); + + status = authsam_reread_user_logon_data(ldb, ctx, msg, &cur); + assert_true(NT_STATUS_EQUAL(status, NT_STATUS_INTERNAL_ERROR)); + + /* + * Check that all allocated memory was freed + */ + after = talloc_total_size(ctx); + assert_int_equal(before, after); + + /* + * Clean up + */ + TALLOC_FREE(ctx); +} + +/* + * authsam_reread_user_logon_data account control flags missing from + * re-read data + * + */ +static void test_reread_missing_account_control(void **state) { + struct ldb_context *ldb = NULL; + struct ldb_message *msg = NULL; + struct ldb_message *cur = NULL; + TALLOC_CTX *ctx = NULL; + size_t before = 0; + size_t after = 0; + NTSTATUS status; + + ctx = talloc_new(*state); + assert_non_null(ctx); + + ldb = ldb_init(ctx, NULL); + assert_non_null(ldb); + + msg = create_message(ctx); + add_sid(msg, "S-1-5-21-2470180966-3899876309-2637894779-1000"); + + msg->dn = ldb_dn_new(ctx, ldb, "CN=User"); + assert_non_null(msg->dn); + + before = talloc_total_size(ctx); + + expect_check(__wrap_dsdb_search_dn, basedn, check_dn, msg->dn); + will_return(__wrap_dsdb_search_dn, build_reread_result(ldb, ctx, -1)); + will_return(__wrap_dsdb_search_dn, LDB_SUCCESS); + + status = authsam_reread_user_logon_data(ldb, ctx, msg, &cur); + assert_true(NT_STATUS_EQUAL(status, NT_STATUS_INTERNAL_ERROR)); + + /* + * Check that all allocated memory was freed + */ + after = talloc_total_size(ctx); + assert_int_equal(before, after); + + /* + * Clean up + */ + TALLOC_FREE(ctx); +} + +/* + * authsam_reread_user_logon_data account locked + * re-read data + * + */ +static void test_reread_account_locked(void **state) { + struct ldb_context *ldb = NULL; + struct ldb_message *msg = NULL; + struct ldb_message *cur = NULL; + TALLOC_CTX *ctx = NULL; + size_t before = 0; + size_t after = 0; + NTSTATUS status; + + ctx = talloc_new(*state); + assert_non_null(ctx); + + ldb = ldb_init(ctx, NULL); + assert_non_null(ldb); + + msg = create_message(ctx); + add_sid(msg, "S-1-5-21-2470180966-3899876309-2637894779-1000"); + + msg->dn = ldb_dn_new(ctx, ldb, "CN=User"); + assert_non_null(msg->dn); + + before = talloc_total_size(ctx); + + expect_check(__wrap_dsdb_search_dn, basedn, check_dn, msg->dn); + will_return(__wrap_dsdb_search_dn, build_reread_result(ldb, ctx, UF_LOCKOUT)); + will_return(__wrap_dsdb_search_dn, LDB_SUCCESS); + + status = authsam_reread_user_logon_data(ldb, ctx, msg, &cur); + assert_true(NT_STATUS_EQUAL(status, NT_STATUS_ACCOUNT_LOCKED_OUT)); + + /* + * Check that all allocated memory was freed + */ + after = talloc_total_size(ctx); + assert_int_equal(before, after); + + /* + * Clean up + */ + TALLOC_FREE(ctx); +} + +/* + * authsam_reread_user_logon_data account is not locked + * re-read data + * + */ +static void test_reread_account_not_locked(void **state) { + struct ldb_context *ldb = NULL; + struct ldb_message *msg = NULL; + struct ldb_message *cur = NULL; + TALLOC_CTX *ctx = NULL; + size_t before = 0; + size_t after = 0; + size_t result_size = 0; + NTSTATUS status; + struct ldb_result *res = NULL; + + ctx = talloc_new(*state); + assert_non_null(ctx); + + ldb = ldb_init(ctx, NULL); + assert_non_null(ldb); + + msg = create_message(ctx); + add_sid(msg, "S-1-5-21-2470180966-3899876309-2637894779-1000"); + + msg->dn = ldb_dn_new(ctx, ldb, "CN=User"); + assert_non_null(msg->dn); + + expect_check(__wrap_dsdb_search_dn, basedn, check_dn, msg->dn); + /* + * authsam_reread_user_logon_data returns the ldb_message portion + * of the ldb_result created by build_reread_result. + * So the tests for memory leaks will need to adjust for that + */ + res = build_reread_result(ldb, ctx, 0); + will_return(__wrap_dsdb_search_dn, res); + will_return(__wrap_dsdb_search_dn, LDB_SUCCESS); + + result_size = talloc_total_size(res) - + talloc_total_size(res->msgs[0]); + before = talloc_total_size(ctx) - result_size; + + status = authsam_reread_user_logon_data(ldb, ctx, msg, &cur); + assert_true(NT_STATUS_IS_OK(status)); + + /* + * Check that all allocated memory was freed + */ + after = talloc_total_size(ctx); + assert_int_equal(before, after); + + /* + * Clean up + */ + TALLOC_FREE(ctx); +} + + +/***************************************************************************** + * authsam_update_bad_pwd_count unit tests + *****************************************************************************/ + +/* + * authsam_update_bad_pwd_account + * + * Unable to read the domain_dn record + * + */ +static void test_update_bad_domain_dn_search_failed(void **state) { + struct ldb_context *ldb = NULL; + struct ldb_message *msg = NULL; + struct ldb_dn *domain_dn = NULL; + TALLOC_CTX *ctx = NULL; + size_t before = 0; + size_t after = 0; + NTSTATUS status; + + ctx = talloc_new(*state); + assert_non_null(ctx); + + ldb = ldb_init(ctx, NULL); + assert_non_null(ldb); + + domain_dn = ldb_dn_new(ctx, ldb, "CN=Domain"); + assert_non_null(domain_dn); + + msg = talloc_zero(ctx, struct ldb_message); + assert_non_null(msg); + + msg->dn = ldb_dn_new(ctx, ldb, "CN=User"); + assert_non_null(msg->dn); + + expect_check(__wrap_dsdb_search_dn, basedn, check_dn, domain_dn); + will_return(__wrap_dsdb_search_dn, NULL); + will_return(__wrap_dsdb_search_dn, LDB_ERR_NO_SUCH_OBJECT); + + before = talloc_total_size(ctx); + + status = authsam_update_bad_pwd_count(ldb, msg, domain_dn); + assert_true(NT_STATUS_EQUAL(status, NT_STATUS_INTERNAL_DB_CORRUPTION)); + + /* + * Check that all allocated memory was freed + */ + after = talloc_total_size(ctx); + assert_int_equal(before, after); + + /* + * Clean up + */ + TALLOC_FREE(ctx); +} + +/* + * authsam_update_bad_pwd_account + * + * authsam_get_user_pso failure + * + */ +static void test_update_bad_get_pso_failed(void **state) { + struct ldb_context *ldb = NULL; + struct ldb_message *msg = NULL; + struct ldb_dn *domain_dn = NULL; + struct ldb_dn *pso_dn = NULL; + const char *pso_dn_str = "CN=PSO"; + TALLOC_CTX *ctx = NULL; + size_t before = 0; + size_t after = 0; + NTSTATUS status; + int ret; + + ctx = talloc_new(*state); + assert_non_null(ctx); + + ldb = ldb_init(ctx, NULL); + assert_non_null(ldb); + + domain_dn = ldb_dn_new(ctx, ldb, "CN=Domain"); + assert_non_null(domain_dn); + + pso_dn = ldb_dn_new(ctx, ldb, pso_dn_str); + assert_non_null(pso_dn); + + msg = talloc_zero(ctx, struct ldb_message); + assert_non_null(msg); + ret = ldb_msg_add_string(msg, "msDS-ResultantPSO", pso_dn_str); + assert_int_equal(LDB_SUCCESS, ret); + + msg->dn = ldb_dn_new(ctx, ldb, "CN=User"); + assert_non_null(msg->dn); + + before = talloc_total_size(ctx); + + expect_check(__wrap_dsdb_search_dn, basedn, check_dn, domain_dn); + will_return(__wrap_dsdb_search_dn, build_domain_pso_result(ldb, ctx)); + will_return(__wrap_dsdb_search_dn, LDB_SUCCESS); + + expect_check(__wrap_dsdb_search_dn, basedn, check_dn, pso_dn); + will_return(__wrap_dsdb_search_dn, NULL); + will_return(__wrap_dsdb_search_dn, LDB_ERR_NO_SUCH_OBJECT); + + expect_check(__wrap_dsdb_search_dn, basedn, check_dn, msg->dn); + will_return(__wrap_dsdb_search_dn, build_reread_result(ldb, ctx, 0)); + will_return(__wrap_dsdb_search_dn, LDB_SUCCESS); + + status = authsam_update_bad_pwd_count(ldb, msg, domain_dn); + assert_true(NT_STATUS_IS_OK(status)); + + /* + * Check that all allocated memory was freed + */ + after = talloc_total_size(ctx); + assert_int_equal(before, after); + + /* + * Clean up + */ + TALLOC_FREE(ctx); +} + + +/* + * authsam_update_bad_pwd_account + * + * start_transaction failure + * + */ +static void test_update_bad_start_txn_failed(void **state) { + struct ldb_context *ldb = NULL; + struct ldb_message *msg = NULL; + struct ldb_dn *domain_dn = NULL; + TALLOC_CTX *ctx = NULL; + size_t before = 0; + size_t after = 0; + NTSTATUS status; + + ctx = talloc_new(*state); + assert_non_null(ctx); + + ldb = ldb_init(ctx, NULL); + assert_non_null(ldb); + + domain_dn = ldb_dn_new(ctx, ldb, "CN=Domain"); + assert_non_null(domain_dn); + + msg = talloc_zero(ctx, struct ldb_message); + assert_non_null(msg); + + before = talloc_total_size(ctx); + + expect_check(__wrap_dsdb_search_dn, basedn, check_dn, domain_dn); + will_return(__wrap_dsdb_search_dn, build_domain_pso_result(ldb, ctx)); + will_return(__wrap_dsdb_search_dn, LDB_SUCCESS); + + ldb_transaction_start_ret = LDB_ERR_OPERATIONS_ERROR; + + status = authsam_update_bad_pwd_count(ldb, msg, domain_dn); + assert_true(NT_STATUS_EQUAL(status, NT_STATUS_INTERNAL_ERROR)); + + /* + * Check that all allocated memory was freed + */ + after = talloc_total_size(ctx); + assert_int_equal(before, after); + + /* + * Clean up + */ + TALLOC_FREE(ctx); +} + +/* + * authsam_update_bad_pwd_account + * + * User details re-read failed + * + */ +static void test_update_bad_reread_failed(void **state) { + struct ldb_context *ldb = NULL; + struct ldb_message *msg = NULL; + struct ldb_dn *domain_dn = NULL; + TALLOC_CTX *ctx = NULL; + size_t before = 0; + size_t after = 0; + NTSTATUS status; + + ctx = talloc_new(*state); + assert_non_null(ctx); + + ldb = ldb_init(ctx, NULL); + assert_non_null(ldb); + + domain_dn = ldb_dn_new(ctx, ldb, "CN=Domain"); + assert_non_null(domain_dn); + + msg = talloc_zero(ctx, struct ldb_message); + assert_non_null(msg); + + msg->dn = ldb_dn_new(ctx, ldb, "CN=User"); + assert_non_null(msg->dn); + + before = talloc_total_size(ctx); + + expect_check(__wrap_dsdb_search_dn, basedn, check_dn, domain_dn); + will_return(__wrap_dsdb_search_dn, build_domain_pso_result(ldb, ctx)); + will_return(__wrap_dsdb_search_dn, LDB_SUCCESS); + + expect_check(__wrap_dsdb_search_dn, basedn, check_dn, msg->dn); + will_return(__wrap_dsdb_search_dn, NULL); + will_return(__wrap_dsdb_search_dn, LDB_ERR_NO_SUCH_OBJECT); + + status = authsam_update_bad_pwd_count(ldb, msg, domain_dn); + assert_true(NT_STATUS_EQUAL(status, NT_STATUS_INTERNAL_ERROR)); + assert_true(transaction_cancelled); + + /* + * Check that all allocated memory was freed + */ + after = talloc_total_size(ctx); + assert_int_equal(before, after); + + /* + * Clean up + */ + TALLOC_FREE(ctx); +} + +/* + * authsam_update_bad_pwd_account + * + * User details re-read reported locked out. + * + */ +static void test_update_bad_reread_locked_out(void **state) { + struct ldb_context *ldb = NULL; + struct ldb_message *msg = NULL; + struct ldb_dn *domain_dn = NULL; + TALLOC_CTX *ctx = NULL; + size_t before = 0; + size_t after = 0; + NTSTATUS status; + + ctx = talloc_new(*state); + assert_non_null(ctx); + + ldb = ldb_init(ctx, NULL); + assert_non_null(ldb); + + domain_dn = ldb_dn_new(ctx, ldb, "CN=Domain"); + assert_non_null(domain_dn); + + msg = create_message(ctx); + add_sid(msg, "S-1-5-21-2470180966-3899876309-2637894779-1000"); + + msg->dn = ldb_dn_new(ctx, ldb, "CN=User"); + assert_non_null(msg->dn); + + before = talloc_total_size(ctx); + + expect_check(__wrap_dsdb_search_dn, basedn, check_dn, domain_dn); + will_return(__wrap_dsdb_search_dn, build_domain_pso_result(ldb, ctx)); + will_return(__wrap_dsdb_search_dn, LDB_SUCCESS); + + expect_check(__wrap_dsdb_search_dn, basedn, check_dn, msg->dn); + will_return(__wrap_dsdb_search_dn, build_reread_result(ldb, ctx, UF_LOCKOUT)); + will_return(__wrap_dsdb_search_dn, LDB_SUCCESS); + + status = authsam_update_bad_pwd_count(ldb, msg, domain_dn); + assert_true(NT_STATUS_EQUAL(status, NT_STATUS_ACCOUNT_LOCKED_OUT)); + assert_false(transaction_cancelled); + assert_true(transaction_committed); + + /* + * Check that all allocated memory was freed + */ + after = talloc_total_size(ctx); + assert_int_equal(before, after); + + /* + * Clean up + */ + TALLOC_FREE(ctx); +} + +/* + * authsam_update_bad_pwd_account + * + * Transaction cancel failure + */ +static void test_update_bad_txn_cancel_failed(void **state) { + struct ldb_context *ldb = NULL; + struct ldb_message *msg = NULL; + struct ldb_dn *domain_dn = NULL; + TALLOC_CTX *ctx = NULL; + size_t before = 0; + size_t after = 0; + NTSTATUS status; + + ctx = talloc_new(*state); + assert_non_null(ctx); + + ldb = ldb_init(ctx, NULL); + assert_non_null(ldb); + + domain_dn = ldb_dn_new(ctx, ldb, "CN=Domain"); + assert_non_null(domain_dn); + + msg = talloc_zero(ctx, struct ldb_message); + assert_non_null(msg); + + msg->dn = ldb_dn_new(ctx, ldb, "CN=User"); + assert_non_null(msg->dn); + + before = talloc_total_size(ctx); + + expect_check(__wrap_dsdb_search_dn, basedn, check_dn, domain_dn); + will_return(__wrap_dsdb_search_dn, build_domain_pso_result(ldb, ctx)); + will_return(__wrap_dsdb_search_dn, LDB_SUCCESS); + + expect_check(__wrap_dsdb_search_dn, basedn, check_dn, msg->dn); + will_return(__wrap_dsdb_search_dn, NULL); + will_return(__wrap_dsdb_search_dn, LDB_ERR_NO_SUCH_OBJECT); + + ldb_transaction_cancel_ret = LDB_ERR_OPERATIONS_ERROR; + + status = authsam_update_bad_pwd_count(ldb, msg, domain_dn); + assert_true(NT_STATUS_EQUAL(status, NT_STATUS_INTERNAL_ERROR)); + assert_true(in_transaction); + assert_false(transaction_cancelled); + assert_false(transaction_committed); + + /* + * Check that all allocated memory was freed + */ + after = talloc_total_size(ctx); + assert_int_equal(before, after); + + /* + * Clean up + */ + TALLOC_FREE(ctx); +} + +/* + * The following tests all expect the same setup, that is a normal + * good user object and empty domain object. + * + * returns the talloc size after result array setup for leak tests + */ +static size_t setup_bad_password_search_results(TALLOC_CTX *ctx, + struct ldb_context *ldb, + struct ldb_dn *domain_dn, + struct ldb_dn *user_dn) +{ + size_t before = 0; + + before = talloc_total_size(ctx); + + expect_check(__wrap_dsdb_search_dn, basedn, check_dn, domain_dn); + will_return(__wrap_dsdb_search_dn, build_domain_pso_result(ldb, ctx)); + will_return(__wrap_dsdb_search_dn, LDB_SUCCESS); + + expect_check(__wrap_dsdb_search_dn, basedn, check_dn, user_dn); + will_return(__wrap_dsdb_search_dn, build_reread_result(ldb, ctx, 0)); + will_return(__wrap_dsdb_search_dn, LDB_SUCCESS); + + return before; +} + + +/* + * authsam_update_bad_pwd_account + * + * dsdb_update_bad_pwd_count failure + * + */ +static void test_update_bad_update_count_failed(void **state) { + struct ldb_context *ldb = NULL; + struct ldb_message *msg = NULL; + struct ldb_dn *domain_dn = NULL; + TALLOC_CTX *ctx = NULL; + size_t before = 0; + size_t after = 0; + NTSTATUS status; + + ctx = talloc_new(*state); + assert_non_null(ctx); + + ldb = ldb_init(ctx, NULL); + assert_non_null(ldb); + + domain_dn = ldb_dn_new(ctx, ldb, "CN=Domain"); + assert_non_null(domain_dn); + + msg = create_message(ctx); + add_sid(msg, "S-1-5-21-2470180966-3899876309-2637894779-1000"); + + msg->dn = ldb_dn_new(ctx, ldb, "CN=User"); + assert_non_null(msg->dn); + + before = setup_bad_password_search_results(ctx, ldb, + domain_dn, + msg->dn); + + dsdb_update_bad_pwd_count_ret = NT_STATUS_INTERNAL_ERROR; + + status = authsam_update_bad_pwd_count(ldb, msg, domain_dn); + assert_true(NT_STATUS_EQUAL(status, NT_STATUS_INTERNAL_ERROR)); + assert_true(transaction_cancelled); + + /* + * Check that all allocated memory was freed + */ + after = talloc_total_size(ctx); + assert_int_equal(before, after); + + /* + * Clean up + */ + TALLOC_FREE(ctx); +} + +/* + * authsam_update_bad_pwd_account + * + * No need to update the bad password stats + * + */ +static void test_update_bad_no_update_required(void **state) { + struct ldb_context *ldb = NULL; + struct ldb_message *msg = NULL; + struct ldb_dn *domain_dn = NULL; + TALLOC_CTX *ctx = NULL; + size_t before = 0; + size_t after = 0; + NTSTATUS status; + + ctx = talloc_new(*state); + assert_non_null(ctx); + + ldb = ldb_init(ctx, NULL); + assert_non_null(ldb); + + domain_dn = ldb_dn_new(ctx, ldb, "CN=Domain"); + assert_non_null(domain_dn); + + msg = create_message(ctx); + add_sid(msg, "S-1-5-21-2470180966-3899876309-2637894779-1000"); + + msg->dn = ldb_dn_new(ctx, ldb, "CN=User"); + assert_non_null(msg->dn); + + before = setup_bad_password_search_results(ctx, ldb, + domain_dn, + msg->dn); + + status = authsam_update_bad_pwd_count(ldb, msg, domain_dn); + assert_true(NT_STATUS_IS_OK(status)); + assert_true(transaction_committed); + + /* + * Check that all allocated memory was freed + */ + after = talloc_total_size(ctx); + assert_int_equal(before, after); + + /* + * Clean up + */ + TALLOC_FREE(ctx); +} + +/* + * authsam_update_bad_pwd_account + * + * Transaction commit failure + * + */ +static void test_update_bad_commit_failed(void **state) { + struct ldb_context *ldb = NULL; + struct ldb_message *msg = NULL; + struct ldb_dn *domain_dn = NULL; + TALLOC_CTX *ctx = NULL; + size_t before = 0; + size_t after = 0; + NTSTATUS status; + + ctx = talloc_new(*state); + assert_non_null(ctx); + + ldb = ldb_init(ctx, NULL); + assert_non_null(ldb); + + domain_dn = ldb_dn_new(ctx, ldb, "CN=Domain"); + assert_non_null(domain_dn); + + msg = create_message(ctx); + add_sid(msg, "S-1-5-21-2470180966-3899876309-2637894779-1000"); + + msg->dn = ldb_dn_new(ctx, ldb, "CN=User"); + assert_non_null(msg->dn); + + before = setup_bad_password_search_results(ctx, ldb, + domain_dn, + msg->dn); + + ldb_transaction_commit_ret = LDB_ERR_OPERATIONS_ERROR; + + status = authsam_update_bad_pwd_count(ldb, msg, domain_dn); + assert_true(NT_STATUS_EQUAL(status, NT_STATUS_INTERNAL_ERROR)); + assert_true(in_transaction); + assert_false(transaction_cancelled); + assert_false(transaction_committed); + + /* + * Check that all allocated memory was freed + */ + after = talloc_total_size(ctx); + assert_int_equal(before, after); + + /* + * Clean up + */ + TALLOC_FREE(ctx); +} + +/* + * authsam_update_bad_pwd_account + * + * ldb_build_mod_req failed building the user update details + * + */ +static void test_update_bad_build_mod_request_failed(void **state) { + struct ldb_context *ldb = NULL; + struct ldb_message *msg = NULL; + struct ldb_dn *domain_dn = NULL; + TALLOC_CTX *ctx = NULL; + size_t before = 0; + size_t after = 0; + NTSTATUS status; + + ctx = talloc_new(*state); + assert_non_null(ctx); + + ldb = ldb_init(ctx, NULL); + assert_non_null(ldb); + + domain_dn = ldb_dn_new(ctx, ldb, "CN=Domain"); + assert_non_null(domain_dn); + + msg = create_message(ctx); + add_sid(msg, "S-1-5-21-2470180966-3899876309-2637894779-1000"); + + msg->dn = ldb_dn_new(ctx, ldb, "CN=User"); + assert_non_null(msg->dn); + + before = setup_bad_password_search_results(ctx, ldb, + domain_dn, + msg->dn); + + dsdb_update_bad_pwd_count_res = talloc_zero(ctx, struct ldb_message); + ldb_build_mod_req_ret = LDB_ERR_OPERATIONS_ERROR; + + status = authsam_update_bad_pwd_count(ldb, msg, domain_dn); + assert_true(NT_STATUS_EQUAL(status, NT_STATUS_INTERNAL_ERROR)); + assert_true(transaction_cancelled); + + /* + * Check that all allocated memory was freed + */ + after = talloc_total_size(ctx); + assert_int_equal(before, after); + + /* + * Clean up + */ + TALLOC_FREE(ctx); +} + +/* + * authsam_update_bad_pwd_account + * + * ldb_request_add_control failed to add DSDB_CONTROL_FORCE_RODC_LOCAL_CHANGE + * to the user update record. + * + */ +static void test_update_bad_add_control_failed(void **state) { + struct ldb_context *ldb = NULL; + struct ldb_message *msg = NULL; + struct ldb_dn *domain_dn = NULL; + TALLOC_CTX *ctx = NULL; + size_t before = 0; + size_t after = 0; + NTSTATUS status; + + ctx = talloc_new(*state); + assert_non_null(ctx); + + ldb = ldb_init(ctx, NULL); + assert_non_null(ldb); + + domain_dn = ldb_dn_new(ctx, ldb, "CN=Domain"); + assert_non_null(domain_dn); + + msg = create_message(ctx); + add_sid(msg, "S-1-5-21-2470180966-3899876309-2637894779-1000"); + + msg->dn = ldb_dn_new(ctx, ldb, "CN=User"); + assert_non_null(msg->dn); + + before = setup_bad_password_search_results(ctx, ldb, + domain_dn, + msg->dn); + + dsdb_update_bad_pwd_count_res = talloc_zero(ctx, struct ldb_message); + ldb_build_mod_req_res = talloc_zero(ctx, struct ldb_request); + ldb_request_add_control_ret = LDB_ERR_OPERATIONS_ERROR; + + status = authsam_update_bad_pwd_count(ldb, msg, domain_dn); + assert_true(NT_STATUS_EQUAL(status, NT_STATUS_INTERNAL_ERROR)); + assert_true(transaction_cancelled); + + /* + * Check that all allocated memory was freed + */ + after = talloc_total_size(ctx); + assert_int_equal(before, after); + + /* + * Clean up + */ + TALLOC_FREE(ctx); +} + +/* + * authsam_update_bad_pwd_account + * + * call to ldb_request failed + * + */ +static void test_update_bad_ldb_request_failed(void **state) { + struct ldb_context *ldb = NULL; + struct ldb_message *msg = NULL; + struct ldb_dn *domain_dn = NULL; + TALLOC_CTX *ctx = NULL; + size_t before = 0; + size_t after = 0; + NTSTATUS status; + + ctx = talloc_new(*state); + assert_non_null(ctx); + + ldb = ldb_init(ctx, NULL); + assert_non_null(ldb); + + domain_dn = ldb_dn_new(ctx, ldb, "CN=Domain"); + assert_non_null(domain_dn); + + msg = create_message(ctx); + add_sid(msg, "S-1-5-21-2470180966-3899876309-2637894779-1000"); + + msg->dn = ldb_dn_new(ctx, ldb, "CN=User"); + assert_non_null(msg->dn); + + before = setup_bad_password_search_results(ctx, ldb, + domain_dn, + msg->dn); + + dsdb_update_bad_pwd_count_res = talloc_zero(ctx, struct ldb_message); + ldb_build_mod_req_res = talloc_zero(ctx, struct ldb_request); + ldb_request_ret = LDB_ERR_OPERATIONS_ERROR; + + status = authsam_update_bad_pwd_count(ldb, msg, domain_dn); + assert_true(NT_STATUS_EQUAL(status, NT_STATUS_INTERNAL_ERROR)); + assert_true(transaction_cancelled); + + /* + * Check that all allocated memory was freed + */ + after = talloc_total_size(ctx); + assert_int_equal(before, after); + + /* + * Clean up + */ + TALLOC_FREE(ctx); +} + +/* + * authsam_update_bad_pwd_account + * + * call to ldb_wait failed + * + */ +static void test_update_bad_ldb_wait_failed(void **state) { + struct ldb_context *ldb = NULL; + struct ldb_message *msg = NULL; + struct ldb_dn *domain_dn = NULL; + TALLOC_CTX *ctx = NULL; + size_t before = 0; + size_t after = 0; + NTSTATUS status; + + ctx = talloc_new(*state); + assert_non_null(ctx); + + ldb = ldb_init(ctx, NULL); + assert_non_null(ldb); + + domain_dn = ldb_dn_new(ctx, ldb, "CN=Domain"); + assert_non_null(domain_dn); + + msg = create_message(ctx); + add_sid(msg, "S-1-5-21-2470180966-3899876309-2637894779-1000"); + + msg->dn = ldb_dn_new(ctx, ldb, "CN=User"); + assert_non_null(msg->dn); + + before = setup_bad_password_search_results(ctx, ldb, + domain_dn, + msg->dn); + + dsdb_update_bad_pwd_count_res = talloc_zero(ctx, struct ldb_message); + ldb_build_mod_req_res = talloc_zero(ctx, struct ldb_request); + ldb_wait_ret = LDB_ERR_OPERATIONS_ERROR; + + status = authsam_update_bad_pwd_count(ldb, msg, domain_dn); + assert_true(NT_STATUS_EQUAL(status, NT_STATUS_INTERNAL_ERROR)); + assert_true(transaction_cancelled); + + /* + * Check that all allocated memory was freed + */ + after = talloc_total_size(ctx); + assert_int_equal(before, after); + + /* + * Clean up + */ + TALLOC_FREE(ctx); +} + +/***************************************************************************** + * authsam_logon_success_accounting unit tests + *****************************************************************************/ +/* + * authsam_logon_success_accounting + * + * start_transaction failure + * + */ +static void test_success_accounting_start_txn_failed(void **state) { + struct ldb_context *ldb = NULL; + struct ldb_message *msg = NULL; + struct ldb_dn *domain_dn = NULL; + TALLOC_CTX *ctx = NULL; + size_t before = 0; + size_t after = 0; + NTSTATUS status; + + ctx = talloc_new(*state); + assert_non_null(ctx); + + ldb = ldb_init(ctx, NULL); + assert_non_null(ldb); + + domain_dn = ldb_dn_new(ctx, ldb, "CN=Domain"); + assert_non_null(domain_dn); + + msg = create_message(ctx); + add_sid(msg, "S-1-5-21-2470180966-3899876309-2637894779-1000"); + + msg->dn = ldb_dn_new(ctx, ldb, "CN=User"); + assert_non_null(msg->dn); + + before = talloc_total_size(ctx); + + expect_check(__wrap_dsdb_search_dn, basedn, check_dn, domain_dn); + will_return(__wrap_dsdb_search_dn, build_domain_pso_result(ldb, ctx)); + will_return(__wrap_dsdb_search_dn, LDB_SUCCESS); + + ldb_transaction_start_ret = LDB_ERR_OPERATIONS_ERROR; + + status = authsam_logon_success_accounting( + ldb, msg, domain_dn, true, NULL); + assert_true(NT_STATUS_EQUAL(status, NT_STATUS_INTERNAL_ERROR)); + + /* + * Check that all allocated memory was freed + */ + after = talloc_total_size(ctx); + assert_int_equal(before, after); + + /* + * Clean up + */ + TALLOC_FREE(ctx); +} + +/* + * authsam_logon_success_accounting + * + * User details re-read failed + * + */ +static void test_success_accounting_reread_failed(void **state) { + struct ldb_context *ldb = NULL; + struct ldb_message *msg = NULL; + struct ldb_dn *domain_dn = NULL; + TALLOC_CTX *ctx = NULL; + size_t before = 0; + size_t after = 0; + NTSTATUS status; + + ctx = talloc_new(*state); + assert_non_null(ctx); + + ldb = ldb_init(ctx, NULL); + assert_non_null(ldb); + + domain_dn = ldb_dn_new(ctx, ldb, "CN=Domain"); + assert_non_null(domain_dn); + + msg = create_message(ctx); + add_sid(msg, "S-1-5-21-2470180966-3899876309-2637894779-1000"); + + msg->dn = ldb_dn_new(ctx, ldb, "CN=User"); + assert_non_null(msg->dn); + + before = talloc_total_size(ctx); + + expect_check(__wrap_dsdb_search_dn, basedn, check_dn, domain_dn); + will_return(__wrap_dsdb_search_dn, build_domain_pso_result(ldb, ctx)); + will_return(__wrap_dsdb_search_dn, LDB_SUCCESS); + + expect_check(__wrap_dsdb_search_dn, basedn, check_dn, msg->dn); + will_return(__wrap_dsdb_search_dn, NULL); + will_return(__wrap_dsdb_search_dn, LDB_ERR_NO_SUCH_OBJECT); + + status = authsam_logon_success_accounting( + ldb, msg, domain_dn, true, NULL); + assert_true(NT_STATUS_EQUAL(status, NT_STATUS_INTERNAL_ERROR)); + assert_true(transaction_cancelled); + + /* + * Check that all allocated memory was freed + */ + after = talloc_total_size(ctx); + assert_int_equal(before, after); + + /* + * Clean up + */ + TALLOC_FREE(ctx); +} + +/* + * authsam_logon_success_accounting + * + * ldb_msg_new failed + * + */ +static void test_success_accounting_ldb_msg_new_failed(void **state) { + struct ldb_context *ldb = NULL; + struct ldb_message *msg = NULL; + struct ldb_dn *domain_dn = NULL; + TALLOC_CTX *ctx = NULL; + size_t before = 0; + size_t after = 0; + NTSTATUS status; + + ctx = talloc_new(*state); + assert_non_null(ctx); + + ldb = ldb_init(ctx, NULL); + assert_non_null(ldb); + + domain_dn = ldb_dn_new(ctx, ldb, "CN=Domain"); + assert_non_null(domain_dn); + + msg = create_message(ctx); + add_sid(msg, "S-1-5-21-2470180966-3899876309-2637894779-1000"); + + msg->dn = ldb_dn_new(ctx, ldb, "CN=User"); + assert_non_null(msg->dn); + + before = talloc_total_size(ctx); + + expect_check(__wrap_dsdb_search_dn, basedn, check_dn, domain_dn); + will_return(__wrap_dsdb_search_dn, build_domain_pso_result(ldb, ctx)); + will_return(__wrap_dsdb_search_dn, LDB_SUCCESS); + + expect_check(__wrap_dsdb_search_dn, basedn, check_dn, msg->dn); + will_return(__wrap_dsdb_search_dn, build_reread_result(ldb, ctx, 0)); + will_return(__wrap_dsdb_search_dn, LDB_SUCCESS); + + ldb_msg_new_fail = true; + + status = authsam_logon_success_accounting( + ldb, msg, domain_dn, true, NULL); + assert_true(NT_STATUS_EQUAL(status, NT_STATUS_NO_MEMORY)); + assert_true(transaction_cancelled); + + /* + * Check that all allocated memory was freed + */ + after = talloc_total_size(ctx); + assert_int_equal(before, after); + + /* + * Clean up + */ + TALLOC_FREE(ctx); +} + +/* + * authsam_logon_success_accounting + * + * samdb_rodc failed + * + */ +static void test_success_accounting_samdb_rodc_failed(void **state) { + struct ldb_context *ldb = NULL; + struct ldb_message *msg = NULL; + struct ldb_dn *domain_dn = NULL; + TALLOC_CTX *ctx = NULL; + size_t before = 0; + size_t after = 0; + NTSTATUS status; + + ctx = talloc_new(*state); + assert_non_null(ctx); + + ldb = ldb_init(ctx, NULL); + assert_non_null(ldb); + + domain_dn = ldb_dn_new(ctx, ldb, "CN=Domain"); + assert_non_null(domain_dn); + + msg = create_message(ctx); + add_sid(msg, "S-1-5-21-2470180966-3899876309-2637894779-1000"); + + msg->dn = ldb_dn_new(ctx, ldb, "CN=User"); + assert_non_null(msg->dn); + + before = talloc_total_size(ctx); + + samdb_rodc_ret = LDB_ERR_OPERATIONS_ERROR; + + status = authsam_logon_success_accounting( + ldb, msg, domain_dn, true, NULL); + assert_true(NT_STATUS_EQUAL(status, NT_STATUS_INTERNAL_ERROR)); + assert_false(in_transaction); + assert_false(transaction_cancelled); + assert_false(transaction_committed); + + /* + * Check that all allocated memory was freed + */ + after = talloc_total_size(ctx); + assert_int_equal(before, after); + + /* + * Clean up + */ + TALLOC_FREE(ctx); +} + +/* + * authsam_logon_success_accounting + * + * authsam_update_lastlogon_timestamp failed + * + */ +static void test_success_accounting_update_lastlogon_failed(void **state) { + struct ldb_context *ldb = NULL; + struct ldb_message *msg = NULL; + struct ldb_dn *domain_dn = NULL; + TALLOC_CTX *ctx = NULL; + size_t before = 0; + size_t after = 0; + NTSTATUS status; + + ctx = talloc_new(*state); + assert_non_null(ctx); + + ldb = ldb_init(ctx, NULL); + assert_non_null(ldb); + + domain_dn = ldb_dn_new(ctx, ldb, "CN=Domain"); + assert_non_null(domain_dn); + + msg = create_message(ctx); + add_sid(msg, "S-1-5-21-2470180966-3899876309-2637894779-1000"); + + msg->dn = ldb_dn_new(ctx, ldb, "CN=User"); + assert_non_null(msg->dn); + + ldb_build_mod_req_res = talloc_zero(ctx, struct ldb_request); + + before = talloc_total_size(ctx); + + expect_check(__wrap_dsdb_search_dn, basedn, check_dn, domain_dn); + will_return(__wrap_dsdb_search_dn, build_domain_pso_result(ldb, ctx)); + will_return(__wrap_dsdb_search_dn, LDB_SUCCESS); + + expect_check(__wrap_dsdb_search_dn, basedn, check_dn, msg->dn); + will_return(__wrap_dsdb_search_dn, build_reread_result(ldb, ctx, 0)); + will_return(__wrap_dsdb_search_dn, LDB_SUCCESS); + + will_return(__wrap_samdb_msg_add_int64, LDB_ERR_OPERATIONS_ERROR); + + status = authsam_logon_success_accounting( + ldb, msg, domain_dn, true, NULL); + assert_true(NT_STATUS_EQUAL(status, NT_STATUS_NO_MEMORY)); + assert_true(transaction_cancelled); + + /* + * Check that all allocated memory was freed + */ + after = talloc_total_size(ctx); + assert_int_equal(before, after); + + /* + * Clean up + */ + TALLOC_FREE(ctx); +} + +/* + * authsam_logon_success_accounting + * + * ldb_build_mod_req failed + * + */ +static void test_success_accounting_build_mod_req_failed(void **state) { + struct ldb_context *ldb = NULL; + struct ldb_message *msg = NULL; + struct ldb_dn *domain_dn = NULL; + TALLOC_CTX *ctx = NULL; + size_t before = 0; + size_t after = 0; + NTSTATUS status; + + ctx = talloc_new(*state); + assert_non_null(ctx); + + ldb = ldb_init(ctx, NULL); + assert_non_null(ldb); + + domain_dn = ldb_dn_new(ctx, ldb, "CN=Domain"); + assert_non_null(domain_dn); + + msg = create_message(ctx); + add_sid(msg, "S-1-5-21-2470180966-3899876309-2637894779-1000"); + + msg->dn = ldb_dn_new(ctx, ldb, "CN=User"); + assert_non_null(msg->dn); + + before = talloc_total_size(ctx); + + expect_check(__wrap_dsdb_search_dn, basedn, check_dn, domain_dn); + will_return(__wrap_dsdb_search_dn, build_domain_pso_result(ldb, ctx)); + will_return(__wrap_dsdb_search_dn, LDB_SUCCESS); + + expect_check(__wrap_dsdb_search_dn, basedn, check_dn, msg->dn); + will_return(__wrap_dsdb_search_dn, build_reread_result(ldb, ctx, 0)); + will_return(__wrap_dsdb_search_dn, LDB_SUCCESS); + + ldb_build_mod_req_ret = LDB_ERR_OPERATIONS_ERROR; + + will_return(__wrap_samdb_msg_add_int64, LDB_SUCCESS); + will_return(__wrap_samdb_msg_add_int64, LDB_SUCCESS); + + status = authsam_logon_success_accounting( + ldb, msg, domain_dn, true, NULL); + assert_true(NT_STATUS_EQUAL(status, NT_STATUS_INTERNAL_ERROR)); + assert_true(transaction_cancelled); + + /* + * Check that all allocated memory was freed + */ + after = talloc_total_size(ctx); + assert_int_equal(before, after); + + /* + * Clean up + */ + TALLOC_FREE(ctx); +} + +/* + * authsam_logon_success_accounting + * + * ldb_request_add_control failed + * + */ +static void test_success_accounting_add_control_failed(void **state) { + struct ldb_context *ldb = NULL; + struct ldb_message *msg = NULL; + struct ldb_dn *domain_dn = NULL; + TALLOC_CTX *ctx = NULL; + size_t before = 0; + size_t after = 0; + NTSTATUS status; + + ctx = talloc_new(*state); + assert_non_null(ctx); + + ldb = ldb_init(ctx, NULL); + assert_non_null(ldb); + + domain_dn = ldb_dn_new(ctx, ldb, "CN=Domain"); + assert_non_null(domain_dn); + + msg = create_message(ctx); + add_sid(msg, "S-1-5-21-2470180966-3899876309-2637894779-1000"); + + msg->dn = ldb_dn_new(ctx, ldb, "CN=User"); + assert_non_null(msg->dn); + + before = talloc_total_size(ctx); + + expect_check(__wrap_dsdb_search_dn, basedn, check_dn, domain_dn); + will_return(__wrap_dsdb_search_dn, build_domain_pso_result(ldb, ctx)); + will_return(__wrap_dsdb_search_dn, LDB_SUCCESS); + + expect_check(__wrap_dsdb_search_dn, basedn, check_dn, msg->dn); + will_return(__wrap_dsdb_search_dn, build_reread_result(ldb, ctx, 0)); + will_return(__wrap_dsdb_search_dn, LDB_SUCCESS); + + ldb_build_mod_req_res = talloc_zero(ldb, struct ldb_request); + ldb_request_add_control_ret = LDB_ERR_OPERATIONS_ERROR; + + will_return(__wrap_samdb_msg_add_int64, LDB_SUCCESS); + will_return(__wrap_samdb_msg_add_int64, LDB_SUCCESS); + + status = authsam_logon_success_accounting( + ldb, msg, domain_dn, true, NULL); + assert_true(NT_STATUS_EQUAL(status, NT_STATUS_INTERNAL_ERROR)); + assert_true(transaction_cancelled); + + /* + * Check that all allocated memory was freed + */ + after = talloc_total_size(ctx); + assert_int_equal(before, after); + + /* + * Clean up + */ + TALLOC_FREE(ctx); +} + +/* + * authsam_logon_success_accounting + * + * ldb_request failed + * + */ +static void test_success_accounting_ldb_request_failed(void **state) { + struct ldb_context *ldb = NULL; + struct ldb_message *msg = NULL; + struct ldb_dn *domain_dn = NULL; + TALLOC_CTX *ctx = NULL; + size_t before = 0; + size_t after = 0; + NTSTATUS status; + + ctx = talloc_new(*state); + assert_non_null(ctx); + + ldb = ldb_init(ctx, NULL); + assert_non_null(ldb); + + domain_dn = ldb_dn_new(ctx, ldb, "CN=Domain"); + assert_non_null(domain_dn); + + msg = create_message(ctx); + add_sid(msg, "S-1-5-21-2470180966-3899876309-2637894779-1000"); + + msg->dn = ldb_dn_new(ctx, ldb, "CN=User"); + assert_non_null(msg->dn); + + before = talloc_total_size(ctx); + + expect_check(__wrap_dsdb_search_dn, basedn, check_dn, domain_dn); + will_return(__wrap_dsdb_search_dn, build_domain_pso_result(ldb, ctx)); + will_return(__wrap_dsdb_search_dn, LDB_SUCCESS); + + expect_check(__wrap_dsdb_search_dn, basedn, check_dn, msg->dn); + will_return(__wrap_dsdb_search_dn, build_reread_result(ldb, ctx, 0)); + will_return(__wrap_dsdb_search_dn, LDB_SUCCESS); + + ldb_build_mod_req_res = talloc_zero(ldb, struct ldb_request); + ldb_request_ret = LDB_ERR_OPERATIONS_ERROR; + + will_return(__wrap_samdb_msg_add_int64, LDB_SUCCESS); + will_return(__wrap_samdb_msg_add_int64, LDB_SUCCESS); + + status = authsam_logon_success_accounting( + ldb, msg, domain_dn, true, NULL); + assert_true(NT_STATUS_EQUAL(status, NT_STATUS_INTERNAL_ERROR)); + assert_true(transaction_cancelled); + + /* + * Check that all allocated memory was freed + */ + after = talloc_total_size(ctx); + assert_int_equal(before, after); + + /* + * Clean up + */ + TALLOC_FREE(ctx); +} + +/* + * authsam_logon_success_accounting + * + * ldb_wait failed + * + */ +static void test_success_accounting_ldb_wait_failed(void **state) { + struct ldb_context *ldb = NULL; + struct ldb_message *msg = NULL; + struct ldb_dn *domain_dn = NULL; + TALLOC_CTX *ctx = NULL; + size_t before = 0; + size_t after = 0; + NTSTATUS status; + + ctx = talloc_new(*state); + assert_non_null(ctx); + + ldb = ldb_init(ctx, NULL); + assert_non_null(ldb); + + domain_dn = ldb_dn_new(ctx, ldb, "CN=Domain"); + assert_non_null(domain_dn); + + msg = create_message(ctx); + add_sid(msg, "S-1-5-21-2470180966-3899876309-2637894779-1000"); + + msg->dn = ldb_dn_new(ctx, ldb, "CN=User"); + assert_non_null(msg->dn); + + before = talloc_total_size(ctx); + + expect_check(__wrap_dsdb_search_dn, basedn, check_dn, domain_dn); + will_return(__wrap_dsdb_search_dn, build_domain_pso_result(ldb, ctx)); + will_return(__wrap_dsdb_search_dn, LDB_SUCCESS); + + expect_check(__wrap_dsdb_search_dn, basedn, check_dn, msg->dn); + will_return(__wrap_dsdb_search_dn, build_reread_result(ldb, ctx, 0)); + will_return(__wrap_dsdb_search_dn, LDB_SUCCESS); + + ldb_build_mod_req_res = talloc_zero(ldb, struct ldb_request); + ldb_wait_ret = LDB_ERR_OPERATIONS_ERROR; + + will_return(__wrap_samdb_msg_add_int64, LDB_SUCCESS); + will_return(__wrap_samdb_msg_add_int64, LDB_SUCCESS); + + status = authsam_logon_success_accounting( + ldb, msg, domain_dn, true, NULL); + assert_true(NT_STATUS_EQUAL(status, NT_STATUS_INTERNAL_ERROR)); + assert_true(transaction_cancelled); + + /* + * Check that all allocated memory was freed + */ + after = talloc_total_size(ctx); + assert_int_equal(before, after); + + /* + * Clean up + */ + TALLOC_FREE(ctx); +} + +/* + * authsam_logon_success_accounting + * + * ldb_transaction_commit failed + * + */ +static void test_success_accounting_commit_failed(void **state) { + struct ldb_context *ldb = NULL; + struct ldb_message *msg = NULL; + struct ldb_dn *domain_dn = NULL; + TALLOC_CTX *ctx = NULL; + size_t before = 0; + size_t after = 0; + NTSTATUS status; + + ctx = talloc_new(*state); + assert_non_null(ctx); + + ldb = ldb_init(ctx, NULL); + assert_non_null(ldb); + + domain_dn = ldb_dn_new(ctx, ldb, "CN=Domain"); + assert_non_null(domain_dn); + + msg = create_message(ctx); + add_sid(msg, "S-1-5-21-2470180966-3899876309-2637894779-1000"); + + msg->dn = ldb_dn_new(ctx, ldb, "CN=User"); + assert_non_null(msg->dn); + + before = talloc_total_size(ctx); + + expect_check(__wrap_dsdb_search_dn, basedn, check_dn, domain_dn); + will_return(__wrap_dsdb_search_dn, build_domain_pso_result(ldb, ctx)); + will_return(__wrap_dsdb_search_dn, LDB_SUCCESS); + + expect_check(__wrap_dsdb_search_dn, basedn, check_dn, msg->dn); + will_return(__wrap_dsdb_search_dn, build_reread_result(ldb, ctx, 0)); + will_return(__wrap_dsdb_search_dn, LDB_SUCCESS); + + ldb_build_mod_req_res = talloc_zero(ldb, struct ldb_request); + ldb_transaction_commit_ret = LDB_ERR_OPERATIONS_ERROR; + + will_return(__wrap_samdb_msg_add_int64, LDB_SUCCESS); + will_return(__wrap_samdb_msg_add_int64, LDB_SUCCESS); + + status = authsam_logon_success_accounting( + ldb, msg, domain_dn, true, NULL); + assert_true(NT_STATUS_EQUAL(status, NT_STATUS_INTERNAL_ERROR)); + assert_true(in_transaction); + assert_false(transaction_cancelled); + assert_false(transaction_committed); + + /* + * Check that all allocated memory was freed + */ + after = talloc_total_size(ctx); + assert_int_equal(before, after); + + /* + * Clean up + */ + TALLOC_FREE(ctx); +} + +/* + * authsam_logon_success_accounting + * + * ldb_wait failed and then ldb_transaction_cancel failed + * + */ +static void test_success_accounting_rollback_failed(void **state) { + struct ldb_context *ldb = NULL; + struct ldb_message *msg = NULL; + struct ldb_dn *domain_dn = NULL; + TALLOC_CTX *ctx = NULL; + size_t before = 0; + size_t after = 0; + NTSTATUS status; + + ctx = talloc_new(*state); + assert_non_null(ctx); + + ldb = ldb_init(ctx, NULL); + assert_non_null(ldb); + + domain_dn = ldb_dn_new(ctx, ldb, "CN=Domain"); + assert_non_null(domain_dn); + + msg = create_message(ctx); + add_sid(msg, "S-1-5-21-2470180966-3899876309-2637894779-1000"); + + msg->dn = ldb_dn_new(ctx, ldb, "CN=User"); + assert_non_null(msg->dn); + + before = talloc_total_size(ctx); + + expect_check(__wrap_dsdb_search_dn, basedn, check_dn, domain_dn); + will_return(__wrap_dsdb_search_dn, build_domain_pso_result(ldb, ctx)); + will_return(__wrap_dsdb_search_dn, LDB_SUCCESS); + + expect_check(__wrap_dsdb_search_dn, basedn, check_dn, msg->dn); + will_return(__wrap_dsdb_search_dn, build_reread_result(ldb, ctx, 0)); + will_return(__wrap_dsdb_search_dn, LDB_SUCCESS); + + ldb_build_mod_req_res = talloc_zero(ldb, struct ldb_request); + ldb_wait_ret = LDB_ERR_OPERATIONS_ERROR; + ldb_transaction_cancel_ret = LDB_ERR_OPERATIONS_ERROR; + + will_return(__wrap_samdb_msg_add_int64, LDB_SUCCESS); + will_return(__wrap_samdb_msg_add_int64, LDB_SUCCESS); + + status = authsam_logon_success_accounting( + ldb, msg, domain_dn, true, NULL); + assert_true(NT_STATUS_EQUAL(status, NT_STATUS_INTERNAL_ERROR)); + assert_true(in_transaction); + assert_false(transaction_cancelled); + assert_false(transaction_committed); + + /* + * Check that all allocated memory was freed + */ + after = talloc_total_size(ctx); + assert_int_equal(before, after); + + /* + * Clean up + */ + TALLOC_FREE(ctx); +} + +/* + * authsam_logon_success_accounting + * + * The bad password indicator is set, but the account is not locked out. + * + */ +static void test_success_accounting_spurious_bad_pwd_indicator(void **state) { + struct ldb_context *ldb = NULL; + struct ldb_message *msg = NULL; + struct ldb_dn *domain_dn = NULL; + TALLOC_CTX *ctx = NULL; + size_t before = 0; + size_t after = 0; + NTSTATUS status; + + ctx = talloc_new(*state); + assert_non_null(ctx); + + ldb = ldb_init(ctx, NULL); + assert_non_null(ldb); + + domain_dn = ldb_dn_new(ctx, ldb, "CN=Domain"); + assert_non_null(domain_dn); + + msg = create_message(ctx); + add_sid(msg, "S-1-5-21-2470180966-3899876309-2637894779-1000"); + + msg->dn = ldb_dn_new(ctx, ldb, "CN=User"); + assert_non_null(msg->dn); + + before = talloc_total_size(ctx); + + expect_check(__wrap_dsdb_search_dn, basedn, check_dn, domain_dn); + will_return(__wrap_dsdb_search_dn, build_domain_pso_result(ldb, ctx)); + will_return(__wrap_dsdb_search_dn, LDB_SUCCESS); + + expect_check(__wrap_dsdb_search_dn, basedn, check_dn, msg->dn); + will_return(__wrap_dsdb_search_dn, build_reread_result(ldb, ctx, 0)); + will_return(__wrap_dsdb_search_dn, LDB_SUCCESS); + + will_return_count(__wrap_samdb_msg_add_int64, LDB_SUCCESS, 2); + + /* + * Set the bad password indicator. + */ + status = authsam_set_bad_password_indicator(ldb, ctx, msg); + assert_true(NT_STATUS_EQUAL(NT_STATUS_OK, status)); + + ldb_build_mod_req_res = talloc_zero(ctx, struct ldb_request); + + status = authsam_logon_success_accounting( + ldb, msg, domain_dn, true, NULL); + assert_true(NT_STATUS_EQUAL(status, NT_STATUS_OK)); + assert_false(in_transaction); + assert_false(transaction_cancelled); + assert_true(transaction_committed); + + /* + * Check that all allocated memory was freed + */ + after = talloc_total_size(ctx); + assert_int_equal(before, after); + + /* + * Clean up + */ + TALLOC_FREE(ctx); +} + +/* + * get_bad_password_db + * + * ldb_get_opaque failure. + */ +static void test_get_bad_password_get_opaque_failed(void **state) { + struct ldb_context *ldb = NULL; + TALLOC_CTX *ctx = NULL; + struct db_context *db = NULL; + size_t before = 0; + size_t after = 0; + + ctx = talloc_new(*state); + assert_non_null(ctx); + + ldb = ldb_init(ctx, NULL); + assert_non_null(ldb); + + /* + * clear the mock ldb_get_opaque return value, so that we get a null + * response. + */ + TALLOC_FREE(ldb_get_opaque_ret); + + before = talloc_total_size(ctx); + + db = authsam_get_bad_password_db(ctx, ldb); + assert_null(db); + + /* + * Check that all allocated memory was freed + */ + after = talloc_total_size(ctx); + assert_int_equal(before, after); + + /* + * Clean up + */ + TALLOC_FREE(ctx); +} + +/* + * get_bad_password_db + * + * cluster_db_tmp_open failure. + */ +static void test_get_bad_password_db_open_failed(void **state) { + struct ldb_context *ldb = NULL; + TALLOC_CTX *ctx = NULL; + struct db_context *db = NULL; + size_t before = 0; + size_t after = 0; + + ctx = talloc_new(*state); + assert_non_null(ctx); + + ldb = ldb_init(ctx, NULL); + assert_non_null(ldb); + + /* + * Clear the mock cluster_db_tmp_open return value so that + * it returns NULL + */ + TALLOC_FREE(cluster_db_tmp_open_ret); + before = talloc_total_size(ctx); + + db = authsam_get_bad_password_db(ctx, ldb); + assert_null(db); + + /* + * Check that all allocated memory was freed + */ + after = talloc_total_size(ctx); + assert_int_equal(before, after); + + /* + * Clean up + */ + TALLOC_FREE(ctx); +} + +/* + * set_bad_password_indicator + * + * set_bad_password_indicator failure. + */ +static void test_set_bad_password_indicator_get_db_failed(void **state) { + struct ldb_context *ldb = NULL; + TALLOC_CTX *ctx = NULL; + NTSTATUS status; + size_t before = 0; + size_t after = 0; + + ctx = talloc_new(*state); + assert_non_null(ctx); + + ldb = ldb_init(ctx, NULL); + assert_non_null(ldb); + + /* + * Clear the mock cluster_db_tmp_open return value so that + * it returns NULL + */ + TALLOC_FREE(cluster_db_tmp_open_ret); + before = talloc_total_size(ctx); + + status = authsam_set_bad_password_indicator(ldb, ctx, NULL); + assert_true(NT_STATUS_EQUAL(NT_STATUS_INTERNAL_ERROR, status)); + + /* + * Check that all allocated memory was freed + */ + after = talloc_total_size(ctx); + assert_int_equal(before, after); + + /* + * Clean up + */ + TALLOC_FREE(ctx); +} + +/* + * set_bad_password_indicator + * + * get_object_sid_as_tdb_data failure. + */ +static void test_set_bad_password_indicator_get_object_sid_failed( + void **state) +{ + struct ldb_context *ldb = NULL; + struct ldb_message *msg = NULL; + TALLOC_CTX *ctx = NULL; + NTSTATUS status; + size_t before = 0; + size_t after = 0; + + ctx = talloc_new(*state); + assert_non_null(ctx); + + ldb = ldb_init(ctx, NULL); + assert_non_null(ldb); + + /* + * The created message does not contain an objectSid, so + * get_object_sid_as_tdb_data will fail. + */ + msg = create_message(ctx); + + before = talloc_total_size(ctx); + + status = authsam_set_bad_password_indicator(ldb, ctx, msg); + assert_true(NT_STATUS_EQUAL(NT_STATUS_INTERNAL_ERROR, status)); + + /* + * Check that all allocated memory was freed + */ + after = talloc_total_size(ctx); + assert_int_equal(before, after); + + /* + * Clean up + */ + TALLOC_FREE(ctx); +} + +/* + * set_bad_password_indicator + * + * dbwrap_store failure. + */ +static void test_set_bad_password_indicator_dbwrap_store_failed( + void **state) +{ + struct ldb_context *ldb = NULL; + struct ldb_message *msg = NULL; + TALLOC_CTX *ctx = NULL; + NTSTATUS status; + size_t before = 0; + size_t after = 0; + + ctx = talloc_new(*state); + assert_non_null(ctx); + + ldb = ldb_init(ctx, NULL); + assert_non_null(ldb); + + msg = create_message(ctx); + add_sid(msg, "S-1-5-21-2470180966-3899876309-2637894779-1010"); + + dbwrap_store_ret = NT_STATUS_INTERNAL_DB_CORRUPTION; + + before = talloc_total_size(ctx); + + status = authsam_set_bad_password_indicator(ldb, ctx, msg); + assert_true(NT_STATUS_EQUAL(NT_STATUS_INTERNAL_DB_CORRUPTION, status)); + + /* + * Check that all allocated memory was freed + */ + after = talloc_total_size(ctx); + assert_int_equal(before, after); + + /* + * Clean up + */ + TALLOC_FREE(ctx); +} + +/* + * check_bad_password_indicator + * + * set_bad_password_indicator failure. + */ +static void test_check_bad_password_indicator_get_db_failed(void **state) { + struct ldb_context *ldb = NULL; + TALLOC_CTX *ctx = NULL; + NTSTATUS status; + size_t before = 0; + size_t after = 0; + bool exists = false; + + ctx = talloc_new(*state); + assert_non_null(ctx); + + ldb = ldb_init(ctx, NULL); + assert_non_null(ldb); + + /* + * Clear the mock cluster_db_tmp_open return value so that + * it returns NULL + */ + TALLOC_FREE(cluster_db_tmp_open_ret); + before = talloc_total_size(ctx); + + status = authsam_check_bad_password_indicator(ldb, ctx, &exists, NULL); + assert_true(NT_STATUS_EQUAL(NT_STATUS_INTERNAL_ERROR, status)); + + /* + * Check that all allocated memory was freed + */ + after = talloc_total_size(ctx); + assert_int_equal(before, after); + + /* + * Clean up + */ + TALLOC_FREE(ctx); +} + +/* + * check_bad_password_indicator + * + * get_object_sid_as_tdb_data failure. + */ +static void test_check_bad_password_indicator_get_object_sid_failed( + void **state) +{ + struct ldb_context *ldb = NULL; + struct ldb_message *msg = NULL; + TALLOC_CTX *ctx = NULL; + NTSTATUS status; + size_t before = 0; + size_t after = 0; + bool exists = false; + + ctx = talloc_new(*state); + assert_non_null(ctx); + + ldb = ldb_init(ctx, NULL); + assert_non_null(ldb); + + /* + * The created message does not contain an objectSid, so + * get_object_sid_as_tdb_data will fail. + */ + msg = create_message(ctx); + + before = talloc_total_size(ctx); + + status = authsam_check_bad_password_indicator(ldb, ctx, &exists, msg); + assert_true(NT_STATUS_EQUAL(NT_STATUS_INTERNAL_ERROR, status)); + + /* + * Check that all allocated memory was freed + */ + after = talloc_total_size(ctx); + assert_int_equal(before, after); + + /* + * Clean up + */ + TALLOC_FREE(ctx); +} + +/* + * clear_bad_password_indicator + * + * set_bad_password_indicator failure. + */ +static void test_clear_bad_password_indicator_get_db_failed(void **state) { + struct ldb_context *ldb = NULL; + TALLOC_CTX *ctx = NULL; + NTSTATUS status; + size_t before = 0; + size_t after = 0; + + ctx = talloc_new(*state); + assert_non_null(ctx); + + ldb = ldb_init(ctx, NULL); + assert_non_null(ldb); + + /* + * Clear the mock cluster_db_tmp_open return value so that + * it returns NULL + */ + TALLOC_FREE(cluster_db_tmp_open_ret); + before = talloc_total_size(ctx); + + status = authsam_clear_bad_password_indicator(ldb, ctx, NULL); + assert_true(NT_STATUS_EQUAL(NT_STATUS_INTERNAL_ERROR, status)); + + /* + * Check that all allocated memory was freed + */ + after = talloc_total_size(ctx); + assert_int_equal(before, after); + + /* + * Clean up + */ + TALLOC_FREE(ctx); +} + +/* + * clear_bad_password_indicator + * + * get_object_sid_as_tdb_data failure. + */ +static void test_clear_bad_password_indicator_get_object_sid_failed( + void **state) +{ + struct ldb_context *ldb = NULL; + struct ldb_message *msg = NULL; + TALLOC_CTX *ctx = NULL; + NTSTATUS status; + size_t before = 0; + size_t after = 0; + + ctx = talloc_new(*state); + assert_non_null(ctx); + + ldb = ldb_init(ctx, NULL); + assert_non_null(ldb); + + /* + * The created message does not contain an objectSid, so + * get_object_sid_as_tdb_data will fail. + */ + msg = create_message(ctx); + + before = talloc_total_size(ctx); + + status = authsam_clear_bad_password_indicator(ldb, ctx, msg); + assert_true(NT_STATUS_EQUAL(NT_STATUS_INTERNAL_ERROR, status)); + + /* + * Check that all allocated memory was freed + */ + after = talloc_total_size(ctx); + assert_int_equal(before, after); + + /* + * Clean up + */ + TALLOC_FREE(ctx); +} + +/* + * clear_bad_password_indicator + * + * dbwrap_delete failure. + */ +static void test_clear_bad_password_indicator_dbwrap_store_failed( + void **state) +{ + struct ldb_context *ldb = NULL; + struct ldb_message *msg = NULL; + TALLOC_CTX *ctx = NULL; + NTSTATUS status; + size_t before = 0; + size_t after = 0; + + ctx = talloc_new(*state); + assert_non_null(ctx); + + ldb = ldb_init(ctx, NULL); + assert_non_null(ldb); + + msg = create_message(ctx); + add_sid(msg, "S-1-5-21-2470180966-3899876309-2637894779-1010"); + + dbwrap_delete_ret = NT_STATUS_INTERNAL_DB_CORRUPTION; + + before = talloc_total_size(ctx); + + status = authsam_clear_bad_password_indicator(ldb, ctx, msg); + assert_true(NT_STATUS_EQUAL(NT_STATUS_INTERNAL_DB_CORRUPTION, status)); + + /* + * Check that all allocated memory was freed + */ + after = talloc_total_size(ctx); + assert_int_equal(before, after); + + /* + * Clean up + */ + TALLOC_FREE(ctx); +} + +/* + * clear_bad_password_indicator + * + * dbwrap_delete returns NT_STATUS_NOT_FOUND. + */ +static void test_clear_bad_pwd_indicator_dbwrap_store_not_found( + void **state) +{ + struct ldb_context *ldb = NULL; + struct ldb_message *msg = NULL; + TALLOC_CTX *ctx = NULL; + NTSTATUS status; + size_t before = 0; + size_t after = 0; + + ctx = talloc_new(*state); + assert_non_null(ctx); + + ldb = ldb_init(ctx, NULL); + assert_non_null(ldb); + + msg = create_message(ctx); + add_sid(msg, "S-1-5-21-2470180966-3899876309-2637894779-1010"); + + dbwrap_delete_ret = NT_STATUS_NOT_FOUND; + + before = talloc_total_size(ctx); + + status = authsam_clear_bad_password_indicator(ldb, ctx, msg); + assert_true(NT_STATUS_IS_OK(status)); + + /* + * Check that all allocated memory was freed + */ + after = talloc_total_size(ctx); + assert_int_equal(before, after); + + /* + * Clean up + */ + TALLOC_FREE(ctx); +} + +int main(int argc, const char **argv) +{ + const struct CMUnitTest tests[] = { + cmocka_unit_test_setup_teardown( + test_reread_read_failure, setup, teardown), + cmocka_unit_test_setup_teardown( + test_reread_missing_account_control, setup, teardown), + cmocka_unit_test_setup_teardown( + test_reread_account_locked, setup, teardown), + cmocka_unit_test_setup_teardown( + test_reread_account_not_locked, setup, teardown), + cmocka_unit_test_setup_teardown( + test_update_bad_domain_dn_search_failed, + setup, + teardown), + cmocka_unit_test_setup_teardown( + test_update_bad_get_pso_failed, setup, teardown), + cmocka_unit_test_setup_teardown( + test_update_bad_start_txn_failed, setup, teardown), + cmocka_unit_test_setup_teardown( + test_update_bad_reread_failed, setup, teardown), + cmocka_unit_test_setup_teardown( + test_update_bad_reread_locked_out, setup, teardown), + cmocka_unit_test_setup_teardown( + test_update_bad_update_count_failed, setup, teardown), + cmocka_unit_test_setup_teardown( + test_update_bad_no_update_required, setup, teardown), + cmocka_unit_test_setup_teardown( + test_update_bad_build_mod_request_failed, + setup, + teardown), + cmocka_unit_test_setup_teardown( + test_update_bad_add_control_failed, setup, teardown), + cmocka_unit_test_setup_teardown( + test_update_bad_ldb_request_failed, setup, teardown), + cmocka_unit_test_setup_teardown( + test_update_bad_ldb_wait_failed, setup, teardown), + cmocka_unit_test_setup_teardown( + test_update_bad_txn_cancel_failed, setup, teardown), + cmocka_unit_test_setup_teardown( + test_update_bad_commit_failed, setup, teardown), + cmocka_unit_test_setup_teardown( + test_success_accounting_start_txn_failed, + setup, + teardown), + cmocka_unit_test_setup_teardown( + test_success_accounting_reread_failed, + setup, + teardown), + cmocka_unit_test_setup_teardown( + test_success_accounting_ldb_msg_new_failed, + setup, + teardown), + cmocka_unit_test_setup_teardown( + test_success_accounting_samdb_rodc_failed, + setup, + teardown), + cmocka_unit_test_setup_teardown( + test_success_accounting_update_lastlogon_failed, + setup, + teardown), + cmocka_unit_test_setup_teardown( + test_success_accounting_build_mod_req_failed, + setup, + teardown), + cmocka_unit_test_setup_teardown( + test_success_accounting_add_control_failed, + setup, + teardown), + cmocka_unit_test_setup_teardown( + test_success_accounting_ldb_request_failed, + setup, + teardown), + cmocka_unit_test_setup_teardown( + test_success_accounting_ldb_wait_failed, + setup, + teardown), + cmocka_unit_test_setup_teardown( + test_success_accounting_commit_failed, + setup, + teardown), + cmocka_unit_test_setup_teardown( + test_success_accounting_rollback_failed, + setup, + teardown), + cmocka_unit_test_setup_teardown( + test_success_accounting_spurious_bad_pwd_indicator, + setup, + teardown), + cmocka_unit_test_setup_teardown( + test_get_bad_password_get_opaque_failed, + setup, + teardown), + cmocka_unit_test_setup_teardown( + test_get_bad_password_db_open_failed, + setup, + teardown), + cmocka_unit_test_setup_teardown( + test_set_bad_password_indicator_get_db_failed, + setup, + teardown), + cmocka_unit_test_setup_teardown( + test_set_bad_password_indicator_get_object_sid_failed, + setup, + teardown), + cmocka_unit_test_setup_teardown( + test_set_bad_password_indicator_dbwrap_store_failed, + setup, + teardown), + cmocka_unit_test_setup_teardown( + test_check_bad_password_indicator_get_db_failed, + setup, + teardown), + cmocka_unit_test_setup_teardown( + test_check_bad_password_indicator_get_object_sid_failed, + setup, + teardown), + cmocka_unit_test_setup_teardown( + test_clear_bad_password_indicator_get_db_failed, + setup, + teardown), + cmocka_unit_test_setup_teardown( + test_clear_bad_password_indicator_get_object_sid_failed, + setup, + teardown), + cmocka_unit_test_setup_teardown( + test_clear_bad_password_indicator_dbwrap_store_failed, + setup, + teardown), + cmocka_unit_test_setup_teardown( + test_clear_bad_pwd_indicator_dbwrap_store_not_found, + setup, + teardown), + }; + + cmocka_set_message_output(CM_OUTPUT_SUBUNIT); + return cmocka_run_group_tests(tests, NULL, NULL); +} diff --git a/source4/auth/wscript_build b/source4/auth/wscript_build index 381a7b19bf0..ff1a61a9566 100644 --- a/source4/auth/wscript_build +++ b/source4/auth/wscript_build @@ -49,6 +49,17 @@ bld.SAMBA_BINARY('test_kerberos', for_selftest=True ) +bld.SAMBA_BINARY('test_auth_sam', + source='tests/sam.c', + deps='cmocka samdb samba-security ldb tevent', + local_include=False, + for_selftest=True, + ldflags=''' + -Wl,--wrap,dsdb_search_dn + -Wl,--wrap,samdb_msg_add_int64 + ''' + ) + pytalloc_util = bld.pyembed_libname('pytalloc-util') pyparam_util = bld.pyembed_libname('pyparam_util') pyldb_util = bld.pyembed_libname('pyldb-util') -- 2.35.0 From 9f6ae44aadb81b33e3025c21af6d6c4c35bc83aa Mon Sep 17 00:00:00 2001 From: Joseph Sutton Date: Tue, 5 Jul 2022 20:17:49 +1200 Subject: [PATCH 15/41] CVE-2021-20251 auth4: Detect ACCOUNT_LOCKED_OUT error for password change This is more specific than NT_STATUS_UNSUCCESSFUL, and for the SAMR password change, matches the result the call to samdb_result_passwords() would give. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14611 Signed-off-by: Joseph Sutton Reviewed-by: Andreas Schneider Reviewed-by: Andrew Bartlett (cherry picked from commit 336e303cf1962b56b64c0d9d2b05ac15d00e8692) --- source4/dsdb/common/util.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/source4/dsdb/common/util.c b/source4/dsdb/common/util.c index 451495fe4c5..aff31ac8651 100644 --- a/source4/dsdb/common/util.c +++ b/source4/dsdb/common/util.c @@ -2301,7 +2301,8 @@ int samdb_set_password_callback(struct ldb_request *req, struct ldb_reply *ares) * change failed. * * Results: NT_STATUS_OK, NT_STATUS_INVALID_PARAMETER, NT_STATUS_UNSUCCESSFUL, - * NT_STATUS_WRONG_PASSWORD, NT_STATUS_PASSWORD_RESTRICTION + * NT_STATUS_WRONG_PASSWORD, NT_STATUS_PASSWORD_RESTRICTION, + * NT_STATUS_ACCESS_DENIED, NT_STATUS_ACCOUNT_LOCKED_OUT, NT_STATUS_NO_MEMORY */ static NTSTATUS samdb_set_password_internal(struct ldb_context *ldb, TALLOC_CTX *mem_ctx, struct ldb_dn *user_dn, struct ldb_dn *domain_dn, @@ -2474,6 +2475,9 @@ static NTSTATUS samdb_set_password_internal(struct ldb_context *ldb, TALLOC_CTX if (W_ERROR_EQUAL(werr, WERR_PASSWORD_RESTRICTION)) { status = NT_STATUS_PASSWORD_RESTRICTION; } + if (W_ERROR_EQUAL(werr, WERR_ACCOUNT_LOCKED_OUT)) { + status = NT_STATUS_ACCOUNT_LOCKED_OUT; + } } } else if (ret == LDB_ERR_NO_SUCH_OBJECT) { /* don't let the caller know if an account doesn't exist */ @@ -2523,6 +2527,7 @@ NTSTATUS samdb_set_password(struct ldb_context *ldb, TALLOC_CTX *mem_ctx, * Results: NT_STATUS_OK, NT_STATUS_INTERNAL_DB_CORRUPTION, * NT_STATUS_INVALID_PARAMETER, NT_STATUS_UNSUCCESSFUL, * NT_STATUS_WRONG_PASSWORD, NT_STATUS_PASSWORD_RESTRICTION, + * NT_STATUS_ACCESS_DENIED, NT_STATUS_ACCOUNT_LOCKED_OUT, NT_STATUS_NO_MEMORY * NT_STATUS_TRANSACTION_ABORTED, NT_STATUS_NO_SUCH_USER */ NTSTATUS samdb_set_password_sid(struct ldb_context *ldb, TALLOC_CTX *mem_ctx, -- 2.35.0 From f1198feb8d7bf27b99e64033aa14d3b200fab61b Mon Sep 17 00:00:00 2001 From: Andrew Bartlett Date: Tue, 30 Mar 2021 18:01:39 +1300 Subject: [PATCH 16/41] CVE-2021-20251 s4 auth: make bad password count increment atomic Ensure that the bad password count is incremented atomically, and that the successful logon accounting data is updated atomically. Use bad password indicator (in a distinct TDB) to determine if to open a transaction We open a transaction when we have seen the hint that this user has recorded a bad password. This allows us to avoid always needing one, while not missing a possible lockout. We also go back and get a transation if we did not take out one out but we chose to do a write (eg for lastLogonTimestamp) Based on patches by Gary Lockyer BUG: https://bugzilla.samba.org/show_bug.cgi?id=14611 Signed-off-by: Andrew Bartlett Reviewed-by: Joseph Sutton Reviewed-by: Andreas Schneider (cherry picked from commit de4cc0a3dae89f3e51a099282615cf80c8539e11) --- selftest/knownfail.d/auth-sam | 12 -- selftest/knownfail_heimdal_kdc | 5 - selftest/knownfail_mit_kdc | 3 - source4/auth/sam.c | 296 +++++++++++++++++++++++++++------ 4 files changed, 246 insertions(+), 70 deletions(-) diff --git a/selftest/knownfail.d/auth-sam b/selftest/knownfail.d/auth-sam index e87e43d509b..048459e6555 100644 --- a/selftest/knownfail.d/auth-sam +++ b/selftest/knownfail.d/auth-sam @@ -11,15 +11,3 @@ ^samba.unittests.auth.sam.test_success_accounting_spurious_bad_pwd_indicator.none ^samba.unittests.auth.sam.test_success_accounting_start_txn_failed.none ^samba.unittests.auth.sam.test_success_accounting_update_lastlogon_failed.none -^samba.unittests.auth.sam.test_update_bad_add_control_failed.none -^samba.unittests.auth.sam.test_update_bad_build_mod_request_failed.none -^samba.unittests.auth.sam.test_update_bad_commit_failed.none -^samba.unittests.auth.sam.test_update_bad_get_pso_failed.none -^samba.unittests.auth.sam.test_update_bad_ldb_request_failed.none -^samba.unittests.auth.sam.test_update_bad_ldb_wait_failed.none -^samba.unittests.auth.sam.test_update_bad_no_update_required.none -^samba.unittests.auth.sam.test_update_bad_reread_failed.none -^samba.unittests.auth.sam.test_update_bad_reread_locked_out.none -^samba.unittests.auth.sam.test_update_bad_start_txn_failed.none -^samba.unittests.auth.sam.test_update_bad_txn_cancel_failed.none -^samba.unittests.auth.sam.test_update_bad_update_count_failed.none diff --git a/selftest/knownfail_heimdal_kdc b/selftest/knownfail_heimdal_kdc index ef913b04952..c0335bf4b52 100644 --- a/selftest/knownfail_heimdal_kdc +++ b/selftest/knownfail_heimdal_kdc @@ -57,10 +57,5 @@ # # Lockout tests # -^samba.tests.krb5.lockout_tests.samba.tests.krb5.lockout_tests.LockoutTests.test_bad_pwd_count_transaction_kdc.ad_dc:local -^samba.tests.krb5.lockout_tests.samba.tests.krb5.lockout_tests.LockoutTests.test_bad_pwd_count_transaction_ntlm.ad_dc:local -^samba.tests.krb5.lockout_tests.samba.tests.krb5.lockout_tests.LockoutTests.test_bad_pwd_count_transaction_rename_kdc.ad_dc:local -^samba.tests.krb5.lockout_tests.samba.tests.krb5.lockout_tests.LockoutTests.test_bad_pwd_count_transaction_rename_ntlm.ad_dc:local -^samba.tests.krb5.lockout_tests.samba.tests.krb5.lockout_tests.LockoutTests.test_bad_pwd_count_transaction_samr.ad_dc:local ^samba.tests.krb5.lockout_tests.samba.tests.krb5.lockout_tests.LockoutTests.test_lockout_race_kdc.ad_dc:local ^samba.tests.krb5.lockout_tests.samba.tests.krb5.lockout_tests.LockoutTests.test_lockout_race_ntlm.ad_dc:local diff --git a/selftest/knownfail_mit_kdc b/selftest/knownfail_mit_kdc index 6ece892767a..0af1f02142d 100644 --- a/selftest/knownfail_mit_kdc +++ b/selftest/knownfail_mit_kdc @@ -443,10 +443,7 @@ samba.tests.krb5.as_canonicalization_tests.samba.tests.krb5.as_canonicalization_ # Lockout tests # ^samba.tests.krb5.lockout_tests.samba.tests.krb5.lockout_tests.LockoutTests.test_bad_pwd_count_transaction_kdc.ad_dc:local -^samba.tests.krb5.lockout_tests.samba.tests.krb5.lockout_tests.LockoutTests.test_bad_pwd_count_transaction_ntlm.ad_dc:local ^samba.tests.krb5.lockout_tests.samba.tests.krb5.lockout_tests.LockoutTests.test_bad_pwd_count_transaction_rename_kdc.ad_dc:local -^samba.tests.krb5.lockout_tests.samba.tests.krb5.lockout_tests.LockoutTests.test_bad_pwd_count_transaction_rename_ntlm.ad_dc:local -^samba.tests.krb5.lockout_tests.samba.tests.krb5.lockout_tests.LockoutTests.test_bad_pwd_count_transaction_samr.ad_dc:local ^samba.tests.krb5.lockout_tests.samba.tests.krb5.lockout_tests.LockoutTests.test_lockout_race_kdc.ad_dc:local ^samba.tests.krb5.lockout_tests.samba.tests.krb5.lockout_tests.LockoutTests.test_lockout_race_ntlm.ad_dc:local ^samba.tests.krb5.lockout_tests.samba.tests.krb5.lockout_tests.LockoutTests.test_lockout_transaction_bad_pwd_kdc.ad_dc:local diff --git a/source4/auth/sam.c b/source4/auth/sam.c index 92c0480ba6a..19199701465 100644 --- a/source4/auth/sam.c +++ b/source4/auth/sam.c @@ -1073,7 +1073,9 @@ NTSTATUS authsam_update_bad_pwd_count(struct ldb_context *sam_ctx, NTSTATUS status; struct ldb_result *domain_res; struct ldb_message *msg_mod = NULL; + struct ldb_message *current = NULL; struct ldb_message *pso_msg = NULL; + bool txn_active = false; TALLOC_CTX *mem_ctx; mem_ctx = talloc_new(msg); @@ -1098,14 +1100,65 @@ NTSTATUS authsam_update_bad_pwd_count(struct ldb_context *sam_ctx, ret, ldb_dn_get_linearized(msg->dn)); } - status = dsdb_update_bad_pwd_count(mem_ctx, sam_ctx, - msg, domain_res->msgs[0], pso_msg, - &msg_mod); + /* + * To ensure that the bad password count is updated atomically, + * we need to: + * begin a transaction + * re-read the account details, + * using the msgs[0], + pso_msg, + &msg_mod); + if (!NT_STATUS_IS_OK(status)) { + status = NT_STATUS_INTERNAL_ERROR; + goto error; + } + + /* + * Write the data back to disk if required. + */ if (msg_mod != NULL) { struct ldb_request *req; @@ -1116,7 +1169,9 @@ NTSTATUS authsam_update_bad_pwd_count(struct ldb_context *sam_ctx, ldb_op_default_callback, NULL); if (ret != LDB_SUCCESS) { - goto done; + TALLOC_FREE(msg_mod); + status = NT_STATUS_INTERNAL_ERROR; + goto error; } ret = ldb_request_add_control(req, @@ -1124,31 +1179,72 @@ NTSTATUS authsam_update_bad_pwd_count(struct ldb_context *sam_ctx, false, NULL); if (ret != LDB_SUCCESS) { talloc_free(req); - goto done; + status = NT_STATUS_INTERNAL_ERROR; + goto error; } - ret = dsdb_autotransaction_request(sam_ctx, req); + /* + * As we're in a transaction, make the ldb request directly + * to avoid the nested transaction that would result if we + * called dsdb_autotransaction_request + */ + ret = ldb_request(sam_ctx, req); + if (ret == LDB_SUCCESS) { + ret = ldb_wait(req->handle, LDB_WAIT_ALL); + } talloc_free(req); - + if (ret != LDB_SUCCESS) { + status = NT_STATUS_INTERNAL_ERROR; + goto error; + } status = authsam_set_bad_password_indicator( sam_ctx, mem_ctx, msg); - /* Failure is ignored for now */ + if (!NT_STATUS_IS_OK(status)) { + goto error; + } } - -done: + /* + * Note that we may not have updated the user record, but + * committing the transaction in that case is still the correct + * thing to do. + * If the transaction was cancelled, this would be logged by + * the dsdb audit log as a failure. When in fact it is expected + * behaviour. + */ +exit: + TALLOC_FREE(mem_ctx); + ret = ldb_transaction_commit(sam_ctx); if (ret != LDB_SUCCESS) { - DBG_ERR("Failed to update badPwdCount, badPasswordTime or " - "set lockoutTime on %s: %s\n", - ldb_dn_get_linearized(msg->dn), - ldb_errstring(sam_ctx)); - TALLOC_FREE(mem_ctx); + DBG_ERR("Error (%d) %s, committing transaction," + " while updating bad password count" + " for (%s)\n", + ret, + ldb_errstring(sam_ctx), + ldb_dn_get_linearized(msg->dn)); return NT_STATUS_INTERNAL_ERROR; } + return status; +error: + DBG_ERR("Failed to update badPwdCount, badPasswordTime or " + "set lockoutTime on %s: %s\n", + ldb_dn_get_linearized(msg->dn), + ldb_errstring(sam_ctx) != NULL ? + ldb_errstring(sam_ctx) :nt_errstr(status)); + if (txn_active) { + ret = ldb_transaction_cancel(sam_ctx); + if (ret != LDB_SUCCESS) { + DBG_ERR("Error rolling back transaction," + " while updating bad password count" + " on %s: %s\n", + ldb_dn_get_linearized(msg->dn), + ldb_errstring(sam_ctx)); + } + } TALLOC_FREE(mem_ctx); - return NT_STATUS_OK; -} + return status; +} static NTSTATUS authsam_update_lastlogon_timestamp(struct ldb_context *sam_ctx, struct ldb_message *msg_mod, @@ -1296,6 +1392,7 @@ NTSTATUS authsam_logon_success_accounting(struct ldb_context *sam_ctx, NTTIME now; NTTIME lastLogonTimestamp; bool am_rodc = false; + bool txn_active = false; bool need_db_reread; mem_ctx = talloc_new(msg); @@ -1303,15 +1400,41 @@ NTSTATUS authsam_logon_success_accounting(struct ldb_context *sam_ctx, return NT_STATUS_NO_MEMORY; } + /* + * Any update of the last logon data, needs to be done inside a + * transaction. + * And the user data needs to be re-read, and the account re-checked + * for lockout. + * + * Howevver we have long-running transactions like replication + * that could otherwise grind the system to a halt so we first + * determine if *this* account has seen a bad password, + * otherwise we only start a transaction if there was a need + * (because a change was to be made). + */ + status = authsam_check_bad_password_indicator( sam_ctx, mem_ctx, &need_db_reread, msg); if (!NT_STATUS_IS_OK(status)) { return status; } +get_transaction: + if (need_db_reread) { struct ldb_message *current = NULL; + /* + * Start a new transaction + */ + ret = ldb_transaction_start(sam_ctx); + if (ret != LDB_SUCCESS) { + status = NT_STATUS_INTERNAL_ERROR; + goto error; + } + + txn_active = true; + /* * Re-read the account details, using the GUID * embedded in DN so this is safe against a race where @@ -1324,7 +1447,15 @@ NTSTATUS authsam_logon_success_accounting(struct ldb_context *sam_ctx, * The re-read can return account locked out, as well * as an internal error */ - return status; + if (NT_STATUS_EQUAL(status, NT_STATUS_ACCOUNT_LOCKED_OUT)) { + /* + * For NT_STATUS_ACCOUNT_LOCKED_OUT we want to commit + * the transaction. Again to avoid cluttering the + * audit logs with spurious errors + */ + goto exit; + } + goto error; } msg = current; } @@ -1345,9 +1476,15 @@ NTSTATUS authsam_logon_success_accounting(struct ldb_context *sam_ctx, msg_mod = ldb_msg_new(mem_ctx); if (msg_mod == NULL) { - TALLOC_FREE(mem_ctx); - return NT_STATUS_NO_MEMORY; + status = NT_STATUS_NO_MEMORY; + goto error; } + + /* + * By using the DN from msg->dn directly, we allow LDB to + * prefer the embedded GUID form, so this is actually quite + * safe even in the case where DN has been changed + */ msg_mod->dn = msg->dn; if (lockoutTime != 0) { @@ -1356,14 +1493,14 @@ NTSTATUS authsam_logon_success_accounting(struct ldb_context *sam_ctx, */ ret = samdb_msg_add_int(sam_ctx, msg_mod, msg_mod, "lockoutTime", 0); if (ret != LDB_SUCCESS) { - TALLOC_FREE(mem_ctx); - return NT_STATUS_NO_MEMORY; + status = NT_STATUS_NO_MEMORY; + goto error; } } else if (badPwdCount != 0) { ret = samdb_msg_add_int(sam_ctx, msg_mod, msg_mod, "badPwdCount", 0); if (ret != LDB_SUCCESS) { - TALLOC_FREE(mem_ctx); - return NT_STATUS_NO_MEMORY; + status = NT_STATUS_NO_MEMORY; + goto error; } } @@ -1375,8 +1512,8 @@ NTSTATUS authsam_logon_success_accounting(struct ldb_context *sam_ctx, ret = samdb_msg_add_int64(sam_ctx, msg_mod, msg_mod, "lastLogon", now); if (ret != LDB_SUCCESS) { - TALLOC_FREE(mem_ctx); - return NT_STATUS_NO_MEMORY; + status = NT_STATUS_NO_MEMORY; + goto error; } } @@ -1390,8 +1527,8 @@ NTSTATUS authsam_logon_success_accounting(struct ldb_context *sam_ctx, ret = samdb_msg_add_int(sam_ctx, msg_mod, msg_mod, "logonCount", logonCount); if (ret != LDB_SUCCESS) { - TALLOC_FREE(mem_ctx); - return NT_STATUS_NO_MEMORY; + status = NT_STATUS_NO_MEMORY; + goto error; } } else { /* Set an unset logonCount to 0 on first successful login */ @@ -1407,16 +1544,16 @@ NTSTATUS authsam_logon_success_accounting(struct ldb_context *sam_ctx, ret = samdb_rodc(sam_ctx, &am_rodc); if (ret != LDB_SUCCESS) { - TALLOC_FREE(mem_ctx); - return NT_STATUS_INTERNAL_ERROR; + status = NT_STATUS_INTERNAL_ERROR; + goto error; } if (!am_rodc) { status = authsam_update_lastlogon_timestamp(sam_ctx, msg_mod, domain_dn, lastLogonTimestamp, now); if (!NT_STATUS_IS_OK(status)) { - TALLOC_FREE(mem_ctx); - return NT_STATUS_NO_MEMORY; + status = NT_STATUS_NO_MEMORY; + goto error; } } else { /* Perform the (async) SendToSAM calls for MS-SAMS */ @@ -1436,6 +1573,16 @@ NTSTATUS authsam_logon_success_accounting(struct ldb_context *sam_ctx, unsigned int i; struct ldb_request *req; + /* + * If it turns out we are going to update the DB, go + * back to the start, get a transaction and the + * current DB state and try again + */ + if (txn_active == false) { + need_db_reread = true; + goto get_transaction; + } + /* mark all the message elements as LDB_FLAG_MOD_REPLACE */ for (i=0;inum_elements;i++) { msg_mod->elements[i].flags = LDB_FLAG_MOD_REPLACE; @@ -1448,35 +1595,84 @@ NTSTATUS authsam_logon_success_accounting(struct ldb_context *sam_ctx, ldb_op_default_callback, NULL); if (ret != LDB_SUCCESS) { - goto done; + status = NT_STATUS_INTERNAL_ERROR; + goto error; } ret = ldb_request_add_control(req, DSDB_CONTROL_FORCE_RODC_LOCAL_CHANGE, false, NULL); if (ret != LDB_SUCCESS) { - talloc_free(req); - goto done; + TALLOC_FREE(req); + status = NT_STATUS_INTERNAL_ERROR; + goto error; } - - ret = dsdb_autotransaction_request(sam_ctx, req); - talloc_free(req); + /* + * As we're in a transaction, make the ldb request directly + * to avoid the nested transaction that would result if we + * called dsdb_autotransaction_request + */ + ret = ldb_request(sam_ctx, req); + if (ret == LDB_SUCCESS) { + ret = ldb_wait(req->handle, LDB_WAIT_ALL); + } + TALLOC_FREE(req); + if (ret != LDB_SUCCESS) { + status = NT_STATUS_INTERNAL_ERROR; + goto error; + } + } + status = authsam_clear_bad_password_indicator(sam_ctx, mem_ctx, msg); + if (!NT_STATUS_IS_OK(status)) { + goto error; } - status = authsam_clear_bad_password_indicator(sam_ctx, mem_ctx, msg); - /* Failure is ignored for now */ + /* + * Note that we may not have updated the user record, but + * committing the transaction in that case is still the correct + * thing to do. + * If the transaction was cancelled, this would be logged by + * the dsdb audit log as a failure. When in fact it is expected + * behaviour. + * + * Thankfully both TDB and LMDB seem to optimise for the empty + * transaction case + */ +exit: + TALLOC_FREE(mem_ctx); -done: + if (txn_active == false) { + return status; + } + + ret = ldb_transaction_commit(sam_ctx); if (ret != LDB_SUCCESS) { - DEBUG(0, ("Failed to set badPwdCount and lockoutTime " - "to 0 and/or lastlogon to now (%lld) " - "%s: %s\n", (long long int)now, - ldb_dn_get_linearized(msg_mod->dn), - ldb_errstring(sam_ctx))); - TALLOC_FREE(mem_ctx); + DBG_ERR("Error (%d) %s, committing transaction," + " while updating successful logon accounting" + " for (%s)\n", + ret, + ldb_errstring(sam_ctx), + ldb_dn_get_linearized(msg->dn)); return NT_STATUS_INTERNAL_ERROR; } + return status; +error: + DBG_ERR("Failed to update badPwdCount, badPasswordTime or " + "set lockoutTime on %s: %s\n", + ldb_dn_get_linearized(msg->dn), + ldb_errstring(sam_ctx) != NULL ? + ldb_errstring(sam_ctx) :nt_errstr(status)); + if (txn_active) { + ret = ldb_transaction_cancel(sam_ctx); + if (ret != LDB_SUCCESS) { + DBG_ERR("Error rolling back transaction," + " while updating bad password count" + " on %s: %s\n", + ldb_dn_get_linearized(msg->dn), + ldb_errstring(sam_ctx)); + } + } TALLOC_FREE(mem_ctx); - return NT_STATUS_OK; + return status; } -- 2.35.0 From 6b8de3171bc66b97c74017ebf4706213d4b31e33 Mon Sep 17 00:00:00 2001 From: Andrew Bartlett Date: Tue, 30 Mar 2021 16:35:44 +1300 Subject: [PATCH 17/41] CVE-2021-20251 auth4: Add missing newline to debug message on PSO read failure BUG: https://bugzilla.samba.org/show_bug.cgi?id=14611 Signed-off-by: Andrew Bartlett Reviewed-by: Joseph Sutton Reviewed-by: Andreas Schneider (cherry picked from commit 4a9e0fdccfa218fbb2c3eb87e1a955ade0364b98) --- source4/auth/sam.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/source4/auth/sam.c b/source4/auth/sam.c index 19199701465..b12a7c981e6 100644 --- a/source4/auth/sam.c +++ b/source4/auth/sam.c @@ -1096,7 +1096,7 @@ NTSTATUS authsam_update_bad_pwd_count(struct ldb_context *sam_ctx, * fallback to using the domain defaults so that we still * record the bad password attempt */ - DBG_ERR("Error (%d) checking PSO for %s", + DBG_ERR("Error (%d) checking PSO for %s\n", ret, ldb_dn_get_linearized(msg->dn)); } -- 2.35.0 From 903d345a055653d323c8b3cbbdbba6702f035ea0 Mon Sep 17 00:00:00 2001 From: Gary Lockyer Date: Thu, 25 Mar 2021 11:30:59 +1300 Subject: [PATCH 18/41] CVE-2021-20251 auth4: Return only the result message and free the surrounding result BUG: https://bugzilla.samba.org/show_bug.cgi?id=14611 Signed-off-by: Andrew Bartlett Reviewed-by: Joseph Sutton Reviewed-by: Andreas Schneider (cherry picked from commit b954acfde258a1909ed60c1c3e1015701582719f) --- selftest/knownfail.d/auth-sam | 1 - source4/auth/sam.c | 3 ++- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/selftest/knownfail.d/auth-sam b/selftest/knownfail.d/auth-sam index 048459e6555..438cea46415 100644 --- a/selftest/knownfail.d/auth-sam +++ b/selftest/knownfail.d/auth-sam @@ -1,4 +1,3 @@ -^samba.unittests.auth.sam.test_reread_account_not_locked.none ^samba.unittests.auth.sam.test_success_accounting_add_control_failed.none ^samba.unittests.auth.sam.test_success_accounting_build_mod_req_failed.none ^samba.unittests.auth.sam.test_success_accounting_commit_failed.none diff --git a/source4/auth/sam.c b/source4/auth/sam.c index b12a7c981e6..69e50e9da18 100644 --- a/source4/auth/sam.c +++ b/source4/auth/sam.c @@ -885,7 +885,8 @@ NTSTATUS authsam_reread_user_logon_data( TALLOC_FREE(res); return NT_STATUS_ACCOUNT_LOCKED_OUT; } - *current = res->msgs[0]; + *current = talloc_steal(mem_ctx, res->msgs[0]); + TALLOC_FREE(res); return NT_STATUS_OK; } -- 2.35.0 From 209514578533fb9bc123c7c1f2bea6aecbc68bc1 Mon Sep 17 00:00:00 2001 From: Andrew Bartlett Date: Thu, 25 Mar 2021 14:42:39 +1300 Subject: [PATCH 19/41] CVE-2021-20251 auth4: Split authsam_calculate_lastlogon_sync_interval() out authsam_calculate_lastlogon_sync_interval() is split out of authsam_update_lastlogon_timestamp() Based on work by Gary Lockyer BUG: https://bugzilla.samba.org/show_bug.cgi?id=14611 Signed-off-by: Andrew Bartlett Reviewed-by: Joseph Sutton Reviewed-by: Andreas Schneider (cherry picked from commit 55147335aec8194b6439169b040556a96db22e95) --- source4/auth/sam.c | 115 ++++++++++++++++++++++++++++----------------- 1 file changed, 73 insertions(+), 42 deletions(-) diff --git a/source4/auth/sam.c b/source4/auth/sam.c index 69e50e9da18..9e4da42632d 100644 --- a/source4/auth/sam.c +++ b/source4/auth/sam.c @@ -1247,40 +1247,36 @@ error: } -static NTSTATUS authsam_update_lastlogon_timestamp(struct ldb_context *sam_ctx, - struct ldb_message *msg_mod, - struct ldb_dn *domain_dn, - NTTIME old_timestamp, - NTTIME now) +/* + * msDS-LogonTimeSyncInterval is an int32_t number of days. + * + * The docs say: "the initial update, after the domain functional + * level is raised to DS_BEHAVIOR_WIN2003 or higher, is calculated as + * 14 days minus a random percentage of 5 days", but we aren't doing + * that. The blogosphere seems to think that this randomised update + * happens everytime, but [MS-ADA1] doesn't agree. + * + * Dochelp referred us to the following blog post: + * http://blogs.technet.com/b/askds/archive/2009/04/15/the-lastlogontimestamp-attribute-what-it-was-designed-for-and-how-it-works.aspx + * + * when msDS-LogonTimeSyncInterval is zero, the lastLogonTimestamp is + * not changed. + */ + +static NTSTATUS authsam_calculate_lastlogon_sync_interval( + struct ldb_context *sam_ctx, + TALLOC_CTX *ctx, + struct ldb_dn *domain_dn, + NTTIME *sync_interval_nt) { - /* - * We only set lastLogonTimestamp if the current value is older than - * now - msDS-LogonTimeSyncInterval days. - * - * msDS-LogonTimeSyncInterval is an int32_t number of days, while - * lastLogonTimestamp is in the 64 bit 100ns NTTIME format. - * - * The docs say: "the initial update, after the domain functional - * level is raised to DS_BEHAVIOR_WIN2003 or higher, is calculated as - * 14 days minus a random percentage of 5 days", but we aren't doing - * that. The blogosphere seems to think that this randomised update - * happens everytime, but [MS-ADA1] doesn't agree. - * - * Dochelp referred us to the following blog post: - * http://blogs.technet.com/b/askds/archive/2009/04/15/the-lastlogontimestamp-attribute-what-it-was-designed-for-and-how-it-works.aspx - * - * en msDS-LogonTimeSyncInterval is zero, the lastLogonTimestamp is - * not changed. - */ static const char *attrs[] = { "msDS-LogonTimeSyncInterval", NULL }; int ret; struct ldb_result *domain_res = NULL; TALLOC_CTX *mem_ctx = NULL; - int32_t sync_interval; - NTTIME sync_interval_nt; + uint32_t sync_interval; - mem_ctx = talloc_new(msg_mod); + mem_ctx = talloc_new(ctx); if (mem_ctx == NULL) { return NT_STATUS_NO_MEMORY; } @@ -1296,15 +1292,7 @@ static NTSTATUS authsam_update_lastlogon_timestamp(struct ldb_context *sam_ctx, "msDS-LogonTimeSyncInterval", 14); DEBUG(5, ("sync interval is %d\n", sync_interval)); - if (sync_interval == 0){ - /* - * Setting msDS-LogonTimeSyncInterval to zero is how you ask - * that nothing happens here. - */ - TALLOC_FREE(mem_ctx); - return NT_STATUS_OK; - } - else if (sync_interval >= 5){ + if (sync_interval >= 5){ /* * Subtract "a random percentage of 5" days. Presumably this * percentage is between 0 and 100, and modulus is accurate @@ -1312,17 +1300,47 @@ static NTSTATUS authsam_update_lastlogon_timestamp(struct ldb_context *sam_ctx, */ uint32_t r = generate_random() % 6; sync_interval -= r; - DEBUG(5, ("randomised sync interval is %d (-%d)\n", sync_interval, r)); + DBG_INFO("randomised sync interval is %d (-%d)\n", sync_interval, r); } /* In the case where sync_interval < 5 there is no randomisation */ - sync_interval_nt = sync_interval * 24LL * 3600LL * 10000000LL; + /* + * msDS-LogonTimeSyncInterval is an int32_t number of days, + * while lastLogonTimestamp (to be updated) is in the 64 bit + * 100ns NTTIME format so we must convert. + */ + *sync_interval_nt = sync_interval * 24LL * 3600LL * 10000000LL; + TALLOC_FREE(mem_ctx); + return NT_STATUS_OK; +} + +/* + * We only set lastLogonTimestamp if the current value is older than + * now - msDS-LogonTimeSyncInterval days. + * + * lastLogonTimestamp is in the 64 bit 100ns NTTIME format + */ +static NTSTATUS authsam_update_lastlogon_timestamp(struct ldb_context *sam_ctx, + struct ldb_message *msg_mod, + struct ldb_dn *domain_dn, + NTTIME old_timestamp, + NTTIME now, + NTTIME sync_interval_nt) +{ + int ret; DEBUG(5, ("old timestamp is %lld, threshold %lld, diff %lld\n", (long long int)old_timestamp, (long long int)(now - sync_interval_nt), (long long int)(old_timestamp - now + sync_interval_nt))); + if (sync_interval_nt == 0){ + /* + * Setting msDS-LogonTimeSyncInterval to zero is how you ask + * that nothing happens here. + */ + return NT_STATUS_OK; + } if (old_timestamp > now){ DEBUG(0, ("lastLogonTimestamp is in the future! (%lld > %lld)\n", (long long int)old_timestamp, (long long int)now)); @@ -1337,11 +1355,9 @@ static NTSTATUS authsam_update_lastlogon_timestamp(struct ldb_context *sam_ctx, "lastLogonTimestamp", now); if (ret != LDB_SUCCESS) { - TALLOC_FREE(mem_ctx); return NT_STATUS_NO_MEMORY; } } - TALLOC_FREE(mem_ctx); return NT_STATUS_OK; } @@ -1550,8 +1566,23 @@ get_transaction: } if (!am_rodc) { - status = authsam_update_lastlogon_timestamp(sam_ctx, msg_mod, domain_dn, - lastLogonTimestamp, now); + NTTIME sync_interval_nt; + + status = authsam_calculate_lastlogon_sync_interval( + sam_ctx, mem_ctx, domain_dn, &sync_interval_nt); + + if (!NT_STATUS_IS_OK(status)) { + status = NT_STATUS_INTERNAL_ERROR; + goto error; + } + + status = authsam_update_lastlogon_timestamp( + sam_ctx, + msg_mod, + domain_dn, + lastLogonTimestamp, + now, + sync_interval_nt); if (!NT_STATUS_IS_OK(status)) { status = NT_STATUS_NO_MEMORY; goto error; -- 2.35.0 From e12313d4333d61a49ac061041f8bc4ae02b62b33 Mon Sep 17 00:00:00 2001 From: Andrew Bartlett Date: Thu, 25 Mar 2021 15:33:08 +1300 Subject: [PATCH 20/41] CVE-2021-20251 auth4: Inline samdb_result_effective_badPwdCount() in authsam_logon_success_accounting() By bringing this function inline it can then be split out in a subsequent commit. Based on work by Gary Lockyer BUG: https://bugzilla.samba.org/show_bug.cgi?id=14611 Signed-off-by: Andrew Bartlett Reviewed-by: Joseph Sutton Reviewed-by: Andreas Schneider (cherry picked from commit 712181032a47318576ef35f6a6cf0f958aa538fb) --- source4/auth/sam.c | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/source4/auth/sam.c b/source4/auth/sam.c index 9e4da42632d..698324e5cc0 100644 --- a/source4/auth/sam.c +++ b/source4/auth/sam.c @@ -1479,11 +1479,17 @@ get_transaction: lockoutTime = ldb_msg_find_attr_as_int64(msg, "lockoutTime", 0); dbBadPwdCount = ldb_msg_find_attr_as_int(msg, "badPwdCount", 0); + tv_now = timeval_current(); + now = timeval_to_nttime(&tv_now); + if (interactive_or_kerberos) { badPwdCount = dbBadPwdCount; } else { - badPwdCount = samdb_result_effective_badPwdCount(sam_ctx, mem_ctx, - domain_dn, msg); + int64_t lockOutObservationWindow = + samdb_result_msds_LockoutObservationWindow( + sam_ctx, mem_ctx, domain_dn, msg); + badPwdCount = dsdb_effective_badPwdCount( + msg, lockOutObservationWindow, now); } lastLogonTimestamp = ldb_msg_find_attr_as_int64(msg, "lastLogonTimestamp", 0); @@ -1521,9 +1527,6 @@ get_transaction: } } - tv_now = timeval_current(); - now = timeval_to_nttime(&tv_now); - if (interactive_or_kerberos || (badPwdCount != 0 && lockoutTime == 0)) { ret = samdb_msg_add_int64(sam_ctx, msg_mod, msg_mod, -- 2.35.0 From b287cf6b911ce096faa769daf4f66ceda708b076 Mon Sep 17 00:00:00 2001 From: Andrew Bartlett Date: Tue, 30 Mar 2021 16:48:31 +1300 Subject: [PATCH 21/41] CVE-2021-20251 auth4: Avoid reading the database twice by precaculating some variables These variables are not important to protect against a race with and a double-read can easily be avoided by moving them up the file a little. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14611 Signed-off-by: Andrew Bartlett Reviewed-by: Joseph Sutton Reviewed-by: Andreas Schneider (cherry picked from commit b5f78b7b895a6b92cfdc9221b18d67ab18bc2a24) --- selftest/knownfail.d/auth-sam | 12 -------- source4/auth/sam.c | 55 +++++++++++++++++++++++------------ 2 files changed, 36 insertions(+), 31 deletions(-) delete mode 100644 selftest/knownfail.d/auth-sam diff --git a/selftest/knownfail.d/auth-sam b/selftest/knownfail.d/auth-sam deleted file mode 100644 index 438cea46415..00000000000 --- a/selftest/knownfail.d/auth-sam +++ /dev/null @@ -1,12 +0,0 @@ -^samba.unittests.auth.sam.test_success_accounting_add_control_failed.none -^samba.unittests.auth.sam.test_success_accounting_build_mod_req_failed.none -^samba.unittests.auth.sam.test_success_accounting_commit_failed.none -^samba.unittests.auth.sam.test_success_accounting_ldb_msg_new_failed.none -^samba.unittests.auth.sam.test_success_accounting_ldb_request_failed.none -^samba.unittests.auth.sam.test_success_accounting_ldb_wait_failed.none -^samba.unittests.auth.sam.test_success_accounting_reread_failed.none -^samba.unittests.auth.sam.test_success_accounting_rollback_failed.none -^samba.unittests.auth.sam.test_success_accounting_samdb_rodc_failed.none -^samba.unittests.auth.sam.test_success_accounting_spurious_bad_pwd_indicator.none -^samba.unittests.auth.sam.test_success_accounting_start_txn_failed.none -^samba.unittests.auth.sam.test_success_accounting_update_lastlogon_failed.none diff --git a/source4/auth/sam.c b/source4/auth/sam.c index 698324e5cc0..219ee10d5bd 100644 --- a/source4/auth/sam.c +++ b/source4/auth/sam.c @@ -1408,6 +1408,8 @@ NTSTATUS authsam_logon_success_accounting(struct ldb_context *sam_ctx, struct timeval tv_now; NTTIME now; NTTIME lastLogonTimestamp; + int64_t lockOutObservationWindow; + NTTIME sync_interval_nt = 0; bool am_rodc = false; bool txn_active = false; bool need_db_reread; @@ -1436,6 +1438,36 @@ NTSTATUS authsam_logon_success_accounting(struct ldb_context *sam_ctx, return status; } + if (interactive_or_kerberos == false) { + /* + * Avoid calculating this twice, it reads the PSO. A + * race on this is unimportant. + */ + lockOutObservationWindow + = samdb_result_msds_LockoutObservationWindow( + sam_ctx, mem_ctx, domain_dn, msg); + } + + ret = samdb_rodc(sam_ctx, &am_rodc); + if (ret != LDB_SUCCESS) { + status = NT_STATUS_INTERNAL_ERROR; + goto error; + } + + if (!am_rodc) { + /* + * Avoid reading the main domain DN twice. A race on + * this is unimportant. + */ + status = authsam_calculate_lastlogon_sync_interval( + sam_ctx, mem_ctx, domain_dn, &sync_interval_nt); + + if (!NT_STATUS_IS_OK(status)) { + status = NT_STATUS_INTERNAL_ERROR; + goto error; + } + } + get_transaction: if (need_db_reread) { @@ -1485,9 +1517,10 @@ get_transaction: if (interactive_or_kerberos) { badPwdCount = dbBadPwdCount; } else { - int64_t lockOutObservationWindow = - samdb_result_msds_LockoutObservationWindow( - sam_ctx, mem_ctx, domain_dn, msg); + /* + * We get lockOutObservationWindow above, before the + * transaction + */ badPwdCount = dsdb_effective_badPwdCount( msg, lockOutObservationWindow, now); } @@ -1562,23 +1595,7 @@ get_transaction: } } - ret = samdb_rodc(sam_ctx, &am_rodc); - if (ret != LDB_SUCCESS) { - status = NT_STATUS_INTERNAL_ERROR; - goto error; - } - if (!am_rodc) { - NTTIME sync_interval_nt; - - status = authsam_calculate_lastlogon_sync_interval( - sam_ctx, mem_ctx, domain_dn, &sync_interval_nt); - - if (!NT_STATUS_IS_OK(status)) { - status = NT_STATUS_INTERNAL_ERROR; - goto error; - } - status = authsam_update_lastlogon_timestamp( sam_ctx, msg_mod, -- 2.35.0 From 62d592a3f19875e4786a1d3b30f77a39cc811509 Mon Sep 17 00:00:00 2001 From: Joseph Sutton Date: Sat, 9 Jul 2022 15:53:51 +1200 Subject: [PATCH 22/41] CVE-2021-20251 s4-auth: Pass through error code from badPwdCount update The error code may be NT_STATUS_ACCOUNT_LOCKED_OUT, which we use in preference to NT_STATUS_WRONG_PASSWORD. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14611 Signed-off-by: Joseph Sutton Reviewed-by: Andreas Schneider Reviewed-by: Andrew Bartlett (cherry picked from commit d8a862cb811489abb67d4cf3a7fbd83d05c7e5cb) --- selftest/knownfail_heimdal_kdc | 1 - selftest/knownfail_mit_kdc | 1 - source4/auth/ntlm/auth_sam.c | 6 +++++- 3 files changed, 5 insertions(+), 3 deletions(-) diff --git a/selftest/knownfail_heimdal_kdc b/selftest/knownfail_heimdal_kdc index c0335bf4b52..eefa125b70f 100644 --- a/selftest/knownfail_heimdal_kdc +++ b/selftest/knownfail_heimdal_kdc @@ -58,4 +58,3 @@ # Lockout tests # ^samba.tests.krb5.lockout_tests.samba.tests.krb5.lockout_tests.LockoutTests.test_lockout_race_kdc.ad_dc:local -^samba.tests.krb5.lockout_tests.samba.tests.krb5.lockout_tests.LockoutTests.test_lockout_race_ntlm.ad_dc:local diff --git a/selftest/knownfail_mit_kdc b/selftest/knownfail_mit_kdc index 0af1f02142d..9b6baa4bd71 100644 --- a/selftest/knownfail_mit_kdc +++ b/selftest/knownfail_mit_kdc @@ -445,7 +445,6 @@ samba.tests.krb5.as_canonicalization_tests.samba.tests.krb5.as_canonicalization_ ^samba.tests.krb5.lockout_tests.samba.tests.krb5.lockout_tests.LockoutTests.test_bad_pwd_count_transaction_kdc.ad_dc:local ^samba.tests.krb5.lockout_tests.samba.tests.krb5.lockout_tests.LockoutTests.test_bad_pwd_count_transaction_rename_kdc.ad_dc:local ^samba.tests.krb5.lockout_tests.samba.tests.krb5.lockout_tests.LockoutTests.test_lockout_race_kdc.ad_dc:local -^samba.tests.krb5.lockout_tests.samba.tests.krb5.lockout_tests.LockoutTests.test_lockout_race_ntlm.ad_dc:local ^samba.tests.krb5.lockout_tests.samba.tests.krb5.lockout_tests.LockoutTests.test_lockout_transaction_bad_pwd_kdc.ad_dc:local ^samba.tests.krb5.lockout_tests.samba.tests.krb5.lockout_tests.LockoutTests.test_lockout_transaction_kdc.ad_dc:local ^samba.tests.krb5.lockout_tests.samba.tests.krb5.lockout_tests.LockoutTests.test_lockout_transaction_rename_kdc.ad_dc:local diff --git a/source4/auth/ntlm/auth_sam.c b/source4/auth/ntlm/auth_sam.c index 1077762c70e..882d92e26ed 100644 --- a/source4/auth/ntlm/auth_sam.c +++ b/source4/auth/ntlm/auth_sam.c @@ -716,7 +716,11 @@ static NTSTATUS authsam_password_check_and_record(struct auth4_context *auth_con } TALLOC_FREE(tmp_ctx); - return NT_STATUS_WRONG_PASSWORD; + + if (NT_STATUS_IS_OK(nt_status)) { + nt_status = NT_STATUS_WRONG_PASSWORD; + } + return nt_status; } static NTSTATUS authsam_authenticate(struct auth4_context *auth_context, -- 2.35.0 From 7471d20096e089cca8d1100c2cd28966e9ef8270 Mon Sep 17 00:00:00 2001 From: Joseph Sutton Date: Sat, 9 Jul 2022 15:44:21 +1200 Subject: [PATCH 23/41] CVE-2021-20251 s4:dsdb: Update bad password count inside transaction Previously, there was a gap between calling dsdb_update_bad_pwd_count() and dsdb_module_modify() where no transaction was in effect. Another process could slip in and modify badPwdCount, only for our update to immediately overwrite it. Doing the update inside the transaction will help for the following commit when we make it atomic. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14611 Signed-off-by: Joseph Sutton Reviewed-by: Andreas Schneider Reviewed-by: Andrew Bartlett (cherry picked from commit a65147a9e98ead70869cdfa20ffcc9c167dbf535) --- .../dsdb/samdb/ldb_modules/password_hash.c | 37 ++++++++++--------- 1 file changed, 19 insertions(+), 18 deletions(-) diff --git a/source4/dsdb/samdb/ldb_modules/password_hash.c b/source4/dsdb/samdb/ldb_modules/password_hash.c index e90482df63e..b9f7fb77e6f 100644 --- a/source4/dsdb/samdb/ldb_modules/password_hash.c +++ b/source4/dsdb/samdb/ldb_modules/password_hash.c @@ -2733,24 +2733,6 @@ static int make_error_and_update_badPwdCount(struct setup_password_fields_io *io int ret; /* The errors we will actually return */ int dbg_ret; /* The errors we can only complain about in logs */ - /* PSO search result is optional (NULL if no PSO applies) */ - if (io->ac->pso_res != NULL) { - pso_msg = io->ac->pso_res->message; - } - - status = dsdb_update_bad_pwd_count(io->ac, ldb, - io->ac->search_res->message, - io->ac->dom_res->message, - pso_msg, - &mod_msg); - if (!NT_STATUS_IS_OK(status)) { - goto done; - } - - if (mod_msg == NULL) { - goto done; - } - /* * OK, horrible semantics ahead. * @@ -2793,6 +2775,24 @@ static int make_error_and_update_badPwdCount(struct setup_password_fields_io *io goto done; } + /* PSO search result is optional (NULL if no PSO applies) */ + if (io->ac->pso_res != NULL) { + pso_msg = io->ac->pso_res->message; + } + + status = dsdb_update_bad_pwd_count(io->ac, ldb, + io->ac->search_res->message, + io->ac->dom_res->message, + pso_msg, + &mod_msg); + if (!NT_STATUS_IS_OK(status)) { + goto end_transaction; + } + + if (mod_msg == NULL) { + goto end_transaction; + } + dbg_ret = dsdb_module_modify(io->ac->module, mod_msg, DSDB_FLAG_NEXT_MODULE, io->ac->req); @@ -2806,6 +2806,7 @@ static int make_error_and_update_badPwdCount(struct setup_password_fields_io *io */ } +end_transaction: dbg_ret = ldb_next_end_trans(io->ac->module); if (dbg_ret != LDB_SUCCESS) { ldb_debug(ldb, LDB_DEBUG_ERROR, -- 2.35.0 From 558f77c1d376bbe75b6fc9def26183856527ee8e Mon Sep 17 00:00:00 2001 From: Joseph Sutton Date: Sat, 9 Jul 2022 15:54:12 +1200 Subject: [PATCH 24/41] CVE-2021-20251 s4:dsdb: Make badPwdCount update atomic We reread the account details inside the transaction in case the account has been locked out in the meantime. If it has, we return the appropriate error code. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14611 Signed-off-by: Joseph Sutton Reviewed-by: Andreas Schneider Reviewed-by: Andrew Bartlett (cherry picked from commit 96479747bdb5bc5f33d903085f5f69793f369e3a) --- selftest/flapping.d/ldap-pwd-change-race | 5 ---- .../dsdb/samdb/ldb_modules/password_hash.c | 27 ++++++++++++++++--- 2 files changed, 24 insertions(+), 8 deletions(-) delete mode 100644 selftest/flapping.d/ldap-pwd-change-race diff --git a/selftest/flapping.d/ldap-pwd-change-race b/selftest/flapping.d/ldap-pwd-change-race deleted file mode 100644 index 54ed56c1134..00000000000 --- a/selftest/flapping.d/ldap-pwd-change-race +++ /dev/null @@ -1,5 +0,0 @@ -# This test currently depends on a race. The password_hash dsdb module -# relinquishes and immediately reacquires a transaction lock, and another -# process may be able to acquire it during the short period of time in which it -# is not held. -^samba.tests.krb5.lockout_tests.samba.tests.krb5.lockout_tests.LockoutTests.test_lockout_race_ldap_pw_change.ad_dc:local diff --git a/source4/dsdb/samdb/ldb_modules/password_hash.c b/source4/dsdb/samdb/ldb_modules/password_hash.c index b9f7fb77e6f..b308226a9f9 100644 --- a/source4/dsdb/samdb/ldb_modules/password_hash.c +++ b/source4/dsdb/samdb/ldb_modules/password_hash.c @@ -45,6 +45,7 @@ #include "lib/crypto/md4.h" #include "param/param.h" #include "lib/krb5_wrap/krb5_samba.h" +#include "auth/auth_sam.h" #include "auth/common_auth.h" #include "lib/messaging/messaging.h" #include "lib/param/loadparm.h" @@ -2729,7 +2730,8 @@ static int make_error_and_update_badPwdCount(struct setup_password_fields_io *io struct ldb_context *ldb = ldb_module_get_ctx(io->ac->module); struct ldb_message *mod_msg = NULL; struct ldb_message *pso_msg = NULL; - NTSTATUS status; + struct ldb_message *current = NULL; + NTSTATUS status = NT_STATUS_OK; int ret; /* The errors we will actually return */ int dbg_ret; /* The errors we can only complain about in logs */ @@ -2775,13 +2777,28 @@ static int make_error_and_update_badPwdCount(struct setup_password_fields_io *io goto done; } + /* + * Re-read the account details, using the GUID in case the DN + * is being changed. + */ + status = authsam_reread_user_logon_data( + ldb, io->ac, + io->ac->search_res->message, + ¤t); + if (!NT_STATUS_IS_OK(status)) { + /* The re-read can return account locked out, as well + * as an internal error + */ + goto end_transaction; + } + /* PSO search result is optional (NULL if no PSO applies) */ if (io->ac->pso_res != NULL) { pso_msg = io->ac->pso_res->message; } status = dsdb_update_bad_pwd_count(io->ac, ldb, - io->ac->search_res->message, + current, io->ac->dom_res->message, pso_msg, &mod_msg); @@ -2831,7 +2848,11 @@ end_transaction: done: ret = LDB_ERR_CONSTRAINT_VIOLATION; - *werror = WERR_INVALID_PASSWORD; + if (NT_STATUS_EQUAL(status, NT_STATUS_ACCOUNT_LOCKED_OUT)) { + *werror = WERR_ACCOUNT_LOCKED_OUT; + } else { + *werror = WERR_INVALID_PASSWORD; + } ldb_asprintf_errstring(ldb, "%08X: %s - check_password_restrictions: " "The old password specified doesn't match!", -- 2.35.0 From c3e056d476ea8ecb4f34277ed25e9dbc32f020f0 Mon Sep 17 00:00:00 2001 From: Joseph Sutton Date: Wed, 6 Jul 2022 11:11:43 +1200 Subject: [PATCH 25/41] CVE-2021-20251 s4:kdc: Move logon success accounting code into existing branch This simplifies the code for the following commit. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14611 Signed-off-by: Joseph Sutton Reviewed-by: Andreas Schneider Reviewed-by: Andrew Bartlett (cherry picked from commit 2b593c34c4f5cb82440b940766e53626c1cbec5b) --- source4/kdc/hdb-samba4.c | 33 +++++++++++++-------------------- 1 file changed, 13 insertions(+), 20 deletions(-) diff --git a/source4/kdc/hdb-samba4.c b/source4/kdc/hdb-samba4.c index 13c3a8bd265..dcfbb07f9fb 100644 --- a/source4/kdc/hdb-samba4.c +++ b/source4/kdc/hdb-samba4.c @@ -601,26 +601,6 @@ static krb5_error_code hdb_samba4_audit(krb5_context context, } switch (hdb_auth_status) { - case KDC_AUTH_EVENT_CLIENT_AUTHORIZED: - { - TALLOC_CTX *frame = talloc_stackframe(); - struct samba_kdc_entry *p = talloc_get_type(entry->context, - struct samba_kdc_entry); - struct netr_SendToSamBase *send_to_sam = NULL; - - /* - * TODO: We could log the AS-REQ authorization success here as - * well. However before we do that, we need to pass - * in the PAC here or re-calculate it. - */ - authsam_logon_success_accounting(kdc_db_ctx->samdb, p->msg, - domain_dn, true, &send_to_sam); - if (kdc_db_ctx->rodc && send_to_sam != NULL) { - reset_bad_password_netlogon(frame, kdc_db_ctx, send_to_sam); - } - talloc_free(frame); - } - FALL_THROUGH; default: { TALLOC_CTX *frame = talloc_stackframe(); @@ -662,6 +642,19 @@ static krb5_error_code hdb_samba4_audit(krb5_context context, ui.auth_description = auth_description; if (hdb_auth_status == KDC_AUTH_EVENT_CLIENT_AUTHORIZED) { + struct netr_SendToSamBase *send_to_sam = NULL; + + /* + * TODO: We could log the AS-REQ authorization success here as + * well. However before we do that, we need to pass + * in the PAC here or re-calculate it. + */ + authsam_logon_success_accounting(kdc_db_ctx->samdb, p->msg, + domain_dn, true, &send_to_sam); + if (kdc_db_ctx->rodc && send_to_sam != NULL) { + reset_bad_password_netlogon(frame, kdc_db_ctx, send_to_sam); + } + /* This is the final sucess */ status = NT_STATUS_OK; } else if (hdb_auth_status == KDC_AUTH_EVENT_VALIDATED_LONG_TERM_KEY) { -- 2.35.0 From 98da8c19f686e3e24840fefd1e06b17e1444ea10 Mon Sep 17 00:00:00 2001 From: Joseph Sutton Date: Fri, 1 Jul 2022 15:04:41 +1200 Subject: [PATCH 26/41] CVE-2021-20251 s4:kdc: Check return status of authsam_logon_success_accounting() If we find that the user has been locked out sometime during the request (due to a race), we will now return an error code. Note that we cannot avoid the MIT KDC aspect of the issue by checking the return status of mit_samba_zero_bad_password_count(), because kdb_vftabl::audit_as_req() returning void means we cannot pass on the result. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14611 Signed-off-by: Joseph Sutton Reviewed-by: Andreas Schneider Reviewed-by: Andrew Bartlett (cherry picked from commit b1e740896ebae14ba64250da2f718e1d707e9eed) --- source4/kdc/hdb-samba4.c | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/source4/kdc/hdb-samba4.c b/source4/kdc/hdb-samba4.c index dcfbb07f9fb..7ac5842bc4d 100644 --- a/source4/kdc/hdb-samba4.c +++ b/source4/kdc/hdb-samba4.c @@ -649,14 +649,21 @@ static krb5_error_code hdb_samba4_audit(krb5_context context, * well. However before we do that, we need to pass * in the PAC here or re-calculate it. */ - authsam_logon_success_accounting(kdc_db_ctx->samdb, p->msg, - domain_dn, true, &send_to_sam); - if (kdc_db_ctx->rodc && send_to_sam != NULL) { + status = authsam_logon_success_accounting(kdc_db_ctx->samdb, p->msg, + domain_dn, true, &send_to_sam); + if (NT_STATUS_EQUAL(status, NT_STATUS_ACCOUNT_LOCKED_OUT)) { + final_ret = KRB5KDC_ERR_CLIENT_REVOKED; + r->error_code = final_ret; + rwdc_fallback = kdc_db_ctx->rodc; + } else if (!NT_STATUS_IS_OK(status)) { + final_ret = KRB5KRB_ERR_GENERIC; + r->error_code = final_ret; + rwdc_fallback = kdc_db_ctx->rodc; + } else if (kdc_db_ctx->rodc && send_to_sam != NULL) { reset_bad_password_netlogon(frame, kdc_db_ctx, send_to_sam); } /* This is the final sucess */ - status = NT_STATUS_OK; } else if (hdb_auth_status == KDC_AUTH_EVENT_VALIDATED_LONG_TERM_KEY) { /* * This was only a pre-authentication success, -- 2.35.0 From de7b800c7d85246924183f0450af9ba24b9062c6 Mon Sep 17 00:00:00 2001 From: Joseph Sutton Date: Sat, 9 Jul 2022 15:54:52 +1200 Subject: [PATCH 27/41] CVE-2021-20251 s4:kdc: Check badPwdCount update return status If the account has been locked out in the meantime (indicated by NT_STATUS_ACCOUNT_LOCKED_OUT), we should return the appropriate error code. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14611 Signed-off-by: Joseph Sutton Reviewed-by: Andreas Schneider Reviewed-by: Andrew Bartlett (cherry picked from commit bdfc9d96f8fe5070ab8a189bbf42ccb7e77afb73) [jsutton@samba.org Fixed knownfail conflicts due to not having claims tests] --- selftest/knownfail_heimdal_kdc | 4 ---- source4/kdc/hdb-samba4.c | 9 +++++++-- 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/selftest/knownfail_heimdal_kdc b/selftest/knownfail_heimdal_kdc index eefa125b70f..4ae27eacb09 100644 --- a/selftest/knownfail_heimdal_kdc +++ b/selftest/knownfail_heimdal_kdc @@ -54,7 +54,3 @@ ^samba.tests.krb5.protected_users_tests.samba.tests.krb5.protected_users_tests.ProtectedUsersTests.test_proxiable_as_protected.ad_dc # ^samba.tests.krb5.protected_users_tests.samba.tests.krb5.protected_users_tests.ProtectedUsersTests.test_samr_change_password_protected.ad_dc -# -# Lockout tests -# -^samba.tests.krb5.lockout_tests.samba.tests.krb5.lockout_tests.LockoutTests.test_lockout_race_kdc.ad_dc:local diff --git a/source4/kdc/hdb-samba4.c b/source4/kdc/hdb-samba4.c index 7ac5842bc4d..96bb3d858f0 100644 --- a/source4/kdc/hdb-samba4.c +++ b/source4/kdc/hdb-samba4.c @@ -703,8 +703,13 @@ static krb5_error_code hdb_samba4_audit(krb5_context context, } else if (hdb_auth_status == KDC_AUTH_EVENT_CLIENT_TIME_SKEW) { status = NT_STATUS_TIME_DIFFERENCE_AT_DC; } else if (hdb_auth_status == KDC_AUTH_EVENT_WRONG_LONG_TERM_KEY) { - authsam_update_bad_pwd_count(kdc_db_ctx->samdb, p->msg, domain_dn); - status = NT_STATUS_WRONG_PASSWORD; + status = authsam_update_bad_pwd_count(kdc_db_ctx->samdb, p->msg, domain_dn); + if (NT_STATUS_EQUAL(status, NT_STATUS_ACCOUNT_LOCKED_OUT)) { + final_ret = KRB5KDC_ERR_CLIENT_REVOKED; + r->error_code = final_ret; + } else { + status = NT_STATUS_WRONG_PASSWORD; + } rwdc_fallback = kdc_db_ctx->rodc; } else if (hdb_auth_status == KDC_AUTH_EVENT_CLIENT_LOCKED_OUT) { status = NT_STATUS_ACCOUNT_LOCKED_OUT; -- 2.35.0 From 0ef6dd027b8a3238670850acf4c20dbc3fc6130e Mon Sep 17 00:00:00 2001 From: Joseph Sutton Date: Sat, 9 Jul 2022 15:55:02 +1200 Subject: [PATCH 28/41] CVE-2021-20251 s4-rpc_server: Check badPwdCount update return status If the account has been locked out in the meantime (indicated by NT_STATUS_ACCOUNT_LOCKED_OUT), we should return the appropriate error code. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14611 Signed-off-by: Joseph Sutton Reviewed-by: Andreas Schneider Reviewed-by: Andrew Bartlett (cherry picked from commit a268a1a0e304d0702469e4ac146d8af5e7384c39) --- source4/rpc_server/samr/samr_password.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/source4/rpc_server/samr/samr_password.c b/source4/rpc_server/samr/samr_password.c index 09d7501d474..e55d5c28b5c 100644 --- a/source4/rpc_server/samr/samr_password.c +++ b/source4/rpc_server/samr/samr_password.c @@ -485,7 +485,11 @@ failed: /* Only update the badPwdCount if we found the user */ if (NT_STATUS_EQUAL(status, NT_STATUS_WRONG_PASSWORD)) { - authsam_update_bad_pwd_count(sam_ctx, msg, ldb_get_default_basedn(sam_ctx)); + NTSTATUS bad_pwd_status = authsam_update_bad_pwd_count( + sam_ctx, msg, ldb_get_default_basedn(sam_ctx)); + if (NT_STATUS_EQUAL(bad_pwd_status, NT_STATUS_ACCOUNT_LOCKED_OUT)) { + status = bad_pwd_status; + } } else if (NT_STATUS_EQUAL(status, NT_STATUS_NO_SUCH_USER)) { /* Don't give the game away: (don't allow anonymous users to prove the existence of usernames) */ status = NT_STATUS_WRONG_PASSWORD; -- 2.35.0 From 7a11535c8ab19df5385d3ccf5571f312b68d9374 Mon Sep 17 00:00:00 2001 From: Joseph Sutton Date: Mon, 4 Jul 2022 20:51:38 +1200 Subject: [PATCH 29/41] CVE-2021-20251 s4:auth_winbind: Check return status of authsam_logon_success_accounting() This may return an error if we find the account is locked out. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14611 Signed-off-by: Joseph Sutton Reviewed-by: Andreas Schneider Reviewed-by: Andrew Bartlett (cherry picked from commit 268ea7bef5af4b9c8a02f4f5856113ff0664d9e8) --- source4/auth/ntlm/auth_winbind.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/source4/auth/ntlm/auth_winbind.c b/source4/auth/ntlm/auth_winbind.c index d7879966603..6381f866667 100644 --- a/source4/auth/ntlm/auth_winbind.c +++ b/source4/auth/ntlm/auth_winbind.c @@ -252,11 +252,14 @@ static void winbind_check_password_done(struct tevent_req *subreq) status = authsam_search_account(state, ctx->auth_ctx->sam_ctx, nt4_account, domain_dn, &msg); if (NT_STATUS_IS_OK(status)) { - authsam_logon_success_accounting( + status = authsam_logon_success_accounting( ctx->auth_ctx->sam_ctx, msg, domain_dn, user_info->flags & USER_INFO_INTERACTIVE_LOGON, NULL); + if (tevent_req_nterror(req, status)) { + return; + } } } -- 2.35.0 From f95b51a176b226f93d3ad69b50da58d59f81ae87 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Mon, 11 Jan 2021 12:11:35 -0800 Subject: [PATCH 30/41] CVE-2021-20251 s3: ensure bad password count atomic updates The bad password count is supposed to limit the number of failed login attempt a user can make before being temporarily locked out, but race conditions between processes have allowed determined attackers to make many more than the specified number of attempts. This is especially bad on constrained or overcommitted hardware. To fix this, once a bad password is detected, we reload the sam account information under a user-specific mutex, ensuring we have an up to date bad password count. Discovered by Nathaniel W. Turner. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14611 Signed-off-by: Jeremy Allison Reviewed-by: Andreas Schneider Reviewed-by: Andrew Bartlett (cherry picked from commit 8587734bf989aeaafa9d09d78d0f381caf52d285) --- source3/auth/check_samsec.c | 77 +++++++++++++++++++++++++++++++++++++ 1 file changed, 77 insertions(+) diff --git a/source3/auth/check_samsec.c b/source3/auth/check_samsec.c index b9563c958a9..bd8ca8de2f0 100644 --- a/source3/auth/check_samsec.c +++ b/source3/auth/check_samsec.c @@ -379,6 +379,8 @@ NTSTATUS check_sam_security(const DATA_BLOB *challenge, const uint8_t *nt_pw; const uint8_t *lm_pw; uint32_t acct_ctrl; + char *mutex_name_by_user = NULL; + struct named_mutex *mtx = NULL; /* the returned struct gets kept on the server_info, by means of a steal further down */ @@ -418,6 +420,79 @@ NTSTATUS check_sam_security(const DATA_BLOB *challenge, challenge, lm_pw, nt_pw, user_info, &user_sess_key, &lm_sess_key); + /* + * We must re-load the sam acount information under a mutex + * lock to ensure we don't miss any concurrent account lockout + * changes. + */ + + /* Clear out old sampass info. */ + TALLOC_FREE(sampass); + acct_ctrl = 0; + username = NULL; + nt_pw = NULL; + lm_pw = NULL; + + sampass = samu_new(mem_ctx); + if (sampass == NULL) { + return NT_STATUS_NO_MEMORY; + } + + mutex_name_by_user = talloc_asprintf(mem_ctx, + "check_sam_security_mutex_%s", + user_info->mapped.account_name); + if (mutex_name_by_user == NULL) { + nt_status = NT_STATUS_NO_MEMORY; + goto done; + } + + /* Grab the named mutex under root with 30 second timeout. */ + become_root(); + mtx = grab_named_mutex(mem_ctx, mutex_name_by_user, 30); + if (mtx != NULL) { + /* Re-load the account information if we got the mutex. */ + ret = pdb_getsampwnam(sampass, user_info->mapped.account_name); + } + unbecome_root(); + + /* Everything from here on until mtx is freed is done under the mutex.*/ + + if (mtx == NULL) { + DBG_ERR("Acquisition of mutex %s failed " + "for user %s\n", + mutex_name_by_user, + user_info->mapped.account_name); + nt_status = NT_STATUS_INTERNAL_ERROR; + goto done; + } + + if (!ret) { + /* + * Re-load of account failed. This could only happen if the + * user was deleted in the meantime. + */ + DBG_NOTICE("reload of user '%s' in passdb failed.\n", + user_info->mapped.account_name); + nt_status = NT_STATUS_NO_SUCH_USER; + goto done; + } + + /* Re-load the account control info. */ + acct_ctrl = pdb_get_acct_ctrl(sampass); + username = pdb_get_username(sampass); + + /* + * Check if the account is now locked out - now under the mutex. + * This can happen if the server is under + * a password guess attack and the ACB_AUTOLOCK is set by + * another process. + */ + if (acct_ctrl & ACB_AUTOLOCK) { + DBG_NOTICE("Account for user %s was locked out.\n", username); + nt_status = NT_STATUS_ACCOUNT_LOCKED_OUT; + goto done; + } + /* Notify passdb backend of login success/failure. If not NT_STATUS_OK the backend doesn't like the login */ @@ -510,6 +585,8 @@ done: TALLOC_FREE(sampass); data_blob_free(&user_sess_key); data_blob_free(&lm_sess_key); + TALLOC_FREE(mutex_name_by_user); + TALLOC_FREE(mtx); return nt_status; } -- 2.35.0 From 76febc0a2817fd2ebbd056f380fb62b2cec82bcd Mon Sep 17 00:00:00 2001 From: Joseph Sutton Date: Tue, 5 Jul 2022 20:17:33 +1200 Subject: [PATCH 31/41] CVE-2021-20251 s3: Ensure bad password count atomic updates for SAMR password change The bad password count is supposed to limit the number of failed login attempt a user can make before being temporarily locked out, but race conditions between processes have allowed determined attackers to make many more than the specified number of attempts. This is especially bad on constrained or overcommitted hardware. To fix this, once a bad password is detected, we reload the sam account information under a user-specific mutex, ensuring we have an up to date bad password count. Derived from a similar patch to source3/auth/check_samsec.c by Jeremy Allison BUG: https://bugzilla.samba.org/show_bug.cgi?id=14611 Signed-off-by: Joseph Sutton Reviewed-by: Andreas Schneider Reviewed-by: Andrew Bartlett (cherry picked from commit 65c473d4a53fc8a22a0d531aff45203ea3a4d99b) --- source3/rpc_server/samr/srv_samr_chgpasswd.c | 81 ++++++++++++++++++-- 1 file changed, 75 insertions(+), 6 deletions(-) diff --git a/source3/rpc_server/samr/srv_samr_chgpasswd.c b/source3/rpc_server/samr/srv_samr_chgpasswd.c index f1d833a630c..9662284eac6 100644 --- a/source3/rpc_server/samr/srv_samr_chgpasswd.c +++ b/source3/rpc_server/samr/srv_samr_chgpasswd.c @@ -1205,6 +1205,8 @@ NTSTATUS pass_oem_change(char *user, const char *rhost, bool ret = false; bool updated_badpw = false; NTSTATUS update_login_attempts_status; + char *mutex_name_by_user = NULL; + struct named_mutex *mtx = NULL; if (!(sampass = samu_new(NULL))) { return NT_STATUS_NO_MEMORY; @@ -1216,15 +1218,15 @@ NTSTATUS pass_oem_change(char *user, const char *rhost, if (ret == false) { DEBUG(0,("pass_oem_change: getsmbpwnam returned NULL\n")); - TALLOC_FREE(sampass); - return NT_STATUS_NO_SUCH_USER; + nt_status = NT_STATUS_NO_SUCH_USER; + goto done; } /* Quit if the account was locked out. */ if (pdb_get_acct_ctrl(sampass) & ACB_AUTOLOCK) { DEBUG(3,("check_sam_security: Account for user %s was locked out.\n", user)); - TALLOC_FREE(sampass); - return NT_STATUS_ACCOUNT_LOCKED_OUT; + nt_status = NT_STATUS_ACCOUNT_LOCKED_OUT; + goto done; } nt_status = check_oem_password(user, @@ -1235,6 +1237,71 @@ NTSTATUS pass_oem_change(char *user, const char *rhost, sampass, &new_passwd); + /* + * We must re-load the sam acount information under a mutex + * lock to ensure we don't miss any concurrent account lockout + * changes. + */ + + /* Clear out old sampass info. */ + TALLOC_FREE(sampass); + + sampass = samu_new(NULL); + if (sampass == NULL) { + return NT_STATUS_NO_MEMORY; + } + + mutex_name_by_user = talloc_asprintf(NULL, + "check_sam_security_mutex_%s", + user); + if (mutex_name_by_user == NULL) { + nt_status = NT_STATUS_NO_MEMORY; + goto done; + } + + /* Grab the named mutex under root with 30 second timeout. */ + become_root(); + mtx = grab_named_mutex(NULL, mutex_name_by_user, 30); + if (mtx != NULL) { + /* Re-load the account information if we got the mutex. */ + ret = pdb_getsampwnam(sampass, user); + } + unbecome_root(); + + /* Everything from here on until mtx is freed is done under the mutex.*/ + + if (mtx == NULL) { + DBG_ERR("Acquisition of mutex %s failed " + "for user %s\n", + mutex_name_by_user, + user); + nt_status = NT_STATUS_INTERNAL_ERROR; + goto done; + } + + if (!ret) { + /* + * Re-load of account failed. This could only happen if the + * user was deleted in the meantime. + */ + DBG_NOTICE("reload of user '%s' in passdb failed.\n", + user); + nt_status = NT_STATUS_NO_SUCH_USER; + goto done; + } + + /* + * Check if the account is now locked out - now under the mutex. + * This can happen if the server is under + * a password guess attack and the ACB_AUTOLOCK is set by + * another process. + */ + if (pdb_get_acct_ctrl(sampass) & ACB_AUTOLOCK) { + DBG_NOTICE("Account for user %s was locked out.\n", user); + nt_status = NT_STATUS_ACCOUNT_LOCKED_OUT; + goto done; + } + /* * Notify passdb backend of login success/failure. If not * NT_STATUS_OK the backend doesn't like the login @@ -1282,8 +1349,7 @@ NTSTATUS pass_oem_change(char *user, const char *rhost, } if (!NT_STATUS_IS_OK(nt_status)) { - TALLOC_FREE(sampass); - return nt_status; + goto done; } /* We've already checked the old password here.... */ @@ -1294,7 +1360,10 @@ NTSTATUS pass_oem_change(char *user, const char *rhost, memset(new_passwd, 0, strlen(new_passwd)); +done: TALLOC_FREE(sampass); + TALLOC_FREE(mutex_name_by_user); + TALLOC_FREE(mtx); return nt_status; } -- 2.35.0 From 89e1c927546d4e660bd6e9d0948774f7cbbc65fa Mon Sep 17 00:00:00 2001 From: Joseph Sutton Date: Tue, 2 Aug 2022 14:35:33 +1200 Subject: [PATCH 32/41] lib:util: Check memset_s() error code in talloc_keep_secret_destructor() Panic if memset_s() fails. Signed-off-by: Joseph Sutton Reviewed-by: Andreas Schneider Reviewed-by: Andrew Bartlett (cherry picked from commit 03a50d8f7d872b6ef701d1207061c88b73d171bb) --- lib/util/talloc_keep_secret.c | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/lib/util/talloc_keep_secret.c b/lib/util/talloc_keep_secret.c index 70d449b5f12..21048659e5d 100644 --- a/lib/util/talloc_keep_secret.c +++ b/lib/util/talloc_keep_secret.c @@ -22,13 +22,26 @@ static int talloc_keep_secret_destructor(void *ptr) { + int ret; size_t size = talloc_get_size(ptr); if (unlikely(size == 0)) { return 0; } - memset_s(ptr, size, 0, size); + ret = memset_s(ptr, size, 0, size); + if (unlikely(ret != 0)) { + char *msg = NULL; + int ret2; + ret2 = asprintf(&msg, + "talloc_keep_secret_destructor: memset_s() failed: %s", + strerror(ret)); + if (ret2 != -1) { + smb_panic(msg); + } else { + smb_panic("talloc_keep_secret_destructor: memset_s() failed"); + } + } return 0; } -- 2.35.0 From 9f8f8ec4c466eb29896fb60934883db602053bf9 Mon Sep 17 00:00:00 2001 From: Joseph Sutton Date: Tue, 2 Aug 2022 14:35:50 +1200 Subject: [PATCH 33/41] libcli:auth: Keep passwords from convert_string_talloc() secret Signed-off-by: Joseph Sutton Reviewed-by: Andreas Schneider Reviewed-by: Andrew Bartlett (cherry picked from commit 6edf88f5c40421b9881666a2e78038ea9c547c24) --- libcli/auth/smbencrypt.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/libcli/auth/smbencrypt.c b/libcli/auth/smbencrypt.c index 7abf6613d80..8492202ea93 100644 --- a/libcli/auth/smbencrypt.c +++ b/libcli/auth/smbencrypt.c @@ -975,6 +975,7 @@ bool decode_pw_buffer(TALLOC_CTX *ctx, DBG_ERR("Failed to convert incoming password\n"); return false; } + talloc_keep_secret(*pp_new_pwrd); #ifdef DEBUG_PASSWORD DEBUG(100,("decode_pw_buffer: new_pwrd: ")); @@ -1067,6 +1068,7 @@ bool decode_pwd_string_from_buffer514(TALLOC_CTX *mem_ctx, if (!ok) { return false; } + talloc_keep_secret(decoded_password->data); return true; } -- 2.35.0 From f7ca489d9a1c7edc6a8fb0a7ecd08c69f9b0f4be Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pavel=20Filipensk=C3=BD?= Date: Mon, 8 Aug 2022 17:47:28 +0200 Subject: [PATCH 34/41] lib:replace: Add macro BURN_STR() to zero memory of a string MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Pavel Filipenský Reviewed-by: Andreas Schneider (cherry picked from commit 8564380346ace981b957bb8464f2ecf007032062) --- lib/replace/replace.h | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/lib/replace/replace.h b/lib/replace/replace.h index da5526c4a2f..bd7f6e53e81 100644 --- a/lib/replace/replace.h +++ b/lib/replace/replace.h @@ -864,6 +864,17 @@ typedef unsigned long long ptrdiff_t ; */ #define BURN_PTR_SIZE(x, s) memset_s((x), (s), 0, (s)) +/** + * Explicitly zero data in string. This is guaranteed to be not optimized + * away. + */ +#define BURN_STR(x) do { \ + if ((x) != NULL) { \ + size_t s = strlen(x); \ + memset_s((x), s, 0, s); \ + } \ + } while(0) + /** * Work out how many elements there are in a static array. */ -- 2.35.0 From 9969ade07f30f0a5efb2b055ccb6d097171cee85 Mon Sep 17 00:00:00 2001 From: Joseph Sutton Date: Tue, 6 Sep 2022 14:54:08 +1200 Subject: [PATCH 35/41] s3:rpc_server: Use BURN_STR() to zero password This ensures these calls are not optimised away. Signed-off-by: Joseph Sutton Reviewed-by: Andreas Schneider Reviewed-by: Andrew Bartlett (cherry picked from commit 1258746ba85b8702628f95a19aba9afea96eab8b) --- source3/rpc_server/samr/srv_samr_chgpasswd.c | 2 +- source3/rpc_server/samr/srv_samr_nt.c | 6 ++---- 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/source3/rpc_server/samr/srv_samr_chgpasswd.c b/source3/rpc_server/samr/srv_samr_chgpasswd.c index 9662284eac6..d720641f05c 100644 --- a/source3/rpc_server/samr/srv_samr_chgpasswd.c +++ b/source3/rpc_server/samr/srv_samr_chgpasswd.c @@ -1358,7 +1358,7 @@ NTSTATUS pass_oem_change(char *user, const char *rhost, True, reject_reason); unbecome_root(); - memset(new_passwd, 0, strlen(new_passwd)); + BURN_STR(new_passwd); done: TALLOC_FREE(sampass); diff --git a/source3/rpc_server/samr/srv_samr_nt.c b/source3/rpc_server/samr/srv_samr_nt.c index ac40cbc418b..14bff5ecd7a 100644 --- a/source3/rpc_server/samr/srv_samr_nt.c +++ b/source3/rpc_server/samr/srv_samr_nt.c @@ -4946,9 +4946,7 @@ static NTSTATUS set_user_info_23(TALLOC_CTX *mem_ctx, } } - if (plaintext_buf) { - memset(plaintext_buf, '\0', strlen(plaintext_buf)); - } + BURN_STR(plaintext_buf); if (IS_SAM_CHANGED(pwd, PDB_GROUPSID) && (!NT_STATUS_IS_OK(status = pdb_set_unix_primary_group(mem_ctx, @@ -5018,7 +5016,7 @@ static bool set_user_info_pw(uint8_t *pass, const char *rhost, struct samu *pwd) } } - memset(plaintext_buf, '\0', strlen(plaintext_buf)); + BURN_STR(plaintext_buf); DEBUG(5,("set_user_info_pw: pdb_update_pwd()\n")); -- 2.35.0 From 3b02a3a8d0901d00f5d57f6dcc9db493ee86a638 Mon Sep 17 00:00:00 2001 From: Joseph Sutton Date: Tue, 2 Aug 2022 14:37:52 +1200 Subject: [PATCH 36/41] CVE-2021-20251 s4-rpc_server: Use authsam_search_account() to find the user This helps the bad password and audit log handling code as it allows assumptions to be made about the attributes found in the variable "msg", such as that DSDB_SEARCH_SHOW_EXTENDED_DN was used. This ensures we can re-search on the DN via the embedded GUID, which in in turn rename-proof. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14611 Signed-off-by: Joseph Sutton Reviewed-by: Andreas Schneider Reviewed-by: Andrew Bartlett (cherry picked from commit fabbea25310a31c0409b1c11eaced39bd8cde8dd) --- source4/rpc_server/samr/samr_password.c | 47 ++++++++----------------- 1 file changed, 15 insertions(+), 32 deletions(-) diff --git a/source4/rpc_server/samr/samr_password.c b/source4/rpc_server/samr/samr_password.c index e55d5c28b5c..904e5fba6c5 100644 --- a/source4/rpc_server/samr/samr_password.c +++ b/source4/rpc_server/samr/samr_password.c @@ -119,16 +119,7 @@ NTSTATUS dcesrv_samr_ChangePasswordUser4(struct dcesrv_call_state *dce_call, { #ifdef HAVE_GNUTLS_PBKDF2 struct ldb_context *sam_ctx = NULL; - struct ldb_message **ldb_res = NULL; - const char *const attrs[] = {"unicodePwd", - "dBCSPwd", - "userAccountControl", - "msDS-ResultantPSO", - "msDS-User-Account-Control-Computed", - "badPwdCount", - "badPasswordTime", - "objectSid", - NULL}; + struct ldb_message *msg = NULL; struct ldb_dn *dn = NULL; const char *samAccountName = NULL; struct dom_sid *objectSid = NULL; @@ -178,35 +169,27 @@ NTSTATUS dcesrv_samr_ChangePasswordUser4(struct dcesrv_call_state *dce_call, } /* - * We need the users dn and the domain dn (derived from the user SID). - * We also need the current lm and nt password hashes in order to - * decrypt the incoming passwords. + * We use authsam_search_account() to be consistent with the + * other callers in the bad password and audit log handling + * systems. It ensures we get DSDB_SEARCH_SHOW_EXTENDED_DN. */ - rc = gendb_search( - sam_ctx, - mem_ctx, - NULL, - &ldb_res, - attrs, - "(&(sAMAccountName=%s)(objectclass=user))", - ldb_binary_encode_string(mem_ctx, r->in.account->string)); - if (rc != 1) { - /* Will be converted to WRONG_PASSWORD below */ + status = authsam_search_account(mem_ctx, + sam_ctx, + r->in.account->string, + ldb_get_default_basedn(sam_ctx), + &msg); + if (!NT_STATUS_IS_OK(status)) { ldb_transaction_cancel(sam_ctx); - DBG_WARNING("Unable to find account: %s", - r->in.account->string); - status = NT_STATUS_NO_SUCH_USER; goto done; } - dn = ldb_res[0]->dn; - samAccountName = - ldb_msg_find_attr_as_string(ldb_res[0], "samAccountName", NULL); - objectSid = samdb_result_dom_sid(ldb_res, ldb_res[0], "objectSid"); + dn = msg->dn; + samAccountName = ldb_msg_find_attr_as_string(msg, "samAccountName", NULL); + objectSid = samdb_result_dom_sid(msg, msg, "objectSid"); status = samdb_result_passwords(mem_ctx, dce_call->conn->dce_ctx->lp_ctx, - ldb_res[0], + msg, &nt_pwd); if (!NT_STATUS_IS_OK(status)) { ldb_transaction_cancel(sam_ctx); @@ -282,7 +265,7 @@ done: /* Only update the badPwdCount if we found the user */ if (NT_STATUS_EQUAL(status, NT_STATUS_WRONG_PASSWORD)) { authsam_update_bad_pwd_count(sam_ctx, - ldb_res[0], + msg, ldb_get_default_basedn(sam_ctx)); } else if (NT_STATUS_EQUAL(status, NT_STATUS_NO_SUCH_USER)) { /* -- 2.35.0 From 28c0374cf794eff3388f9244efff44d9f71ce94c Mon Sep 17 00:00:00 2001 From: Joseph Sutton Date: Tue, 2 Aug 2022 14:39:06 +1200 Subject: [PATCH 37/41] CVE-2021-20251 s4-rpc_server: Use user privileges for SAMR password change We don't (and shouldn't) need system prvileges to perform the password change, so drop to the privileges of the user by setting DSDB_SESSION_INFO. We need to reuse the same sam_ctx: creating a new one with only user privileges would not work, because any database modifications would be blocked by the transaction taken out on the original context. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14611 Signed-off-by: Joseph Sutton Reviewed-by: Andreas Schneider Reviewed-by: Andrew Bartlett (cherry picked from commit f74f92aea164af40d9177b332778a76d7ecabcbd) --- source4/rpc_server/samr/dcesrv_samr.c | 9 +++++--- source4/rpc_server/samr/samr_password.c | 28 ++++++++++++++++++++++--- 2 files changed, 31 insertions(+), 6 deletions(-) diff --git a/source4/rpc_server/samr/dcesrv_samr.c b/source4/rpc_server/samr/dcesrv_samr.c index f9a6f16e33f..b1342cbfe84 100644 --- a/source4/rpc_server/samr/dcesrv_samr.c +++ b/source4/rpc_server/samr/dcesrv_samr.c @@ -4091,7 +4091,8 @@ static NTSTATUS dcesrv_samr_SetUserInfo(struct dcesrv_call_state *dce_call, TALL sam_ctx, a_state->account_dn, a_state->domain_state->domain_dn, - &r->in.info->info31.password); + &r->in.info->info31.password, + DSDB_PASSWORD_RESET); if (!NT_STATUS_IS_OK(status)) { goto done; } @@ -4244,7 +4245,8 @@ static NTSTATUS dcesrv_samr_SetUserInfo(struct dcesrv_call_state *dce_call, TALL a_state->sam_ctx, a_state->account_dn, a_state->domain_state->domain_dn, - &r->in.info->info32.password); + &r->in.info->info32.password, + DSDB_PASSWORD_RESET); } else IFSET(SAMR_FIELD_LM_PASSWORD_PRESENT) { @@ -4255,7 +4257,8 @@ static NTSTATUS dcesrv_samr_SetUserInfo(struct dcesrv_call_state *dce_call, TALL a_state->sam_ctx, a_state->account_dn, a_state->domain_state->domain_dn, - &r->in.info->info32.password); + &r->in.info->info32.password, + DSDB_PASSWORD_RESET); } if (!NT_STATUS_IS_OK(status)) { goto done; diff --git a/source4/rpc_server/samr/samr_password.c b/source4/rpc_server/samr/samr_password.c index 904e5fba6c5..c6cca985d8e 100644 --- a/source4/rpc_server/samr/samr_password.c +++ b/source4/rpc_server/samr/samr_password.c @@ -134,6 +134,8 @@ NTSTATUS dcesrv_samr_ChangePasswordUser4(struct dcesrv_call_state *dce_call, .data = cdk_data, .length = sizeof(cdk_data), }; + struct auth_session_info *call_session_info = NULL; + struct auth_session_info *old_session_info = NULL; NTSTATUS status = NT_STATUS_WRONG_PASSWORD; int rc; @@ -219,14 +221,33 @@ NTSTATUS dcesrv_samr_ChangePasswordUser4(struct dcesrv_call_state *dce_call, goto done; } + /* Drop to user privileges for the password change */ + + old_session_info = ldb_get_opaque(sam_ctx, DSDB_SESSION_INFO); + call_session_info = dcesrv_call_session_info(dce_call); + + rc = ldb_set_opaque(sam_ctx, DSDB_SESSION_INFO, call_session_info); + if (rc != LDB_SUCCESS) { + ldb_transaction_cancel(sam_ctx); + status = NT_STATUS_INVALID_SYSTEM_SERVICE; + goto done; + } + status = samr_set_password_aes(dce_call, mem_ctx, &cdk, sam_ctx, dn, NULL, - r->in.password); + r->in.password, + DSDB_PASSWORD_CHECKED_AND_CORRECT); BURN_DATA(cdk_data); + + /* Restore our privileges to system level */ + if (old_session_info != NULL) { + ldb_set_opaque(sam_ctx, DSDB_SESSION_INFO, old_session_info); + } + if (!NT_STATUS_IS_OK(status)) { ldb_transaction_cancel(sam_ctx); status = NT_STATUS_WRONG_PASSWORD; @@ -741,7 +762,8 @@ NTSTATUS samr_set_password_aes(struct dcesrv_call_state *dce_call, struct ldb_context *sam_ctx, struct ldb_dn *account_dn, struct ldb_dn *domain_dn, - struct samr_EncryptedPasswordAES *pwbuf) + struct samr_EncryptedPasswordAES *pwbuf, + enum dsdb_password_checked old_password_checked) { DATA_BLOB pw_data = data_blob_null; DATA_BLOB new_password = data_blob_null; @@ -779,7 +801,7 @@ NTSTATUS samr_set_password_aes(struct dcesrv_call_state *dce_call, domain_dn, &new_password, NULL, - DSDB_PASSWORD_RESET, + old_password_checked, NULL, NULL); TALLOC_FREE(new_password.data); -- 2.35.0 From 9b64629c9f368554d1fdcdb7886d9dab7c05096e Mon Sep 17 00:00:00 2001 From: Joseph Sutton Date: Tue, 2 Aug 2022 14:39:43 +1200 Subject: [PATCH 38/41] CVE-2021-20251 s4-rpc_server: Extend scope of transaction for ChangePasswordUser3 Now the initial account search is performed under the transaction, ensuring the overall password change is atomic. We set DSDB_SESSION_INFO to drop our privileges to those of the user before we perform the actual password change, and restore them afterwards if we need to update the bad password count. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14611 Signed-off-by: Joseph Sutton Reviewed-by: Andreas Schneider Reviewed-by: Andrew Bartlett (cherry picked from commit fcabcb326d385c1e1daaa8dae9820e33a3868f56) --- source4/rpc_server/samr/samr_password.c | 42 +++++++++++++++++++------ 1 file changed, 33 insertions(+), 9 deletions(-) diff --git a/source4/rpc_server/samr/samr_password.c b/source4/rpc_server/samr/samr_password.c index c6cca985d8e..4691f9a47a9 100644 --- a/source4/rpc_server/samr/samr_password.c +++ b/source4/rpc_server/samr/samr_password.c @@ -330,6 +330,8 @@ NTSTATUS dcesrv_samr_ChangePasswordUser3(struct dcesrv_call_state *dce_call, = lpcfg_ntlm_auth(lp_ctx); gnutls_cipher_hd_t cipher_hnd = NULL; gnutls_datum_t nt_session_key; + struct auth_session_info *call_session_info = NULL; + struct auth_session_info *old_session_info = NULL; int rc; *r->out.dominfo = NULL; @@ -354,6 +356,12 @@ NTSTATUS dcesrv_samr_ChangePasswordUser3(struct dcesrv_call_state *dce_call, return NT_STATUS_INVALID_SYSTEM_SERVICE; } + ret = ldb_transaction_start(sam_ctx); + if (ret != LDB_SUCCESS) { + DEBUG(1, ("Failed to start transaction: %s\n", ldb_errstring(sam_ctx))); + return NT_STATUS_TRANSACTION_ABORTED; + } + /* * We use authsam_search_account() to be consistent with the * other callers in the bad password and audit log handling @@ -365,6 +373,7 @@ NTSTATUS dcesrv_samr_ChangePasswordUser3(struct dcesrv_call_state *dce_call, ldb_get_default_basedn(sam_ctx), &msg); if (!NT_STATUS_IS_OK(status)) { + ldb_transaction_cancel(sam_ctx); goto failed; } @@ -375,11 +384,13 @@ NTSTATUS dcesrv_samr_ChangePasswordUser3(struct dcesrv_call_state *dce_call, status = samdb_result_passwords(mem_ctx, lp_ctx, msg, &nt_pwd); if (!NT_STATUS_IS_OK(status) ) { + ldb_transaction_cancel(sam_ctx); goto failed; } if (!nt_pwd) { status = NT_STATUS_WRONG_PASSWORD; + ldb_transaction_cancel(sam_ctx); goto failed; } @@ -395,6 +406,7 @@ NTSTATUS dcesrv_samr_ChangePasswordUser3(struct dcesrv_call_state *dce_call, NULL); if (rc < 0) { status = gnutls_error_to_ntstatus(rc, NT_STATUS_CRYPTO_SYSTEM_INVALID); + ldb_transaction_cancel(sam_ctx); goto failed; } @@ -404,17 +416,20 @@ NTSTATUS dcesrv_samr_ChangePasswordUser3(struct dcesrv_call_state *dce_call, gnutls_cipher_deinit(cipher_hnd); if (rc < 0) { status = gnutls_error_to_ntstatus(rc, NT_STATUS_CRYPTO_SYSTEM_INVALID); + ldb_transaction_cancel(sam_ctx); goto failed; } if (!extract_pw_from_buffer(mem_ctx, r->in.nt_password->data, &new_password)) { DEBUG(3,("samr: failed to decode password buffer\n")); status = NT_STATUS_WRONG_PASSWORD; + ldb_transaction_cancel(sam_ctx); goto failed; } if (r->in.nt_verifier == NULL) { status = NT_STATUS_WRONG_PASSWORD; + ldb_transaction_cancel(sam_ctx); goto failed; } @@ -424,23 +439,25 @@ NTSTATUS dcesrv_samr_ChangePasswordUser3(struct dcesrv_call_state *dce_call, rc = E_old_pw_hash(new_nt_hash, nt_pwd->hash, nt_verifier.hash); if (rc != 0) { status = gnutls_error_to_ntstatus(rc, NT_STATUS_ACCESS_DISABLED_BY_POLICY_OTHER); + ldb_transaction_cancel(sam_ctx); goto failed; } if (!mem_equal_const_time(nt_verifier.hash, r->in.nt_verifier->hash, 16)) { status = NT_STATUS_WRONG_PASSWORD; + ldb_transaction_cancel(sam_ctx); goto failed; } - /* Connect to a SAMDB with user privileges for the password change */ - sam_ctx = dcesrv_samdb_connect_as_user(mem_ctx, dce_call); - if (sam_ctx == NULL) { - return NT_STATUS_INVALID_SYSTEM_SERVICE; - } + /* Drop to user privileges for the password change */ - ret = ldb_transaction_start(sam_ctx); + old_session_info = ldb_get_opaque(sam_ctx, DSDB_SESSION_INFO); + call_session_info = dcesrv_call_session_info(dce_call); + + ret = ldb_set_opaque(sam_ctx, DSDB_SESSION_INFO, call_session_info); if (ret != LDB_SUCCESS) { - DEBUG(1, ("Failed to start transaction: %s\n", ldb_errstring(sam_ctx))); - return NT_STATUS_TRANSACTION_ABORTED; + status = NT_STATUS_INVALID_SYSTEM_SERVICE; + ldb_transaction_cancel(sam_ctx); + goto failed; } /* Performs the password modification. We pass the old hashes read out @@ -454,6 +471,11 @@ NTSTATUS dcesrv_samr_ChangePasswordUser3(struct dcesrv_call_state *dce_call, &reason, &dominfo); + /* Restore our privileges to system level */ + if (old_session_info != NULL) { + ldb_set_opaque(sam_ctx, DSDB_SESSION_INFO, old_session_info); + } + if (!NT_STATUS_IS_OK(status)) { ldb_transaction_cancel(sam_ctx); goto failed; @@ -489,7 +511,9 @@ failed: /* Only update the badPwdCount if we found the user */ if (NT_STATUS_EQUAL(status, NT_STATUS_WRONG_PASSWORD)) { - NTSTATUS bad_pwd_status = authsam_update_bad_pwd_count( + NTSTATUS bad_pwd_status; + + bad_pwd_status = authsam_update_bad_pwd_count( sam_ctx, msg, ldb_get_default_basedn(sam_ctx)); if (NT_STATUS_EQUAL(bad_pwd_status, NT_STATUS_ACCOUNT_LOCKED_OUT)) { status = bad_pwd_status; -- 2.35.0 From 1820b263f7f672349cae965ca22ec997ef0095ec Mon Sep 17 00:00:00 2001 From: Joseph Sutton Date: Tue, 2 Aug 2022 14:40:01 +1200 Subject: [PATCH 39/41] CVE-2021-20251 dsdb/common: Remove transaction logic from samdb_set_password() All of its callers, where necessary, take out a transaction covering the entire password set or change operation, so a transaction is no longer needed here. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14611 Signed-off-by: Joseph Sutton Reviewed-by: Andreas Schneider Reviewed-by: Andrew Bartlett (cherry picked from commit 7981cba87e3a7256b12bfc5fdd89b136c12979ff) --- source4/dsdb/common/util.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/source4/dsdb/common/util.c b/source4/dsdb/common/util.c index aff31ac8651..be0a2cd4a33 100644 --- a/source4/dsdb/common/util.c +++ b/source4/dsdb/common/util.c @@ -2414,7 +2414,10 @@ static NTSTATUS samdb_set_password_internal(struct ldb_context *ldb, TALLOC_CTX return NT_STATUS_NO_MEMORY; } - ret = dsdb_autotransaction_request(ldb, req); + ret = ldb_request(ldb, req); + if (ret == LDB_SUCCESS) { + ret = ldb_wait(req->handle, LDB_WAIT_ALL); + } if (req->context != NULL) { struct ldb_control *control = talloc_get_type_abort(req->context, -- 2.35.0 From 58548b7d04b194f031fcfee5676bbbb32b27f356 Mon Sep 17 00:00:00 2001 From: Joseph Sutton Date: Tue, 2 Aug 2022 14:43:09 +1200 Subject: [PATCH 40/41] CVE-2021-20251 s3:rpc_server: Split change_oem_password() call out of samr_set_password_aes() Now samr_set_password_aes() just returns the new password in a similar manner to check_oem_password(). This simplifies the logic for the following change to recheck whether the account is locked out, and to update the bad password count. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14611 Signed-off-by: Joseph Sutton Reviewed-by: Andreas Schneider Reviewed-by: Andrew Bartlett (cherry picked from commit 1d869a2a666cfada1495d891021de6c2b8567a96) --- source3/rpc_server/samr/srv_samr_chgpasswd.c | 36 +++++++------------- source3/rpc_server/samr/srv_samr_nt.c | 24 +++++++++---- source3/rpc_server/samr/srv_samr_util.h | 8 +++-- 3 files changed, 36 insertions(+), 32 deletions(-) diff --git a/source3/rpc_server/samr/srv_samr_chgpasswd.c b/source3/rpc_server/samr/srv_samr_chgpasswd.c index d720641f05c..8563beb48a7 100644 --- a/source3/rpc_server/samr/srv_samr_chgpasswd.c +++ b/source3/rpc_server/samr/srv_samr_chgpasswd.c @@ -1072,10 +1072,10 @@ NTSTATUS check_password_complexity(const char *username, is correct before calling. JRA. ************************************************************/ -static NTSTATUS change_oem_password(struct samu *hnd, const char *rhost, - char *old_passwd, char *new_passwd, - bool as_root, - enum samPwdChangeReason *samr_reject_reason) +NTSTATUS change_oem_password(struct samu *hnd, const char *rhost, + char *old_passwd, char *new_passwd, + bool as_root, + enum samPwdChangeReason *samr_reject_reason) { uint32_t min_len; uint32_t refuse; @@ -1369,21 +1369,20 @@ done: } NTSTATUS samr_set_password_aes(TALLOC_CTX *mem_ctx, - struct samu *sampass, - const char *rhost, const DATA_BLOB *cdk, struct samr_EncryptedPasswordAES *pwbuf, - enum samPwdChangeReason *reject_reason) + char **new_password_str) { DATA_BLOB pw_data = data_blob_null; DATA_BLOB new_password = data_blob_null; const DATA_BLOB ciphertext = data_blob_const(pwbuf->cipher, pwbuf->cipher_len); DATA_BLOB iv = data_blob_const(pwbuf->salt, sizeof(pwbuf->salt)); - char *new_password_str = NULL; NTSTATUS status; bool ok; + *new_password_str = NULL; + status = samba_gnutls_aead_aes_256_cbc_hmac_sha512_decrypt( mem_ctx, &ciphertext, @@ -1407,23 +1406,14 @@ NTSTATUS samr_set_password_aes(TALLOC_CTX *mem_ctx, return NT_STATUS_WRONG_PASSWORD; } - new_password_str = talloc_strndup(mem_ctx, - (char *)new_password.data, - new_password.length); + *new_password_str = talloc_strndup(mem_ctx, + (char *)new_password.data, + new_password.length); TALLOC_FREE(new_password.data); - if (new_password_str == NULL) { + if (*new_password_str == NULL) { return NT_STATUS_NO_MEMORY; } + talloc_keep_secret(*new_password_str); - become_root(); - status = change_oem_password(sampass, - rhost, - NULL, - new_password_str, - true, - reject_reason); - unbecome_root(); - TALLOC_FREE(new_password_str); - - return status; + return NT_STATUS_OK; } diff --git a/source3/rpc_server/samr/srv_samr_nt.c b/source3/rpc_server/samr/srv_samr_nt.c index 14bff5ecd7a..1036410f534 100644 --- a/source3/rpc_server/samr/srv_samr_nt.c +++ b/source3/rpc_server/samr/srv_samr_nt.c @@ -7681,7 +7681,6 @@ NTSTATUS _samr_ChangePasswordUser4(struct pipes_struct *p, struct dcesrv_connection *dcesrv_conn = dce_call->conn; const struct tsocket_address *remote_address = dcesrv_connection_get_remote_address(dcesrv_conn); - enum samPwdChangeReason reject_reason; char *rhost = NULL; struct samu *sampass = NULL; char *username = NULL; @@ -7697,6 +7696,7 @@ NTSTATUS _samr_ChangePasswordUser4(struct pipes_struct *p, .data = cdk_data, .length = sizeof(cdk_data), }; + char *new_passwd = NULL; NTSTATUS status = NT_STATUS_WRONG_PASSWORD; bool ok; int rc; @@ -7766,19 +7766,31 @@ NTSTATUS _samr_ChangePasswordUser4(struct pipes_struct *p, } status = samr_set_password_aes(frame, - sampass, - rhost, &cdk, r->in.password, - &reject_reason); + &new_passwd); BURN_DATA(cdk_data); - if (NT_STATUS_EQUAL(status, NT_STATUS_NO_SUCH_USER)) { - return NT_STATUS_WRONG_PASSWORD; + if (!NT_STATUS_IS_OK(status)) { + goto done; } + become_root(); + status = change_oem_password(sampass, + rhost, + NULL, + new_passwd, + true, + NULL); + unbecome_root(); + TALLOC_FREE(new_passwd); + done: TALLOC_FREE(frame); + if (NT_STATUS_EQUAL(status, NT_STATUS_NO_SUCH_USER)) { + return NT_STATUS_WRONG_PASSWORD; + } + return status; #else /* HAVE_GNUTLS_PBKDF2 */ p->fault_state = DCERPC_FAULT_OP_RNG_ERROR; diff --git a/source3/rpc_server/samr/srv_samr_util.h b/source3/rpc_server/samr/srv_samr_util.h index a702d5d449c..5e839ac77c0 100644 --- a/source3/rpc_server/samr/srv_samr_util.h +++ b/source3/rpc_server/samr/srv_samr_util.h @@ -69,6 +69,10 @@ void copy_pwd_expired_to_sam_passwd(struct samu *to, bool chgpasswd(const char *name, const char *rhost, const struct passwd *pass, const char *oldpass, const char *newpass, bool as_root); +NTSTATUS change_oem_password(struct samu *hnd, const char *rhost, + char *old_passwd, char *new_passwd, + bool as_root, + enum samPwdChangeReason *samr_reject_reason); NTSTATUS pass_oem_change(char *user, const char *rhost, uchar password_encrypted_with_lm_hash[516], const uchar old_lm_hash_encrypted[16], @@ -80,8 +84,6 @@ NTSTATUS check_password_complexity(const char *username, const char *password, enum samPwdChangeReason *samr_reject_reason); NTSTATUS samr_set_password_aes(TALLOC_CTX *mem_ctx, - struct samu *sampass, - const char *rhost, const DATA_BLOB *cdk, struct samr_EncryptedPasswordAES *pwbuf, - enum samPwdChangeReason *reject_reason); + char **new_password_str); -- 2.35.0 From 150264142437ba9df3d2d90dc2534bcd163fdfd1 Mon Sep 17 00:00:00 2001 From: Joseph Sutton Date: Tue, 2 Aug 2022 14:43:19 +1200 Subject: [PATCH 41/41] CVE-2021-20251 s3: Ensure bad password count atomic updates for SAMR AES password change The bad password count is supposed to limit the number of failed login attempt a user can make before being temporarily locked out, but race conditions between processes have allowed determined attackers to make many more than the specified number of attempts. This is especially bad on constrained or overcommitted hardware. To fix this, once a bad password is detected, we reload the sam account information under a user-specific mutex, ensuring we have an up to date bad password count. We also update the bad password count if the password is wrong, which we did not previously do. Derived from a similar patch to source3/auth/check_samsec.c by Jeremy Allison BUG: https://bugzilla.samba.org/show_bug.cgi?id=14611 Signed-off-by: Joseph Sutton Reviewed-by: Andreas Schneider Reviewed-by: Andrew Bartlett Autobuild-User(master): Andrew Bartlett Autobuild-Date(master): Tue Sep 13 00:08:07 UTC 2022 on sn-devel-184 (cherry picked from commit 8ae0c38d54f065915e927bbfe1b656400a79eb13) --- source3/rpc_server/samr/srv_samr_nt.c | 117 ++++++++++++++++++++++++++ 1 file changed, 117 insertions(+) diff --git a/source3/rpc_server/samr/srv_samr_nt.c b/source3/rpc_server/samr/srv_samr_nt.c index 1036410f534..5f93d4287ad 100644 --- a/source3/rpc_server/samr/srv_samr_nt.c +++ b/source3/rpc_server/samr/srv_samr_nt.c @@ -7697,6 +7697,10 @@ NTSTATUS _samr_ChangePasswordUser4(struct pipes_struct *p, .length = sizeof(cdk_data), }; char *new_passwd = NULL; + bool updated_badpw = false; + NTSTATUS update_login_attempts_status; + char *mutex_name_by_user = NULL; + struct named_mutex *mtx = NULL; NTSTATUS status = NT_STATUS_WRONG_PASSWORD; bool ok; int rc; @@ -7770,6 +7774,119 @@ NTSTATUS _samr_ChangePasswordUser4(struct pipes_struct *p, r->in.password, &new_passwd); BURN_DATA(cdk_data); + + /* + * We must re-load the sam acount information under a mutex + * lock to ensure we don't miss any concurrent account lockout + * changes. + */ + + /* Clear out old sampass info. */ + TALLOC_FREE(sampass); + + sampass = samu_new(frame); + if (sampass == NULL) { + status = NT_STATUS_NO_MEMORY; + goto done; + } + + mutex_name_by_user = talloc_asprintf(frame, + "check_sam_security_mutex_%s", + username); + if (mutex_name_by_user == NULL) { + status = NT_STATUS_NO_MEMORY; + goto done; + } + + /* Grab the named mutex under root with 30 second timeout. */ + become_root(); + mtx = grab_named_mutex(frame, mutex_name_by_user, 30); + if (mtx != NULL) { + /* Re-load the account information if we got the mutex. */ + ok = pdb_getsampwnam(sampass, username); + } + unbecome_root(); + + /* Everything from here on until mtx is freed is done under the mutex.*/ + + if (mtx == NULL) { + DBG_ERR("Acquisition of mutex %s failed " + "for user %s\n", + mutex_name_by_user, + username); + status = NT_STATUS_INTERNAL_ERROR; + goto done; + } + + if (!ok) { + /* + * Re-load of account failed. This could only happen if the + * user was deleted in the meantime. + */ + DBG_NOTICE("reload of user '%s' in passdb failed.\n", + username); + status = NT_STATUS_NO_SUCH_USER; + goto done; + } + + /* + * Check if the account is now locked out - now under the mutex. + * This can happen if the server is under + * a password guess attack and the ACB_AUTOLOCK is set by + * another process. + */ + if (pdb_get_acct_ctrl(sampass) & ACB_AUTOLOCK) { + DBG_NOTICE("Account for user %s was locked out.\n", username); + status = NT_STATUS_ACCOUNT_LOCKED_OUT; + goto done; + } + + /* + * Notify passdb backend of login success/failure. If not + * NT_STATUS_OK the backend doesn't like the login + */ + update_login_attempts_status = pdb_update_login_attempts( + sampass, NT_STATUS_IS_OK(status)); + + if (!NT_STATUS_IS_OK(status)) { + bool increment_bad_pw_count = false; + + if (NT_STATUS_EQUAL(status, NT_STATUS_WRONG_PASSWORD) && + (pdb_get_acct_ctrl(sampass) & ACB_NORMAL) && + NT_STATUS_IS_OK(update_login_attempts_status)) + { + increment_bad_pw_count = true; + } + + if (increment_bad_pw_count) { + pdb_increment_bad_password_count(sampass); + updated_badpw = true; + } else { + pdb_update_bad_password_count(sampass, + &updated_badpw); + } + } else { + if ((pdb_get_acct_ctrl(sampass) & ACB_NORMAL) && + (pdb_get_bad_password_count(sampass) > 0)) + { + pdb_set_bad_password_count(sampass, 0, PDB_CHANGED); + pdb_set_bad_password_time(sampass, 0, PDB_CHANGED); + updated_badpw = true; + } + } + + if (updated_badpw) { + NTSTATUS update_status; + become_root(); + update_status = pdb_update_sam_account(sampass); + unbecome_root(); + + if (!NT_STATUS_IS_OK(update_status)) { + DEBUG(1, ("Failed to modify entry: %s\n", + nt_errstr(update_status))); + } + } + if (!NT_STATUS_IS_OK(status)) { goto done; } -- 2.35.0