From 5aa79e3263979e0251925bc666c32f31132c370b Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Tue, 26 Nov 2024 11:10:16 +0100 Subject: [PATCH] s4:rpc_server/netlogon: fix dcesrv_netr_ServerPasswordSet[2] for ServerAuthenticateKerberos Review with: git show --patience Signed-off-by: Stefan Metzmacher Reviewed-by: Andreas Schneider --- .../knownfail.d/samba.tests.krb5.netlogon | 2 - .../samba4.rpc.netlogon.netlogon.SetPassword2 | 4 -- source4/rpc_server/netlogon/dcerpc_netlogon.c | 46 +++++++++++++++---- 3 files changed, 37 insertions(+), 15 deletions(-) delete mode 100644 selftest/knownfail.d/samba4.rpc.netlogon.netlogon.SetPassword2 diff --git a/selftest/knownfail.d/samba.tests.krb5.netlogon b/selftest/knownfail.d/samba.tests.krb5.netlogon index 2f64171e5e2..a59934805b4 100644 --- a/selftest/knownfail.d/samba.tests.krb5.netlogon +++ b/selftest/knownfail.d/samba.tests.krb5.netlogon @@ -1,6 +1,4 @@ # This is not implemented yet ^samba.tests.krb5.netlogon.*.NetlogonSchannel.test_ticket_samlogon # These will be fixed in the next commits -^samba.tests.krb5.netlogon.*.NetlogonSchannel.test_check_passwords_.*_auth3_e13fffff -^samba.tests.krb5.netlogon.*.NetlogonSchannel.test_check_passwords_.*_authK ^samba.tests.krb5.netlogon.*.NetlogonSchannel.test_.*_samlogon_.*_authK diff --git a/selftest/knownfail.d/samba4.rpc.netlogon.netlogon.SetPassword2 b/selftest/knownfail.d/samba4.rpc.netlogon.netlogon.SetPassword2 deleted file mode 100644 index 43211a64223..00000000000 --- a/selftest/knownfail.d/samba4.rpc.netlogon.netlogon.SetPassword2 +++ /dev/null @@ -1,4 +0,0 @@ -# The ad_dc_ntvfs allows negotiating des crypto -# it means ServerPasswordSet2 don't use crypto at all -# We'll adjust the server in the next commits -^samba4.rpc.netlogon.*.netlogon.SetPassword2.*.ad_dc_ntvfs diff --git a/source4/rpc_server/netlogon/dcerpc_netlogon.c b/source4/rpc_server/netlogon/dcerpc_netlogon.c index 91f7a655d9e..0c36ad6be20 100644 --- a/source4/rpc_server/netlogon/dcerpc_netlogon.c +++ b/source4/rpc_server/netlogon/dcerpc_netlogon.c @@ -1101,6 +1101,10 @@ static NTSTATUS dcesrv_netr_ServerPasswordSet(struct dcesrv_call_state *dce_call return NT_STATUS_INVALID_SYSTEM_SERVICE; } + if (creds->negotiate_flags & NETLOGON_NEG_SUPPORTS_KERBEROS_AUTH) { + return NT_STATUS_NOT_SUPPORTED; + } + nt_status = netlogon_creds_decrypt_samr_Password(creds, r->in.new_password, auth_type, @@ -1190,9 +1194,40 @@ static NTSTATUS dcesrv_netr_ServerPasswordSet2(struct dcesrv_call_state *dce_cal if (!extract_pw_from_buffer(mem_ctx, password_buf.data, &new_password)) { DEBUG(3,("samr: failed to decode password buffer\n")); + return NT_STATUS_ACCESS_DENIED; + } + + /* + * We don't allow empty passwords for machine accounts. + */ + if (new_password.length < 2) { + DBG_WARNING("Empty password Length[%zu]\n", + new_password.length); return NT_STATUS_WRONG_PASSWORD; } + if (creds->negotiate_flags & NETLOGON_NEG_SUPPORTS_KERBEROS_AUTH) { + /* + * netlogon_creds_decrypt_samr_CryptPassword + * already checked for DCERPC_AUTH_LEVEL_PRIVACY + */ + goto checked_encryption; + } else if (creds->negotiate_flags & NETLOGON_NEG_SUPPORTS_AES) { + /* + * check it's encrypted + */ + } else if (creds->negotiate_flags & NETLOGON_NEG_ARCFOUR) { + /* + * check it's encrypted + */ + } else { + /* + * netlogon_creds_decrypt_samr_CryptPassword + * already checked for DCERPC_AUTH_LEVEL_PRIVACY + */ + goto checked_encryption; + } + /* * Make sure the length field was encrypted, * otherwise we are under attack. @@ -1203,15 +1238,6 @@ static NTSTATUS dcesrv_netr_ServerPasswordSet2(struct dcesrv_call_state *dce_cal return NT_STATUS_WRONG_PASSWORD; } - /* - * We don't allow empty passwords for machine accounts. - */ - if (new_password.length < 2) { - DBG_WARNING("Empty password Length[%zu]\n", - new_password.length); - return NT_STATUS_WRONG_PASSWORD; - } - /* * Make sure the confounder part of CryptPassword * buffer was encrypted, otherwise we are under attack. @@ -1239,6 +1265,8 @@ static NTSTATUS dcesrv_netr_ServerPasswordSet2(struct dcesrv_call_state *dce_cal return NT_STATUS_WRONG_PASSWORD; } +checked_encryption: + /* * don't allow zero buffers */