From a3e3d05f35d6082ea48450060b39084e3d0e4056 Mon Sep 17 00:00:00 2001 From: Andreas Schneider Date: Mon, 10 Oct 2022 15:15:20 +0200 Subject: [PATCH 1/5] s3:librpc: Improve GSE error message BUG: https://bugzilla.samba.org/show_bug.cgi?id=15206 Signed-off-by: Andreas Schneider Reviewed-by: Noel Power --- source3/librpc/crypto/gse.c | 21 +++++++++++++++++++-- 1 file changed, 19 insertions(+), 2 deletions(-) diff --git a/source3/librpc/crypto/gse.c b/source3/librpc/crypto/gse.c index c50a8a036df..c2cac7abf82 100644 --- a/source3/librpc/crypto/gse.c +++ b/source3/librpc/crypto/gse.c @@ -546,11 +546,28 @@ init_sec_context_done: goto done; case GSS_S_FAILURE: switch (gss_min) { - case (OM_uint32)KRB5KDC_ERR_S_PRINCIPAL_UNKNOWN: - DBG_NOTICE("Server principal not found\n"); + case (OM_uint32)KRB5KDC_ERR_S_PRINCIPAL_UNKNOWN: { + gss_buffer_desc name_token = { + .length = 0, + }; + + gss_maj = gss_display_name(&gss_min, + gse_ctx->server_name, + &name_token, + NULL); + if (gss_maj == GSS_S_COMPLETE) { + DBG_NOTICE("Server principal %.*s not found\n", + (int)name_token.length, + (char *)name_token.value); + gss_release_buffer(&gss_maj, &name_token); + } else { + DBG_NOTICE("Server principal not found\n"); + } + /* Make SPNEGO ignore us, we can't go any further here */ status = NT_STATUS_INVALID_PARAMETER; goto done; + } case (OM_uint32)KRB5KRB_AP_ERR_TKT_EXPIRED: DBG_NOTICE("Ticket expired\n"); /* Make SPNEGO ignore us, we can't go any further here */ -- 2.37.3 From d2e2e9acd717e45806f1b19378e09f39c8fe3da8 Mon Sep 17 00:00:00 2001 From: Andreas Schneider Date: Fri, 7 Oct 2022 14:35:15 +0200 Subject: [PATCH 2/5] s3:rpcclient: Pass salt down to init_samr_CryptPasswordAES() BUG: https://bugzilla.samba.org/show_bug.cgi?id=15206 Signed-off-by: Andreas Schneider Reviewed-by: Noel Power --- source3/rpc_client/init_samr.c | 15 ++++----------- source3/rpc_client/init_samr.h | 1 + source3/rpcclient/cmd_samr.c | 8 ++++++++ source4/libnet/libnet_passwd.c | 13 +++++++------ source4/torture/rpc/samr.c | 27 +++++++++++++++++++++++++++ 5 files changed, 47 insertions(+), 17 deletions(-) diff --git a/source3/rpc_client/init_samr.c b/source3/rpc_client/init_samr.c index 68f42b602b3..52fa2f90d6e 100644 --- a/source3/rpc_client/init_samr.c +++ b/source3/rpc_client/init_samr.c @@ -79,6 +79,7 @@ NTSTATUS init_samr_CryptPassword(const char *pwd, NTSTATUS init_samr_CryptPasswordAES(TALLOC_CTX *mem_ctx, const char *password, + DATA_BLOB *salt, DATA_BLOB *session_key, struct samr_EncryptedPasswordAES *ppwd_buf) { @@ -87,12 +88,6 @@ NTSTATUS init_samr_CryptPasswordAES(TALLOC_CTX *mem_ctx, .data = pw_data, .length = sizeof(pw_data), }; - size_t iv_size = gnutls_cipher_get_iv_size(GNUTLS_CIPHER_AES_256_CBC); - uint8_t iv_data[iv_size]; - DATA_BLOB iv = { - .data = iv_data, - .length = iv_size, - }; DATA_BLOB ciphertext = data_blob_null; NTSTATUS status = NT_STATUS_UNSUCCESSFUL; bool ok; @@ -101,8 +96,6 @@ NTSTATUS init_samr_CryptPasswordAES(TALLOC_CTX *mem_ctx, return NT_STATUS_INVALID_PARAMETER; } - generate_nonce_buffer(iv.data, iv.length); - ok = encode_pwd_buffer514_from_str(pw_data, password, STR_UNICODE); if (!ok) { return NT_STATUS_INTERNAL_ERROR; @@ -114,7 +107,7 @@ NTSTATUS init_samr_CryptPasswordAES(TALLOC_CTX *mem_ctx, session_key, &samr_aes256_enc_key_salt, &samr_aes256_mac_key_salt, - &iv, + salt, &ciphertext, ppwd_buf->auth_data); BURN_DATA(pw_data); @@ -126,8 +119,8 @@ NTSTATUS init_samr_CryptPasswordAES(TALLOC_CTX *mem_ctx, ppwd_buf->cipher = ciphertext.data; ppwd_buf->PBKDF2Iterations = 0; - SMB_ASSERT(iv.length == sizeof(ppwd_buf->salt)); - memcpy(ppwd_buf->salt, iv.data, iv.length); + SMB_ASSERT(salt->length == sizeof(ppwd_buf->salt)); + memcpy(ppwd_buf->salt, salt->data, salt->length); return NT_STATUS_OK; } diff --git a/source3/rpc_client/init_samr.h b/source3/rpc_client/init_samr.h index 940534e7168..71b4c0e573d 100644 --- a/source3/rpc_client/init_samr.h +++ b/source3/rpc_client/init_samr.h @@ -47,6 +47,7 @@ NTSTATUS init_samr_CryptPassword(const char *pwd, */ NTSTATUS init_samr_CryptPasswordAES(TALLOC_CTX *mem_ctx, const char *password, + DATA_BLOB *salt, DATA_BLOB *session_key, struct samr_EncryptedPasswordAES *ppwd_buf); diff --git a/source3/rpcclient/cmd_samr.c b/source3/rpcclient/cmd_samr.c index 9ccd2f78a8d..8106ca90cf2 100644 --- a/source3/rpcclient/cmd_samr.c +++ b/source3/rpcclient/cmd_samr.c @@ -3172,6 +3172,11 @@ static NTSTATUS cmd_samr_setuserinfo_int(struct rpc_pipe_client *cli, uint8_t nt_hash[16]; uint8_t lm_hash[16]; DATA_BLOB session_key; + uint8_t salt_data[16]; + DATA_BLOB salt = { + .data = salt_data, + .length = sizeof(salt_data), + }; uint8_t password_expired = 0; struct dcerpc_binding_handle *b = cli->binding_handle; TALLOC_CTX *frame = NULL; @@ -3198,6 +3203,8 @@ static NTSTATUS cmd_samr_setuserinfo_int(struct rpc_pipe_client *cli, goto done; } + generate_nonce_buffer(salt.data, salt.length); + switch(level) { case 18: case 21: @@ -3220,6 +3227,7 @@ static NTSTATUS cmd_samr_setuserinfo_int(struct rpc_pipe_client *cli, case 31: status = init_samr_CryptPasswordAES(frame, param, + &salt, &session_key, &pwd_buf_aes); if (!NT_STATUS_IS_OK(status)) { diff --git a/source4/libnet/libnet_passwd.c b/source4/libnet/libnet_passwd.c index 4f662110e55..a1672104824 100644 --- a/source4/libnet/libnet_passwd.c +++ b/source4/libnet/libnet_passwd.c @@ -57,13 +57,13 @@ static NTSTATUS libnet_ChangePassword_samr_aes(TALLOC_CTX *mem_ctx, struct samr_EncryptedPasswordAES pwd_buf = { .cipher_len = 0 }; - DATA_BLOB iv = { + DATA_BLOB salt = { .data = pwd_buf.salt, .length = sizeof(pwd_buf.salt), }; - gnutls_datum_t iv_datum = { - .data = iv.data, - .size = iv.length, + gnutls_datum_t salt_datum = { + .data = pwd_buf.salt, + .size = sizeof(pwd_buf.salt), }; uint64_t pbkdf2_iterations = generate_random_u64_range(5000, 1000000); NTSTATUS status; @@ -71,11 +71,11 @@ static NTSTATUS libnet_ChangePassword_samr_aes(TALLOC_CTX *mem_ctx, E_md4hash(old_password, old_nt_key_data); - generate_nonce_buffer(iv.data, iv.length); + generate_nonce_buffer(salt.data, salt.length); rc = gnutls_pbkdf2(GNUTLS_MAC_SHA512, &old_nt_key, - &iv_datum, + &salt_datum, pbkdf2_iterations, cek.data, cek.length); @@ -86,6 +86,7 @@ static NTSTATUS libnet_ChangePassword_samr_aes(TALLOC_CTX *mem_ctx, status = init_samr_CryptPasswordAES(mem_ctx, new_password, + &salt, &cek, &pwd_buf); data_blob_clear(&cek); diff --git a/source4/torture/rpc/samr.c b/source4/torture/rpc/samr.c index de354659067..0b1880efa18 100644 --- a/source4/torture/rpc/samr.c +++ b/source4/torture/rpc/samr.c @@ -783,6 +783,11 @@ static bool test_SetUserPass_32(struct dcerpc_pipe *p, struct torture_context *t struct samr_SetUserInfo s; union samr_UserInfo u; DATA_BLOB session_key; + uint8_t salt_data[16]; + DATA_BLOB salt = { + .data = salt_data, + .length = sizeof(salt_data), + }; char *newpass = NULL; struct dcerpc_binding_handle *b = p->binding_handle; struct samr_GetUserPwInfo pwp; @@ -818,8 +823,11 @@ static bool test_SetUserPass_32(struct dcerpc_pipe *p, struct torture_context *t return false; } + generate_nonce_buffer(salt.data, salt.length); + status = init_samr_CryptPasswordAES(tctx, newpass, + &salt, &session_key, &u.info32.password); torture_assert_ntstatus_ok(tctx, @@ -852,6 +860,7 @@ static bool test_SetUserPass_32(struct dcerpc_pipe *p, struct torture_context *t status = init_samr_CryptPasswordAES(tctx, newpass, + &salt, &session_key, &u.info32.password); torture_assert_ntstatus_ok(tctx, @@ -896,6 +905,11 @@ static bool test_SetUserPass_31(struct dcerpc_pipe *p, struct torture_context *t union samr_UserInfo u; bool ret = true; DATA_BLOB session_key; + uint8_t salt_data[16]; + DATA_BLOB salt = { + .data = salt_data, + .length = sizeof(salt_data), + }; char *newpass; struct dcerpc_binding_handle *b = p->binding_handle; struct samr_GetUserPwInfo pwp; @@ -931,8 +945,11 @@ static bool test_SetUserPass_31(struct dcerpc_pipe *p, struct torture_context *t return false; } + generate_nonce_buffer(salt.data, salt.length); + status = init_samr_CryptPasswordAES(tctx, newpass, + &salt, &session_key, &u.info31.password); torture_assert_ntstatus_ok(tctx, @@ -959,6 +976,7 @@ static bool test_SetUserPass_31(struct dcerpc_pipe *p, struct torture_context *t status = init_samr_CryptPasswordAES(tctx, newpass, + &salt, &session_key, &u.info31.password); torture_assert_ntstatus_ok(tctx, @@ -1381,6 +1399,11 @@ static bool test_SetUserPass_level_ex(struct dcerpc_pipe *p, union samr_UserInfo u; bool ret = true; DATA_BLOB session_key; + uint8_t salt_data[16]; + DATA_BLOB salt = { + .data = salt_data, + .length = sizeof(salt_data), + }; char *newpass; struct dcerpc_binding_handle *b = p->binding_handle; struct samr_GetUserPwInfo pwp; @@ -1490,6 +1513,8 @@ static bool test_SetUserPass_level_ex(struct dcerpc_pipe *p, return false; } + generate_nonce_buffer(salt.data, salt.length); + switch (level) { case 18: { @@ -1561,6 +1586,7 @@ static bool test_SetUserPass_level_ex(struct dcerpc_pipe *p, case 31: status = init_samr_CryptPasswordAES(tctx, newpass, + &salt, &session_key, &u.info31.password); @@ -1568,6 +1594,7 @@ static bool test_SetUserPass_level_ex(struct dcerpc_pipe *p, case 32: status = init_samr_CryptPasswordAES(tctx, newpass, + &salt, &session_key, &u.info32.password); -- 2.37.3 From 1d630363c9b2497266e418aad89c55d5b51a63ad Mon Sep 17 00:00:00 2001 From: Andreas Schneider Date: Mon, 17 Oct 2022 09:02:28 +0200 Subject: [PATCH 3/5] s4:libnet: If we successfully changed the password we are done BUG: https://bugzilla.samba.org/show_bug.cgi?id=15206 Signed-off-by: Andreas Schneider Reviewed-by: Noel Power --- source4/libnet/libnet_passwd.c | 32 ++++++++++++++++++-------------- 1 file changed, 18 insertions(+), 14 deletions(-) diff --git a/source4/libnet/libnet_passwd.c b/source4/libnet/libnet_passwd.c index a1672104824..b17614bcd97 100644 --- a/source4/libnet/libnet_passwd.c +++ b/source4/libnet/libnet_passwd.c @@ -101,7 +101,7 @@ static NTSTATUS libnet_ChangePassword_samr_aes(TALLOC_CTX *mem_ctx, r.in.password = &pwd_buf; status = dcerpc_samr_ChangePasswordUser4_r(h, mem_ctx, &r); - if (NT_STATUS_IS_OK(status)) { + if (!NT_STATUS_IS_OK(status)) { goto done; } if (!NT_STATUS_IS_OK(r.out.result)) { @@ -112,6 +112,7 @@ static NTSTATUS libnet_ChangePassword_samr_aes(TALLOC_CTX *mem_ctx, account->string, nt_errstr(status)); status = r.out.result; + goto done; } done: @@ -424,20 +425,23 @@ static NTSTATUS libnet_ChangePassword_samr(struct libnet_context *ctx, TALLOC_CT r->samr.in.oldpassword, r->samr.in.newpassword, &(r->samr.out.error_string)); - if (!NT_STATUS_IS_OK(status)) { - if (NT_STATUS_EQUAL(status, - NT_STATUS_RPC_PROCNUM_OUT_OF_RANGE) || - NT_STATUS_EQUAL(status, NT_STATUS_NOT_SUPPORTED) || - NT_STATUS_EQUAL(status, NT_STATUS_NOT_IMPLEMENTED)) { - /* - * Don't fallback to RC4 based SAMR if weak crypto is not - * allowed. - */ - if (lpcfg_weak_crypto(ctx->lp_ctx) == - SAMBA_WEAK_CRYPTO_DISALLOWED) { - goto disconnect; - } + if (NT_STATUS_IS_OK(status)) { + goto disconnect; + } else if (NT_STATUS_EQUAL(status, + NT_STATUS_RPC_PROCNUM_OUT_OF_RANGE) || + NT_STATUS_EQUAL(status, NT_STATUS_NOT_SUPPORTED) || + NT_STATUS_EQUAL(status, NT_STATUS_NOT_IMPLEMENTED)) { + /* + * Don't fallback to RC4 based SAMR if weak crypto is not + * allowed. + */ + if (lpcfg_weak_crypto(ctx->lp_ctx) == + SAMBA_WEAK_CRYPTO_DISALLOWED) { + goto disconnect; } + } else { + /* libnet_ChangePassword_samr_aes is implemented and failed */ + goto disconnect; } status = libnet_ChangePassword_samr_rc4( -- 2.37.3 From 9a4a169ab34641afb87e7f81708c9a72b321879e Mon Sep 17 00:00:00 2001 From: Noel Power Date: Fri, 21 Oct 2022 17:40:36 +0100 Subject: [PATCH 4/5] s4/rpc_server/sambr: don't mutate the return of samdb_set_password_aes prior to this commit return of samdb_set_password_aes was set to NT_STATUS_WRONG_PASSWORD on failure. Useful status that should be returned such as NT_STATUS_PASSWORD_RESTRICTION are swallowed here otherwise (and in this case can be partially responsible for failures in test samba.tests.auth_log_pass_change (with later gnutls) Signed-off-by: Noel Power Reviewed-by: Andreas Schneider --- source4/rpc_server/samr/samr_password.c | 1 - 1 file changed, 1 deletion(-) diff --git a/source4/rpc_server/samr/samr_password.c b/source4/rpc_server/samr/samr_password.c index 4691f9a47a9..b581be6361c 100644 --- a/source4/rpc_server/samr/samr_password.c +++ b/source4/rpc_server/samr/samr_password.c @@ -250,7 +250,6 @@ NTSTATUS dcesrv_samr_ChangePasswordUser4(struct dcesrv_call_state *dce_call, if (!NT_STATUS_IS_OK(status)) { ldb_transaction_cancel(sam_ctx); - status = NT_STATUS_WRONG_PASSWORD; goto done; } -- 2.37.3 From b8b36ecba0f22dbc203c12627ebd629c2437c635 Mon Sep 17 00:00:00 2001 From: Noel Power Date: Fri, 21 Oct 2022 17:14:44 +0100 Subject: [PATCH 5/5] python/samba/tests: fix samba.tests.auth_log_pass_change for later gnutls later gnutls that support GNUTLS_PBKDF2 currently fail, we need to conditionally switch test data to reflect use of 'samr_ChangePasswordUser3' or 'samr_ChangePasswordUser4' depending on whether GNUTLS_PBKDF2 is supported or not Signed-off-by: Noel Power Reviewed-by: Andreas Schneider --- python/samba/tests/auth_log_pass_change.py | 20 ++++++++++++++++---- source4/selftest/tests.py | 9 ++++++--- 2 files changed, 22 insertions(+), 7 deletions(-) diff --git a/python/samba/tests/auth_log_pass_change.py b/python/samba/tests/auth_log_pass_change.py index 972af2158dd..1ca46c586b3 100644 --- a/python/samba/tests/auth_log_pass_change.py +++ b/python/samba/tests/auth_log_pass_change.py @@ -72,6 +72,18 @@ class AuthLogPassChangeTests(samba.tests.auth_log_base.AuthLogTestBase): # discard any auth log messages for the password setup self.discardMessages() + gnutls_pbkdf2_support = samba.tests.env_get_var_value( + 'GNUTLS_PBKDF2_SUPPORT', + allow_missing=True) + if gnutls_pbkdf2_support is None: + gnutls_pbkdf2_support = '0' + self.gnutls_pbkdf2_support = bool(int(gnutls_pbkdf2_support)) + + def _authDescription(self): + if self.gnutls_pbkdf2_support: + return "samr_ChangePasswordUser4" + else: + return "samr_ChangePasswordUser3" def tearDown(self): super(AuthLogPassChangeTests, self).tearDown() @@ -83,7 +95,7 @@ class AuthLogPassChangeTests(samba.tests.auth_log_base.AuthLogTestBase): (msg["Authentication"]["serviceDescription"] == "SAMR Password Change") and (msg["Authentication"]["authDescription"] == - "samr_ChangePasswordUser3") and + self._authDescription()) and (msg["Authentication"]["eventId"] == EVT_ID_SUCCESSFUL_LOGON) and (msg["Authentication"]["logonType"] == @@ -109,7 +121,7 @@ class AuthLogPassChangeTests(samba.tests.auth_log_base.AuthLogTestBase): (msg["Authentication"]["serviceDescription"] == "SAMR Password Change") and (msg["Authentication"]["authDescription"] == - "samr_ChangePasswordUser3") and + self._authDescription()) and (msg["Authentication"]["eventId"] == EVT_ID_UNSUCCESSFUL_LOGON) and (msg["Authentication"]["logonType"] == @@ -141,7 +153,7 @@ class AuthLogPassChangeTests(samba.tests.auth_log_base.AuthLogTestBase): (msg["Authentication"]["serviceDescription"] == "SAMR Password Change") and (msg["Authentication"]["authDescription"] == - "samr_ChangePasswordUser3") and + self._authDescription()) and (msg["Authentication"]["eventId"] == EVT_ID_UNSUCCESSFUL_LOGON) and (msg["Authentication"]["logonType"] == @@ -174,7 +186,7 @@ class AuthLogPassChangeTests(samba.tests.auth_log_base.AuthLogTestBase): (msg["Authentication"]["serviceDescription"] == "SAMR Password Change") and (msg["Authentication"]["authDescription"] == - "samr_ChangePasswordUser3") and + self._authDescription()) and (msg["Authentication"]["eventId"] == EVT_ID_UNSUCCESSFUL_LOGON) and (msg["Authentication"]["logonType"] == diff --git a/source4/selftest/tests.py b/source4/selftest/tests.py index a803d4704ea..c92105586a7 100755 --- a/source4/selftest/tests.py +++ b/source4/selftest/tests.py @@ -1094,9 +1094,11 @@ if have_heimdal_support: environ={'CLIENT_IP': '10.53.57.11', 'SOCKET_WRAPPER_DEFAULT_IFACE': 11}) planoldpythontestsuite("ad_dc_smb1", "samba.tests.auth_log_pass_change", - extra_args=['-U"$USERNAME%$PASSWORD"']) + extra_args=['-U"$USERNAME%$PASSWORD"'], + environ={'GNUTLS_PBKDF2_SUPPORT': gnutls_pbkdf2_support}) planoldpythontestsuite("ad_dc_ntvfs", "samba.tests.auth_log_pass_change", - extra_args=['-U"$USERNAME%$PASSWORD"']) + extra_args=['-U"$USERNAME%$PASSWORD"'], + environ={'GNUTLS_PBKDF2_SUPPORT': gnutls_pbkdf2_support}) # these tests use a NCA local RPC connection, so always run on the # :local testenv, and so don't need to fake a client connection @@ -1113,7 +1115,8 @@ if have_heimdal_support: "samba.tests.auth_log_winbind", extra_args=['-U"$DC_USERNAME%$DC_PASSWORD"']) planoldpythontestsuite("ad_dc", "samba.tests.audit_log_pass_change", - extra_args=['-U"$USERNAME%$PASSWORD"']) + extra_args=['-U"$USERNAME%$PASSWORD"'], + environ={'GNUTLS_PBKDF2_SUPPORT': gnutls_pbkdf2_support}) planoldpythontestsuite("ad_dc", "samba.tests.audit_log_dsdb", extra_args=['-U"$USERNAME%$PASSWORD"']) planoldpythontestsuite("ad_dc", "samba.tests.group_audit", -- 2.37.3