From fc49a2d4ee3e355ba276c811261fd7bb318be7aa Mon Sep 17 00:00:00 2001 From: Joseph Sutton Date: Tue, 15 Mar 2022 15:34:34 +1300 Subject: [PATCH 1/2] s4-kdc: Handle previously unhandled auth event types Cases to handle KDC_AUTH_EVENT_VALIDATED_LONG_TERM_KEY and KDC_AUTH_EVENT_PREAUTH_SUCCEEDED were removed in: commit 791be84c3eecb95e03611458e2305bae272ba267 Author: Stefan Metzmacher Date: Wed Mar 2 10:10:08 2022 +1300 s4:kdc: hdb_samba4_audit() is only called once per request Normally these auth event types are overwritten with the KDC_AUTH_EVENT_CLIENT_AUTHORIZED event type, but if a client passes the pre-authentication check, and happens to fail the client access check (e.g. because the account is disabled), we get error messages of the form: hdb_samba4_audit: Unhandled hdb_auth_status=9 => INTERNAL_ERROR To avoid such errors, use the error code provided in the request structure to obtain a relevant status code in cases not handled explicitly. For unexpected values we return KRB5KRB_ERR_GENERIC in order to hopefully prevent success. And within make test we panic in order let a ci run fail. BUG: https://bugzilla.samba.org/show_bug.cgi?id=15015 Pair-Programmed-With: Stefan Metzmacher Signed-off-by: Joseph Sutton Signed-off-by: Stefan Metzmacher Reviewed-by: Andrew Bartlett (cherry picked from commit b01388da8a72c11c46bb27e773b354520bc6ac88) --- source4/kdc/hdb-samba4.c | 43 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 43 insertions(+) diff --git a/source4/kdc/hdb-samba4.c b/source4/kdc/hdb-samba4.c index 96e88423528..47b98a7fa8c 100644 --- a/source4/kdc/hdb-samba4.c +++ b/source4/kdc/hdb-samba4.c @@ -613,7 +613,40 @@ static krb5_error_code hdb_samba4_audit(krb5_context context, ui.auth_description = auth_description; if (hdb_auth_status == KDC_AUTH_EVENT_CLIENT_AUTHORIZED) { + /* 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, + * but we didn't reach the final + * KDC_AUTH_EVENT_CLIENT_AUTHORIZED, + * so consult the error code. + */ + if (r->error_code == 0) { + DBG_ERR("ERROR: VALIDATED_LONG_TERM_KEY " + "with error=0 => INTERNAL_ERROR\n"); + status = NT_STATUS_INTERNAL_ERROR; + final_ret = KRB5KRB_ERR_GENERIC; + r->error_code = final_ret; + } else { + status = krb5_to_nt_status(r->error_code); + } + } else if (hdb_auth_status == KDC_AUTH_EVENT_PREAUTH_SUCCEEDED) { + /* + * This was only a pre-authentication success, + * but we didn't reach the final + * KDC_AUTH_EVENT_CLIENT_AUTHORIZED, + * so consult the error code. + */ + if (r->error_code == 0) { + DBG_ERR("ERROR: PREAUTH_SUCCEEDED " + "with error=0 => INTERNAL_ERROR\n"); + status = NT_STATUS_INTERNAL_ERROR; + final_ret = KRB5KRB_ERR_GENERIC; + r->error_code = final_ret; + } else { + status = krb5_to_nt_status(r->error_code); + } } 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) { @@ -641,6 +674,8 @@ static krb5_error_code hdb_samba4_audit(krb5_context context, DBG_ERR("Unhandled hdb_auth_status=%d => INTERNAL_ERROR\n", hdb_auth_status); status = NT_STATUS_INTERNAL_ERROR; + final_ret = KRB5KRB_ERR_GENERIC; + r->error_code = final_ret; } if (rwdc_fallback) { @@ -665,6 +700,14 @@ static krb5_error_code hdb_samba4_audit(krb5_context context, domain_name, account_name, sid); + if (final_ret == KRB5KRB_ERR_GENERIC && socket_wrapper_enabled()) { + /* + * If we're running under make test + * just panic + */ + DBG_ERR("Unexpected situation => PANIC\n"); + smb_panic("hdb_samba4_audit: Unexpected situation"); + } TALLOC_FREE(frame); break; } -- 2.25.1 From ccd42b6d2bc00593c7ee62fc92529e4f36974e6f Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Wed, 16 Mar 2022 09:21:03 +0100 Subject: [PATCH 2/2] s4:kdc: tunnel the check_client_access status to hdb_samba4_audit() Otherwise useful information gets lost while converting from NTSTATUS to krb5_error and back to NTSTATUS again. E.g. NT_STATUS_ACCOUNT_DISABLED would be audited as NT_STATUS_ACCOUNT_LOCKED_OUT. BUG: https://bugzilla.samba.org/show_bug.cgi?id=15015 Signed-off-by: Stefan Metzmacher Reviewed-by: Andrew Bartlett (cherry picked from commit 5294dc80090482d5669126802672eb2c89e269cf) --- source4/kdc/hdb-samba4.c | 4 ++++ source4/kdc/pac-glue.c | 1 + source4/kdc/samba_kdc.h | 1 + 3 files changed, 6 insertions(+) diff --git a/source4/kdc/hdb-samba4.c b/source4/kdc/hdb-samba4.c index 47b98a7fa8c..ae0da3c350c 100644 --- a/source4/kdc/hdb-samba4.c +++ b/source4/kdc/hdb-samba4.c @@ -628,6 +628,8 @@ static krb5_error_code hdb_samba4_audit(krb5_context context, status = NT_STATUS_INTERNAL_ERROR; final_ret = KRB5KRB_ERR_GENERIC; r->error_code = final_ret; + } else if (!NT_STATUS_IS_OK(p->reject_status)) { + status = p->reject_status; } else { status = krb5_to_nt_status(r->error_code); } @@ -644,6 +646,8 @@ static krb5_error_code hdb_samba4_audit(krb5_context context, status = NT_STATUS_INTERNAL_ERROR; final_ret = KRB5KRB_ERR_GENERIC; r->error_code = final_ret; + } else if (!NT_STATUS_IS_OK(p->reject_status)) { + status = p->reject_status; } else { status = krb5_to_nt_status(r->error_code); } diff --git a/source4/kdc/pac-glue.c b/source4/kdc/pac-glue.c index dc6db122865..f0181d2e676 100644 --- a/source4/kdc/pac-glue.c +++ b/source4/kdc/pac-glue.c @@ -1143,6 +1143,7 @@ NTSTATUS samba_kdc_check_client_access(struct samba_kdc_entry *kdc_entry, workstation, client_name, true, password_change); + kdc_entry->reject_status = nt_status; talloc_free(tmp_ctx); return nt_status; } diff --git a/source4/kdc/samba_kdc.h b/source4/kdc/samba_kdc.h index a354f3e8db3..9b16fcc3b92 100644 --- a/source4/kdc/samba_kdc.h +++ b/source4/kdc/samba_kdc.h @@ -61,6 +61,7 @@ struct samba_kdc_entry { bool is_trust; void *entry_ex; uint32_t supported_enctypes; + NTSTATUS reject_status; }; extern struct hdb_method hdb_samba4_interface; -- 2.25.1